Skip to content

Add isolint custom linter and documentation for both linters#197

Open
yanyi-wego wants to merge 7 commits intomainfrom
custom-linter-currency-site-package
Open

Add isolint custom linter and documentation for both linters#197
yanyi-wego wants to merge 7 commits intomainfrom
custom-linter-currency-site-package

Conversation

@yanyi-wego
Copy link
Contributor

@yanyi-wego yanyi-wego commented Mar 3, 2026

Summary

  • Adds isolint, a new golangci-lint module plugin that flags raw uppercase ISO code string literals ("USD", "SG") and recommends typed constants from currency and site packages, with auto-fix support
  • Adds AGENTS.md and docs/ documentation to both isolint and stringlint covering architecture, testing patterns, and design decisions
  • Adds revive linter integration with Makefiles for both linter packages

Approach

isolint follows the same architecture as stringlintgo/analysis analyzer with inspector.WithStack filtered traversal, golangci-lint module plugin registration, and analysistest-based tests with .go.golden files for suggested fix verification.

Key design choices:

  • LoadModeSyntax — only needs string literal values, no type-checking required. Cheapest load mode; avoids forcing type-checking on other linters in the golangci-lint batch
  • Delegates validation to source packagescurrency.IsISO4217() and site.Currency() instead of hardcoded maps
  • Fast-path filtering — byte-length guard (len(lit.Value) != 4 && != 5) eliminates most string literals before strconv.Unquote allocates
  • Uppercase only — only flags "USD", "SG" etc.; lowercase ("usd", "sg") and mixed case ("Usd") are intentionally skipped to avoid false positives on English words and identifiers like "id", "to", "no"
  • Context-aware skips — string arguments to ORM methods (Select, Pluck, Omit), HTTP methods (Query, Param, FormValue), and filter methods (Equals, NotEquals) are skipped as they represent column/parameter names, not ISO code values
  • Nil pass.Pkg guard — golangci-lint may leave pass.Pkg nil under LoadModeSyntax; the analyzer tolerates this safely

Documentation for both linters was restructured into AGENTS.md (quick reference) and docs/ (detailed decisions and testing guides).

Testing

  • analysistest.Run + RunWithSuggestedFixes with positive tests (// want annotations), negative tests (valid.go, valid_contexts.go), and golden files
  • Unit tests and benchmarks for IsCurrencyCode / IsSiteCode in codes_test.go
  • Dedicated nil_pkg_test.go for the nil pass.Pkg edge case

yanyi-wego and others added 7 commits March 3, 2026 10:44
Raw ISO code strings like "USD" or "SG" scattered across the
codebase are fragile — typos compile fine and silently break
runtime behavior. This linter flags them at build time and
suggests the typed constants from currency and site packages,
with auto-fix support via SuggestedFix text edits.

Co-Authored-By: Claude Opus 4.6 <noreply@anthropic.com>
The stringlint linter shipped without agent-facing docs.
This adds testing guidance (analysistest bidirectional
contract, positive/negative test patterns) and performance
notes explaining why LoadModeTypesInfo is required and what
the linter already does well to stay fast within that
constraint.

Co-Authored-By: Claude Opus 4.6 <noreply@anthropic.com>
golangci-lint may leave pass.Pkg unset when running analyzers
under LoadModeSyntax, since the load mode is a hint to the
runner — not a guarantee that Pkg is populated. analysistest
always uses the full go/packages loader with type-checking,
so pass.Pkg is always non-nil in tests — making this panic
invisible to the test suite and only surfacing in production
golangci-lint runs.

Co-Authored-By: Claude Opus 4.6 <noreply@anthropic.com>
The CI pre-push hook runs revive alongside go vet, but
AGENTS.md did not document revive as a command. This meant
agents (and contributors) could write code that passes
go vet and tests but fails the pre-push lint gate. Adding
the command to both linter AGENTS.md files closes that gap.

Co-Authored-By: Claude Opus 4.6 <noreply@anthropic.com>
Not all environments have standalone revive installed.
golangci-lint is already a dependency for running the
custom module plugins, so providing it as an alternative
ensures linting works without extra tool installation.

Co-Authored-By: Claude Opus 4.6 <noreply@anthropic.com>
Move testing guide and design rationale into
docs/decisions.md and docs/testing.md. AGENTS.md
keeps bullets and links for fast context loading;
docs/ holds the why for progressive disclosure.

Co-Authored-By: Claude Opus 4.6 <noreply@anthropic.com>
Restrict detection to uppercase-only ISO literals — lowercase and
mixed case are no longer flagged, eliminating false positives on
common English words ("id", "to", "no", "do").

Add context-aware skipping via inspector.WithStack: import paths
and arguments to ORM/HTTP/filter methods (skipMethods) are now
excluded. Split AGENTS.md rationale into docs/ for progressive
disclosure.

Co-Authored-By: Claude Opus 4.6 <noreply@anthropic.com>
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment

Labels

None yet

Projects

None yet

Development

Successfully merging this pull request may close these issues.

2 participants