Skip to content

panicinlibrarycode: init() and documented-contract exemptions cross FuncLit boundaries — false negatives for panics in nested cl [Content truncated due to length] #41606

Description

@github-actions

Summary

panicinlibrarycode exempts panics that sit lexically inside init() functions and inside functions whose doc comment documents a panic contract (panics if/on ...). Both exemption checks walk to the nearest enclosing *ast.FuncDecl while skipping *ast.FuncLit boundaries, so a panic() inside a function literal nested in such a function is wrongly suppressed. This is a false negative on exactly the kind of deferred/stored/goroutine closure where a panic is most dangerous.

The linter already knows how to respect FuncLit boundaries — its sibling exemption isInSyncOnceDoFuncLit iterates enclosing *ast.FuncLit nodes correctly. The two FuncDecl-only walks are inconsistent with it.

Evidence

pkg/linters/panic-in-library-code/panic-in-library-code.go

isInInitFunction (lines 181–193):

func isInInitFunction(cur inspector.Cursor) bool {
	for encl := range cur.Enclosing((*ast.FuncDecl)(nil)) { // skips FuncLit ancestors
		decl, ok := encl.Node().(*ast.FuncDecl)
		if !ok { break }
		if decl.Recv == nil && decl.Name != nil && decl.Name.Name == "init" {
			return true
		}
		break // only the immediate enclosing FuncDecl
	}
	return false
}

hasDocumentedPanicContract (lines 195–213) has the identical shape: cur.Enclosing((*ast.FuncDecl)(nil)) filters to FuncDecl only, so any intervening FuncLit is transparent and the walk lands on the outer documented FuncDecl.

Because Enclosing((*ast.FuncDecl)(nil)) yields the innermost matching ancestor and the filter excludes FuncLit, a closure defined inside init() (or inside a documented function) inherits the parent's exemption:

func init() {
	http.HandleFunc("/", func(w http.ResponseWriter, r *http.Request) {
		panic("runs at request time, NOT init time") // exempted (FN) — init() can't return an error, but this handler can
	})
}

// Parse panics if the input is malformed.
func Parse(s string) {
	registerCallback(func() {
		panic("invoked later, unrelated to Parse's contract") // exempted (FN)
	})
}

The stored/registered closure runs in a completely different context from the init() load phase or the documented synchronous contract, yet inherits the exemption.

Why this is the established sergo scope-boundary pattern

Same root cause as the already-accepted findings:

Canonical correct shape in this same repo: deferinloop / execcommandwithoutcontext use cur.Enclosing((*ast.FuncDecl)(nil), (*ast.FuncLit)(nil)) and break at the FuncLit boundary. And this very file's isInSyncOnceDoFuncLit (lines 88–108) already iterates cur.Enclosing((*ast.FuncLit)(nil)) correctly — so the fix is consistent with existing in-file precedent.

Testdata gap

pkg/linters/panic-in-library-code/testdata/src/panicinlibrarycode/panicinlibrarycode.go covers a direct panic in init() (line 43) and a direct panic in the documented function documentedPreconditionPanics (line 70), but no case where the panic sits inside a FuncLit nested in either — so the false negative is invisible to the suite.

Recommendation

Make both exemptions stop at the FuncLit boundary, mirroring isInSyncOnceDoFuncLit and the deferinloop precedent. Include *ast.FuncLit in the Enclosing filter and treat an innermost *ast.FuncLit as "not directly in init / not the documented body":

for encl := range cur.Enclosing((*ast.FuncDecl)(nil), (*ast.FuncLit)(nil)) {
	if _, isLit := encl.Node().(*ast.FuncLit); isLit {
		return false // panic is inside a closure, not directly in init()/the documented FuncDecl
	}
	decl, ok := encl.Node().(*ast.FuncDecl)
	if !ok { break }
	// ... existing init()/doc-contract checks ...
	break
}

(Apply the analogous change to hasDocumentedPanicContract.)

Secondary observation (same function)

hasDocumentedPanicContract matches the doc substrings panics if/on, panic if/on anywhere in the comment, so a negated contract such as // Foo never panics on a nil input. also exempts every panic in Foo. Lower priority than the FuncLit boundary, but worth tightening (e.g. anchor the phrase) while touching this function.

Validation checklist

  • Add testdata: a panic inside a func(){...} closure declared in init() → expect a diagnostic.
  • Add testdata: a panic inside a closure nested in a panics if ... documented function → expect a diagnostic.
  • Confirm direct-in-init() and direct-in-documented-function panics remain exempt (no regression on lines 43/70 cases).
  • Confirm sync.Once.Do closure panic stays exempt (line 51 case unchanged).

Effort

Small — two functions in one file plus two testdata cases. Pattern and correct reference already exist in-repo.

References: §28218238847

Generated by 🤖 Sergo - Serena Go Expert · 236.2 AIC · ⌖ 13.7 AIC · ⊞ 5.9K ·

  • expires on Jul 2, 2026, 9:11 PM UTC-08:00

Activity

Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment

Metadata

Metadata

Labels

cookieIssue Monster Loves Cookies!sergo

Type

No type

Projects

No projects

    Milestone

    No milestone

    Relationships

    None yet

    Development

    No branches or pull requests

    Issue actions