-
-
Notifications
You must be signed in to change notification settings - Fork 1.5k
Add noinlineerr linter #5826
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.
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
Add noinlineerr linter #5826
Conversation
Hey, thank you for opening your first Pull Request ! |
In order for a pull request adding a linter to be reviewed, the linter and the PR must follow some requirements.
Pull Request Description
Linter
The Linter Tests Inside Golangci-lint
|
@ldez thank you for checklist. Fixed:
Quick question:
I thought they already do – did I miss something? Could you clarify what exactly should be renamed or matched? |
I detected at least one false-positive: func _() {
if ok, _ := strconv.ParseBool("1"); ok {
fmt.Println("ok")
}
} The linter is very limited: it only handles explicit |
Even if I don't like inlined I don't know what should exactly be implemented, but I think the topic needs to be approached from a broader perspective, maybe with options and more. |
@ldez Thanks for the feedback! Do you have any specific ideas in mind? Some thoughts I had: Adding an option to choose the preferred style (though I’m not sure if it makes sense to only prefer the inline variant). Adding autofix support — though that’s a bit more complex for me at the moment and would need some digging in. Happy to hear any other suggestions you might have! |
@ldez Also, just to clarify — did you mean adding an option like “Check for blank identifiers” (e.g. |
Hey! 👋 Thanks again for the feedback — I’ve addressed most of the points in the latest update. Here’s what’s changed: Changes since last review:Proper type check for error: Auto-fix support: Avoids false positives: Question: Options?Right now the linter enforces a strict rule (any inline Possible ideas:
Let me know if you think this is worth exploring, or if the linter should stay strict and opinionated. |
The proposed options are not what I expected.
I'm talking about a broader perspective of inlined |
About the broader perspective — I get your point, but I’d prefer to keep the scope limited to I’ve often seen inline error handling being overused or misused, whereas inline Init with other values is way less common — That said, building a separate linter to cover all inline |
6c44991
to
ae87644
Compare
ok, so we agree that if a new linter is added that handles the whole topic of inlined If we agree on that, we can accept this linter. |
Totally agree |
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
LGTM
@ldez fyi it looks like this linter is not listed in https://golangci-lint.run/usage/linters/ |
@G-Rath refresh the page: https://golangci-lint.run/usage/linters/#noinlineerr |
* build(deps): bump github.com/go-critic/go-critic from 0.12.0 to 0.13.0 (golangci#5579) * build(deps): bump mvdan.cc/unparam to HEAD (golangci#5584) * build(deps): bump github.com/timakin/bodyclose from ed6a65f985e3 to 1db5c5ca4d67 (golangci#5585) * docs: improve Docker example with cache reuse (golangci#5586) * docs: add a migration guide for VSCode integration (golangci#5587) * build(deps): bump github.com/ckaznocha/intrange from 0.3.0 to 0.3.1 (golangci#5589) * feat: add option stdin for fmt command (golangci#5588) * build(deps): bump github.com/daixiang0/gci from 0.13.5 to 0.13.6 (golangci#5592) * fix: funlen ignore-comments (golangci#5594) * docs: cleaning * chore: prepare release * docs: update GitHub Action assets (golangci#5595) * docs: fix displaying GoLand logo on a dark theme (golangci#5598) * fix: validate version before configuration (golangci#5599) * fix: copy golines settings during linter settings load (golangci#5607) * fix: forbidigo migration (golangci#5606) * docs: add example of the field version (golangci#5609) * chore: prepare release * docs: update GitHub Action assets (golangci#5611) Co-authored-by: Fernandez Ludovic <[email protected]> * docs: adaptive stargazers image (golangci#5613) * fix: enable formatters with flags (golangci#5620) * docs: add section about flags migration (golangci#5621) * docs: replace 'disable-all' with 'default: none' (golangci#5622) * docs: update reference comment for errchkjson (golangci#5623) * fix: formatter validation message (golangci#5624) * docs: info about the logo * docs: fix flags example * docs: fix flags example * docs: clean old configuration examples (golangci#5626) * fix: use absolute filepath inside base rule source (golangci#5629) * chore: prepare release * docs: update GitHub Action assets (golangci#5634) * docs: add version field to configuration sample (golangci#5632) * docs: remove golint in nolint example (golangci#5635) * build(deps): bump github.com/kunwardeep/paralleltest from 1.0.10 to 1.0.13 (golangci#5636) * docs: fix config reference for formatters (golangci#5638) * docs: improve version documentation (golangci#5639) * docs: fix settings examples (golangci#5643) * build(deps): bump tar-fs and puppeteer in /docs (golangci#5653) * build(deps): bump github.com/ghostiam/protogetter from 0.3.12 to 0.3.13 (golangci#5658) Co-authored-by: Fernandez Ludovic <[email protected]> * build(deps): bump github.com/Antonboom/testifylint from 1.6.0 to 1.6.1 (golangci#5654) Co-authored-by: Fernandez Ludovic <[email protected]> * docs: cleanup config reference (golangci#5659) * dev: rewrite and simplify thanks page generation (golangci#5662) * docs: remove tenv linter settings (golangci#5665) * docs: improve default exclusions render (golangci#5661) * docs: fix `go get -tool` command (golangci#5669) * fix: gocritic importshadow checker (golangci#5673) * build(deps): bump the linter-testdata group across 2 directories with 4 updates (golangci#5676) Co-authored-by: Fernandez Ludovic <[email protected]> * docs: update demo (golangci#5677) * build(deps): bump github.com/alexkohler/nakedret/v2 from 2.0.5 to 2.0.6 (golangci#5681) Co-authored-by: Fernandez Ludovic <[email protected]> * build(deps): bump github.com/shirou/gopsutil/v4 from 4.25.2 to 4.25.3 (golangci#5680) Co-authored-by: Fernandez Ludovic <[email protected]> * docs: add link to issues related to some integrations (golangci#5682) * build(deps): bump github.com/Crocmagnon/fatcontext from 0.7.1 to 0.7.2 (golangci#5685) * feat: add config path placeholder (golangci#5650) * feat: add an option to display absolute paths (golangci#5651) * feat: colored diff for fmt command (golangci#5652) * build(deps): bump github.com/tomarrell/wrapcheck/v2 from 2.10.0 to 2.11.0 (golangci#5656) Co-authored-by: Fernandez Ludovic <[email protected]> * build(deps): bump github.com/kunwardeep/paralleltest from 1.0.13 to 1.0.14 (golangci#5657) Co-authored-by: Fernandez Ludovic <[email protected]> * build(deps): bump github.com/mgechev/revive from 1.7.0 to 1.8.0 (golangci#5663) Co-authored-by: Fernandez Ludovic <[email protected]> * build(deps): bump github.com/polyfloyd/go-errorlint from 1.7.1 to 1.8.0 (golangci#5686) * feat: add warn-unused option for fmt command (golangci#5668) * build(deps): bump github.com/firefart/nonamedreturns from 1.0.5 to 1.0.6 (golangci#5687) * build(deps): bump github.com/bombsimon/wsl/v4 from 4.6.0 to 4.7.0 (golangci#5689) * build(deps): bump github.com/alecthomas/chroma/v2 from 2.15.0 to 2.16.0 (golangci#5690) * build(deps): bump go-simpler.org/sloglint from 0.9.0 to 0.10.0 (golangci#5688) Co-authored-by: Fernandez Ludovic <[email protected]> * build(deps): bump dependencies in the linter-testdata group (golangci#5691) Co-authored-by: Fernandez Ludovic <[email protected]> * docs: add `redundant-test-main-exit` to `revive` rules (golangci#5692) Co-authored-by: Fernandez Ludovic <[email protected]> * build(deps): bump github.com/alingse/nilnesserr from 0.1.2 to 0.2.0 (golangci#5693) * build(deps): bump github.com/securego/gosec/v2 from 2.22.2 to 2.22.3 (golangci#5694) Co-authored-by: Fernandez Ludovic <[email protected]> * fix: memory leaks when using go1.(N) with golangci-lint built with with go1.(N-1) (golangci#5695) * build(deps): bump go-simpler.org/sloglint from 0.10.0 to 0.10.1 (golangci#5696) * docs: explicitly describe that the `migrate` command automatically migrate `linters.presets` (golangci#5697) Co-authored-by: Fernandez Ludovic <[email protected]> * build(deps): bump golang.org/x/sys from 0.31.0 to 0.32.0 (golangci#5699) * build(deps): bump go-simpler.org/sloglint from 0.10.1 to 0.11.0 (golangci#5698) Co-authored-by: Fernandez Ludovic <[email protected]> * build(deps): bump golang.org/x/oauth2 from 0.28.0 to 0.29.0 in /scripts/gen_github_action_config in the scripts group (golangci#5704) * feat: add golangci-lint-fmt pre-commit hook (golangci#5705) * build(deps): bump golang.org/x/tools from 0.31.0 to 0.32.0 (golangci#5708) * build(deps): bump github.com/butuzov/ireturn from 0.3.1 to 0.4.0 (golangci#5710) * build(deps): bump github.com/pelletier/go-toml/v2 from 2.2.3 to 2.2.4 (golangci#5711) * docs: update section about vscode integration (golangci#5706) Co-authored-by: logica0419 <[email protected]> * build(deps): bump github.com/jgautheron/goconst from 1.7.1 to 1.8.1 (golangci#5712) Co-authored-by: Fernandez Ludovic <[email protected]> * build(deps): bump github.com/golangci/unconvert to HEAD (golangci#5713) * build(deps): bump github.com/timonwong/loggercheck from 0.10.1 to 0.11.0 (golangci#5715) * govet: add `httpmux` analyzer (golangci#5717) * dev: update JSON schema * build(deps): bump github.com/mgechev/revive from 1.8.0 to 1.9.0 (golangci#5721) * Add funcorder linter (golangci#5630) * chore: prepare release * docs: update GitHub Action assets (golangci#5723) * docs: update documentation assets (golangci#5722) Co-authored-by: Fernandez Ludovic <[email protected]> * chore: downgrade goreleaser to v2.8.1 Related to a regression inside v2.8.2 * chore: prepare release * docs: update changelog * docs: update GitHub Action assets (golangci#5724) * build(deps): bump github.com/ghostiam/protogetter from 0.3.13 to 0.3.14 (golangci#5727) * build(deps): bump mvdan.cc/gofumpt from 0.7.0 to 0.8.0 (golangci#5728) * build(deps): bump github.com/ldez/exptostd from 0.4.2 to 0.4.3 (golangci#5730) * build(deps): bump github.com/ldez/usetesting from 0.4.2 to 0.4.3 (golangci#5729) * dev: simplify mnd implementation (golangci#5731) * build(deps): bump github.com/ghostiam/protogetter from 0.3.14 to 0.3.15 (golangci#5732) * docs: fix default value of linters.exclusions.generated (golangci#5735) * chore: prepare release * docs: update GitHub Action assets (golangci#5738) * docs: update section about GoLand integration (golangci#5740) * fix: order of staticcheck settings during migration (golangci#5741) Co-authored-by: Fernandez Ludovic <[email protected]> * fix: add go.mod hash to the cache salt (golangci#5739) * dev: disable check-generated job (golangci#5742) * fix: related information position (golangci#5746) * docs: GoLand IDE only support v1 for now (golangci#5750) * fix: convert uint as pointer of uint for the migration (golangci#5755) * build(deps): bump go.augendre.info/fatcontext from 0.7.2 to 0.8.0 (golangci#5757) * docs: GoLand IDE support v2 (golangci#5758) * chore: prepare release * docs: update GitHub Action assets (golangci#5762) * chore: prepare release (golangci#5763) * docs: update GitHub Action assets (golangci#5764) * chore: prepare release (golangci#5765) * docs: update GitHub Action assets (golangci#5766) * docs: add note about golangci-lint v2 integration in VS Code (golangci#5768) * build(deps): bump go-simpler.org/musttag from 0.13.0 to 0.13.1 (golangci#5769) * build(deps): bump github.com/alecthomas/chroma/v2 from 2.16.0 to 2.17.0 (golangci#5772) * build(deps): bump github.com/tetafro/godot from 1.5.0 to 1.5.1 (golangci#5770) Co-authored-by: Fernandez Ludovic <[email protected]> * build(deps): bump base-x from 3.0.9 to 3.0.11 in /docs (golangci#5776) * build(deps): bump the linter-testdata group across 2 directories with 2 updates (golangci#5777) * build(deps): bump github.com/shirou/gopsutil/v4 from 4.25.3 to 4.25.4 (golangci#5778) * build(deps): bump github.com/alecthomas/chroma/v2 from 2.17.0 to 2.17.2 (golangci#5779) * chore: prepare release * docs: update GitHub Action assets * build(deps): bump github.com/manuelarte/funcorder from 0.2.1 to 0.3.0 (golangci#5743) Co-authored-by: Fernandez Ludovic <[email protected]> * build(deps): bump github.com/sonatard/noctx from 0.1.0 to 0.3.3 (golangci#5771) Co-authored-by: Fernandez Ludovic <[email protected]> * build(deps): bump golangci/golangci-lint-action from 7.0.0 to 8.0.0 in the github-actions group (golangci#5780) * build(deps): bump golang.org/x/oauth2 from 0.29.0 to 0.30.0 in /scripts/gen_github_action_config in the scripts group (golangci#5781) * build(deps): bump github.com/jjti/go-spancheck from 0.6.4 to 0.6.5 (golangci#5784) * build(deps): bump golang.org/x/sys from 0.32.0 to 0.33.0 (golangci#5785) * build(deps): bump golang.org/x/tools from 0.32.0 to 0.33.0 (golangci#5786) Co-authored-by: Fernandez Ludovic <[email protected]> * build(deps): bump github.com/ashanbrown/makezero from 1.2.0 to 2.0.1 (golangci#5782) * build(deps): bump github.com/ashanbrown/forbidigo from 1.6.0 to 2.1.0 (golangci#5783) * build(deps): bump github.com/securego/gosec/v2 from 2.22.3 to 2.22.4 (golangci#5788) * build(deps): bump github.com/manuelarte/funcorder from 0.3.0 to 0.5.0 (golangci#5792) * Add swaggo/swag formatter (golangci#5749) Co-authored-by: Fernandez Ludovic <[email protected]> * feat: add arangolint linter (golangci#5718) Co-authored-by: Fernandez Ludovic <[email protected]> * feat: add embeddedstructfieldcheck linter (golangci#5761) Co-authored-by: Fernandez Ludovic <[email protected]> * fix: exclusions path-expect (golangci#5798) * dev: simplify linter constructors (golangci#5796) * dev: remove AUR publishing (golangci#5799) * dev: restore aur-bin (golangci#5803) * chore: remove dead code (golangci#5801) * errcheck: add verbose option (golangci#5802) * docs: fix revive defer rule configuration (golangci#5804) Co-authored-by: Fernandez Ludovic <[email protected]> * build(deps): bump github.com/Abirdcfly/dupword from 0.1.3 to 0.1.4 (golangci#5809) * build(deps): bump github.com/uudashr/iface from 1.3.1 to 1.3.2 (golangci#5810) * build(deps): bump github.com/alecthomas/chroma/v2 from 2.17.2 to 2.18.0 (golangci#5812) * build(deps): bump github.com/manuelarte/embeddedstructfieldcheck from 0.2.1 to 0.3.0 (golangci#5811) Co-authored-by: Fernandez Ludovic <[email protected]> * build(deps): bump github.com/golangci/misspell from 0.6.0 to 0.7.0 (golangci#5813) * build(deps): bump the linter-testdata group across 2 directories with 2 updates (golangci#5814) Co-authored-by: Fernandez Ludovic <[email protected]> * dev: consistent version (golangci#5817) * docs: add description for linters.default sets (golangci#5818) * build(deps): bump github.com/uudashr/iface from 1.3.2 to 1.4.0 (golangci#5820) Co-authored-by: Fernandez Ludovic <[email protected]> * docs: improve typecheck FAQ (golangci#5821) Co-authored-by: Simon Sawert <[email protected]> * docs: note about plugin system (golangci#5822) * build(deps): bump github.com/jgautheron/goconst from 1.8.1 to 1.8.2 (golangci#5825) * fix: write the input to stdout when using stdin and there are no changes (golangci#5827) * build(deps): bump github.com/santhosh-tekuri/jsonschema/v6 from 6.0.1 to 6.0.2 (golangci#5829) * build(deps): bump github.com/sashamelentyev/usestdlibvars from 1.28.0 to 1.29.0 (golangci#5828) Co-authored-by: Fernandez Ludovic <[email protected]> * fix: error message when trying to migrate a migrated config (golangci#5836) Co-authored-by: Fernandez Ludovic <[email protected]> * fix: formatters CLI flags help message (golangci#5835) * build(deps): bump github.com/golangci/plugin-module-register from 0.1.1 to 0.1.2 (golangci#5838) * build(deps): bump github.com/mgechev/revive from 1.9.0 to 1.10.0 (golangci#5837) Co-authored-by: Fernandez Ludovic <[email protected]> * build(deps): bump github.com/Abirdcfly/dupword from 0.1.4 to 0.1.5 (golangci#5839) * chore: improve pull request template * build(deps): bump github.com/Abirdcfly/dupword from 0.1.5 to 0.1.6 (golangci#5841) * Add noinlineerr linter (golangci#5826) * dev: some simplifications (golangci#5843) * dev: improve comments (golangci#5846) * build(deps): bump github.com/shirou/gopsutil/v4 from 4.25.4 to 4.25.5 (golangci#5849) * build(deps): bump the linter-testdata group across 4 directories with 4 updates (golangci#5850) * docs: improve module plugin explanation (golangci#5851) * dev: display list of staticcheck checks in debug (golangci#5853) * dev: correct debug log for default rules in revive (golangci#5852) * dev: use slice of pointers (golangci#5854) * dev: handle linter major versions (golangci#5848) * feat: deprecate print-resources-usage flag (golangci#5860) * docs: add a note about `GL_DEBUG=staticcheck` (golangci#5861) * fix: deduplicate typecheck errors (golangci#5864) * build(deps): bump golang.org/x/mod from 0.24.0 to 0.25.0 (golangci#5868) * build(deps): bump golang.org/x/tools from 0.33.0 to 0.34.0 (golangci#5867) * build(deps): bump github.com/ldez/gomoddirectives from 0.6.1 to 0.7.0 (golangci#5869) * dev: log level colors (golangci#5866) * docs: update gomoddirectives configuration * build(deps): bump github.com/ldez/exptostd from 0.4.3 to 0.4.4 (golangci#5876) * build(deps): bump github.com/ldez/usetesting from 0.4.3 to 0.5.0 (golangci#5877) Co-authored-by: Fernandez Ludovic <[email protected]> * build(deps): bump brace-expansion from 1.1.11 to 1.1.12 in /docs (golangci#5878) * build(deps): bump github.com/securego/gosec/v2 from 2.22.4 to 2.22.5 (golangci#5880) * fix: typecheck memory leak (golangci#5884) * dev: remove useless condition * docs: update migration guide with --print-issued-lines (golangci#5886) * fix: stop the analysis after the first package analysis error (golangci#5885) * build(deps): bump github.com/go-viper/mapstructure/v2 from 2.2.1 to 2.3.0 (golangci#5888) * build(deps): bump brace-expansion from 1.1.11 to 1.1.12 in /.github/peril (golangci#5889) * fix: formats consistently the code with gci (golangci#5893) * build(deps): bump github.com/sonatard/noctx from 0.3.3 to 0.3.4 (golangci#5895) Co-authored-by: Fernandez Ludovic <[email protected]> * fix: unique version per custom build (golangci#5896) * build(deps): bump github.com/bombsimon/wsl/v5 from 4.7.0 to 5.0.0 (golangci#5900) Co-authored-by: Fernandez Ludovic <[email protected]> * dev: add suggested migration for linter configuration (golangci#5902) * chore: prepare release * docs: update documentation assets (golangci#5903) Co-authored-by: Fernandez Ludovic <[email protected]> * varnamelen: fix configuration (golangci#5904) * chore: prepare release * docs: update GitHub Action assets * dev: fix GitHub Action assets generation (golangci#5906) * build(deps): bump the linter-testdata group across 2 directories with 3 updates (golangci#5908) * docs: add the command to check the Go version used to build (golangci#5913) * build(deps): bump github.com/alecthomas/chroma/v2 from 2.18.0 to 2.19.0 (golangci#5914) * build(deps): bump github.com/shirou/gopsutil/v4 from 4.25.5 to 4.25.6 (golangci#5918) * godot: add noinline value into the JSONSchema (golangci#5922) * docs: fix tutorial URL (golangci#5925) * build(deps): bump golang.org/x/mod from 0.25.0 to 0.26.0 (golangci#5927) * build(deps): bump github.com/AlwxSin/noinlineerr from 1.0.3 to 1.0.4 (golangci#5928) * build(deps): bump golang.org/x/sys from 0.33.0 to 0.34.0 (golangci#5931) * docs: improve debug keys documentation (golangci#5930) * fix: panic: close of closed channel (golangci#5929) * chore: prepare release * docs: update GitHub Action assets * build(deps): bump github.com/uudashr/iface from 1.4.0 to 1.4.1 (golangci#5915) * build(deps): bump github.com/sonatard/noctx from 0.3.4 to 0.3.5 (golangci#5916) Co-authored-by: Fernandez Ludovic <[email protected]> * build(deps): bump github.com/bombsimon/wsl/v5 from 5.0.0 to 5.1.0 (golangci#5917) * build(deps): bump github.com/nunnatsa/ginkgolinter from 0.19.1 to 0.20.0 (golangci#5932) Co-authored-by: Fernandez Ludovic <[email protected]> * build(deps): bump golang.org/x/sync from 0.15.0 to 0.16.0 (golangci#5934) * build(deps): bump golang.org/x/tools from 0.34.0 to 0.35.0 (golangci#5935) * build(deps): bump github.com/mgechev/revive from 1.10.0 to 1.11.0 (golangci#5933) Co-authored-by: Fernandez Ludovic <[email protected]> * build(deps): bump go-simpler.org/sloglint from 0.11.0 to 0.11.1 (golangci#5936) * fix: panic: close of closed channel (golangci#5939) * build(deps): bump on-headers and compression in /docs (golangci#5944) * build(deps): bump github.com/go-viper/mapstructure/v2 from 2.3.0 to 2.4.0 (golangci#5947) * build(deps): bump github.com/spf13/pflag from 1.0.6 to 1.0.7 (golangci#5948) * build(deps): bump github.com/AlwxSin/noinlineerr from 1.0.4 to 1.0.5 (golangci#5949) * build(deps): bump github.com/securego/gosec/v2 from 2.22.5 to 2.22.6 (golangci#5950) * chore: prepare release * docs: update documentation assets (golangci#5951) Co-authored-by: Fernandez Ludovic <[email protected]> * docs: update GitHub Action assets (golangci#5952) * build(deps): bump github.com/securego/gosec/v2 from 2.22.6 to 2.22.7 (golangci#5953) * build(deps): bump form-data from 3.0.1 to 3.0.4 in /docs (golangci#5954) * tagliatelle: force upper case for custom initialisms (golangci#5956) * build(deps): bump github.com/daixiang0/gci from 0.13.6 to 0.13.7 (golangci#5957) * build(deps): bump github.com/ldez/grignotin from 0.9.0 to 0.10.0 (golangci#5958) * build(deps): bump github.com/bombsimon/wsl/v5 from 5.1.0 to 5.1.1 (golangci#5959) * build(deps): bump github.com/sonatard/noctx from 0.3.5 to 0.4.0 (golangci#5960) * build(deps): bump the linter-testdata group across 3 directories with 3 updates (golangci#5964) * chore: prepare release * chore(deps): update dependency go to v1.24.5 --------- Co-authored-by: dependabot[bot] <49699333+dependabot[bot]@users.noreply.github.com> Co-authored-by: Ludovic Fernandez <[email protected]> Co-authored-by: GolangCI-Lint Releaser <[email protected]> Co-authored-by: Oleksandr Redko <[email protected]> Co-authored-by: Matthew Gabeler-Lee <[email protected]> Co-authored-by: kasaikou <[email protected]> Co-authored-by: Gareth Jones <[email protected]> Co-authored-by: Tatsuya Kyushima <[email protected]> Co-authored-by: logica0419 <[email protected]> Co-authored-by: Manuel Doncel Martos <[email protected]> Co-authored-by: Chris Reeves <[email protected]> Co-authored-by: Nikita Mironov <[email protected]> Co-authored-by: sivchari <[email protected]> Co-authored-by: Tom Vendolsky <[email protected]> Co-authored-by: Cooper Benson <[email protected]> Co-authored-by: Gabriel Augendre <[email protected]> Co-authored-by: Manuel Doncel Martos <[email protected]> Co-authored-by: Takeo Kasai <[email protected]> Co-authored-by: Simon Sawert <[email protected]> Co-authored-by: ccoVeille <[email protected]> Co-authored-by: Alwx <[email protected]> Co-authored-by: Ville Skyttä <[email protected]> Co-authored-by: alaudaa-renovate[bot] <219066560+alaudaa-renovate[bot]@users.noreply.github.com>
Hi @AlwxSin: I just noticed your contribution while our team was trying to adopt the latest golangci-lint. First, thanks for your work on this, and I understand the intent behind it, but I’d like to offer a different perspective. One important advantage of inline error handling is that it limits the scope of the error variable to exactly where it’s relevant. When an error is only used for a single operation, there’s no need to keep it alive in a wider scope. This helps avoid accidental reuse or confusion with unrelated errors, and keeps reasoning about the code more localized. When inline handling is disallowed, you’re forced to declare a top-level err and reuse it for multiple unrelated calls, which can subtly reduce readability. For example:
Here, a single With inline checks, each error is self-contained and unambiguous: This approach makes it immediately clear where each error came from, and the variable’s scope exactly matches its relevance. It’s also worth noting that major style guides explicitly recommend inline error handling in certain cases: Uber Go Style Guide – Reduce Scope of Variables For these reasons, I believe your cc @ldez, enabling this linter by default really goes against widely known, established style recommendations. Could we at least mark it as not recommended by default? |
The linter is not enabled by default, no new linters are. If you don't agree with the style (I don't), don't enable the linter. |
This linter, like other linters, is related to style. |
Hi @bombsimon @ldez , thanks for the quick feedback. First, I have to apologise, after checking our golangci.yaml, I realised the issue happened because we have But this actually highlights a point: our team (and I suspect many others) treat golangci-lint as a trusted guideline for recommended code style. When new linters are added, the implicit expectation is that they are widely recognized, well-accepted, and non-controversial. In this case, as I mentioned earlier, many in the Go community consider banning inline error handling a code smell rather than a style preference, as I mentioned in the Uber’s guideline, which explicitly encourages the opposite. By including a specific linter, golangci-lint still implicitly signals that this style is worth enforcing, since golangci-lint is trusted by a huge portion of the Go community, that signal matters I believe. However, if the position of golangci-lint is that style linters are purely neutral and not meant to guide or educate on best practices, then I understand, but that’s a different philosophy than what I think many users expect. Perhaps we’ve simply placed too much expectation on golangci-lint as a de facto style authority, but that expectation is real. Follow-up question: if someone later proposed a linter that enforces the use of inline error handling, which directly conflicts with this |
@ruanxin I completely understand that handling errors inline is largely a matter of personal preference. Personally, I find it harder to read code where errors are inlined, because it shifts the focus from the function call itself to the error handling logic. That’s the main reason why I created this linter. I also intentionally decided not to enable this linter by default, knowing that in existing codebases this could cause significant issues. In my view, this is a highly opinionated linter, so the decision to enable it should be left entirely to the user. P.S.I respect the guidelines from Uber and Google, though I see them as recommendations, not mandatory rules, and I prefer a different approach in this case. |
golangci-lint is "neutral" but tries not to recommend bad practices. I would emphasize that the user guides you are quoting don't enforce the usage of error inlining. I have a lot of examples where error inlining created unwanted complexity by hiding factorization or creating shadow declarations. Always not inlining is not a bad practice; this is just a point of view. So, I don't think we will accept a linter that forces the usage of error inlining, but you can create a plugin if you want. |
@ruanxin ldez’s comment actually touches on exactly this point — if there is a linter that comprehensively covers all cases of inline if usage and allows enforcing or forbidding any variation, then my noinlineerr linter would become unnecessary. |
If so, I think the discussion so far has been around how to make sharable presets/sharable standards such as the Some reference to previous issues/discussions where recommended/sharable configuration has been discussed:
|
Adding noinlineerr linter.
Purpose - allow only one style of error handling.
Instead of:
Prefer more explicit and readable: