You signed in with another tab or window. Reload to refresh your session.You signed out in another tab or window. Reload to refresh your session.You switched accounts on another tab or window. Reload to refresh your session.Dismiss alert
# ADR-38742: Enforce File-Path Constant Usage via a Custom Linter
2
+
3
+
**Date**: 2026-06-12
4
+
**Status**: Draft
5
+
6
+
## Context
7
+
8
+
The codebase repeatedly embeds filesystem path string literals (e.g. `/tmp/gh-aw/awf-config.json`, `${RUNNER_TEMP}/...`, `.github/...`) inline across many files. When the same path is duplicated as a literal in several places, a change to that path requires hunting down every copy, and drift between copies causes subtle, hard-to-diagnose bugs — especially when paths appear in log/print output. The project already maintains a suite of custom `go/analysis` analyzers under `pkg/linters/` (24 existing analyzers wired into a multichecker binary), and has previously chosen the custom-linter approach to enforce conventions mechanically rather than by code-review convention (see [ADR-38704](38704-enforce-context-aware-sleep-via-custom-linter.md)). This PR addresses the path-literal duplication problem within that established pattern.
9
+
10
+
## Decision
11
+
12
+
We will enforce file-path constant usage with a new custom `go/analysis` analyzer, `hardcodedfilepath`, added to `pkg/linters/` and wired into the multichecker binary (`cmd/linters/main.go`). The analyzer flags string literals that look like filesystem paths (recognized prefix + minimum length), and either suggests an existing named path constant when the literal matches one (scanning the current package and imported `*constants*` packages), or recommends extracting a new named constant when no match exists. It additionally annotates findings that appear inside log/print calls, skips const-declaration sites and format-template strings, and honours `//nolint:hardcodedfilepath` suppression. We chose this approach because it reuses the existing linter infrastructure and CI enforcement, making the convention self-checking rather than dependent on reviewer vigilance.
13
+
14
+
## Alternatives Considered
15
+
16
+
### Alternative 1: Rely on code review and documentation
17
+
Document the "use a named path constant" convention and enforce it during PR review. Rejected because it does not scale — reviewers miss inline literals, and the existing duplication shows convention alone has not prevented the problem. It also provides no actionable, located diagnostics.
18
+
19
+
### Alternative 2: Use an existing general-purpose linter (e.g. goconst, golangci-lint rules)
20
+
`goconst` detects repeated string literals generally. Rejected because it cannot reason about path-specific heuristics (path prefixes, `${RUNNER_TEMP}` templating, log/print correlation) or cross-reference values against exported constants in dedicated `*constants*` packages, and it would produce noise on non-path strings. The project's custom-analyzer pattern allows precise, domain-specific diagnostics.
21
+
22
+
## Consequences
23
+
24
+
### Positive
25
+
- Path-literal duplication is caught mechanically in CI with located, actionable diagnostics that name the constant to use.
26
+
- Reinforces a single source of truth for shared paths, reducing drift-related bugs.
27
+
- Consistent with the established `pkg/linters/` pattern, so contributors already know how to read, suppress, and extend it.
28
+
29
+
### Negative
30
+
- Heuristic detection (prefix list + length threshold) will produce false positives/negatives; the prefix list and `minPathRuneLen` must be maintained as path conventions evolve.
31
+
- Adds a 25th analyzer to the multichecker, marginally increasing lint runtime and the maintenance surface.
32
+
- The constant-matching logic depends on the `*constants*` package-path naming convention; constants defined elsewhere will not be recognized as suggestions.
33
+
34
+
### Neutral
35
+
- Suppression is available via `//nolint:hardcodedfilepath` for intentional inline literals.
36
+
- Test files are skipped, so test fixtures may continue to use inline path literals.
37
+
38
+
---
39
+
40
+
*This is a DRAFT ADR generated by the [Design Decision Gate](https://github.com/github/gh-aw/actions/runs/27386237174) workflow. The PR author must review, complete, and finalize this document before the PR can merge.*
Copy file name to clipboardExpand all lines: pkg/linters/README.md
+5Lines changed: 5 additions & 0 deletions
Display the source diff
Display the rich diff
Original file line number
Diff line number
Diff line change
@@ -15,6 +15,7 @@ This package currently provides custom Go analyzers in the following subpackages
15
15
-`execcommandwithoutcontext` — reports `exec.Command(...)` calls inside functions that already receive `context.Context` and should use `exec.CommandContext(...)`.
16
16
-`fmterrorfnoverbs` — reports `fmt.Errorf` calls whose format string contains no verbs, recommending `errors.New` instead.
17
17
-`fprintlnsprintf` — reports `fmt.Fprintln(..., fmt.Sprintf(...))` patterns and recommends direct formatting calls.
18
+
-`hardcodedfilepath` — reports hard-coded file path string literals that match known path constants or should be extracted into named constants; also annotates paths that appear in log/print calls.
18
19
-`jsonmarshalignoredeerror` — reports `json.Marshal` and `json.Unmarshal` calls where the error return is discarded with `_`.
19
20
-`largefunc` — reports function bodies that exceed a configurable line-count threshold.
20
21
-`lenstringzero` — reports `len(s) == 0` / `len(s) != 0` comparisons on string values that should use `s == ""` / `s != ""`.
@@ -47,6 +48,7 @@ This package currently provides custom Go analyzers in the following subpackages
47
48
|`fileclosenotdeferred`| Custom `go/analysis` analyzer that flags file `Close()` calls that are not deferred immediately |
48
49
|`fmterrorfnoverbs`| Custom `go/analysis` analyzer that flags `fmt.Errorf` calls with no format verbs, recommending `errors.New`|
49
50
|`fprintlnsprintf`| Custom `go/analysis` analyzer that flags `fmt.Fprintln(..., fmt.Sprintf(...))` patterns |
51
+
|`hardcodedfilepath`| Custom `go/analysis` analyzer that flags hard-coded file path string literals that match known path constants or should be extracted as named constants; annotates paths in log/print calls |
50
52
|`jsonmarshalignoredeerror`| Custom `go/analysis` analyzer that flags `json.Marshal`/`json.Unmarshal` calls where the error return is discarded with `_`|
51
53
|`largefunc`| Custom `go/analysis` analyzer that flags large functions with actionable diagnostics |
52
54
|`lenstringzero`| Custom `go/analysis` analyzer that flags `len(s) == 0` / `len(s) != 0` on string values that should use `s == ""` / `s != ""`|
0 commit comments