updated output format for reports

docs/diff.md

@@ -105,9 +105,11 @@ sh noverify.sh
 The linter will now find a new error:
 
 ```
-<critical> WARNING unused: Variable $a is unused (use $_ to ignore this inspection or specify --unused-var-regex flag) at /Users/petrmakhnev/swiftmailer/test_file.php:4
-  $a = 100;
-  ^^
+<critical> WARNING unused: Variable $a is unused at /Users/petrmakhnev/swiftmailer/test_file.php:4
+    |
+106 |  $a = 100;
+    |  ^^ use `$_` to ignore (see --unused-var-regex flag)
+
 ```
 
 If we create a commit:

docs/getting_started.md

@@ -148,9 +148,10 @@ noverify check --unused-var-regex='^null$|^e$' --allow-checks='unused' ./lib
 Then only a single place will be found where the declared variable is not really used.
 
 ```
-<critical> WARNING unused: Variable $name is unused (use $_ to ignore this inspection or specify --unused-var-regex flag) at swiftmailer/lib/classes/Swift/Mailer.php:73
-            foreach ($message->getTo() as $address => $name) {
-                                                      ^^^^^
+<critical> WARNING unused: Variable $name is unused at swiftmailer/lib/classes/Swift/Mailer.php:73
+    |
+106 |     foreach ($message->getTo() as $address => $name) {}
+    |                                               ^^^^^ use `$_` to ignore (see --unused-var-regex flag)
 ```
 
 In order to fix it, it is enough to rename the variable to `$null`.

src/cmd/main.go

@@ -10,6 +10,7 @@ import (
 	"path/filepath"
 	"runtime"
 	"runtime/pprof"
+	"strconv"
 	"strings"
 	"sync/atomic"
 	"time"
@@ -336,8 +337,19 @@ func FormatReport(r *linter.Report) string {
 		cursor.WriteString(strings.Repeat("^", r.EndChar-r.StartChar))
 	}
 
-	return fmt.Sprintf("%-7s %s at %s:%d\n%s\n%s",
-		r.Severity(), msg, r.Filename, r.Line, r.Context, cursor.String())
+	return fmt.Sprintf("%-7s %s at %s:%d\n%s",
+		r.Severity(), msg, r.Filename, r.Line, formatContext(r, cursor))
+}
+
+func formatContext(r *linter.Report, cursor strings.Builder) string {
+	line := r.Line
+	width := len(strconv.Itoa(line))
+
+	context := fmt.Sprintf(`%[1]s |
+%[2]d |%[3]s
+%[1]s |%[4]s %[5]s
+`, strings.Repeat(" ", width), r.Line, r.Context, cursor.String(), r.Hint)
+	return context
 }
 
 type ReportsStat struct {
@@ -389,22 +401,24 @@ func processReports(runner *LinterRunner, cfg *MainConfig, diff []*linter.Report
 			// Should never fail to marshal our own reports.
 			panic(fmt.Sprintf("report list marshaling failed: %v", err))
 		}
-	} else {
-		for _, report := range filtered {
-			format := ""
 
-			if runner.checkersFilter.IsCriticalReport(report) {
-				format += "<critical> "
-			}
+		return stat
+	}
 
-			if runner.config.Checkers.Autofixable(report.CheckName) {
-				format += "<autofixable> "
-			}
+	for _, report := range filtered {
+		format := ""
 
-			format += "%s\n"
+		if runner.checkersFilter.IsCriticalReport(report) {
+			format += "<critical> "
+		}
 
-			fmt.Fprintf(runner.outputFp, format, FormatReport(report))
+		if runner.config.Checkers.Autofixable(report.CheckName) {
+			format += "<autofixable> "
 		}
+
+		format += "%s\n"
+
+		fmt.Fprintf(runner.outputFp, format, FormatReport(report))
 	}
 
 	return stat

src/linter/block.go

@@ -133,6 +133,14 @@ func (b *blockWalker) report(n ir.Node, level int, checkName, msg string, args .
 	b.r.Report(n, level, checkName, msg, args...)
 }
 
+func (b *blockWalker) reportWithHint(n ir.Node, level int, checkName, msg, hint string, args ...interface{}) {
+	if b.isSuppressed(n, checkName) {
+		return
+	}
+
+	b.r.ReportWithHint(n, level, checkName, msg, hint, args...)
+}
+
 func (b *blockWalker) isSuppressed(n ir.Node, checkName string) bool {
 	if containLinterSuppress(n, checkName) {
 		return true
@@ -1223,7 +1231,8 @@ func (b *blockWalker) handleForeach(s *ir.ForeachStmt) bool {
 
 		b.untrackVarName(key.Name)
 
-		b.report(s.Key, LevelWarning, "unused", "Foreach key $%s is unused, can simplify $%s => $%s to just $%s", key.Name, key.Name, variable.Name, variable.Name)
+		hint := fmt.Sprintf("can simplify $%s => $%s to just $%s", key.Name, variable.Name, variable.Name)
+		b.reportWithHint(s.Key, LevelWarning, "unused", `Foreach key $%s is unused`, hint, key.Name)
 	}
 
 	return false
@@ -2256,7 +2265,7 @@ func (b *blockWalker) flushUnused() {
 			}
 
 			visitedMap[n] = struct{}{}
-			b.report(n, LevelWarning, "unused", `Variable $%s is unused (use $_ to ignore this inspection or specify --unused-var-regex flag)`, name)
+			b.reportWithHint(n, LevelWarning, "unused", `Variable $%s is unused`, "use `$_` to ignore (see --unused-var-regex flag)", name)
 		}
 	}
 }

src/linter/report.go

@@ -1142,6 +1142,7 @@ type Report struct {
 	CheckName string `json:"check_name"`
 	Level     int    `json:"level"`
 	Context   string `json:"context"`
+	Hint      string `json:"hint"`
 	Message   string `json:"message"`
 	Filename  string `json:"filename"`
 	Line      int    `json:"line"`

src/linter/root.go

@@ -1509,7 +1509,15 @@ func (d *rootWalker) ReportPHPDoc(phpDocLocation PHPDocLocation, level int, chec
 	d.ReportLocation(loc, level, checkName, msg, args...)
 }
 
+func (d *rootWalker) ReportWithHint(n ir.Node, level int, checkName, msg, hint string, args ...interface{}) {
+	d.report(n, level, checkName, msg, hint, args...)
+}
+
 func (d *rootWalker) Report(n ir.Node, level int, checkName, msg string, args ...interface{}) {
+	d.report(n, level, checkName, msg, "", args...)
+}
+
+func (d *rootWalker) report(n ir.Node, level int, checkName, msg, hint string, args ...interface{}) {
 	var pos position.Position
 
 	if n == nil {
@@ -1556,10 +1564,14 @@ func (d *rootWalker) Report(n ir.Node, level int, checkName, msg string, args ..
 		}
 	}
 
-	d.ReportLocation(loc, level, checkName, msg, args...)
+	d.reportLocation(loc, level, checkName, msg, hint, args...)
 }
 
 func (d *rootWalker) ReportLocation(loc ir.Location, level int, checkName, msg string, args ...interface{}) {
+	d.reportLocation(loc, level, checkName, msg, "", args...)
+}
+
+func (d *rootWalker) reportLocation(loc ir.Location, level int, checkName, msg, hint string, args ...interface{}) {
 	if !d.metaInfo().IsIndexingComplete() {
 		return
 	}
@@ -1606,6 +1618,7 @@ func (d *rootWalker) ReportLocation(loc ir.Location, level int, checkName, msg s
 		Level:     level,
 		Filename:  strings.ReplaceAll(d.ctx.st.CurrentFile, "\\", "/"), // To make output stable between platforms, see #572
 		Message:   fmt.Sprintf(msg, args...),
+		Hint:      hint,
 		Hash:      hash,
 	})
 }

src/linttest/golden_linttest.go

@@ -1,6 +1,7 @@
 package linttest
 
 import (
+	"bytes"
 	"encoding/json"
 	"fmt"
 	"io/ioutil"
@@ -19,6 +20,9 @@ import (
 	"github.com/VKCOM/noverify/src/utils"
 )
 
+// whenever dump expected output to passed golden file
+const dumpExpected = false
+
 type GoldenTestSuite struct {
 	suite *Suite
 
@@ -34,8 +38,12 @@ type GoldenTestSuite struct {
 	BaseDir        string
 	GoldenFileName string
 
+	// dump expected output to passed golden file
+	dump bool
+
 	// want is a golden file contents.
 	want       []byte
+	path       string
 	reportFile *linterOutput
 
 	// flag indicating that the structure is ready for use.
@@ -85,6 +93,8 @@ func PrepareGoldenTestSuite(s *GoldenTestSuite, t *testing.T, l *linter.Linter,
 	s.prepared = true
 	s.GoldenFileName = goldenFileName
 	s.Deps = append(s.Deps, defaultStubs...)
+
+	s.dump = dumpExpected
 }
 
 func (s *GoldenTestSuite) AddDeps(deps []string) {
@@ -124,7 +134,7 @@ func runGoldenTest(s *GoldenTestSuite) {
 
 		reports := s.suite.RunFilterLinter(s.Disable)
 
-		s.checkGoldenOutput(s.want, reports)
+		s.checkGoldenOutput(s.want, reports, s.path, s.dump)
 	})
 }
 
@@ -134,6 +144,7 @@ func (s *GoldenTestSuite) loadGoldenFile() {
 	if err != nil {
 		s.suite.t.Fatalf("read golden file: %v", err)
 	}
+	s.path = path
 	s.want = want
 	if s.SrcDir == "" {
 		s.SrcDir = filepath.Join("testdata", s.Name)
@@ -157,8 +168,22 @@ func (s *GoldenTestSuite) loadReportsFile(filename string) {
 	s.reportFile = &output
 }
 
-func (s *GoldenTestSuite) checkGoldenOutput(want []byte, reports []*linter.Report) {
+func (s *GoldenTestSuite) checkGoldenOutput(want []byte, reports []*linter.Report, path string, dump bool) {
 	haveLines := s.formatReportLines(reports)
+	if dump {
+		var buf bytes.Buffer
+		for _, line := range haveLines {
+			buf.WriteString(line)
+			buf.WriteString("\n")
+		}
+		err := ioutil.WriteFile(path, bytes.TrimSuffix(buf.Bytes(), []byte("\n")), 0644)
+		if err != nil {
+			s.suite.t.Fatalf("write golden file: %v", err)
+			return
+		}
+		return
+	}
+
 	wantString := string(want)
 	wantLines := strings.Split(strings.ReplaceAll(wantString, "\r", ""), "\n")
 
@@ -327,7 +352,7 @@ func (s *GoldenE2ETestSuite) RunOnlyTests() {
 					r.Filename = strings.TrimPrefix(r.Filename, "/")
 				}
 
-				test.checkGoldenOutput(test.want, test.reportFile.Reports)
+				test.checkGoldenOutput(test.want, test.reportFile.Reports, test.path, test.dump)
 			})
 		}
 	})

src/tests/checkers/arrow_functions_test.go

@@ -74,10 +74,10 @@ function foo() {
 	test.Expect = []string{
 		`Cannot find referenced variable $undefined_variable`,
 		`Possibly undefined variable $maybe_defined`,
-		`Variable $a is unused (use $_ to ignore this inspection or specify --unused-var-regex flag)`,
-		`Variable $a is unused (use $_ to ignore this inspection or specify --unused-var-regex flag)`,
-		`Variable $a is unused (use $_ to ignore this inspection or specify --unused-var-regex flag)`,
-		`Variable $a is unused (use $_ to ignore this inspection or specify --unused-var-regex flag)`,
+		`Variable $a is unused`,
+		`Variable $a is unused`,
+		`Variable $a is unused`,
+		`Variable $a is unused`,
 		`Cannot find referenced variable $a`,
 		`Cannot find referenced variable $x`,
 		`Cannot find referenced variable $y`,
@@ -110,8 +110,8 @@ function f() {
 }
 `)
 	test.Expect = []string{
-		`Variable $a2 is unused (use $_ to ignore this inspection or specify --unused-var-regex flag)`,
-		`Variable $a is unused (use $_ to ignore this inspection or specify --unused-var-regex flag)`,
+		`Variable $a2 is unused`,
+		`Variable $a is unused`,
 	}
 	test.RunAndMatch()
 }

src/tests/checkers/basic_test.go

@@ -191,8 +191,8 @@ function f() {
 }
 `)
 	test.Expect = []string{
-		`Foreach key $i is unused, can simplify $i => $v to just $v`,
-		`Foreach key $i is unused, can simplify $i => $v to just $v`,
+		`Foreach key $i is unused`,
+		`Foreach key $i is unused`,
 	}
 	test.RunAndMatch()
 }

src/tests/golden/testdata/baseline-test/golden.txt

@@ -1,6 +1,10 @@
 WARNING discardExpr: Expression evaluated but not used at testdata/baseline-test/src/file1.php:10
-  "this line is not suppressed";
-  ^^^^^^^^^^^^^^^^^^^^^^^^^^^^^
+   |
+10 |  "this line is not suppressed";
+   |  ^^^^^^^^^^^^^^^^^^^^^^^^^^^^^ 
+
 ERROR   undefinedClass: Class or interface named \NonExisting does not exist at testdata/baseline-test/src/file2.php:5
-$bad = new NonExisting();
-           ^^^^^^^^^^^
+  |
+5 |$bad = new NonExisting();
+  |           ^^^^^^^^^^^ 
+

src/tests/golden/testdata/ctype/golden.txt

@@ -1,18 +1,30 @@
 MAYBE   regexpSimplify: May re-write '/[^0-9]/' as '/\D/' at testdata/ctype/ctype.php:84
-        return \is_string($text) && '' !== $text && !preg_match('/[^0-9]/', $text);
-                                                                ^^^^^^^^^^
+   |
+84 |        return \is_string($text) && '' !== $text && !preg_match('/[^0-9]/', $text);
+   |                                                                ^^^^^^^^^^ 
+
 WARNING regexpVet: suspicious char range '!-~' in [^!-~] at testdata/ctype/ctype.php:100
-        return \is_string($text) && '' !== $text && !preg_match('/[^!-~]/', $text);
-                                                                ^^^^^^^^^^
+    |
+100 |        return \is_string($text) && '' !== $text && !preg_match('/[^!-~]/', $text);
+    |                                                                ^^^^^^^^^^ 
+
 WARNING regexpVet: suspicious char range ' -~' in [^ -~] at testdata/ctype/ctype.php:132
-        return \is_string($text) && '' !== $text && !preg_match('/[^ -~]/', $text);
-                                                                ^^^^^^^^^^
+    |
+132 |        return \is_string($text) && '' !== $text && !preg_match('/[^ -~]/', $text);
+    |                                                                ^^^^^^^^^^ 
+
 MAYBE   regexpSimplify: May re-write '/[^!-\/\:-@\[-`\{-~]/' as '/[^!-\/:-@\[-`\{-~]/' at testdata/ctype/ctype.php:148
-        return \is_string($text) && '' !== $text && !preg_match('/[^!-\/\:-@\[-`\{-~]/', $text);
-                                                                ^^^^^^^^^^^^^^^^^^^^^^^
+    |
+148 |        return \is_string($text) && '' !== $text && !preg_match('/[^!-\/\:-@\[-`\{-~]/', $text);
+    |                                                                ^^^^^^^^^^^^^^^^^^^^^^^ 
+
 WARNING regexpVet: suspicious char range '!-\/' in [^!-\/\:-@\[-`\{-~] at testdata/ctype/ctype.php:148
-        return \is_string($text) && '' !== $text && !preg_match('/[^!-\/\:-@\[-`\{-~]/', $text);
-                                                                ^^^^^^^^^^^^^^^^^^^^^^^
+    |
+148 |        return \is_string($text) && '' !== $text && !preg_match('/[^!-\/\:-@\[-`\{-~]/', $text);
+    |                                                                ^^^^^^^^^^^^^^^^^^^^^^^ 
+
 MAYBE   regexpSimplify: May re-write '/[^\s]/' as '/\S/' at testdata/ctype/ctype.php:164
-        return \is_string($text) && '' !== $text && !preg_match('/[^\s]/', $text);
-                                                                ^^^^^^^^^
+    |
+164 |        return \is_string($text) && '' !== $text && !preg_match('/[^\s]/', $text);
+    |                                                                ^^^^^^^^^ 
+