diff --git a/src/linter/quickfix.go b/src/linter/quickfix.go index ef83aa8ad..bb45b0e4b 100644 --- a/src/linter/quickfix.go +++ b/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) diff --git a/src/linter/report.go b/src/linter/report.go index d1914da36..edeb13187 100644 --- a/src/linter/report.go +++ b/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, diff --git a/src/linter/root_checker.go b/src/linter/root_checker.go index 149b767d7..c2b4c6719 100644 --- a/src/linter/root_checker.go +++ b/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) diff --git a/src/tests/checkers/basic_test.go b/src/tests/checkers/basic_test.go index 3594f26c9..58cfe8e1f 100644 --- a/src/tests/checkers/basic_test.go +++ b/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() } diff --git a/src/tests/checkers/oop_test.go b/src/tests/checkers/oop_test.go index 7e6a49888..f22bb2e9b 100644 --- a/src/tests/checkers/oop_test.go +++ b/src/tests/checkers/oop_test.go @@ -765,7 +765,9 @@ func TestStaticResolutionInsideSameClass(t *testing.T) { } func TestStaticResolutionInsideOtherStaticResolution(t *testing.T) { - linttest.SimpleNegativeTest(t, `testProperty; echo $result; }`) + + test.Expect = []string{"Type for $testProperty can be wrote explicitly from typeHint"} + test.RunAndMatch() } func TestInheritanceLoop(t *testing.T) { diff --git a/src/tests/checkers/paramClobber_test.go b/src/tests/checkers/paramClobber_test.go index d7ee55f3a..eeec57029 100644 --- a/src/tests/checkers/paramClobber_test.go +++ b/src/tests/checkers/paramClobber_test.go @@ -18,7 +18,8 @@ function f($x, $y) { } func TestParamClobberReferenced(t *testing.T) { - linttest.SimpleNegativeTest(t, `good6['y']->value; } func TestShapeReturn(t *testing.T) { - linttest.SimpleNegativeTest(t, `next->next; `) + test.Expect = []string{ + `Type for $next can be wrote explicitly from typeHint`, + } + test.RunAndMatch() } func TestTuple(t *testing.T) { - linttest.SimpleNegativeTest(t, `good2[0]->value; echo $t->good3[1][0]->value; `) + test.Expect = []string{ + `Type for $good1 can be wrote explicitly from typeHint`, + } + test.RunAndMatch() } diff --git a/src/tests/checkers/typehint_test.go b/src/tests/checkers/typehint_test.go index db14990a3..b8258d2e2 100644 --- a/src/tests/checkers/typehint_test.go +++ b/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(`connection = @ftp_ssl_connect($this->getHost(), $this->getPort(), $this->getTimeout()); ^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^ @@ -22,6 +58,18 @@ MAYBE regexpSimplify: May re-write '/^total [0-9]*$/' as '/^total \d*$/' at te WARNING errorSilence: Don't use @, silencing errors is bad practice at testdata/flysystem/src/Adapter/Ftp.php:570 $response = @ftp_raw($this->connection, trim($command)); ^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^ +WARNING implicitParamType: Type for $transferMode can be wrote explicitly from typeHint at testdata/flysystem/src/Adapter/Ftp.php:22 + protected $transferMode = FTP_BINARY; + ^^^^^^^^^^^^^ +WARNING implicitParamType: Type for $recurseManually can be wrote explicitly from typeHint at testdata/flysystem/src/Adapter/Ftp.php:32 + protected $recurseManually = false; + ^^^^^^^^^^^^^^^^ +WARNING implicitParamType: Type for $utf8 can be wrote explicitly from typeHint at testdata/flysystem/src/Adapter/Ftp.php:37 + protected $utf8 = false; + ^^^^^ +WARNING implicitParamType: Type for $isPureFtpd can be wrote explicitly from typeHint at testdata/flysystem/src/Adapter/Ftp.php:64 + protected $isPureFtpd; + ^^^^^^^^^^^ WARNING errorSilence: Don't use @, silencing errors is bad practice at testdata/flysystem/src/Adapter/Ftpd.php:15 if (@ftp_chdir($this->getConnection(), $path) === true) { ^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^ @@ -49,6 +97,15 @@ WARNING errorSilence: Don't use @, silencing errors is bad practice at testdata/ MAYBE invalidDocblockType: Void type can only be used as a standalone type for the return type at testdata/flysystem/src/Adapter/Local.php:444 * @return array|void ^^^^^^^^^^ +WARNING implicitParamType: Type for $pathSeparator can be wrote explicitly from typeHint at testdata/flysystem/src/Adapter/Local.php:47 + protected $pathSeparator = DIRECTORY_SEPARATOR; + ^^^^^^^^^^^^^^ +WARNING implicitParamType: Type for $writeFlags can be wrote explicitly from typeHint at testdata/flysystem/src/Adapter/Local.php:57 + protected $writeFlags; + ^^^^^^^^^^^ +WARNING implicitParamType: Type for $linkHandling can be wrote explicitly from typeHint at testdata/flysystem/src/Adapter/Local.php:62 + private $linkHandling; + ^^^^^^^^^^^^^ WARNING invalidDocblockRef: @see tag refers to unknown symbol League\Flysystem\ReadInterface::readStream at testdata/flysystem/src/Adapter/Polyfill/StreamedReadingTrait.php:17 * @see League\Flysystem\ReadInterface::readStream() ^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^ @@ -61,6 +118,18 @@ MAYBE missingPhpdoc: Missing PHPDoc for \League\Flysystem\Adapter\Polyfill\Str MAYBE missingPhpdoc: Missing PHPDoc for \League\Flysystem\Adapter\Polyfill\StreamedWritingTrait::update public method at testdata/flysystem/src/Adapter/Polyfill/StreamedWritingTrait.php:59 abstract public function update($pash, $contents, Config $config); ^^^^^^ +WARNING implicitParamType: Type for $path can be wrote explicitly from typeHint at testdata/flysystem/src/FileExistsException.php:12 + protected $path; + ^^^^^ +WARNING implicitParamType: Type for $path can be wrote explicitly from typeHint at testdata/flysystem/src/FileNotFoundException.php:12 + protected $path; + ^^^^^ +WARNING implicitParamType: Type for $previous can be wrote explicitly from typeHint at testdata/flysystem/src/FileNotFoundException.php:21 + public function __construct($path, $code = 0, BaseException $previous = null) + ^^^^^ +WARNING implicitParamType: Type for $previous can be wrote explicitly from typeHint at testdata/flysystem/src/FileNotFoundException.php:21 + public function __construct($path, $code = 0, BaseException $previous = null) + ^^^^^ MAYBE typeHint: Specify the type for the parameter $config in PHPDoc, 'array' type hint too generic at testdata/flysystem/src/Filesystem.php:63 public function write($path, $contents, array $config = []) ^^^^^ @@ -85,6 +154,9 @@ MAYBE typeHint: Specify the type for the parameter $config in PHPDoc, 'array' WARNING unused: Variable $e is unused (use $_ to ignore this inspection or specify --unused-var-regex flag) at testdata/flysystem/src/Handler.php:129 } catch (BadMethodCallException $e) { ^^ +WARNING implicitParamType: Type for $path can be wrote explicitly from typeHint at testdata/flysystem/src/Handler.php:15 + protected $path; + ^^^^^ WARNING unused: Variable $e is unused (use $_ to ignore this inspection or specify --unused-var-regex flag) at testdata/flysystem/src/MountManager.php:275 } catch (PluginNotFoundException $e) { ^^ @@ -106,6 +178,9 @@ MAYBE missingPhpdoc: Missing PHPDoc for \League\Flysystem\SafeStorage::storeSa MAYBE missingPhpdoc: Missing PHPDoc for \League\Flysystem\SafeStorage::retrieveSafely public method at testdata/flysystem/src/SafeStorage.php:28 public function retrieveSafely($key) ^^^^^^^^^^^^^^ +WARNING implicitParamType: Type for $hash can be wrote explicitly from typeHint at testdata/flysystem/src/SafeStorage.php:10 + private $hash; + ^^^^^ MAYBE missingPhpdoc: Missing PHPDoc for \League\Flysystem\UnreadableFileException::forFileInfo public method at testdata/flysystem/src/UnreadableFileException.php:9 public static function forFileInfo(SplFileInfo $fileInfo) ^^^^^^^^^^^ @@ -121,9 +196,21 @@ MAYBE regexpSimplify: May re-write '#^[a-zA-Z]{1}:$#' as '#^[a-zA-Z]:$#' at te MAYBE typeHint: Specify the type for the parameter $entry in PHPDoc, 'array' type hint too generic at testdata/flysystem/src/Util/ContentListingFormatter.php:52 private function addPathInfo(array $entry) ^^^^^^^^^^^ +WARNING implicitParamType: Type for $directory can be wrote explicitly from typeHint at testdata/flysystem/src/Util/ContentListingFormatter.php:15 + private $directory; + ^^^^^^^^^^ +WARNING implicitParamType: Type for $recursive can be wrote explicitly from typeHint at testdata/flysystem/src/Util/ContentListingFormatter.php:20 + private $recursive; + ^^^^^^^^^^ +WARNING implicitParamType: Type for $caseSensitive can be wrote explicitly from typeHint at testdata/flysystem/src/Util/ContentListingFormatter.php:25 + private $caseSensitive; + ^^^^^^^^^^^^^^ WARNING unused: Variable $e is unused (use $_ to ignore this inspection or specify --unused-var-regex flag) at testdata/flysystem/src/Util/MimeType.php:208 } catch (ErrorException $e) { ^^ MAYBE ternarySimplify: Could rewrite as `static::$extensionToMimeTypeMap[$extension] ?? 'text/plain'` at testdata/flysystem/src/Util/MimeType.php:222 return isset(static::$extensionToMimeTypeMap[$extension]) ^^^^^^^^^^^ +WARNING implicitParamType: Type for $algo can be wrote explicitly from typeHint at testdata/flysystem/src/Util/StreamHasher.php:10 + private $algo; + ^^^^^ diff --git a/src/tests/golden/testdata/inflector/golden.txt b/src/tests/golden/testdata/inflector/golden.txt index 92cc75ead..5494b4974 100644 --- a/src/tests/golden/testdata/inflector/golden.txt +++ b/src/tests/golden/testdata/inflector/golden.txt @@ -22,3 +22,18 @@ WARNING errorSilence: Don't use @, silencing errors is bad practice at testdata/ WARNING errorSilence: Don't use @, silencing errors is bad practice at testdata/inflector/lib/Doctrine/Common/Inflector/Inflector.php:181 @trigger_error(sprintf('The "%s" method is deprecated and will be dropped in Doctrine Inflector 3.0. Please update to the new Inflector API.', __METHOD__), E_USER_DEPRECATED); ^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^ +WARNING implicitParamType: Type for $pattern can be wrote explicitly from typeHint at testdata/inflector/lib/Doctrine/Inflector/Rules/Pattern.php:12 + private $pattern; + ^^^^^^^^ +WARNING implicitParamType: Type for $regex can be wrote explicitly from typeHint at testdata/inflector/lib/Doctrine/Inflector/Rules/Pattern.php:15 + private $regex; + ^^^^^^ +WARNING implicitParamType: Type for $regex can be wrote explicitly from typeHint at testdata/inflector/lib/Doctrine/Inflector/Rules/Patterns.php:17 + private $regex; + ^^^^^^ +WARNING implicitParamType: Type for $replacement can be wrote explicitly from typeHint at testdata/inflector/lib/Doctrine/Inflector/Rules/Transformation.php:16 + private $replacement; + ^^^^^^^^^^^^ +WARNING implicitParamType: Type for $word can be wrote explicitly from typeHint at testdata/inflector/lib/Doctrine/Inflector/Rules/Word.php:10 + private $word; + ^^^^^ diff --git a/src/tests/golden/testdata/math/golden.txt b/src/tests/golden/testdata/math/golden.txt index ae3f8b9b4..525c77a47 100644 --- a/src/tests/golden/testdata/math/golden.txt +++ b/src/tests/golden/testdata/math/golden.txt @@ -1,9 +1,18 @@ WARNING unused: Variable $a is unused (use $_ to ignore this inspection or specify --unused-var-regex flag) at testdata/math/src/BigDecimal.php:292 [$a, $b] = $this->scaleValues($this, $that); ^^ +WARNING implicitParamType: Type for $value can be wrote explicitly from typeHint at testdata/math/src/BigDecimal.php:28 + private $value; + ^^^^^^ +WARNING implicitParamType: Type for $scale can be wrote explicitly from typeHint at testdata/math/src/BigDecimal.php:37 + private $scale; + ^^^^^^ MAYBE trailingComma: Last element in a multi-line array should have a trailing comma at testdata/math/src/BigInteger.php:435 new BigInteger($remainder) ^^^^^^^^^^^^^^^^^^^^^^^^^^ +WARNING implicitParamType: Type for $value can be wrote explicitly from typeHint at testdata/math/src/BigInteger.php:32 + private $value; + ^^^^^^ MAYBE ternarySimplify: Could rewrite as `$matches['fractional'] ?? ''` at testdata/math/src/BigNumber.php:90 $fractional = isset($matches['fractional']) ? $matches['fractional'] : ''; ^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^ @@ -19,6 +28,9 @@ MAYBE trailingComma: Last element in a multi-line array should have a trailing MAYBE trailingComma: Last element in a multi-line array should have a trailing comma at testdata/math/src/Internal/Calculator/NativeCalculator.php:187 (string) $r ^^^^^^^^^^^ +WARNING implicitParamType: Type for $maxDigits can be wrote explicitly from typeHint at testdata/math/src/Internal/Calculator/NativeCalculator.php:28 + private $maxDigits; + ^^^^^^^^^^ ERROR classMembersOrder: Constant UNNECESSARY must go before methods in the class RoundingMode at testdata/math/src/RoundingMode.php:33 public const UNNECESSARY = 0; ^^^^^^^^^^^^^^^^^^^^^^^^^^^^^ diff --git a/src/tests/golden/testdata/mustache/golden.txt b/src/tests/golden/testdata/mustache/golden.txt index dfdf00ada..ecd0d7116 100644 --- a/src/tests/golden/testdata/mustache/golden.txt +++ b/src/tests/golden/testdata/mustache/golden.txt @@ -172,3 +172,6 @@ WARNING errorSilence: Don't use @, silencing errors is bad practice at testdata/ WARNING unused: Variable $v is unused (use $_ to ignore this inspection or specify --unused-var-regex flag) at testdata/mustache/src/Mustache/Template.php:122 foreach ($value as $k => $v) { ^^ +WARNING implicitParamType: Type for $strictCallables can be wrote explicitly from typeHint at testdata/mustache/src/Mustache/Template.php:27 + protected $strictCallables = false; + ^^^^^^^^^^^^^^^^ diff --git a/src/tests/golden/testdata/phpdoc/golden.txt b/src/tests/golden/testdata/phpdoc/golden.txt index ee111c4ef..1f3953178 100644 --- a/src/tests/golden/testdata/phpdoc/golden.txt +++ b/src/tests/golden/testdata/phpdoc/golden.txt @@ -37,6 +37,9 @@ MAYBE invalidDocblockType: Repeated nullable doesn't make sense at testdata/ph WARNING invalidDocblock: Multiline PHPDoc comment should start with /**, not /* at testdata/phpdoc/phpdoc.php:31 /* ^^ +WARNING implicitParamType: Type for $a can be wrote explicitly from typeHint at testdata/phpdoc/phpdoc.php:34 +function g($a) { + ^^ WARNING invalidDocblockRef: @see tag refers to unknown symbol FooUnExisting at testdata/phpdoc/phpdoc.php:54 * @see FooUnExisting ^^^^^^^^^^^^^ @@ -85,6 +88,30 @@ MAYBE invalidDocblockType: Shape param #1: want key:type, found x at testdata/ WARNING invalidDocblock: Multiline PHPDoc comment should start with /**, not /* at testdata/phpdoc/phpdoc.php:70 /* ^^^^ +WARNING implicitParamType: Type for $a can be wrote explicitly from typeHint at testdata/phpdoc/phpdoc.php:57 +function f($a, $b, $c, $d, $e, $f, $g, $h, $a1, $b1, $c1, $d1, $e1, $f1, $g1, $h1) { + ^^ +WARNING implicitParamType: Type for $unexisting can be wrote explicitly from typeHint at testdata/phpdoc/phpdoc.php:57 +function f($a, $b, $c, $d, $e, $f, $g, $h, $a1, $b1, $c1, $d1, $e1, $f1, $g1, $h1) { + ^^ +WARNING implicitParamType: Type for can be wrote explicitly from typeHint at testdata/phpdoc/phpdoc.php:57 +function f($a, $b, $c, $d, $e, $f, $g, $h, $a1, $b1, $c1, $d1, $e1, $f1, $g1, $h1) { + ^^ +WARNING implicitParamType: Type for $b can be wrote explicitly from typeHint at testdata/phpdoc/phpdoc.php:57 +function f($a, $b, $c, $d, $e, $f, $g, $h, $a1, $b1, $c1, $d1, $e1, $f1, $g1, $h1) { + ^^ +WARNING implicitParamType: Type for $e can be wrote explicitly from typeHint at testdata/phpdoc/phpdoc.php:57 +function f($a, $b, $c, $d, $e, $f, $g, $h, $a1, $b1, $c1, $d1, $e1, $f1, $g1, $h1) { + ^^ +WARNING implicitParamType: Type for $f can be wrote explicitly from typeHint at testdata/phpdoc/phpdoc.php:57 +function f($a, $b, $c, $d, $e, $f, $g, $h, $a1, $b1, $c1, $d1, $e1, $f1, $g1, $h1) { + ^^ +WARNING implicitParamType: Type for $h can be wrote explicitly from typeHint at testdata/phpdoc/phpdoc.php:57 +function f($a, $b, $c, $d, $e, $f, $g, $h, $a1, $b1, $c1, $d1, $e1, $f1, $g1, $h1) { + ^^ +WARNING implicitParamType: Type for $a can be wrote explicitly from typeHint at testdata/phpdoc/phpdoc.php:21 + public $a = 100; + ^^ WARNING invalidDocblock: Multiline PHPDoc comment should start with /**, not /* at testdata/phpdoc/phpdoc.php:31 /* ^^ diff --git a/src/tests/golden/testdata/quickfix/phpTypeHintApply.php b/src/tests/golden/testdata/quickfix/phpTypeHintApply.php new file mode 100644 index 000000000..91904f51e --- /dev/null +++ b/src/tests/golden/testdata/quickfix/phpTypeHintApply.php @@ -0,0 +1,32 @@ + "" ^^^^^^^^^^^^^^^^^^^^^^^ diff --git a/src/types/map.go b/src/types/map.go index f942d81ba..77345a992 100644 --- a/src/types/map.go +++ b/src/types/map.go @@ -37,6 +37,14 @@ type Map struct { m map[string]struct{} } +func (m Map) GetTypes() []string { + var types []string + for typ := range m.m { + types = append(types, typ) + } + return types +} + // IsPrecise reports whether the type set represented by the map is precise // enough to perform typecheck-like analysis. // diff --git a/src/types/predicates.go b/src/types/predicates.go index d0b4034f2..d4e8de4c8 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 IsObject(s string) bool { + return strings.HasPrefix(s, `object`) +} + func IsClosure(s string) bool { return strings.HasPrefix(s, `\Closure`) }