diff --git a/config/config.go b/config/config.go index ec0698c64..a5bd8a0c3 100644 --- a/config/config.go +++ b/config/config.go @@ -6,6 +6,7 @@ import ( "fmt" "os" "reflect" + "slices" "strings" "github.com/BurntSushi/toml" @@ -126,6 +127,40 @@ var allRules = append([]lint.Rule{ &rule.MarshalReceiverRule{}, }, defaultRules...) +// AllRuleNames returns the sorted names of all rules registered in revive. +func AllRuleNames() []string { + return ruleNames(allRules) +} + +// DefaultRuleNames returns the sorted names of the rules that are enabled by default. +func DefaultRuleNames() []string { + return ruleNames(defaultRules) +} + +// EnabledRuleNames returns the sorted names of the rules that are enabled in the given configuration. +func EnabledRuleNames(config *lint.Config) []string { + if config == nil { + return nil + } + var names []string + for name, ruleConfig := range config.Rules { + if !ruleConfig.Disabled { + names = append(names, name) + } + } + slices.Sort(names) + return names +} + +func ruleNames(rules []lint.Rule) []string { + names := make([]string, len(rules)) + for i, r := range rules { + names[i] = r.Name() + } + slices.Sort(names) + return names +} + // allFormatters is a list of all available formatters to output the linting results. // Keep the list sorted and in sync with available formatters in README.md. var allFormatters = []lint.Formatter{ @@ -255,7 +290,9 @@ func validateConfig(config *lint.Config) error { return nil } -func normalizeConfig(config *lint.Config) { +// Normalize fills in default rule entries (according to the EnableAllRules / EnableDefaultRules options) +// and propagates the configured severity to rules and directives that don't define their own. +func Normalize(config *lint.Config) { if len(config.Rules) == 0 { config.Rules = map[string]lint.RuleConfig{} } @@ -292,14 +329,15 @@ func normalizeConfig(config *lint.Config) { } } -const defaultConfidence = 0.8 +// DefaultConfidence is the default confidence level for revive's linter. +const DefaultConfidence = 0.8 // GetConfig yields the configuration. func GetConfig(configPath string) (*lint.Config, error) { config := &lint.Config{} switch { case configPath != "": - config.Confidence = defaultConfidence + config.Confidence = DefaultConfidence data, err := os.ReadFile(configPath) //nolint:gosec // ignore G304: potential file inclusion via variable if err != nil { return nil, errors.New("cannot read the config file") @@ -310,14 +348,14 @@ func GetConfig(configPath string) (*lint.Config, error) { } default: // no configuration provided - config = defaultConfig() + config = Default() } if err := validateConfig(config); err != nil { return nil, err } - normalizeConfig(config) + Normalize(config) return config, nil } @@ -334,9 +372,10 @@ func GetFormatter(formatterName string) (lint.Formatter, error) { return f, nil } -func defaultConfig() *lint.Config { +// Default returns the default linter configuration, used when no configuration is provided. +func Default() *lint.Config { defaultConfig := lint.Config{ - Confidence: defaultConfidence, + Confidence: DefaultConfidence, Severity: lint.SeverityWarning, Rules: map[string]lint.RuleConfig{}, } diff --git a/config/config_test.go b/config/config_test.go index f359736c0..d7cc3aa6d 100644 --- a/config/config_test.go +++ b/config/config_test.go @@ -777,3 +777,155 @@ func TestGetFormatter(t *testing.T) { } }) } + +func TestAllRuleNames(t *testing.T) { + names := config.AllRuleNames() + + if len(names) == 0 { + t.Fatal("AllRuleNames returned no rules") + } + if !slices.IsSorted(names) { + t.Errorf("AllRuleNames should be sorted, got %v", names) + } + if compacted := slices.Compact(slices.Clone(names)); len(compacted) != len(names) { + t.Errorf("AllRuleNames should not contain duplicates, got %v", names) + } + for _, want := range []string{"argument-limit", "cyclomatic", "exported", "var-naming"} { + if !slices.Contains(names, want) { + t.Errorf("AllRuleNames should contain %q", want) + } + } + // Every default rule is also part of all rules. + for _, name := range config.DefaultRuleNames() { + if !slices.Contains(names, name) { + t.Errorf("AllRuleNames is missing default rule %q", name) + } + } +} + +func TestDefaultRuleNames(t *testing.T) { + names := config.DefaultRuleNames() + + if len(names) == 0 { + t.Fatal("DefaultRuleNames returned no rules") + } + if !slices.IsSorted(names) { + t.Errorf("DefaultRuleNames should be sorted, got %v", names) + } + if compacted := slices.Compact(slices.Clone(names)); len(compacted) != len(names) { + t.Errorf("DefaultRuleNames should not contain duplicates, got %v", names) + } + for _, want := range []string{"blank-imports", "exported", "var-naming"} { + if !slices.Contains(names, want) { + t.Errorf("DefaultRuleNames should contain %q", want) + } + } + // Default rules are a strict subset of all rules. + if len(names) >= len(config.AllRuleNames()) { + t.Errorf("DefaultRuleNames (%d) should be fewer than AllRuleNames (%d)", len(names), len(config.AllRuleNames())) + } +} + +func TestEnabledRuleNames(t *testing.T) { + t.Run("returns only enabled rules, sorted", func(t *testing.T) { + cfg := &lint.Config{ + Rules: lint.RulesConfig{ + "var-naming": {}, + "exported": {Disabled: true}, + "argument-limit": {}, + "cyclomatic": {Disabled: true}, + }, + } + + got := config.EnabledRuleNames(cfg) + want := []string{"argument-limit", "var-naming"} + if !slices.Equal(got, want) { + t.Errorf("EnabledRuleNames: expected %v, got %v", want, got) + } + }) + + t.Run("empty config has no enabled rules", func(t *testing.T) { + if got := config.EnabledRuleNames(&lint.Config{}); len(got) != 0 { + t.Errorf("EnabledRuleNames: expected none, got %v", got) + } + }) + + t.Run("nil config has no enabled rules", func(t *testing.T) { + if got := config.EnabledRuleNames(nil); len(got) != 0 { + t.Errorf("EnabledRuleNames: expected none, got %v", got) + } + }) +} + +func TestDefaultConfidence(t *testing.T) { + if config.DefaultConfidence != 0.8 { + t.Errorf("DefaultConfidence: expected 0.8, got %v", config.DefaultConfidence) + } +} + +func TestDefault(t *testing.T) { + cfg := config.Default() + + if cfg.Confidence != config.DefaultConfidence { + t.Errorf("Confidence: expected %v, got %v", config.DefaultConfidence, cfg.Confidence) + } + if cfg.Severity != lint.SeverityWarning { + t.Errorf("Severity: expected %v, got %v", lint.SeverityWarning, cfg.Severity) + } + + var ruleNames []string + for name := range cfg.Rules { + ruleNames = append(ruleNames, name) + } + slices.Sort(ruleNames) + if want := config.DefaultRuleNames(); !slices.Equal(ruleNames, want) { + t.Errorf("Default config rules: expected %v, got %v", want, ruleNames) + } +} + +func TestNormalize(t *testing.T) { + t.Run("enable-default-rules adds default rule entries", func(t *testing.T) { + cfg := &lint.Config{EnableDefaultRules: true} + + config.Normalize(cfg) + + if got := config.EnabledRuleNames(cfg); !slices.Equal(got, config.DefaultRuleNames()) { + t.Errorf("expected default rules %v, got %v", config.DefaultRuleNames(), got) + } + }) + + t.Run("severity is propagated to rules and directives without their own", func(t *testing.T) { + cfg := &lint.Config{ + Severity: lint.SeverityError, + Rules: lint.RulesConfig{ + "exported": {}, + "var-naming": {Severity: lint.SeverityWarning}, + }, + Directives: lint.DirectivesConfig{ + "specify-disable-reason": {}, + }, + } + + config.Normalize(cfg) + + if got := cfg.Rules["exported"].Severity; got != lint.SeverityError { + t.Errorf("exported severity: expected %q, got %q", lint.SeverityError, got) + } + if got := cfg.Rules["var-naming"].Severity; got != lint.SeverityWarning { + t.Errorf("var-naming severity should be preserved: expected %q, got %q", lint.SeverityWarning, got) + } + if got := cfg.Directives["specify-disable-reason"].Severity; got != lint.SeverityError { + t.Errorf("directive severity: expected %q, got %q", lint.SeverityError, got) + } + }) + + t.Run("nil rules map is initialized", func(t *testing.T) { + cfg := &lint.Config{} + + config.Normalize(cfg) + + if cfg.Rules == nil { + t.Error("Rules map should be initialized") + } + }) +} diff --git a/formatter/checkstyle.go b/formatter/checkstyle.go index 1df1f5573..bb824a438 100644 --- a/formatter/checkstyle.go +++ b/formatter/checkstyle.go @@ -40,7 +40,7 @@ func (*Checkstyle) Format(failures <-chan lint.Failure, config lint.Config) (str Col: failure.Position.Start.Column, What: what, Confidence: failure.Confidence, - Severity: severity(config, failure), + Severity: failure.SeverityFor(&config), RuleName: failure.RuleName, } fn := failure.Filename() diff --git a/formatter/friendly.go b/formatter/friendly.go index cb1afcb3d..c170e1dc8 100644 --- a/formatter/friendly.go +++ b/formatter/friendly.go @@ -35,7 +35,7 @@ func (f *Friendly) Format(failures <-chan lint.Failure, config lint.Config) (str warningEmoji := color.YellowString("⚠") errorEmoji := color.RedString("✘") for failure := range failures { - sev := severity(config, failure) + sev := failure.SeverityFor(&config) firstCol := warningEmoji if sev == lint.SeverityError { firstCol = errorEmoji diff --git a/formatter/json.go b/formatter/json.go index 46a61980c..c3feea615 100644 --- a/formatter/json.go +++ b/formatter/json.go @@ -29,7 +29,7 @@ func (*JSON) Format(failures <-chan lint.Failure, config lint.Config) (string, e var slice []jsonObject for failure := range failures { obj := jsonObject{} - obj.Severity = severity(config, failure) + obj.Severity = failure.SeverityFor(&config) obj.Failure = failure slice = append(slice, obj) } diff --git a/formatter/ndjson.go b/formatter/ndjson.go index f80b5bcbc..8ca416545 100644 --- a/formatter/ndjson.go +++ b/formatter/ndjson.go @@ -24,7 +24,7 @@ func (*NDJSON) Format(failures <-chan lint.Failure, config lint.Config) (string, enc := json.NewEncoder(&buf) for failure := range failures { obj := jsonObject{} - obj.Severity = severity(config, failure) + obj.Severity = failure.SeverityFor(&config) obj.Failure = failure err := enc.Encode(obj) if err != nil { diff --git a/formatter/severity.go b/formatter/severity.go deleted file mode 100644 index a43bf3192..000000000 --- a/formatter/severity.go +++ /dev/null @@ -1,13 +0,0 @@ -package formatter - -import "github.com/mgechev/revive/lint" - -func severity(config lint.Config, failure lint.Failure) lint.Severity { - if config, ok := config.Rules[failure.RuleName]; ok && config.Severity == lint.SeverityError { - return lint.SeverityError - } - if config, ok := config.Directives[failure.RuleName]; ok && config.Severity == lint.SeverityError { - return lint.SeverityError - } - return lint.SeverityWarning -} diff --git a/formatter/stylish.go b/formatter/stylish.go index 100a7927b..54ad98020 100644 --- a/formatter/stylish.go +++ b/formatter/stylish.go @@ -42,7 +42,7 @@ func (*Stylish) Format(failures <-chan lint.Failure, config lint.Config) (string for f := range failures { total++ - currentType := severity(config, f) + currentType := f.SeverityFor(&config) if currentType == lint.SeverityError { totalErrors++ } diff --git a/lint/failure.go b/lint/failure.go index 01ed09115..7a0d0726a 100644 --- a/lint/failure.go +++ b/lint/failure.go @@ -98,6 +98,21 @@ func (f *Failure) IsInternal() bool { return f.Category == failureCategoryInternal } +// SeverityFor returns the effective severity of the failure under the given configuration. +// A failure is an error if its rule or directive is configured with [SeverityError]; otherwise it is a warning. +func (f *Failure) SeverityFor(config *Config) Severity { + if config == nil { + return SeverityWarning + } + if c, ok := config.Rules[f.RuleName]; ok && c.Severity == SeverityError { + return SeverityError + } + if c, ok := config.Directives[f.RuleName]; ok && c.Severity == SeverityError { + return SeverityError + } + return SeverityWarning +} + // NewInternalFailure yields an internal failure with the given message as failure message. func NewInternalFailure(message string) Failure { return Failure{ diff --git a/lint/failure_test.go b/lint/failure_test.go new file mode 100644 index 000000000..cebcc174a --- /dev/null +++ b/lint/failure_test.go @@ -0,0 +1,48 @@ +package lint + +import "testing" + +func TestFailureSeverityFor(t *testing.T) { + for name, tc := range map[string]struct { + config *Config + failure Failure + want Severity + }{ + "rule configured as error": { + config: &Config{Rules: RulesConfig{"r": {Severity: SeverityError}}}, + failure: Failure{RuleName: "r"}, + want: SeverityError, + }, + "rule configured as warning": { + config: &Config{Rules: RulesConfig{"r": {Severity: SeverityWarning}}}, + failure: Failure{RuleName: "r"}, + want: SeverityWarning, + }, + "directive configured as error": { + config: &Config{Directives: DirectivesConfig{"d": {Severity: SeverityError}}}, + failure: Failure{RuleName: "d"}, + want: SeverityError, + }, + "rule without severity defaults to warning": { + config: &Config{Rules: RulesConfig{"r": {}}}, + failure: Failure{RuleName: "r"}, + want: SeverityWarning, + }, + "rule not in config defaults to warning": { + config: &Config{}, + failure: Failure{RuleName: "unknown"}, + want: SeverityWarning, + }, + "nil config defaults to warning": { + config: nil, + failure: Failure{RuleName: "r"}, + want: SeverityWarning, + }, + } { + t.Run(name, func(t *testing.T) { + if got := tc.failure.SeverityFor(tc.config); got != tc.want { + t.Errorf("SeverityFor: expected %q, got %q", tc.want, got) + } + }) + } +} diff --git a/revivelib/core.go b/revivelib/core.go index 40abe859d..9111d8f2c 100755 --- a/revivelib/core.go +++ b/revivelib/core.go @@ -149,11 +149,7 @@ func (r *Revive) Format( exitCode = conf.WarningCode } - if c, ok := conf.Rules[failure.RuleName]; ok && c.Severity == lint.SeverityError { - exitCode = conf.ErrorCode - } - - if c, ok := conf.Directives[failure.RuleName]; ok && c.Severity == lint.SeverityError { + if failure.SeverityFor(conf) == lint.SeverityError { exitCode = conf.ErrorCode }