Repository: VKCOM/noverify
Pull request #669 (2020-08-13 12:37:03)

meta: fix string const value for ''-literals
Since we used strconv.Unquote, ""-literal quotes were
removed while ''-literals were stored with double pairs of quotes.
This can be fixed with our own unquote function.

As we need that unquote in more than 1 package, it was moved
to irutil package.

To avoid quotes confusion in future, we start storing string values
without quotes in meta. Since we store type tag, we don't need
quotes to determine the value kind during decoding.

Signed-off-by: Iskander Sharipov <quasilyte@gmail.com>


Patch:
diff --git a/src/ir/irutil/irtutil.go b/src/ir/irutil/irtutil.go
index 7762ffc..5cf053a 100644
--- a/src/ir/irutil/irtutil.go
+++ b/src/ir/irutil/irtutil.go
@@ -7,6 +7,14 @@ import (
 
 //go:generate go run ./codegen.go
 
+// Unquote returns unquoted version of s, if there are any quotes.
+func Unquote(s string) string {
+	if len(s) >= 2 && s[0] == '\'' || s[0] == '"' {
+		return s[1 : len(s)-1]
+	}
+	return s
+}
+
 func NodeSliceClone(xs []ir.Node) []ir.Node {
 	cloned := make([]ir.Node, len(xs))
 	for i, x := range xs {
diff --git a/src/linter/and_walker.go b/src/linter/and_walker.go
index 21d8511..ce58c7c 100644
--- a/src/linter/and_walker.go
+++ b/src/linter/and_walker.go
@@ -2,6 +2,7 @@ package linter
 
 import (
 	"github.com/VKCOM/noverify/src/ir"
+	"github.com/VKCOM/noverify/src/ir/irutil"
 	"github.com/VKCOM/noverify/src/meta"
 	"github.com/VKCOM/noverify/src/solver"
 )
@@ -30,13 +31,13 @@ func (a *andWalker) EnterNode(w ir.Node) (res bool) {
 			methodName := n.Arg(1).Expr
 			lit, ok := methodName.(*ir.String)
 			if ok {
-				a.b.ctx.addCustomMethod(obj, unquote(lit.Value))
+				a.b.ctx.addCustomMethod(obj, irutil.Unquote(lit.Value))
 			}
 		case len(n.Args) == 1 && nm.Value == `function_exists`:
 			functionName := n.Arg(0).Expr
 			lit, ok := functionName.(*ir.String)
 			if ok {
-				a.b.ctx.addCustomFunction(unquote(lit.Value))
+				a.b.ctx.addCustomFunction(irutil.Unquote(lit.Value))
 			}
 		}
 
diff --git a/src/linter/block.go b/src/linter/block.go
index 0c06a4f..f578006 100644
--- a/src/linter/block.go
+++ b/src/linter/block.go
@@ -6,6 +6,7 @@ import (
 	"strings"
 
 	"github.com/VKCOM/noverify/src/ir"
+	"github.com/VKCOM/noverify/src/ir/irutil"
 	"github.com/VKCOM/noverify/src/meta"
 	"github.com/VKCOM/noverify/src/php/parser/freefloating"
 	"github.com/VKCOM/noverify/src/phpdoc"
@@ -788,7 +789,7 @@ func (b *BlockWalker) handleCompactCallArgs(args []ir.Node) {
 
 	for _, s := range strs {
 		v := &ir.SimpleVar{
-			Name:     unquote(s.Value),
+			Name:     irutil.Unquote(s.Value),
 			Position: ir.GetPosition(s),
 		}
 		b.handleVariable(v)
@@ -1613,7 +1614,7 @@ func (b *BlockWalker) handleAssignShapeToList(items []*ir.ArrayItemExpr, info me
 			var key string
 			switch keyNode := item.Key.(type) {
 			case *ir.String:
-				key = unquote(keyNode.Value)
+				key = irutil.Unquote(keyNode.Value)
 			case *ir.Lnumber:
 				key = keyNode.Value
 			case *ir.Dnumber:
diff --git a/src/linter/block_linter.go b/src/linter/block_linter.go
index 42ccb6f..f26c69e 100644
--- a/src/linter/block_linter.go
+++ b/src/linter/block_linter.go
@@ -539,7 +539,7 @@ func (b *blockLinter) checkArray(arr *ir.ArrayExpr) {
 
 		switch k := item.Key.(type) {
 		case *ir.String:
-			key = unquote(k.Value)
+			key = irutil.Unquote(k.Value)
 			constKey = true
 		case *ir.Lnumber:
 			key = k.Value
diff --git a/src/linter/cache.go b/src/linter/cache.go
index c55e32b..4bcae4a 100644
--- a/src/linter/cache.go
+++ b/src/linter/cache.go
@@ -38,7 +38,8 @@ import (
 //     38 - replaced TypesMap.immutable:bool with flags:uint8.
 //          added mapPrecise flag to mark precise type maps.
 //     39 - added new field Value in ConstantInfo
-const cacheVersion = 39
+//     40 - changed string const value storage (no quotes)
+const cacheVersion = 40
 
 var (
 	errWrongVersion = errors.New("Wrong cache version")
diff --git a/src/linter/regexp_utils.go b/src/linter/regexp_utils.go
index fa9f1b3..48ccfd2 100644
--- a/src/linter/regexp_utils.go
+++ b/src/linter/regexp_utils.go
@@ -4,6 +4,7 @@ import (
 	"strings"
 
 	"github.com/VKCOM/noverify/src/ir"
+	"github.com/VKCOM/noverify/src/ir/irutil"
 )
 
 type regexpPattern struct {
@@ -32,7 +33,7 @@ func newRegexpPattern(lit *ir.String) (regexpPattern, bool) {
 	switch lit.Value[0] {
 	case '\'':
 		// Single quotes only interpret \' and \\ sequences.
-		pat := unquote(lit.Value)
+		pat := irutil.Unquote(lit.Value)
 		pat = strings.ReplaceAll(pat, `\\`, `\`)
 		pat = strings.ReplaceAll(pat, `\'`, `'`)
 		result.value = pat
diff --git a/src/linter/report.go b/src/linter/report.go
index 319a5d6..0f6bf79 100644
--- a/src/linter/report.go
+++ b/src/linter/report.go
@@ -715,14 +715,6 @@ func isUnderscore(s string) bool {
 	return s == "_"
 }
 
-// unquote returns unquoted version of s, if there are any quotes.
-func unquote(s string) string {
-	if len(s) >= 2 && s[0] == '\'' || s[0] == '"' {
-		return s[1 : len(s)-1]
-	}
-	return s
-}
-
 func linterError(filename, format string, args ...interface{}) {
 	log.Printf("error: "+filename+": "+format, args...)
 }
diff --git a/src/meta/metainfo.go b/src/meta/metainfo.go
index 2895053..b4acf47 100644
--- a/src/meta/metainfo.go
+++ b/src/meta/metainfo.go
@@ -7,6 +7,7 @@ import (
 	"sync"
 
 	"github.com/VKCOM/noverify/src/ir"
+	"github.com/VKCOM/noverify/src/ir/irutil"
 )
 
 var (
@@ -445,7 +446,7 @@ func (c ConstantValue) String() string {
 	if c.Type == Undefined {
 		return "Undefined type"
 	}
-	return fmt.Sprintf("%d: %s", c.Type, c.Value)
+	return fmt.Sprintf("%d: %v", c.Type, c.Value)
 }
 
 func (c ConstantValue) IsEqual(v ConstantValue) bool {
@@ -457,11 +458,8 @@ func (c ConstantValue) IsEqual(v ConstantValue) bool {
 }
 
 func NewConstantValueFromString(value string) ConstantValue {
-	unquotedValue, err := strconv.Unquote(value)
-	if err != nil {
-		unquotedValue = value
-	}
-	return ConstantValue{Value: fmt.Sprintf("\"%s\"", unquotedValue), Type: String}
+	v := irutil.Unquote(value)
+	return ConstantValue{Value: v, Type: String}
 }
 
 func NewConstantValueFromFloat(value float64) ConstantValue {


Test patch:
diff --git a/src/linter/cache_test.go b/src/linter/cache_test.go
index bf5f223..9111945 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 := 4066
+		wantLen := 4064
 		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 := "7987e3df6f3073570d28eaac3e197f6a09866d1c384aeb4a093e02aeeb345ade12210a73467ae55a939e22d50e8bdd4d45bae7e149787f2f7327c39d382d3d18"
+		wantStrings := "d2a5e66052a3e5e124dcd761d355da18c5863a07b93d12eb79e82c7fd8d07fbaae8b253a61e2b94e58c40fbdeb24bcc56d23aa0543c75510ff74336c3dc47bfe"
 		haveStrings := collectCacheStrings(buf.String())
 		if haveStrings != wantStrings {
 			t.Errorf("cache strings mismatch:\nhave: %q\nwant: %q", haveStrings, wantStrings)
diff --git a/src/tests/checkers/basic_test.go b/src/tests/checkers/basic_test.go
index fc3ca70..a0a049d 100644
--- a/src/tests/checkers/basic_test.go
+++ b/src/tests/checkers/basic_test.go
@@ -1349,7 +1349,7 @@ $b = [
 `)
 	test.Expect = []string{
 		"Duplicate array key '1'",
-		"Duplicate array key '\"apple\"'",
+		"Duplicate array key 'apple'",
 		"Duplicate array key '0'",
 		"Duplicate array key '-1'",
 		"Duplicate array key '2'",