Skip to content
Open
Show file tree
Hide file tree
Changes from 1 commit
Commits
File filter

Filter by extension

Filter by extension

Conversations
Failed to load comments.
Loading
Jump to
Jump to file
Failed to load files.
Loading
Diff view
Diff view
50 changes: 43 additions & 7 deletions config/config.go
Original file line number Diff line number Diff line change
Expand Up @@ -6,6 +6,7 @@ import (
"fmt"
"os"
"reflect"
"slices"
"strings"

"github.com/BurntSushi/toml"
Expand Down Expand Up @@ -127,6 +128,37 @@ 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 {
var names []string
for name, ruleConfig := range config.Rules {
if !ruleConfig.Disabled {
names = append(names, name)
}
}
slices.Sort(names)
return names
}
Comment thread
alexandear marked this conversation as resolved.

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{
Expand Down Expand Up @@ -256,7 +288,9 @@ func validateConfig(config *lint.Config) error {
return nil
}

func normalizeConfig(config *lint.Config) {
// NormalizeConfig 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 NormalizeConfig(config *lint.Config) {
if len(config.Rules) == 0 {
config.Rules = map[string]lint.RuleConfig{}
}
Expand Down Expand Up @@ -293,14 +327,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")
Expand All @@ -311,14 +346,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)
NormalizeConfig(config)
return config, nil
}

Expand All @@ -335,9 +370,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{},
}
Expand Down
146 changes: 146 additions & 0 deletions config/config_test.go
Original file line number Diff line number Diff line change
Expand Up @@ -777,3 +777,149 @@ 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)
}
})
}

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 TestNormalizeConfig(t *testing.T) {
t.Run("enable-default-rules adds default rule entries", func(t *testing.T) {
cfg := &lint.Config{EnableDefaultRules: true}

config.NormalizeConfig(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.NormalizeConfig(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.NormalizeConfig(cfg)

if cfg.Rules == nil {
t.Error("Rules map should be initialized")
}
})
}
2 changes: 1 addition & 1 deletion formatter/checkstyle.go
Original file line number Diff line number Diff line change
Expand Up @@ -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()
Expand Down
2 changes: 1 addition & 1 deletion formatter/friendly.go
Original file line number Diff line number Diff line change
Expand Up @@ -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
Expand Down
2 changes: 1 addition & 1 deletion formatter/json.go
Original file line number Diff line number Diff line change
Expand Up @@ -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)
}
Expand Down
2 changes: 1 addition & 1 deletion formatter/ndjson.go
Original file line number Diff line number Diff line change
Expand Up @@ -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 {
Expand Down
13 changes: 0 additions & 13 deletions formatter/severity.go

This file was deleted.

2 changes: 1 addition & 1 deletion formatter/stylish.go
Original file line number Diff line number Diff line change
Expand Up @@ -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++
}
Expand Down
12 changes: 12 additions & 0 deletions lint/failure.go
Original file line number Diff line number Diff line change
Expand Up @@ -98,6 +98,18 @@ 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 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
}
Comment thread
alexandear marked this conversation as resolved.

// NewInternalFailure yields an internal failure with the given message as failure message.
func NewInternalFailure(message string) Failure {
return Failure{
Expand Down
43 changes: 43 additions & 0 deletions lint/failure_test.go
Original file line number Diff line number Diff line change
@@ -0,0 +1,43 @@
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,
},
} {
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)
}
})
}
}
6 changes: 1 addition & 5 deletions revivelib/core.go
Original file line number Diff line number Diff line change
Expand Up @@ -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
}

Expand Down
Loading