diff --git a/src/linter/block_linter.go b/src/linter/block_linter.go index e69b0965..bd7bc368 100644 --- a/src/linter/block_linter.go +++ b/src/linter/block_linter.go @@ -11,7 +11,6 @@ import ( "github.com/VKCOM/noverify/src/ir/phpcore" "github.com/VKCOM/noverify/src/linter/autogen" "github.com/VKCOM/noverify/src/meta" - "github.com/VKCOM/noverify/src/phpdoc" "github.com/VKCOM/noverify/src/quickfix" "github.com/VKCOM/noverify/src/solver" "github.com/VKCOM/noverify/src/types" @@ -33,8 +32,6 @@ func (b *blockLinter) enterNode(n ir.Node) { case *ir.ClassStmt: b.checkClass(n) - case *ir.ClassMethodStmt: - println("") case *ir.FunctionCallExpr: b.checkFunctionCall(n) @@ -210,98 +207,15 @@ func (b *blockLinter) checkUnaryPlus(n *ir.UnaryPlusExpr) { b.report(n, LevelWarning, "strangeCast", "Unary plus with non-constant expression, possible type cast, use an explicit cast to int or float instead of using the unary plus") } -func (b *blockLinter) checkMethodTypeHint(method *ir.ClassMethodStmt) { - for _, comment := range method.Doc.Parsed { - var typeContainer, ok = comment.(*phpdoc.TypeVarCommentPart) - if !ok { - continue - } - - if typeContainer.Name() != "param" { - continue - } - - var typeParam = typeContainer.Type.Source - - for _, param := range method.Params { - var typedParam, ok = param.(*ir.Parameter) - if ok { - var variable = typedParam.Variable - - var paramType, ok = typedParam.VariableType.(*ir.Name) - if paramType != nil && ok { - // TODO: quickFix -> remove @param from typeHint - break - } - - if !types.IsTrivial(typeContainer.Type.Source) && !types.IsClass(typeContainer.Type.Source) { - continue - } - - // TODO: quickFix -> remove @param from typeHint - var varDollar = typeContainer.Var - var variableWithType = typeParam + " " + varDollar - b.walker.report(variable, LevelWarning, "implicitParamType", "Type for %s can be wrote explicitly from typeHint", varDollar) - b.walker.r.addQuickFix("implicitParamType", b.quickfix.FunctionParamTypeReplacementFromTypeHint(variable, variableWithType)) - } - } - } -} - -func (b *blockLinter) checkPropertyTypeHint(property *ir.PropertyListStmt) { - for _, comment := range property.Doc.Parsed { - var typeContainer, ok = comment.(*phpdoc.TypeVarCommentPart) - if !ok { - continue - } - - if typeContainer.Name() != "var" { - continue - } - - for _, NodeProperty := range property.Properties { - var typedProperty, ok = NodeProperty.(*ir.PropertyStmt) - if !ok { - continue - } - - var variable = typedProperty.Variable - var typeHintType = typeContainer.Type.Source - var propertyType, okCast = property.Type.(*ir.Name) - - if okCast { - if propertyType.Value == typeHintType { - // TODO: should delete doctype param - break - } - } - - if !types.IsTrivial(typeHintType) && !types.IsClass(typeHintType) { - continue - } - - // TODO: quickFix -> remove @param from typeHint - var varDollar = "$" + variable.Name - var variableWithType = typeHintType + " " + varDollar - b.walker.report(variable, LevelWarning, "implicitParamType", "Type for %s can be wrote explicitly from typeHint", varDollar) - b.walker.r.addQuickFix("implicitParamType", b.quickfix.FunctionParamTypeReplacementFromTypeHint(variable, variableWithType)) - } - } -} - func (b *blockLinter) checkClass(class *ir.ClassStmt) { const classMethod = 0 const classOtherMember = 1 var members = make([]int, 0, len(class.Stmts)) for _, stmt := range class.Stmts { - switch value := stmt.(type) { + switch stmt.(type) { case *ir.ClassMethodStmt: - b.checkMethodTypeHint(value) members = append(members, classMethod) - case *ir.PropertyListStmt: - b.checkPropertyTypeHint(value) - members = append(members, classOtherMember) default: members = append(members, classOtherMember) } diff --git a/src/linter/quickfix.go b/src/linter/quickfix.go index a3902c6d..bb45b0e4 100644 --- a/src/linter/quickfix.go +++ b/src/linter/quickfix.go @@ -48,7 +48,7 @@ func (g *QuickFixGenerator) NullForNotNullableProperty(prop *ir.PropertyStmt) qu } } -func (g *QuickFixGenerator) FunctionParamTypeReplacementFromTypeHint(prop *ir.SimpleVar, variableWithType string) quickfix.TextEdit { +func (g *QuickFixGenerator) FunctionParamTypeReplacementFromTypeHint(prop *ir.Parameter, variableWithType string) quickfix.TextEdit { return quickfix.TextEdit{ StartPos: prop.Position.StartPos, EndPos: prop.Position.EndPos, diff --git a/src/linter/root_checker.go b/src/linter/root_checker.go index bc085588..c2b4c671 100644 --- a/src/linter/root_checker.go +++ b/src/linter/root_checker.go @@ -55,54 +55,6 @@ func newRootChecker(walker *rootWalker, quickfix *QuickFixGenerator) *rootChecke return c } -func (r *rootChecker) CheckFunctionTypeHint(fun *ir.FunctionStmt) { - for _, comment := range fun.Doc.Parsed { - var typeContainer, ok = comment.(*phpdoc.TypeVarCommentPart) - if !ok { - continue - } - - if typeContainer.Name() != "param" { - continue - } - - var typeParam = typeContainer.Type.Source - - for _, param := range fun.Params { - var typedParam, ok = param.(*ir.Parameter) - if ok { - var variable = typedParam.Variable - - var paramType, ok = typedParam.VariableType.(*ir.Name) - if paramType != nil && ok { - // TODO: quickFix -> remove @param from typeHint - break - } - - converted := phpdoctypes.ToRealType(r.normalizer.ClassFQNProvider(), r.normalizer.KPHP(), typeContainer.Type) - if cap(converted.Types) > 1 { - continue - } - - if converted.Types == nil { - continue - } - - if !types.IsTrivial(converted.Types[0].Elem) && !types.IsClass(converted.Types[0].Elem) { - continue - } - - // TODO: quickFix -> remove @param from typeHint - var varDollar = typeContainer.Var - var variableWithType = typeParam + " " + varDollar - r.walker.Report(variable, LevelWarning, "implicitParamType", "Type for %s can be wrote explicitly from typeHint", varDollar) - r.walker.addQuickFix("implicitParamType", r.quickfix.FunctionParamTypeReplacementFromTypeHint(variable, variableWithType)) - break - } - } - } -} - func (r *rootChecker) CheckFunction(fun *ir.FunctionStmt) bool { r.CheckKeywordCase(fun, "function") @@ -135,7 +87,6 @@ func (r *rootChecker) CheckFunction(fun *ir.FunctionStmt) bool { r.walker.handleFuncStmts(funcParams.params, nil, fun.Stmts, sc) - r.CheckFunctionTypeHint(fun) return false } @@ -632,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/closure_test.go b/src/tests/checkers/closure_test.go index b4ba0ec6..647d597d 100644 --- a/src/tests/checkers/closure_test.go +++ b/src/tests/checkers/closure_test.go @@ -66,7 +66,6 @@ function f(callable $s, callable $s1, callable $s2) {} ) test.Expect = []string{ "Lost return type for callable(...), if the function returns nothing, specify void explicitly", - "Type for $s 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 7e6d044b..b8258d2e 100644 --- a/src/tests/checkers/typehint_test.go +++ b/src/tests/checkers/typehint_test.go @@ -96,12 +96,6 @@ function f4() { }; } `) - test.Expect = []string{ - `Type for $a can be wrote explicitly from typeHint`, - `Type for $a can be wrote explicitly from typeHint`, - `Type for $a can be wrote explicitly from typeHint`, - `Type for $a can be wrote explicitly from typeHint`, - } test.RunAndMatch() } @@ -179,7 +173,7 @@ private $foo2 ; * @param $str string * @param $str2 string */ -function test($str, string $str){ +function test($str, string $str2){ } } @@ -192,11 +186,8 @@ function test2($str){ test.Expect = []string{ `Specify the access modifier for \SimpleClass::test method explicitly`, `Non-canonical order of variable and type `, - `@param for non-existing argument $str2`, `Non-canonical order of variable and type`, - `Type for $foo2 can be wrote explicitly from typeHint`, `Type for $str can be wrote explicitly from typeHint`, - `Type for $str2 can be wrote explicitly from typeHint`, `Non-canonical order of variable and type`, `Type for $str can be wrote explicitly from typeHint`, } diff --git a/src/types/map.go b/src/types/map.go index f942d81b..77345a99 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 d0b4034f..d4e8de4c 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`) }