Skip to content

Commit b34c8e9

Browse files
alexandearalexandear
authored
refactor: fix musttag lint issues (#1464)
1 parent e5f7980 commit b34c8e9

File tree

7 files changed

+220
-81
lines changed

7 files changed

+220
-81
lines changed

‎.golangci.yml‎

Lines changed: 1 addition & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -12,6 +12,7 @@ linters:
1212
- govet
1313
- ineffassign
1414
- misspell
15+
- musttag
1516
- nolintlint
1617
- revive
1718
- thelper

‎config/config_test.go‎

Lines changed: 203 additions & 67 deletions
Original file line numberDiff line numberDiff line change
@@ -1,85 +1,221 @@
11
package config
22

33
import (
4-
"reflect"
4+
"path/filepath"
55
"strings"
66
"testing"
77

8+
goversion "github.com/hashicorp/go-version"
9+
810
"github.com/mgechev/revive/lint"
911
"github.com/mgechev/revive/rule"
1012
)
1113

1214
func TestGetConfig(t *testing.T) {
13-
tt := map[string]struct {
14-
confPath string
15-
wantConfig *lint.Config
16-
wantError string
17-
wantConfidence float64
18-
}{
19-
"non-reg issue #470": {
20-
confPath: "testdata/issue-470.toml",
21-
wantError: "",
22-
},
23-
"unknown file": {
24-
confPath: "unknown",
25-
wantError: "cannot read the config file",
26-
},
27-
"malformed file": {
28-
confPath: "testdata/malformed.toml",
29-
wantError: "cannot parse the config file",
30-
},
31-
"default config": {
32-
wantConfig: func() *lint.Config {
33-
c := defaultConfig()
34-
normalizeConfig(c)
35-
return c
36-
}(),
37-
wantConfidence: defaultConfidence,
38-
},
39-
"config from file issue #585": {
40-
confPath: "testdata/issue-585.toml",
41-
wantConfidence: 0.0,
42-
},
43-
"config from file default confidence issue #585": {
44-
confPath: "testdata/issue-585-defaultConfidence.toml",
45-
wantConfidence: defaultConfidence,
46-
},
47-
}
15+
t.Run("ok", func(t *testing.T) {
16+
for name, tc := range map[string]struct {
17+
confPath string
18+
wantConfig lint.Config
19+
}{
20+
"default config": {
21+
wantConfig: func() lint.Config {
22+
c := defaultConfig()
23+
normalizeConfig(c)
24+
return *c
25+
}(),
26+
},
27+
"non-reg issue #470": {
28+
confPath: "issue-470.toml",
29+
wantConfig: lint.Config{
30+
Confidence: 0.8,
31+
Severity: lint.SeverityWarning,
32+
Rules: lint.RulesConfig{
33+
"add-constant": {
34+
Severity: lint.SeverityWarning,
35+
Arguments: lint.Arguments{
36+
map[string]any{
37+
"maxLitCount": "3",
38+
"allowStrs": `"`,
39+
"allowFloats": "0.0,1.0,1.,2.0,2.",
40+
"allowInts": "0,1,2",
41+
},
42+
},
43+
},
44+
},
45+
},
46+
},
47+
"config from file issue #585": {
48+
confPath: "issue-585.toml",
49+
wantConfig: lint.Config{
50+
Confidence: 0.0,
51+
Severity: lint.SeverityWarning,
52+
},
53+
},
54+
"config from file default confidence issue #585": {
55+
confPath: "issue-585-defaultConfidence.toml",
56+
wantConfig: lint.Config{
57+
Confidence: 0.8,
58+
Severity: lint.SeverityWarning,
59+
},
60+
},
61+
"config from file goVersion": {
62+
confPath: "goVersion.toml",
63+
wantConfig: lint.Config{
64+
Confidence: 0.8,
65+
GoVersion: goversion.Must(goversion.NewSemver("1.20.0")),
66+
},
67+
},
68+
"config from file ignoreGeneratedHeader": {
69+
confPath: "ignoreGeneratedHeader.toml",
70+
wantConfig: lint.Config{
71+
Confidence: 0.8,
72+
IgnoreGeneratedHeader: true,
73+
},
74+
},
75+
} {
76+
t.Run(name, func(t *testing.T) {
77+
var cfgPath string
78+
if tc.confPath != "" {
79+
cfgPath = filepath.Join("testdata", tc.confPath)
80+
}
4881

49-
for name, tc := range tt {
50-
t.Run(name, func(t *testing.T) {
51-
cfg, err := GetConfig(tc.confPath)
52-
switch {
53-
case err != nil && tc.wantError == "":
54-
t.Fatalf("Unexpected error\n\t%v", err)
55-
case err != nil && !strings.Contains(err.Error(), tc.wantError):
56-
t.Fatalf("Expected error\n\t%q\ngot:\n\t%v", tc.wantError, err)
57-
case tc.wantConfig != nil && !reflect.DeepEqual(cfg, tc.wantConfig):
58-
t.Fatalf("Expected config\n\t%+v\ngot:\n\t%+v", tc.wantConfig, cfg)
59-
case tc.wantConfig != nil && tc.wantConfidence != cfg.Confidence:
60-
t.Fatalf("Expected confidence\n\t%+v\ngot:\n\t%+v", tc.wantConfidence, cfg.Confidence)
82+
cfg, err := GetConfig(cfgPath)
83+
84+
if err != nil {
85+
t.Fatalf("Unexpected error %v", err)
86+
}
87+
if cfg.IgnoreGeneratedHeader != tc.wantConfig.IgnoreGeneratedHeader {
88+
t.Errorf("IgnoreGeneratedHeader: expected %v, got %v", tc.wantConfig.IgnoreGeneratedHeader, cfg.IgnoreGeneratedHeader)
89+
}
90+
if cfg.Confidence != tc.wantConfig.Confidence {
91+
t.Errorf("Confidence: expected %v, got %v", tc.wantConfig.Confidence, cfg.Confidence)
92+
}
93+
if cfg.Severity != tc.wantConfig.Severity {
94+
t.Errorf("Severity: expected %v, got %v", tc.wantConfig.Severity, cfg.Severity)
95+
}
96+
if cfg.EnableAllRules != tc.wantConfig.EnableAllRules {
97+
t.Errorf("EnableAllRules: expected %v, got %v", tc.wantConfig.EnableAllRules, cfg.EnableAllRules)
98+
}
99+
if cfg.ErrorCode != tc.wantConfig.ErrorCode {
100+
t.Errorf("ErrorCode: expected %v, got %v", tc.wantConfig.ErrorCode, cfg.ErrorCode)
101+
}
102+
if cfg.WarningCode != tc.wantConfig.WarningCode {
103+
t.Errorf("WarningCode: expected %v, got %v", tc.wantConfig.WarningCode, cfg.WarningCode)
104+
}
105+
if !tc.wantConfig.GoVersion.Equal(cfg.GoVersion) {
106+
t.Errorf("GoVersion: expected %v, got %v", tc.wantConfig.GoVersion, cfg.GoVersion)
107+
}
108+
109+
if len(cfg.Exclude) != len(tc.wantConfig.Exclude) {
110+
t.Errorf("Exclude length: expected %v, got %v", len(tc.wantConfig.Exclude), len(cfg.Exclude))
111+
} else {
112+
for i, exclude := range tc.wantConfig.Exclude {
113+
if cfg.Exclude[i] != exclude {
114+
t.Errorf("Exclude[%d]: expected %v, got %v", i, exclude, cfg.Exclude[i])
115+
}
116+
}
117+
}
118+
119+
if len(cfg.Rules) != len(tc.wantConfig.Rules) {
120+
t.Errorf("Rules count: expected %v, got %v", len(tc.wantConfig.Rules), len(cfg.Rules))
121+
}
122+
for ruleName, wantRule := range tc.wantConfig.Rules {
123+
gotRule, exists := cfg.Rules[ruleName]
124+
if !exists {
125+
t.Errorf("Rule %q: expected to exist, but not found", ruleName)
126+
continue
127+
}
128+
if gotRule.Disabled != wantRule.Disabled {
129+
t.Errorf("Rule %q Disabled: expected %v, got %v", ruleName, wantRule.Disabled, gotRule.Disabled)
130+
}
131+
if gotRule.Severity != wantRule.Severity {
132+
t.Errorf("Rule %q Severity: expected %v, got %v", ruleName, wantRule.Severity, gotRule.Severity)
133+
}
134+
if len(gotRule.Arguments) != len(wantRule.Arguments) {
135+
t.Errorf("Rule %q Arguments length: expected %v, got %v", ruleName, len(wantRule.Arguments), len(gotRule.Arguments))
136+
}
137+
if len(gotRule.Exclude) != len(wantRule.Exclude) {
138+
t.Errorf("Rule %q Exclude length: expected %v, got %v", ruleName, len(wantRule.Exclude), len(gotRule.Exclude))
139+
} else {
140+
for i, wantExclude := range wantRule.Exclude {
141+
if gotRule.Exclude[i] != wantExclude {
142+
t.Errorf("Rule %q Exclude[%d]: expected %v, got %v", ruleName, i, wantExclude, gotRule.Exclude[i])
143+
}
144+
}
145+
}
146+
}
147+
// Check for unexpected rules in actual config
148+
for ruleName := range cfg.Rules {
149+
if _, exists := tc.wantConfig.Rules[ruleName]; !exists {
150+
t.Errorf("Rule %q: found in actual config but not expected", ruleName)
151+
}
152+
}
153+
154+
if len(cfg.Directives) != len(tc.wantConfig.Directives) {
155+
t.Errorf("Directives count: expected %v, got %v", len(tc.wantConfig.Directives), len(cfg.Directives))
156+
}
157+
for directiveName, wantDirective := range tc.wantConfig.Directives {
158+
gotDirective, exists := cfg.Directives[directiveName]
159+
if !exists {
160+
t.Errorf("Directive %q: expected to exist, but not found", directiveName)
161+
continue
162+
}
163+
if gotDirective.Severity != wantDirective.Severity {
164+
t.Errorf("Directive %q Severity: expected %v, got %v", directiveName, wantDirective.Severity, gotDirective.Severity)
165+
}
166+
}
167+
// Check for unexpected directives in actual config
168+
for directiveName := range cfg.Directives {
169+
if _, exists := tc.wantConfig.Directives[directiveName]; !exists {
170+
t.Errorf("Directive %q: found in actual config but not expected", directiveName)
171+
}
172+
}
173+
})
174+
}
175+
176+
t.Run("rule-level file filter excludes", func(t *testing.T) {
177+
cfg, err := GetConfig("testdata/rule-level-exclude-850.toml")
178+
if err != nil {
179+
t.Fatal("should be valid config")
180+
}
181+
r1 := cfg.Rules["r1"]
182+
if len(r1.Exclude) > 0 {
183+
t.Fatal("r1 should have empty excludes")
184+
}
185+
r2 := cfg.Rules["r2"]
186+
if len(r2.Exclude) != 1 {
187+
t.Fatal("r2 should have exclude set")
188+
}
189+
if !r2.MustExclude("some/file.go") {
190+
t.Fatal("r2 should be initialized and exclude some/file.go")
191+
}
192+
if r2.MustExclude("some/any-other.go") {
193+
t.Fatal("r2 should not exclude some/any-other.go")
61194
}
62195
})
63-
}
196+
})
64197

65-
t.Run("rule-level file filter excludes", func(t *testing.T) {
66-
cfg, err := GetConfig("testdata/rule-level-exclude-850.toml")
67-
if err != nil {
68-
t.Fatal("should be valid config")
69-
}
70-
r1 := cfg.Rules["r1"]
71-
if len(r1.Exclude) > 0 {
72-
t.Fatal("r1 should have empty excludes")
73-
}
74-
r2 := cfg.Rules["r2"]
75-
if len(r2.Exclude) != 1 {
76-
t.Fatal("r2 should have exclude set")
77-
}
78-
if !r2.MustExclude("some/file.go") {
79-
t.Fatal("r2 should be initialized and exclude some/file.go")
80-
}
81-
if r2.MustExclude("some/any-other.go") {
82-
t.Fatal("r2 should not exclude some/any-other.go")
198+
t.Run("failure", func(t *testing.T) {
199+
for name, tc := range map[string]struct {
200+
confPath string
201+
wantError string
202+
}{
203+
"unknown file": {
204+
confPath: "unknown",
205+
wantError: "cannot read the config file",
206+
},
207+
"malformed file": {
208+
confPath: "malformed.toml",
209+
wantError: "cannot parse the config file",
210+
},
211+
} {
212+
t.Run(name, func(t *testing.T) {
213+
_, err := GetConfig(filepath.Join("testdata", tc.confPath))
214+
215+
if err != nil && !strings.Contains(err.Error(), tc.wantError) {
216+
t.Errorf("Unexpected error: want %q, got: %q", tc.wantError, err)
217+
}
218+
})
83219
}
84220
})
85221
}

‎config/testdata/goVersion.toml‎

Lines changed: 1 addition & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -0,0 +1 @@
1+
goVersion = "1.20"

‎config/testdata/ignoreGeneratedHeader.toml‎

Lines changed: 1 addition & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -0,0 +1 @@
1+
ignoreGeneratedHeader = true

‎formatter/json.go‎

Lines changed: 1 addition & 1 deletion
Original file line numberDiff line numberDiff line change
@@ -19,7 +19,7 @@ func (*JSON) Name() string {
1919

2020
// jsonObject defines a JSON object of an failure.
2121
type jsonObject struct {
22-
Severity lint.Severity
22+
Severity lint.Severity `json:"Severity"`
2323
lint.Failure `json:",inline"`
2424
}
2525

‎lint/config.go‎

Lines changed: 4 additions & 4 deletions
Original file line numberDiff line numberDiff line change
@@ -56,9 +56,9 @@ type DirectivesConfig = map[string]DirectiveConfig
5656

5757
// Config defines the config of the linter.
5858
type Config struct {
59-
IgnoreGeneratedHeader bool `toml:"ignoreGeneratedHeader"`
60-
Confidence float64
61-
Severity Severity
59+
IgnoreGeneratedHeader bool `toml:"ignoreGeneratedHeader"`
60+
Confidence float64 `toml:"confidence"`
61+
Severity Severity `toml:"severity"`
6262
EnableAllRules bool `toml:"enableAllRules"`
6363
Rules RulesConfig `toml:"rule"`
6464
ErrorCode int `toml:"errorCode"`
@@ -67,5 +67,5 @@ type Config struct {
6767
Exclude []string `toml:"exclude"`
6868
// If set, overrides the go language version specified in go.mod of
6969
// packages being linted, and assumes this specific language version.
70-
GoVersion *goversion.Version
70+
GoVersion *goversion.Version `toml:"goVersion"`
7171
}

‎lint/failure.go‎

Lines changed: 9 additions & 9 deletions
Original file line numberDiff line numberDiff line change
@@ -64,20 +64,20 @@ type Severity string
6464

6565
// FailurePosition returns the failure position.
6666
type FailurePosition struct {
67-
Start token.Position
68-
End token.Position
67+
Start token.Position `json:"Start"`
68+
End token.Position `json:"End"`
6969
}
7070

7171
// Failure defines a struct for a linting failure.
7272
type Failure struct {
73-
Failure string
74-
RuleName string
75-
Category FailureCategory
76-
Position FailurePosition
77-
Node ast.Node `json:"-"`
78-
Confidence float64
73+
Failure string `json:"Failure"`
74+
RuleName string `json:"RuleName"`
75+
Category FailureCategory `json:"Category"`
76+
Position FailurePosition `json:"Position"`
77+
Node ast.Node `json:"-"`
78+
Confidence float64 `json:"Confidence"`
7979
// For future use
80-
ReplacementLine string
80+
ReplacementLine string `json:"ReplacementLine"`
8181
}
8282

8383
// GetFilename returns the filename.

0 commit comments

Comments
 (0)