From b09e18ee716e0cbab060115e11a81f3ecdf50b07 Mon Sep 17 00:00:00 2001 From: Fernandez Ludovic Date: Fri, 8 Mar 2024 17:15:13 +0100 Subject: [PATCH 1/3] fix: rule severity is required --- pkg/config/severity.go | 4 ++ pkg/config/severity_test.go | 109 +++++++++++++++++++++++++++++++++++- 2 files changed, 110 insertions(+), 3 deletions(-) diff --git a/pkg/config/severity.go b/pkg/config/severity.go index c707959ca403..49874c9a4c3a 100644 --- a/pkg/config/severity.go +++ b/pkg/config/severity.go @@ -34,5 +34,9 @@ type SeverityRule struct { } func (s *SeverityRule) Validate() error { + if s.Severity == "" { + return errors.New("severity should be set") + } + return s.BaseRule.Validate(severityRuleMinConditionsCount) } diff --git a/pkg/config/severity_test.go b/pkg/config/severity_test.go index 60c2ad25707d..b7a98aea328d 100644 --- a/pkg/config/severity_test.go +++ b/pkg/config/severity_test.go @@ -7,7 +7,94 @@ import ( ) func TestSeverity_Validate(t *testing.T) { + testCases := []struct { + desc string + severity *Severity + }{ + { + desc: "default with rules", + severity: &Severity{ + Default: "high", + Rules: []SeverityRule{ + { + Severity: "low", + BaseRule: BaseRule{ + Path: "test", + }, + }, + }, + }, + }, + { + desc: "default without rules", + severity: &Severity{ + Default: "high", + }, + }, + } + + for _, test := range testCases { + test := test + t.Run(test.desc, func(t *testing.T) { + t.Parallel() + + err := test.severity.Validate() + require.NoError(t, err) + }) + } +} + +func TestSeverity_Validate_error(t *testing.T) { + testCases := []struct { + desc string + severity *Severity + expected string + }{ + { + desc: "missing default severity", + severity: &Severity{ + Default: "", + Rules: []SeverityRule{ + { + Severity: "low", + BaseRule: BaseRule{ + Path: "test", + }, + }, + }, + }, + expected: "can't set severity rule option: no default severity defined", + }, + { + desc: "missing rule severity", + severity: &Severity{ + Default: "high", + Rules: []SeverityRule{ + { + BaseRule: BaseRule{ + Path: "test", + }, + }, + }, + }, + expected: "error in severity rule #0: severity should be set", + }, + } + + for _, test := range testCases { + test := test + t.Run(test.desc, func(t *testing.T) { + t.Parallel() + + err := test.severity.Validate() + require.EqualError(t, err, test.expected) + }) + } +} + +func TestSeverityRule_Validate(t *testing.T) { rule := &SeverityRule{ + Severity: "low", BaseRule: BaseRule{ Path: "test", }, @@ -17,20 +104,32 @@ func TestSeverity_Validate(t *testing.T) { require.NoError(t, err) } -func TestSeverity_Validate_error(t *testing.T) { +func TestSeverityRule_Validate_error(t *testing.T) { testCases := []struct { desc string rule *SeverityRule expected string }{ { - desc: "empty rule", - rule: &SeverityRule{}, + desc: "missing severity", + rule: &SeverityRule{ + BaseRule: BaseRule{ + Path: "test", + }, + }, + expected: "severity should be set", + }, + { + desc: "empty rule", + rule: &SeverityRule{ + Severity: "low", + }, expected: "at least 1 of (text, source, path[-except], linters) should be set", }, { desc: "invalid path rule", rule: &SeverityRule{ + Severity: "low", BaseRule: BaseRule{ Path: "**test", }, @@ -40,6 +139,7 @@ func TestSeverity_Validate_error(t *testing.T) { { desc: "invalid path-except rule", rule: &SeverityRule{ + Severity: "low", BaseRule: BaseRule{ PathExcept: "**test", }, @@ -49,6 +149,7 @@ func TestSeverity_Validate_error(t *testing.T) { { desc: "invalid text rule", rule: &SeverityRule{ + Severity: "low", BaseRule: BaseRule{ Text: "**test", }, @@ -58,6 +159,7 @@ func TestSeverity_Validate_error(t *testing.T) { { desc: "invalid source rule", rule: &SeverityRule{ + Severity: "low", BaseRule: BaseRule{ Source: "**test", }, @@ -67,6 +169,7 @@ func TestSeverity_Validate_error(t *testing.T) { { desc: "path and path-expect", rule: &SeverityRule{ + Severity: "low", BaseRule: BaseRule{ Path: "test", PathExcept: "test", From 35bf4226494d86162a6f73a4752bc8e35b455e0a Mon Sep 17 00:00:00 2001 From: Fernandez Ludovic Date: Fri, 8 Mar 2024 17:22:57 +0100 Subject: [PATCH 2/3] fix: rule severity should be different than the default one --- pkg/config/severity.go | 5 +++++ pkg/config/severity_test.go | 15 +++++++++++++++ 2 files changed, 20 insertions(+) diff --git a/pkg/config/severity.go b/pkg/config/severity.go index 49874c9a4c3a..3eeb80696020 100644 --- a/pkg/config/severity.go +++ b/pkg/config/severity.go @@ -20,6 +20,11 @@ func (s *Severity) Validate() error { } for i, rule := range s.Rules { + if s.Default == rule.Severity { + return fmt.Errorf("error in severity rule #%d: the default severity (%q) is the same as the rule severity, this rule can be removed", + i, s.Default) + } + if err := rule.Validate(); err != nil { return fmt.Errorf("error in severity rule #%d: %w", i, err) } diff --git a/pkg/config/severity_test.go b/pkg/config/severity_test.go index b7a98aea328d..db61c98ba029 100644 --- a/pkg/config/severity_test.go +++ b/pkg/config/severity_test.go @@ -79,6 +79,21 @@ func TestSeverity_Validate_error(t *testing.T) { }, expected: "error in severity rule #0: severity should be set", }, + { + desc: "same severity between default and rule", + severity: &Severity{ + Default: "high", + Rules: []SeverityRule{ + { + Severity: "high", + BaseRule: BaseRule{ + Path: "test", + }, + }, + }, + }, + expected: `error in severity rule #0: the default severity ("high") is the same as the rule severity, this rule can be removed`, + }, } for _, test := range testCases { From 782e32741688f0e02d0e3c9faba520ddb92494da Mon Sep 17 00:00:00 2001 From: Fernandez Ludovic Date: Sat, 9 Mar 2024 13:56:35 +0100 Subject: [PATCH 3/3] review --- pkg/config/severity.go | 5 ----- pkg/config/severity_test.go | 29 ++++++++++++++--------------- 2 files changed, 14 insertions(+), 20 deletions(-) diff --git a/pkg/config/severity.go b/pkg/config/severity.go index 3eeb80696020..49874c9a4c3a 100644 --- a/pkg/config/severity.go +++ b/pkg/config/severity.go @@ -20,11 +20,6 @@ func (s *Severity) Validate() error { } for i, rule := range s.Rules { - if s.Default == rule.Severity { - return fmt.Errorf("error in severity rule #%d: the default severity (%q) is the same as the rule severity, this rule can be removed", - i, s.Default) - } - if err := rule.Validate(); err != nil { return fmt.Errorf("error in severity rule #%d: %w", i, err) } diff --git a/pkg/config/severity_test.go b/pkg/config/severity_test.go index db61c98ba029..049fd1663a8b 100644 --- a/pkg/config/severity_test.go +++ b/pkg/config/severity_test.go @@ -31,6 +31,20 @@ func TestSeverity_Validate(t *testing.T) { Default: "high", }, }, + { + desc: "same severity between default and rule", + severity: &Severity{ + Default: "high", + Rules: []SeverityRule{ + { + Severity: "high", + BaseRule: BaseRule{ + Path: "test", + }, + }, + }, + }, + }, } for _, test := range testCases { @@ -79,21 +93,6 @@ func TestSeverity_Validate_error(t *testing.T) { }, expected: "error in severity rule #0: severity should be set", }, - { - desc: "same severity between default and rule", - severity: &Severity{ - Default: "high", - Rules: []SeverityRule{ - { - Severity: "high", - BaseRule: BaseRule{ - Path: "test", - }, - }, - }, - }, - expected: `error in severity rule #0: the default severity ("high") is the same as the rule severity, this rule can be removed`, - }, } for _, test := range testCases {