diff --git a/internal/apply/apply.go b/internal/apply/apply.go index d4191459..d7b12980 100644 --- a/internal/apply/apply.go +++ b/internal/apply/apply.go @@ -63,19 +63,21 @@ func (a *applier) walk(node ast.Vertex, class *ast.StmtClass) { func (a *applier) applyFunction(function *ast.StmtFunction) { name := phpast.ShortName(function.Name) + added := map[string]string{} for position, paramNode := range function.Params { param, ok := paramNode.(*ast.Parameter) if !ok || !typeable(param) { continue } if resolved, ok := a.types.Function(name, position); ok { - a.setType(param, resolved) + added[phpast.VariableName(param.Var)] = a.setType(param, resolved) continue } if resolved, ok := a.defaultType(param, "", ""); ok { - a.setType(param, resolved) + added[phpast.VariableName(param.Var)] = a.setType(param, resolved) } } + phpast.StripRedundantDocParams(function, added) } func (a *applier) applyMethod(method *ast.StmtClassMethod, class *ast.StmtClass) { @@ -94,19 +96,21 @@ func (a *applier) applyMethod(method *ast.StmtClassMethod, class *ast.StmtClass) classFQCN = a.names[class] } + added := map[string]string{} for position, paramNode := range method.Params { param, ok := paramNode.(*ast.Parameter) if !ok || !typeable(param) { continue } if resolved, ok := a.types.Method(className, name, position); ok { - a.setType(param, resolved) + added[phpast.VariableName(param.Var)] = a.setType(param, resolved) continue } if resolved, ok := a.defaultType(param, className, classFQCN); ok { - a.setType(param, resolved) + added[phpast.VariableName(param.Var)] = a.setType(param, resolved) } } + phpast.StripRedundantDocParams(method, added) } // defaultType infers a parameter type from its literal default value, so @@ -145,7 +149,9 @@ func (a *applier) objectFQCN(expr ast.Vertex, enclosingFQCN string) string { return a.names[classNode] } -func (a *applier) setType(param *ast.Parameter, resolved aggregate.Resolved) { +// setType writes the resolved type onto the parameter and returns the type text +// written, so the caller can drop a now-redundant @param doc line. +func (a *applier) setType(param *ast.Parameter, resolved aggregate.Resolved) string { nullable := resolved.Nullable || hasNullDefault(param) members := make([]string, len(resolved.Types)) @@ -167,7 +173,7 @@ func (a *applier) setType(param *ast.Parameter, resolved aggregate.Resolved) { Expr: &ast.Identifier{IdentifierTkn: &token.Token{Value: []byte(members[0])}}, } a.added++ - return + return "?" + members[0] } text := strings.Join(members, "|") @@ -176,6 +182,7 @@ func (a *applier) setType(param *ast.Parameter, resolved aggregate.Resolved) { } param.Type = &ast.Identifier{IdentifierTkn: &token.Token{Value: []byte(text), FreeFloating: leading}} a.added++ + return text } // takeVarLeading returns the leading whitespace tokens of the parameter's diff --git a/internal/apply/apply_test.go b/internal/apply/apply_test.go index 74ba9a7a..f4436dfd 100644 --- a/internal/apply/apply_test.go +++ b/internal/apply/apply_test.go @@ -78,6 +78,21 @@ func TestApply(t *testing.T) { target: "set(1); }\n}", want: "set(1); }\n}", }, + { + name: "removes a redundant param doc and its empty comment", + target: "set(\"x\"); }\n}", + want: "set(\"x\"); }\n}", + }, + { + name: "keeps other doc tags when removing a redundant param", + target: "set(1); }\n}", + want: "set(1); }\n}", + }, + { + name: "keeps a param doc that has a description", + target: "set(\"x\"); }\n}", + want: "set(\"x\"); }\n}", + }, { name: "unions multiple observed types", target: "set(1); $this->set(\"x\"); }\n}", diff --git a/internal/phpast/docparam_test.go b/internal/phpast/docparam_test.go new file mode 100644 index 00000000..533748db --- /dev/null +++ b/internal/phpast/docparam_test.go @@ -0,0 +1,59 @@ +package phpast_test + +import ( + "testing" + + "github.com/rectorphp/argtyper/internal/phpast" + "github.com/rectorphp/php-parser-in-go/pkg/ast" +) + +func TestStripRedundantDocParams(t *testing.T) { + tests := []struct { + name string + src string + added map[string]string + want string + }{ + { + name: "removes matching scalar tag and empties the doc", + src: " $name` line with no trailing +// description, so only a fully redundant tag is removed. +var docParamLine = regexp.MustCompile(`^\s*\*?\s*@param\s+(\S+)\s+\$(\w+)\s*$`) + var phpVersion, _ = version.New("8.3") var vertexType = reflect.TypeOf((*ast.Vertex)(nil)).Elem() @@ -115,6 +121,128 @@ func ObjectClassNode(expr ast.Vertex) ast.Vertex { return nil } +// StripRedundantDocParams removes `@param` lines from a function or method's doc +// comment when the type they declare equals the type just added for that +// parameter (added maps parameter name to the written type text). When nothing +// meaningful remains, the whole doc comment is removed. +func StripRedundantDocParams(node ast.Vertex, added map[string]string) { + if len(added) == 0 { + return + } + leading := leadingToken(node) + if leading == nil { + return + } + + index := -1 + for i, free := range leading.FreeFloating { + if free.ID == token.T_DOC_COMMENT { + index = i + break + } + } + if index < 0 { + return + } + + stripped, changed, empty := stripDocParamLines(string(leading.FreeFloating[index].Value), added) + if !changed { + return + } + if !empty { + leading.FreeFloating[index].Value = []byte(stripped) + return + } + + // The doc comment is now empty, so drop it along with the blank line it + // leaves behind (the whitespace token in front of it). + drop := map[int]bool{index: true} + if index > 0 && leading.FreeFloating[index-1].ID == token.T_WHITESPACE { + drop[index-1] = true + } + kept := leading.FreeFloating[:0:0] + for i, free := range leading.FreeFloating { + if !drop[i] { + kept = append(kept, free) + } + } + leading.FreeFloating = kept +} + +// stripDocParamLines removes redundant @param lines, reporting whether anything +// changed and whether the doc comment has no content left. +func stripDocParamLines(doc string, added map[string]string) (result string, changed, empty bool) { + lines := strings.Split(doc, "\n") + kept := make([]string, 0, len(lines)) + for i := 0; i < len(lines); i++ { + line := lines[i] + if match := docParamLine.FindStringSubmatch(line); match != nil { + if native, ok := added[match[2]]; ok && normalizeDocType(match[1]) == normalizeDocType(native) { + changed = true + // also drop a blank comment line that followed the tag + if i+1 < len(lines) && isBlankCommentLine(lines[i+1]) { + i++ + } + continue + } + } + kept = append(kept, line) + } + if !changed { + return doc, false, false + } + return strings.Join(kept, "\n"), true, docIsEmpty(kept) +} + +// isBlankCommentLine reports whether a doc line carries only the `*` marker. +func isBlankCommentLine(line string) bool { + text := strings.TrimSpace(line) + return text == "" || text == "*" +} + +// docIsEmpty reports whether the remaining lines carry no content beyond the +// comment markers. +func docIsEmpty(lines []string) bool { + for _, line := range lines { + text := strings.TrimSpace(line) + text = strings.TrimPrefix(text, "/**") + text = strings.TrimSuffix(text, "*/") + text = strings.TrimPrefix(text, "*") + if strings.TrimSpace(text) != "" { + return false + } + } + return true +} + +func normalizeDocType(name string) string { + return strings.TrimPrefix(name, "\\") +} + +// leadingToken returns the first token of a function or method, which carries +// the doc comment in its leading free-floating tokens. +func leadingToken(node ast.Vertex) *token.Token { + switch typed := node.(type) { + case *ast.StmtClassMethod: + if len(typed.Modifiers) > 0 { + if first := identifierToken(typed.Modifiers[0]); first != nil { + return first + } + } + return typed.FunctionTkn + case *ast.StmtFunction: + return typed.FunctionTkn + } + return nil +} + +func identifierToken(node ast.Vertex) *token.Token { + if identifier, ok := node.(*ast.Identifier); ok { + return identifier.IdentifierTkn + } + return nil +} + // IsThisVariable reports whether a node is the `$this` variable. func IsThisVariable(node ast.Vertex) bool { return VariableName(node) == "this"