diff --git a/.golangci.yml b/.golangci.yml index a8b485b..216d9c4 100644 --- a/.golangci.yml +++ b/.golangci.yml @@ -12,6 +12,7 @@ linters: - govet - ineffassign - misspell + - musttag - nolintlint - revive - thelper diff --git a/config/config_test.go b/config/config_test.go index 74dbf4d..45cd51b 100644 --- a/config/config_test.go +++ b/config/config_test.go @@ -1,85 +1,221 @@ package config import ( - "reflect" + "path/filepath" "strings" "testing" + goversion "github.com/hashicorp/go-version" + "github.com/mgechev/revive/lint" "github.com/mgechev/revive/rule" ) func TestGetConfig(t *testing.T) { - tt := map[string]struct { - confPath string - wantConfig *lint.Config - wantError string - wantConfidence float64 - }{ - "non-reg issue #470": { - confPath: "testdata/issue-470.toml", - wantError: "", - }, - "unknown file": { - confPath: "unknown", - wantError: "cannot read the config file", - }, - "malformed file": { - confPath: "testdata/malformed.toml", - wantError: "cannot parse the config file", - }, - "default config": { - wantConfig: func() *lint.Config { - c := defaultConfig() - normalizeConfig(c) - return c - }(), - wantConfidence: defaultConfidence, - }, - "config from file issue #585": { - confPath: "testdata/issue-585.toml", - wantConfidence: 0.0, - }, - "config from file default confidence issue #585": { - confPath: "testdata/issue-585-defaultConfidence.toml", - wantConfidence: defaultConfidence, - }, - } + t.Run("ok", func(t *testing.T) { + for name, tc := range map[string]struct { + confPath string + wantConfig lint.Config + }{ + "default config": { + wantConfig: func() lint.Config { + c := defaultConfig() + normalizeConfig(c) + return *c + }(), + }, + "non-reg issue #470": { + confPath: "issue-470.toml", + wantConfig: lint.Config{ + Confidence: 0.8, + Severity: lint.SeverityWarning, + Rules: lint.RulesConfig{ + "add-constant": { + Severity: lint.SeverityWarning, + Arguments: lint.Arguments{ + map[string]any{ + "maxLitCount": "3", + "allowStrs": `"`, + "allowFloats": "0.0,1.0,1.,2.0,2.", + "allowInts": "0,1,2", + }, + }, + }, + }, + }, + }, + "config from file issue #585": { + confPath: "issue-585.toml", + wantConfig: lint.Config{ + Confidence: 0.0, + Severity: lint.SeverityWarning, + }, + }, + "config from file default confidence issue #585": { + confPath: "issue-585-defaultConfidence.toml", + wantConfig: lint.Config{ + Confidence: 0.8, + Severity: lint.SeverityWarning, + }, + }, + "config from file goVersion": { + confPath: "goVersion.toml", + wantConfig: lint.Config{ + Confidence: 0.8, + GoVersion: goversion.Must(goversion.NewSemver("1.20.0")), + }, + }, + "config from file ignoreGeneratedHeader": { + confPath: "ignoreGeneratedHeader.toml", + wantConfig: lint.Config{ + Confidence: 0.8, + IgnoreGeneratedHeader: true, + }, + }, + } { + t.Run(name, func(t *testing.T) { + var cfgPath string + if tc.confPath != "" { + cfgPath = filepath.Join("testdata", tc.confPath) + } - for name, tc := range tt { - t.Run(name, func(t *testing.T) { - cfg, err := GetConfig(tc.confPath) - switch { - case err != nil && tc.wantError == "": - t.Fatalf("Unexpected error\n\t%v", err) - case err != nil && !strings.Contains(err.Error(), tc.wantError): - t.Fatalf("Expected error\n\t%q\ngot:\n\t%v", tc.wantError, err) - case tc.wantConfig != nil && !reflect.DeepEqual(cfg, tc.wantConfig): - t.Fatalf("Expected config\n\t%+v\ngot:\n\t%+v", tc.wantConfig, cfg) - case tc.wantConfig != nil && tc.wantConfidence != cfg.Confidence: - t.Fatalf("Expected confidence\n\t%+v\ngot:\n\t%+v", tc.wantConfidence, cfg.Confidence) + cfg, err := GetConfig(cfgPath) + + if err != nil { + t.Fatalf("Unexpected error %v", err) + } + if cfg.IgnoreGeneratedHeader != tc.wantConfig.IgnoreGeneratedHeader { + t.Errorf("IgnoreGeneratedHeader: expected %v, got %v", tc.wantConfig.IgnoreGeneratedHeader, cfg.IgnoreGeneratedHeader) + } + if cfg.Confidence != tc.wantConfig.Confidence { + t.Errorf("Confidence: expected %v, got %v", tc.wantConfig.Confidence, cfg.Confidence) + } + if cfg.Severity != tc.wantConfig.Severity { + t.Errorf("Severity: expected %v, got %v", tc.wantConfig.Severity, cfg.Severity) + } + if cfg.EnableAllRules != tc.wantConfig.EnableAllRules { + t.Errorf("EnableAllRules: expected %v, got %v", tc.wantConfig.EnableAllRules, cfg.EnableAllRules) + } + if cfg.ErrorCode != tc.wantConfig.ErrorCode { + t.Errorf("ErrorCode: expected %v, got %v", tc.wantConfig.ErrorCode, cfg.ErrorCode) + } + if cfg.WarningCode != tc.wantConfig.WarningCode { + t.Errorf("WarningCode: expected %v, got %v", tc.wantConfig.WarningCode, cfg.WarningCode) + } + if !tc.wantConfig.GoVersion.Equal(cfg.GoVersion) { + t.Errorf("GoVersion: expected %v, got %v", tc.wantConfig.GoVersion, cfg.GoVersion) + } + + if len(cfg.Exclude) != len(tc.wantConfig.Exclude) { + t.Errorf("Exclude length: expected %v, got %v", len(tc.wantConfig.Exclude), len(cfg.Exclude)) + } else { + for i, exclude := range tc.wantConfig.Exclude { + if cfg.Exclude[i] != exclude { + t.Errorf("Exclude[%d]: expected %v, got %v", i, exclude, cfg.Exclude[i]) + } + } + } + + if len(cfg.Rules) != len(tc.wantConfig.Rules) { + t.Errorf("Rules count: expected %v, got %v", len(tc.wantConfig.Rules), len(cfg.Rules)) + } + for ruleName, wantRule := range tc.wantConfig.Rules { + gotRule, exists := cfg.Rules[ruleName] + if !exists { + t.Errorf("Rule %q: expected to exist, but not found", ruleName) + continue + } + if gotRule.Disabled != wantRule.Disabled { + t.Errorf("Rule %q Disabled: expected %v, got %v", ruleName, wantRule.Disabled, gotRule.Disabled) + } + if gotRule.Severity != wantRule.Severity { + t.Errorf("Rule %q Severity: expected %v, got %v", ruleName, wantRule.Severity, gotRule.Severity) + } + if len(gotRule.Arguments) != len(wantRule.Arguments) { + t.Errorf("Rule %q Arguments length: expected %v, got %v", ruleName, len(wantRule.Arguments), len(gotRule.Arguments)) + } + if len(gotRule.Exclude) != len(wantRule.Exclude) { + t.Errorf("Rule %q Exclude length: expected %v, got %v", ruleName, len(wantRule.Exclude), len(gotRule.Exclude)) + } else { + for i, wantExclude := range wantRule.Exclude { + if gotRule.Exclude[i] != wantExclude { + t.Errorf("Rule %q Exclude[%d]: expected %v, got %v", ruleName, i, wantExclude, gotRule.Exclude[i]) + } + } + } + } + // Check for unexpected rules in actual config + for ruleName := range cfg.Rules { + if _, exists := tc.wantConfig.Rules[ruleName]; !exists { + t.Errorf("Rule %q: found in actual config but not expected", ruleName) + } + } + + if len(cfg.Directives) != len(tc.wantConfig.Directives) { + t.Errorf("Directives count: expected %v, got %v", len(tc.wantConfig.Directives), len(cfg.Directives)) + } + for directiveName, wantDirective := range tc.wantConfig.Directives { + gotDirective, exists := cfg.Directives[directiveName] + if !exists { + t.Errorf("Directive %q: expected to exist, but not found", directiveName) + continue + } + if gotDirective.Severity != wantDirective.Severity { + t.Errorf("Directive %q Severity: expected %v, got %v", directiveName, wantDirective.Severity, gotDirective.Severity) + } + } + // Check for unexpected directives in actual config + for directiveName := range cfg.Directives { + if _, exists := tc.wantConfig.Directives[directiveName]; !exists { + t.Errorf("Directive %q: found in actual config but not expected", directiveName) + } + } + }) + } + + t.Run("rule-level file filter excludes", func(t *testing.T) { + cfg, err := GetConfig("testdata/rule-level-exclude-850.toml") + if err != nil { + t.Fatal("should be valid config") + } + r1 := cfg.Rules["r1"] + if len(r1.Exclude) > 0 { + t.Fatal("r1 should have empty excludes") + } + r2 := cfg.Rules["r2"] + if len(r2.Exclude) != 1 { + t.Fatal("r2 should have exclude set") + } + if !r2.MustExclude("some/file.go") { + t.Fatal("r2 should be initialized and exclude some/file.go") + } + if r2.MustExclude("some/any-other.go") { + t.Fatal("r2 should not exclude some/any-other.go") } }) - } + }) - t.Run("rule-level file filter excludes", func(t *testing.T) { - cfg, err := GetConfig("testdata/rule-level-exclude-850.toml") - if err != nil { - t.Fatal("should be valid config") - } - r1 := cfg.Rules["r1"] - if len(r1.Exclude) > 0 { - t.Fatal("r1 should have empty excludes") - } - r2 := cfg.Rules["r2"] - if len(r2.Exclude) != 1 { - t.Fatal("r2 should have exclude set") - } - if !r2.MustExclude("some/file.go") { - t.Fatal("r2 should be initialized and exclude some/file.go") - } - if r2.MustExclude("some/any-other.go") { - t.Fatal("r2 should not exclude some/any-other.go") + t.Run("failure", func(t *testing.T) { + for name, tc := range map[string]struct { + confPath string + wantError string + }{ + "unknown file": { + confPath: "unknown", + wantError: "cannot read the config file", + }, + "malformed file": { + confPath: "malformed.toml", + wantError: "cannot parse the config file", + }, + } { + t.Run(name, func(t *testing.T) { + _, err := GetConfig(filepath.Join("testdata", tc.confPath)) + + if err != nil && !strings.Contains(err.Error(), tc.wantError) { + t.Errorf("Unexpected error: want %q, got: %q", tc.wantError, err) + } + }) } }) } diff --git a/config/testdata/goVersion.toml b/config/testdata/goVersion.toml new file mode 100644 index 0000000..476fbd1 --- /dev/null +++ b/config/testdata/goVersion.toml @@ -0,0 +1 @@ +goVersion = "1.20" diff --git a/config/testdata/ignoreGeneratedHeader.toml b/config/testdata/ignoreGeneratedHeader.toml new file mode 100644 index 0000000..ad228b3 --- /dev/null +++ b/config/testdata/ignoreGeneratedHeader.toml @@ -0,0 +1 @@ +ignoreGeneratedHeader = true diff --git a/formatter/json.go b/formatter/json.go index c7144e1..292c06b 100644 --- a/formatter/json.go +++ b/formatter/json.go @@ -19,7 +19,7 @@ func (*JSON) Name() string { // jsonObject defines a JSON object of an failure. type jsonObject struct { - Severity lint.Severity + Severity lint.Severity `json:"Severity"` lint.Failure `json:",inline"` } diff --git a/lint/config.go b/lint/config.go index a305a74..ae9a20a 100644 --- a/lint/config.go +++ b/lint/config.go @@ -56,9 +56,9 @@ type DirectivesConfig = map[string]DirectiveConfig // Config defines the config of the linter. type Config struct { - IgnoreGeneratedHeader bool `toml:"ignoreGeneratedHeader"` - Confidence float64 - Severity Severity + IgnoreGeneratedHeader bool `toml:"ignoreGeneratedHeader"` + Confidence float64 `toml:"confidence"` + Severity Severity `toml:"severity"` EnableAllRules bool `toml:"enableAllRules"` Rules RulesConfig `toml:"rule"` ErrorCode int `toml:"errorCode"` @@ -67,5 +67,5 @@ type Config struct { Exclude []string `toml:"exclude"` // If set, overrides the go language version specified in go.mod of // packages being linted, and assumes this specific language version. - GoVersion *goversion.Version + GoVersion *goversion.Version `toml:"goVersion"` } diff --git a/lint/failure.go b/lint/failure.go index 9ad17c3..d9ff93e 100644 --- a/lint/failure.go +++ b/lint/failure.go @@ -64,20 +64,20 @@ type Severity string // FailurePosition returns the failure position. type FailurePosition struct { - Start token.Position - End token.Position + Start token.Position `json:"Start"` + End token.Position `json:"End"` } // Failure defines a struct for a linting failure. type Failure struct { - Failure string - RuleName string - Category FailureCategory - Position FailurePosition - Node ast.Node `json:"-"` - Confidence float64 + Failure string `json:"Failure"` + RuleName string `json:"RuleName"` + Category FailureCategory `json:"Category"` + Position FailurePosition `json:"Position"` + Node ast.Node `json:"-"` + Confidence float64 `json:"Confidence"` // For future use - ReplacementLine string + ReplacementLine string `json:"ReplacementLine"` } // GetFilename returns the filename.