Repository: VKCOM/noverify
Pull request #1253 (2025-04-16 02:01:32)

linter: function param type mismatch with phpdoc
We have rule for nullability and i wanna extend checking the compliance of what is in typehints and phpdoc:

- [x]  array support
- [x]  union types support
- [x]  callable support
- [x] classic types
- [x] interfaces
- [x] classes




Patch:
diff --git a/src/linter/block.go b/src/linter/block.go
index ec2c899..fb8919c 100644
--- a/src/linter/block.go
+++ b/src/linter/block.go
@@ -551,17 +551,167 @@ func (b *blockWalker) handleAndCheckGlobalStmt(s *ir.GlobalStmt) {
 	}
 }
 
-func (b *blockWalker) CheckParamNullability(params []ir.Node) {
+func (b *blockWalker) checkPhpDocTypesWithTypeHints(param *ir.Parameter, phpDocParamTypes map[string]string) {
+	if len(phpDocParamTypes) == 0 {
+		return
+	}
+
+	// Build the lookup key, with fallback if "&$" did not find
+	name := param.Variable.Name
+	key := "$" + name
+	if param.ByRef {
+		if _, ok := phpDocParamTypes["&$"+name]; ok {
+			key = "&$" + name
+		}
+	}
+	rawDoc := strings.TrimSpace(phpDocParamTypes[key])
+	if rawDoc == "" && param.ByRef {
+		rawDoc = strings.TrimSpace(phpDocParamTypes["$"+name])
+	}
+
+	if rawDoc == "" {
+		return
+	}
+
+	b.checkDiffPhpDocWithTypeHints(param.VariableType, name, rawDoc, b.linter.classParseState().Uses)
+}
+
+func (b *blockWalker) checkDiffPhpDocWithTypeHints(
+	node ir.Node,
+	paramName, rawPhpDocType string,
+	uses map[string]string,
+) {
+	// 1) Normalization
+	doc := strings.TrimSpace(rawPhpDocType)
+	doc = strings.TrimPrefix(doc, `\`)
+	doc = strings.TrimSpace(doc)
+
+	// 2) Unpack Nullable from AST
+	var isNullable bool
+	if n, ok := node.(*ir.Nullable); ok {
+		isNullable = true
+		// remove ? and null for type checking
+		doc = strings.TrimPrefix(doc, "?")
+		parts := strings.Split(doc, "|")
+		clean := make([]string, 0, len(parts))
+		for _, p := range parts {
+			p = strings.TrimSpace(p)
+			if p != "" && p != "null" {
+				clean = append(clean, p)
+			}
+		}
+		doc = strings.Join(clean, "|")
+		node = n.Expr
+	}
+
+	// 3) AST type
+	var typeValue string
+	switch t := node.(type) {
+	case *ir.Name:
+		typeValue = t.Value
+	case *ir.Identifier:
+		typeValue = t.Value
+	default:
+		return
+	}
+
+	// 4) Getting alias
+	alias := uses[typeValue]
+	typeValue = strings.TrimPrefix(typeValue, `\`)
+
+	if alias == "" {
+		alias = uses[typeValue]
+	}
+
+	alias = strings.TrimPrefix(alias, `\`)
+
+	// 5) Helper
+	report := func() {
+		b.linter.report(
+			node, LevelWarning, "funcParamTypeMissMatch",
+			"param $%s miss matched with phpdoc type <<%s>>",
+			paramName, rawPhpDocType,
+		)
+	}
+
+	// 6) if hint nullable, but PHPDoc has no '?' or 'null' - report
+	if isNullable && !strings.Contains(rawPhpDocType, "?") && !strings.Contains(rawPhpDocType, "null") {
+		report()
+		return
+	}
+
+	// 7) callable
+	if typeValue == "callable" {
+		if !strings.HasPrefix(doc, "callable") {
+			report()
+		}
+		return
+	}
+
+	// 8) Union
+	if strings.Contains(doc, "|") {
+		parts := strings.Split(doc, "|")
+		for i := range parts {
+			parts[i] = strings.TrimSpace(parts[i])
+		}
+		// if union include null, but hint not nullable - report
+		if !isNullable {
+			for _, p := range parts {
+				if p == "null" {
+					report()
+					return
+				}
+			}
+		}
+		// Any contains in union
+		for _, p := range parts {
+			// array[]
+			if typeValue == "array" && (p == "array" || strings.HasSuffix(p, "[]")) {
+				return
+			}
+			// boolean-boolean
+			if types.IsBoolean(p) && types.IsBoolean(typeValue) {
+				return
+			}
+			// 1-1 or alias
+			if p == typeValue || p == alias {
+				return
+			}
+		}
+		report()
+		return
+	}
+
+	// 9) Single/general: array / boolean / 1-1 / alias
+	switch {
+	case typeValue == "array":
+		if doc != "array" && !strings.HasSuffix(doc, "[]") {
+			report()
+		}
+	case types.IsBoolean(doc) && types.IsBoolean(typeValue):
+		// ok
+	case doc == typeValue:
+		// ok
+	case alias != "" && doc == alias:
+		// ok
+	default:
+		report()
+	}
+}
+
+func (b *blockWalker) CheckParamNullability(params []ir.Node, phpDocParamTypes map[string]string) {
 	for _, param := range params {
 		if p, ok := param.(*ir.Parameter); ok {
 			var paramType ir.Node
-			paramType, paramOk := p.VariableType.(*ir.Name)
-			if !paramOk {
-				paramIdentifier, paramIdentifierOk := p.VariableType.(*ir.Identifier)
-				if !paramIdentifierOk {
-					continue
-				}
-				paramType = paramIdentifier
+
+			b.checkPhpDocTypesWithTypeHints(p, phpDocParamTypes)
+			switch typ := p.VariableType.(type) {
+			case *ir.Name, *ir.Identifier:
+				paramType = typ
+			case *ir.Nullable:
+				continue
+			default:
+				continue
 			}
 
 			paramName, ok := paramType.(*ir.Name)
@@ -586,9 +736,28 @@ func (b *blockWalker) CheckParamNullability(params []ir.Node) {
 	}
 }
 
+func (b *blockWalker) getParamsTypesFromPhpDoc(doc phpdoc.Comment) map[string]string {
+	if len(doc.Parsed) == 0 {
+		return nil
+	}
+	phpDocParamTypes := make(map[string]string)
+
+	for _, part := range doc.Parsed {
+		if part.Name() == "param" {
+			param, ok := part.(*phpdoc.TypeVarCommentPart)
+			if ok {
+				phpDocParamTypes[param.Var] = param.Type.Expr.Value
+			}
+		}
+	}
+	return phpDocParamTypes
+}
+
 func (b *blockWalker) handleFunction(fun *ir.FunctionStmt) bool {
 	if b.ignoreFunctionBodies {
-		b.CheckParamNullability(fun.Params)
+		phpDocParamTypes := b.getParamsTypesFromPhpDoc(fun.Doc)
+
+		b.CheckParamNullability(fun.Params, phpDocParamTypes)
 		return false
 	}
 
@@ -1628,7 +1797,8 @@ func (b *blockWalker) handleCallArgs(args []ir.Node, fn meta.FuncInfo) {
 				ArgTypes: funcArgTypes,
 			}
 
-			b.CheckParamNullability(a.Params)
+			phpDocParamTypes := b.getParamsTypesFromPhpDoc(a.Doc)
+			b.CheckParamNullability(a.Params, phpDocParamTypes)
 			b.enterClosure(a, isInstance, typ, closureSolver)
 		default:
 			a.Walk(b)
diff --git a/src/linter/block_linter.go b/src/linter/block_linter.go
index ace77db..61bc6c3 100644
--- a/src/linter/block_linter.go
+++ b/src/linter/block_linter.go
@@ -44,10 +44,12 @@ func (b *blockLinter) enterNode(n ir.Node) {
 		b.checkFunctionCall(n)
 
 	case *ir.ArrowFunctionExpr:
-		b.walker.CheckParamNullability(n.Params)
+		phpDocParamTypes := b.walker.getParamsTypesFromPhpDoc(n.Doc)
+		b.walker.CheckParamNullability(n.Params, phpDocParamTypes)
 
 	case *ir.ClosureExpr:
-		b.walker.CheckParamNullability(n.Params)
+		phpDocParamTypes := b.walker.getParamsTypesFromPhpDoc(n.Doc)
+		b.walker.CheckParamNullability(n.Params, phpDocParamTypes)
 
 	case *ir.MethodCallExpr:
 		b.checkMethodCall(n)
@@ -238,7 +240,8 @@ func (b *blockLinter) checkTrait(n *ir.TraitStmt) {
 	for _, stmt := range n.Stmts {
 		method, ok := stmt.(*ir.ClassMethodStmt)
 		if ok {
-			b.walker.CheckParamNullability(method.Params)
+			phpDocParamTypes := b.walker.getParamsTypesFromPhpDoc(method.Doc)
+			b.walker.CheckParamNullability(method.Params, phpDocParamTypes)
 		}
 	}
 }
@@ -252,7 +255,8 @@ func (b *blockLinter) checkClass(class *ir.ClassStmt) {
 		switch value := stmt.(type) {
 		case *ir.ClassMethodStmt:
 			members = append(members, classMethod)
-			b.walker.CheckParamNullability(value.Params)
+			phpDocParamTypes := b.walker.getParamsTypesFromPhpDoc(value.Doc)
+			b.walker.CheckParamNullability(value.Params, phpDocParamTypes)
 		default:
 			members = append(members, classOtherMember)
 		}
@@ -1629,7 +1633,8 @@ func (b *blockLinter) checkInterfaceStmt(iface *ir.InterfaceStmt) {
 					b.report(x, LevelWarning, "nonPublicInterfaceMember", "'%s' can't be %s", methodName, modifier.Value)
 				}
 			}
-			b.walker.CheckParamNullability(x.Params)
+			phpDocParamTypes := b.walker.getParamsTypesFromPhpDoc(x.Doc)
+			b.walker.CheckParamNullability(x.Params, phpDocParamTypes)
 		case *ir.ClassConstListStmt:
 			for _, modifier := range x.Modifiers {
 				if strings.EqualFold(modifier.Value, "private") || strings.EqualFold(modifier.Value, "protected") {
diff --git a/src/linter/report.go b/src/linter/report.go
index 0af6f29..1ea600d 100644
--- a/src/linter/report.go
+++ b/src/linter/report.go
@@ -1337,6 +1337,31 @@ class Main {
 			Before:   `if (gettype($a) == "string") { ... }`,
 			After:    `if (is_string($a)) { ... }`,
 		},
+
+		{
+			Name:     "funcParamTypeMissMatch",
+			Default:  true,
+			Quickfix: false,
+			Comment:  `Report function typehint and phpdoc mismatch.`,
+			Before: `
+/**
+ *
+ * @param ?string $name
+ * @return string
+ */
+function wrongParam(string $name): string {
+    return "Hello, $name";
+}`,
+			After: `
+/**
+ *
+ * @param ?string $name
+ * @return string
+ */
+function wrongParam(?string $name): string {
+    return "Hello, $name";
+}`,
+		},
 	}
 
 	for _, info := range allChecks {
diff --git a/src/types/map.go b/src/types/map.go
index dafe8f3..07a10e0 100644
--- a/src/types/map.go
+++ b/src/types/map.go
@@ -280,7 +280,7 @@ func (m Map) IsBoolean() bool {
 	for typ := range m.m {
 		if UnwrapElemOf(typ) == "bool" || strings.HasSuffix(typ, "bool") ||
 			strings.HasSuffix(typ, "true") ||
-			strings.HasSuffix(typ, "false") {
+			strings.HasSuffix(typ, "false") || strings.HasSuffix(typ, "boolean") {
 			return true
 		}
 	}
diff --git a/src/types/predicates.go b/src/types/predicates.go
index 56d330c..93ada84 100644
--- a/src/types/predicates.go
+++ b/src/types/predicates.go
@@ -12,6 +12,10 @@ func IsShape(s string) bool {
 	return strings.HasPrefix(s, `\shape$`)
 }
 
+func IsBoolean(s string) bool {
+	return strings.HasPrefix(s, "bool") || strings.HasPrefix(s, "boolean")
+}
+
 func IsClosure(s string) bool {
 	return strings.HasPrefix(s, `\Closure`)
 }


Test patch:
diff --git a/src/tests/checkers/func_params_type_mismatch_phpdoc_test.go b/src/tests/checkers/func_params_type_mismatch_phpdoc_test.go
new file mode 100644
index 0000000..c886493
--- /dev/null
+++ b/src/tests/checkers/func_params_type_mismatch_phpdoc_test.go
@@ -0,0 +1,327 @@
+package checkers_test
+
+import (
+	"testing"
+
+	"github.com/VKCOM/noverify/src/linttest"
+)
+
+func TestFunctionParamTypeMismatch(t *testing.T) {
+	test := linttest.NewSuite(t)
+	test.AddFile(`<?php
+// Functions testing parameter type mismatch between phpdoc and typehint
+
+/**
+ * Function with correct non-nullable parameter: both phpdoc and typehint are non-nullable.
+ *
+ * @param int $a
+ */
+function funcCorrect1(int $a) {
+}
+
+/**
+ * Function with correct nullable parameter: both phpdoc and typehint are nullable.
+ *
+ * @param int|null $b
+ */
+function funcCorrect2(?int $b) {
+}
+
+/**
+ * Function with error: phpdoc specifies non-nullable int, but typehint is nullable (?int).
+ *
+ * @param int $c
+ */
+function funcError1(?int $c) {
+}
+
+/**
+ * Function with error: phpdoc specifies nullable int (int|null), but typehint is non-nullable int.
+ *
+ * @param int|null $d
+ */
+function funcError2(int $d) {
+}
+`)
+	test.Expect = []string{
+		`param $c miss matched with phpdoc type <<int>>`,
+		`param $d miss matched with phpdoc type <<int|null>>`,
+	}
+	test.RunAndMatch()
+}
+
+func TestMethodParamTypeMismatch(t *testing.T) {
+	test := linttest.NewSuite(t)
+	test.AddFile(`<?php
+// Class methods testing parameter type mismatch between phpdoc and typehint
+
+class Sample {
+  /**
+   * Method with correct non-nullable parameter: both phpdoc and typehint match.
+   *
+   * @param string $name
+   */
+  public function methodCorrect(string $name) {
+  }
+
+  /**
+   * Method with error: phpdoc specifies nullable string (string|null), but typehint is non-nullable.
+   *
+   * @param string|null $title
+   */
+  public function methodError1(string $title) {
+  }
+
+  /**
+   * Method with error: phpdoc specifies non-nullable string, but typehint is nullable (?string).
+   *
+   * @param string $desc
+   */
+  public function methodError2(?string $desc) {
+  }
+}
+`)
+	test.Expect = []string{
+		`param $title miss matched with phpdoc type <<string|null>>`,
+		`param $desc miss matched with phpdoc type <<string>>`,
+	}
+	test.RunAndMatch()
+}
+
+func TestIgnoreReturnTypeMismatch(t *testing.T) {
+	test := linttest.NewSuite(t)
+	test.AddFile(`<?php
+// This test ensures that only parameter types are checked,
+// and any mismatches in return types are ignored.
+
+/**
+ * Function with a mismatch in return type between phpdoc and typehint.
+ * The parameter type is correct and should not trigger an error.
+ *
+ * @param int $a
+ * @return int|null
+ */
+function returnMismatch(int $a): int {
+    return $a > 0 ? $a : null;
+}
+
+// Call the function to trigger linting
+returnMismatch(5);
+`)
+	test.Expect = []string{
+		`Expression evaluated but not used`,
+	}
+	test.RunAndMatch()
+}
+
+func TestArrayParamTypeMismatch(t *testing.T) {
+	test := linttest.NewSuite(t)
+	test.AddFile(`<?php
+// Functions testing array parameter types
+
+/**
+ * Function with correct array parameter:
+ * phpdoc specifies "array" and typehint is array.
+ *
+ * @param array $a
+ */
+function funcArrayCorrect1(array $a) {
+}
+
+/**
+ * Function with correct array parameter:
+ * phpdoc specifies "int[]" and typehint is array.
+ *
+ * @param int[] $b
+ */
+function funcArrayCorrect2(array $b) {
+}
+
+/**
+ * Function with error: phpdoc specifies non-nullable "array",
+ * but typehint is nullable array (?array).
+ *
+ * @param array $c
+ */
+function funcArrayError1(?array $c) {
+}
+
+`)
+	test.Expect = []string{
+		`param $c miss matched with phpdoc type <<array>>`,
+	}
+	test.RunAndMatch()
+}
+
+func TestByReferenceCorrect(t *testing.T) {
+	test := linttest.NewSuite(t)
+	test.AddFile(`<?php
+/**
+ * Clear standalone line tokens.
+ *
+ * @param array $nodes  Parsed nodes
+ * @param array $tokens Tokens to be parsed
+ *
+ * @return array|null Resulting indent token, if any
+ */
+function clearStandaloneLines(array &$nodes, array &$tokens) {
+}
+
+`)
+	// No errors expected.
+	test.Expect = []string{}
+	test.RunAndMatch()
+}
+
+func TestByReferenceMismatch(t *testing.T) {
+	test := linttest.NewSuite(t)
+	test.AddFile(`<?php
+/**
+ * Incorrect by-reference parameter:
+ * PHPDoc specifies int, but typehint is array.
+ *
+ * @param int $a
+ */
+function byRefError(array &$a) {
+}
+`)
+	test.Expect = []string{
+		`param $a miss matched with phpdoc type <<int>>`,
+	}
+	test.RunAndMatch()
+}
+
+func TestCallableCorrect(t *testing.T) {
+	test := linttest.NewSuite(t)
+	test.AddFile(`<?php
+/**
+ * Function with callable parameter.
+ *
+ * @param callable(int, string): Boo|Foo $s
+ */
+function testCallable(callable $s) {
+  // Dummy call.
+  $s(1, "test");
+}
+
+testCallable(function($a, $b) {
+  return new stdClass();
+});
+`)
+	test.Expect = []string{}
+	test.RunAndMatch()
+}
+
+func TestInterfaceCorrect(t *testing.T) {
+	test := linttest.NewSuite(t)
+	test.AddFile(`<?php
+interface Responsible {}
+
+/**
+ * Uses Responsible interface value.
+ *
+ * @param \Responsible $r
+ */
+function reference_iface(Responsible $r) {
+}
+
+reference_iface(new class implements Responsible {});
+`)
+	test.Expect = []string{}
+	test.RunAndMatch()
+}
+
+func TestUnionMismatch(t *testing.T) {
+	test := linttest.NewSuite(t)
+	test.AddFile(`<?php
+/**
+ * Function with error: phpdoc specifies nullable int (int|null),
+ * but typehint is non-nullable int.
+ *
+ * @param int|null $d
+ */
+function funcError2(int $d) {
+}
+
+funcError2(5);
+`)
+	test.Expect = []string{
+		`param $d miss matched with phpdoc type <<int|null>>`,
+	}
+	test.RunAndMatch()
+}
+
+func TestNullableCorrect(t *testing.T) {
+	test := linttest.NewSuite(t)
+	test.AddFile(`<?php
+/**
+ * Function with correct nullable parameter.
+ *
+ * @param int|null $d
+ */
+function nullableCorrect(?int $d) {
+}
+
+nullableCorrect(null);
+`)
+	test.Expect = []string{}
+	test.RunAndMatch()
+}
+
+func TestArrayCorrect(t *testing.T) {
+	test := linttest.NewSuite(t)
+	test.AddFile(`<?php
+/**
+ * Function with array parameters.
+ *
+ * @param array $a
+ * @param int[] $b
+ */
+function funcArray(array $a, array $b) {
+}
+
+funcArray([], []);
+`)
+	test.Expect = []string{}
+	test.RunAndMatch()
+}
+
+func TestBoolSynonym(t *testing.T) {
+	test := linttest.NewSuite(t)
+	test.AddFile(`<?php
+/**
+ * Function with boolean type synonym.
+ *
+ * @param boolean $flag
+ */
+function testBool(bool $flag) {
+}
+`)
+	test.Expect = []string{
+		`Use bool type instead of boolean`,
+	}
+	test.RunAndMatch()
+}
+
+func TestAliasNormalization(t *testing.T) {
+	test := linttest.NewSuite(t)
+	test.AddFile(`<?php
+namespace Test;
+
+use A as B;
+
+/**
+ * Function with aliased type.
+ *
+ * @param \A $item
+ */
+function testAlias(B $item) {
+}
+
+`)
+	test.Expect = []string{
+		`Class or interface named \A does not exist`,
+		`Class or interface named \A does not exist`,
+	}
+	test.RunAndMatch()
+}
diff --git a/src/tests/checkers/not_explicit_nullable_types_test.go b/src/tests/checkers/not_explicit_nullable_types_test.go
index e48d8bf..3dd0c5b 100644
--- a/src/tests/checkers/not_explicit_nullable_types_test.go
+++ b/src/tests/checkers/not_explicit_nullable_types_test.go
@@ -18,7 +18,7 @@ function nullableString(?string $a = null) {}
 func TestNotNullableArray(t *testing.T) {
 	test := linttest.NewSuite(t)
 	test.AddFile(`<?php
-/** @param string[] array */
+/** @param string[] $a */
 function nullableArray(array $a = null) {}
 `)
 
diff --git a/src/tests/golden/testdata/mustache/golden.txt b/src/tests/golden/testdata/mustache/golden.txt
index 38e42ad..40ca38a 100644
--- a/src/tests/golden/testdata/mustache/golden.txt
+++ b/src/tests/golden/testdata/mustache/golden.txt
@@ -127,6 +127,9 @@ WARNING notNullSafetyFunctionArgumentVariable: not null safety call in function
 MAYBE   callSimplify: Could simplify to $id[0] at testdata/mustache/src/Mustache/Compiler.php:646
             if (substr($id, 0, 1) === '.') {
                 ^^^^^^^^^^^^^^^^^
+WARNING funcParamTypeMissMatch: param $tree miss matched with phpdoc type <<string>> at testdata/mustache/src/Mustache/Compiler.php:43
+    public function compile($source, array $tree, $name, $customEscape = false, $charset = 'UTF-8', $strictCallables = false, $entityFlags = ENT_COMPAT)
+                                     ^^^^^
 ERROR   classMembersOrder: Constant KLASS must go before methods in the class Mustache_Compiler at testdata/mustache/src/Mustache/Compiler.php:184
     const KLASS = '<?php
     ^^^^^^^