Skip to content

Commit cf975a4

Browse files
ldezldez
committed
chore: separation of concerns: move validation to configuration struct
1 parent bfd4182 commit cf975a4

File tree

5 files changed

+278
-252
lines changed

5 files changed

+278
-252
lines changed

‎pkg/config/config.go

Lines changed: 9 additions & 8 deletions
Original file line numberDiff line numberDiff line change
@@ -32,16 +32,17 @@ func (c *Config) GetConfigDir() string {
3232
}
3333

3434
func (c *Config) Validate() error {
35-
if err := c.Issues.Validate(); err != nil {
36-
return err
35+
validators := []func() error{
36+
c.Issues.Validate,
37+
c.Severity.Validate,
38+
c.LintersSettings.Validate,
39+
c.Linters.Validate,
3740
}
3841

39-
if err := c.Severity.Validate(); err != nil {
40-
return err
41-
}
42-
43-
if err := c.LintersSettings.Validate(); err != nil {
44-
return err
42+
for _, v := range validators {
43+
if err := v(); err != nil {
44+
return err
45+
}
4546
}
4647

4748
return nil

‎pkg/config/linters.go

Lines changed: 54 additions & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -1,5 +1,10 @@
11
package config
22

3+
import (
4+
"errors"
5+
"fmt"
6+
)
7+
38
type Linters struct {
49
Enable []string
510
Disable []string
@@ -9,3 +14,52 @@ type Linters struct {
914

1015
Presets []string
1116
}
17+
18+
func (l *Linters) Validate() error {
19+
if err := l.validateAllDisableEnableOptions(); err != nil {
20+
return err
21+
}
22+
23+
if err := l.validateDisabledAndEnabledAtOneMoment(); err != nil {
24+
return err
25+
}
26+
27+
return nil
28+
}
29+
30+
func (l *Linters) validateAllDisableEnableOptions() error {
31+
if l.EnableAll && l.DisableAll {
32+
return errors.New("--enable-all and --disable-all options must not be combined")
33+
}
34+
35+
if l.DisableAll {
36+
if len(l.Enable) == 0 && len(l.Presets) == 0 {
37+
return errors.New("all linters were disabled, but no one linter was enabled: must enable at least one")
38+
}
39+
40+
if len(l.Disable) != 0 {
41+
return fmt.Errorf("can't combine options --disable-all and --disable %s", l.Disable[0])
42+
}
43+
}
44+
45+
if l.EnableAll && len(l.Enable) != 0 && !l.Fast {
46+
return fmt.Errorf("can't combine options --enable-all and --enable %s", l.Enable[0])
47+
}
48+
49+
return nil
50+
}
51+
52+
func (l *Linters) validateDisabledAndEnabledAtOneMoment() error {
53+
enabledLintersSet := map[string]bool{}
54+
for _, name := range l.Enable {
55+
enabledLintersSet[name] = true
56+
}
57+
58+
for _, name := range l.Disable {
59+
if enabledLintersSet[name] {
60+
return fmt.Errorf("linter %q can't be disabled and enabled at one moment", name)
61+
}
62+
}
63+
64+
return nil
65+
}

‎pkg/config/linters_test.go

Lines changed: 214 additions & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -0,0 +1,214 @@
1+
package config
2+
3+
import (
4+
"testing"
5+
6+
"github.com/stretchr/testify/require"
7+
)
8+
9+
func TestLinters_validateDisabledAndEnabledAtOneMoment(t *testing.T) {
10+
testCases := []struct {
11+
desc string
12+
cfg *Linters
13+
}{
14+
{
15+
desc: "2 different sets",
16+
cfg: &Linters{
17+
Enable: []string{"dupl", "gofmt", "misspell"},
18+
Disable: []string{"goimports", "gosec", "nolintlint"},
19+
},
20+
},
21+
{
22+
desc: "only enable",
23+
cfg: &Linters{
24+
Enable: []string{"goimports", "gosec", "nolintlint"},
25+
Disable: nil,
26+
},
27+
},
28+
{
29+
desc: "only disable",
30+
cfg: &Linters{
31+
Enable: nil,
32+
Disable: []string{"dupl", "gofmt", "misspell"},
33+
},
34+
},
35+
{
36+
desc: "no sets",
37+
cfg: &Linters{
38+
Enable: nil,
39+
Disable: nil,
40+
},
41+
},
42+
}
43+
44+
for _, test := range testCases {
45+
test := test
46+
t.Run(test.desc, func(t *testing.T) {
47+
t.Parallel()
48+
49+
err := test.cfg.validateDisabledAndEnabledAtOneMoment()
50+
require.NoError(t, err)
51+
})
52+
}
53+
}
54+
55+
func TestLinters_validateDisabledAndEnabledAtOneMoment_error(t *testing.T) {
56+
testCases := []struct {
57+
desc string
58+
cfg *Linters
59+
expected string
60+
}{
61+
{
62+
desc: "disable one linter of the enabled linters",
63+
cfg: &Linters{
64+
Enable: []string{"dupl", "gofmt", "misspell"},
65+
Disable: []string{"dupl", "gosec", "nolintlint"},
66+
},
67+
expected: `linter "dupl" can't be disabled and enabled at one moment`,
68+
},
69+
{
70+
desc: "disable multiple enabled linters",
71+
cfg: &Linters{
72+
Enable: []string{"dupl", "gofmt", "misspell"},
73+
Disable: []string{"dupl", "gofmt", "misspell"},
74+
},
75+
expected: `linter "dupl" can't be disabled and enabled at one moment`,
76+
},
77+
}
78+
79+
for _, test := range testCases {
80+
test := test
81+
t.Run(test.desc, func(t *testing.T) {
82+
t.Parallel()
83+
84+
err := test.cfg.validateDisabledAndEnabledAtOneMoment()
85+
require.Error(t, err)
86+
87+
require.EqualError(t, err, test.expected)
88+
})
89+
}
90+
}
91+
92+
func TestLinters_validateAllDisableEnableOptions(t *testing.T) {
93+
testCases := []struct {
94+
desc string
95+
cfg *Linters
96+
}{
97+
{
98+
desc: "nothing",
99+
cfg: &Linters{},
100+
},
101+
{
102+
desc: "enable and disable",
103+
cfg: &Linters{
104+
Enable: []string{"goimports", "gosec", "nolintlint"},
105+
EnableAll: false,
106+
Disable: []string{"dupl", "gofmt", "misspell"},
107+
DisableAll: false,
108+
},
109+
},
110+
{
111+
desc: "disable-all and enable",
112+
cfg: &Linters{
113+
Enable: []string{"goimports", "gosec", "nolintlint"},
114+
EnableAll: false,
115+
Disable: nil,
116+
DisableAll: true,
117+
},
118+
},
119+
{
120+
desc: "enable-all and disable",
121+
cfg: &Linters{
122+
Enable: nil,
123+
EnableAll: true,
124+
Disable: []string{"goimports", "gosec", "nolintlint"},
125+
DisableAll: false,
126+
},
127+
},
128+
{
129+
desc: "enable-all and enable and fast",
130+
cfg: &Linters{
131+
Enable: []string{"dupl", "gofmt", "misspell"},
132+
EnableAll: true,
133+
Disable: nil,
134+
DisableAll: false,
135+
Fast: true,
136+
},
137+
},
138+
}
139+
140+
for _, test := range testCases {
141+
test := test
142+
t.Run(test.desc, func(t *testing.T) {
143+
t.Parallel()
144+
145+
err := test.cfg.validateAllDisableEnableOptions()
146+
require.NoError(t, err)
147+
})
148+
}
149+
}
150+
151+
func TestLinters_validateAllDisableEnableOptions_error(t *testing.T) {
152+
testCases := []struct {
153+
desc string
154+
cfg *Linters
155+
expected string
156+
}{
157+
{
158+
desc: "enable-all and disable-all",
159+
cfg: &Linters{
160+
Enable: nil,
161+
EnableAll: true,
162+
Disable: nil,
163+
DisableAll: true,
164+
Fast: false,
165+
},
166+
expected: "--enable-all and --disable-all options must not be combined",
167+
},
168+
{
169+
desc: "disable-all and disable no enable no preset",
170+
cfg: &Linters{
171+
Enable: nil,
172+
EnableAll: false,
173+
Disable: []string{"dupl", "gofmt", "misspell"},
174+
DisableAll: true,
175+
Fast: false,
176+
},
177+
expected: "all linters were disabled, but no one linter was enabled: must enable at least one",
178+
},
179+
{
180+
desc: "disable-all and disable with enable",
181+
cfg: &Linters{
182+
Enable: []string{"nolintlint"},
183+
EnableAll: false,
184+
Disable: []string{"dupl", "gofmt", "misspell"},
185+
DisableAll: true,
186+
Fast: false,
187+
},
188+
expected: "can't combine options --disable-all and --disable dupl",
189+
},
190+
{
191+
desc: "enable-all and enable",
192+
cfg: &Linters{
193+
Enable: []string{"dupl", "gofmt", "misspell"},
194+
EnableAll: true,
195+
Disable: nil,
196+
DisableAll: false,
197+
Fast: false,
198+
},
199+
expected: "can't combine options --enable-all and --enable dupl",
200+
},
201+
}
202+
203+
for _, test := range testCases {
204+
test := test
205+
t.Run(test.desc, func(t *testing.T) {
206+
t.Parallel()
207+
208+
err := test.cfg.validateAllDisableEnableOptions()
209+
require.Error(t, err)
210+
211+
require.EqualError(t, err, test.expected)
212+
})
213+
}
214+
}

‎pkg/lint/lintersdb/validator.go

Lines changed: 1 addition & 39 deletions
Original file line numberDiff line numberDiff line change
@@ -31,9 +31,8 @@ func (v Validator) validateEnabledDisabledLintersConfig(cfg *config.Linters) err
3131
validators := []func(cfg *config.Linters) error{
3232
v.validateLintersNames,
3333
v.validatePresets,
34-
v.validateAllDisableEnableOptions,
35-
v.validateDisabledAndEnabledAtOneMoment,
3634
}
35+
3736
for _, v := range validators {
3837
if err := v(cfg); err != nil {
3938
return err
@@ -79,40 +78,3 @@ func (v Validator) validatePresets(cfg *config.Linters) error {
7978

8079
return nil
8180
}
82-
83-
func (v Validator) validateAllDisableEnableOptions(cfg *config.Linters) error {
84-
if cfg.EnableAll && cfg.DisableAll {
85-
return errors.New("--enable-all and --disable-all options must not be combined")
86-
}
87-
88-
if cfg.DisableAll {
89-
if len(cfg.Enable) == 0 && len(cfg.Presets) == 0 {
90-
return errors.New("all linters were disabled, but no one linter was enabled: must enable at least one")
91-
}
92-
93-
if len(cfg.Disable) != 0 {
94-
return fmt.Errorf("can't combine options --disable-all and --disable %s", cfg.Disable[0])
95-
}
96-
}
97-
98-
if cfg.EnableAll && len(cfg.Enable) != 0 && !cfg.Fast {
99-
return fmt.Errorf("can't combine options --enable-all and --enable %s", cfg.Enable[0])
100-
}
101-
102-
return nil
103-
}
104-
105-
func (v Validator) validateDisabledAndEnabledAtOneMoment(cfg *config.Linters) error {
106-
enabledLintersSet := map[string]bool{}
107-
for _, name := range cfg.Enable {
108-
enabledLintersSet[name] = true
109-
}
110-
111-
for _, name := range cfg.Disable {
112-
if enabledLintersSet[name] {
113-
return fmt.Errorf("linter %q can't be disabled and enabled at one moment", name)
114-
}
115-
}
116-
117-
return nil
118-
}

0 commit comments

Comments
 (0)