Repository: VKCOM/noverify
Pull request #587 (2020-07-15 16:06:13)

ReportByLine for report with a context line with specific line
Solve #585 


Patch:
diff --git a/src/linter/root.go b/src/linter/root.go
index 1fbd883..29f9074 100644
--- a/src/linter/root.go
+++ b/src/linter/root.go
@@ -216,7 +216,7 @@ func (d *RootWalker) EnterNode(w walker.Walkable) (res bool) {
 			for _, cs := range *ffs {
 				for _, c := range cs {
 					if c.StringType == freefloating.CommentType {
-						d.handleComment(c, n)
+						d.handleComment(c)
 					}
 				}
 			}
@@ -378,8 +378,7 @@ func (d *RootWalker) parseStartPos(pos *position.Position) (startLn []byte, star
 	return startLn, startChar
 }
 
-// Report registers a single report message about some found problem.
-func (d *RootWalker) Report(n node.Node, level int, checkName, msg string, args ...interface{}) {
+func (d *RootWalker) report(n node.Node, lineNumber int, level int, checkName, msg string, args ...interface{}) {
 	if !meta.IsIndexingComplete() {
 		return
 	}
@@ -387,51 +386,74 @@ func (d *RootWalker) Report(n node.Node, level int, checkName, msg string, args
 		return
 	}
 	// We don't report anything if linter was disabled by a
-	// successfull @linter disable, unless it's the linterError.
+	// successful @linter disable, unless it's the linterError.
 	if d.linterDisabled && checkName != "linterError" {
 		return
 	}
 
+	isReportForNode := lineNumber == 0
+	isReportForLine := !isReportForNode
+
 	var pos position.Position
+	var startLn []byte
+	var startChar int
+	var endLn []byte
+	var endChar int
 
-	if n == nil {
-		// Hack to parse syntax error message from php-parser.
-		// When in language server mode, do not map syntax errors in order not to
-		// complain about unfinished piece of code that user is currently writing.
-		if strings.Contains(msg, "syntax error") && strings.Contains(msg, " at line ") && !LangServer {
-			// it is in form "Syntax error: syntax error: unexpected '*' at line 4"
-			if lastIdx := strings.LastIndexByte(msg, ' '); lastIdx > 0 {
-				lineNumStr := msg[lastIdx+1:]
-				lineNum, err := strconv.Atoi(lineNumStr)
-				if err == nil {
-					pos.StartLine = lineNum
-					pos.EndLine = lineNum
-					msg = msg[0:lastIdx]
-					msg = strings.TrimSuffix(msg, " at line")
+	if isReportForNode {
+		if n == nil {
+			// Hack to parse syntax error message from php-parser.
+			// When in language server mode, do not map syntax errors in order not to
+			// complain about unfinished piece of code that user is currently writing.
+			if strings.Contains(msg, "syntax error") && strings.Contains(msg, " at line ") && !LangServer {
+				// it is in form "Syntax error: syntax error: unexpected '*' at line 4"
+				if lastIdx := strings.LastIndexByte(msg, ' '); lastIdx > 0 {
+					lineNumStr := msg[lastIdx+1:]
+					lineNum, err := strconv.Atoi(lineNumStr)
+					if err == nil {
+						pos.StartLine = lineNum
+						pos.EndLine = lineNum
+						msg = msg[0:lastIdx]
+						msg = strings.TrimSuffix(msg, " at line")
+					}
 				}
 			}
+		} else {
+			pos = *n.GetPosition()
 		}
-	} else {
-		pos = *n.GetPosition()
-	}
 
-	var endLn []byte
-	var endChar int
+		startLn, startChar = d.parseStartPos(&pos)
+
+		if pos.EndLine >= 1 && len(d.Lines) > pos.EndLine {
+			endLn = d.Lines[pos.EndLine-1]
+			p := d.LinesPositions[pos.EndLine-1]
+			if pos.EndPos > p {
+				endChar = pos.EndPos - p
+			}
+		} else {
+			endLn = startLn
+		}
 
-	startLn, startChar := d.parseStartPos(&pos)
+		if endChar == 0 {
+			endChar = len(endLn)
+		}
+	} else if isReportForLine {
+		if lineNumber < 1 || lineNumber > len(d.Lines) {
+			return
+		}
 
-	if pos.EndLine >= 1 && len(d.Lines) > pos.EndLine {
-		endLn = d.Lines[pos.EndLine-1]
-		p := d.LinesPositions[pos.EndLine-1]
-		if pos.EndPos > p {
-			endChar = pos.EndPos - p
+		startLn = d.Lines[lineNumber-1]
+		startChar = 0
+		endChar = len(startLn)
+
+		if strings.HasSuffix(string(startLn), "\r") {
+			endChar = endChar - 1
 		}
-	} else {
-		endLn = startLn
-	}
 
-	if endChar == 0 {
-		endChar = len(endLn)
+		pos = position.Position{
+			StartLine: lineNumber,
+			EndLine:   lineNumber,
+		}
 	}
 
 	if LangServer {
@@ -484,6 +506,16 @@ func (d *RootWalker) Report(n node.Node, level int, checkName, msg string, args
 	}
 }
 
+// Report registers a single report message about some found problem.
+func (d *RootWalker) Report(n node.Node, level int, checkName, msg string, args ...interface{}) {
+	d.report(n, 0, level, checkName, msg, args...)
+}
+
+// ReportByLine registers a single report message about some found problem in lineIndex code line.
+func (d *RootWalker) ReportByLine(lineNumber int, level int, checkName, msg string, args ...interface{}) {
+	d.report(nil, lineNumber, level, checkName, msg, args...)
+}
+
 // reportHash computes the report signature hash for the baseline.
 func (d *RootWalker) reportHash(pos *position.Position, startLine []byte, checkName, msg string) uint64 {
 	// Since we store class::method scope, renaming a class would cause baseline
@@ -566,7 +598,7 @@ func (d *RootWalker) reportUndefinedVariable(v node.Node, maybeHave bool) {
 	}
 }
 
-func (d *RootWalker) handleComment(c freefloating.String, n node.Node) {
+func (d *RootWalker) handleComment(c freefloating.String) {
 	if c.StringType != freefloating.CommentType {
 		return
 	}
@@ -586,7 +618,8 @@ func (d *RootWalker) handleComment(c freefloating.String, n node.Node) {
 				continue
 			}
 			if d.linterDisabled {
-				d.Report(nil, LevelInformation, "linterError", "Linter is already disabled for this file")
+				needleLine := ln.Line() + c.Position.StartLine - 1
+				d.ReportByLine(needleLine, LevelInformation, "linterError", "Linter is already disabled for this file")
 				continue
 			}
 			canDisable := false
@@ -595,7 +628,8 @@ func (d *RootWalker) handleComment(c freefloating.String, n node.Node) {
 			}
 			d.linterDisabled = canDisable
 			if !canDisable {
-				d.Report(nil, LevelInformation, "linterError", "You are not allowed to disable linter")
+				needleLine := ln.Line() + c.Position.StartLine - 1
+				d.ReportByLine(needleLine, LevelInformation, "linterError", "You are not allowed to disable linter")
 			}
 		}
 	}


Test patch:
diff --git a/src/linttest/basic_test.go b/src/linttest/basic_test.go
index 48582a2..a26348a 100644
--- a/src/linttest/basic_test.go
+++ b/src/linttest/basic_test.go
@@ -168,12 +168,12 @@ $_ = array(1);
 `)
 	test.Expect = []string{
 		`You are not allowed to disable linter`,
-		`Use of old array syntax (use short form instead)`,
 		`You are not allowed to disable linter`,
-		`Duplicate array key '1'`,
 		`You are not allowed to disable linter`,
 		`You are not allowed to disable linter`,
 		`Use of old array syntax (use short form instead)`,
+		`Duplicate array key '1'`,
+		`Use of old array syntax (use short form instead)`,
 	}
 	test.RunAndMatch()
 }
diff --git a/src/linttest/golden_test.go b/src/linttest/golden_test.go
index fabfc71..623fd89 100644
--- a/src/linttest/golden_test.go
+++ b/src/linttest/golden_test.go
@@ -176,6 +176,10 @@ func TestGolden(t *testing.T) {
 				`stubs/phpstorm-stubs/curl/curl_d.php`,
 			},
 		},
+
+		{
+			name: "output-test",
+		},
 	}
 
 	for _, target := range targets {
diff --git a/src/linttest/testdata/output-test/golden.txt b/src/linttest/testdata/output-test/golden.txt
new file mode 100644
index 0000000..9dd60fb
--- /dev/null
+++ b/src/linttest/testdata/output-test/golden.txt
@@ -0,0 +1,18 @@
+INFO    linterError: You are not allowed to disable linter at testdata/output-test/output_test.php:3
+/** @linter disable */
+^^^^^^^^^^^^^^^^^^^^^^
+INFO    linterError: You are not allowed to disable linter at testdata/output-test/output_test.php:6
+ * @linter disable
+^^^^^^^^^^^^^^^^^^
+INFO    linterError: You are not allowed to disable linter at testdata/output-test/output_test.php:10
+ * @linter disable
+^^^^^^^^^^^^^^^^^^
+INFO    linterError: You are not allowed to disable linter at testdata/output-test/output_test.php:11
+ * @linter disable
+^^^^^^^^^^^^^^^^^^
+INFO    linterError: You are not allowed to disable linter at testdata/output-test/output_test.php:12
+ * @linter disable
+^^^^^^^^^^^^^^^^^^
+INFO    linterError: You are not allowed to disable linter at testdata/output-test/output_test.php:13
+ * @linter disable
+^^^^^^^^^^^^^^^^^^
diff --git a/src/linttest/testdata/output-test/output_test.php b/src/linttest/testdata/output-test/output_test.php
new file mode 100644
index 0000000..ccdf914
--- /dev/null
+++ b/src/linttest/testdata/output-test/output_test.php
@@ -0,0 +1,14 @@
+<?php
+
+/** @linter disable */
+
+/**
+ * @linter disable
+ */
+
+/**
+ * @linter disable
+ * @linter disable
+ * @linter disable
+ * @linter disable
+ */
\ No newline at end of file