diff --git a/README.md b/README.md index 97aa389..f6b8e05 100644 --- a/README.md +++ b/README.md @@ -39,17 +39,37 @@ and [2022](https://www.jetbrains.com/lp/devecosystem-2022/go/#which-testing-fram ``` $ go install github.com/Antonboom/testifylint@latest $ testifylint -h -$ testifylint ./... +$ testifylint -fix ./... ``` ## Configuring +### CLI + +```bash +# Use default checkers +$ testifylint ./... + +# Enable checkers in addition to enabled by default checkers +$ testifylint --enable=require-error,empty ./... + +# Disable checkers from enabled by default checkers +$ testifylint --disable=require-error,empty ./... + +# Enable some checkers only +$ testifylint --disable-all --enable=require-error,empty ./... + +# Disable some checkers only +$ testifylint --enable-all --disable=require-error,empty ./... + +# Checker specific settings +$ testifylint --suite-extra-assert-call.mode=require ./... +$ testifylint --expected-actual.pattern=^wanted$ ./... ``` -$ testifylint --enable-all ./... -$ testifylint --enable=empty,error-is-as ./... -$ testifylint --enable=expected-actual --expected-actual.pattern=^wanted$ ./... -$ testifylint --enable=suite-extra-assert-call --suite-extra-assert-call.mode=require ./... -``` + +### golangci-lint + +https://golangci-lint.run/usage/linters/#testifylint ## Checkers diff --git a/analyzer/analyzer_test.go b/analyzer/analyzer_test.go index a0de713..70fe523 100644 --- a/analyzer/analyzer_test.go +++ b/analyzer/analyzer_test.go @@ -19,7 +19,7 @@ func TestTestifyLint(t *testing.T) { }{ { dir: "base-test", - flags: map[string]string{"enable": checkers.NewBoolCompare().Name()}, + flags: map[string]string{"disable-all": "true", "enable": checkers.NewBoolCompare().Name()}, }, { dir: "checkers-priority", @@ -27,11 +27,12 @@ func TestTestifyLint(t *testing.T) { }, { dir: "error-as-target", - flags: map[string]string{"enable": checkers.NewErrorIsAs().Name()}, + flags: map[string]string{"disable-all": "true", "enable": checkers.NewErrorIsAs().Name()}, }, { dir: "expected-var-custom-pattern", flags: map[string]string{ + "disable-all": "true", "enable": checkers.NewExpectedActual().Name(), "expected-actual.pattern": "goldenValue", }, @@ -43,6 +44,7 @@ func TestTestifyLint(t *testing.T) { { dir: "suite-require-extra-assert-call", flags: map[string]string{ + "disable-all": "true", "enable": checkers.NewSuiteExtraAssertCall().Name(), "suite-extra-assert-call.mode": "require", }, @@ -76,9 +78,13 @@ func TestTestifyLint_CheckersDefault(t *testing.T) { t.Parallel() anlzr := analyzer.New() + if err := anlzr.Flags.Set("disable-all", "true"); err != nil { + t.Fatal(err) + } if err := anlzr.Flags.Set("enable", checker); err != nil { t.Fatal(err) } + pkg := filepath.Join("checkers-default", checker) analysistest.RunWithSuggestedFixes(t, analysistest.TestData(), anlzr, pkg) }) diff --git a/analyzer/checkers_factory.go b/analyzer/checkers_factory.go index e87e35e..58b5d51 100644 --- a/analyzer/checkers_factory.go +++ b/analyzer/checkers_factory.go @@ -9,14 +9,34 @@ import ( // newCheckers accepts linter config and returns slices of enabled checkers sorted by priority. func newCheckers(cfg config.Config) ([]checkers.RegularChecker, []checkers.AdvancedChecker, error) { - enabledCheckers := cfg.EnabledCheckers - if len(enabledCheckers) == 0 { - enabledCheckers = checkers.EnabledByDefault() + if err := cfg.Validate(); err != nil { + return nil, nil, err } + + enabledCheckersSet := make(map[string]struct{}) + if cfg.EnableAll { - enabledCheckers = checkers.All() + for _, checker := range checkers.All() { + enabledCheckersSet[checker] = struct{}{} + } + } else if !cfg.DisableAll { + for _, checker := range checkers.EnabledByDefault() { + enabledCheckersSet[checker] = struct{}{} + } } + for _, checker := range cfg.EnabledCheckers { + enabledCheckersSet[checker] = struct{}{} + } + + for _, checker := range cfg.DisabledCheckers { + delete(enabledCheckersSet, checker) + } + + enabledCheckers := make([]string, 0, len(enabledCheckersSet)) + for v := range enabledCheckersSet { + enabledCheckers = append(enabledCheckers, v) + } checkers.SortByPriority(enabledCheckers) regularCheckers := make([]checkers.RegularChecker, 0, len(enabledCheckers)) diff --git a/analyzer/checkers_factory_test.go b/analyzer/checkers_factory_test.go index 2b17fe0..998748e 100644 --- a/analyzer/checkers_factory_test.go +++ b/analyzer/checkers_factory_test.go @@ -41,6 +41,11 @@ func Test_newCheckers(t *testing.T) { checkers.NewSuiteDontUsePkg(), } + enabledByDefaultAdvancedCheckers := []checkers.AdvancedChecker{} + allAdvancedCheckers := []checkers.AdvancedChecker{ + checkers.NewSuiteTHelper(), + } + cases := []struct { name string cfg config.Config @@ -51,65 +56,83 @@ func Test_newCheckers(t *testing.T) { name: "no config", cfg: config.Config{}, expRegular: enabledByDefaultRegularCheckers, - expAdvanced: []checkers.AdvancedChecker{}, + expAdvanced: enabledByDefaultAdvancedCheckers, }, { - name: "no enabled checkers", - cfg: config.Config{ - EnabledCheckers: []string{}, - }, + name: "default config", + cfg: config.NewDefault(), expRegular: enabledByDefaultRegularCheckers, - expAdvanced: []checkers.AdvancedChecker{}, + expAdvanced: enabledByDefaultAdvancedCheckers, }, { - name: "no enabled checkers but enable-all true", - cfg: config.Config{ - EnabledCheckers: []string{}, - EnableAll: true, - }, - expRegular: allRegularCheckers, - expAdvanced: []checkers.AdvancedChecker{ - checkers.NewSuiteTHelper(), - }, - }, - { - name: "enabled checkers defined", + name: "enable two checkers only", cfg: config.Config{ + DisableAll: true, EnabledCheckers: config.KnownCheckersValue{ - checkers.NewSuiteTHelper().Name(), checkers.NewRequireError().Name(), - checkers.NewSuiteExtraAssertCall().Name(), checkers.NewLen().Name(), }, }, expRegular: []checkers.RegularChecker{ checkers.NewLen(), checkers.NewRequireError(), - checkers.NewSuiteExtraAssertCall(), }, - expAdvanced: []checkers.AdvancedChecker{ - checkers.NewSuiteTHelper(), + expAdvanced: []checkers.AdvancedChecker{}, + }, + { + name: "disable two checkers only", + cfg: config.Config{ + EnableAll: true, + DisabledCheckers: config.KnownCheckersValue{ + checkers.NewRequireError().Name(), + checkers.NewSuiteTHelper().Name(), + }, }, + expRegular: filter(allRegularCheckers, config.KnownCheckersValue{ + checkers.NewRequireError().Name(), + checkers.NewSuiteTHelper().Name(), + }), + expAdvanced: filter(allAdvancedCheckers, config.KnownCheckersValue{ + checkers.NewRequireError().Name(), + checkers.NewSuiteTHelper().Name(), + }), }, { - name: "enabled checkers defined but enable-all true", + name: "enable one checker in addition to enabled by default checkers", cfg: config.Config{ EnabledCheckers: config.KnownCheckersValue{ checkers.NewSuiteTHelper().Name(), - checkers.NewRequireError().Name(), - checkers.NewSuiteExtraAssertCall().Name(), - checkers.NewLen().Name(), }, - EnableAll: true, }, expRegular: allRegularCheckers, expAdvanced: []checkers.AdvancedChecker{ checkers.NewSuiteTHelper(), }, }, + { + name: "disable three checkers from enabled by default checkers", + cfg: config.Config{ + DisabledCheckers: config.KnownCheckersValue{ + checkers.NewNilCompare().Name(), + checkers.NewErrorNil().Name(), + checkers.NewRequireError().Name(), + }, + }, + expRegular: filter(enabledByDefaultRegularCheckers, config.KnownCheckersValue{ + checkers.NewNilCompare().Name(), + checkers.NewErrorNil().Name(), + checkers.NewRequireError().Name(), + }), + expAdvanced: filter(enabledByDefaultAdvancedCheckers, config.KnownCheckersValue{ + checkers.NewNilCompare().Name(), + checkers.NewErrorNil().Name(), + checkers.NewRequireError().Name(), + }), + }, { name: "expected-actual pattern defined", cfg: config.Config{ + DisableAll: true, EnabledCheckers: config.KnownCheckersValue{checkers.NewExpectedActual().Name()}, ExpectedActual: config.ExpectedActualConfig{ ExpVarPattern: config.RegexpValue{Regexp: pattern}, @@ -123,6 +146,7 @@ func Test_newCheckers(t *testing.T) { { name: "suite-extra-assert-call mode defined", cfg: config.Config{ + DisableAll: true, EnabledCheckers: config.KnownCheckersValue{checkers.NewSuiteExtraAssertCall().Name()}, SuiteExtraAssertCall: config.SuiteExtraAssertCallConfig{ Mode: checkers.SuiteExtraAssertCallModeRequire, @@ -143,18 +167,36 @@ func Test_newCheckers(t *testing.T) { } if !reflect.DeepEqual(tt.expRegular, rc) { - t.Fatalf("unexpected regular checkers: %#v", rc) + t.Fatalf("unexpected regular checkers: %#v != %#v", rc, tt.expRegular) } if !reflect.DeepEqual(tt.expAdvanced, ac) { - t.Fatalf("unexpected expAdvanced checkers: %#v", ac) + t.Fatalf("unexpected expAdvanced checkers: %#v != %#v", ac, tt.expAdvanced) } }) } } +func Test_newCheckers_invalidConfig(t *testing.T) { + _, _, err := newCheckers(config.Config{EnableAll: true, DisableAll: true}) + if nil == err { + t.Fatal("no error but expected") + } +} + func Test_newCheckers_unknownChecker(t *testing.T) { _, _, err := newCheckers(config.Config{EnabledCheckers: config.KnownCheckersValue{"unknown"}}) if nil == err { t.Fatal("no error but expected") } } + +func filter[T checkers.Checker](in []T, exclude config.KnownCheckersValue) []T { + result := make([]T, 0) + for _, v := range in { + if exclude.Contains(v.Name()) { + continue + } + result = append(result, v) + } + return result +} diff --git a/internal/config/config.go b/internal/config/config.go index 51f6270..6b5dfee 100644 --- a/internal/config/config.go +++ b/internal/config/config.go @@ -1,7 +1,9 @@ package config import ( + "errors" "flag" + "fmt" "github.com/Antonboom/testifylint/internal/checkers" ) @@ -9,8 +11,10 @@ import ( // NewDefault builds default testifylint config. func NewDefault() Config { return Config{ - EnableAll: false, - EnabledCheckers: checkers.EnabledByDefault(), + EnableAll: false, + DisabledCheckers: nil, + DisableAll: false, + EnabledCheckers: nil, ExpectedActual: ExpectedActualConfig{ ExpVarPattern: RegexpValue{checkers.DefaultExpectedVarPattern}, }, @@ -22,8 +26,11 @@ func NewDefault() Config { // Config implements testifylint configuration. type Config struct { - EnableAll bool - EnabledCheckers KnownCheckersValue + EnableAll bool + DisabledCheckers KnownCheckersValue + DisableAll bool + EnabledCheckers KnownCheckersValue + ExpectedActual ExpectedActualConfig SuiteExtraAssertCall SuiteExtraAssertCallConfig } @@ -38,10 +45,43 @@ type SuiteExtraAssertCallConfig struct { Mode checkers.SuiteExtraAssertCallMode } +func (cfg Config) Validate() error { + if cfg.EnableAll { + if cfg.DisableAll { + return errors.New("enable-all and disable-all options must not be combined") + } + + if len(cfg.EnabledCheckers) != 0 { + return errors.New("enable-all and enable options must not be combined") + } + } + + if cfg.DisableAll { + if len(cfg.DisabledCheckers) != 0 { + return errors.New("disable-all and disable options must not be combined") + } + + if len(cfg.EnabledCheckers) == 0 { + return errors.New("all checkers were disabled, but no one checker was enabled: at least one must be enabled") + } + } + + for _, checker := range cfg.DisabledCheckers { + if cfg.EnabledCheckers.Contains(checker) { + return fmt.Errorf("checker %q disabled and enabled at one moment", checker) + } + } + + return nil +} + // BindToFlags binds Config fields to according flags. func BindToFlags(cfg *Config, fs *flag.FlagSet) { fs.BoolVar(&cfg.EnableAll, "enable-all", false, "enable all checkers") - fs.Var(&cfg.EnabledCheckers, "enable", "comma separated list of enabled checkers") + fs.Var(&cfg.DisabledCheckers, "disable", "comma separated list of disabled checkers (to exclude from enabled by default)") + fs.BoolVar(&cfg.DisableAll, "disable-all", false, "disable all checkers") + fs.Var(&cfg.EnabledCheckers, "enable", "comma separated list of enabled checkers (in addition to enabled by default)") + fs.Var(&cfg.ExpectedActual.ExpVarPattern, "expected-actual.pattern", "regexp for expected variable name") fs.Var(NewEnumValue(suiteExtraAssertCallModeAsString, &cfg.SuiteExtraAssertCall.Mode), "suite-extra-assert-call.mode", "to require or remove extra Assert() call") diff --git a/internal/config/config_test.go b/internal/config/config_test.go index 4bb75a1..9edd020 100644 --- a/internal/config/config_test.go +++ b/internal/config/config_test.go @@ -2,8 +2,6 @@ package config_test import ( "flag" - "slices" - "strings" "testing" "github.com/Antonboom/testifylint/internal/checkers" @@ -16,7 +14,13 @@ func TestNewDefault(t *testing.T) { if cfg.EnableAll { t.Fatal() } - if !slices.Equal(cfg.EnabledCheckers, checkers.EnabledByDefault()) { + if len(cfg.DisabledCheckers) != 0 { + t.Fatal() + } + if cfg.DisableAll { + t.Fatal() + } + if len(cfg.EnabledCheckers) != 0 { t.Fatal() } if cfg.ExpectedActual.ExpVarPattern.String() != checkers.DefaultExpectedVarPattern.String() { @@ -27,6 +31,109 @@ func TestNewDefault(t *testing.T) { } } +func TestConfig_Validate(t *testing.T) { + cases := []struct { + name string + cfg config.Config + wantErr bool + }{ + // Positive. + { + name: "default config", + cfg: config.NewDefault(), + wantErr: false, + }, + { + name: "enable-all and disable simultaneously", + cfg: config.Config{ + EnableAll: true, + DisabledCheckers: config.KnownCheckersValue{checkers.NewErrorNil().Name()}, + }, + wantErr: false, + }, + { + name: "disable-all and enable simultaneously", + cfg: config.Config{ + DisableAll: true, + EnabledCheckers: config.KnownCheckersValue{checkers.NewErrorNil().Name()}, + }, + wantErr: false, + }, + { + name: "enable some checkers", + cfg: config.Config{ + EnabledCheckers: config.KnownCheckersValue{checkers.NewErrorNil().Name()}, + }, + wantErr: false, + }, + { + name: "disable some checkers", + cfg: config.Config{ + DisabledCheckers: config.KnownCheckersValue{checkers.NewErrorNil().Name()}, + }, + wantErr: false, + }, + { + name: "enable and disable simultaneously different checkers", + cfg: config.Config{ + DisabledCheckers: config.KnownCheckersValue{checkers.NewRequireError().Name()}, + EnabledCheckers: config.KnownCheckersValue{checkers.NewErrorNil().Name()}, + }, + wantErr: false, + }, + + // Negative. + { + name: "enable-all and disable-all simultaneously", + cfg: config.Config{ + EnableAll: true, + DisableAll: true, + }, + wantErr: true, + }, + { + name: "enable-all and enable simultaneously", + cfg: config.Config{ + EnableAll: true, + EnabledCheckers: config.KnownCheckersValue{checkers.NewExpectedActual().Name()}, + }, + wantErr: true, + }, + { + name: "disable-all and disable simultaneously", + cfg: config.Config{ + DisableAll: true, + DisabledCheckers: config.KnownCheckersValue{checkers.NewExpectedActual().Name()}, + }, + wantErr: true, + }, + { + name: "disable-all and no enable", + cfg: config.Config{ + DisableAll: true, + }, + wantErr: true, + }, + { + name: "enable and disable simultaneously the same checker", + cfg: config.Config{ + EnabledCheckers: config.KnownCheckersValue{checkers.NewNilCompare().Name(), checkers.NewExpectedActual().Name()}, + DisabledCheckers: config.KnownCheckersValue{checkers.NewExpectedActual().Name()}, + }, + wantErr: true, + }, + } + + for _, tt := range cases { + t.Run(tt.name, func(t *testing.T) { + err := tt.cfg.Validate() + if (err != nil) != tt.wantErr { + t.Fatal(err) + } + }) + } +} + func TestBindToFlags(t *testing.T) { cfg := config.NewDefault() fs := flag.NewFlagSet("TestBindToFlags", flag.PanicOnError) @@ -35,7 +142,9 @@ func TestBindToFlags(t *testing.T) { for flagName, defaultVal := range map[string]string{ "enable-all": "false", - "enable": strings.Join(cfg.EnabledCheckers, ","), + "disable": "", + "disable-all": "false", + "enable": "", "expected-actual.pattern": cfg.ExpectedActual.ExpVarPattern.String(), "suite-extra-assert-call.mode": "remove", } { diff --git a/internal/config/flag_value_types.go b/internal/config/flag_value_types.go index 2f0ee97..5b08ec4 100644 --- a/internal/config/flag_value_types.go +++ b/internal/config/flag_value_types.go @@ -35,6 +35,15 @@ func (kcv *KnownCheckersValue) Set(v string) error { return nil } +func (kcv KnownCheckersValue) Contains(v string) bool { + for _, checker := range kcv { + if checker == v { + return true + } + } + return false +} + // RegexpValue is a special wrapper for support of flag.FlagSet over regexp.Regexp. // Original regexp is available through RegexpValue.Regexp. type RegexpValue struct { diff --git a/internal/config/flag_value_types_test.go b/internal/config/flag_value_types_test.go index 0f78325..c5d703a 100644 --- a/internal/config/flag_value_types_test.go +++ b/internal/config/flag_value_types_test.go @@ -54,9 +54,31 @@ func TestKnownCheckersValue_Set(t *testing.T) { } } -func TestRegexpValue_String_ZeroValue(t *testing.T) { - var r config.RegexpValue - if r.String() != "" { +func TestKnownCheckersValue_String(t *testing.T) { + t.Run("zero value", func(t *testing.T) { + emptyKc := config.KnownCheckersValue{}.String() + if emptyKc != "" { + t.Fatal(emptyKc) + } + }) + + t.Run("not zero value", func(t *testing.T) { + kcAsStr := (config.KnownCheckersValue{"require-error", "nil-compare"}).String() + if kcAsStr != "require-error,nil-compare" { + t.Fatal(kcAsStr) + } + }) +} + +func TestKnownCheckersValue_Contains(t *testing.T) { + kc := config.KnownCheckersValue{"require-error", "nil-compare"} + if !kc.Contains("require-error") { + t.Fatal() + } + if !kc.Contains("nil-compare") { + t.Fatal() + } + if kc.Contains("bool-compare") { t.Fatal() } } @@ -95,6 +117,22 @@ func TestRegexp_Set(t *testing.T) { } } +func TestRegexpValue_String(t *testing.T) { + t.Run("zero value", func(t *testing.T) { + var r config.RegexpValue + if v := r.String(); v != "" { + t.Fatal(v) + } + }) + + t.Run("not zero value", func(t *testing.T) { + r := config.RegexpValue{Regexp: regexp.MustCompile(`exp`)} + if v := r.String(); v != "exp" { + t.Fatal(v) + } + }) +} + func TestEnumValue(t *testing.T) { type workMode int const ( @@ -139,21 +177,23 @@ func TestEnumValue(t *testing.T) { }) } -func Test_EnumValue_String_ZeroValue(t *testing.T) { - var ev config.EnumValue[int] - if ev.String() != "" { - t.Fatal() - } -} +func Test_EnumValue_String(t *testing.T) { + t.Run("zero value", func(t *testing.T) { + var ev config.EnumValue[int] + if ev.String() != "" { + t.Fatal() + } + }) -func Test_EnumValue_String_UnknownDefault(t *testing.T) { - var business int - v := config.NewEnumValue(map[string]int{ - "b2c": 1, - "b2b": 2, - }, &business) + t.Run("unknown default", func(t *testing.T) { + var business int + v := config.NewEnumValue(map[string]int{ + "b2c": 1, + "b2b": 2, + }, &business) - if v.String() != "" { - t.Fatal() - } + if v.String() != "" { + t.Fatal() + } + }) }