Guard suggested fixes against comment loss in five linters

pkg/linters/appendoneelement/appendoneelement.go

@@ -74,19 +74,22 @@ func analyzeAppendOneElement(pass *analysis.Pass, n ast.Node, generatedFiles fil
 		return
 	}
 
-	pass.Report(analysis.Diagnostic{
+	diag := analysis.Diagnostic{
 		Pos:     call.Pos(),
 		End:     call.End(),
 		Message: fmt.Sprintf("append(s, %s...) can be simplified to append(s, %s)", litText, elemText),
-		SuggestedFixes: []analysis.SuggestedFix{{
+	}
+	if !astutil.HasOverlappingComment(pass.Files, call.Pos(), call.End()) {
+		diag.SuggestedFixes = []analysis.SuggestedFix{{
 			Message: fmt.Sprintf("Replace %s... with %s", litText, elemText),
 			TextEdits: []analysis.TextEdit{{
 				Pos:     call.Pos(),
 				End:     call.End(),
 				NewText: fmt.Appendf(nil, "append(%s, %s)", sliceText, elemText),
 			}},
-		}},
-	})
+		}}
+	}
+	pass.Report(diag)
 }
 
 // matchSingleElementSpread validates that call.Args[1] is a single-element slice

pkg/linters/appendoneelement/testdata/src/appendoneelement/appendoneelement.go

@@ -19,6 +19,13 @@ func badVar() {
 	_ = s
 }
 
+func badWithComments() {
+	s := []int{1}
+	x := 99
+	s = append(s, []int{x /* important */}...) // want `append\(s, \[\]int\{x\}\.\.\.\) can be simplified to append\(s, x\)`
+	_ = s
+}
+
 func good() {
 	s := []int{1, 2}
 	// Multiple elements — keep the spread form.

pkg/linters/appendoneelement/testdata/src/appendoneelement/appendoneelement.go.golden

@@ -19,6 +19,13 @@ func badVar() {
 	_ = s
 }
 
+func badWithComments() {
+	s := []int{1}
+	x := 99
+	s = append(s, []int{x /* important */}...) // want `append\(s, \[\]int\{x\}\.\.\.\) can be simplified to append\(s, x\)`
+	_ = s
+}
+
 func good() {
 	s := []int{1, 2}
 	// Multiple elements — keep the spread form.

pkg/linters/internal/astutil/astutil.go

@@ -657,7 +657,11 @@ func SwapImportEdits(fset *token.FileSet, file *ast.File, addPkg, removePkg stri
 // BuildContainsFix builds the suggested fix rewriting a comparison to
 // strings.Contains. fixMessage is used as the SuggestedFix.Message field so
 // callers can identify the rewritten function (e.g. "Index" vs "Count").
-func BuildContainsFix(expr *ast.BinaryExpr, pkgText, sText, subText string, negated bool, fixMessage string) []analysis.SuggestedFix {
+func BuildContainsFix(files []*ast.File, expr *ast.BinaryExpr, pkgText, sText, subText string, negated bool, fixMessage string) []analysis.SuggestedFix {
+	if HasOverlappingComment(files, expr.Pos(), expr.End()) {
+		return nil
+	}
+
 	var replacement string
 	if negated {
 		replacement = "!" + pkgText + ".Contains(" + sText + ", " + subText + ")"

pkg/linters/internal/astutil/astutil_test.go

@@ -484,7 +484,7 @@ func TestBuildContainsFix(t *testing.T) {
 		Y:  ast.NewIdent("b"),
 	}
 
-	fixes := BuildContainsFix(expr, "strings", "s", "sub", false, "test message")
+	fixes := BuildContainsFix(nil, expr, "strings", "s", "sub", false, "test message")
 	if len(fixes) != 1 {
 		t.Fatalf("got %d fixes, want 1", len(fixes))
 	}
@@ -496,13 +496,42 @@ func TestBuildContainsFix(t *testing.T) {
 	}
 
 	// negated
-	fixes = BuildContainsFix(expr, "strings", "s", "sub", true, "negated message")
+	fixes = BuildContainsFix(nil, expr, "strings", "s", "sub", true, "negated message")
 	if got := string(fixes[0].TextEdits[0].NewText); got != "!strings.Contains(s, sub)" {
 		t.Fatalf("negated NewText = %q, want %q", got, "!strings.Contains(s, sub)")
 	}
 	if fixes[0].Message != "negated message" {
 		t.Fatalf("negated Message = %q, want %q", fixes[0].Message, "negated message")
 	}
+
+	// overlapping comment suppresses fix
+	src := `package p
+func f() bool {
+	return strings.Count("a", "b" /* comment */) > 0
+}`
+	fset := token.NewFileSet()
+	file, err := parser.ParseFile(fset, "p.go", src, parser.ParseComments)
+	if err != nil {
+		t.Fatalf("ParseFile failed: %v", err)
+	}
+	var binExpr *ast.BinaryExpr
+	ast.Inspect(file, func(n ast.Node) bool {
+		if be, ok := n.(*ast.BinaryExpr); ok {
+			binExpr = be
+			return false
+		}
+		return true
+	})
+	if binExpr == nil {
+		t.Fatal("expected BinaryExpr")
+	}
+	if !HasOverlappingComment([]*ast.File{file}, binExpr.Pos(), binExpr.End()) {
+		t.Fatal("expected HasOverlappingComment to be true for test expression")
+	}
+	fixesWithComment := BuildContainsFix([]*ast.File{file}, binExpr, "strings", "s", "sub", false, "test message")
+	if len(fixesWithComment) != 0 {
+		t.Fatalf("got %d fixes with overlapping comment, want 0", len(fixesWithComment))
+	}
 }
 
 func TestByteStringTypeHelpers(t *testing.T) {

pkg/linters/stringscountcontains/stringscountcontains.go

@@ -69,7 +69,7 @@ func analyzeCountContains(pass *analysis.Pass, n ast.Node, generatedFiles filech
 		Pos:            expr.Pos(),
 		End:            expr.End(),
 		Message:        msg,
-		SuggestedFixes: astutil.BuildContainsFix(expr, pkgText, sText, subText, negated, "Replace strings.Count comparison with strings.Contains"),
+		SuggestedFixes: astutil.BuildContainsFix(pass.Files, expr, pkgText, sText, subText, negated, "Replace strings.Count comparison with strings.Contains"),
 	})
 }
 

pkg/linters/stringscountcontains/testdata/src/stringscountcontains/stringscountcontains.go

@@ -62,3 +62,7 @@ func badParenCountGTR(s, sub string) bool {
 func badParenYodaCountEQL(s, sub string) bool {
 	return 0 == (strings.Count(s, sub)) // want `use !strings\.Contains\(s, sub\) instead of strings\.Count comparison`
 }
+
+func badCountWithComments(s, sub string) bool {
+	return strings.Count(s, sub /* substr */) > 0 // want `use strings\.Contains\(s, sub\) instead of strings\.Count comparison`
+}

pkg/linters/stringscountcontains/testdata/src/stringscountcontains/stringscountcontains.go.golden

@@ -62,3 +62,7 @@ func badParenCountGTR(s, sub string) bool {
 func badParenYodaCountEQL(s, sub string) bool {
 	return !strings.Contains(s, sub) // want `use !strings\.Contains\(s, sub\) instead of strings\.Count comparison`
 }
+
+func badCountWithComments(s, sub string) bool {
+	return strings.Count(s, sub /* substr */) > 0 // want `use strings\.Contains\(s, sub\) instead of strings\.Count comparison`
+}

pkg/linters/stringsindexcontains/stringsindexcontains.go

@@ -64,7 +64,7 @@ func analyzeIndexContains(pass *analysis.Pass, n ast.Node, generatedFiles filech
 	} else {
 		msg = "use strings.Contains(" + sText + ", " + subText + ") instead of strings.Index comparison"
 	}
-	fix := astutil.BuildContainsFix(expr, pkgText, sText, subText, negated, "Replace strings.Index comparison with strings.Contains")
+	fix := astutil.BuildContainsFix(pass.Files, expr, pkgText, sText, subText, negated, "Replace strings.Index comparison with strings.Contains")
 	pass.Report(analysis.Diagnostic{
 		Pos:            expr.Pos(),
 		End:            expr.End(),

pkg/linters/stringsindexcontains/testdata/src/stringsindexcontains/stringsindexcontains.go

@@ -72,3 +72,7 @@ func badParenContains(s, sub string) bool {
 func badParenYodaNotContains(s, sub string) bool {
 	return -1 == (strings.Index(s, sub)) // want `use !strings\.Contains\(s, sub\) instead of strings\.Index comparison`
 }
+
+func badIndexWithComments(s, sub string) bool {
+	return strings.Index(s, sub /* substr */) != -1 // want `use strings\.Contains\(s, sub\) instead of strings\.Index comparison`
+}

pkg/linters/stringsindexcontains/testdata/src/stringsindexcontains/stringsindexcontains.go.golden

@@ -72,3 +72,7 @@ func badParenContains(s, sub string) bool {
 func badParenYodaNotContains(s, sub string) bool {
 	return !strings.Contains(s, sub) // want `use !strings\.Contains\(s, sub\) instead of strings\.Index comparison`
 }
+
+func badIndexWithComments(s, sub string) bool {
+	return strings.Index(s, sub /* substr */) != -1 // want `use strings\.Contains\(s, sub\) instead of strings\.Index comparison`
+}

pkg/linters/stringsjoinone/stringsjoinone.go

@@ -78,19 +78,22 @@ func analyzeJoinOne(pass *analysis.Pass, n ast.Node, generatedFiles filecheck.Ge
 		return
 	}
 
-	pass.Report(analysis.Diagnostic{
+	diag := analysis.Diagnostic{
 		Pos:     call.Pos(),
 		End:     call.End(),
 		Message: fmt.Sprintf("strings.Join called with a single-element slice; use %s directly", replacementText),
-		SuggestedFixes: []analysis.SuggestedFix{{
+	}
+	if !astutil.HasOverlappingComment(pass.Files, call.Pos(), call.End()) {
+		diag.SuggestedFixes = []analysis.SuggestedFix{{
 			Message: "Replace strings.Join call with " + replacementText,
 			TextEdits: []analysis.TextEdit{{
 				Pos:     call.Pos(),
 				End:     call.End(),
 				NewText: []byte(replacementText),
 			}},
-		}},
-	})
+		}}
+	}
+	pass.Report(diag)
 }
 
 // isSafeToDiscardSeparator reports whether sep is a compile-time constant and

pkg/linters/stringsjoinone/testdata/src/stringsjoinone/stringsjoinone.go

@@ -28,6 +28,11 @@ func joinOneSliced(a, b string) string {
 	return strings.Join([]string{a + b}, "")[1:] // want `strings\.Join called with a single-element slice`
 }
 
+// flagged: single-element []string literal with inline comment; fix suppressed to preserve comment.
+func joinOneWithComments(name string) string {
+	return strings.Join([]string{name /* display */}, ",") // want `strings\.Join called with a single-element slice`
+}
+
 // not flagged: two-element slice literal.
 func joinTwo(a, b string) string {
 	return strings.Join([]string{a, b}, ", ")

pkg/linters/stringsjoinone/testdata/src/stringsjoinone/stringsjoinone.go.golden

@@ -28,6 +28,11 @@ func joinOneSliced(a, b string) string {
 	return (a + b)[1:] // want `strings\.Join called with a single-element slice`
 }
 
+// flagged: single-element []string literal with inline comment; fix suppressed to preserve comment.
+func joinOneWithComments(name string) string {
+	return strings.Join([]string{name /* display */}, ",") // want `strings\.Join called with a single-element slice`
+}
+
 // not flagged: two-element slice literal.
 func joinTwo(a, b string) string {
 	return strings.Join([]string{a, b}, ", ")

pkg/linters/timenowsub/testdata/src/timenowsub/timenowsub.go

@@ -25,6 +25,10 @@ func badIndex(starts []time.Time, i int) time.Duration {
 	return time.Now().Sub(starts[i]) // want `time\.Now\(\)\.Sub\(starts\[i\]\) can be simplified to time\.Since\(starts\[i\]\)`
 }
 
+func badWithComments(start time.Time) time.Duration {
+	return time.Now().Sub(start /* measured from init */) // want `time\.Now\(\)\.Sub\(start\) can be simplified to time\.Since\(start\)`
+}
+
 func good(t time.Time) {
 	_ = time.Since(t)
 }

pkg/linters/timenowsub/testdata/src/timenowsub/timenowsub.go.golden

@@ -25,6 +25,10 @@ func badIndex(starts []time.Time, i int) time.Duration {
 	return time.Since(starts[i]) // want `time\.Now\(\)\.Sub\(starts\[i\]\) can be simplified to time\.Since\(starts\[i\]\)`
 }
 
+func badWithComments(start time.Time) time.Duration {
+	return time.Now().Sub(start /* measured from init */) // want `time\.Now\(\)\.Sub\(start\) can be simplified to time\.Since\(start\)`
+}
+
 func good(t time.Time) {
 	_ = time.Since(t)
 }

pkg/linters/timenowsub/timenowsub.go

@@ -73,19 +73,22 @@ func analyzeTimeNowSub(pass *analysis.Pass, n ast.Node, generatedFiles filecheck
 	}
 	sinceText := qualifier + ".Since(" + argText + ")"
 
-	pass.Report(analysis.Diagnostic{
+	diag := analysis.Diagnostic{
 		Pos:     outer.Pos(),
 		End:     outer.End(),
 		Message: fmt.Sprintf("%s.Now().Sub(%s) can be simplified to %s", qualifier, argText, sinceText),
-		SuggestedFixes: []analysis.SuggestedFix{{
+	}
+	if !astutil.HasOverlappingComment(pass.Files, outer.Pos(), outer.End()) {
+		diag.SuggestedFixes = []analysis.SuggestedFix{{
 			Message: fmt.Sprintf("Replace %s.Now().Sub(%s) with %s", qualifier, argText, sinceText),
 			TextEdits: []analysis.TextEdit{{
 				Pos:     outer.Pos(),
 				End:     outer.End(),
 				NewText: []byte(sinceText),
 			}},
-		}},
-	})
+		}}
+	}
+	pass.Report(diag)
 }
 
 // timeNowQualifier reports the imported identifier used for time.Now().