diff --git a/.github/workflows/pr-code-quality-reviewer.lock.yml b/.github/workflows/pr-code-quality-reviewer.lock.yml index 2eb30522101..de80f4e5b1d 100644 --- a/.github/workflows/pr-code-quality-reviewer.lock.yml +++ b/.github/workflows/pr-code-quality-reviewer.lock.yml @@ -1,4 +1,4 @@ -# gh-aw-metadata: {"schema_version":"v3","frontmatter_hash":"59d9079693d460a3223abbea8407918eec6100f4c836dd061666af1f483d2d77","strict":true,"agent_id":"copilot"} +# gh-aw-metadata: {"schema_version":"v3","frontmatter_hash":"7725718bcffab710f5005e7e836f7d77ec6a461a281ea997fbbb7e5e1b82b94e","strict":true,"agent_id":"copilot"} # gh-aw-manifest: {"version":1,"secrets":["COPILOT_GITHUB_TOKEN","GH_AW_GITHUB_MCP_SERVER_TOKEN","GH_AW_GITHUB_TOKEN","GH_AW_OTEL_GRAFANA_AUTHORIZATION","GH_AW_OTEL_GRAFANA_ENDPOINT","GH_AW_OTEL_SENTRY_AUTHORIZATION","GH_AW_OTEL_SENTRY_ENDPOINT","GITHUB_TOKEN"],"actions":[{"repo":"actions/checkout","sha":"de0fac2e4500dabe0009e67214ff5f5447ce83dd","version":"v6.0.2"},{"repo":"actions/download-artifact","sha":"3e5f45b2cfb9172054b4087a40e8e0b5a5461e7c","version":"v8.0.1"},{"repo":"actions/github-script","sha":"3a2844b7e9c422d3c10d287c895573f7108da1b3","version":"v9.0.0"},{"repo":"actions/setup-node","sha":"48b55a011bda9f5d6aeb4c2d9c7362e8dae4041e","version":"v6.4.0"},{"repo":"actions/upload-artifact","sha":"043fb46d1a93c77aae656e7c1c64a875d1fc6a0a","version":"v7.0.1"}],"containers":[{"image":"ghcr.io/github/gh-aw-firewall/agent:0.25.50"},{"image":"ghcr.io/github/gh-aw-firewall/api-proxy:0.25.50"},{"image":"ghcr.io/github/gh-aw-firewall/squid:0.25.50"},{"image":"ghcr.io/github/gh-aw-mcpg:v0.3.16","digest":"sha256:8001e4bfa52d45abd05c45a8f855ce62bc893eb66e4807bb487bf2ff07fc1473","pinned_image":"ghcr.io/github/gh-aw-mcpg:v0.3.16@sha256:8001e4bfa52d45abd05c45a8f855ce62bc893eb66e4807bb487bf2ff07fc1473"},{"image":"ghcr.io/github/github-mcp-server:v1.0.4"},{"image":"node:lts-alpine","digest":"sha256:d1b3b4da11eefd5941e7f0b9cf17783fc99d9c6fc34884a665f40a06dbdfc94f","pinned_image":"node:lts-alpine@sha256:d1b3b4da11eefd5941e7f0b9cf17783fc99d9c6fc34884a665f40a06dbdfc94f"}]} # ___ _ _ # / _ \ | | (_) @@ -30,7 +30,6 @@ # - shared/otlp.md # - shared/pr-code-review-config.md # - shared/pr-review-base.md -# - shared/reporting.md # # Secrets used: # - COPILOT_GITHUB_TOKEN @@ -254,20 +253,20 @@ jobs: run: | bash "${RUNNER_TEMP}/gh-aw/actions/create_prompt_first.sh" { - cat << 'GH_AW_PROMPT_03f9bc9676534c1b_EOF' + cat << 'GH_AW_PROMPT_4f5501639950bddf_EOF' - GH_AW_PROMPT_03f9bc9676534c1b_EOF + GH_AW_PROMPT_4f5501639950bddf_EOF cat "${RUNNER_TEMP}/gh-aw/prompts/xpia.md" cat "${RUNNER_TEMP}/gh-aw/prompts/temp_folder_prompt.md" cat "${RUNNER_TEMP}/gh-aw/prompts/markdown.md" cat "${RUNNER_TEMP}/gh-aw/prompts/safe_outputs_prompt.md" - cat << 'GH_AW_PROMPT_03f9bc9676534c1b_EOF' + cat << 'GH_AW_PROMPT_4f5501639950bddf_EOF' Tools: create_pull_request_review_comment(max:10), submit_pull_request_review, missing_tool, missing_data, noop - GH_AW_PROMPT_03f9bc9676534c1b_EOF + GH_AW_PROMPT_4f5501639950bddf_EOF cat "${RUNNER_TEMP}/gh-aw/prompts/mcp_cli_tools_prompt.md" - cat << 'GH_AW_PROMPT_03f9bc9676534c1b_EOF' + cat << 'GH_AW_PROMPT_4f5501639950bddf_EOF' The following GitHub context information is available for this workflow: {{#if github.actor}} @@ -296,20 +295,19 @@ jobs: {{/if}} - GH_AW_PROMPT_03f9bc9676534c1b_EOF + GH_AW_PROMPT_4f5501639950bddf_EOF cat "${RUNNER_TEMP}/gh-aw/prompts/github_mcp_tools_with_safeoutputs_prompt.md" if [ "$GITHUB_EVENT_NAME" = "issue_comment" ] && [ -n "$GH_AW_IS_PR_COMMENT" ] || [ "$GITHUB_EVENT_NAME" = "pull_request_review_comment" ] || [ "$GITHUB_EVENT_NAME" = "pull_request_review" ]; then cat "${RUNNER_TEMP}/gh-aw/prompts/pr_context_prompt.md" fi - cat << 'GH_AW_PROMPT_03f9bc9676534c1b_EOF' + cat << 'GH_AW_PROMPT_4f5501639950bddf_EOF' - {{#runtime-import .github/workflows/shared/reporting.md}} {{#runtime-import .github/workflows/shared/otlp.md}} {{#runtime-import .github/workflows/shared/github-guard-policy.md}} {{#runtime-import .github/workflows/shared/pr-code-review-config.md}} {{#runtime-import .github/workflows/shared/noop-reminder.md}} {{#runtime-import .github/workflows/pr-code-quality-reviewer.md}} - GH_AW_PROMPT_03f9bc9676534c1b_EOF + GH_AW_PROMPT_4f5501639950bddf_EOF } > "$GH_AW_PROMPT" - name: Interpolate variables and render templates uses: actions/github-script@3a2844b7e9c422d3c10d287c895573f7108da1b3 # v9.0.0 @@ -528,9 +526,9 @@ jobs: mkdir -p "${RUNNER_TEMP}/gh-aw/safeoutputs" mkdir -p /tmp/gh-aw/safeoutputs mkdir -p /tmp/gh-aw/mcp-logs/safeoutputs - cat > "${RUNNER_TEMP}/gh-aw/safeoutputs/config.json" << 'GH_AW_SAFE_OUTPUTS_CONFIG_11433c4998bea54d_EOF' + cat > "${RUNNER_TEMP}/gh-aw/safeoutputs/config.json" << 'GH_AW_SAFE_OUTPUTS_CONFIG_7c6eb47588630d8a_EOF' {"create_pull_request_review_comment":{"max":10,"side":"RIGHT"},"create_report_incomplete_issue":{},"missing_data":{},"missing_tool":{},"noop":{"max":1,"report-as-issue":"true"},"report_incomplete":{},"submit_pull_request_review":{"max":1}} - GH_AW_SAFE_OUTPUTS_CONFIG_11433c4998bea54d_EOF + GH_AW_SAFE_OUTPUTS_CONFIG_7c6eb47588630d8a_EOF - name: Generate Safe Outputs Tools env: GH_AW_TOOLS_META_JSON: | @@ -758,7 +756,7 @@ jobs: mkdir -p /home/runner/.copilot GH_AW_NODE=$(which node 2>/dev/null || command -v node 2>/dev/null || echo node) - cat << GH_AW_MCP_CONFIG_9e7251805d038456_EOF | "$GH_AW_NODE" "${RUNNER_TEMP}/gh-aw/actions/start_mcp_gateway.cjs" + cat << GH_AW_MCP_CONFIG_93da1e40a4216991_EOF | "$GH_AW_NODE" "${RUNNER_TEMP}/gh-aw/actions/start_mcp_gateway.cjs" { "mcpServers": { "github": { @@ -807,7 +805,7 @@ jobs: } } } - GH_AW_MCP_CONFIG_9e7251805d038456_EOF + GH_AW_MCP_CONFIG_93da1e40a4216991_EOF - name: Mount MCP servers as CLIs id: mount-mcp-clis continue-on-error: true diff --git a/cmd/linters/main.go b/cmd/linters/main.go index 31e19d8031c..ccc733d9574 100644 --- a/cmd/linters/main.go +++ b/cmd/linters/main.go @@ -20,6 +20,7 @@ import ( "github.com/github/gh-aw/pkg/linters/errormessage" "github.com/github/gh-aw/pkg/linters/errstringmatch" "github.com/github/gh-aw/pkg/linters/excessivefuncparams" + "github.com/github/gh-aw/pkg/linters/fileclosenotdeferred" "github.com/github/gh-aw/pkg/linters/largefunc" "github.com/github/gh-aw/pkg/linters/osexitinlibrary" "github.com/github/gh-aw/pkg/linters/rawloginlib" @@ -33,6 +34,7 @@ func main() { errormessage.Analyzer, errstringmatch.Analyzer, excessivefuncparams.Analyzer, + fileclosenotdeferred.Analyzer, largefunc.Analyzer, osexitinlibrary.Analyzer, rawloginlib.Analyzer, diff --git a/docs/adr/33834-add-file-close-not-deferred-linter.md b/docs/adr/33834-add-file-close-not-deferred-linter.md new file mode 100644 index 00000000000..a220df7a7cd --- /dev/null +++ b/docs/adr/33834-add-file-close-not-deferred-linter.md @@ -0,0 +1,84 @@ +# ADR-33834: Add file-close-not-deferred Linter + +**Date**: 2026-05-21 +**Status**: Draft +**Deciders**: Unknown + +--- + +## Part 1 — Narrative (Human-Friendly) + +### Context + +An automated scan of the codebase (linter-miner run #12) found 3 instances where files opened via `os.Open` / `os.OpenFile` are closed manually instead of with `defer file.Close()` — `pkg/cli/workflows.go:307`, `pkg/cli/workflows.go:396`, and `pkg/cli/mcp_logs_guardrail.go:87`. Manual close is the canonical Go anti-pattern for file handles: early returns, panics, or new branches added later between open and close cause descriptor leaks, and forgetting `defer` is easy to miss in review. The repository already houses a family of small, focused, in-house analyzers under `pkg/linters/` (e.g., `largefunc`, `osexitinlibrary`, `regexpcompileinfunction`) registered through `cmd/linters/main.go`, so the established convention is to add another analyzer rather than rely on review discipline or external tooling. + +### Decision + +We will add a new static-analysis linter, `fileclosenotdeferred`, that flags functions where a file opened by `os.Open`, `os.Create`, or `os.OpenFile` is closed via a non-deferred `Close()` call. The linter lives under `pkg/linters/fileclosenotdeferred/`, is registered in `cmd/linters/main.go` alongside the existing analyzers, walks each `*ast.FuncDecl` body (excluding nested `*ast.FuncLit` closures to avoid false positives) tracking per-variable state keyed by `types.Object` (to correctly handle variable shadowing), and reports the open position when a variable has a manual `Close()` and no matching `defer`. Test files are excluded via the shared `pkg/linters/internal/filecheck.IsTestFile` helper. + +### Alternatives Considered + +#### Alternative 1: Fix the 3 known instances and rely on review + +Patch the three flagged call sites to use `defer file.Close()` and trust reviewers to catch new instances. Rejected because the same anti-pattern was already present in three independent files, indicating that human review alone has not been sufficient; a mechanical check on every PR is cheaper and cannot be forgotten as the codebase grows. + +#### Alternative 2: Use a third-party linter (e.g., `gocritic`, `bodyclose`) + +General-purpose Go linters offer resource-leak rules with broader coverage. Rejected to stay consistent with the project's convention of small, focused, in-house analyzers under `pkg/linters/`, each as its own package with custom logic. Pulling in an external linter for a single rule introduces a new dependency surface, inconsistent rule configuration, and noise from rules the project has not opted into. + +#### Alternative 3: Combine with a broader "resource hygiene" linter + +Bundle this rule with future checks (e.g., `http.Response.Body` close, `sql.Rows` close, `io.Closer` in general) into one analyzer. Rejected because the existing convention is one analyzer per rule, which keeps each rule's `Analyzer.Doc` URL narrow, independently disable-able, and easy to extend without coupling unrelated checks. + +### Consequences + +#### Positive +- New non-deferred `Close()` patterns introduced after merge are caught by `make golint-custom` and fail in CI rather than landing on `main`. +- The linter follows the same `pkg/linters//` layout, `Analyzer` shape, and `testdata` convention as the sibling analyzers, so contributors can extend it without learning a new pattern. +- Creates incentive to clean up the 3 pre-existing manual-close sites in `pkg/cli/` to maintain a clean linter signal. + +#### Negative +- Detection is structural and intentionally narrow: it matches only direct calls to `os.Open` / `os.Create` / `os.OpenFile` and direct `.Close()` calls. It will miss aliased imports (`import o "os"`), files returned from helper functions, and `Close()` called on a field or selector. False negatives are accepted in exchange for zero false positives. +- The 3 pre-existing violations are not fixed in this PR, so the linter cannot be made a blocking CI gate without follow-up work or suppression. +- Adds one more analyzer to the registry, marginally increasing `cmd/linters/main.go` compile and run time. + +#### Neutral +- Test files are deliberately excluded via `filecheck.IsTestFile`, matching the convention used by the sibling linters. Test fixtures may legitimately close files inline. +- The `hasManuaClose` typo has been corrected to `hasManualClose`; the state struct is internal only. +- The diagnostic reports at the open position, not the manual-close position, so the warning points the reader to the place where `defer` should be inserted. + +--- + +## Part 2 — Normative Specification (RFC 2119) + +> The key words **MUST**, **MUST NOT**, **REQUIRED**, **SHALL**, **SHALL NOT**, **SHOULD**, **SHOULD NOT**, **RECOMMENDED**, **MAY**, and **OPTIONAL** in this section are to be interpreted as described in [RFC 2119](https://www.rfc-editor.org/rfc/rfc2119). + +### Linter Behaviour + +1. The analyzer **MUST** be exported as `fileclosenotdeferred.Analyzer` with `Name` equal to `"fileclosenotdeferred"`. +2. The analyzer **MUST** report a diagnostic for every local variable assigned from a call to `os.Open`, `os.Create`, or `os.OpenFile` when that variable's `Close()` method is invoked without a matching `defer` statement in the same `*ast.FuncDecl` body. +3. The analyzer **MUST NOT** report a diagnostic when the file variable's `Close()` is invoked via a `defer` statement anywhere in the same function body. +4. The analyzer **MUST NOT** report a diagnostic when the result of the open call is discarded with the blank identifier (`_`). +5. The analyzer **MUST NOT** report a diagnostic when the containing file is a Go test file as determined by `pkg/linters/internal/filecheck.IsTestFile`. +6. The diagnostic `Pos` **MUST** be the position of the originating open call (`os.Open` / `os.Create` / `os.OpenFile`), not the position of the manual `Close()`. +7. The diagnostic `Message` **SHOULD** read `"file Close() should be deferred immediately after successful open to prevent resource leaks"` so downstream tooling can match on a stable string. +8. The analyzer **MUST** declare `inspect.Analyzer` in its `Requires` list. + +### Registration + +1. The analyzer **MUST** be registered in `cmd/linters/main.go` via the `multichecker.Main` argument list alongside the existing custom analyzers. +2. The package import in `cmd/linters/main.go` **MUST** use the path `github.com/github/gh-aw/pkg/linters/fileclosenotdeferred`. + +### Package Layout + +1. The linter source **MUST** live under `pkg/linters/fileclosenotdeferred/`. +2. Test fixtures **MUST** live under `pkg/linters/fileclosenotdeferred/testdata/src/fileclosenotdeferred/` and **MUST** use `// want` comments compatible with `golang.org/x/tools/go/analysis/analysistest`. +3. The test fixtures **MUST** include at least one positive case (manual `Close()` flagged), one negative case (`defer file.Close()` not flagged), and one blank-identifier case (`_, err := os.Open(...)` not flagged). + +### Conformance + +An implementation is considered conformant with this ADR if it satisfies all **MUST** and **MUST NOT** requirements above. Failure to meet any **MUST** or **MUST NOT** requirement constitutes non-conformance. + +--- + +*This is a DRAFT ADR generated by the [Design Decision Gate](https://github.com/github/gh-aw/actions/runs/26245378772) workflow. The PR author must review, complete, and finalize this document before the PR can merge.* diff --git a/linters b/linters index ef8527aaa18..8aadbce8f6f 100755 Binary files a/linters and b/linters differ diff --git a/pkg/linters/fileclosenotdeferred/fileclosenotdeferred.go b/pkg/linters/fileclosenotdeferred/fileclosenotdeferred.go new file mode 100644 index 00000000000..acf2c62c838 --- /dev/null +++ b/pkg/linters/fileclosenotdeferred/fileclosenotdeferred.go @@ -0,0 +1,169 @@ +// Package fileclosenotdeferred implements a Go analysis linter that flags +// file operations where Close() is not immediately deferred. +package fileclosenotdeferred + +import ( + "go/ast" + "go/token" + "go/types" + + "golang.org/x/tools/go/analysis" + "golang.org/x/tools/go/analysis/passes/inspect" + "golang.org/x/tools/go/ast/inspector" + + "github.com/github/gh-aw/pkg/linters/internal/filecheck" +) + +// Analyzer is the file-close-not-deferred analysis pass. +var Analyzer = &analysis.Analyzer{ + Name: "fileclosenotdeferred", + Doc: "reports file operations where Close() is not immediately deferred, which can lead to resource leaks", + URL: "https://github.com/github/gh-aw/tree/main/pkg/linters/fileclosenotdeferred", + Requires: []*analysis.Analyzer{inspect.Analyzer}, + Run: run, +} + +func run(pass *analysis.Pass) (any, error) { + insp := pass.ResultOf[inspect.Analyzer].(*inspector.Inspector) + + nodeFilter := []ast.Node{ + (*ast.FuncDecl)(nil), + } + + insp.Preorder(nodeFilter, func(n ast.Node) { + fn, ok := n.(*ast.FuncDecl) + if !ok || fn.Body == nil { + return + } + + pos := pass.Fset.PositionFor(fn.Pos(), false) + if filecheck.IsTestFile(pos.Filename) { + return + } + + // Track file variables: types.Object -> *fileVarState (open position, hasDefer, hasManualClose) + // Keyed by types.Object so variable shadowing across inner scopes is handled correctly. + fileVars := make(map[types.Object]*fileVarState) + + // Walk all statements in the function body, including nested blocks, + // but stop at function literals so closures are analysed independently. + ast.Inspect(fn.Body, func(node ast.Node) bool { + if node == nil { + return false + } + + // Do not descend into function literals — closures are independent execution + // contexts and should be analyzed separately to avoid false positives. + if _, ok := node.(*ast.FuncLit); ok { + return false + } + + // Look for assignments like: file, err := os.Open(...) + if assign, ok := node.(*ast.AssignStmt); ok { + for i, rhs := range assign.Rhs { + if call, ok := rhs.(*ast.CallExpr); ok && isFileOpenCall(call) { + if i < len(assign.Lhs) { + if ident, ok := assign.Lhs[i].(*ast.Ident); ok && ident.Name != "_" { + obj := pass.TypesInfo.ObjectOf(ident) + if obj != nil { + // If this object was already tracked from a prior open on the + // same binding (plain = reassignment), report any unresolved + // violation immediately before overwriting the state. + if prev, exists := fileVars[obj]; exists && prev.hasManualClose && !prev.hasDefer { + pass.Report(analysis.Diagnostic{ + Pos: prev.openPos, + Message: "file Close() should be deferred immediately after successful open to prevent resource leaks", + }) + } + fileVars[obj] = &fileVarState{ + openPos: call.Pos(), + } + } + } + } + } + } + } + + // Look for defer file.Close() + if deferStmt, ok := node.(*ast.DeferStmt); ok { + if obj := getCloseCallObj(pass, deferStmt.Call); obj != nil { + if state, found := fileVars[obj]; found { + state.hasDefer = true + } + } + } + + // Look for non-deferred file.Close() in expression statements + if exprStmt, ok := node.(*ast.ExprStmt); ok { + if call, ok := exprStmt.X.(*ast.CallExpr); ok { + if obj := getCloseCallObj(pass, call); obj != nil { + if state, found := fileVars[obj]; found { + state.hasManualClose = true + } + } + } + } + + // Look for non-deferred file.Close() in assignments (e.g., closeErr := fd.Close()) + if assign, ok := node.(*ast.AssignStmt); ok { + for _, rhs := range assign.Rhs { + if call, ok := rhs.(*ast.CallExpr); ok { + if obj := getCloseCallObj(pass, call); obj != nil { + if state, found := fileVars[obj]; found { + state.hasManualClose = true + } + } + } + } + } + + return true + }) + + // Report files with manual close but no defer + for _, state := range fileVars { + if state.hasManualClose && !state.hasDefer { + pass.Report(analysis.Diagnostic{ + Pos: state.openPos, + Message: "file Close() should be deferred immediately after successful open to prevent resource leaks", + }) + } + } + }) + + return nil, nil +} + +type fileVarState struct { + openPos token.Pos + hasDefer bool + hasManualClose bool +} + +// isFileOpenCall returns true if the call is os.Open, os.Create, or os.OpenFile +func isFileOpenCall(call *ast.CallExpr) bool { + sel, ok := call.Fun.(*ast.SelectorExpr) + if !ok { + return false + } + ident, ok := sel.X.(*ast.Ident) + if !ok || ident.Name != "os" { + return false + } + return sel.Sel.Name == "Open" || sel.Sel.Name == "Create" || sel.Sel.Name == "OpenFile" +} + +// getCloseCallObj returns the types.Object for the receiver if call is like file.Close(), +// enabling correct identification across variable shadowing. +func getCloseCallObj(pass *analysis.Pass, call *ast.CallExpr) types.Object { + sel, ok := call.Fun.(*ast.SelectorExpr) + if !ok || sel.Sel.Name != "Close" { + return nil + } + ident, ok := sel.X.(*ast.Ident) + if !ok { + return nil + } + return pass.TypesInfo.ObjectOf(ident) +} diff --git a/pkg/linters/fileclosenotdeferred/fileclosenotdeferred_test.go b/pkg/linters/fileclosenotdeferred/fileclosenotdeferred_test.go new file mode 100644 index 00000000000..428338dd3ec --- /dev/null +++ b/pkg/linters/fileclosenotdeferred/fileclosenotdeferred_test.go @@ -0,0 +1,16 @@ +//go:build !integration + +package fileclosenotdeferred_test + +import ( + "testing" + + "golang.org/x/tools/go/analysis/analysistest" + + "github.com/github/gh-aw/pkg/linters/fileclosenotdeferred" +) + +func TestFileCloseNotDeferred(t *testing.T) { + testdata := analysistest.TestData() + analysistest.Run(t, testdata, fileclosenotdeferred.Analyzer, "fileclosenotdeferred") +} diff --git a/pkg/linters/fileclosenotdeferred/testdata/src/fileclosenotdeferred/fileclosenotdeferred.go b/pkg/linters/fileclosenotdeferred/testdata/src/fileclosenotdeferred/fileclosenotdeferred.go new file mode 100644 index 00000000000..c07e5ca221a --- /dev/null +++ b/pkg/linters/fileclosenotdeferred/testdata/src/fileclosenotdeferred/fileclosenotdeferred.go @@ -0,0 +1,104 @@ +package fileclosenotdeferred + +import "os" + +// flagged: file.Close() not deferred +func ReadFileManualClose() error { + file, err := os.Open("test.txt") // want `file Close\(\) should be deferred immediately after successful open to prevent resource leaks` + if err != nil { + return err + } + // ... code that might return early ... + file.Close() + return nil +} + +// not flagged: defer used correctly +func ReadFileDeferClose() error { + file, err := os.Open("test.txt") + if err != nil { + return err + } + defer file.Close() + // ... rest of code ... + return nil +} + +// flagged: os.Create with manual close +func CreateFileManualClose() error { + f, err := os.Create("output.txt") // want `file Close\(\) should be deferred immediately after successful open to prevent resource leaks` + if err != nil { + return err + } + f.Close() + return nil +} + +// flagged: os.Open with Close() assigned to error variable +func ReadFileCloseWithErrAssign() error { + file, err := os.Open("test.txt") // want `file Close\(\) should be deferred immediately after successful open to prevent resource leaks` + if err != nil { + return err + } + // ... code ... + closeErr := file.Close() + if closeErr != nil { + return closeErr + } + return nil +} + +// not flagged: blank identifier +func IgnoredFile() error { + _, err := os.Open("test.txt") + return err +} + +// not flagged: os.Open inside a closure — the closure is a separate execution +// context and must not be attributed to the outer function's fileVars. +func OpenInClosure() { + fn := func() error { + f, err := os.Open("test.txt") + if err != nil { + return err + } + defer f.Close() + return nil + } + _ = fn() +} + +// not flagged: inner variable named 'f' shadows outer 'f'; the inner open +// is deferred, so neither binding triggers the diagnostic. +func ShadowedVar() error { + f, err := os.Open("outer.txt") + if err != nil { + return err + } + defer f.Close() + if true { + f, err := os.Open("inner.txt") // shadows outer f + if err != nil { + return err + } + defer f.Close() + _ = f + } + return nil +} + +// flagged: same variable reassigned via plain = after manual close; the first +// open's violation must not be hidden by the second open's defer. +func ReopenWithManualCloseThenDefer() error { + f, err := os.Open("first.txt") // want `file Close\(\) should be deferred immediately after successful open to prevent resource leaks` + if err != nil { + return err + } + f.Close() // manual close — violation for first open + f, err = os.Open("second.txt") + if err != nil { + return err + } + defer f.Close() // deferred — ok for second open + return nil +}