Skip to content

Commit 06d2956

Browse files
authored
linter: function param type mismatch with phpdoc (#1253)
1 parent a8c6bbf commit 06d2956

8 files changed

Lines changed: 551 additions & 17 deletions

File tree

src/linter/block.go

Lines changed: 180 additions & 10 deletions
Original file line numberDiff line numberDiff line change
@@ -551,17 +551,167 @@ func (b *blockWalker) handleAndCheckGlobalStmt(s *ir.GlobalStmt) {
551551
}
552552
}
553553

554-
func (b *blockWalker) CheckParamNullability(params []ir.Node) {
554+
func (b *blockWalker) checkPhpDocTypesWithTypeHints(param *ir.Parameter, phpDocParamTypes map[string]string) {
555+
if len(phpDocParamTypes) == 0 {
556+
return
557+
}
558+
559+
// Build the lookup key, with fallback if "&$" did not find
560+
name := param.Variable.Name
561+
key := "$" + name
562+
if param.ByRef {
563+
if _, ok := phpDocParamTypes["&$"+name]; ok {
564+
key = "&$" + name
565+
}
566+
}
567+
rawDoc := strings.TrimSpace(phpDocParamTypes[key])
568+
if rawDoc == "" && param.ByRef {
569+
rawDoc = strings.TrimSpace(phpDocParamTypes["$"+name])
570+
}
571+
572+
if rawDoc == "" {
573+
return
574+
}
575+
576+
b.checkDiffPhpDocWithTypeHints(param.VariableType, name, rawDoc, b.linter.classParseState().Uses)
577+
}
578+
579+
func (b *blockWalker) checkDiffPhpDocWithTypeHints(
580+
node ir.Node,
581+
paramName, rawPhpDocType string,
582+
uses map[string]string,
583+
) {
584+
// 1) Normalization
585+
doc := strings.TrimSpace(rawPhpDocType)
586+
doc = strings.TrimPrefix(doc, `\`)
587+
doc = strings.TrimSpace(doc)
588+
589+
// 2) Unpack Nullable from AST
590+
var isNullable bool
591+
if n, ok := node.(*ir.Nullable); ok {
592+
isNullable = true
593+
// remove ? and null for type checking
594+
doc = strings.TrimPrefix(doc, "?")
595+
parts := strings.Split(doc, "|")
596+
clean := make([]string, 0, len(parts))
597+
for _, p := range parts {
598+
p = strings.TrimSpace(p)
599+
if p != "" && p != "null" {
600+
clean = append(clean, p)
601+
}
602+
}
603+
doc = strings.Join(clean, "|")
604+
node = n.Expr
605+
}
606+
607+
// 3) AST type
608+
var typeValue string
609+
switch t := node.(type) {
610+
case *ir.Name:
611+
typeValue = t.Value
612+
case *ir.Identifier:
613+
typeValue = t.Value
614+
default:
615+
return
616+
}
617+
618+
// 4) Getting alias
619+
alias := uses[typeValue]
620+
typeValue = strings.TrimPrefix(typeValue, `\`)
621+
622+
if alias == "" {
623+
alias = uses[typeValue]
624+
}
625+
626+
alias = strings.TrimPrefix(alias, `\`)
627+
628+
// 5) Helper
629+
report := func() {
630+
b.linter.report(
631+
node, LevelWarning, "funcParamTypeMissMatch",
632+
"param $%s miss matched with phpdoc type <<%s>>",
633+
paramName, rawPhpDocType,
634+
)
635+
}
636+
637+
// 6) if hint nullable, but PHPDoc has no '?' or 'null' - report
638+
if isNullable && !strings.Contains(rawPhpDocType, "?") && !strings.Contains(rawPhpDocType, "null") {
639+
report()
640+
return
641+
}
642+
643+
// 7) callable
644+
if typeValue == "callable" {
645+
if !strings.HasPrefix(doc, "callable") {
646+
report()
647+
}
648+
return
649+
}
650+
651+
// 8) Union
652+
if strings.Contains(doc, "|") {
653+
parts := strings.Split(doc, "|")
654+
for i := range parts {
655+
parts[i] = strings.TrimSpace(parts[i])
656+
}
657+
// if union include null, but hint not nullable - report
658+
if !isNullable {
659+
for _, p := range parts {
660+
if p == "null" {
661+
report()
662+
return
663+
}
664+
}
665+
}
666+
// Any contains in union
667+
for _, p := range parts {
668+
// array[]
669+
if typeValue == "array" && (p == "array" || strings.HasSuffix(p, "[]")) {
670+
return
671+
}
672+
// boolean-boolean
673+
if types.IsBoolean(p) && types.IsBoolean(typeValue) {
674+
return
675+
}
676+
// 1-1 or alias
677+
if p == typeValue || p == alias {
678+
return
679+
}
680+
}
681+
report()
682+
return
683+
}
684+
685+
// 9) Single/general: array / boolean / 1-1 / alias
686+
switch {
687+
case typeValue == "array":
688+
if doc != "array" && !strings.HasSuffix(doc, "[]") {
689+
report()
690+
}
691+
case types.IsBoolean(doc) && types.IsBoolean(typeValue):
692+
// ok
693+
case doc == typeValue:
694+
// ok
695+
case alias != "" && doc == alias:
696+
// ok
697+
default:
698+
report()
699+
}
700+
}
701+
702+
func (b *blockWalker) CheckParamNullability(params []ir.Node, phpDocParamTypes map[string]string) {
555703
for _, param := range params {
556704
if p, ok := param.(*ir.Parameter); ok {
557705
var paramType ir.Node
558-
paramType, paramOk := p.VariableType.(*ir.Name)
559-
if !paramOk {
560-
paramIdentifier, paramIdentifierOk := p.VariableType.(*ir.Identifier)
561-
if !paramIdentifierOk {
562-
continue
563-
}
564-
paramType = paramIdentifier
706+
707+
b.checkPhpDocTypesWithTypeHints(p, phpDocParamTypes)
708+
switch typ := p.VariableType.(type) {
709+
case *ir.Name, *ir.Identifier:
710+
paramType = typ
711+
case *ir.Nullable:
712+
continue
713+
default:
714+
continue
565715
}
566716

567717
paramName, ok := paramType.(*ir.Name)
@@ -586,9 +736,28 @@ func (b *blockWalker) CheckParamNullability(params []ir.Node) {
586736
}
587737
}
588738

739+
func (b *blockWalker) getParamsTypesFromPhpDoc(doc phpdoc.Comment) map[string]string {
740+
if len(doc.Parsed) == 0 {
741+
return nil
742+
}
743+
phpDocParamTypes := make(map[string]string)
744+
745+
for _, part := range doc.Parsed {
746+
if part.Name() == "param" {
747+
param, ok := part.(*phpdoc.TypeVarCommentPart)
748+
if ok {
749+
phpDocParamTypes[param.Var] = param.Type.Expr.Value
750+
}
751+
}
752+
}
753+
return phpDocParamTypes
754+
}
755+
589756
func (b *blockWalker) handleFunction(fun *ir.FunctionStmt) bool {
590757
if b.ignoreFunctionBodies {
591-
b.CheckParamNullability(fun.Params)
758+
phpDocParamTypes := b.getParamsTypesFromPhpDoc(fun.Doc)
759+
760+
b.CheckParamNullability(fun.Params, phpDocParamTypes)
592761
return false
593762
}
594763

@@ -1628,7 +1797,8 @@ func (b *blockWalker) handleCallArgs(args []ir.Node, fn meta.FuncInfo) {
16281797
ArgTypes: funcArgTypes,
16291798
}
16301799

1631-
b.CheckParamNullability(a.Params)
1800+
phpDocParamTypes := b.getParamsTypesFromPhpDoc(a.Doc)
1801+
b.CheckParamNullability(a.Params, phpDocParamTypes)
16321802
b.enterClosure(a, isInstance, typ, closureSolver)
16331803
default:
16341804
a.Walk(b)

src/linter/block_linter.go

Lines changed: 10 additions & 5 deletions
Original file line numberDiff line numberDiff line change
@@ -44,10 +44,12 @@ func (b *blockLinter) enterNode(n ir.Node) {
4444
b.checkFunctionCall(n)
4545

4646
case *ir.ArrowFunctionExpr:
47-
b.walker.CheckParamNullability(n.Params)
47+
phpDocParamTypes := b.walker.getParamsTypesFromPhpDoc(n.Doc)
48+
b.walker.CheckParamNullability(n.Params, phpDocParamTypes)
4849

4950
case *ir.ClosureExpr:
50-
b.walker.CheckParamNullability(n.Params)
51+
phpDocParamTypes := b.walker.getParamsTypesFromPhpDoc(n.Doc)
52+
b.walker.CheckParamNullability(n.Params, phpDocParamTypes)
5153

5254
case *ir.MethodCallExpr:
5355
b.checkMethodCall(n)
@@ -238,7 +240,8 @@ func (b *blockLinter) checkTrait(n *ir.TraitStmt) {
238240
for _, stmt := range n.Stmts {
239241
method, ok := stmt.(*ir.ClassMethodStmt)
240242
if ok {
241-
b.walker.CheckParamNullability(method.Params)
243+
phpDocParamTypes := b.walker.getParamsTypesFromPhpDoc(method.Doc)
244+
b.walker.CheckParamNullability(method.Params, phpDocParamTypes)
242245
}
243246
}
244247
}
@@ -252,7 +255,8 @@ func (b *blockLinter) checkClass(class *ir.ClassStmt) {
252255
switch value := stmt.(type) {
253256
case *ir.ClassMethodStmt:
254257
members = append(members, classMethod)
255-
b.walker.CheckParamNullability(value.Params)
258+
phpDocParamTypes := b.walker.getParamsTypesFromPhpDoc(value.Doc)
259+
b.walker.CheckParamNullability(value.Params, phpDocParamTypes)
256260
default:
257261
members = append(members, classOtherMember)
258262
}
@@ -1629,7 +1633,8 @@ func (b *blockLinter) checkInterfaceStmt(iface *ir.InterfaceStmt) {
16291633
b.report(x, LevelWarning, "nonPublicInterfaceMember", "'%s' can't be %s", methodName, modifier.Value)
16301634
}
16311635
}
1632-
b.walker.CheckParamNullability(x.Params)
1636+
phpDocParamTypes := b.walker.getParamsTypesFromPhpDoc(x.Doc)
1637+
b.walker.CheckParamNullability(x.Params, phpDocParamTypes)
16331638
case *ir.ClassConstListStmt:
16341639
for _, modifier := range x.Modifiers {
16351640
if strings.EqualFold(modifier.Value, "private") || strings.EqualFold(modifier.Value, "protected") {

src/linter/report.go

Lines changed: 25 additions & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -1337,6 +1337,31 @@ class Main {
13371337
Before: `if (gettype($a) == "string") { ... }`,
13381338
After: `if (is_string($a)) { ... }`,
13391339
},
1340+
1341+
{
1342+
Name: "funcParamTypeMissMatch",
1343+
Default: true,
1344+
Quickfix: false,
1345+
Comment: `Report function typehint and phpdoc mismatch.`,
1346+
Before: `
1347+
/**
1348+
*
1349+
* @param ?string $name
1350+
* @return string
1351+
*/
1352+
function wrongParam(string $name): string {
1353+
return "Hello, $name";
1354+
}`,
1355+
After: `
1356+
/**
1357+
*
1358+
* @param ?string $name
1359+
* @return string
1360+
*/
1361+
function wrongParam(?string $name): string {
1362+
return "Hello, $name";
1363+
}`,
1364+
},
13401365
}
13411366

13421367
for _, info := range allChecks {

0 commit comments

Comments
 (0)