Repository: VKCOM/noverify
Pull request #677 (2020-08-14 08:32:45)

linter: improve dup array key messages
Print both duplicated key syntax (as in the source file) and value,
but do not print the value if it's identical to the syntax string.

Since ir.String can contain non-printable chars like \n, we print
values with %q, so \n will be displayed as "\n" and will not end
up creating an actual newline in the output.

Signed-off-by: Iskander Sharipov <quasilyte@gmail.com>


Patch:
diff --git a/src/linter/block_linter.go b/src/linter/block_linter.go
index 18f6bd6..44cd385 100644
--- a/src/linter/block_linter.go
+++ b/src/linter/block_linter.go
@@ -520,7 +520,7 @@ func (b *blockLinter) checkArray(arr *ir.ArrayExpr) {
 	items := arr.Items
 	haveKeys := false
 	haveImplicitKeys := false
-	keys := make(map[string]struct{}, len(items))
+	keys := make(map[string]ir.Node, len(items))
 
 	for _, item := range items {
 		if item.Val == nil {
@@ -579,11 +579,17 @@ func (b *blockLinter) checkArray(arr *ir.ArrayExpr) {
 			continue
 		}
 
-		if _, ok := keys[key]; ok {
-			b.report(item.Key, LevelWarning, "dupArrayKeys", "Duplicate array key '%s'", key)
+		if n, ok := keys[key]; ok {
+			origKey := string(b.walker.r.nodeText(n))
+			dupKey := fmt.Sprintf("%#q", key)
+			msg := fmt.Sprintf("Duplicate array key %s", origKey)
+			if origKey != dupKey && origKey != key {
+				msg += " (value " + dupKey + ")"
+			}
+			b.report(item.Key, LevelWarning, "dupArrayKeys", "%s", msg)
 		}
 
-		keys[key] = struct{}{}
+		keys[key] = item.Key
 	}
 
 	if haveImplicitKeys && haveKeys {
@@ -637,8 +643,7 @@ func (b *blockLinter) checkRegexp(e *ir.FunctionCallExpr, arg *ir.Argument) {
 	pat := s.Value
 	simplified := b.walker.r.reSimplifier.simplifyRegexp(pat)
 	if simplified != "" {
-		pos := ir.GetPosition(s)
-		rawPattern := b.walker.r.fileContents[pos.StartPos:pos.EndPos]
+		rawPattern := b.walker.r.nodeText(s)
 		b.report(arg, LevelDoNotReject, "regexpSimplify", "May re-write %s as '%s'",
 			rawPattern, simplified)
 	}
diff --git a/src/linter/root.go b/src/linter/root.go
index 7c52914..6926ce0 100644
--- a/src/linter/root.go
+++ b/src/linter/root.go
@@ -1913,6 +1913,11 @@ func (d *RootWalker) checkKeywordCase(n ir.Node, keyword string) {
 	d.checkKeywordCasePos(n, ir.GetPosition(n).StartPos, keyword)
 }
 
+func (d *RootWalker) nodeText(n ir.Node) []byte {
+	pos := ir.GetPosition(n)
+	return d.fileContents[pos.StartPos:pos.EndPos]
+}
+
 func (d *RootWalker) parseClassPHPDoc(n ir.Node, doc string) classPhpDocParseResult {
 	var result classPhpDocParseResult
 


Test patch:
diff --git a/src/tests/checkers/basic_test.go b/src/tests/checkers/basic_test.go
index 0b63518..d5d2e53 100644
--- a/src/tests/checkers/basic_test.go
+++ b/src/tests/checkers/basic_test.go
@@ -296,7 +296,7 @@ $_ = array(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'`,
+		`Duplicate array key 1`,
 		`Use of old array syntax (use short form instead)`,
 	}
 	test.RunAndMatch()
@@ -1289,6 +1289,15 @@ func TestAllowAssignmentInForLoop(t *testing.T) {
 	`)
 }
 
+func TestDuplicateArrayKeyEscapes(t *testing.T) {
+	test := linttest.NewSuite(t)
+	test.AddFile(`<?php
+$_ = ["\n" => 1, "\xa" => 2];
+`)
+	test.Expect = []string{`Duplicate array key "\n"`}
+	test.RunAndMatch()
+}
+
 func TestDuplicateArrayKeyGood(t *testing.T) {
 	linttest.SimpleNegativeTest(t, `<?php
 $valid_quotes = [
@@ -1328,7 +1337,7 @@ $b = [
   FIRST_SEARCH_KEY => 1,
   SECOND_SEARCH_KEY => 45,
 ];
-	
+
 const START_PERCENT = 0.1;
 const END_PERCENT = 0.1;
 $c = [
@@ -1355,11 +1364,11 @@ $b = [
 
 `)
 	test.Expect = []string{
-		"Duplicate array key '1'",
-		"Duplicate array key 'apple'",
-		"Duplicate array key '0'",
-		"Duplicate array key '-1'",
-		"Duplicate array key '2'",
+		"Duplicate array key MAX_VALUE (value `1`)",
+		"Duplicate array key FIRST_SEARCH_KEY (value `apple`)",
+		"Duplicate array key START_PERCENT (value `0`)",
+		"Duplicate array key START_PERCENT_REVERT (value `-1`)",
+		"Duplicate array key START_PERCENT_NORMAL (value `2`)",
 	}
 
 	test.RunAndMatch()