inspection: phpdoc type to typehint

src/linter/quickfix.go

@@ -5,6 +5,7 @@ import (
 	"fmt"
 
 	"github.com/VKCOM/noverify/src/ir"
+	"github.com/VKCOM/noverify/src/phpdoc"
 	"github.com/VKCOM/noverify/src/quickfix"
 	"github.com/VKCOM/noverify/src/workspace"
 )
@@ -47,6 +48,22 @@ func (g *QuickFixGenerator) NullForNotNullableProperty(prop *ir.PropertyStmt) qu
 	}
 }
 
+func (g *QuickFixGenerator) FunctionParamTypeReplacementFromTypeHint(prop *ir.Parameter, variableWithType string) quickfix.TextEdit {
+	return quickfix.TextEdit{
+		StartPos:    prop.Position.StartPos,
+		EndPos:      prop.Position.EndPos,
+		Replacement: variableWithType,
+	}
+}
+
+func (g *QuickFixGenerator) RemoveParamTypeHint(prop *phpdoc.TypeVarCommentPart) quickfix.TextEdit {
+	return quickfix.TextEdit{
+		StartPos:    int(prop.Type.Expr.Begin),
+		EndPos:      int(prop.Type.Expr.End),
+		Replacement: "",
+	}
+}
+
 func (g *QuickFixGenerator) GetType(node ir.Node, isFunctionName, nodeText string, isNegative bool) quickfix.TextEdit {
 	pos := ir.GetPosition(node)
 

src/linter/report.go

@@ -16,6 +16,16 @@ const (
 
 func addBuiltinCheckers(reg *CheckersRegistry) {
 	allChecks := []CheckerInfo{
+		{
+			Name:     "implicitParamType",
+			Default:  true,
+			Quickfix: true,
+			Comment:  `Report implicitly specifying the type of a parameter.`,
+			Before: `/* @param $str string */
+function test($str){}`,
+			After: `function test(string $str){}`,
+		},
+
 		{
 			Name:     "stripTags",
 			Default:  true,

src/linter/root_checker.go

@@ -583,9 +583,48 @@ func (r *rootChecker) CheckTypeHintNode(n ir.Node, place string) {
 	})
 }
 
+func (r *rootChecker) CheckFuncParamTypeHint(param *ir.Parameter, phpDocParamTypes phpdoctypes.ParamsMap) {
+	if param.AttrGroups != nil || param.Variadic || param.Variable.Name == "closure" || param.Variable.Name == "referent" || param.Variable.Name == "callback" || types.IsObject(param.Variable.Name) {
+		return // callback referent newThis value
+	}
+
+	var phpDocType types.Map
+
+	if phpDocParamType, ok := phpDocParamTypes[param.Variable.Name]; ok {
+		phpDocType = phpDocParamType.Typ
+
+		var allTypes = phpDocType.GetTypes()
+		if len(allTypes) > 1 || allTypes == nil {
+			return
+		}
+
+		switch paramType := param.VariableType.(type) {
+		case *ir.Name:
+			if paramType != nil {
+				// TODO: quickFix -> remove @param from typeHint
+				return
+			}
+		case *ir.Identifier:
+			return
+		}
+
+		for _, typ := range phpDocType.GetTypes() {
+			if !types.IsTrivial(typ) || types.IsClass(typ) || types.IsArray(typ) || types.IsShape(typ) || typ == "mixed" {
+				continue
+			}
+
+			var varDollar = "$" + param.Variable.Name
+			var variableWithType = typ + " " + varDollar
+			r.walker.Report(param, LevelWarning, "implicitParamType", "Type for %s can be wrote explicitly from typeHint", varDollar)
+			r.walker.addQuickFix("implicitParamType", r.quickfix.FunctionParamTypeReplacementFromTypeHint(param, variableWithType))
+		}
+	}
+}
+
 func (r *rootChecker) CheckFuncParams(funcName *ir.Identifier, params []ir.Node, funcParams parseFuncParamsResult, phpDocParamTypes phpdoctypes.ParamsMap) {
 	for _, param := range params {
 		r.checkFuncParam(param.(*ir.Parameter))
+		r.CheckFuncParamTypeHint(param.(*ir.Parameter), phpDocParamTypes)
 	}
 
 	r.checkParamsTypeHint(funcName, funcParams, phpDocParamTypes)

src/tests/checkers/basic_test.go

@@ -555,6 +555,8 @@ function f1() {}
 		`Void type can only be used as a standalone type for the return type`,
 		`Void type can only be used as a standalone type for the return type`,
 		`Void type can only be used as a standalone type for the return type`,
+		`Type for $x can be wrote explicitly from typeHint`,
+		`Type for $y can be wrote explicitly from typeHint`,
 	}
 	test.RunAndMatch()
 }

src/tests/checkers/oop_test.go

@@ -765,7 +765,9 @@ func TestStaticResolutionInsideSameClass(t *testing.T) {
 }
 
 func TestStaticResolutionInsideOtherStaticResolution(t *testing.T) {
-	linttest.SimpleNegativeTest(t, `<?php
+	test := linttest.NewSuite(t)
+
+	test.AddFile(`<?php
 	class SomeMainClass extends ParentClass {
 		/**
 		* @var string
@@ -807,6 +809,9 @@ func TestStaticResolutionInsideOtherStaticResolution(t *testing.T) {
 		$result .= $objectB->testProperty;
 		echo $result;
 	}`)
+
+	test.Expect = []string{"Type for $testProperty can be wrote explicitly from typeHint"}
+	test.RunAndMatch()
 }
 
 func TestInheritanceLoop(t *testing.T) {

src/tests/checkers/paramClobber_test.go

@@ -18,7 +18,8 @@ function f($x, $y) {
 }
 
 func TestParamClobberReferenced(t *testing.T) {
-	linttest.SimpleNegativeTest(t, `<?php
+	test := linttest.NewSuite(t)
+	test.AddFile(`<?php
 /**
  * @param mixed[] $x
  */
@@ -27,6 +28,8 @@ function f(array $x) {
   return $x;
 }
 `)
+	test.Expect = []string{"Type for $x can be wrote explicitly from typeHint"}
+	test.RunAndMatch()
 }
 
 func TestParamClobberConditional(t *testing.T) {

src/tests/checkers/phpdoc_test.go

@@ -367,6 +367,10 @@ func TestPHPDocSyntax(t *testing.T) {
 		`Expected a type, found '-'; if you want to express 'any' type, use 'mixed'`,
 		`Malformed @param $a tag (maybe type is missing?)`,
 		`Malformed @param tag (maybe var is missing?)`,
+		`Type for $x can be wrote explicitly from typeHint`,
+		`Type for $y can be wrote explicitly from typeHint`,
+		`Type for $z can be wrote explicitly from typeHint`,
+		`Type for  can be wrote explicitly from typeHint`,
 	}
 	test.RunAndMatch()
 }
@@ -445,6 +449,7 @@ func TestPHPDocType(t *testing.T) {
 		`Use bool type instead of boolean`,
 		`Nullable syntax is ?T, not T?`,
 		`Array syntax is T[], not []T`,
+		`Type for $x1 can be wrote explicitly from typeHint`,
 	}
 	test.RunAndMatch()
 }
@@ -522,6 +527,10 @@ function f2($a) {
 		`Multiline PHPDoc comment should start with /**, not /*`,
 		`Multiline PHPDoc comment should start with /**, not /*`,
 		`Multiline PHPDoc comment should start with /**, not /*`,
+		`Type for $a can be wrote explicitly from typeHint`,
+		`Type for $a can be wrote explicitly from typeHint`,
+		`Type for $item can be wrote explicitly from typeHint`,
+		`Type for $item2 can be wrote explicitly from typeHint`,
 	}
 	test.RunAndMatch()
 }

src/tests/checkers/printf_test.go

@@ -37,6 +37,7 @@ function f($s, array $a) {
 		`'-flag expects a char, found end of string`,
 		`potential array to string conversion`,
 		`potential array to string conversion`,
+		`Type for $a can be wrote explicitly from typeHint`,
 	}
 	test.RunAndMatch()
 }
@@ -72,6 +73,7 @@ function f($s, array $a) {
 		`'-flag expects a char, found end of string`,
 		`potential array to string conversion`,
 		`potential array to string conversion`,
+		`Type for $a can be wrote explicitly from typeHint`,
 	}
 	test.RunAndMatch()
 }

src/tests/checkers/shape_test.go

@@ -134,7 +134,8 @@ echo $t->good6['y']->value;
 }
 
 func TestShapeReturn(t *testing.T) {
-	linttest.SimpleNegativeTest(t, `<?php
+	test := linttest.NewSuite(t)
+	test.AddFile(`<?php
 class MyList {
   /** @var \MyList */
   public $next;
@@ -146,10 +147,15 @@ function new_shape1() { return []; }
 $s1 = new_shape1();
 echo $s1['list']->next->next;
 `)
+	test.Expect = []string{
+		`Type for $next can be wrote explicitly from typeHint`,
+	}
+	test.RunAndMatch()
 }
 
 func TestTuple(t *testing.T) {
-	linttest.SimpleNegativeTest(t, `<?php
+	test := linttest.NewSuite(t)
+	test.AddFile(`<?php
 class Box { public $value; }
 
 class T {
@@ -167,4 +173,8 @@ $t = new T();
 echo $t->good2[0]->value;
 echo $t->good3[1][0]->value;
 `)
+	test.Expect = []string{
+		`Type for $good1 can be wrote explicitly from typeHint`,
+	}
+	test.RunAndMatch()
 }

src/tests/checkers/typehint_test.go

@@ -133,3 +133,63 @@ function f2() {
 	}
 	linttest.RunFilterMatch(test, "typeHint")
 }
+
+func TestTypeHintToFunParam(t *testing.T) {
+	test := linttest.NewSuite(t)
+	test.AddFile(`<?php
+declare(strict_types = 1);
+/* @param $str string */
+function test($str){}
+`)
+	test.Expect = []string{
+		`Non-canonical order of variable and type`,
+		`Type for $str can be wrote explicitly from typeHint`,
+	}
+	test.RunAndMatch()
+}
+
+func TestTypeHintClassAndFunc(t *testing.T) {
+	test := linttest.NewSuite(t)
+	test.AddFile(`<?php
+declare(strict_types = 1);
+
+class SimpleClass
+{
+
+/**
+ * This is what the variable does. The var line contains the type stored in this variable.
+ * @var string
+ */
+private string $foo;
+
+
+/**
+ * This is what the variable does. The var line contains the type stored in this variable.
+ * @var string
+ */
+private $foo2 ;
+
+/**
+ * @param $str string
+ * @param $str2 string
+ */
+function test($str, string $str2){
+}
+}
+
+/**
+ * @param $str string
+ */
+function test2($str){
+}
+`)
+	test.Expect = []string{
+		`Specify the access modifier for \SimpleClass::test method explicitly`,
+		`Non-canonical order of variable and type `,
+		`Non-canonical order of variable and type`,
+		`Type for $str can be wrote explicitly from typeHint`,
+		`Non-canonical order of variable and type`,
+		`Type for $str can be wrote explicitly from typeHint`,
+	}
+	test.RunAndMatch()
+}

src/tests/golden/testdata/embeddedrules/golden.txt

@@ -67,6 +67,9 @@ WARNING bitwiseOps: Used | bitwise operator over bool operands, perhaps || is in
 MAYBE   callSimplify: Could simplify to array_key_exists('abc', $array) at testdata/embeddedrules/callSimplify.php:7
     $_ = in_array('abc', array_keys($array)); // bad
          ^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^
+WARNING implicitParamType: Type for $array can be wrote explicitly from typeHint at testdata/embeddedrules/callSimplify.php:6
+function in_array_over_array_keys(array $array) {
+                                        ^^^^^^
 MAYBE   callSimplify: Could simplify to $str[$index] at testdata/embeddedrules/callSimplify.php:14
     $_ = substr($str, $index, 1);
          ^^^^^^^^^^^^^^^^^^^^^^^
@@ -82,6 +85,12 @@ MAYBE   callSimplify: Could simplify to $array[] = $val at testdata/embeddedrule
 MAYBE   callSimplify: Could simplify to $array[] = 10 at testdata/embeddedrules/callSimplify.php:28
     array_push($array, 10);
     ^^^^^^^^^^^^^^^^^^^^^^
+WARNING implicitParamType: Type for $array can be wrote explicitly from typeHint at testdata/embeddedrules/callSimplify.php:26
+function some_array_push(array $array, int $val) {
+                               ^^^^^^
+WARNING implicitParamType: Type for $val can be wrote explicitly from typeHint at testdata/embeddedrules/callSimplify.php:26
+function some_array_push(array $array, int $val) {
+                               ^^^^^^
 WARNING indexingSyntax: a{i} indexing is deprecated since PHP 7.4, use a[i] instead at testdata/embeddedrules/indexingSyntax.php:14
     $_ = $a{0};
          ^^^^^
@@ -157,6 +166,15 @@ WARNING offBy1: Probably intended to use sizeof-1 as an index at testdata/embedd
 WARNING offBy1: Probably intended to use count-1 as an index at testdata/embeddedrules/offBy1.php:14
   if ($tabs[count($tabs)] == "") {
       ^^^^^^^^^^^^^^^^^^^
+WARNING implicitParamType: Type for $xs can be wrote explicitly from typeHint at testdata/embeddedrules/offBy1.php:7
+function test_countindex_bad(array $xs, array $tabs) {
+                                   ^^^
+WARNING implicitParamType: Type for $tabs can be wrote explicitly from typeHint at testdata/embeddedrules/offBy1.php:7
+function test_countindex_bad(array $xs, array $tabs) {
+                                   ^^^
+WARNING implicitParamType: Type for $xs can be wrote explicitly from typeHint at testdata/embeddedrules/offBy1.php:24
+function test_countindex_good(array $xs) {
+                                    ^^^
 WARNING precedence: == has higher precedence than & at testdata/embeddedrules/precedence.php:4
   $_ = 0 == $mask & $x;
        ^^^^^^^^^^^^^^^
@@ -310,3 +328,6 @@ MAYBE   ternarySimplify: Could rewrite as `$x_arr[10] ?? null` at testdata/embed
 MAYBE   ternarySimplify: Could rewrite as `(bool)($flags & SOME_MASK)` at testdata/embeddedrules/ternarySimplify.php:46
     sink(($flags & SOME_MASK) ? true : false);
          ^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^
+WARNING implicitParamType: Type for $flags can be wrote explicitly from typeHint at testdata/embeddedrules/ternarySimplify.php:45
+function ternarySimplify_issue540($flags) {
+                                  ^^^^^^