Skip to content

panicinlibrarycode: enforce FuncLit boundaries for init/doc panic exemptions #41631

New issue

Have a question about this project? Sign up for a free GitHub account to open an issue and contact its maintainers and the community.

Sign up for GitHub

By clicking “Sign up for GitHub”, you agree to our terms of service and privacy statement. We’ll occasionally send you account related emails.

Already on GitHub? Sign in to your account

Merged
pelikhan merged 2 commits into from
Jun 26, 2026
+24 −2
Merged

panicinlibrarycode: enforce FuncLit boundaries for init/doc panic exemptions #41631

Show file tree
Hide file tree
Changes from all commits
Commits
Show all changes
2 commits
Select commit Hold shift + click to select a range
dd00ff5
Initial plan
Copilot Jun 26, 2026
6c642b8
fix panicinlibrarycode FuncLit exemption boundaries
Copilot Jun 26, 2026
File filter

Filter by extension

Filter by extension
Conversations
Failed to load comments.
Loading
Jump to
Jump to file
Failed to load files.
Loading
Diff view
Diff view
10 changes: 8 additions & 2 deletions pkg/linters/panic-in-library-code/panic-in-library-code.go
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -179,7 +179,10 @@ func isFmtSprintf(pass *analysis.Pass, call *ast.CallExpr) bool {
// function. Only top-level (no receiver) init functions are recognized;
// methods named init are ordinary methods and are not exempt.
func isInInitFunction(cur inspector.Cursor) bool {
for encl := range cur.Enclosing((*ast.FuncDecl)(nil)) {
for encl := range cur.Enclosing((*ast.FuncDecl)(nil), (*ast.FuncLit)(nil)) {
if _, isFuncLit := encl.Node().(*ast.FuncLit); isFuncLit {
return false
}

@github-actions github-actions Bot Jun 26, 2026

Copy link
Copy Markdown
Contributor

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

[/improve-codebase-architecture] The identical 3-line FuncLit boundary guard is duplicated verbatim in isInInitFunction (lines 183–185) and hasDocumentedPanicContract (lines 200–202). A third exemption function added later would likely miss this invariant.

💡 Suggested: extract a shared helper 
// findEnclosingFuncDecl returns the innermost enclosing *ast.FuncDecl,
// or nil if a FuncLit boundary is crossed first.
func findEnclosingFuncDecl(cur inspector.Cursor) *ast.FuncDecl {
    for encl := range cur.Enclosing((*ast.FuncDecl)(nil), (*ast.FuncLit)(nil)) {
        if _, ok := encl.Node().(*ast.FuncLit); ok {
            return nil
        }
        if decl, ok := encl.Node().(*ast.FuncDecl); ok {
            return decl
        }
        break
    }
    return nil
}

Both callers become single-expression checks, and the boundary invariant lives in one place.

decl, ok := encl.Node().(*ast.FuncDecl)
if !ok {

@github-actions github-actions Bot Jun 26, 2026

Copy link
Copy Markdown
Contributor

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Dead code: if !ok { break } is unreachable after the new FuncLit early-return, silently misleading future readers about the invariants.

💡 Details and fix

cur.Enclosing((*ast.FuncDecl)(nil), (*ast.FuncLit)(nil)) yields only *ast.FuncDecl and *ast.FuncLit nodes. The new early return false handles every *ast.FuncLit, so any code that reaches this type assertion is guaranteed to hold a *ast.FuncDecl — ok is always true. The if !ok { break } block is permanently dead.

The same issue exists at line 204 in hasDocumentedPanicContract.

Replace both with a direct assertion that makes the invariant explicit:

decl := encl.Node().(*ast.FuncDecl)

break
Expand All @@ -193,7 +196,10 @@ func isInInitFunction(cur inspector.Cursor) bool {
}

func hasDocumentedPanicContract(cur inspector.Cursor) bool {
for encl := range cur.Enclosing((*ast.FuncDecl)(nil)) {
for encl := range cur.Enclosing((*ast.FuncDecl)(nil), (*ast.FuncLit)(nil)) {
if _, isFuncLit := encl.Node().(*ast.FuncLit); isFuncLit {
return false
}
decl, ok := encl.Node().(*ast.FuncDecl)
if !ok {
break
Expand Down
16 changes: 16 additions & 0 deletions pkg/linters/panic-in-library-code/testdata/src/panicinlibrarycode/panicinlibrarycode.go
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -43,6 +43,13 @@ func init() {
panic("startup registration failure") // should not be flagged
}

func init() {
handler := func() {
panic("handler panic outside init flow") // want `avoid panic in library code; return an error instead`
}
_ = handler
}

@github-actions github-actions Bot Jun 26, 2026

Copy link
Copy Markdown
Contributor

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

[/tdd] The test covers one level of closure nesting. Consider adding a two-level case to explicitly document that the boundary check handles deep nesting (the algorithm is correct because Enclosing yields innermost-first, but a test makes that contract visible).

💡 Suggested test case 
func init() {
    outer := func() {
        inner := func() {
            panic("deeply nested") // want `avoid panic in library code; return an error instead`
        }
        _ = inner
    }
    _ = outer
}

Without this, a future change that accidentally breaks inner-to-outer traversal order would not be caught by these tests.


// ok: panic inside a sync.Once.Do callback.

@github-actions github-actions Bot Jun 26, 2026

Copy link
Copy Markdown
Contributor

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Missing test for sync.Once.Do callback inside init() — the exemption of this case now depends silently on check-ordering in shouldSkipPanic, with no regression test to protect it.

💡 Details and suggested test

The FuncLit boundary change makes isInInitFunction return false for any panic inside a closure within init(). The sync.Once.Do callback is still correctly exempt only because isInSyncOnceDoFuncLit short-circuits before isInInitFunction in shouldSkipPanic:

return isInSyncOnceDoFuncLit(pass, cur) || // must fire first
    panicMessageStartsWithBUG(pass, call) ||
    isInInitFunction(cur) ||               // now returns false for any FuncLit boundary
    hasDocumentedPanicContract(cur)

This ordering contract is undocumented. If the checks are ever reordered, the sync.Once.Do-inside-init() shape silently breaks (becomes a false positive). No test covers it today.

Add a companion case here:

func init() {
    once.Do(func() {
        panic("lazy init inside init()") // should not be flagged
    })
}

var once sync.Once

Expand Down Expand Up @@ -71,6 +78,15 @@ func documentedPreconditionPanics(mode string) {
}
}

// documentedClosureRegistration panics if called with an empty input.
func documentedClosureRegistration(input string) {
callback := func() {
panic("callback panic should be reported") // want `avoid panic in library code; return an error instead`
}
_ = callback
_ = input
}

// ok: method named init is NOT a top-level init; its panic should be flagged.
type myInitType struct{}

Expand Down
Loading