From 604d6bcb9ce5bdb7db818c1bc0fb5c3f923834de Mon Sep 17 00:00:00 2001 From: albertony <12441419+albertony@users.noreply.github.com> Date: Sat, 1 Jun 2024 12:25:42 +0200 Subject: [PATCH] build: enable custom linting rules with ruleguard via gocritic --- .golangci.yml | 49 ++++++++++++++++++++++++++++++++++++++++++------- bin/rules.go | 51 +++++++++++++++++++++++++++++++++++++++++++++++++++ go.mod | 1 + go.sum | 2 ++ 4 files changed, 96 insertions(+), 7 deletions(-) create mode 100644 bin/rules.go diff --git a/.golangci.yml b/.golangci.yml index f4092493a..4d3a8480c 100644 --- a/.golangci.yml +++ b/.golangci.yml @@ -100,10 +100,45 @@ linters-settings: # as documented here: https://staticcheck.io/docs/configuration/options/#checks checks: ["all", "-ST1000", "-ST1003", "-ST1016", "-ST1020", "-ST1021", "-ST1022", "-ST1023"] gocritic: - disabled-checks: - - appendAssign - - captLocal - - commentFormatting - - exitAfterDefer - - ifElseChain - - singleCaseSwitch + # Enable all default checks with some exceptions and some additions (commented). + # Cannot use both enabled-checks and disabled-checks, so must specify all to be used. + disable-all: true + enabled-checks: + #- appendAssign # Enabled by default + - argOrder + - assignOp + - badCall + - badCond + #- captLocal # Enabled by default + - caseOrder + - codegenComment + #- commentFormatting # Enabled by default + - defaultCaseOrder + - deprecatedComment + - dupArg + - dupBranchBody + - dupCase + - dupSubExpr + - elseif + #- exitAfterDefer # Enabled by default + - flagDeref + - flagName + #- ifElseChain # Enabled by default + - mapKey + - newDeref + - offBy1 + - regexpMust + - ruleguard # Not enabled by default + #- singleCaseSwitch # Enabled by default + - sloppyLen + - sloppyTypeAssert + - switchTrue + - typeSwitchVar + - underef + - unlambda + - unslice + - valSwap + - wrapperFunc + settings: + ruleguard: + rules: "${configDir}/bin/rules.go" diff --git a/bin/rules.go b/bin/rules.go new file mode 100644 index 000000000..75816e142 --- /dev/null +++ b/bin/rules.go @@ -0,0 +1,51 @@ +// Ruleguard file implementing custom linting rules. +// +// Note that when used from golangci-lint (using the gocritic linter configured +// with the ruleguard check), because rule files are not handled by +// golangci-lint itself, changes will not invalidate the golangci-lint cache, +// and you must manually clean to cache (golangci-lint cache clean) for them to +// be considered, as explained here: +// https://www.quasilyte.dev/blog/post/ruleguard/#using-from-the-golangci-lint +// +// Note that this file is ignored from build with a build constraint, but using +// a different than "ignore" to avoid go mod tidy making dsl an indirect +// dependency, as explained here: +// https://github.com/quasilyte/go-ruleguard?tab=readme-ov-file#troubleshooting + +//go:build ruleguard +// +build ruleguard + +// Package gorules implementing custom linting rules using ruleguard +package gorules + +import "github.com/quasilyte/go-ruleguard/dsl" + +// Suggest rewriting "log.(Print|Fatal|Panic)(f|ln)?" to +// "fs.(Printf|Fatalf|Panicf)", and do it if running golangci-lint with +// argument --fix. The suggestion wraps a single non-string single argument or +// variadic arguments in fmt.Sprint to be compatible with format string +// argument of fs functions. +// +// Caveats: +// - After applying the suggestions, imports may have to be fixed manually, +// removing unused "log", adding "github.com/rclone/rclone/fs" and "fmt", +// and if there was a variable named "fs" or "fmt" in the scope the name +// clash must be fixed. +// - Suggested code is incorrect when within fs package itself, due to the +// "fs."" prefix. Could handle it using condition +// ".Where(m.File().PkgPath.Matches(`github.com/rclone/rclone/fs`))" +// but not sure how to avoid duplicating all checks with and without this +// condition so haven't bothered yet. +func useFsLog(m dsl.Matcher) { + m.Match(`log.Print($x)`, `log.Println($x)`).Where(m["x"].Type.Is(`string`)).Suggest(`fs.Logf(nil, $x)`) + m.Match(`log.Print($*args)`, `log.Println($*args)`).Suggest(`fs.Logf(nil, fmt.Sprint($args))`) + m.Match(`log.Printf($*args)`).Suggest(`fs.Logf(nil, $args)`) + + m.Match(`log.Fatal($x)`, `log.Fatalln($x)`).Where(m["x"].Type.Is(`string`)).Suggest(`fs.Fatalf(nil, $x)`) + m.Match(`log.Fatal($*args)`, `log.Fatalln($*args)`).Suggest(`fs.Fatalf(nil, fmt.Sprint($args))`) + m.Match(`log.Fatalf($*args)`).Suggest(`fs.Fatalf(nil, $args)`) + + m.Match(`log.Panic($x)`, `log.Panicln($x)`).Where(m["x"].Type.Is(`string`)).Suggest(`fs.Panicf(nil, $x)`) + m.Match(`log.Panic($*args)`, `log.Panicln($*args)`).Suggest(`fs.Panicf(nil, fmt.Sprint($args))`) + m.Match(`log.Panicf($*args)`).Suggest(`fs.Panicf(nil, $args)`) +} diff --git a/go.mod b/go.mod index e860ff31a..1e55be925 100644 --- a/go.mod +++ b/go.mod @@ -58,6 +58,7 @@ require ( github.com/pmezard/go-difflib v1.0.1-0.20181226105442-5d4384ee4fb2 github.com/prometheus/client_golang v1.19.1 github.com/putdotio/go-putio/putio v0.0.0-20200123120452-16d982cac2b8 + github.com/quasilyte/go-ruleguard/dsl v0.3.22 github.com/rclone/gofakes3 v0.0.3-0.20240807151802-e80146f8de87 github.com/rfjakob/eme v1.1.2 github.com/rivo/uniseg v0.4.7 diff --git a/go.sum b/go.sum index 3b48fdf64..81ff97f66 100644 --- a/go.sum +++ b/go.sum @@ -511,6 +511,8 @@ github.com/prometheus/procfs v0.12.0 h1:jluTpSng7V9hY0O2R9DzzJHYb2xULk9VTR1V1R/k github.com/prometheus/procfs v0.12.0/go.mod h1:pcuDEFsWDnvcgNzo4EEweacyhjeA9Zk3cnaOZAZEfOo= github.com/putdotio/go-putio/putio v0.0.0-20200123120452-16d982cac2b8 h1:Y258uzXU/potCYnQd1r6wlAnoMB68BiCkCcCnKx1SH8= github.com/putdotio/go-putio/putio v0.0.0-20200123120452-16d982cac2b8/go.mod h1:bSJjRokAHHOhA+XFxplld8w2R/dXLH7Z3BZ532vhFwU= +github.com/quasilyte/go-ruleguard/dsl v0.3.22 h1:wd8zkOhSNr+I+8Qeciml08ivDt1pSXe60+5DqOpCjPE= +github.com/quasilyte/go-ruleguard/dsl v0.3.22/go.mod h1:KeCP03KrjuSO0H1kTuZQCWlQPulDV6YMIXmpQss17rU= github.com/quic-go/qtls-go1-20 v0.4.1 h1:D33340mCNDAIKBqXuAvexTNMUByrYmFYVfKfDN5nfFs= github.com/quic-go/qtls-go1-20 v0.4.1/go.mod h1:X9Nh97ZL80Z+bX/gUXMbipO6OxdiDi58b/fMC9mAL+k= github.com/quic-go/quic-go v0.40.1 h1:X3AGzUNFs0jVuO3esAGnTfvdgvL4fq655WaOi1snv1Q=