Repository: VKCOM/noverify
Pull request #762 (2020-09-03 10:44:39)

linter: improve list(...)=$a type inference for arrays
Like PhpStorm, collect all array types and assign a union
type of all element types to the list variables.

	$xs = [1, 2];
	list ($a, $b) = $xs;
	// $a is int
	// $b is int

This way we get more types information.

Signed-off-by: Iskander Sharipov <quasilyte@gmail.com>


Patch:
diff --git a/src/linter/block.go b/src/linter/block.go
index 7092c8d..f9fa1b5 100644
--- a/src/linter/block.go
+++ b/src/linter/block.go
@@ -1674,17 +1674,56 @@ func (b *BlockWalker) handleAssignShapeToList(items []*ir.ArrayItemExpr, info me
 		} else {
 			tp = prop.Typ
 		}
-		b.handleVariableNode(item.Val, tp, "assign")
+		b.handleVariableNode(item.Val, tp, "list-assign")
+	}
+}
+
+func (b *BlockWalker) handleAssignList(list *ir.ListExpr, rhs ir.Node) {
+	typ := solver.ExprType(b.ctx.sc, b.r.ctx.st, rhs)
+
+	// TODO: test if we can prealloc elemTypes to const size hint like 2
+	// and get stack allocation which will help to avoid the unwanted heap allocs.
+	// Hint: only const (literal) size hints work for this.
+	// Hint: check the compiler output to see whether elemTypes "escape" or not.
+	//
+	// We store meta.Type instead of string to avoid the need to do strings.Join
+	// when we want to create a TypesMap.
+	var elemTypes []meta.Type
+	var shapeType string
+	typ.Iterate(func(typ string) {
+		switch {
+		case meta.IsShapeType(typ):
+			shapeType = typ
+		case meta.IsArrayType(typ):
+			elemType := strings.TrimSuffix(typ, "[]")
+			elemTypes = append(elemTypes, meta.Type{Elem: elemType})
+		}
+	})
+
+	// Try to handle it as a shape assignment.
+	if shapeType != "" {
+		class, ok := meta.Info.GetClass(shapeType)
+		if ok {
+			b.handleAssignShapeToList(list.Items, class)
+			return
+		}
 	}
-}
 
-func (b *BlockWalker) handleAssignList(items []*ir.ArrayItemExpr, info meta.ClassInfo, isShape bool) {
-	if isShape {
-		b.handleAssignShapeToList(items, info)
-	} else {
-		for _, item := range items {
-			b.handleVariableNode(item.Val, meta.NewTypesMap("unknown_from_list"), "assign")
+	// Try to handle it as an array assignment.
+	if len(elemTypes) != 0 {
+		elemTypeMap := meta.NewTypesMapFromTypes(elemTypes).Immutable()
+		for _, item := range list.Items {
+			b.handleVariableNode(item.Val, elemTypeMap, "list-assign")
 		}
+		return
+	}
+
+	// Fallback: define vars with unknown types.
+	//
+	// TODO: shouldn't it be a mixed type? I would prefer a "mixed" type here
+	// and "unknown from list" reason.
+	for _, item := range list.Items {
+		b.handleVariableNode(item.Val, meta.NewTypesMap("unknown_from_list"), "list-assign")
 	}
 }
 
@@ -1715,23 +1754,7 @@ func (b *BlockWalker) handleAssign(a *ir.Assign) bool {
 			return true
 		}
 
-		tp := solver.ExprType(b.ctx.sc, b.r.ctx.st, a.Expression)
-		var shapeType string
-		tp.Iterate(func(t string) {
-			if meta.IsShapeType(t) {
-				shapeType = t
-			}
-		})
-
-		var class meta.ClassInfo
-		var ok bool
-		var isShape bool
-		if shapeType != "" {
-			class, ok = meta.Info.GetClass(shapeType)
-			isShape = ok
-		}
-
-		b.handleAssignList(v.Items, class, isShape)
+		b.handleAssignList(v, a.Expression)
 	case *ir.PropertyFetchExpr:
 		v.Property.Walk(b)
 		sv, ok := v.Variable.(*ir.SimpleVar)


Test patch:
diff --git a/src/tests/exprtype/exprtype_test.go b/src/tests/exprtype/exprtype_test.go
index dd5116b..8d285b7 100644
--- a/src/tests/exprtype/exprtype_test.go
+++ b/src/tests/exprtype/exprtype_test.go
@@ -46,6 +46,65 @@ func init() {
 	})
 }
 
+func TestExprTypeListOverArray(t *testing.T) {
+	code := `<?php
+/**
+ * @param int[] $xs
+ */
+function ints($xs) {
+  list ($a, $b) = $xs;
+  exprtype($a, 'int');
+  exprtype($b, 'int');
+}
+
+/**
+ * @param string[]|false $xs
+ */
+function strings_or_false($xs) {
+  list ($a, $b) = $xs;
+  exprtype($a, 'string');
+  exprtype($b, 'string');
+}
+
+/**
+ * @param null|\Foo[]|int $xs
+ */
+function null_or_foos_or_int($xs) {
+  list ($a, $b) = $xs;
+  exprtype($a, '\Foo');
+  exprtype($b, '\Foo');
+}
+
+/**
+ * @param int[]|string[] $xs
+ */
+function ints_or_strings($xs) {
+  list ($a, $b) = $xs;
+  exprtype($a, 'int|string');
+  exprtype($b, 'int|string');
+}
+
+/**
+ * @param mixed[]|string[]|int[]|false $xs
+ */
+function mixeds_or_strings_or_ints_or_false($xs) {
+  list ($a, $b) = $xs;
+  exprtype($a, 'int|mixed|string');
+  exprtype($b, 'int|mixed|string');
+}
+
+/**
+ * @param int|float|null $xs
+ */
+function not_an_array($xs) {
+  list ($a, $b) = $xs;
+  exprtype($a, 'unknown_from_list');
+  exprtype($b, 'unknown_from_list');
+}
+`
+	runExprTypeTest(t, &exprTypeTestParams{code: code})
+}
+
 func TestExprTypeForeachKey(t *testing.T) {
 	code := `<?php
 $xs = [[1], [2]];
diff --git a/src/tests/golden/testdata/underscore/golden.txt b/src/tests/golden/testdata/underscore/golden.txt
index e302d71..c9170ab 100644
--- a/src/tests/golden/testdata/underscore/golden.txt
+++ b/src/tests/golden/testdata/underscore/golden.txt
@@ -106,16 +106,16 @@ INFO    unused: Variable __ is unused (use $_ to ignore this inspection) at test
 INFO    unused: Variable args is unused (use $_ to ignore this inspection) at testdata/underscore/underscore.php:615
     $args = self::_wrapArgs(func_get_args(), 1);
     ^^^^^
-ERROR   undefined: Property {\__|null|unknown_from_list}->isEqual does not exist at testdata/underscore/underscore.php:723
+ERROR   undefined: Property {\__|mixed|null}->isEqual does not exist at testdata/underscore/underscore.php:723
       if(is_object($a) && isset($a->isEqual)) return self::_wrap($a->isEqual($b));
                                     ^^^^^^^
-ERROR   undefined: Property {null|unknown_from_list}->isEqual does not exist at testdata/underscore/underscore.php:724
+ERROR   undefined: Property {mixed|null}->isEqual does not exist at testdata/underscore/underscore.php:724
       if(is_object($b) && isset($b->isEqual)) return self::_wrap($b->isEqual($a));
                                     ^^^^^^^
-ERROR   undefined: Call to undefined method {null|unknown_from_list}->isEqual() at testdata/underscore/underscore.php:724
+ERROR   undefined: Call to undefined method {mixed|null}->isEqual() at testdata/underscore/underscore.php:724
       if(is_object($b) && isset($b->isEqual)) return self::_wrap($b->isEqual($a));
                                                                      ^^^^^^^
-MAYBE   arrayAccess: Array access to non-array type \__|null|unknown_from_list at testdata/underscore/underscore.php:725
+MAYBE   arrayAccess: Array access to non-array type \__|mixed|null at testdata/underscore/underscore.php:725
       if(is_array($a) && array_key_exists('isEqual', $a)) return self::_wrap($a['isEqual']($b));
                                                                              ^^
 ERROR   undefined: Property {mixed}->_uniqueId does not exist at testdata/underscore/underscore.php:822