Skip to content

Commit 3dc28bb

Browse files
charlieegan3charlieegan3
authored
bundle: Surface configured rule notices (#1827)
If a rule is configred, and set to non-ignore level, then we should show the notices from that rule if it is not run to aid users. Fixes #1795 Signed-off-by: Charlie Egan <charlie_egan@apple.com>
1 parent 593e221 commit 3dc28bb

8 files changed

Lines changed: 146 additions & 26 deletions

File tree

‎bundle/regal/config/config.rego‎

Lines changed: 4 additions & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -28,6 +28,10 @@ docs["resolve_url"](url, category) := replace(
2828
# description: the default configuration with user config merged on top (if provided)
2929
merged_config := data.internal.combined_config
3030

31+
# METADATA
32+
# description: the config the user manually configured without defaults
33+
user_config := data.internal.user_config
34+
3135
# ensure that config.rules can be referenced in tests even without mocking
3236
default rules := {}
3337

‎bundle/regal/main/main.rego‎

Lines changed: 11 additions & 9 deletions
Original file line numberDiff line numberDiff line change
@@ -10,11 +10,20 @@ package regal.main
1010

1111
import data.regal.ast
1212
import data.regal.config
13+
import data.regal.notices
1314
import data.regal.util
1415

1516
# METADATA
1617
# description: set of all notices returned from linter rules
17-
lint.notices contains _grouped_notices[_][_][_] if "lint" in input.regal.operations
18+
lint.notices contains notice if {
19+
"lint" in input.regal.operations
20+
21+
some category, title
22+
_rules_to_run[category][title]
23+
24+
rule_notices := notices.promoted_notices[category][title]
25+
some notice in rule_notices
26+
}
1827

1928
# METADATA
2029
# description: map of all ignore directives encountered when linting
@@ -57,13 +66,6 @@ _rules_to_run[category] contains title if {
5766
not config.excluded_file(category, title, relative_filename)
5867
}
5968

60-
_grouped_notices[category][title] contains notice if {
61-
some category, title
62-
_rules_to_run[category][title]
63-
64-
some notice in data.regal.rules[category][title].notices
65-
}
66-
6769
# METADATA
6870
# title: report
6971
# description: |
@@ -94,7 +96,7 @@ report contains violation if {
9496
some category, title
9597
_rules_to_run[category][title]
9698

97-
count(object.get(_grouped_notices, [category, title], [])) == 0
99+
count(object.get(notices.promoted_notices, [category, title], [])) == 0
98100

99101
some violation in data.regal.rules[category][title].report
100102

‎bundle/regal/main/main_test.rego‎

Lines changed: 0 additions & 16 deletions
Original file line numberDiff line numberDiff line change
@@ -358,22 +358,6 @@ test_rules_to_run_not_excluded if {
358358
rules_to_run == {"testing": {"test"}}
359359
}
360360

361-
test_notices if {
362-
notice := {
363-
"category": "idiomatic",
364-
"description": "here's a notice",
365-
"level": "notice",
366-
"title": "testme notice",
367-
"severity": "none",
368-
}
369-
370-
notices := main.lint.notices with main._rules_to_run as {"idiomatic": {"testme"}}
371-
with data.regal.rules.idiomatic.testme.notices as {notice} # regal ignore:unresolved-reference
372-
with input.regal.operations as ["lint"]
373-
374-
notices == {notice}
375-
}
376-
377361
test_main_fail_when_input_not_object if {
378362
violation := {
379363
"category": "error",

‎bundle/regal/notices/notices.rego‎

Lines changed: 46 additions & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -0,0 +1,46 @@
1+
# METADATA
2+
# description: |
3+
# package with functionality for post-processing notices
4+
# to ensure they are presented correctly as errors when relevant.
5+
package regal.notices
6+
7+
import data.regal.config
8+
9+
# METADATA
10+
# scope: rule
11+
# description: |
12+
# promoted_notices maps notices from rules, potentially changing their severity
13+
# based on user configuration
14+
promoted_notices[category][title] contains original_notice if {
15+
some category, title
16+
notices := data.regal.rules[category][title].notices
17+
18+
some original_notice in notices
19+
20+
not config.user_config.rules[category][title]
21+
}
22+
23+
promoted_notices[category][title] contains notice if {
24+
some category, title
25+
notices := data.regal.rules[category][title].notices
26+
27+
some notice in notices
28+
29+
rule_config := config.user_config.rules[category][title]
30+
object.get(rule_config, "level", "") == "ignore"
31+
}
32+
33+
promoted_notices[category][title] contains notice if {
34+
some category, title
35+
notices := data.regal.rules[category][title].notices
36+
37+
some original_notice in notices
38+
39+
rule_config := config.user_config.rules[category][title]
40+
object.get(rule_config, "level", "") != "ignore"
41+
42+
# Use configured level as severity, or default to "error"
43+
new_severity := object.get(rule_config, "level", "error")
44+
45+
notice := object.union(original_notice, {"severity": new_severity})
46+
}

‎bundle/regal/notices/notices_test.rego‎

Lines changed: 72 additions & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -0,0 +1,72 @@
1+
package regal.notices_test
2+
3+
import data.regal.notices
4+
5+
test_promoted_notices_with_level_error if {
6+
result := notices.promoted_notices with data.regal.rules.imports["use-rego-v1"].notices as {_example_notice}
7+
with data.internal.user_config as {"rules": {"imports": {"use-rego-v1": {"level": "error"}}}}
8+
9+
# With user config level set to error, the severity should be promoted to error
10+
result.imports["use-rego-v1"] == {object.union(_example_notice, {"severity": "error"})}
11+
}
12+
13+
test_promoted_notices_with_level_ignore if {
14+
result := notices.promoted_notices with data.regal.rules.imports["use-rego-v1"].notices as {_example_notice}
15+
with data.internal.user_config as {"rules": {"imports": {"use-rego-v1": {"level": "ignore"}}}}
16+
17+
# With user config level set to ignore, the severity should stay none
18+
result.imports["use-rego-v1"] == {_example_notice}
19+
}
20+
21+
test_promoted_notices_with_level_warning if {
22+
result := notices.promoted_notices with data.regal.rules.imports["use-rego-v1"].notices as {_example_notice}
23+
with data.internal.user_config as {"rules": {"imports": {"use-rego-v1": {"level": "warning"}}}}
24+
25+
# With user config level set to warning, the severity should be promoted to warning
26+
result.imports["use-rego-v1"] == {object.union(_example_notice, {"severity": "warning"})}
27+
}
28+
29+
test_promoted_notices_configured_without_level if {
30+
# Rule is configured but level field is not present
31+
result := notices.promoted_notices with data.regal.rules.imports["use-rego-v1"].notices as {_example_notice}
32+
with data.internal.user_config as {"rules": {"imports": {"use-rego-v1": {}}}}
33+
34+
# When configured without level, should default to error
35+
result.imports["use-rego-v1"] == {object.union(_example_notice, {"severity": "error"})}
36+
}
37+
38+
test_promoted_notices_mixed_configured_and_unconfigured if {
39+
notice_configured_rule := {
40+
"category": "imports",
41+
"description": "Configured rule",
42+
"level": "notice",
43+
"title": "use-rego-v1",
44+
"severity": "none",
45+
}
46+
47+
notice_unconfigured_rule := {
48+
"category": "bugs",
49+
"description": "Unconfigured rule",
50+
"level": "notice",
51+
"title": "deprecated-builtin",
52+
"severity": "none",
53+
}
54+
55+
result := notices.promoted_notices with data.regal.rules.imports["use-rego-v1"].notices as {notice_configured_rule}
56+
with data.regal.rules.bugs["deprecated-builtin"].notices as {notice_unconfigured_rule}
57+
with data.internal.user_config as {"rules": {"imports": {"use-rego-v1": {"level": "error"}}}}
58+
59+
# Configured rule should have severity promoted to error
60+
result.imports["use-rego-v1"] == {object.union(notice_configured_rule, {"severity": "error"})}
61+
62+
# Unconfigured rule should keep original severity: none
63+
result.bugs["deprecated-builtin"] == {notice_unconfigured_rule}
64+
}
65+
66+
_example_notice := {
67+
"category": "imports",
68+
"description": "Test rule description",
69+
"level": "notice",
70+
"title": "use-rego-v1",
71+
"severity": "none",
72+
}

‎internal/lsp/eval.go‎

Lines changed: 6 additions & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -119,6 +119,11 @@ func prepareRegoArgs(
119119
evalConfig = *cfg
120120
}
121121

122+
userConfigMap := map[string]any{}
123+
if cfg != nil {
124+
userConfigMap = config.ToMap(*cfg)
125+
}
126+
122127
internalBundle := &bundle.Bundle{
123128
Manifest: bundle.Manifest{
124129
Roots: &[]string{"internal"},
@@ -127,6 +132,7 @@ func prepareRegoArgs(
127132
Data: map[string]any{
128133
"internal": map[string]any{
129134
"combined_config": config.ToMap(evalConfig),
135+
"user_config": userConfigMap,
130136
"capabilities": caps,
131137
},
132138
},

‎internal/lsp/eval_test.go‎

Lines changed: 1 addition & 1 deletion
Original file line numberDiff line numberDiff line change
@@ -76,7 +76,7 @@ func TestEvalWorkspacePathInternalData(t *testing.T) {
7676
val := testutil.MustBe[[]any](t, res.Value)
7777
act := util.Sorted(testutil.Must(util.AnySliceTo[string](val))(t))
7878

79-
if exp := []string{"capabilities", "combined_config"}; !slices.Equal(exp, act) {
79+
if exp := []string{"capabilities", "combined_config", "user_config"}; !slices.Equal(exp, act) {
8080
t.Fatalf("expected %v, got %v", exp, act)
8181
}
8282
}

‎pkg/linter/linter.go‎

Lines changed: 6 additions & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -648,6 +648,11 @@ func (l Linter) createDataBundle(conf config.Config) *bundle.Bundle {
648648
"ignore_files": util.NilSliceToEmpty(l.ignoreFiles),
649649
}
650650

651+
userConfigMap := map[string]any{}
652+
if l.userConfig != nil {
653+
userConfigMap = config.ToMap(*l.userConfig)
654+
}
655+
651656
return &bundle.Bundle{
652657
Manifest: bundle.Manifest{
653658
Roots: &[]string{"internal", "eval"},
@@ -659,6 +664,7 @@ func (l Linter) createDataBundle(conf config.Config) *bundle.Bundle {
659664
},
660665
"internal": map[string]any{
661666
"combined_config": config.ToMap(conf),
667+
"user_config": userConfigMap,
662668
"capabilities": rio.ToMap(config.CapabilitiesForThisVersion()),
663669
"path_prefix": l.pathPrefix,
664670
},

0 commit comments

Comments
 (0)