From d18599f12f239055cfcebc492a81b5f2d3e263ca Mon Sep 17 00:00:00 2001 From: Nuruddin Ashr Date: Sun, 14 Jul 2024 10:19:32 +0700 Subject: [PATCH 01/14] Add iface linter --- .golangci.next.reference.yml | 9 + add-iface-patchfile | 440 ++++++++++++++++++ jsonschema/golangci.next.jsonschema.json | 21 + pkg/config/linters_settings.go | 5 + pkg/golinters/iface/iface.go | 75 +++ pkg/golinters/iface/iface_integration_test.go | 11 + pkg/golinters/iface/iface_test.go | 61 +++ pkg/golinters/iface/testdata/duplicate.go | 11 + pkg/golinters/iface/testdata/duplicate.yml | 4 + pkg/golinters/iface/testdata/empty.go | 6 + pkg/golinters/iface/testdata/empty.yml | 4 + pkg/golinters/iface/testdata/opaque.go | 19 + pkg/golinters/iface/testdata/opaque.yml | 4 + pkg/golinters/iface/testdata/unused.go | 35 ++ pkg/golinters/iface/testdata/unused.yml | 4 + pkg/lint/lintersdb/builder_linter.go | 7 + 16 files changed, 716 insertions(+) create mode 100644 add-iface-patchfile create mode 100644 pkg/golinters/iface/iface.go create mode 100644 pkg/golinters/iface/iface_integration_test.go create mode 100644 pkg/golinters/iface/iface_test.go create mode 100644 pkg/golinters/iface/testdata/duplicate.go create mode 100644 pkg/golinters/iface/testdata/duplicate.yml create mode 100644 pkg/golinters/iface/testdata/empty.go create mode 100644 pkg/golinters/iface/testdata/empty.yml create mode 100644 pkg/golinters/iface/testdata/opaque.go create mode 100644 pkg/golinters/iface/testdata/opaque.yml create mode 100644 pkg/golinters/iface/testdata/unused.go create mode 100644 pkg/golinters/iface/testdata/unused.yml diff --git a/.golangci.next.reference.yml b/.golangci.next.reference.yml index 9f7ca0f049d1..2e390fcb7d68 100644 --- a/.golangci.next.reference.yml +++ b/.golangci.next.reference.yml @@ -1222,6 +1222,15 @@ linters-settings: # Default: false var-require-grouping: true + iface: + # By default set to empty. Leave it empty means all analyzers are enabled. + # Default: [] + enable: + - unused + - empty + - duplicate + - opaque + importas: # Do not allow unaliased imports of aliased packages. # Default: false diff --git a/add-iface-patchfile b/add-iface-patchfile new file mode 100644 index 000000000000..c5605292a38b --- /dev/null +++ b/add-iface-patchfile @@ -0,0 +1,440 @@ +diff --git a/.golangci.next.reference.yml b/.golangci.next.reference.yml +index 62b7fd65..fe85c519 100644 +--- a/.golangci.next.reference.yml ++++ b/.golangci.next.reference.yml +@@ -1207,6 +1207,15 @@ linters-settings: + # Default: false + var-require-grouping: true + ++ iface: ++ # By default set to empty. Leave it empty means all analyzers are enabled. ++ # Default: [] ++ enable: ++ - unused ++ - empty ++ - duplicate ++ - opaque ++ + importas: + # Do not allow unaliased imports of aliased packages. + # Default: false +diff --git a/go.mod b/go.mod +index cea54248..7340bb23 100644 +--- a/go.mod ++++ b/go.mod +@@ -1,6 +1,6 @@ + module github.com/golangci/golangci-lint + +-go 1.21.0 ++go 1.22.5 + + require ( + 4d63.com/gocheckcompilerdirectives v1.2.1 +@@ -115,6 +115,7 @@ require ( + github.com/ultraware/funlen v0.1.0 + github.com/ultraware/whitespace v0.1.1 + github.com/uudashr/gocognit v1.1.2 ++ github.com/uudashr/iface v1.0.0 + github.com/valyala/quicktemplate v1.8.0 + github.com/xen0n/gosmopolitan v1.2.2 + github.com/yagipy/maintidx v1.0.0 +diff --git a/go.sum b/go.sum +index cd184b5c..d9a4d9e6 100644 +--- a/go.sum ++++ b/go.sum +@@ -557,6 +557,8 @@ github.com/ultraware/whitespace v0.1.1 h1:bTPOGejYFulW3PkcrqkeQwOd6NKOOXvmGD9bo/ + github.com/ultraware/whitespace v0.1.1/go.mod h1:XcP1RLD81eV4BW8UhQlpaR+SDc2givTvyI8a586WjW8= + github.com/uudashr/gocognit v1.1.2 h1:l6BAEKJqQH2UpKAPKdMfZf5kE4W/2xk8pfU1OVLvniI= + github.com/uudashr/gocognit v1.1.2/go.mod h1:aAVdLURqcanke8h3vg35BC++eseDm66Z7KmchI5et4k= ++github.com/uudashr/iface v1.0.0 h1:NoiiU7xzO9C/4xSMHH7Me7V2qZtRslVHKCIXYvA6VR8= ++github.com/uudashr/iface v1.0.0/go.mod h1:uapH12iu4iwziF3F0exELbQSgJuf3WIwKrNwphQ2xIc= + github.com/valyala/bytebufferpool v1.0.0 h1:GqA5TC/0021Y/b9FG4Oi9Mr3q7XYx6KllzawFIhcdPw= + github.com/valyala/bytebufferpool v1.0.0/go.mod h1:6bBcMArwyJ5K/AmCkWv1jt77kVWyCJ6HpOuEn7z0Csc= + github.com/valyala/quicktemplate v1.8.0 h1:zU0tjbIqTRgKQzFY1L42zq0qR3eh4WoQQdIdqCysW5k= +diff --git a/jsonschema/golangci.next.jsonschema.json b/jsonschema/golangci.next.jsonschema.json +index 0999c278..7d67c1bd 100644 +--- a/jsonschema/golangci.next.jsonschema.json ++++ b/jsonschema/golangci.next.jsonschema.json +@@ -287,6 +287,14 @@ + "waitgroup-by-value" + ] + }, ++ "iface-analyzers": { ++ "enum": [ ++ "empty", ++ "unused", ++ "duplicate", ++ "opaque" ++ ] ++ }, + "linters": { + "$comment": "anyOf with enum is used to allow auto completion of non-custom linters", + "description": "Linters usable.", +@@ -1897,6 +1905,19 @@ + } + } + }, ++ "iface": { ++ "type": "object", ++ "additionalProperties": false, ++ "properties": { ++ "enable": { ++ "description": "Enable analyzers by name.", ++ "type": "array", ++ "items": { ++ "$ref": "#/definitions/iface-analyzers" ++ } ++ } ++ } ++ }, + "importas": { + "type": "object", + "additionalProperties": false, +diff --git a/pkg/config/linters_settings.go b/pkg/config/linters_settings.go +index f159db2a..1a82ad8d 100644 +--- a/pkg/config/linters_settings.go ++++ b/pkg/config/linters_settings.go +@@ -233,6 +233,7 @@ type LintersSettings struct { + Gosmopolitan GosmopolitanSettings + Govet GovetSettings + Grouper GrouperSettings ++ Iface IfaceSettings + ImportAs ImportAsSettings + Inamedparam INamedParamSettings + InterfaceBloat InterfaceBloatSettings +@@ -647,6 +648,10 @@ type GrouperSettings struct { + VarRequireGrouping bool `mapstructure:"var-require-grouping"` + } + ++type IfaceSettings struct { ++ Enable []string `mapstructure:"enable"` ++} ++ + type ImportAsSettings struct { + Alias []ImportAsAlias + NoUnaliased bool `mapstructure:"no-unaliased"` +diff --git a/pkg/golinters/iface/iface.go b/pkg/golinters/iface/iface.go +new file mode 100644 +index 00000000..e0388a4d +--- /dev/null ++++ b/pkg/golinters/iface/iface.go +@@ -0,0 +1,75 @@ ++package iface ++ ++import ( ++ "slices" ++ ++ "github.com/golangci/golangci-lint/pkg/config" ++ "github.com/golangci/golangci-lint/pkg/goanalysis" ++ "github.com/uudashr/iface/duplicate" ++ "github.com/uudashr/iface/empty" ++ "github.com/uudashr/iface/opaque" ++ "github.com/uudashr/iface/unused" ++ "golang.org/x/tools/go/analysis" ++) ++ ++var allAnalyzers = []*analysis.Analyzer{ ++ unused.Analyzer, ++ empty.Analyzer, ++ duplicate.Analyzer, ++ opaque.Analyzer, ++} ++ ++func New(settings *config.IfaceSettings) *goanalysis.Linter { ++ var conf map[string]map[string]any ++ ++ analyzers := analyzersFromSettings(settings) ++ ++ return goanalysis.NewLinter( ++ "iface", ++ "Detect the incorrect use of interfaces, helping developers avoid interface pollution.", ++ analyzers, ++ conf, ++ ).WithLoadMode(goanalysis.LoadModeTypesInfo) ++} ++ ++func analyzersFromSettings(settings *config.IfaceSettings) []*analysis.Analyzer { ++ if settings == nil || len(settings.Enable) == 0 { ++ return allAnalyzers ++ } ++ ++ enabledNames := uniqueNames(settings.Enable) ++ ++ var analyzers []*analysis.Analyzer ++ ++ for _, a := range allAnalyzers { ++ found := slices.ContainsFunc(enabledNames, func(name string) bool { ++ return name == a.Name ++ }) ++ ++ if !found { ++ continue ++ } ++ ++ analyzers = append(analyzers, a) ++ } ++ ++ return analyzers ++} ++ ++func uniqueNames(names []string) []string { ++ if len(names) == 0 { ++ return nil ++ } ++ ++ namesMap := map[string]struct{}{} ++ for _, name := range names { ++ namesMap[name] = struct{}{} ++ } ++ ++ uniqueNames := make([]string, 0, len(namesMap)) ++ ++ for name := range namesMap { ++ uniqueNames = append(uniqueNames, name) ++ } ++ return uniqueNames ++} +diff --git a/pkg/golinters/iface/iface_integration_test.go b/pkg/golinters/iface/iface_integration_test.go +new file mode 100644 +index 00000000..01d587c8 +--- /dev/null ++++ b/pkg/golinters/iface/iface_integration_test.go +@@ -0,0 +1,11 @@ ++package iface ++ ++import ( ++ "testing" ++ ++ "github.com/golangci/golangci-lint/test/testshared/integration" ++) ++ ++func TestFromTestdata(t *testing.T) { ++ integration.RunTestdata(t) ++} +diff --git a/pkg/golinters/iface/iface_test.go b/pkg/golinters/iface/iface_test.go +new file mode 100644 +index 00000000..9a6d4432 +--- /dev/null ++++ b/pkg/golinters/iface/iface_test.go +@@ -0,0 +1,61 @@ ++package iface ++ ++import ( ++ "testing" ++ ++ "github.com/golangci/golangci-lint/pkg/config" ++) ++ ++func TestAnalyzersFromSettings(t *testing.T) { ++ testCases := map[string]struct { ++ enable []string ++ expectedEnabled []string ++ }{ ++ "nil analyzers": { ++ enable: nil, ++ expectedEnabled: []string{"unused", "empty", "duplicate", "opaque"}, ++ }, ++ "empty analyzers": { ++ enable: []string{}, ++ expectedEnabled: []string{"unused", "empty", "duplicate", "opaque"}, ++ }, ++ "unused only": { ++ enable: []string{"unused"}, ++ expectedEnabled: []string{"unused"}, ++ }, ++ "some analyzers": { ++ enable: []string{"unused", "opaque"}, ++ expectedEnabled: []string{"unused", "opaque"}, ++ }, ++ "duplicate analyzers": { ++ enable: []string{"unused", "opaque", "unused"}, ++ expectedEnabled: []string{"unused", "opaque"}, ++ }, ++ "all analyzers": { ++ enable: []string{"unused", "opaque", "empty", "duplicate"}, ++ expectedEnabled: []string{"unused", "empty", "duplicate", "opaque"}, ++ }, ++ } ++ ++ for name, tc := range testCases { ++ t.Run(name, func(t *testing.T) { ++ settings := &config.IfaceSettings{Enable: tc.enable} ++ analyzers := analyzersFromSettings(settings) ++ ++ if len(analyzers) != len(tc.expectedEnabled) { ++ t.Errorf("expected %d analyzers, got %d", len(tc.enable), len(analyzers)) ++ } ++ ++ LoopSettings: ++ for _, a := range analyzers { ++ for _, name := range tc.expectedEnabled { ++ if a.Name == name { ++ continue LoopSettings ++ } ++ } ++ ++ t.Errorf("unexpected analyzer %q", a.Name) ++ } ++ }) ++ } ++} +diff --git a/pkg/golinters/iface/testdata/duplicate.go b/pkg/golinters/iface/testdata/duplicate.go +new file mode 100644 +index 00000000..4a24804b +--- /dev/null ++++ b/pkg/golinters/iface/testdata/duplicate.go +@@ -0,0 +1,11 @@ ++//golangcitest:args -Eiface ++//golangcitest:config_path testdata/duplicate.yml ++package testdata ++ ++type Pinger interface { // want "interface Pinger contains duplicate methods or type constraints from another interface, causing redundancy" ++ Ping() error ++} ++ ++type Healthcheck interface { // want "interface Healthcheck contains duplicate methods or type constraints from another interface, causing redundancy" ++ Ping() error ++} +diff --git a/pkg/golinters/iface/testdata/duplicate.yml b/pkg/golinters/iface/testdata/duplicate.yml +new file mode 100644 +index 00000000..1ac2f360 +--- /dev/null ++++ b/pkg/golinters/iface/testdata/duplicate.yml +@@ -0,0 +1,4 @@ ++linters-settings: ++ iface: ++ enable: ++ - duplicate +diff --git a/pkg/golinters/iface/testdata/empty.go b/pkg/golinters/iface/testdata/empty.go +new file mode 100644 +index 00000000..50ebb24d +--- /dev/null ++++ b/pkg/golinters/iface/testdata/empty.go +@@ -0,0 +1,6 @@ ++//golangcitest:args -Eiface ++//golangcitest:config_path testdata/empty.yml ++package testdata ++ ++type Entity interface { // want "interface Entity is empty, providing no meaningful behavior" ++} +diff --git a/pkg/golinters/iface/testdata/empty.yml b/pkg/golinters/iface/testdata/empty.yml +new file mode 100644 +index 00000000..7837406a +--- /dev/null ++++ b/pkg/golinters/iface/testdata/empty.yml +@@ -0,0 +1,4 @@ ++linters-settings: ++ iface: ++ enable: ++ - empty +diff --git a/pkg/golinters/iface/testdata/opaque.go b/pkg/golinters/iface/testdata/opaque.go +new file mode 100644 +index 00000000..de401dd8 +--- /dev/null ++++ b/pkg/golinters/iface/testdata/opaque.go +@@ -0,0 +1,19 @@ ++//golangcitest:args -Eiface ++//golangcitest:config_path testdata/opaque.yml ++package testdata ++ ++type Server interface { ++ Serve() error ++} ++ ++type server struct { ++ addr string ++} ++ ++func (s server) Serve() error { ++ return nil ++} ++ ++func NewServer(addr string) Server { // want "NewServer function return Server interface at the 1st result, abstract a single concrete implementation of \\*server" ++ return &server{addr: addr} ++} +diff --git a/pkg/golinters/iface/testdata/opaque.yml b/pkg/golinters/iface/testdata/opaque.yml +new file mode 100644 +index 00000000..71bd7047 +--- /dev/null ++++ b/pkg/golinters/iface/testdata/opaque.yml +@@ -0,0 +1,4 @@ ++linters-settings: ++ iface: ++ enable: ++ - opaque +diff --git a/pkg/golinters/iface/testdata/unused.go b/pkg/golinters/iface/testdata/unused.go +new file mode 100644 +index 00000000..69cc117c +--- /dev/null ++++ b/pkg/golinters/iface/testdata/unused.go +@@ -0,0 +1,35 @@ ++//golangcitest:args -Eiface ++//golangcitest:config_path testdata/unused.yml ++package testdata ++ ++type User struct { ++ ID string ++ Name string ++} ++ ++type UserRepository interface { // want "interface UserRepository is declared but not used within the package" ++ UserOf(id string) (*User, error) ++} ++ ++type UserRepositorySQL struct { ++} ++ ++func (r *UserRepositorySQL) UserOf(id string) (*User, error) { ++ return nil, nil ++} ++ ++type Granter interface { ++ Grant(permission string) error ++} ++ ++func AllowAll(g Granter) error { ++ return g.Grant("all") ++} ++ ++type Allower interface { ++ Allow(permission string) error ++} ++ ++func Allow(x interface{}) { ++ _ = x.(Allower) ++} +diff --git a/pkg/golinters/iface/testdata/unused.yml b/pkg/golinters/iface/testdata/unused.yml +new file mode 100644 +index 00000000..a0511b3d +--- /dev/null ++++ b/pkg/golinters/iface/testdata/unused.yml +@@ -0,0 +1,4 @@ ++linters-settings: ++ iface: ++ enable: ++ - unused +diff --git a/pkg/lint/lintersdb/builder_linter.go b/pkg/lint/lintersdb/builder_linter.go +index 7b83a815..893e2bdb 100644 +--- a/pkg/lint/lintersdb/builder_linter.go ++++ b/pkg/lint/lintersdb/builder_linter.go +@@ -55,6 +55,7 @@ import ( + "github.com/golangci/golangci-lint/pkg/golinters/gosmopolitan" + "github.com/golangci/golangci-lint/pkg/golinters/govet" + "github.com/golangci/golangci-lint/pkg/golinters/grouper" ++ "github.com/golangci/golangci-lint/pkg/golinters/iface" + "github.com/golangci/golangci-lint/pkg/golinters/importas" + "github.com/golangci/golangci-lint/pkg/golinters/inamedparam" + "github.com/golangci/golangci-lint/pkg/golinters/ineffassign" +@@ -470,6 +471,12 @@ func (LinterBuilder) Build(cfg *config.Config) ([]*linter.Config, error) { + WithPresets(linter.PresetStyle). + WithURL("https://github.com/leonklingele/grouper"), + ++ linter.NewConfig(iface.New(&cfg.LintersSettings.Iface)). ++ WithSince("v1.60.0"). ++ WithLoadForGoAnalysis(). ++ WithPresets(linter.PresetStyle, linter.PresetMetaLinter). ++ WithURL("http://github.com/uudashr/iface"), ++ + linter.NewConfig(linter.NewNoopDeprecated("ifshort", cfg, linter.DeprecationError)). + WithSince("v1.36.0"). + WithPresets(linter.PresetStyle). diff --git a/jsonschema/golangci.next.jsonschema.json b/jsonschema/golangci.next.jsonschema.json index ebe16acc7bf2..c51c4be819c2 100644 --- a/jsonschema/golangci.next.jsonschema.json +++ b/jsonschema/golangci.next.jsonschema.json @@ -293,6 +293,14 @@ "waitgroup-by-value" ] }, + "iface-analyzers": { + "enum": [ + "empty", + "unused", + "duplicate", + "opaque" + ] + }, "linters": { "$comment": "anyOf with enum is used to allow auto completion of non-custom linters", "description": "Linters usable.", @@ -1910,6 +1918,19 @@ } } }, + "iface": { + "type": "object", + "additionalProperties": false, + "properties": { + "enable": { + "description": "Enable analyzers by name.", + "type": "array", + "items": { + "$ref": "#/definitions/iface-analyzers" + } + } + } + }, "importas": { "type": "object", "additionalProperties": false, diff --git a/pkg/config/linters_settings.go b/pkg/config/linters_settings.go index 109de42431e1..57e5555d54bc 100644 --- a/pkg/config/linters_settings.go +++ b/pkg/config/linters_settings.go @@ -233,6 +233,7 @@ type LintersSettings struct { Gosmopolitan GosmopolitanSettings Govet GovetSettings Grouper GrouperSettings + Iface IfaceSettings ImportAs ImportAsSettings Inamedparam INamedParamSettings InterfaceBloat InterfaceBloatSettings @@ -648,6 +649,10 @@ type GrouperSettings struct { VarRequireGrouping bool `mapstructure:"var-require-grouping"` } +type IfaceSettings struct { + Enable []string `mapstructure:"enable"` +} + type ImportAsSettings struct { Alias []ImportAsAlias NoUnaliased bool `mapstructure:"no-unaliased"` diff --git a/pkg/golinters/iface/iface.go b/pkg/golinters/iface/iface.go new file mode 100644 index 000000000000..e0388a4d0156 --- /dev/null +++ b/pkg/golinters/iface/iface.go @@ -0,0 +1,75 @@ +package iface + +import ( + "slices" + + "github.com/golangci/golangci-lint/pkg/config" + "github.com/golangci/golangci-lint/pkg/goanalysis" + "github.com/uudashr/iface/duplicate" + "github.com/uudashr/iface/empty" + "github.com/uudashr/iface/opaque" + "github.com/uudashr/iface/unused" + "golang.org/x/tools/go/analysis" +) + +var allAnalyzers = []*analysis.Analyzer{ + unused.Analyzer, + empty.Analyzer, + duplicate.Analyzer, + opaque.Analyzer, +} + +func New(settings *config.IfaceSettings) *goanalysis.Linter { + var conf map[string]map[string]any + + analyzers := analyzersFromSettings(settings) + + return goanalysis.NewLinter( + "iface", + "Detect the incorrect use of interfaces, helping developers avoid interface pollution.", + analyzers, + conf, + ).WithLoadMode(goanalysis.LoadModeTypesInfo) +} + +func analyzersFromSettings(settings *config.IfaceSettings) []*analysis.Analyzer { + if settings == nil || len(settings.Enable) == 0 { + return allAnalyzers + } + + enabledNames := uniqueNames(settings.Enable) + + var analyzers []*analysis.Analyzer + + for _, a := range allAnalyzers { + found := slices.ContainsFunc(enabledNames, func(name string) bool { + return name == a.Name + }) + + if !found { + continue + } + + analyzers = append(analyzers, a) + } + + return analyzers +} + +func uniqueNames(names []string) []string { + if len(names) == 0 { + return nil + } + + namesMap := map[string]struct{}{} + for _, name := range names { + namesMap[name] = struct{}{} + } + + uniqueNames := make([]string, 0, len(namesMap)) + + for name := range namesMap { + uniqueNames = append(uniqueNames, name) + } + return uniqueNames +} diff --git a/pkg/golinters/iface/iface_integration_test.go b/pkg/golinters/iface/iface_integration_test.go new file mode 100644 index 000000000000..01d587c8a285 --- /dev/null +++ b/pkg/golinters/iface/iface_integration_test.go @@ -0,0 +1,11 @@ +package iface + +import ( + "testing" + + "github.com/golangci/golangci-lint/test/testshared/integration" +) + +func TestFromTestdata(t *testing.T) { + integration.RunTestdata(t) +} diff --git a/pkg/golinters/iface/iface_test.go b/pkg/golinters/iface/iface_test.go new file mode 100644 index 000000000000..9a6d4432cb83 --- /dev/null +++ b/pkg/golinters/iface/iface_test.go @@ -0,0 +1,61 @@ +package iface + +import ( + "testing" + + "github.com/golangci/golangci-lint/pkg/config" +) + +func TestAnalyzersFromSettings(t *testing.T) { + testCases := map[string]struct { + enable []string + expectedEnabled []string + }{ + "nil analyzers": { + enable: nil, + expectedEnabled: []string{"unused", "empty", "duplicate", "opaque"}, + }, + "empty analyzers": { + enable: []string{}, + expectedEnabled: []string{"unused", "empty", "duplicate", "opaque"}, + }, + "unused only": { + enable: []string{"unused"}, + expectedEnabled: []string{"unused"}, + }, + "some analyzers": { + enable: []string{"unused", "opaque"}, + expectedEnabled: []string{"unused", "opaque"}, + }, + "duplicate analyzers": { + enable: []string{"unused", "opaque", "unused"}, + expectedEnabled: []string{"unused", "opaque"}, + }, + "all analyzers": { + enable: []string{"unused", "opaque", "empty", "duplicate"}, + expectedEnabled: []string{"unused", "empty", "duplicate", "opaque"}, + }, + } + + for name, tc := range testCases { + t.Run(name, func(t *testing.T) { + settings := &config.IfaceSettings{Enable: tc.enable} + analyzers := analyzersFromSettings(settings) + + if len(analyzers) != len(tc.expectedEnabled) { + t.Errorf("expected %d analyzers, got %d", len(tc.enable), len(analyzers)) + } + + LoopSettings: + for _, a := range analyzers { + for _, name := range tc.expectedEnabled { + if a.Name == name { + continue LoopSettings + } + } + + t.Errorf("unexpected analyzer %q", a.Name) + } + }) + } +} diff --git a/pkg/golinters/iface/testdata/duplicate.go b/pkg/golinters/iface/testdata/duplicate.go new file mode 100644 index 000000000000..4a24804b6f30 --- /dev/null +++ b/pkg/golinters/iface/testdata/duplicate.go @@ -0,0 +1,11 @@ +//golangcitest:args -Eiface +//golangcitest:config_path testdata/duplicate.yml +package testdata + +type Pinger interface { // want "interface Pinger contains duplicate methods or type constraints from another interface, causing redundancy" + Ping() error +} + +type Healthcheck interface { // want "interface Healthcheck contains duplicate methods or type constraints from another interface, causing redundancy" + Ping() error +} diff --git a/pkg/golinters/iface/testdata/duplicate.yml b/pkg/golinters/iface/testdata/duplicate.yml new file mode 100644 index 000000000000..1ac2f3609b80 --- /dev/null +++ b/pkg/golinters/iface/testdata/duplicate.yml @@ -0,0 +1,4 @@ +linters-settings: + iface: + enable: + - duplicate diff --git a/pkg/golinters/iface/testdata/empty.go b/pkg/golinters/iface/testdata/empty.go new file mode 100644 index 000000000000..50ebb24dd8d9 --- /dev/null +++ b/pkg/golinters/iface/testdata/empty.go @@ -0,0 +1,6 @@ +//golangcitest:args -Eiface +//golangcitest:config_path testdata/empty.yml +package testdata + +type Entity interface { // want "interface Entity is empty, providing no meaningful behavior" +} diff --git a/pkg/golinters/iface/testdata/empty.yml b/pkg/golinters/iface/testdata/empty.yml new file mode 100644 index 000000000000..7837406a15ef --- /dev/null +++ b/pkg/golinters/iface/testdata/empty.yml @@ -0,0 +1,4 @@ +linters-settings: + iface: + enable: + - empty diff --git a/pkg/golinters/iface/testdata/opaque.go b/pkg/golinters/iface/testdata/opaque.go new file mode 100644 index 000000000000..de401dd8d526 --- /dev/null +++ b/pkg/golinters/iface/testdata/opaque.go @@ -0,0 +1,19 @@ +//golangcitest:args -Eiface +//golangcitest:config_path testdata/opaque.yml +package testdata + +type Server interface { + Serve() error +} + +type server struct { + addr string +} + +func (s server) Serve() error { + return nil +} + +func NewServer(addr string) Server { // want "NewServer function return Server interface at the 1st result, abstract a single concrete implementation of \\*server" + return &server{addr: addr} +} diff --git a/pkg/golinters/iface/testdata/opaque.yml b/pkg/golinters/iface/testdata/opaque.yml new file mode 100644 index 000000000000..71bd7047cab7 --- /dev/null +++ b/pkg/golinters/iface/testdata/opaque.yml @@ -0,0 +1,4 @@ +linters-settings: + iface: + enable: + - opaque diff --git a/pkg/golinters/iface/testdata/unused.go b/pkg/golinters/iface/testdata/unused.go new file mode 100644 index 000000000000..69cc117cfd3a --- /dev/null +++ b/pkg/golinters/iface/testdata/unused.go @@ -0,0 +1,35 @@ +//golangcitest:args -Eiface +//golangcitest:config_path testdata/unused.yml +package testdata + +type User struct { + ID string + Name string +} + +type UserRepository interface { // want "interface UserRepository is declared but not used within the package" + UserOf(id string) (*User, error) +} + +type UserRepositorySQL struct { +} + +func (r *UserRepositorySQL) UserOf(id string) (*User, error) { + return nil, nil +} + +type Granter interface { + Grant(permission string) error +} + +func AllowAll(g Granter) error { + return g.Grant("all") +} + +type Allower interface { + Allow(permission string) error +} + +func Allow(x interface{}) { + _ = x.(Allower) +} diff --git a/pkg/golinters/iface/testdata/unused.yml b/pkg/golinters/iface/testdata/unused.yml new file mode 100644 index 000000000000..a0511b3dd3bd --- /dev/null +++ b/pkg/golinters/iface/testdata/unused.yml @@ -0,0 +1,4 @@ +linters-settings: + iface: + enable: + - unused diff --git a/pkg/lint/lintersdb/builder_linter.go b/pkg/lint/lintersdb/builder_linter.go index 50d9cc154461..5856ff6a5743 100644 --- a/pkg/lint/lintersdb/builder_linter.go +++ b/pkg/lint/lintersdb/builder_linter.go @@ -55,6 +55,7 @@ import ( "github.com/golangci/golangci-lint/pkg/golinters/gosmopolitan" "github.com/golangci/golangci-lint/pkg/golinters/govet" "github.com/golangci/golangci-lint/pkg/golinters/grouper" + "github.com/golangci/golangci-lint/pkg/golinters/iface" "github.com/golangci/golangci-lint/pkg/golinters/importas" "github.com/golangci/golangci-lint/pkg/golinters/inamedparam" "github.com/golangci/golangci-lint/pkg/golinters/ineffassign" @@ -472,6 +473,12 @@ func (LinterBuilder) Build(cfg *config.Config) ([]*linter.Config, error) { WithPresets(linter.PresetStyle). WithURL("https://github.com/leonklingele/grouper"), + linter.NewConfig(iface.New(&cfg.LintersSettings.Iface)). + WithSince("v1.60.0"). + WithLoadForGoAnalysis(). + WithPresets(linter.PresetStyle, linter.PresetMetaLinter). + WithURL("http://github.com/uudashr/iface"), + linter.NewConfig(linter.NewNoopDeprecated("ifshort", cfg, linter.DeprecationError)). WithSince("v1.36.0"). WithPresets(linter.PresetStyle). From 731471c2d2eecc531844b5dff4c666b5050dc4f4 Mon Sep 17 00:00:00 2001 From: Nuruddin Ashr Date: Sun, 14 Jul 2024 10:36:32 +0700 Subject: [PATCH 02/14] Reformat using goimports with -local --- pkg/golinters/iface/iface.go | 5 +++-- 1 file changed, 3 insertions(+), 2 deletions(-) diff --git a/pkg/golinters/iface/iface.go b/pkg/golinters/iface/iface.go index e0388a4d0156..6208b351677c 100644 --- a/pkg/golinters/iface/iface.go +++ b/pkg/golinters/iface/iface.go @@ -3,13 +3,14 @@ package iface import ( "slices" - "github.com/golangci/golangci-lint/pkg/config" - "github.com/golangci/golangci-lint/pkg/goanalysis" "github.com/uudashr/iface/duplicate" "github.com/uudashr/iface/empty" "github.com/uudashr/iface/opaque" "github.com/uudashr/iface/unused" "golang.org/x/tools/go/analysis" + + "github.com/golangci/golangci-lint/pkg/config" + "github.com/golangci/golangci-lint/pkg/goanalysis" ) var allAnalyzers = []*analysis.Analyzer{ From a7180e44ac93df7ee0af36fcdbde89e400379be2 Mon Sep 17 00:00:00 2001 From: Nuruddin Ashr Date: Sun, 14 Jul 2024 14:44:10 +0700 Subject: [PATCH 03/14] Add stdlib import --- pkg/golinters/iface/testdata/unused.go | 6 ++++-- 1 file changed, 4 insertions(+), 2 deletions(-) diff --git a/pkg/golinters/iface/testdata/unused.go b/pkg/golinters/iface/testdata/unused.go index 69cc117cfd3a..2debbe73be49 100644 --- a/pkg/golinters/iface/testdata/unused.go +++ b/pkg/golinters/iface/testdata/unused.go @@ -2,19 +2,21 @@ //golangcitest:config_path testdata/unused.yml package testdata +import "context" + type User struct { ID string Name string } type UserRepository interface { // want "interface UserRepository is declared but not used within the package" - UserOf(id string) (*User, error) + UserOf(ctx context.Context, id string) (*User, error) } type UserRepositorySQL struct { } -func (r *UserRepositorySQL) UserOf(id string) (*User, error) { +func (r *UserRepositorySQL) UserOf(ctx context.Context, id string) (*User, error) { return nil, nil } From 00b224c22f259a4f9bb1c0489a1bae1a91b5ad70 Mon Sep 17 00:00:00 2001 From: Fernandez Ludovic Date: Thu, 18 Jul 2024 22:03:06 +0200 Subject: [PATCH 04/14] review --- .golangci.next.reference.yml | 18 +- add-iface-patchfile | 440 ----------------------- jsonschema/golangci.next.jsonschema.json | 29 +- pkg/config/linters_settings.go | 9 +- pkg/golinters/iface/iface.go | 58 +-- pkg/golinters/iface/iface_test.go | 61 ---- pkg/lint/lintersdb/builder_linter.go | 4 +- 7 files changed, 48 insertions(+), 571 deletions(-) delete mode 100644 add-iface-patchfile delete mode 100644 pkg/golinters/iface/iface_test.go diff --git a/.golangci.next.reference.yml b/.golangci.next.reference.yml index 2e390fcb7d68..40171ca97283 100644 --- a/.golangci.next.reference.yml +++ b/.golangci.next.reference.yml @@ -1223,13 +1223,15 @@ linters-settings: var-require-grouping: true iface: - # By default set to empty. Leave it empty means all analyzers are enabled. - # Default: [] - enable: - - unused - - empty - - duplicate - - opaque + # Identifies interfaces that are not used anywhere in the same package where the interface is defined. + # Default: false + unused: true + # Identifies interfaces in the same package with identical methods or constraints. + # Default: false + identical: true + # Identifies functions that return interfaces, but the actual returned value is always a single concrete implementation. + # Default: false + opaque: true importas: # Do not allow unaliased imports of aliased packages. @@ -2668,6 +2670,7 @@ linters: - gosmopolitan - govet - grouper + - iface - importas - inamedparam - ineffassign @@ -2784,6 +2787,7 @@ linters: - gosmopolitan - govet - grouper + - iface - importas - inamedparam - ineffassign diff --git a/add-iface-patchfile b/add-iface-patchfile deleted file mode 100644 index c5605292a38b..000000000000 --- a/add-iface-patchfile +++ /dev/null @@ -1,440 +0,0 @@ -diff --git a/.golangci.next.reference.yml b/.golangci.next.reference.yml -index 62b7fd65..fe85c519 100644 ---- a/.golangci.next.reference.yml -+++ b/.golangci.next.reference.yml -@@ -1207,6 +1207,15 @@ linters-settings: - # Default: false - var-require-grouping: true - -+ iface: -+ # By default set to empty. Leave it empty means all analyzers are enabled. -+ # Default: [] -+ enable: -+ - unused -+ - empty -+ - duplicate -+ - opaque -+ - importas: - # Do not allow unaliased imports of aliased packages. - # Default: false -diff --git a/go.mod b/go.mod -index cea54248..7340bb23 100644 ---- a/go.mod -+++ b/go.mod -@@ -1,6 +1,6 @@ - module github.com/golangci/golangci-lint - --go 1.21.0 -+go 1.22.5 - - require ( - 4d63.com/gocheckcompilerdirectives v1.2.1 -@@ -115,6 +115,7 @@ require ( - github.com/ultraware/funlen v0.1.0 - github.com/ultraware/whitespace v0.1.1 - github.com/uudashr/gocognit v1.1.2 -+ github.com/uudashr/iface v1.0.0 - github.com/valyala/quicktemplate v1.8.0 - github.com/xen0n/gosmopolitan v1.2.2 - github.com/yagipy/maintidx v1.0.0 -diff --git a/go.sum b/go.sum -index cd184b5c..d9a4d9e6 100644 ---- a/go.sum -+++ b/go.sum -@@ -557,6 +557,8 @@ github.com/ultraware/whitespace v0.1.1 h1:bTPOGejYFulW3PkcrqkeQwOd6NKOOXvmGD9bo/ - github.com/ultraware/whitespace v0.1.1/go.mod h1:XcP1RLD81eV4BW8UhQlpaR+SDc2givTvyI8a586WjW8= - github.com/uudashr/gocognit v1.1.2 h1:l6BAEKJqQH2UpKAPKdMfZf5kE4W/2xk8pfU1OVLvniI= - github.com/uudashr/gocognit v1.1.2/go.mod h1:aAVdLURqcanke8h3vg35BC++eseDm66Z7KmchI5et4k= -+github.com/uudashr/iface v1.0.0 h1:NoiiU7xzO9C/4xSMHH7Me7V2qZtRslVHKCIXYvA6VR8= -+github.com/uudashr/iface v1.0.0/go.mod h1:uapH12iu4iwziF3F0exELbQSgJuf3WIwKrNwphQ2xIc= - github.com/valyala/bytebufferpool v1.0.0 h1:GqA5TC/0021Y/b9FG4Oi9Mr3q7XYx6KllzawFIhcdPw= - github.com/valyala/bytebufferpool v1.0.0/go.mod h1:6bBcMArwyJ5K/AmCkWv1jt77kVWyCJ6HpOuEn7z0Csc= - github.com/valyala/quicktemplate v1.8.0 h1:zU0tjbIqTRgKQzFY1L42zq0qR3eh4WoQQdIdqCysW5k= -diff --git a/jsonschema/golangci.next.jsonschema.json b/jsonschema/golangci.next.jsonschema.json -index 0999c278..7d67c1bd 100644 ---- a/jsonschema/golangci.next.jsonschema.json -+++ b/jsonschema/golangci.next.jsonschema.json -@@ -287,6 +287,14 @@ - "waitgroup-by-value" - ] - }, -+ "iface-analyzers": { -+ "enum": [ -+ "empty", -+ "unused", -+ "duplicate", -+ "opaque" -+ ] -+ }, - "linters": { - "$comment": "anyOf with enum is used to allow auto completion of non-custom linters", - "description": "Linters usable.", -@@ -1897,6 +1905,19 @@ - } - } - }, -+ "iface": { -+ "type": "object", -+ "additionalProperties": false, -+ "properties": { -+ "enable": { -+ "description": "Enable analyzers by name.", -+ "type": "array", -+ "items": { -+ "$ref": "#/definitions/iface-analyzers" -+ } -+ } -+ } -+ }, - "importas": { - "type": "object", - "additionalProperties": false, -diff --git a/pkg/config/linters_settings.go b/pkg/config/linters_settings.go -index f159db2a..1a82ad8d 100644 ---- a/pkg/config/linters_settings.go -+++ b/pkg/config/linters_settings.go -@@ -233,6 +233,7 @@ type LintersSettings struct { - Gosmopolitan GosmopolitanSettings - Govet GovetSettings - Grouper GrouperSettings -+ Iface IfaceSettings - ImportAs ImportAsSettings - Inamedparam INamedParamSettings - InterfaceBloat InterfaceBloatSettings -@@ -647,6 +648,10 @@ type GrouperSettings struct { - VarRequireGrouping bool `mapstructure:"var-require-grouping"` - } - -+type IfaceSettings struct { -+ Enable []string `mapstructure:"enable"` -+} -+ - type ImportAsSettings struct { - Alias []ImportAsAlias - NoUnaliased bool `mapstructure:"no-unaliased"` -diff --git a/pkg/golinters/iface/iface.go b/pkg/golinters/iface/iface.go -new file mode 100644 -index 00000000..e0388a4d ---- /dev/null -+++ b/pkg/golinters/iface/iface.go -@@ -0,0 +1,75 @@ -+package iface -+ -+import ( -+ "slices" -+ -+ "github.com/golangci/golangci-lint/pkg/config" -+ "github.com/golangci/golangci-lint/pkg/goanalysis" -+ "github.com/uudashr/iface/duplicate" -+ "github.com/uudashr/iface/empty" -+ "github.com/uudashr/iface/opaque" -+ "github.com/uudashr/iface/unused" -+ "golang.org/x/tools/go/analysis" -+) -+ -+var allAnalyzers = []*analysis.Analyzer{ -+ unused.Analyzer, -+ empty.Analyzer, -+ duplicate.Analyzer, -+ opaque.Analyzer, -+} -+ -+func New(settings *config.IfaceSettings) *goanalysis.Linter { -+ var conf map[string]map[string]any -+ -+ analyzers := analyzersFromSettings(settings) -+ -+ return goanalysis.NewLinter( -+ "iface", -+ "Detect the incorrect use of interfaces, helping developers avoid interface pollution.", -+ analyzers, -+ conf, -+ ).WithLoadMode(goanalysis.LoadModeTypesInfo) -+} -+ -+func analyzersFromSettings(settings *config.IfaceSettings) []*analysis.Analyzer { -+ if settings == nil || len(settings.Enable) == 0 { -+ return allAnalyzers -+ } -+ -+ enabledNames := uniqueNames(settings.Enable) -+ -+ var analyzers []*analysis.Analyzer -+ -+ for _, a := range allAnalyzers { -+ found := slices.ContainsFunc(enabledNames, func(name string) bool { -+ return name == a.Name -+ }) -+ -+ if !found { -+ continue -+ } -+ -+ analyzers = append(analyzers, a) -+ } -+ -+ return analyzers -+} -+ -+func uniqueNames(names []string) []string { -+ if len(names) == 0 { -+ return nil -+ } -+ -+ namesMap := map[string]struct{}{} -+ for _, name := range names { -+ namesMap[name] = struct{}{} -+ } -+ -+ uniqueNames := make([]string, 0, len(namesMap)) -+ -+ for name := range namesMap { -+ uniqueNames = append(uniqueNames, name) -+ } -+ return uniqueNames -+} -diff --git a/pkg/golinters/iface/iface_integration_test.go b/pkg/golinters/iface/iface_integration_test.go -new file mode 100644 -index 00000000..01d587c8 ---- /dev/null -+++ b/pkg/golinters/iface/iface_integration_test.go -@@ -0,0 +1,11 @@ -+package iface -+ -+import ( -+ "testing" -+ -+ "github.com/golangci/golangci-lint/test/testshared/integration" -+) -+ -+func TestFromTestdata(t *testing.T) { -+ integration.RunTestdata(t) -+} -diff --git a/pkg/golinters/iface/iface_test.go b/pkg/golinters/iface/iface_test.go -new file mode 100644 -index 00000000..9a6d4432 ---- /dev/null -+++ b/pkg/golinters/iface/iface_test.go -@@ -0,0 +1,61 @@ -+package iface -+ -+import ( -+ "testing" -+ -+ "github.com/golangci/golangci-lint/pkg/config" -+) -+ -+func TestAnalyzersFromSettings(t *testing.T) { -+ testCases := map[string]struct { -+ enable []string -+ expectedEnabled []string -+ }{ -+ "nil analyzers": { -+ enable: nil, -+ expectedEnabled: []string{"unused", "empty", "duplicate", "opaque"}, -+ }, -+ "empty analyzers": { -+ enable: []string{}, -+ expectedEnabled: []string{"unused", "empty", "duplicate", "opaque"}, -+ }, -+ "unused only": { -+ enable: []string{"unused"}, -+ expectedEnabled: []string{"unused"}, -+ }, -+ "some analyzers": { -+ enable: []string{"unused", "opaque"}, -+ expectedEnabled: []string{"unused", "opaque"}, -+ }, -+ "duplicate analyzers": { -+ enable: []string{"unused", "opaque", "unused"}, -+ expectedEnabled: []string{"unused", "opaque"}, -+ }, -+ "all analyzers": { -+ enable: []string{"unused", "opaque", "empty", "duplicate"}, -+ expectedEnabled: []string{"unused", "empty", "duplicate", "opaque"}, -+ }, -+ } -+ -+ for name, tc := range testCases { -+ t.Run(name, func(t *testing.T) { -+ settings := &config.IfaceSettings{Enable: tc.enable} -+ analyzers := analyzersFromSettings(settings) -+ -+ if len(analyzers) != len(tc.expectedEnabled) { -+ t.Errorf("expected %d analyzers, got %d", len(tc.enable), len(analyzers)) -+ } -+ -+ LoopSettings: -+ for _, a := range analyzers { -+ for _, name := range tc.expectedEnabled { -+ if a.Name == name { -+ continue LoopSettings -+ } -+ } -+ -+ t.Errorf("unexpected analyzer %q", a.Name) -+ } -+ }) -+ } -+} -diff --git a/pkg/golinters/iface/testdata/duplicate.go b/pkg/golinters/iface/testdata/duplicate.go -new file mode 100644 -index 00000000..4a24804b ---- /dev/null -+++ b/pkg/golinters/iface/testdata/duplicate.go -@@ -0,0 +1,11 @@ -+//golangcitest:args -Eiface -+//golangcitest:config_path testdata/duplicate.yml -+package testdata -+ -+type Pinger interface { // want "interface Pinger contains duplicate methods or type constraints from another interface, causing redundancy" -+ Ping() error -+} -+ -+type Healthcheck interface { // want "interface Healthcheck contains duplicate methods or type constraints from another interface, causing redundancy" -+ Ping() error -+} -diff --git a/pkg/golinters/iface/testdata/duplicate.yml b/pkg/golinters/iface/testdata/duplicate.yml -new file mode 100644 -index 00000000..1ac2f360 ---- /dev/null -+++ b/pkg/golinters/iface/testdata/duplicate.yml -@@ -0,0 +1,4 @@ -+linters-settings: -+ iface: -+ enable: -+ - duplicate -diff --git a/pkg/golinters/iface/testdata/empty.go b/pkg/golinters/iface/testdata/empty.go -new file mode 100644 -index 00000000..50ebb24d ---- /dev/null -+++ b/pkg/golinters/iface/testdata/empty.go -@@ -0,0 +1,6 @@ -+//golangcitest:args -Eiface -+//golangcitest:config_path testdata/empty.yml -+package testdata -+ -+type Entity interface { // want "interface Entity is empty, providing no meaningful behavior" -+} -diff --git a/pkg/golinters/iface/testdata/empty.yml b/pkg/golinters/iface/testdata/empty.yml -new file mode 100644 -index 00000000..7837406a ---- /dev/null -+++ b/pkg/golinters/iface/testdata/empty.yml -@@ -0,0 +1,4 @@ -+linters-settings: -+ iface: -+ enable: -+ - empty -diff --git a/pkg/golinters/iface/testdata/opaque.go b/pkg/golinters/iface/testdata/opaque.go -new file mode 100644 -index 00000000..de401dd8 ---- /dev/null -+++ b/pkg/golinters/iface/testdata/opaque.go -@@ -0,0 +1,19 @@ -+//golangcitest:args -Eiface -+//golangcitest:config_path testdata/opaque.yml -+package testdata -+ -+type Server interface { -+ Serve() error -+} -+ -+type server struct { -+ addr string -+} -+ -+func (s server) Serve() error { -+ return nil -+} -+ -+func NewServer(addr string) Server { // want "NewServer function return Server interface at the 1st result, abstract a single concrete implementation of \\*server" -+ return &server{addr: addr} -+} -diff --git a/pkg/golinters/iface/testdata/opaque.yml b/pkg/golinters/iface/testdata/opaque.yml -new file mode 100644 -index 00000000..71bd7047 ---- /dev/null -+++ b/pkg/golinters/iface/testdata/opaque.yml -@@ -0,0 +1,4 @@ -+linters-settings: -+ iface: -+ enable: -+ - opaque -diff --git a/pkg/golinters/iface/testdata/unused.go b/pkg/golinters/iface/testdata/unused.go -new file mode 100644 -index 00000000..69cc117c ---- /dev/null -+++ b/pkg/golinters/iface/testdata/unused.go -@@ -0,0 +1,35 @@ -+//golangcitest:args -Eiface -+//golangcitest:config_path testdata/unused.yml -+package testdata -+ -+type User struct { -+ ID string -+ Name string -+} -+ -+type UserRepository interface { // want "interface UserRepository is declared but not used within the package" -+ UserOf(id string) (*User, error) -+} -+ -+type UserRepositorySQL struct { -+} -+ -+func (r *UserRepositorySQL) UserOf(id string) (*User, error) { -+ return nil, nil -+} -+ -+type Granter interface { -+ Grant(permission string) error -+} -+ -+func AllowAll(g Granter) error { -+ return g.Grant("all") -+} -+ -+type Allower interface { -+ Allow(permission string) error -+} -+ -+func Allow(x interface{}) { -+ _ = x.(Allower) -+} -diff --git a/pkg/golinters/iface/testdata/unused.yml b/pkg/golinters/iface/testdata/unused.yml -new file mode 100644 -index 00000000..a0511b3d ---- /dev/null -+++ b/pkg/golinters/iface/testdata/unused.yml -@@ -0,0 +1,4 @@ -+linters-settings: -+ iface: -+ enable: -+ - unused -diff --git a/pkg/lint/lintersdb/builder_linter.go b/pkg/lint/lintersdb/builder_linter.go -index 7b83a815..893e2bdb 100644 ---- a/pkg/lint/lintersdb/builder_linter.go -+++ b/pkg/lint/lintersdb/builder_linter.go -@@ -55,6 +55,7 @@ import ( - "github.com/golangci/golangci-lint/pkg/golinters/gosmopolitan" - "github.com/golangci/golangci-lint/pkg/golinters/govet" - "github.com/golangci/golangci-lint/pkg/golinters/grouper" -+ "github.com/golangci/golangci-lint/pkg/golinters/iface" - "github.com/golangci/golangci-lint/pkg/golinters/importas" - "github.com/golangci/golangci-lint/pkg/golinters/inamedparam" - "github.com/golangci/golangci-lint/pkg/golinters/ineffassign" -@@ -470,6 +471,12 @@ func (LinterBuilder) Build(cfg *config.Config) ([]*linter.Config, error) { - WithPresets(linter.PresetStyle). - WithURL("https://github.com/leonklingele/grouper"), - -+ linter.NewConfig(iface.New(&cfg.LintersSettings.Iface)). -+ WithSince("v1.60.0"). -+ WithLoadForGoAnalysis(). -+ WithPresets(linter.PresetStyle, linter.PresetMetaLinter). -+ WithURL("http://github.com/uudashr/iface"), -+ - linter.NewConfig(linter.NewNoopDeprecated("ifshort", cfg, linter.DeprecationError)). - WithSince("v1.36.0"). - WithPresets(linter.PresetStyle). diff --git a/jsonschema/golangci.next.jsonschema.json b/jsonschema/golangci.next.jsonschema.json index c51c4be819c2..7c4d12225529 100644 --- a/jsonschema/golangci.next.jsonschema.json +++ b/jsonschema/golangci.next.jsonschema.json @@ -293,14 +293,6 @@ "waitgroup-by-value" ] }, - "iface-analyzers": { - "enum": [ - "empty", - "unused", - "duplicate", - "opaque" - ] - }, "linters": { "$comment": "anyOf with enum is used to allow auto completion of non-custom linters", "description": "Linters usable.", @@ -362,6 +354,7 @@ "gosmopolitan", "govet", "grouper", + "iface", "ifshort", "importas", "inamedparam", @@ -1922,12 +1915,20 @@ "type": "object", "additionalProperties": false, "properties": { - "enable": { - "description": "Enable analyzers by name.", - "type": "array", - "items": { - "$ref": "#/definitions/iface-analyzers" - } + "unused": { + "description": "Identifies interfaces that are not used anywhere in the same package where the interface is defined.", + "type": "boolean", + "default": false + }, + "identical": { + "description": "Identifies interfaces in the same package with identical methods or constraints.", + "type": "boolean", + "default": false + }, + "opaque": { + "description": "Identifies functions that return interfaces, but the actual returned value is always a single concrete implementation.", + "type": "boolean", + "default": false } } }, diff --git a/pkg/config/linters_settings.go b/pkg/config/linters_settings.go index 57e5555d54bc..f282682f5000 100644 --- a/pkg/config/linters_settings.go +++ b/pkg/config/linters_settings.go @@ -88,6 +88,11 @@ var defaultLintersSettings = LintersSettings{ IgnoreTests: true, WatchForScripts: []string{"Han"}, }, + Iface: IfaceSettings{ + Unused: true, + Identical: true, + Opaque: true, + }, Inamedparam: INamedParamSettings{ SkipSingleParam: false, }, @@ -650,7 +655,9 @@ type GrouperSettings struct { } type IfaceSettings struct { - Enable []string `mapstructure:"enable"` + Unused bool `mapstructure:"unused"` + Identical bool `mapstructure:"identical"` + Opaque bool `mapstructure:"opaque"` } type ImportAsSettings struct { diff --git a/pkg/golinters/iface/iface.go b/pkg/golinters/iface/iface.go index 6208b351677c..dacd062fb6ec 100644 --- a/pkg/golinters/iface/iface.go +++ b/pkg/golinters/iface/iface.go @@ -1,10 +1,7 @@ package iface import ( - "slices" - - "github.com/uudashr/iface/duplicate" - "github.com/uudashr/iface/empty" + "github.com/uudashr/iface/identical" "github.com/uudashr/iface/opaque" "github.com/uudashr/iface/unused" "golang.org/x/tools/go/analysis" @@ -13,64 +10,33 @@ import ( "github.com/golangci/golangci-lint/pkg/goanalysis" ) -var allAnalyzers = []*analysis.Analyzer{ - unused.Analyzer, - empty.Analyzer, - duplicate.Analyzer, - opaque.Analyzer, -} - func New(settings *config.IfaceSettings) *goanalysis.Linter { - var conf map[string]map[string]any - - analyzers := analyzersFromSettings(settings) - return goanalysis.NewLinter( "iface", "Detect the incorrect use of interfaces, helping developers avoid interface pollution.", - analyzers, - conf, + analyzersFromSettings(settings), + nil, ).WithLoadMode(goanalysis.LoadModeTypesInfo) } func analyzersFromSettings(settings *config.IfaceSettings) []*analysis.Analyzer { - if settings == nil || len(settings.Enable) == 0 { - return allAnalyzers + if settings == nil { // FIXME + return nil } - enabledNames := uniqueNames(settings.Enable) - var analyzers []*analysis.Analyzer - for _, a := range allAnalyzers { - found := slices.ContainsFunc(enabledNames, func(name string) bool { - return name == a.Name - }) - - if !found { - continue - } - - analyzers = append(analyzers, a) + if settings.Unused { + analyzers = append(analyzers, unused.Analyzer) } - return analyzers -} - -func uniqueNames(names []string) []string { - if len(names) == 0 { - return nil + if settings.Identical { + analyzers = append(analyzers, identical.Analyzer) } - namesMap := map[string]struct{}{} - for _, name := range names { - namesMap[name] = struct{}{} + if settings.Opaque { + analyzers = append(analyzers, opaque.Analyzer) } - uniqueNames := make([]string, 0, len(namesMap)) - - for name := range namesMap { - uniqueNames = append(uniqueNames, name) - } - return uniqueNames + return analyzers } diff --git a/pkg/golinters/iface/iface_test.go b/pkg/golinters/iface/iface_test.go deleted file mode 100644 index 9a6d4432cb83..000000000000 --- a/pkg/golinters/iface/iface_test.go +++ /dev/null @@ -1,61 +0,0 @@ -package iface - -import ( - "testing" - - "github.com/golangci/golangci-lint/pkg/config" -) - -func TestAnalyzersFromSettings(t *testing.T) { - testCases := map[string]struct { - enable []string - expectedEnabled []string - }{ - "nil analyzers": { - enable: nil, - expectedEnabled: []string{"unused", "empty", "duplicate", "opaque"}, - }, - "empty analyzers": { - enable: []string{}, - expectedEnabled: []string{"unused", "empty", "duplicate", "opaque"}, - }, - "unused only": { - enable: []string{"unused"}, - expectedEnabled: []string{"unused"}, - }, - "some analyzers": { - enable: []string{"unused", "opaque"}, - expectedEnabled: []string{"unused", "opaque"}, - }, - "duplicate analyzers": { - enable: []string{"unused", "opaque", "unused"}, - expectedEnabled: []string{"unused", "opaque"}, - }, - "all analyzers": { - enable: []string{"unused", "opaque", "empty", "duplicate"}, - expectedEnabled: []string{"unused", "empty", "duplicate", "opaque"}, - }, - } - - for name, tc := range testCases { - t.Run(name, func(t *testing.T) { - settings := &config.IfaceSettings{Enable: tc.enable} - analyzers := analyzersFromSettings(settings) - - if len(analyzers) != len(tc.expectedEnabled) { - t.Errorf("expected %d analyzers, got %d", len(tc.enable), len(analyzers)) - } - - LoopSettings: - for _, a := range analyzers { - for _, name := range tc.expectedEnabled { - if a.Name == name { - continue LoopSettings - } - } - - t.Errorf("unexpected analyzer %q", a.Name) - } - }) - } -} diff --git a/pkg/lint/lintersdb/builder_linter.go b/pkg/lint/lintersdb/builder_linter.go index 5856ff6a5743..c4d2d8736ba2 100644 --- a/pkg/lint/lintersdb/builder_linter.go +++ b/pkg/lint/lintersdb/builder_linter.go @@ -476,8 +476,8 @@ func (LinterBuilder) Build(cfg *config.Config) ([]*linter.Config, error) { linter.NewConfig(iface.New(&cfg.LintersSettings.Iface)). WithSince("v1.60.0"). WithLoadForGoAnalysis(). - WithPresets(linter.PresetStyle, linter.PresetMetaLinter). - WithURL("http://github.com/uudashr/iface"), + WithPresets(linter.PresetStyle). + WithURL("https://github.com/uudashr/iface"), linter.NewConfig(linter.NewNoopDeprecated("ifshort", cfg, linter.DeprecationError)). WithSince("v1.36.0"). From a64220f4c934f40829de2e70fb7540b685c1635b Mon Sep 17 00:00:00 2001 From: Nuruddin Ashr Date: Sun, 14 Jul 2024 10:19:32 +0700 Subject: [PATCH 05/14] Add iface linter --- .golangci.next.reference.yml | 16 +- add-iface-patchfile | 440 +++++++++++++++++++++++ jsonschema/golangci.next.jsonschema.json | 28 +- pkg/config/linters_settings.go | 4 +- pkg/golinters/iface/iface.go | 63 +++- pkg/golinters/iface/iface_test.go | 61 ++++ pkg/golinters/iface/testdata/unused.go | 6 +- pkg/lint/lintersdb/builder_linter.go | 4 +- 8 files changed, 575 insertions(+), 47 deletions(-) create mode 100644 add-iface-patchfile create mode 100644 pkg/golinters/iface/iface_test.go diff --git a/.golangci.next.reference.yml b/.golangci.next.reference.yml index 40171ca97283..9f7ef4fa92b1 100644 --- a/.golangci.next.reference.yml +++ b/.golangci.next.reference.yml @@ -1223,15 +1223,13 @@ linters-settings: var-require-grouping: true iface: - # Identifies interfaces that are not used anywhere in the same package where the interface is defined. - # Default: false - unused: true - # Identifies interfaces in the same package with identical methods or constraints. - # Default: false - identical: true - # Identifies functions that return interfaces, but the actual returned value is always a single concrete implementation. - # Default: false - opaque: true + # By default set to empty. Leave it empty means all analyzers are enabled. + # Default: [] + enable: + - unused + - empty + - duplicate + - opaque importas: # Do not allow unaliased imports of aliased packages. diff --git a/add-iface-patchfile b/add-iface-patchfile new file mode 100644 index 000000000000..c5605292a38b --- /dev/null +++ b/add-iface-patchfile @@ -0,0 +1,440 @@ +diff --git a/.golangci.next.reference.yml b/.golangci.next.reference.yml +index 62b7fd65..fe85c519 100644 +--- a/.golangci.next.reference.yml ++++ b/.golangci.next.reference.yml +@@ -1207,6 +1207,15 @@ linters-settings: + # Default: false + var-require-grouping: true + ++ iface: ++ # By default set to empty. Leave it empty means all analyzers are enabled. ++ # Default: [] ++ enable: ++ - unused ++ - empty ++ - duplicate ++ - opaque ++ + importas: + # Do not allow unaliased imports of aliased packages. + # Default: false +diff --git a/go.mod b/go.mod +index cea54248..7340bb23 100644 +--- a/go.mod ++++ b/go.mod +@@ -1,6 +1,6 @@ + module github.com/golangci/golangci-lint + +-go 1.21.0 ++go 1.22.5 + + require ( + 4d63.com/gocheckcompilerdirectives v1.2.1 +@@ -115,6 +115,7 @@ require ( + github.com/ultraware/funlen v0.1.0 + github.com/ultraware/whitespace v0.1.1 + github.com/uudashr/gocognit v1.1.2 ++ github.com/uudashr/iface v1.0.0 + github.com/valyala/quicktemplate v1.8.0 + github.com/xen0n/gosmopolitan v1.2.2 + github.com/yagipy/maintidx v1.0.0 +diff --git a/go.sum b/go.sum +index cd184b5c..d9a4d9e6 100644 +--- a/go.sum ++++ b/go.sum +@@ -557,6 +557,8 @@ github.com/ultraware/whitespace v0.1.1 h1:bTPOGejYFulW3PkcrqkeQwOd6NKOOXvmGD9bo/ + github.com/ultraware/whitespace v0.1.1/go.mod h1:XcP1RLD81eV4BW8UhQlpaR+SDc2givTvyI8a586WjW8= + github.com/uudashr/gocognit v1.1.2 h1:l6BAEKJqQH2UpKAPKdMfZf5kE4W/2xk8pfU1OVLvniI= + github.com/uudashr/gocognit v1.1.2/go.mod h1:aAVdLURqcanke8h3vg35BC++eseDm66Z7KmchI5et4k= ++github.com/uudashr/iface v1.0.0 h1:NoiiU7xzO9C/4xSMHH7Me7V2qZtRslVHKCIXYvA6VR8= ++github.com/uudashr/iface v1.0.0/go.mod h1:uapH12iu4iwziF3F0exELbQSgJuf3WIwKrNwphQ2xIc= + github.com/valyala/bytebufferpool v1.0.0 h1:GqA5TC/0021Y/b9FG4Oi9Mr3q7XYx6KllzawFIhcdPw= + github.com/valyala/bytebufferpool v1.0.0/go.mod h1:6bBcMArwyJ5K/AmCkWv1jt77kVWyCJ6HpOuEn7z0Csc= + github.com/valyala/quicktemplate v1.8.0 h1:zU0tjbIqTRgKQzFY1L42zq0qR3eh4WoQQdIdqCysW5k= +diff --git a/jsonschema/golangci.next.jsonschema.json b/jsonschema/golangci.next.jsonschema.json +index 0999c278..7d67c1bd 100644 +--- a/jsonschema/golangci.next.jsonschema.json ++++ b/jsonschema/golangci.next.jsonschema.json +@@ -287,6 +287,14 @@ + "waitgroup-by-value" + ] + }, ++ "iface-analyzers": { ++ "enum": [ ++ "empty", ++ "unused", ++ "duplicate", ++ "opaque" ++ ] ++ }, + "linters": { + "$comment": "anyOf with enum is used to allow auto completion of non-custom linters", + "description": "Linters usable.", +@@ -1897,6 +1905,19 @@ + } + } + }, ++ "iface": { ++ "type": "object", ++ "additionalProperties": false, ++ "properties": { ++ "enable": { ++ "description": "Enable analyzers by name.", ++ "type": "array", ++ "items": { ++ "$ref": "#/definitions/iface-analyzers" ++ } ++ } ++ } ++ }, + "importas": { + "type": "object", + "additionalProperties": false, +diff --git a/pkg/config/linters_settings.go b/pkg/config/linters_settings.go +index f159db2a..1a82ad8d 100644 +--- a/pkg/config/linters_settings.go ++++ b/pkg/config/linters_settings.go +@@ -233,6 +233,7 @@ type LintersSettings struct { + Gosmopolitan GosmopolitanSettings + Govet GovetSettings + Grouper GrouperSettings ++ Iface IfaceSettings + ImportAs ImportAsSettings + Inamedparam INamedParamSettings + InterfaceBloat InterfaceBloatSettings +@@ -647,6 +648,10 @@ type GrouperSettings struct { + VarRequireGrouping bool `mapstructure:"var-require-grouping"` + } + ++type IfaceSettings struct { ++ Enable []string `mapstructure:"enable"` ++} ++ + type ImportAsSettings struct { + Alias []ImportAsAlias + NoUnaliased bool `mapstructure:"no-unaliased"` +diff --git a/pkg/golinters/iface/iface.go b/pkg/golinters/iface/iface.go +new file mode 100644 +index 00000000..e0388a4d +--- /dev/null ++++ b/pkg/golinters/iface/iface.go +@@ -0,0 +1,75 @@ ++package iface ++ ++import ( ++ "slices" ++ ++ "github.com/golangci/golangci-lint/pkg/config" ++ "github.com/golangci/golangci-lint/pkg/goanalysis" ++ "github.com/uudashr/iface/duplicate" ++ "github.com/uudashr/iface/empty" ++ "github.com/uudashr/iface/opaque" ++ "github.com/uudashr/iface/unused" ++ "golang.org/x/tools/go/analysis" ++) ++ ++var allAnalyzers = []*analysis.Analyzer{ ++ unused.Analyzer, ++ empty.Analyzer, ++ duplicate.Analyzer, ++ opaque.Analyzer, ++} ++ ++func New(settings *config.IfaceSettings) *goanalysis.Linter { ++ var conf map[string]map[string]any ++ ++ analyzers := analyzersFromSettings(settings) ++ ++ return goanalysis.NewLinter( ++ "iface", ++ "Detect the incorrect use of interfaces, helping developers avoid interface pollution.", ++ analyzers, ++ conf, ++ ).WithLoadMode(goanalysis.LoadModeTypesInfo) ++} ++ ++func analyzersFromSettings(settings *config.IfaceSettings) []*analysis.Analyzer { ++ if settings == nil || len(settings.Enable) == 0 { ++ return allAnalyzers ++ } ++ ++ enabledNames := uniqueNames(settings.Enable) ++ ++ var analyzers []*analysis.Analyzer ++ ++ for _, a := range allAnalyzers { ++ found := slices.ContainsFunc(enabledNames, func(name string) bool { ++ return name == a.Name ++ }) ++ ++ if !found { ++ continue ++ } ++ ++ analyzers = append(analyzers, a) ++ } ++ ++ return analyzers ++} ++ ++func uniqueNames(names []string) []string { ++ if len(names) == 0 { ++ return nil ++ } ++ ++ namesMap := map[string]struct{}{} ++ for _, name := range names { ++ namesMap[name] = struct{}{} ++ } ++ ++ uniqueNames := make([]string, 0, len(namesMap)) ++ ++ for name := range namesMap { ++ uniqueNames = append(uniqueNames, name) ++ } ++ return uniqueNames ++} +diff --git a/pkg/golinters/iface/iface_integration_test.go b/pkg/golinters/iface/iface_integration_test.go +new file mode 100644 +index 00000000..01d587c8 +--- /dev/null ++++ b/pkg/golinters/iface/iface_integration_test.go +@@ -0,0 +1,11 @@ ++package iface ++ ++import ( ++ "testing" ++ ++ "github.com/golangci/golangci-lint/test/testshared/integration" ++) ++ ++func TestFromTestdata(t *testing.T) { ++ integration.RunTestdata(t) ++} +diff --git a/pkg/golinters/iface/iface_test.go b/pkg/golinters/iface/iface_test.go +new file mode 100644 +index 00000000..9a6d4432 +--- /dev/null ++++ b/pkg/golinters/iface/iface_test.go +@@ -0,0 +1,61 @@ ++package iface ++ ++import ( ++ "testing" ++ ++ "github.com/golangci/golangci-lint/pkg/config" ++) ++ ++func TestAnalyzersFromSettings(t *testing.T) { ++ testCases := map[string]struct { ++ enable []string ++ expectedEnabled []string ++ }{ ++ "nil analyzers": { ++ enable: nil, ++ expectedEnabled: []string{"unused", "empty", "duplicate", "opaque"}, ++ }, ++ "empty analyzers": { ++ enable: []string{}, ++ expectedEnabled: []string{"unused", "empty", "duplicate", "opaque"}, ++ }, ++ "unused only": { ++ enable: []string{"unused"}, ++ expectedEnabled: []string{"unused"}, ++ }, ++ "some analyzers": { ++ enable: []string{"unused", "opaque"}, ++ expectedEnabled: []string{"unused", "opaque"}, ++ }, ++ "duplicate analyzers": { ++ enable: []string{"unused", "opaque", "unused"}, ++ expectedEnabled: []string{"unused", "opaque"}, ++ }, ++ "all analyzers": { ++ enable: []string{"unused", "opaque", "empty", "duplicate"}, ++ expectedEnabled: []string{"unused", "empty", "duplicate", "opaque"}, ++ }, ++ } ++ ++ for name, tc := range testCases { ++ t.Run(name, func(t *testing.T) { ++ settings := &config.IfaceSettings{Enable: tc.enable} ++ analyzers := analyzersFromSettings(settings) ++ ++ if len(analyzers) != len(tc.expectedEnabled) { ++ t.Errorf("expected %d analyzers, got %d", len(tc.enable), len(analyzers)) ++ } ++ ++ LoopSettings: ++ for _, a := range analyzers { ++ for _, name := range tc.expectedEnabled { ++ if a.Name == name { ++ continue LoopSettings ++ } ++ } ++ ++ t.Errorf("unexpected analyzer %q", a.Name) ++ } ++ }) ++ } ++} +diff --git a/pkg/golinters/iface/testdata/duplicate.go b/pkg/golinters/iface/testdata/duplicate.go +new file mode 100644 +index 00000000..4a24804b +--- /dev/null ++++ b/pkg/golinters/iface/testdata/duplicate.go +@@ -0,0 +1,11 @@ ++//golangcitest:args -Eiface ++//golangcitest:config_path testdata/duplicate.yml ++package testdata ++ ++type Pinger interface { // want "interface Pinger contains duplicate methods or type constraints from another interface, causing redundancy" ++ Ping() error ++} ++ ++type Healthcheck interface { // want "interface Healthcheck contains duplicate methods or type constraints from another interface, causing redundancy" ++ Ping() error ++} +diff --git a/pkg/golinters/iface/testdata/duplicate.yml b/pkg/golinters/iface/testdata/duplicate.yml +new file mode 100644 +index 00000000..1ac2f360 +--- /dev/null ++++ b/pkg/golinters/iface/testdata/duplicate.yml +@@ -0,0 +1,4 @@ ++linters-settings: ++ iface: ++ enable: ++ - duplicate +diff --git a/pkg/golinters/iface/testdata/empty.go b/pkg/golinters/iface/testdata/empty.go +new file mode 100644 +index 00000000..50ebb24d +--- /dev/null ++++ b/pkg/golinters/iface/testdata/empty.go +@@ -0,0 +1,6 @@ ++//golangcitest:args -Eiface ++//golangcitest:config_path testdata/empty.yml ++package testdata ++ ++type Entity interface { // want "interface Entity is empty, providing no meaningful behavior" ++} +diff --git a/pkg/golinters/iface/testdata/empty.yml b/pkg/golinters/iface/testdata/empty.yml +new file mode 100644 +index 00000000..7837406a +--- /dev/null ++++ b/pkg/golinters/iface/testdata/empty.yml +@@ -0,0 +1,4 @@ ++linters-settings: ++ iface: ++ enable: ++ - empty +diff --git a/pkg/golinters/iface/testdata/opaque.go b/pkg/golinters/iface/testdata/opaque.go +new file mode 100644 +index 00000000..de401dd8 +--- /dev/null ++++ b/pkg/golinters/iface/testdata/opaque.go +@@ -0,0 +1,19 @@ ++//golangcitest:args -Eiface ++//golangcitest:config_path testdata/opaque.yml ++package testdata ++ ++type Server interface { ++ Serve() error ++} ++ ++type server struct { ++ addr string ++} ++ ++func (s server) Serve() error { ++ return nil ++} ++ ++func NewServer(addr string) Server { // want "NewServer function return Server interface at the 1st result, abstract a single concrete implementation of \\*server" ++ return &server{addr: addr} ++} +diff --git a/pkg/golinters/iface/testdata/opaque.yml b/pkg/golinters/iface/testdata/opaque.yml +new file mode 100644 +index 00000000..71bd7047 +--- /dev/null ++++ b/pkg/golinters/iface/testdata/opaque.yml +@@ -0,0 +1,4 @@ ++linters-settings: ++ iface: ++ enable: ++ - opaque +diff --git a/pkg/golinters/iface/testdata/unused.go b/pkg/golinters/iface/testdata/unused.go +new file mode 100644 +index 00000000..69cc117c +--- /dev/null ++++ b/pkg/golinters/iface/testdata/unused.go +@@ -0,0 +1,35 @@ ++//golangcitest:args -Eiface ++//golangcitest:config_path testdata/unused.yml ++package testdata ++ ++type User struct { ++ ID string ++ Name string ++} ++ ++type UserRepository interface { // want "interface UserRepository is declared but not used within the package" ++ UserOf(id string) (*User, error) ++} ++ ++type UserRepositorySQL struct { ++} ++ ++func (r *UserRepositorySQL) UserOf(id string) (*User, error) { ++ return nil, nil ++} ++ ++type Granter interface { ++ Grant(permission string) error ++} ++ ++func AllowAll(g Granter) error { ++ return g.Grant("all") ++} ++ ++type Allower interface { ++ Allow(permission string) error ++} ++ ++func Allow(x interface{}) { ++ _ = x.(Allower) ++} +diff --git a/pkg/golinters/iface/testdata/unused.yml b/pkg/golinters/iface/testdata/unused.yml +new file mode 100644 +index 00000000..a0511b3d +--- /dev/null ++++ b/pkg/golinters/iface/testdata/unused.yml +@@ -0,0 +1,4 @@ ++linters-settings: ++ iface: ++ enable: ++ - unused +diff --git a/pkg/lint/lintersdb/builder_linter.go b/pkg/lint/lintersdb/builder_linter.go +index 7b83a815..893e2bdb 100644 +--- a/pkg/lint/lintersdb/builder_linter.go ++++ b/pkg/lint/lintersdb/builder_linter.go +@@ -55,6 +55,7 @@ import ( + "github.com/golangci/golangci-lint/pkg/golinters/gosmopolitan" + "github.com/golangci/golangci-lint/pkg/golinters/govet" + "github.com/golangci/golangci-lint/pkg/golinters/grouper" ++ "github.com/golangci/golangci-lint/pkg/golinters/iface" + "github.com/golangci/golangci-lint/pkg/golinters/importas" + "github.com/golangci/golangci-lint/pkg/golinters/inamedparam" + "github.com/golangci/golangci-lint/pkg/golinters/ineffassign" +@@ -470,6 +471,12 @@ func (LinterBuilder) Build(cfg *config.Config) ([]*linter.Config, error) { + WithPresets(linter.PresetStyle). + WithURL("https://github.com/leonklingele/grouper"), + ++ linter.NewConfig(iface.New(&cfg.LintersSettings.Iface)). ++ WithSince("v1.60.0"). ++ WithLoadForGoAnalysis(). ++ WithPresets(linter.PresetStyle, linter.PresetMetaLinter). ++ WithURL("http://github.com/uudashr/iface"), ++ + linter.NewConfig(linter.NewNoopDeprecated("ifshort", cfg, linter.DeprecationError)). + WithSince("v1.36.0"). + WithPresets(linter.PresetStyle). diff --git a/jsonschema/golangci.next.jsonschema.json b/jsonschema/golangci.next.jsonschema.json index 7c4d12225529..2a0e5326d263 100644 --- a/jsonschema/golangci.next.jsonschema.json +++ b/jsonschema/golangci.next.jsonschema.json @@ -293,6 +293,14 @@ "waitgroup-by-value" ] }, + "iface-analyzers": { + "enum": [ + "empty", + "unused", + "duplicate", + "opaque" + ] + }, "linters": { "$comment": "anyOf with enum is used to allow auto completion of non-custom linters", "description": "Linters usable.", @@ -1915,20 +1923,12 @@ "type": "object", "additionalProperties": false, "properties": { - "unused": { - "description": "Identifies interfaces that are not used anywhere in the same package where the interface is defined.", - "type": "boolean", - "default": false - }, - "identical": { - "description": "Identifies interfaces in the same package with identical methods or constraints.", - "type": "boolean", - "default": false - }, - "opaque": { - "description": "Identifies functions that return interfaces, but the actual returned value is always a single concrete implementation.", - "type": "boolean", - "default": false + "enable": { + "description": "Enable analyzers by name.", + "type": "array", + "items": { + "$ref": "#/definitions/iface-analyzers" + } } } }, diff --git a/pkg/config/linters_settings.go b/pkg/config/linters_settings.go index f282682f5000..750deecfef72 100644 --- a/pkg/config/linters_settings.go +++ b/pkg/config/linters_settings.go @@ -655,9 +655,7 @@ type GrouperSettings struct { } type IfaceSettings struct { - Unused bool `mapstructure:"unused"` - Identical bool `mapstructure:"identical"` - Opaque bool `mapstructure:"opaque"` + Enable []string `mapstructure:"enable"` } type ImportAsSettings struct { diff --git a/pkg/golinters/iface/iface.go b/pkg/golinters/iface/iface.go index dacd062fb6ec..e0388a4d0156 100644 --- a/pkg/golinters/iface/iface.go +++ b/pkg/golinters/iface/iface.go @@ -1,42 +1,75 @@ package iface import ( - "github.com/uudashr/iface/identical" - "github.com/uudashr/iface/opaque" - "github.com/uudashr/iface/unused" - "golang.org/x/tools/go/analysis" + "slices" "github.com/golangci/golangci-lint/pkg/config" "github.com/golangci/golangci-lint/pkg/goanalysis" + "github.com/uudashr/iface/duplicate" + "github.com/uudashr/iface/empty" + "github.com/uudashr/iface/opaque" + "github.com/uudashr/iface/unused" + "golang.org/x/tools/go/analysis" ) +var allAnalyzers = []*analysis.Analyzer{ + unused.Analyzer, + empty.Analyzer, + duplicate.Analyzer, + opaque.Analyzer, +} + func New(settings *config.IfaceSettings) *goanalysis.Linter { + var conf map[string]map[string]any + + analyzers := analyzersFromSettings(settings) + return goanalysis.NewLinter( "iface", "Detect the incorrect use of interfaces, helping developers avoid interface pollution.", - analyzersFromSettings(settings), - nil, + analyzers, + conf, ).WithLoadMode(goanalysis.LoadModeTypesInfo) } func analyzersFromSettings(settings *config.IfaceSettings) []*analysis.Analyzer { - if settings == nil { // FIXME - return nil + if settings == nil || len(settings.Enable) == 0 { + return allAnalyzers } + enabledNames := uniqueNames(settings.Enable) + var analyzers []*analysis.Analyzer - if settings.Unused { - analyzers = append(analyzers, unused.Analyzer) + for _, a := range allAnalyzers { + found := slices.ContainsFunc(enabledNames, func(name string) bool { + return name == a.Name + }) + + if !found { + continue + } + + analyzers = append(analyzers, a) } - if settings.Identical { - analyzers = append(analyzers, identical.Analyzer) + return analyzers +} + +func uniqueNames(names []string) []string { + if len(names) == 0 { + return nil } - if settings.Opaque { - analyzers = append(analyzers, opaque.Analyzer) + namesMap := map[string]struct{}{} + for _, name := range names { + namesMap[name] = struct{}{} } - return analyzers + uniqueNames := make([]string, 0, len(namesMap)) + + for name := range namesMap { + uniqueNames = append(uniqueNames, name) + } + return uniqueNames } diff --git a/pkg/golinters/iface/iface_test.go b/pkg/golinters/iface/iface_test.go new file mode 100644 index 000000000000..9a6d4432cb83 --- /dev/null +++ b/pkg/golinters/iface/iface_test.go @@ -0,0 +1,61 @@ +package iface + +import ( + "testing" + + "github.com/golangci/golangci-lint/pkg/config" +) + +func TestAnalyzersFromSettings(t *testing.T) { + testCases := map[string]struct { + enable []string + expectedEnabled []string + }{ + "nil analyzers": { + enable: nil, + expectedEnabled: []string{"unused", "empty", "duplicate", "opaque"}, + }, + "empty analyzers": { + enable: []string{}, + expectedEnabled: []string{"unused", "empty", "duplicate", "opaque"}, + }, + "unused only": { + enable: []string{"unused"}, + expectedEnabled: []string{"unused"}, + }, + "some analyzers": { + enable: []string{"unused", "opaque"}, + expectedEnabled: []string{"unused", "opaque"}, + }, + "duplicate analyzers": { + enable: []string{"unused", "opaque", "unused"}, + expectedEnabled: []string{"unused", "opaque"}, + }, + "all analyzers": { + enable: []string{"unused", "opaque", "empty", "duplicate"}, + expectedEnabled: []string{"unused", "empty", "duplicate", "opaque"}, + }, + } + + for name, tc := range testCases { + t.Run(name, func(t *testing.T) { + settings := &config.IfaceSettings{Enable: tc.enable} + analyzers := analyzersFromSettings(settings) + + if len(analyzers) != len(tc.expectedEnabled) { + t.Errorf("expected %d analyzers, got %d", len(tc.enable), len(analyzers)) + } + + LoopSettings: + for _, a := range analyzers { + for _, name := range tc.expectedEnabled { + if a.Name == name { + continue LoopSettings + } + } + + t.Errorf("unexpected analyzer %q", a.Name) + } + }) + } +} diff --git a/pkg/golinters/iface/testdata/unused.go b/pkg/golinters/iface/testdata/unused.go index 2debbe73be49..69cc117cfd3a 100644 --- a/pkg/golinters/iface/testdata/unused.go +++ b/pkg/golinters/iface/testdata/unused.go @@ -2,21 +2,19 @@ //golangcitest:config_path testdata/unused.yml package testdata -import "context" - type User struct { ID string Name string } type UserRepository interface { // want "interface UserRepository is declared but not used within the package" - UserOf(ctx context.Context, id string) (*User, error) + UserOf(id string) (*User, error) } type UserRepositorySQL struct { } -func (r *UserRepositorySQL) UserOf(ctx context.Context, id string) (*User, error) { +func (r *UserRepositorySQL) UserOf(id string) (*User, error) { return nil, nil } diff --git a/pkg/lint/lintersdb/builder_linter.go b/pkg/lint/lintersdb/builder_linter.go index c4d2d8736ba2..5856ff6a5743 100644 --- a/pkg/lint/lintersdb/builder_linter.go +++ b/pkg/lint/lintersdb/builder_linter.go @@ -476,8 +476,8 @@ func (LinterBuilder) Build(cfg *config.Config) ([]*linter.Config, error) { linter.NewConfig(iface.New(&cfg.LintersSettings.Iface)). WithSince("v1.60.0"). WithLoadForGoAnalysis(). - WithPresets(linter.PresetStyle). - WithURL("https://github.com/uudashr/iface"), + WithPresets(linter.PresetStyle, linter.PresetMetaLinter). + WithURL("http://github.com/uudashr/iface"), linter.NewConfig(linter.NewNoopDeprecated("ifshort", cfg, linter.DeprecationError)). WithSince("v1.36.0"). From 473882014d91a47d7c64f45eba758147f5a3038b Mon Sep 17 00:00:00 2001 From: Nuruddin Ashr Date: Wed, 18 Sep 2024 18:34:30 +0700 Subject: [PATCH 06/14] Remove "empty" and rename "duplicate" to "identical" --- .golangci.next.reference.yml | 9 +++++++-- jsonschema/golangci.next.jsonschema.json | 3 +-- pkg/golinters/iface/iface.go | 6 ++---- pkg/golinters/iface/iface_test.go | 8 ++++---- pkg/golinters/iface/testdata/duplicate.go | 11 ----------- pkg/golinters/iface/testdata/empty.go | 6 ------ pkg/golinters/iface/testdata/empty.yml | 4 ---- pkg/golinters/iface/testdata/identical.go | 11 +++++++++++ .../iface/testdata/{duplicate.yml => identical.yml} | 2 +- 9 files changed, 26 insertions(+), 34 deletions(-) delete mode 100644 pkg/golinters/iface/testdata/duplicate.go delete mode 100644 pkg/golinters/iface/testdata/empty.go delete mode 100644 pkg/golinters/iface/testdata/empty.yml create mode 100644 pkg/golinters/iface/testdata/identical.go rename pkg/golinters/iface/testdata/{duplicate.yml => identical.yml} (68%) diff --git a/.golangci.next.reference.yml b/.golangci.next.reference.yml index 9f7ef4fa92b1..f20874515ede 100644 --- a/.golangci.next.reference.yml +++ b/.golangci.next.reference.yml @@ -1227,9 +1227,14 @@ linters-settings: # Default: [] enable: - unused - - empty - - duplicate + - identical - opaque + settings: + unused: + # Comma-separated list of packages to exclude from the check. + # Default: [] + exclude: + - github.com/example/log importas: # Do not allow unaliased imports of aliased packages. diff --git a/jsonschema/golangci.next.jsonschema.json b/jsonschema/golangci.next.jsonschema.json index 2a0e5326d263..994039bb5265 100644 --- a/jsonschema/golangci.next.jsonschema.json +++ b/jsonschema/golangci.next.jsonschema.json @@ -295,9 +295,8 @@ }, "iface-analyzers": { "enum": [ - "empty", "unused", - "duplicate", + "identical", "opaque" ] }, diff --git a/pkg/golinters/iface/iface.go b/pkg/golinters/iface/iface.go index e0388a4d0156..b992e94671a1 100644 --- a/pkg/golinters/iface/iface.go +++ b/pkg/golinters/iface/iface.go @@ -5,8 +5,7 @@ import ( "github.com/golangci/golangci-lint/pkg/config" "github.com/golangci/golangci-lint/pkg/goanalysis" - "github.com/uudashr/iface/duplicate" - "github.com/uudashr/iface/empty" + "github.com/uudashr/iface/identical" "github.com/uudashr/iface/opaque" "github.com/uudashr/iface/unused" "golang.org/x/tools/go/analysis" @@ -14,8 +13,7 @@ import ( var allAnalyzers = []*analysis.Analyzer{ unused.Analyzer, - empty.Analyzer, - duplicate.Analyzer, + identical.Analyzer, opaque.Analyzer, } diff --git a/pkg/golinters/iface/iface_test.go b/pkg/golinters/iface/iface_test.go index 9a6d4432cb83..95cb71c98c04 100644 --- a/pkg/golinters/iface/iface_test.go +++ b/pkg/golinters/iface/iface_test.go @@ -13,11 +13,11 @@ func TestAnalyzersFromSettings(t *testing.T) { }{ "nil analyzers": { enable: nil, - expectedEnabled: []string{"unused", "empty", "duplicate", "opaque"}, + expectedEnabled: []string{"unused", "identical", "opaque"}, }, "empty analyzers": { enable: []string{}, - expectedEnabled: []string{"unused", "empty", "duplicate", "opaque"}, + expectedEnabled: []string{"unused", "identical", "opaque"}, }, "unused only": { enable: []string{"unused"}, @@ -32,8 +32,8 @@ func TestAnalyzersFromSettings(t *testing.T) { expectedEnabled: []string{"unused", "opaque"}, }, "all analyzers": { - enable: []string{"unused", "opaque", "empty", "duplicate"}, - expectedEnabled: []string{"unused", "empty", "duplicate", "opaque"}, + enable: []string{"unused", "opaque", "identical"}, + expectedEnabled: []string{"unused", "identical", "opaque"}, }, } diff --git a/pkg/golinters/iface/testdata/duplicate.go b/pkg/golinters/iface/testdata/duplicate.go deleted file mode 100644 index 4a24804b6f30..000000000000 --- a/pkg/golinters/iface/testdata/duplicate.go +++ /dev/null @@ -1,11 +0,0 @@ -//golangcitest:args -Eiface -//golangcitest:config_path testdata/duplicate.yml -package testdata - -type Pinger interface { // want "interface Pinger contains duplicate methods or type constraints from another interface, causing redundancy" - Ping() error -} - -type Healthcheck interface { // want "interface Healthcheck contains duplicate methods or type constraints from another interface, causing redundancy" - Ping() error -} diff --git a/pkg/golinters/iface/testdata/empty.go b/pkg/golinters/iface/testdata/empty.go deleted file mode 100644 index 50ebb24dd8d9..000000000000 --- a/pkg/golinters/iface/testdata/empty.go +++ /dev/null @@ -1,6 +0,0 @@ -//golangcitest:args -Eiface -//golangcitest:config_path testdata/empty.yml -package testdata - -type Entity interface { // want "interface Entity is empty, providing no meaningful behavior" -} diff --git a/pkg/golinters/iface/testdata/empty.yml b/pkg/golinters/iface/testdata/empty.yml deleted file mode 100644 index 7837406a15ef..000000000000 --- a/pkg/golinters/iface/testdata/empty.yml +++ /dev/null @@ -1,4 +0,0 @@ -linters-settings: - iface: - enable: - - empty diff --git a/pkg/golinters/iface/testdata/identical.go b/pkg/golinters/iface/testdata/identical.go new file mode 100644 index 000000000000..f13e756aaecf --- /dev/null +++ b/pkg/golinters/iface/testdata/identical.go @@ -0,0 +1,11 @@ +//golangcitest:args -Eiface +//golangcitest:config_path testdata/identical.yml +package testdata + +type Pinger interface { // want "interface Pinger contains identical methods or type constraints from another interface, causing redundancy" + Ping() error +} + +type Healthcheck interface { // want "interface Healthcheck contains identical methods or type constraints from another interface, causing redundancy" + Ping() error +} diff --git a/pkg/golinters/iface/testdata/duplicate.yml b/pkg/golinters/iface/testdata/identical.yml similarity index 68% rename from pkg/golinters/iface/testdata/duplicate.yml rename to pkg/golinters/iface/testdata/identical.yml index 1ac2f3609b80..c6117380d661 100644 --- a/pkg/golinters/iface/testdata/duplicate.yml +++ b/pkg/golinters/iface/testdata/identical.yml @@ -1,4 +1,4 @@ linters-settings: iface: enable: - - duplicate + - identical From c0387d518f5a4a49e8d63251e87c8cbd67eebc4b Mon Sep 17 00:00:00 2001 From: Fernandez Ludovic Date: Wed, 18 Sep 2024 14:06:00 +0200 Subject: [PATCH 07/14] review --- go.mod | 1 + go.sum | 2 ++ 2 files changed, 3 insertions(+) diff --git a/go.mod b/go.mod index c85dcdb5f2df..5ac2e81e0fd2 100644 --- a/go.mod +++ b/go.mod @@ -116,6 +116,7 @@ require ( github.com/ultraware/funlen v0.1.0 github.com/ultraware/whitespace v0.1.1 github.com/uudashr/gocognit v1.1.3 + github.com/uudashr/iface v1.2.0 github.com/valyala/quicktemplate v1.8.0 github.com/xen0n/gosmopolitan v1.2.2 github.com/yagipy/maintidx v1.0.0 diff --git a/go.sum b/go.sum index 97cc520e1fe9..1b2d54a3100d 100644 --- a/go.sum +++ b/go.sum @@ -561,6 +561,8 @@ github.com/ultraware/whitespace v0.1.1 h1:bTPOGejYFulW3PkcrqkeQwOd6NKOOXvmGD9bo/ github.com/ultraware/whitespace v0.1.1/go.mod h1:XcP1RLD81eV4BW8UhQlpaR+SDc2givTvyI8a586WjW8= github.com/uudashr/gocognit v1.1.3 h1:l+a111VcDbKfynh+airAy/DJQKaXh2m9vkoysMPSZyM= github.com/uudashr/gocognit v1.1.3/go.mod h1:aKH8/e8xbTRBwjbCkwZ8qt4l2EpKXl31KMHgSS+lZ2U= +github.com/uudashr/iface v1.2.0 h1:ECJjh5q/1Zmnv/2yFpWV6H3oMg5+Mo+vL0aqw9Gjazo= +github.com/uudashr/iface v1.2.0/go.mod h1:Ux/7d/rAF3owK4m53cTVXL4YoVHKNqnoOeQHn2xrlp0= github.com/valyala/bytebufferpool v1.0.0 h1:GqA5TC/0021Y/b9FG4Oi9Mr3q7XYx6KllzawFIhcdPw= github.com/valyala/bytebufferpool v1.0.0/go.mod h1:6bBcMArwyJ5K/AmCkWv1jt77kVWyCJ6HpOuEn7z0Csc= github.com/valyala/quicktemplate v1.8.0 h1:zU0tjbIqTRgKQzFY1L42zq0qR3eh4WoQQdIdqCysW5k= From 8cea470bad945c3f0a64b505146bf0978c39646e Mon Sep 17 00:00:00 2001 From: Fernandez Ludovic Date: Wed, 18 Sep 2024 14:07:09 +0200 Subject: [PATCH 08/14] review --- add-iface-patchfile | 440 -------------------------------------------- 1 file changed, 440 deletions(-) delete mode 100644 add-iface-patchfile diff --git a/add-iface-patchfile b/add-iface-patchfile deleted file mode 100644 index c5605292a38b..000000000000 --- a/add-iface-patchfile +++ /dev/null @@ -1,440 +0,0 @@ -diff --git a/.golangci.next.reference.yml b/.golangci.next.reference.yml -index 62b7fd65..fe85c519 100644 ---- a/.golangci.next.reference.yml -+++ b/.golangci.next.reference.yml -@@ -1207,6 +1207,15 @@ linters-settings: - # Default: false - var-require-grouping: true - -+ iface: -+ # By default set to empty. Leave it empty means all analyzers are enabled. -+ # Default: [] -+ enable: -+ - unused -+ - empty -+ - duplicate -+ - opaque -+ - importas: - # Do not allow unaliased imports of aliased packages. - # Default: false -diff --git a/go.mod b/go.mod -index cea54248..7340bb23 100644 ---- a/go.mod -+++ b/go.mod -@@ -1,6 +1,6 @@ - module github.com/golangci/golangci-lint - --go 1.21.0 -+go 1.22.5 - - require ( - 4d63.com/gocheckcompilerdirectives v1.2.1 -@@ -115,6 +115,7 @@ require ( - github.com/ultraware/funlen v0.1.0 - github.com/ultraware/whitespace v0.1.1 - github.com/uudashr/gocognit v1.1.2 -+ github.com/uudashr/iface v1.0.0 - github.com/valyala/quicktemplate v1.8.0 - github.com/xen0n/gosmopolitan v1.2.2 - github.com/yagipy/maintidx v1.0.0 -diff --git a/go.sum b/go.sum -index cd184b5c..d9a4d9e6 100644 ---- a/go.sum -+++ b/go.sum -@@ -557,6 +557,8 @@ github.com/ultraware/whitespace v0.1.1 h1:bTPOGejYFulW3PkcrqkeQwOd6NKOOXvmGD9bo/ - github.com/ultraware/whitespace v0.1.1/go.mod h1:XcP1RLD81eV4BW8UhQlpaR+SDc2givTvyI8a586WjW8= - github.com/uudashr/gocognit v1.1.2 h1:l6BAEKJqQH2UpKAPKdMfZf5kE4W/2xk8pfU1OVLvniI= - github.com/uudashr/gocognit v1.1.2/go.mod h1:aAVdLURqcanke8h3vg35BC++eseDm66Z7KmchI5et4k= -+github.com/uudashr/iface v1.0.0 h1:NoiiU7xzO9C/4xSMHH7Me7V2qZtRslVHKCIXYvA6VR8= -+github.com/uudashr/iface v1.0.0/go.mod h1:uapH12iu4iwziF3F0exELbQSgJuf3WIwKrNwphQ2xIc= - github.com/valyala/bytebufferpool v1.0.0 h1:GqA5TC/0021Y/b9FG4Oi9Mr3q7XYx6KllzawFIhcdPw= - github.com/valyala/bytebufferpool v1.0.0/go.mod h1:6bBcMArwyJ5K/AmCkWv1jt77kVWyCJ6HpOuEn7z0Csc= - github.com/valyala/quicktemplate v1.8.0 h1:zU0tjbIqTRgKQzFY1L42zq0qR3eh4WoQQdIdqCysW5k= -diff --git a/jsonschema/golangci.next.jsonschema.json b/jsonschema/golangci.next.jsonschema.json -index 0999c278..7d67c1bd 100644 ---- a/jsonschema/golangci.next.jsonschema.json -+++ b/jsonschema/golangci.next.jsonschema.json -@@ -287,6 +287,14 @@ - "waitgroup-by-value" - ] - }, -+ "iface-analyzers": { -+ "enum": [ -+ "empty", -+ "unused", -+ "duplicate", -+ "opaque" -+ ] -+ }, - "linters": { - "$comment": "anyOf with enum is used to allow auto completion of non-custom linters", - "description": "Linters usable.", -@@ -1897,6 +1905,19 @@ - } - } - }, -+ "iface": { -+ "type": "object", -+ "additionalProperties": false, -+ "properties": { -+ "enable": { -+ "description": "Enable analyzers by name.", -+ "type": "array", -+ "items": { -+ "$ref": "#/definitions/iface-analyzers" -+ } -+ } -+ } -+ }, - "importas": { - "type": "object", - "additionalProperties": false, -diff --git a/pkg/config/linters_settings.go b/pkg/config/linters_settings.go -index f159db2a..1a82ad8d 100644 ---- a/pkg/config/linters_settings.go -+++ b/pkg/config/linters_settings.go -@@ -233,6 +233,7 @@ type LintersSettings struct { - Gosmopolitan GosmopolitanSettings - Govet GovetSettings - Grouper GrouperSettings -+ Iface IfaceSettings - ImportAs ImportAsSettings - Inamedparam INamedParamSettings - InterfaceBloat InterfaceBloatSettings -@@ -647,6 +648,10 @@ type GrouperSettings struct { - VarRequireGrouping bool `mapstructure:"var-require-grouping"` - } - -+type IfaceSettings struct { -+ Enable []string `mapstructure:"enable"` -+} -+ - type ImportAsSettings struct { - Alias []ImportAsAlias - NoUnaliased bool `mapstructure:"no-unaliased"` -diff --git a/pkg/golinters/iface/iface.go b/pkg/golinters/iface/iface.go -new file mode 100644 -index 00000000..e0388a4d ---- /dev/null -+++ b/pkg/golinters/iface/iface.go -@@ -0,0 +1,75 @@ -+package iface -+ -+import ( -+ "slices" -+ -+ "github.com/golangci/golangci-lint/pkg/config" -+ "github.com/golangci/golangci-lint/pkg/goanalysis" -+ "github.com/uudashr/iface/duplicate" -+ "github.com/uudashr/iface/empty" -+ "github.com/uudashr/iface/opaque" -+ "github.com/uudashr/iface/unused" -+ "golang.org/x/tools/go/analysis" -+) -+ -+var allAnalyzers = []*analysis.Analyzer{ -+ unused.Analyzer, -+ empty.Analyzer, -+ duplicate.Analyzer, -+ opaque.Analyzer, -+} -+ -+func New(settings *config.IfaceSettings) *goanalysis.Linter { -+ var conf map[string]map[string]any -+ -+ analyzers := analyzersFromSettings(settings) -+ -+ return goanalysis.NewLinter( -+ "iface", -+ "Detect the incorrect use of interfaces, helping developers avoid interface pollution.", -+ analyzers, -+ conf, -+ ).WithLoadMode(goanalysis.LoadModeTypesInfo) -+} -+ -+func analyzersFromSettings(settings *config.IfaceSettings) []*analysis.Analyzer { -+ if settings == nil || len(settings.Enable) == 0 { -+ return allAnalyzers -+ } -+ -+ enabledNames := uniqueNames(settings.Enable) -+ -+ var analyzers []*analysis.Analyzer -+ -+ for _, a := range allAnalyzers { -+ found := slices.ContainsFunc(enabledNames, func(name string) bool { -+ return name == a.Name -+ }) -+ -+ if !found { -+ continue -+ } -+ -+ analyzers = append(analyzers, a) -+ } -+ -+ return analyzers -+} -+ -+func uniqueNames(names []string) []string { -+ if len(names) == 0 { -+ return nil -+ } -+ -+ namesMap := map[string]struct{}{} -+ for _, name := range names { -+ namesMap[name] = struct{}{} -+ } -+ -+ uniqueNames := make([]string, 0, len(namesMap)) -+ -+ for name := range namesMap { -+ uniqueNames = append(uniqueNames, name) -+ } -+ return uniqueNames -+} -diff --git a/pkg/golinters/iface/iface_integration_test.go b/pkg/golinters/iface/iface_integration_test.go -new file mode 100644 -index 00000000..01d587c8 ---- /dev/null -+++ b/pkg/golinters/iface/iface_integration_test.go -@@ -0,0 +1,11 @@ -+package iface -+ -+import ( -+ "testing" -+ -+ "github.com/golangci/golangci-lint/test/testshared/integration" -+) -+ -+func TestFromTestdata(t *testing.T) { -+ integration.RunTestdata(t) -+} -diff --git a/pkg/golinters/iface/iface_test.go b/pkg/golinters/iface/iface_test.go -new file mode 100644 -index 00000000..9a6d4432 ---- /dev/null -+++ b/pkg/golinters/iface/iface_test.go -@@ -0,0 +1,61 @@ -+package iface -+ -+import ( -+ "testing" -+ -+ "github.com/golangci/golangci-lint/pkg/config" -+) -+ -+func TestAnalyzersFromSettings(t *testing.T) { -+ testCases := map[string]struct { -+ enable []string -+ expectedEnabled []string -+ }{ -+ "nil analyzers": { -+ enable: nil, -+ expectedEnabled: []string{"unused", "empty", "duplicate", "opaque"}, -+ }, -+ "empty analyzers": { -+ enable: []string{}, -+ expectedEnabled: []string{"unused", "empty", "duplicate", "opaque"}, -+ }, -+ "unused only": { -+ enable: []string{"unused"}, -+ expectedEnabled: []string{"unused"}, -+ }, -+ "some analyzers": { -+ enable: []string{"unused", "opaque"}, -+ expectedEnabled: []string{"unused", "opaque"}, -+ }, -+ "duplicate analyzers": { -+ enable: []string{"unused", "opaque", "unused"}, -+ expectedEnabled: []string{"unused", "opaque"}, -+ }, -+ "all analyzers": { -+ enable: []string{"unused", "opaque", "empty", "duplicate"}, -+ expectedEnabled: []string{"unused", "empty", "duplicate", "opaque"}, -+ }, -+ } -+ -+ for name, tc := range testCases { -+ t.Run(name, func(t *testing.T) { -+ settings := &config.IfaceSettings{Enable: tc.enable} -+ analyzers := analyzersFromSettings(settings) -+ -+ if len(analyzers) != len(tc.expectedEnabled) { -+ t.Errorf("expected %d analyzers, got %d", len(tc.enable), len(analyzers)) -+ } -+ -+ LoopSettings: -+ for _, a := range analyzers { -+ for _, name := range tc.expectedEnabled { -+ if a.Name == name { -+ continue LoopSettings -+ } -+ } -+ -+ t.Errorf("unexpected analyzer %q", a.Name) -+ } -+ }) -+ } -+} -diff --git a/pkg/golinters/iface/testdata/duplicate.go b/pkg/golinters/iface/testdata/duplicate.go -new file mode 100644 -index 00000000..4a24804b ---- /dev/null -+++ b/pkg/golinters/iface/testdata/duplicate.go -@@ -0,0 +1,11 @@ -+//golangcitest:args -Eiface -+//golangcitest:config_path testdata/duplicate.yml -+package testdata -+ -+type Pinger interface { // want "interface Pinger contains duplicate methods or type constraints from another interface, causing redundancy" -+ Ping() error -+} -+ -+type Healthcheck interface { // want "interface Healthcheck contains duplicate methods or type constraints from another interface, causing redundancy" -+ Ping() error -+} -diff --git a/pkg/golinters/iface/testdata/duplicate.yml b/pkg/golinters/iface/testdata/duplicate.yml -new file mode 100644 -index 00000000..1ac2f360 ---- /dev/null -+++ b/pkg/golinters/iface/testdata/duplicate.yml -@@ -0,0 +1,4 @@ -+linters-settings: -+ iface: -+ enable: -+ - duplicate -diff --git a/pkg/golinters/iface/testdata/empty.go b/pkg/golinters/iface/testdata/empty.go -new file mode 100644 -index 00000000..50ebb24d ---- /dev/null -+++ b/pkg/golinters/iface/testdata/empty.go -@@ -0,0 +1,6 @@ -+//golangcitest:args -Eiface -+//golangcitest:config_path testdata/empty.yml -+package testdata -+ -+type Entity interface { // want "interface Entity is empty, providing no meaningful behavior" -+} -diff --git a/pkg/golinters/iface/testdata/empty.yml b/pkg/golinters/iface/testdata/empty.yml -new file mode 100644 -index 00000000..7837406a ---- /dev/null -+++ b/pkg/golinters/iface/testdata/empty.yml -@@ -0,0 +1,4 @@ -+linters-settings: -+ iface: -+ enable: -+ - empty -diff --git a/pkg/golinters/iface/testdata/opaque.go b/pkg/golinters/iface/testdata/opaque.go -new file mode 100644 -index 00000000..de401dd8 ---- /dev/null -+++ b/pkg/golinters/iface/testdata/opaque.go -@@ -0,0 +1,19 @@ -+//golangcitest:args -Eiface -+//golangcitest:config_path testdata/opaque.yml -+package testdata -+ -+type Server interface { -+ Serve() error -+} -+ -+type server struct { -+ addr string -+} -+ -+func (s server) Serve() error { -+ return nil -+} -+ -+func NewServer(addr string) Server { // want "NewServer function return Server interface at the 1st result, abstract a single concrete implementation of \\*server" -+ return &server{addr: addr} -+} -diff --git a/pkg/golinters/iface/testdata/opaque.yml b/pkg/golinters/iface/testdata/opaque.yml -new file mode 100644 -index 00000000..71bd7047 ---- /dev/null -+++ b/pkg/golinters/iface/testdata/opaque.yml -@@ -0,0 +1,4 @@ -+linters-settings: -+ iface: -+ enable: -+ - opaque -diff --git a/pkg/golinters/iface/testdata/unused.go b/pkg/golinters/iface/testdata/unused.go -new file mode 100644 -index 00000000..69cc117c ---- /dev/null -+++ b/pkg/golinters/iface/testdata/unused.go -@@ -0,0 +1,35 @@ -+//golangcitest:args -Eiface -+//golangcitest:config_path testdata/unused.yml -+package testdata -+ -+type User struct { -+ ID string -+ Name string -+} -+ -+type UserRepository interface { // want "interface UserRepository is declared but not used within the package" -+ UserOf(id string) (*User, error) -+} -+ -+type UserRepositorySQL struct { -+} -+ -+func (r *UserRepositorySQL) UserOf(id string) (*User, error) { -+ return nil, nil -+} -+ -+type Granter interface { -+ Grant(permission string) error -+} -+ -+func AllowAll(g Granter) error { -+ return g.Grant("all") -+} -+ -+type Allower interface { -+ Allow(permission string) error -+} -+ -+func Allow(x interface{}) { -+ _ = x.(Allower) -+} -diff --git a/pkg/golinters/iface/testdata/unused.yml b/pkg/golinters/iface/testdata/unused.yml -new file mode 100644 -index 00000000..a0511b3d ---- /dev/null -+++ b/pkg/golinters/iface/testdata/unused.yml -@@ -0,0 +1,4 @@ -+linters-settings: -+ iface: -+ enable: -+ - unused -diff --git a/pkg/lint/lintersdb/builder_linter.go b/pkg/lint/lintersdb/builder_linter.go -index 7b83a815..893e2bdb 100644 ---- a/pkg/lint/lintersdb/builder_linter.go -+++ b/pkg/lint/lintersdb/builder_linter.go -@@ -55,6 +55,7 @@ import ( - "github.com/golangci/golangci-lint/pkg/golinters/gosmopolitan" - "github.com/golangci/golangci-lint/pkg/golinters/govet" - "github.com/golangci/golangci-lint/pkg/golinters/grouper" -+ "github.com/golangci/golangci-lint/pkg/golinters/iface" - "github.com/golangci/golangci-lint/pkg/golinters/importas" - "github.com/golangci/golangci-lint/pkg/golinters/inamedparam" - "github.com/golangci/golangci-lint/pkg/golinters/ineffassign" -@@ -470,6 +471,12 @@ func (LinterBuilder) Build(cfg *config.Config) ([]*linter.Config, error) { - WithPresets(linter.PresetStyle). - WithURL("https://github.com/leonklingele/grouper"), - -+ linter.NewConfig(iface.New(&cfg.LintersSettings.Iface)). -+ WithSince("v1.60.0"). -+ WithLoadForGoAnalysis(). -+ WithPresets(linter.PresetStyle, linter.PresetMetaLinter). -+ WithURL("http://github.com/uudashr/iface"), -+ - linter.NewConfig(linter.NewNoopDeprecated("ifshort", cfg, linter.DeprecationError)). - WithSince("v1.36.0"). - WithPresets(linter.PresetStyle). From 76a37b657e9db18830ffeb51e311b7b18a7ea318 Mon Sep 17 00:00:00 2001 From: Fernandez Ludovic Date: Wed, 18 Sep 2024 14:13:55 +0200 Subject: [PATCH 09/14] review --- pkg/config/linters_settings.go | 5 ----- pkg/golinters/iface/iface.go | 5 +++-- 2 files changed, 3 insertions(+), 7 deletions(-) diff --git a/pkg/config/linters_settings.go b/pkg/config/linters_settings.go index 750deecfef72..57e5555d54bc 100644 --- a/pkg/config/linters_settings.go +++ b/pkg/config/linters_settings.go @@ -88,11 +88,6 @@ var defaultLintersSettings = LintersSettings{ IgnoreTests: true, WatchForScripts: []string{"Han"}, }, - Iface: IfaceSettings{ - Unused: true, - Identical: true, - Opaque: true, - }, Inamedparam: INamedParamSettings{ SkipSingleParam: false, }, diff --git a/pkg/golinters/iface/iface.go b/pkg/golinters/iface/iface.go index b992e94671a1..375b13606503 100644 --- a/pkg/golinters/iface/iface.go +++ b/pkg/golinters/iface/iface.go @@ -3,12 +3,13 @@ package iface import ( "slices" - "github.com/golangci/golangci-lint/pkg/config" - "github.com/golangci/golangci-lint/pkg/goanalysis" "github.com/uudashr/iface/identical" "github.com/uudashr/iface/opaque" "github.com/uudashr/iface/unused" "golang.org/x/tools/go/analysis" + + "github.com/golangci/golangci-lint/pkg/config" + "github.com/golangci/golangci-lint/pkg/goanalysis" ) var allAnalyzers = []*analysis.Analyzer{ From 1965bf2d42caecbc4b4ea85c57948fe491db1ea6 Mon Sep 17 00:00:00 2001 From: Fernandez Ludovic Date: Thu, 19 Sep 2024 01:07:10 +0200 Subject: [PATCH 10/14] review --- .golangci.next.reference.yml | 5 +- jsonschema/golangci.next.jsonschema.json | 11 ++++ pkg/config/linters_settings.go | 3 +- pkg/golinters/iface/iface.go | 58 +++++++---------- pkg/golinters/iface/iface_test.go | 61 ----------------- pkg/golinters/iface/testdata/identical.go | 11 ---- pkg/golinters/iface/testdata/iface.go | 64 ++++++++++++++++++ .../iface/testdata/iface_identical.go | 65 +++++++++++++++++++ .../{identical.yml => iface_identical.yml} | 0 pkg/golinters/iface/testdata/iface_opaque.go | 65 +++++++++++++++++++ .../testdata/{opaque.yml => iface_opaque.yml} | 0 pkg/golinters/iface/testdata/iface_unused.go | 65 +++++++++++++++++++ .../testdata/{unused.yml => iface_unused.yml} | 0 .../{unused.go => iface_unused_settings.go} | 35 +++++++++- .../iface/testdata/iface_unused_settings.yml | 8 +++ pkg/golinters/iface/testdata/opaque.go | 19 ------ pkg/lint/lintersdb/builder_linter.go | 12 ++-- 17 files changed, 344 insertions(+), 138 deletions(-) delete mode 100644 pkg/golinters/iface/iface_test.go delete mode 100644 pkg/golinters/iface/testdata/identical.go create mode 100644 pkg/golinters/iface/testdata/iface.go create mode 100644 pkg/golinters/iface/testdata/iface_identical.go rename pkg/golinters/iface/testdata/{identical.yml => iface_identical.yml} (100%) create mode 100644 pkg/golinters/iface/testdata/iface_opaque.go rename pkg/golinters/iface/testdata/{opaque.yml => iface_opaque.yml} (100%) create mode 100644 pkg/golinters/iface/testdata/iface_unused.go rename pkg/golinters/iface/testdata/{unused.yml => iface_unused.yml} (100%) rename pkg/golinters/iface/testdata/{unused.go => iface_unused_settings.go} (51%) create mode 100644 pkg/golinters/iface/testdata/iface_unused_settings.yml delete mode 100644 pkg/golinters/iface/testdata/opaque.go diff --git a/.golangci.next.reference.yml b/.golangci.next.reference.yml index f20874515ede..7be05e74d9da 100644 --- a/.golangci.next.reference.yml +++ b/.golangci.next.reference.yml @@ -1223,7 +1223,8 @@ linters-settings: var-require-grouping: true iface: - # By default set to empty. Leave it empty means all analyzers are enabled. + # By default, set to empty. + # Empty means that all analyzers are enabled. # Default: [] enable: - unused @@ -1231,7 +1232,7 @@ linters-settings: - opaque settings: unused: - # Comma-separated list of packages to exclude from the check. + # List of packages path to exclude from the check. # Default: [] exclude: - github.com/example/log diff --git a/jsonschema/golangci.next.jsonschema.json b/jsonschema/golangci.next.jsonschema.json index 994039bb5265..2681b85662ff 100644 --- a/jsonschema/golangci.next.jsonschema.json +++ b/jsonschema/golangci.next.jsonschema.json @@ -1928,6 +1928,17 @@ "items": { "$ref": "#/definitions/iface-analyzers" } + }, + "settings": { + "type": "object", + "propertyNames": { + "$ref": "#/definitions/iface-analyzers" + }, + "patternProperties": { + "^.*$": { + "type": "object" + } + } } } }, diff --git a/pkg/config/linters_settings.go b/pkg/config/linters_settings.go index 57e5555d54bc..cdc2eefd57f6 100644 --- a/pkg/config/linters_settings.go +++ b/pkg/config/linters_settings.go @@ -650,7 +650,8 @@ type GrouperSettings struct { } type IfaceSettings struct { - Enable []string `mapstructure:"enable"` + Enable []string `mapstructure:"enable"` + Settings map[string]map[string]any `mapstructure:"settings"` } type ImportAsSettings struct { diff --git a/pkg/golinters/iface/iface.go b/pkg/golinters/iface/iface.go index 375b13606503..011532219f86 100644 --- a/pkg/golinters/iface/iface.go +++ b/pkg/golinters/iface/iface.go @@ -2,73 +2,59 @@ package iface import ( "slices" + "strings" "github.com/uudashr/iface/identical" "github.com/uudashr/iface/opaque" "github.com/uudashr/iface/unused" + "golang.org/x/exp/maps" "golang.org/x/tools/go/analysis" "github.com/golangci/golangci-lint/pkg/config" "github.com/golangci/golangci-lint/pkg/goanalysis" ) -var allAnalyzers = []*analysis.Analyzer{ - unused.Analyzer, - identical.Analyzer, - opaque.Analyzer, -} - func New(settings *config.IfaceSettings) *goanalysis.Linter { var conf map[string]map[string]any - - analyzers := analyzersFromSettings(settings) + if settings != nil { + conf = settings.Settings + } return goanalysis.NewLinter( "iface", "Detect the incorrect use of interfaces, helping developers avoid interface pollution.", - analyzers, + analyzersFromSettings(settings), conf, ).WithLoadMode(goanalysis.LoadModeTypesInfo) } func analyzersFromSettings(settings *config.IfaceSettings) []*analysis.Analyzer { - if settings == nil || len(settings.Enable) == 0 { - return allAnalyzers + allAnalyzers := map[string]*analysis.Analyzer{ + "unused": unused.Analyzer, + "identical": identical.Analyzer, + "opaque": opaque.Analyzer, } - enabledNames := uniqueNames(settings.Enable) - - var analyzers []*analysis.Analyzer + if settings == nil || len(settings.Enable) == 0 { + analyzers := maps.Values(allAnalyzers) - for _, a := range allAnalyzers { - found := slices.ContainsFunc(enabledNames, func(name string) bool { - return name == a.Name + // To have a deterministic order. + slices.SortFunc(analyzers, func(a *analysis.Analyzer, b *analysis.Analyzer) int { + return strings.Compare(a.Name, b.Name) }) - if !found { - continue - } + return analyzers + } - analyzers = append(analyzers, a) + var analyzers []*analysis.Analyzer + for _, name := range uniqueNames(settings.Enable) { + analyzers = append(analyzers, allAnalyzers[name]) } return analyzers } func uniqueNames(names []string) []string { - if len(names) == 0 { - return nil - } - - namesMap := map[string]struct{}{} - for _, name := range names { - namesMap[name] = struct{}{} - } - - uniqueNames := make([]string, 0, len(namesMap)) - - for name := range namesMap { - uniqueNames = append(uniqueNames, name) - } - return uniqueNames + slices.Sort(names) + return slices.Compact(names) } diff --git a/pkg/golinters/iface/iface_test.go b/pkg/golinters/iface/iface_test.go deleted file mode 100644 index 95cb71c98c04..000000000000 --- a/pkg/golinters/iface/iface_test.go +++ /dev/null @@ -1,61 +0,0 @@ -package iface - -import ( - "testing" - - "github.com/golangci/golangci-lint/pkg/config" -) - -func TestAnalyzersFromSettings(t *testing.T) { - testCases := map[string]struct { - enable []string - expectedEnabled []string - }{ - "nil analyzers": { - enable: nil, - expectedEnabled: []string{"unused", "identical", "opaque"}, - }, - "empty analyzers": { - enable: []string{}, - expectedEnabled: []string{"unused", "identical", "opaque"}, - }, - "unused only": { - enable: []string{"unused"}, - expectedEnabled: []string{"unused"}, - }, - "some analyzers": { - enable: []string{"unused", "opaque"}, - expectedEnabled: []string{"unused", "opaque"}, - }, - "duplicate analyzers": { - enable: []string{"unused", "opaque", "unused"}, - expectedEnabled: []string{"unused", "opaque"}, - }, - "all analyzers": { - enable: []string{"unused", "opaque", "identical"}, - expectedEnabled: []string{"unused", "identical", "opaque"}, - }, - } - - for name, tc := range testCases { - t.Run(name, func(t *testing.T) { - settings := &config.IfaceSettings{Enable: tc.enable} - analyzers := analyzersFromSettings(settings) - - if len(analyzers) != len(tc.expectedEnabled) { - t.Errorf("expected %d analyzers, got %d", len(tc.enable), len(analyzers)) - } - - LoopSettings: - for _, a := range analyzers { - for _, name := range tc.expectedEnabled { - if a.Name == name { - continue LoopSettings - } - } - - t.Errorf("unexpected analyzer %q", a.Name) - } - }) - } -} diff --git a/pkg/golinters/iface/testdata/identical.go b/pkg/golinters/iface/testdata/identical.go deleted file mode 100644 index f13e756aaecf..000000000000 --- a/pkg/golinters/iface/testdata/identical.go +++ /dev/null @@ -1,11 +0,0 @@ -//golangcitest:args -Eiface -//golangcitest:config_path testdata/identical.yml -package testdata - -type Pinger interface { // want "interface Pinger contains identical methods or type constraints from another interface, causing redundancy" - Ping() error -} - -type Healthcheck interface { // want "interface Healthcheck contains identical methods or type constraints from another interface, causing redundancy" - Ping() error -} diff --git a/pkg/golinters/iface/testdata/iface.go b/pkg/golinters/iface/testdata/iface.go new file mode 100644 index 000000000000..d9867d887bae --- /dev/null +++ b/pkg/golinters/iface/testdata/iface.go @@ -0,0 +1,64 @@ +//golangcitest:args -Eiface +package testdata + +// identical + +type Pinger interface { // want "identical: interface Pinger contains identical methods or type constraints from another interface, causing redundancy" + Ping() error +} + +type Healthcheck interface { // want "identical: interface Healthcheck contains identical methods or type constraints from another interface, causing redundancy" + Ping() error +} + +// opaque + +type Server interface { + Serve() error +} + +type server struct { + addr string +} + +func (s server) Serve() error { + return nil +} + +func NewServer(addr string) Server { // want "opaque: NewServer function return Server interface at the 1st result, abstract a single concrete implementation of \\*server" + return &server{addr: addr} +} + +// unused + +type User struct { + ID string + Name string +} + +type UserRepository interface { // want "unused: interface UserRepository is declared but not used within the package" + UserOf(id string) (*User, error) +} + +type UserRepositorySQL struct { +} + +func (r *UserRepositorySQL) UserOf(id string) (*User, error) { + return nil, nil +} + +type Granter interface { + Grant(permission string) error +} + +func AllowAll(g Granter) error { + return g.Grant("all") +} + +type Allower interface { + Allow(permission string) error +} + +func Allow(x interface{}) { + _ = x.(Allower) +} diff --git a/pkg/golinters/iface/testdata/iface_identical.go b/pkg/golinters/iface/testdata/iface_identical.go new file mode 100644 index 000000000000..316b35ac2bf9 --- /dev/null +++ b/pkg/golinters/iface/testdata/iface_identical.go @@ -0,0 +1,65 @@ +//golangcitest:args -Eiface +//golangcitest:config_path testdata/iface_identical.yml +package testdata + +// identical + +type Pinger interface { // want "identical: interface Pinger contains identical methods or type constraints from another interface, causing redundancy" + Ping() error +} + +type Healthcheck interface { // want "identical: interface Healthcheck contains identical methods or type constraints from another interface, causing redundancy" + Ping() error +} + +// opaque + +type Server interface { + Serve() error +} + +type server struct { + addr string +} + +func (s server) Serve() error { + return nil +} + +func NewServer(addr string) Server { + return &server{addr: addr} +} + +// unused + +type User struct { + ID string + Name string +} + +type UserRepository interface { + UserOf(id string) (*User, error) +} + +type UserRepositorySQL struct { +} + +func (r *UserRepositorySQL) UserOf(id string) (*User, error) { + return nil, nil +} + +type Granter interface { + Grant(permission string) error +} + +func AllowAll(g Granter) error { + return g.Grant("all") +} + +type Allower interface { + Allow(permission string) error +} + +func Allow(x interface{}) { + _ = x.(Allower) +} diff --git a/pkg/golinters/iface/testdata/identical.yml b/pkg/golinters/iface/testdata/iface_identical.yml similarity index 100% rename from pkg/golinters/iface/testdata/identical.yml rename to pkg/golinters/iface/testdata/iface_identical.yml diff --git a/pkg/golinters/iface/testdata/iface_opaque.go b/pkg/golinters/iface/testdata/iface_opaque.go new file mode 100644 index 000000000000..3f3e632a12ec --- /dev/null +++ b/pkg/golinters/iface/testdata/iface_opaque.go @@ -0,0 +1,65 @@ +//golangcitest:args -Eiface +//golangcitest:config_path testdata/iface_opaque.yml +package testdata + +// identical + +type Pinger interface { + Ping() error +} + +type Healthcheck interface { + Ping() error +} + +// opaque + +type Server interface { + Serve() error +} + +type server struct { + addr string +} + +func (s server) Serve() error { + return nil +} + +func NewServer(addr string) Server { // want "opaque: NewServer function return Server interface at the 1st result, abstract a single concrete implementation of \\*server" + return &server{addr: addr} +} + +// unused + +type User struct { + ID string + Name string +} + +type UserRepository interface { + UserOf(id string) (*User, error) +} + +type UserRepositorySQL struct { +} + +func (r *UserRepositorySQL) UserOf(id string) (*User, error) { + return nil, nil +} + +type Granter interface { + Grant(permission string) error +} + +func AllowAll(g Granter) error { + return g.Grant("all") +} + +type Allower interface { + Allow(permission string) error +} + +func Allow(x interface{}) { + _ = x.(Allower) +} diff --git a/pkg/golinters/iface/testdata/opaque.yml b/pkg/golinters/iface/testdata/iface_opaque.yml similarity index 100% rename from pkg/golinters/iface/testdata/opaque.yml rename to pkg/golinters/iface/testdata/iface_opaque.yml diff --git a/pkg/golinters/iface/testdata/iface_unused.go b/pkg/golinters/iface/testdata/iface_unused.go new file mode 100644 index 000000000000..db3f94af10a3 --- /dev/null +++ b/pkg/golinters/iface/testdata/iface_unused.go @@ -0,0 +1,65 @@ +//golangcitest:args -Eiface +//golangcitest:config_path testdata/iface_unused.yml +package testdata + +// identical + +type Pinger interface { // want "unused: interface Pinger is declared but not used within the package" + Ping() error +} + +type Healthcheck interface { // want "unused: interface Healthcheck is declared but not used within the package" + Ping() error +} + +// opaque + +type Server interface { + Serve() error +} + +type server struct { + addr string +} + +func (s server) Serve() error { + return nil +} + +func NewServer(addr string) Server { + return &server{addr: addr} +} + +// unused + +type User struct { + ID string + Name string +} + +type UserRepository interface { // want "unused: interface UserRepository is declared but not used within the package" + UserOf(id string) (*User, error) +} + +type UserRepositorySQL struct { +} + +func (r *UserRepositorySQL) UserOf(id string) (*User, error) { + return nil, nil +} + +type Granter interface { + Grant(permission string) error +} + +func AllowAll(g Granter) error { + return g.Grant("all") +} + +type Allower interface { + Allow(permission string) error +} + +func Allow(x interface{}) { + _ = x.(Allower) +} diff --git a/pkg/golinters/iface/testdata/unused.yml b/pkg/golinters/iface/testdata/iface_unused.yml similarity index 100% rename from pkg/golinters/iface/testdata/unused.yml rename to pkg/golinters/iface/testdata/iface_unused.yml diff --git a/pkg/golinters/iface/testdata/unused.go b/pkg/golinters/iface/testdata/iface_unused_settings.go similarity index 51% rename from pkg/golinters/iface/testdata/unused.go rename to pkg/golinters/iface/testdata/iface_unused_settings.go index 69cc117cfd3a..7c8b4491dd8c 100644 --- a/pkg/golinters/iface/testdata/unused.go +++ b/pkg/golinters/iface/testdata/iface_unused_settings.go @@ -1,13 +1,44 @@ //golangcitest:args -Eiface -//golangcitest:config_path testdata/unused.yml +//golangcitest:config_path testdata/iface_unused_settings.yml +//golangcitest:expected_exitcode 0 package testdata +// identical + +type Pinger interface { + Ping() error +} + +type Healthcheck interface { + Ping() error +} + +// opaque + +type Server interface { + Serve() error +} + +type server struct { + addr string +} + +func (s server) Serve() error { + return nil +} + +func NewServer(addr string) Server { + return &server{addr: addr} +} + +// unused + type User struct { ID string Name string } -type UserRepository interface { // want "interface UserRepository is declared but not used within the package" +type UserRepository interface { UserOf(id string) (*User, error) } diff --git a/pkg/golinters/iface/testdata/iface_unused_settings.yml b/pkg/golinters/iface/testdata/iface_unused_settings.yml new file mode 100644 index 000000000000..8e885bf188e1 --- /dev/null +++ b/pkg/golinters/iface/testdata/iface_unused_settings.yml @@ -0,0 +1,8 @@ +linters-settings: + iface: + enable: + - unused + settings: + unused: + exclude: + - "command-line-arguments" # fake package name used for test inside golangci-lint diff --git a/pkg/golinters/iface/testdata/opaque.go b/pkg/golinters/iface/testdata/opaque.go deleted file mode 100644 index de401dd8d526..000000000000 --- a/pkg/golinters/iface/testdata/opaque.go +++ /dev/null @@ -1,19 +0,0 @@ -//golangcitest:args -Eiface -//golangcitest:config_path testdata/opaque.yml -package testdata - -type Server interface { - Serve() error -} - -type server struct { - addr string -} - -func (s server) Serve() error { - return nil -} - -func NewServer(addr string) Server { // want "NewServer function return Server interface at the 1st result, abstract a single concrete implementation of \\*server" - return &server{addr: addr} -} diff --git a/pkg/lint/lintersdb/builder_linter.go b/pkg/lint/lintersdb/builder_linter.go index 5856ff6a5743..236295c87895 100644 --- a/pkg/lint/lintersdb/builder_linter.go +++ b/pkg/lint/lintersdb/builder_linter.go @@ -473,18 +473,18 @@ func (LinterBuilder) Build(cfg *config.Config) ([]*linter.Config, error) { WithPresets(linter.PresetStyle). WithURL("https://github.com/leonklingele/grouper"), - linter.NewConfig(iface.New(&cfg.LintersSettings.Iface)). - WithSince("v1.60.0"). - WithLoadForGoAnalysis(). - WithPresets(linter.PresetStyle, linter.PresetMetaLinter). - WithURL("http://github.com/uudashr/iface"), - linter.NewConfig(linter.NewNoopDeprecated("ifshort", cfg, linter.DeprecationError)). WithSince("v1.36.0"). WithPresets(linter.PresetStyle). WithURL("https://github.com/esimonov/ifshort"). DeprecatedError("The repository of the linter has been deprecated by the owner.", "v1.48.0", ""), + linter.NewConfig(iface.New(&cfg.LintersSettings.Iface)). + WithSince("v1.62.0"). + WithLoadForGoAnalysis(). + WithPresets(linter.PresetStyle, linter.PresetMetaLinter). + WithURL("https://github.com/uudashr/iface"), + linter.NewConfig(importas.New(&cfg.LintersSettings.ImportAs)). WithSince("v1.38.0"). WithPresets(linter.PresetStyle). From 682c7babbcf9c7fee143ad77df6290ba4e776101 Mon Sep 17 00:00:00 2001 From: Fernandez Ludovic Date: Mon, 23 Sep 2024 13:19:41 +0200 Subject: [PATCH 11/14] review --- pkg/lint/lintersdb/builder_linter.go | 2 +- 1 file changed, 1 insertion(+), 1 deletion(-) diff --git a/pkg/lint/lintersdb/builder_linter.go b/pkg/lint/lintersdb/builder_linter.go index 236295c87895..f5290fa6842e 100644 --- a/pkg/lint/lintersdb/builder_linter.go +++ b/pkg/lint/lintersdb/builder_linter.go @@ -482,7 +482,7 @@ func (LinterBuilder) Build(cfg *config.Config) ([]*linter.Config, error) { linter.NewConfig(iface.New(&cfg.LintersSettings.Iface)). WithSince("v1.62.0"). WithLoadForGoAnalysis(). - WithPresets(linter.PresetStyle, linter.PresetMetaLinter). + WithPresets(linter.PresetStyle). WithURL("https://github.com/uudashr/iface"), linter.NewConfig(importas.New(&cfg.LintersSettings.ImportAs)). From 8c5f0a1f90b97443751413d59b7d52333402868b Mon Sep 17 00:00:00 2001 From: Nuruddin Ashr Date: Tue, 24 Sep 2024 13:29:01 +0700 Subject: [PATCH 12/14] Enable 'identical' by default, disable the rest --- .golangci.next.reference.yml | 7 +++---- jsonschema/golangci.next.jsonschema.json | 2 +- pkg/golinters/iface/iface.go | 19 ++++++++----------- .../iface/testdata/{iface.go => iface_all.go} | 1 + pkg/golinters/iface/testdata/iface_all.yml | 6 ++++++ ...ce_unused_settings.go => iface_default.go} | 6 ++---- .../iface/testdata/iface_unused_settings.yml | 8 -------- 7 files changed, 21 insertions(+), 28 deletions(-) rename pkg/golinters/iface/testdata/{iface.go => iface_all.go} (96%) create mode 100644 pkg/golinters/iface/testdata/iface_all.yml rename pkg/golinters/iface/testdata/{iface_unused_settings.go => iface_default.go} (71%) delete mode 100644 pkg/golinters/iface/testdata/iface_unused_settings.yml diff --git a/.golangci.next.reference.yml b/.golangci.next.reference.yml index 7be05e74d9da..6989ec4d8e1c 100644 --- a/.golangci.next.reference.yml +++ b/.golangci.next.reference.yml @@ -1223,12 +1223,11 @@ linters-settings: var-require-grouping: true iface: - # By default, set to empty. - # Empty means that all analyzers are enabled. - # Default: [] + # By default is set to 'identical' only. + # Default: [identical] enable: - - unused - identical + - unused - opaque settings: unused: diff --git a/jsonschema/golangci.next.jsonschema.json b/jsonschema/golangci.next.jsonschema.json index 2681b85662ff..992d6cd9d77f 100644 --- a/jsonschema/golangci.next.jsonschema.json +++ b/jsonschema/golangci.next.jsonschema.json @@ -295,8 +295,8 @@ }, "iface-analyzers": { "enum": [ - "unused", "identical", + "unused", "opaque" ] }, diff --git a/pkg/golinters/iface/iface.go b/pkg/golinters/iface/iface.go index 011532219f86..31f88160eafd 100644 --- a/pkg/golinters/iface/iface.go +++ b/pkg/golinters/iface/iface.go @@ -2,12 +2,10 @@ package iface import ( "slices" - "strings" "github.com/uudashr/iface/identical" "github.com/uudashr/iface/opaque" "github.com/uudashr/iface/unused" - "golang.org/x/exp/maps" "golang.org/x/tools/go/analysis" "github.com/golangci/golangci-lint/pkg/config" @@ -30,24 +28,23 @@ func New(settings *config.IfaceSettings) *goanalysis.Linter { func analyzersFromSettings(settings *config.IfaceSettings) []*analysis.Analyzer { allAnalyzers := map[string]*analysis.Analyzer{ - "unused": unused.Analyzer, "identical": identical.Analyzer, + "unused": unused.Analyzer, "opaque": opaque.Analyzer, } if settings == nil || len(settings.Enable) == 0 { - analyzers := maps.Values(allAnalyzers) - - // To have a deterministic order. - slices.SortFunc(analyzers, func(a *analysis.Analyzer, b *analysis.Analyzer) int { - return strings.Compare(a.Name, b.Name) - }) - - return analyzers + // Default enable `identical` analyzer only + return []*analysis.Analyzer{identical.Analyzer} } var analyzers []*analysis.Analyzer for _, name := range uniqueNames(settings.Enable) { + if _, ok := allAnalyzers[name]; !ok { + // skip unknown analyzer + continue + } + analyzers = append(analyzers, allAnalyzers[name]) } diff --git a/pkg/golinters/iface/testdata/iface.go b/pkg/golinters/iface/testdata/iface_all.go similarity index 96% rename from pkg/golinters/iface/testdata/iface.go rename to pkg/golinters/iface/testdata/iface_all.go index d9867d887bae..f3d70b80f887 100644 --- a/pkg/golinters/iface/testdata/iface.go +++ b/pkg/golinters/iface/testdata/iface_all.go @@ -1,4 +1,5 @@ //golangcitest:args -Eiface +//golangcitest:config_path testdata/iface_all.yml package testdata // identical diff --git a/pkg/golinters/iface/testdata/iface_all.yml b/pkg/golinters/iface/testdata/iface_all.yml new file mode 100644 index 000000000000..e00cb8970659 --- /dev/null +++ b/pkg/golinters/iface/testdata/iface_all.yml @@ -0,0 +1,6 @@ +linters-settings: + iface: + enable: + - unused + - identical + - opaque diff --git a/pkg/golinters/iface/testdata/iface_unused_settings.go b/pkg/golinters/iface/testdata/iface_default.go similarity index 71% rename from pkg/golinters/iface/testdata/iface_unused_settings.go rename to pkg/golinters/iface/testdata/iface_default.go index 7c8b4491dd8c..e576369c9df7 100644 --- a/pkg/golinters/iface/testdata/iface_unused_settings.go +++ b/pkg/golinters/iface/testdata/iface_default.go @@ -1,15 +1,13 @@ //golangcitest:args -Eiface -//golangcitest:config_path testdata/iface_unused_settings.yml -//golangcitest:expected_exitcode 0 package testdata // identical -type Pinger interface { +type Pinger interface { // want "identical: interface Pinger contains identical methods or type constraints from another interface, causing redundancy" Ping() error } -type Healthcheck interface { +type Healthcheck interface { // want "identical: interface Healthcheck contains identical methods or type constraints from another interface, causing redundancy" Ping() error } diff --git a/pkg/golinters/iface/testdata/iface_unused_settings.yml b/pkg/golinters/iface/testdata/iface_unused_settings.yml deleted file mode 100644 index 8e885bf188e1..000000000000 --- a/pkg/golinters/iface/testdata/iface_unused_settings.yml +++ /dev/null @@ -1,8 +0,0 @@ -linters-settings: - iface: - enable: - - unused - settings: - unused: - exclude: - - "command-line-arguments" # fake package name used for test inside golangci-lint From e4f10d28a546ec99c202935a60033d1f8d6bd269 Mon Sep 17 00:00:00 2001 From: Fernandez Ludovic Date: Tue, 22 Oct 2024 21:28:59 +0200 Subject: [PATCH 13/14] review --- .golangci.next.reference.yml | 10 +++++----- pkg/golinters/iface/testdata/iface_all.go | 5 ++++- pkg/golinters/iface/testdata/iface_default.go | 5 ++++- pkg/golinters/iface/testdata/iface_identical.go | 5 ++++- pkg/golinters/iface/testdata/iface_opaque.go | 5 ++++- pkg/golinters/iface/testdata/iface_unused.go | 5 ++++- 6 files changed, 25 insertions(+), 10 deletions(-) diff --git a/.golangci.next.reference.yml b/.golangci.next.reference.yml index 6edf5a6d57ea..5e9afbb1eb74 100644 --- a/.golangci.next.reference.yml +++ b/.golangci.next.reference.yml @@ -1931,12 +1931,12 @@ linters-settings: var-require-grouping: true iface: - # By default is set to 'identical' only. - # Default: [identical] + # List of analyzers. + # Default: ["identical"] enable: - - identical - - unused - - opaque + - identical # Identifies interfaces in the same package that have identical method sets. + - unused # Identifies interfaces that are not used anywhere in the same package where the interface is defined. + - opaque # Identifies functions that return interfaces, but the actual returned value is always a single concrete implementation. settings: unused: # List of packages path to exclude from the check. diff --git a/pkg/golinters/iface/testdata/iface_all.go b/pkg/golinters/iface/testdata/iface_all.go index f3d70b80f887..3a6f912a1d2a 100644 --- a/pkg/golinters/iface/testdata/iface_all.go +++ b/pkg/golinters/iface/testdata/iface_all.go @@ -2,6 +2,8 @@ //golangcitest:config_path testdata/iface_all.yml package testdata +import "fmt" + // identical type Pinger interface { // want "identical: interface Pinger contains identical methods or type constraints from another interface, causing redundancy" @@ -60,6 +62,7 @@ type Allower interface { Allow(permission string) error } -func Allow(x interface{}) { +func Allow(x any) { _ = x.(Allower) + fmt.Println("allow") } diff --git a/pkg/golinters/iface/testdata/iface_default.go b/pkg/golinters/iface/testdata/iface_default.go index e576369c9df7..34117389738b 100644 --- a/pkg/golinters/iface/testdata/iface_default.go +++ b/pkg/golinters/iface/testdata/iface_default.go @@ -1,6 +1,8 @@ //golangcitest:args -Eiface package testdata +import "fmt" + // identical type Pinger interface { // want "identical: interface Pinger contains identical methods or type constraints from another interface, causing redundancy" @@ -59,6 +61,7 @@ type Allower interface { Allow(permission string) error } -func Allow(x interface{}) { +func Allow(x any) { _ = x.(Allower) + fmt.Println("allow") } diff --git a/pkg/golinters/iface/testdata/iface_identical.go b/pkg/golinters/iface/testdata/iface_identical.go index 316b35ac2bf9..821aff7bd41e 100644 --- a/pkg/golinters/iface/testdata/iface_identical.go +++ b/pkg/golinters/iface/testdata/iface_identical.go @@ -2,6 +2,8 @@ //golangcitest:config_path testdata/iface_identical.yml package testdata +import "fmt" + // identical type Pinger interface { // want "identical: interface Pinger contains identical methods or type constraints from another interface, causing redundancy" @@ -60,6 +62,7 @@ type Allower interface { Allow(permission string) error } -func Allow(x interface{}) { +func Allow(x any) { _ = x.(Allower) + fmt.Println("allow") } diff --git a/pkg/golinters/iface/testdata/iface_opaque.go b/pkg/golinters/iface/testdata/iface_opaque.go index 3f3e632a12ec..b7ebeddc600f 100644 --- a/pkg/golinters/iface/testdata/iface_opaque.go +++ b/pkg/golinters/iface/testdata/iface_opaque.go @@ -2,6 +2,8 @@ //golangcitest:config_path testdata/iface_opaque.yml package testdata +import "fmt" + // identical type Pinger interface { @@ -60,6 +62,7 @@ type Allower interface { Allow(permission string) error } -func Allow(x interface{}) { +func Allow(x any) { _ = x.(Allower) + fmt.Println("allow") } diff --git a/pkg/golinters/iface/testdata/iface_unused.go b/pkg/golinters/iface/testdata/iface_unused.go index db3f94af10a3..b75fdc8964c6 100644 --- a/pkg/golinters/iface/testdata/iface_unused.go +++ b/pkg/golinters/iface/testdata/iface_unused.go @@ -2,6 +2,8 @@ //golangcitest:config_path testdata/iface_unused.yml package testdata +import "fmt" + // identical type Pinger interface { // want "unused: interface Pinger is declared but not used within the package" @@ -60,6 +62,7 @@ type Allower interface { Allow(permission string) error } -func Allow(x interface{}) { +func Allow(x any) { _ = x.(Allower) + fmt.Println("allow") } From b65ac5294855145f24f33b0a89702dd5ba356506 Mon Sep 17 00:00:00 2001 From: Fernandez Ludovic Date: Tue, 22 Oct 2024 21:39:27 +0200 Subject: [PATCH 14/14] review: jsonschema --- jsonschema/golangci.next.jsonschema.json | 19 +++++++++++++------ 1 file changed, 13 insertions(+), 6 deletions(-) diff --git a/jsonschema/golangci.next.jsonschema.json b/jsonschema/golangci.next.jsonschema.json index 1d702c6976a8..7a012ec288df 100644 --- a/jsonschema/golangci.next.jsonschema.json +++ b/jsonschema/golangci.next.jsonschema.json @@ -1943,12 +1943,19 @@ }, "settings": { "type": "object", - "propertyNames": { - "$ref": "#/definitions/iface-analyzers" - }, - "patternProperties": { - "^.*$": { - "type": "object" + "additionalProperties": false, + "properties": { + "unused": { + "type": "object", + "additionalProperties": false, + "properties": { + "exclude": { + "type": "array", + "items": { + "type": "string" + } + } + } } } }