Repository: VKCOM/noverify
Pull request #584 (2020-07-15 12:10:20)

linter: Added saving of constant value in ConstantInfo #583
solve #583 

quasilyte: I would also suggest trying to write more detailed commit messages.

This might be helpful:
https://golang.org/doc/contribute.html#commit_messages

Patch:
diff --git a/src/linter/block.go b/src/linter/block.go
index 1f6e379..4a9a849 100644
--- a/src/linter/block.go
+++ b/src/linter/block.go
@@ -4,6 +4,7 @@ import (
 	"fmt"
 	"math"
 	"regexp"
+	"strconv"
 	"strings"
 
 	"github.com/VKCOM/noverify/src/meta"
@@ -210,8 +211,8 @@ func (b *BlockWalker) checkBinaryDupArgs(n, left, right node.Node) {
 	if !b.sideEffectFree(left) || !b.sideEffectFree(right) {
 		return
 	}
-	if astutil.NodeEqual(left, right) {
-		b.r.Report(n, LevelWarning, "dupSubExpr", "duplicated operands in %s expression", binaryOpString(n))
+	if b.NodeEqual(left, right) {
+		b.r.Report(n, LevelWarning, "dupSubExpr", "duplicated operands value in %s expression", binaryOpString(n))
 	}
 }
 
@@ -1361,6 +1362,28 @@ func (b *BlockWalker) handleArrayItems(arr node.Node, items []*expr.ArrayItem) b
 			constKey = true
 		case *scalar.Lnumber:
 			key = k.Value
+			constKey = true
+		case *expr.ConstFetch:
+			_, info, ok := solver.GetConstant(b.r.ctx.st, k.Constant)
+
+			if !ok {
+				continue
+			}
+			if info.Value.Type == meta.Undefined {
+				continue
+			}
+
+			key = info.Value.Value
+
+			if info.Value.Type == meta.Float {
+				val, err := strconv.ParseFloat(key, 64)
+				if err != nil {
+					continue
+				}
+				value := int64(val)
+				key = fmt.Sprint(value)
+			}
+
 			constKey = true
 		}
 
@@ -2436,3 +2459,29 @@ func (b *BlockWalker) isVoid(n node.Node) bool {
 func (b *BlockWalker) exprType(n node.Node) meta.TypesMap {
 	return solver.ExprTypeCustom(b.ctx.sc, b.r.ctx.st, n, b.ctx.customTypes)
 }
+
+func (b *BlockWalker) NodeEqual(x, y node.Node) bool {
+	if x == nil || y == nil {
+		return x == y
+	}
+	switch x := x.(type) {
+	case *expr.ConstFetch:
+		y, ok := y.(*expr.ConstFetch)
+		if !ok {
+			return false
+		}
+		_, info1, ok := solver.GetConstant(b.r.ctx.st, x.Constant)
+		if !ok {
+			return false
+		}
+		_, info2, ok := solver.GetConstant(b.r.ctx.st, y.Constant)
+		if !ok {
+			return false
+		}
+
+		return info1.Value.IsEqual(info2.Value)
+
+	default:
+		return astutil.NodeEqual(x, y)
+	}
+}
diff --git a/src/linter/cache.go b/src/linter/cache.go
index 0fb2afc..c55e32b 100644
--- a/src/linter/cache.go
+++ b/src/linter/cache.go
@@ -37,7 +37,8 @@ import (
 //          changed meta.scopeVar bool fields representation
 //     38 - replaced TypesMap.immutable:bool with flags:uint8.
 //          added mapPrecise flag to mark precise type maps.
-const cacheVersion = 38
+//     39 - added new field Value in ConstantInfo
+const cacheVersion = 39
 
 var (
 	errWrongVersion = errors.New("Wrong cache version")
diff --git a/src/linter/root.go b/src/linter/root.go
index 6e0638f..1fbd883 100644
--- a/src/linter/root.go
+++ b/src/linter/root.go
@@ -860,11 +860,14 @@ func (d *RootWalker) enterClassConstList(s *stmt.ClassConstList) bool {
 		d.checkCommentMisspellings(c, c.PhpDocComment)
 		typ := solver.ExprTypeLocal(d.scope(), d.ctx.st, c.Expr)
 
+		value, _ := solver.GetConstantValue(c)
+
 		// TODO: handle duplicate constant
 		cl.Constants[nm] = meta.ConstantInfo{
 			Pos:         d.getElementPos(c),
 			Typ:         typ.Immutable(),
 			AccessLevel: accessLevel,
+			Value:       value,
 		}
 	}
 
@@ -1573,12 +1576,15 @@ func (d *RootWalker) enterConstList(lst *stmt.ConstList) bool {
 	for _, sNode := range lst.Consts {
 		s := sNode.(*stmt.Constant)
 
+		value, _ := solver.GetConstantValue(s)
+
 		id := s.ConstantName
 		nm := d.ctx.st.Namespace + `\` + id.Value
 
 		d.meta.Constants[nm] = meta.ConstantInfo{
-			Pos: d.getElementPos(s),
-			Typ: solver.ExprTypeLocal(d.scope(), d.ctx.st, s.Expr),
+			Pos:   d.getElementPos(s),
+			Typ:   solver.ExprTypeLocal(d.scope(), d.ctx.st, s.Expr),
+			Value: value,
 		}
 	}
 
diff --git a/src/meta/metainfo.go b/src/meta/metainfo.go
index c9fdb8e..130889c 100644
--- a/src/meta/metainfo.go
+++ b/src/meta/metainfo.go
@@ -1,6 +1,8 @@
 package meta
 
 import (
+	"fmt"
+	"strconv"
 	"strings"
 	"sync"
 
@@ -367,10 +369,61 @@ type PropertyInfo struct {
 	AccessLevel AccessLevel
 }
 
+type ConstantValueType uint8
+
+const (
+	Undefined ConstantValueType = iota
+	Integer
+	Float
+	String
+)
+
+type ConstantValue struct {
+	Type  ConstantValueType
+	Value string
+}
+
+func (cv ConstantValue) String() string {
+	if cv.Type == Undefined {
+		return "Undefined type"
+	}
+
+	return fmt.Sprintf("%d: %s", cv.Type, cv.Value)
+}
+
+func (cv ConstantValue) IsEqual(v ConstantValue) bool {
+	if v.Type == Undefined || cv.Type == Undefined {
+		return false
+	}
+
+	return cv.Value == v.Value
+}
+
+func NewConstantValueFromString(value string) ConstantValue {
+	unquotedValue, err := strconv.Unquote(value)
+	if err != nil {
+		unquotedValue = value
+	}
+	return ConstantValue{Value: fmt.Sprintf("\"%s\"", unquotedValue), Type: String}
+}
+
+func NewConstantValueFromFloat(value float64) ConstantValue {
+	return ConstantValue{Value: fmt.Sprintf("%f", value), Type: Float}
+}
+
+func NewConstantValueFromInt(value int64) ConstantValue {
+	return ConstantValue{Value: fmt.Sprintf("%d", value), Type: Integer}
+}
+
+func NewUndefinedConstantValue() ConstantValue {
+	return ConstantValue{Type: Undefined}
+}
+
 type ConstantInfo struct {
 	Pos         ElementPosition
 	Typ         TypesMap
 	AccessLevel AccessLevel
+	Value       ConstantValue
 }
 
 type ClassFlags uint8
diff --git a/src/solver/exprtype.go b/src/solver/exprtype.go
index 9321c72..4fa9851 100644
--- a/src/solver/exprtype.go
+++ b/src/solver/exprtype.go
@@ -2,6 +2,7 @@ package solver
 
 import (
 	"log"
+	"strconv"
 	"strings"
 
 	"github.com/VKCOM/noverify/src/meta"
@@ -13,6 +14,7 @@ import (
 	"github.com/VKCOM/noverify/src/php/parser/node/expr/cast"
 	"github.com/VKCOM/noverify/src/php/parser/node/name"
 	"github.com/VKCOM/noverify/src/php/parser/node/scalar"
+	"github.com/VKCOM/noverify/src/php/parser/node/stmt"
 )
 
 func bitwiseOpType(sc *meta.Scope, cs *meta.ClassParseState, left, right node.Node, custom []CustomType) meta.TypesMap {
@@ -409,3 +411,33 @@ func ExprTypeLocalCustom(sc *meta.Scope, cs *meta.ClassParseState, n node.Node,
 	}
 	return res
 }
+
+func GetConstantValue(c *stmt.Constant) (meta.ConstantValue, bool) {
+	switch c := c.Expr.(type) {
+	case *scalar.Lnumber:
+		return getConstantValue(c, 1)
+	case *scalar.Dnumber:
+		return getConstantValue(c, 1)
+	case *scalar.String:
+		return getConstantValue(c, 1)
+	case *expr.UnaryMinus:
+		return getConstantValue(c.Expr, -1)
+	default:
+		return meta.NewUndefinedConstantValue(), false
+	}
+}
+
+func getConstantValue(n node.Node, modifier int64) (meta.ConstantValue, bool) {
+	switch c := n.(type) {
+	case *scalar.Lnumber:
+		value, err := strconv.ParseInt(c.Value, 10, 64)
+		return meta.NewConstantValueFromInt(value * modifier), err == nil
+	case *scalar.Dnumber:
+		value, err := strconv.ParseFloat(c.Value, 64)
+		return meta.NewConstantValueFromFloat(value * float64(modifier)), err == nil
+	case *scalar.String:
+		return meta.NewConstantValueFromString(c.Value), true
+	default:
+		return meta.NewUndefinedConstantValue(), false
+	}
+}


Test patch:
diff --git a/src/linter/cache_test.go b/src/linter/cache_test.go
index fff3ed4..fd0bb38 100644
--- a/src/linter/cache_test.go
+++ b/src/linter/cache_test.go
@@ -111,7 +111,7 @@ main();
 		//
 		// If cache encoding changes, there is a very high chance that
 		// encoded data lengh will change as well.
-		wantLen := 4031
+		wantLen := 4109
 		haveLen := buf.Len()
 		if haveLen != wantLen {
 			t.Errorf("cache len mismatch:\nhave: %d\nwant: %d", haveLen, wantLen)
@@ -120,7 +120,7 @@ main();
 		// 2. Check cache "strings" hash.
 		//
 		// It catches new fields in cached types, field renames and encoding of additional named attributes.
-		wantStrings := "0cf356617cd2000854fcb3e8aab4ff06ea8f775a61562f043716ead6083074d1140f4b57d2ce05037ea24117273252a03184ea7f0de788759a6921a8869c8b76"
+		wantStrings := "f5c3f410fd821309de4eaf6bc68ca6aee07dcb904edeaa7b8cb6798291103f9036dbc5ffbff87dc01f04d26f0483a623caf5511f60d4b66a60952870a516c11c"
 		haveStrings := collectCacheStrings(buf.String())
 		if haveStrings != wantStrings {
 			t.Errorf("cache strings mismatch:\nhave: %q\nwant: %q", haveStrings, wantStrings)
diff --git a/src/linttest/basic_test.go b/src/linttest/basic_test.go
index e43ab21..48582a2 100644
--- a/src/linttest/basic_test.go
+++ b/src/linttest/basic_test.go
@@ -1178,17 +1178,70 @@ $valid_quotes = [
 func TestDuplicateArrayKey(t *testing.T) {
 	test := linttest.NewSuite(t)
 	test.AddFile(`<?php
-	function test() {
-	  return [
-		  'key1' => 'something',
-		  'key2' => 'other_thing',
-		  'key1' => 'third_thing', // duplicate
-	  ];
-	}`)
+function test() {
+  return [
+	  'key1' => 'something',
+	  'key2' => 'other_thing',
+	  'key1' => 'third_thing', // duplicate
+  ];
+}
+`)
 	test.Expect = []string{"Duplicate array key 'key1'"}
 	test.RunAndMatch()
 }
 
+func TestDuplicateArrayKeyWithConstants(t *testing.T) {
+	test := linttest.NewSuite(t)
+	test.AddFile(`<?php
+const MAX_VALUE = 1;
+const MIN_VALUE = 1;
+$a = [
+  MAX_VALUE => 'something',
+  MIN_VALUE => 'other_thing',
+];
+const FIRST_SEARCH_KEY = "apple";
+const SECOND_SEARCH_KEY = "apple";
+$b = [
+  FIRST_SEARCH_KEY => 1,
+  SECOND_SEARCH_KEY => 45,
+];
+	
+const START_PERCENT = 0.1;
+const END_PERCENT = 0.1;
+$c = [
+  START_PERCENT => 1,
+  END_PERCENT => 45,
+];
+
+
+const START_PERCENT_REVERT = -1;
+const END_PERCENT_REVERT = -1.51;
+
+$a = [
+	START_PERCENT_REVERT => 2,
+	END_PERCENT_REVERT => 3,
+];
+
+const START_PERCENT_NORMAL = 2;
+const END_PERCENT_NORMAL = 2.51;
+
+$b = [
+	START_PERCENT_NORMAL => 2,
+	END_PERCENT_NORMAL => 3,
+];
+
+`)
+	test.Expect = []string{
+		"Duplicate array key '1'",
+		"Duplicate array key '\"apple\"'",
+		"Duplicate array key '0'",
+		"Duplicate array key '-1'",
+		"Duplicate array key '2'",
+	}
+
+	test.RunAndMatch()
+}
+
 func TestMixedArrayKeys(t *testing.T) {
 	test := linttest.NewSuite(t)
 	test.AddFile(`<?php
diff --git a/src/linttest/codedup_test.go b/src/linttest/codedup_test.go
index c7594fb..07d457f 100644
--- a/src/linttest/codedup_test.go
+++ b/src/linttest/codedup_test.go
@@ -49,26 +49,26 @@ function f($x) {
 }
 `)
 	test.Expect = []string{
-		`duplicated operands in & expression`,
-		`duplicated operands in | expression`,
-		`duplicated operands in ^ expression`,
-		`duplicated operands in and expression`,
-		`duplicated operands in && expression`,
-		`duplicated operands in or expression`,
-		`duplicated operands in || expression`,
-		`duplicated operands in xor expression`,
-		`duplicated operands in - expression`,
-		`duplicated operands in / expression`,
-		`duplicated operands in % expression`,
-		`duplicated operands in == expression`,
-		`duplicated operands in === expression`,
-		`duplicated operands in != expression`,
-		`duplicated operands in !== expression`,
-		`duplicated operands in < expression`,
-		`duplicated operands in <= expression`,
-		`duplicated operands in > expression`,
-		`duplicated operands in >= expression`,
-		`duplicated operands in <=> expression`,
+		`duplicated operands value in & expression`,
+		`duplicated operands value in | expression`,
+		`duplicated operands value in ^ expression`,
+		`duplicated operands value in and expression`,
+		`duplicated operands value in && expression`,
+		`duplicated operands value in or expression`,
+		`duplicated operands value in || expression`,
+		`duplicated operands value in xor expression`,
+		`duplicated operands value in - expression`,
+		`duplicated operands value in / expression`,
+		`duplicated operands value in % expression`,
+		`duplicated operands value in == expression`,
+		`duplicated operands value in === expression`,
+		`duplicated operands value in != expression`,
+		`duplicated operands value in !== expression`,
+		`duplicated operands value in < expression`,
+		`duplicated operands value in <= expression`,
+		`duplicated operands value in > expression`,
+		`duplicated operands value in >= expression`,
+		`duplicated operands value in <=> expression`,
 	}
 	test.RunAndMatch()
 }
@@ -104,17 +104,17 @@ function f() {
 }
 `)
 	test.Expect = []string{
-		`duplicated operands in & expression`,
-		`duplicated operands in | expression`,
-		`duplicated operands in ^ expression`,
-		`duplicated operands in and expression`,
-		`duplicated operands in && expression`,
-		`duplicated operands in or expression`,
-		`duplicated operands in || expression`,
-		`duplicated operands in xor expression`,
-		`duplicated operands in % expression`,
-		`duplicated operands in < expression`,
-		`duplicated operands in > expression`,
+		`duplicated operands value in & expression`,
+		`duplicated operands value in | expression`,
+		`duplicated operands value in ^ expression`,
+		`duplicated operands value in and expression`,
+		`duplicated operands value in && expression`,
+		`duplicated operands value in or expression`,
+		`duplicated operands value in || expression`,
+		`duplicated operands value in xor expression`,
+		`duplicated operands value in % expression`,
+		`duplicated operands value in < expression`,
+		`duplicated operands value in > expression`,
 	}
 	test.RunAndMatch()
 }