Skip to content

Commit

Permalink
config: Support the defaulting of rules in config (#587)
Browse files Browse the repository at this point in the history
* config: Support the defaulting of rules in config

This PR implements an approach to rule defaulting described in:
#153 (comment)

Signed-off-by: Charlie Egan <charlie@styra.com>

* Address linter and refactor

Signed-off-by: Charlie Egan <charlie@styra.com>

* Add e2e test for config defaulting and flag use

Signed-off-by: Charlie Egan <charlie@styra.com>

* Add new notes for defaulting in README

Signed-off-by: Charlie Egan <charlie@styra.com>

---------

Signed-off-by: Charlie Egan <charlie@styra.com>
  • Loading branch information
charlieegan3 authored Mar 11, 2024
1 parent 9da1796 commit 9e9a30e
Show file tree
Hide file tree
Showing 8 changed files with 580 additions and 54 deletions.
59 changes: 54 additions & 5 deletions README.md
Original file line number Diff line number Diff line change
Expand Up @@ -361,13 +361,34 @@ provided will be used to override the default configuration.

## Ignoring Rules

If some rule doesn't align with your team's preferences, don't worry! Regal is not meant to be the law, and some rules
may not make sense for your project, or parts of it. Regal provides several different ways to ignore rules, either
entirely, or with more granularity.
If one of Regal's rules doesn't align with your team's preferences, don't worry! Regal is not meant to be the law,
and some rules may not make sense for your project, or parts of it.
Regal provides several different methods to ignore rules with varying precedence.
The available methods are (ranked highest to lowest precedence):

### Ignoring a Rule Entirely
- [Inline Ignore Directives](#inline-ignore-directives) cannot be overridden by any other method.
- Enabling or Disabling Rules with CLI flags.
- Enabling or Disabling Rules with `--enable` and `--disable` CLI flags.
- Enabling or Disabling Rules with `--enable-category` and `--disable-category` CLI flags.
- Enabling or Disabling All Rules with `--enable-all` and `--disable-all` CLI flags.
- See [Ignoring Rules via CLI Flags](#ignoring-rules-via-cli-flags) for more details.
- [Ignoring a Rule In Config](#ignoring-a-rule-in-config)
- [Ignoring a Category In Config](#ignoring-a-category-in-config)
- [Ignoring All Rules In Config](#ignoring-all-rules-in-config)

If you want to ignore a rule entirely, set its level to `ignore` in the configuration file:
In summary, the CLI flags will override any configuration provided in the file, and inline ignore directives for a
specific line will override any other method.

It's also possible to ignore messages on a per-file basis. The available methods are (ranked High to Lowest precedence):

- Using the `--ignore-files` CLI flag.
See [Ignoring Rules via CLI Flags](#ignoring-rules-via-cli-flags).
- [Ignoring Files Globally](#ignoring-files-globally) or
[Ignoring a Rule in Some Files](#ignoring-a-rule-in-some-files).

### Ignoring a Rule In Config

If you want to ignore a rule, set its level to `ignore` in the configuration file:

```yaml
rules:
Expand All @@ -377,6 +398,34 @@ rules:
level: ignore
```

### Ignoring a Category In Config

If you want to ignore a category of rules, set its default level to `ignore` in the configuration file:

```yaml
rules:
style:
default:
level: ignore
```

### Ignoring All Rules In Config

If you want to ignore all rules, set the default level to `ignore` in the configuration file:

```yaml
rules:
default:
level: ignore
# then you can re-enable specific rules or categories
testing:
default:
level: error
style:
opa-fmt:
level: error
```

**Tip**: providing a comment on ignored rules is a good way to communicate why the decision was made.

### Ignoring a Rule in Some Files
Expand Down
74 changes: 74 additions & 0 deletions e2e/cli_test.go
Original file line number Diff line number Diff line change
Expand Up @@ -577,6 +577,80 @@ func TestMergeRuleConfigWithoutLevel(t *testing.T) {
expectExitCode(t, err, 3, &stdout, &stderr)
}

func TestConfigDefaultingWithDisableDirective(t *testing.T) {
t.Parallel()

stdout := bytes.Buffer{}
stderr := bytes.Buffer{}

cwd := testutil.Must(os.Getwd())(t)

err := regal(&stdout, &stderr)(
"lint",
"--disable-category=testing",
"--config-file",
cwd+filepath.FromSlash("/testdata/configs/defaulting.yaml"),
cwd+filepath.FromSlash("/testdata/defaulting"),
)

// ignored by flag ignore directive
if strings.Contains(stdout.String(), "print-or-trace-call") {
t.Errorf("expected stdout to not contain print-or-trace-call")
t.Log("stdout:\n", stdout.String())
}

// ignored by config
if strings.Contains(stdout.String(), "opa-fmt") {
t.Errorf("expected stdout to not contain print-or-trace-call")
t.Log("stdout:\n", stdout.String())
}

// this error should not be ignored
if !strings.Contains(stdout.String(), "top-level-iteration") {
t.Errorf("expected stdout to contain top-level-iteration")
t.Log("stdout:\n", stdout.String())
}

expectExitCode(t, err, 3, &stdout, &stderr)
}

func TestConfigDefaultingWithEnableDirective(t *testing.T) {
t.Parallel()

stdout := bytes.Buffer{}
stderr := bytes.Buffer{}

cwd := testutil.Must(os.Getwd())(t)

err := regal(&stdout, &stderr)(
"lint",
"--enable-all",
"--config-file",
cwd+filepath.FromSlash("/testdata/configs/defaulting.yaml"),
cwd+filepath.FromSlash("/testdata/defaulting"),
)

// re-enabled by flag enable directive
if !strings.Contains(stdout.String(), "print-or-trace-call") {
t.Errorf("expected stdout to contain print-or-trace-call")
t.Log("stdout:\n", stdout.String())
}

// re-enabled by flag enable directive
if !strings.Contains(stdout.String(), "opa-fmt") {
t.Errorf("expected stdout to contain opa-fmt")
t.Log("stdout:\n", stdout.String())
}

// this error should not be ignored
if !strings.Contains(stdout.String(), "top-level-iteration") {
t.Errorf("expected stdout to contain top-level-iteration")
t.Log("stdout:\n", stdout.String())
}

expectExitCode(t, err, 3, &stdout, &stderr)
}

func TestLintWithCustomCapabilitiesAndUnmetRequirement(t *testing.T) {
t.Parallel()

Expand Down
11 changes: 11 additions & 0 deletions e2e/testdata/configs/defaulting.yaml
Original file line number Diff line number Diff line change
@@ -0,0 +1,11 @@
rules:
default:
level: ignore
bugs:
default:
level: error
rule-shadows-builtin:
level: ignore
testing:
print-or-trace-call:
level: error
9 changes: 9 additions & 0 deletions e2e/testdata/defaulting/example.rego
Original file line number Diff line number Diff line change
@@ -0,0 +1,9 @@
package p
import rego.v1
boo := input.hoo[_]
opa_fmt := "fail"
or := 1

allow if {
print("hello")
}
Loading

0 comments on commit 9e9a30e

Please sign in to comment.