Remove a param docblock made redundant by the added type

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

internal/apply/apply_test.go

@@ -78,6 +78,21 @@ func TestApply(t *testing.T) {
 			target: "<?php\nfinal class A {\n    public function set(string $v) {}\n    public function go() { $this->set(1); }\n}",
 			want:   "<?php\nfinal class A {\n    public function set(string $v) {}\n    public function go() { $this->set(1); }\n}",
 		},
+		{
+			name:   "removes a redundant param doc and its empty comment",
+			target: "<?php\nfinal class A {\n    /**\n     * @param string $v\n     */\n    public function set($v) {}\n    public function go() { $this->set(\"x\"); }\n}",
+			want:   "<?php\nfinal class A {\n    public function set(string $v) {}\n    public function go() { $this->set(\"x\"); }\n}",
+		},
+		{
+			name:   "keeps other doc tags when removing a redundant param",
+			target: "<?php\nfinal class A {\n    /**\n     * @param int $id\n     *\n     * @throws \\Exception\n     */\n    public function set($id) {}\n    public function go() { $this->set(1); }\n}",
+			want:   "<?php\nfinal class A {\n    /**\n     * @throws \\Exception\n     */\n    public function set(int $id) {}\n    public function go() { $this->set(1); }\n}",
+		},
+		{
+			name:   "keeps a param doc that has a description",
+			target: "<?php\nfinal class A {\n    /**\n     * @param string $v the value\n     */\n    public function set($v) {}\n    public function go() { $this->set(\"x\"); }\n}",
+			want:   "<?php\nfinal class A {\n    /**\n     * @param string $v the value\n     */\n    public function set(string $v) {}\n    public function go() { $this->set(\"x\"); }\n}",
+		},
 		{
 			name:   "unions multiple observed types",
 			target: "<?php\nfinal class A {\n    public function set($v) {}\n    public function go() { $this->set(1); $this->set(\"x\"); }\n}",

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:   "<?php\nclass A {\n    /**\n     * @param string $v\n     */\n    public function m($v) {}\n}",
+			added: map[string]string{"v": "string"},
+			want:  "<?php\nclass A {\n    public function m($v) {}\n}",
+		},
+		{
+			name:  "matches object type regardless of leading backslash",
+			src:   "<?php\nclass A {\n    /**\n     * @param DateTime $d\n     */\n    public function m($d) {}\n}",
+			added: map[string]string{"d": "\\DateTime"},
+			want:  "<?php\nclass A {\n    public function m($d) {}\n}",
+		},
+		{
+			name:  "keeps a tag whose type differs from the added type",
+			src:   "<?php\nclass A {\n    /**\n     * @param mixed $v\n     */\n    public function m($v) {}\n}",
+			added: map[string]string{"v": "\\DateTime"},
+			want:  "<?php\nclass A {\n    /**\n     * @param mixed $v\n     */\n    public function m($v) {}\n}",
+		},
+	}
+
+	for _, test := range tests {
+		t.Run(test.name, func(t *testing.T) {
+			root, err := phpast.Parse([]byte(test.src))
+			if err != nil {
+				t.Fatal(err)
+			}
+			var walk func(ast.Vertex)
+			walk = func(node ast.Vertex) {
+				if method, ok := node.(*ast.StmtClassMethod); ok {
+					phpast.StripRedundantDocParams(method, test.added)
+				}
+				for _, child := range phpast.Children(node) {
+					walk(child)
+				}
+			}
+			walk(root)
+
+			if got := phpast.Print(root); got != test.want {
+				t.Errorf("\n got: %q\nwant: %q", got, test.want)
+			}
+		})
+	}
+}

internal/phpast/phpast.go

@@ -5,17 +5,23 @@ package phpast
 import (
 	"bytes"
 	"reflect"
+	"regexp"
 	"strings"
 
 	"github.com/rectorphp/php-parser-in-go/pkg/ast"
 	"github.com/rectorphp/php-parser-in-go/pkg/conf"
 	"github.com/rectorphp/php-parser-in-go/pkg/parser"
+	"github.com/rectorphp/php-parser-in-go/pkg/token"
 	"github.com/rectorphp/php-parser-in-go/pkg/version"
 	"github.com/rectorphp/php-parser-in-go/pkg/visitor/nsresolver"
 	"github.com/rectorphp/php-parser-in-go/pkg/visitor/printer"
 	"github.com/rectorphp/php-parser-in-go/pkg/visitor/traverser"
 )
 
+// docParamLine matches a plain `@param <type> $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"