diff --git a/.github/workflows/agentic-auto-upgrade.yml b/.github/workflows/agentic-auto-upgrade.yml index 7035101ee35..e169cea32b6 100644 --- a/.github/workflows/agentic-auto-upgrade.yml +++ b/.github/workflows/agentic-auto-upgrade.yml @@ -34,7 +34,7 @@ name: Agentic Auto-Upgrade on: schedule: - - cron: "21 3 * * 5" # Weekly (auto-upgrade) + - cron: "11 4 * * 6" # Weekly (auto-upgrade) workflow_dispatch: permissions: diff --git a/pkg/linters/internal/astutil/astutil.go b/pkg/linters/internal/astutil/astutil.go index c9e524defa5..44abedde63e 100644 --- a/pkg/linters/internal/astutil/astutil.go +++ b/pkg/linters/internal/astutil/astutil.go @@ -345,6 +345,27 @@ func QualifierShadowed(pkg *types.Package, pos token.Pos, name, importPath strin return pkgName.Imported().Path() != importPath } +// HasOverlappingComment reports whether any comment group in files overlaps +// the half-open range [start, end). This is used by linters to suppress a +// SuggestedFix when a comment inside the to-be-replaced span would otherwise +// be silently discarded by the autofix tool. +func HasOverlappingComment(files []*ast.File, start, end token.Pos) bool { + for _, file := range files { + if file == nil { + continue + } + if end <= file.Pos() || start >= file.End() { + continue + } + for _, group := range file.Comments { + if group.Pos() < end && start < group.End() { + return true + } + } + } + return false +} + // IsByteSlice reports whether expr has underlying type []byte ([]uint8). func IsByteSlice(pass *analysis.Pass, expr ast.Expr) bool { t := pass.TypesInfo.TypeOf(expr) diff --git a/pkg/linters/mapclearloop/mapclearloop.go b/pkg/linters/mapclearloop/mapclearloop.go index 6771d4b4afc..ea597a8f6c8 100644 --- a/pkg/linters/mapclearloop/mapclearloop.go +++ b/pkg/linters/mapclearloop/mapclearloop.go @@ -137,7 +137,7 @@ func run(pass *analysis.Pass) (any, error) { End: rangeStmt.End(), Message: "range-delete loop over map can be replaced with clear(" + mText + ")", } - if !hasOverlappingComment(pass.Files, rangeStmt.Pos(), rangeStmt.End()) { + if !astutil.HasOverlappingComment(pass.Files, rangeStmt.Pos(), rangeStmt.End()) { diag.SuggestedFixes = []analysis.SuggestedFix{{ Message: "Replace range-delete loop with clear", TextEdits: []analysis.TextEdit{{ @@ -170,21 +170,6 @@ func builtinVisibleAtPos(pkg *types.Package, pos token.Pos, name string) bool { return ok && builtin.Name() == name } -// hasOverlappingComment reports whether any comment group overlaps [start, end). -func hasOverlappingComment(files []*ast.File, start, end token.Pos) bool { - for _, file := range files { - if end <= file.Pos() || start >= file.End() { - continue - } - for _, group := range file.Comments { - if group.Pos() < end && start < group.End() { - return true - } - } - } - return false -} - // sameObject reports whether expr refers to the same declared object as ref. // ref is expected to be an *ast.Ident or *ast.SelectorExpr. func sameObject(pass *analysis.Pass, expr, ref ast.Expr) bool { diff --git a/pkg/linters/mapdeletecheck/mapdeletecheck.go b/pkg/linters/mapdeletecheck/mapdeletecheck.go index f4a0994eee4..8f1992fe696 100644 --- a/pkg/linters/mapdeletecheck/mapdeletecheck.go +++ b/pkg/linters/mapdeletecheck/mapdeletecheck.go @@ -116,19 +116,22 @@ func run(pass *analysis.Pass) (any, error) { mText := astutil.NodeText(pass.Fset, mapExpr) kText := astutil.NodeText(pass.Fset, keyExpr) - pass.Report(analysis.Diagnostic{ + diag := analysis.Diagnostic{ Pos: ifStmt.Pos(), End: ifStmt.End(), Message: "redundant existence check before delete: delete(" + mText + ", " + kText + ") is already a no-op when the key is absent; remove the if statement", - SuggestedFixes: []analysis.SuggestedFix{{ + } + if !astutil.HasOverlappingComment(pass.Files, ifStmt.Pos(), ifStmt.End()) { + diag.SuggestedFixes = []analysis.SuggestedFix{{ Message: "Replace if-check with plain delete", TextEdits: []analysis.TextEdit{{ Pos: ifStmt.Pos(), End: ifStmt.End(), NewText: []byte("delete(" + mText + ", " + kText + ")"), }}, - }}, - }) + }} + } + pass.Report(diag) }) return nil, nil diff --git a/pkg/linters/mapdeletecheck/testdata/src/mapdeletecheck/mapdeletecheck.go b/pkg/linters/mapdeletecheck/testdata/src/mapdeletecheck/mapdeletecheck.go index a9628c58fe0..e3f707511e9 100644 --- a/pkg/linters/mapdeletecheck/testdata/src/mapdeletecheck/mapdeletecheck.go +++ b/pkg/linters/mapdeletecheck/testdata/src/mapdeletecheck/mapdeletecheck.go @@ -4,16 +4,36 @@ func bad() { m := map[string]int{"a": 1, "b": 2} k := "a" - if _, ok := m[k]; ok { // want `redundant existence check before delete` + // want +1 `redundant existence check before delete` + if _, ok := m[k]; ok { delete(m, k) } // With a literal key. - if _, ok := m["b"]; ok { // want `redundant existence check before delete` + // want +1 `redundant existence check before delete` + if _, ok := m["b"]; ok { delete(m, "b") } } +func badWithComments() { + cache := map[string]int{"key": 1} + key := "key" + + // Leading comment inside the if body suppresses autofix but still reports. + // want +1 `redundant existence check before delete` + if _, ok := cache[key]; ok { + // evict the stale entry before refetching + delete(cache, key) + } + + // Trailing comment on the delete line suppresses autofix but still reports. + // want +1 `redundant existence check before delete` + if _, ok := cache[key]; ok { + delete(cache, key) // evict + } +} + func good() { m := map[string]int{"a": 1} k := "a" diff --git a/pkg/linters/mapdeletecheck/testdata/src/mapdeletecheck/mapdeletecheck.go.golden b/pkg/linters/mapdeletecheck/testdata/src/mapdeletecheck/mapdeletecheck.go.golden index a117246e63c..ceaeaf583cc 100644 --- a/pkg/linters/mapdeletecheck/testdata/src/mapdeletecheck/mapdeletecheck.go.golden +++ b/pkg/linters/mapdeletecheck/testdata/src/mapdeletecheck/mapdeletecheck.go.golden @@ -4,12 +4,32 @@ func bad() { m := map[string]int{"a": 1, "b": 2} k := "a" + // want +1 `redundant existence check before delete` delete(m, k) // With a literal key. + // want +1 `redundant existence check before delete` delete(m, "b") } +func badWithComments() { + cache := map[string]int{"key": 1} + key := "key" + + // Leading comment inside the if body suppresses autofix but still reports. + // want +1 `redundant existence check before delete` + if _, ok := cache[key]; ok { + // evict the stale entry before refetching + delete(cache, key) + } + + // Trailing comment on the delete line suppresses autofix but still reports. + // want +1 `redundant existence check before delete` + if _, ok := cache[key]; ok { + delete(cache, key) // evict + } +} + func good() { m := map[string]int{"a": 1} k := "a"