From 305f8b4a06c3459fa3328be97f5e77f7bb23bb49 Mon Sep 17 00:00:00 2001 From: "K. Alex Mills" Date: Tue, 8 Apr 2025 20:05:55 -0500 Subject: [PATCH] feat: goconst supports finding duplicate constants and evaluating constant expressions --- .golangci.next.reference.yml | 6 +++ go.mod | 2 +- go.sum | 4 +- pkg/config/linters_settings.go | 18 ++++---- pkg/golinters/goconst/goconst.go | 46 +++++++++++-------- .../testdata/goconst_eval_expressions.go | 23 ++++++++++ .../testdata/goconst_eval_expressions.yml | 9 ++++ .../testdata/goconst_find_duplicates.go | 15 ++++++ .../testdata/goconst_find_duplicates.yml | 7 +++ pkg/lint/lintersdb/builder_linter.go | 1 + 10 files changed, 102 insertions(+), 29 deletions(-) create mode 100644 pkg/golinters/goconst/testdata/goconst_eval_expressions.go create mode 100644 pkg/golinters/goconst/testdata/goconst_eval_expressions.yml create mode 100644 pkg/golinters/goconst/testdata/goconst_find_duplicates.go create mode 100644 pkg/golinters/goconst/testdata/goconst_find_duplicates.yml diff --git a/.golangci.next.reference.yml b/.golangci.next.reference.yml index 6245b6245ba0..3d7b597ab351 100644 --- a/.golangci.next.reference.yml +++ b/.golangci.next.reference.yml @@ -632,6 +632,12 @@ linters: # Exclude strings matching the given regular expression. # Default: "" ignore-strings: 'foo.+' + # Look for duplicate consts with matching values. + # Default: false + find-duplicates: true + # Evaluate constant expressions when searching for constants. + # Default: false + eval-const-expressions: true gocritic: # Disable all checks. diff --git a/go.mod b/go.mod index 6b1a093621de..e27d813f4062 100644 --- a/go.mod +++ b/go.mod @@ -57,7 +57,7 @@ require ( github.com/gostaticanalysis/forcetypeassert v0.2.0 github.com/gostaticanalysis/nilerr v0.1.1 github.com/hashicorp/go-version v1.7.0 - github.com/jgautheron/goconst v1.7.1 + github.com/jgautheron/goconst v1.8.0 github.com/jingyugao/rowserrcheck v1.1.1 github.com/jjti/go-spancheck v0.6.4 github.com/julz/importas v0.2.0 diff --git a/go.sum b/go.sum index 259c79bb7137..e40a37b887b9 100644 --- a/go.sum +++ b/go.sum @@ -331,8 +331,8 @@ github.com/hexops/gotextdiff v1.0.3/go.mod h1:pSWU5MAI3yDq+fZBTazCSJysOMbxWL1BSo github.com/ianlancetaylor/demangle v0.0.0-20181102032728-5e5cf60278f6/go.mod h1:aSSvb/t6k1mPoxDqO4vJh6VOCGPwU4O0C2/Eqndh1Sc= github.com/inconshreveable/mousetrap v1.1.0 h1:wN+x4NVGpMsO7ErUn/mUI3vEoE6Jt13X2s0bqwp9tc8= github.com/inconshreveable/mousetrap v1.1.0/go.mod h1:vpF70FUmC8bwa3OWnCshd2FqLfsEA9PFc4w1p2J65bw= -github.com/jgautheron/goconst v1.7.1 h1:VpdAG7Ca7yvvJk5n8dMwQhfEZJh95kl/Hl9S1OI5Jkk= -github.com/jgautheron/goconst v1.7.1/go.mod h1:aAosetZ5zaeC/2EfMeRswtxUFBpe2Hr7HzkgX4fanO4= +github.com/jgautheron/goconst v1.8.0 h1:6XbYfLvGzSu8Fx9x1psDTsM61ecm2txB5CkGGLcfYG4= +github.com/jgautheron/goconst v1.8.0/go.mod h1:A0oxgBCHy55NQn6sYpO7UdnA9p+h7cPtoOZUmvNIako= github.com/jingyugao/rowserrcheck v1.1.1 h1:zibz55j/MJtLsjP1OF4bSdgXxwL1b+Vn7Tjzq7gFzUs= github.com/jingyugao/rowserrcheck v1.1.1/go.mod h1:4yvlZSDb3IyDTUZJUmpZfm2Hwok+Dtp+nu2qOq+er9c= github.com/jjti/go-spancheck v0.6.4 h1:Tl7gQpYf4/TMU7AT84MN83/6PutY21Nb9fuQjFTpRRc= diff --git a/pkg/config/linters_settings.go b/pkg/config/linters_settings.go index 046645e40f98..b5698e479dde 100644 --- a/pkg/config/linters_settings.go +++ b/pkg/config/linters_settings.go @@ -451,14 +451,16 @@ type GocognitSettings struct { } type GoConstSettings struct { - IgnoreStrings string `mapstructure:"ignore-strings"` - MatchWithConstants bool `mapstructure:"match-constant"` - MinStringLen int `mapstructure:"min-len"` - MinOccurrencesCount int `mapstructure:"min-occurrences"` - ParseNumbers bool `mapstructure:"numbers"` - NumberMin int `mapstructure:"min"` - NumberMax int `mapstructure:"max"` - IgnoreCalls bool `mapstructure:"ignore-calls"` + IgnoreStrings string `mapstructure:"ignore-strings"` + MatchWithConstants bool `mapstructure:"match-constant"` + MinStringLen int `mapstructure:"min-len"` + MinOccurrencesCount int `mapstructure:"min-occurrences"` + FindDuplicates bool `mapstructure:"find-duplicates"` + EvalConstExpressions bool `mapstructure:"eval-const-expressions"` + ParseNumbers bool `mapstructure:"numbers"` + NumberMin int `mapstructure:"min"` + NumberMax int `mapstructure:"max"` + IgnoreCalls bool `mapstructure:"ignore-calls"` } type GoCriticSettings struct { diff --git a/pkg/golinters/goconst/goconst.go b/pkg/golinters/goconst/goconst.go index 26196f6929d6..5a8a62dc7efa 100644 --- a/pkg/golinters/goconst/goconst.go +++ b/pkg/golinters/goconst/goconst.go @@ -45,22 +45,25 @@ func New(settings *config.GoConstSettings) *goanalysis.Linter { linterName, "Finds repeated strings that could be replaced by a constant", []*analysis.Analyzer{analyzer}, - nil, - ).WithIssuesReporter(func(*linter.Context) []goanalysis.Issue { - return resIssues - }).WithLoadMode(goanalysis.LoadModeSyntax) + nil). + WithIssuesReporter(func(*linter.Context) []goanalysis.Issue { + return resIssues + }). + WithLoadMode(goanalysis.LoadModeTypesInfo) } func runGoconst(pass *analysis.Pass, settings *config.GoConstSettings) ([]goanalysis.Issue, error) { cfg := goconstAPI.Config{ - IgnoreStrings: settings.IgnoreStrings, - MatchWithConstants: settings.MatchWithConstants, - MinStringLength: settings.MinStringLen, - MinOccurrences: settings.MinOccurrencesCount, - ParseNumbers: settings.ParseNumbers, - NumberMin: settings.NumberMin, - NumberMax: settings.NumberMax, - ExcludeTypes: map[goconstAPI.Type]bool{}, + IgnoreStrings: []string{settings.IgnoreStrings}, + MatchWithConstants: settings.MatchWithConstants, + MinStringLength: settings.MinStringLen, + MinOccurrences: settings.MinOccurrencesCount, + FindDuplicates: settings.FindDuplicates, + ParseNumbers: settings.ParseNumbers, + EvalConstExpressions: settings.EvalConstExpressions, + NumberMin: settings.NumberMin, + NumberMax: settings.NumberMax, + ExcludeTypes: map[goconstAPI.Type]bool{}, // Should be managed with `linters.exclusions.rules`. IgnoreTests: false, @@ -70,7 +73,7 @@ func runGoconst(pass *analysis.Pass, settings *config.GoConstSettings) ([]goanal cfg.ExcludeTypes[goconstAPI.Call] = true } - lintIssues, err := goconstAPI.Run(pass.Files, pass.Fset, &cfg) + lintIssues, err := goconstAPI.Run(pass.Files, pass.Fset, pass.TypesInfo, &cfg) if err != nil { return nil, err } @@ -81,12 +84,19 @@ func runGoconst(pass *analysis.Pass, settings *config.GoConstSettings) ([]goanal res := make([]goanalysis.Issue, 0, len(lintIssues)) for _, i := range lintIssues { - text := fmt.Sprintf("string %s has %d occurrences", internal.FormatCode(i.Str, nil), i.OccurrencesCount) + var text string + if i.OccurrencesCount > 0 { + text = fmt.Sprintf("string %s has %d occurrences", internal.FormatCode(i.Str, nil), i.OccurrencesCount) + + if i.MatchingConst == "" { + text += ", make it a constant" + } else { + text += fmt.Sprintf(", but such constant %s already exists", internal.FormatCode(i.MatchingConst, nil)) + } + } - if i.MatchingConst == "" { - text += ", make it a constant" - } else { - text += fmt.Sprintf(", but such constant %s already exists", internal.FormatCode(i.MatchingConst, nil)) + if i.DuplicateConst != "" { + text = fmt.Sprintf("const definition is duplicate of %s at %s", internal.FormatCode(i.DuplicateConst, nil), i.DuplicatePos.String()) } res = append(res, goanalysis.NewIssue(&result.Issue{ diff --git a/pkg/golinters/goconst/testdata/goconst_eval_expressions.go b/pkg/golinters/goconst/testdata/goconst_eval_expressions.go new file mode 100644 index 000000000000..f214a8ee7897 --- /dev/null +++ b/pkg/golinters/goconst/testdata/goconst_eval_expressions.go @@ -0,0 +1,23 @@ +//golangcitest:args -Egoconst +//golangcitest:config_path testdata/goconst_eval_expressions.yml +package testdata + +const Host = "www.golangci.com" +const LinterPath = Host + "/goconst" + +const path = "www.golangci.com/goconst" // want "const definition is duplicate of `LinterPath` at goconst_eval_expressions.go:6:20" + +const KiB = 1 << 10 + +func EvalExpr() { + println(path) + + const duplicated = "www.golangci.com/goconst" // want "const definition is duplicate of `LinterPath` at goconst_eval_expressions.go:6:20" + println(duplicated) + + var unique = "www.golangci.com/another-linter" + println(unique) + + const Kilobytes = 1024 // want "const definition is duplicate of `KiB` at goconst_eval_expressions.go:10:13" + println(Kilobytes) +} diff --git a/pkg/golinters/goconst/testdata/goconst_eval_expressions.yml b/pkg/golinters/goconst/testdata/goconst_eval_expressions.yml new file mode 100644 index 000000000000..bdaa8b288086 --- /dev/null +++ b/pkg/golinters/goconst/testdata/goconst_eval_expressions.yml @@ -0,0 +1,9 @@ +version: "2" + +linters: + settings: + goconst: + find-duplicates: true + eval-const-expressions: true + numbers: true + diff --git a/pkg/golinters/goconst/testdata/goconst_find_duplicates.go b/pkg/golinters/goconst/testdata/goconst_find_duplicates.go new file mode 100644 index 000000000000..464cae23c8d3 --- /dev/null +++ b/pkg/golinters/goconst/testdata/goconst_find_duplicates.go @@ -0,0 +1,15 @@ +//golangcitest:args -Egoconst +//golangcitest:config_path testdata/goconst_find_duplicates.yml +package testdata + +const AConst = "test" +const ( + AnotherConst = "test" // want "const definition is duplicate of `AConst` at goconst_find_duplicates.go:5:7" + UnrelatedConst = "i'm unrelated" +) + +func Bazoo() { + const Duplicated = "test" // want "const definition is duplicate of `AConst` at goconst_find_duplicates.go:5:7" + + const NotDuplicated = "i'm not duplicated" +} diff --git a/pkg/golinters/goconst/testdata/goconst_find_duplicates.yml b/pkg/golinters/goconst/testdata/goconst_find_duplicates.yml new file mode 100644 index 000000000000..d8824e0e1cf5 --- /dev/null +++ b/pkg/golinters/goconst/testdata/goconst_find_duplicates.yml @@ -0,0 +1,7 @@ +version: "2" + +linters: + settings: + goconst: + find-duplicates: true + diff --git a/pkg/lint/lintersdb/builder_linter.go b/pkg/lint/lintersdb/builder_linter.go index c246250dad16..a05e0bbd7f4c 100644 --- a/pkg/lint/lintersdb/builder_linter.go +++ b/pkg/lint/lintersdb/builder_linter.go @@ -300,6 +300,7 @@ func (LinterBuilder) Build(cfg *config.Config) ([]*linter.Config, error) { linter.NewConfig(goconst.New(&cfg.Linters.Settings.Goconst)). WithSince("v1.0.0"). + WithLoadForGoAnalysis(). WithURL("https://github.com/jgautheron/goconst"), linter.NewConfig(gocritic.New(&cfg.Linters.Settings.Gocritic, placeholderReplacer)).