From e205cd9e4a9f27c52d7d0a6e7ac250c12113def8 Mon Sep 17 00:00:00 2001 From: Anders Eknert Date: Fri, 27 Sep 2024 18:11:06 +0200 Subject: [PATCH] Go code cleanup (#1148) - Use short form `if err := ...; err != nil {` statements where applicable. - Extract some of the larger raw strings out of tests and into testdata Signed-off-by: Anders Eknert --- build/dprint.json | 4 +- cmd/fix.go | 18 +- cmd/lint.go | 3 +- cmd/new.go | 18 +- cmd/version.go | 3 +- e2e/cli_test.go | 139 ++---- internal/lsp/bundles/bundles_test.go | 3 +- internal/lsp/bundles/cache_test.go | 9 +- internal/lsp/completions/providers/policy.go | 6 +- internal/lsp/config/watcher.go | 9 +- internal/lsp/config/watcher_test.go | 12 +- internal/lsp/eval.go | 3 +- internal/lsp/eval_test.go | 6 +- internal/lsp/lint.go | 6 +- internal/lsp/rego/rego.go | 3 +- internal/lsp/server.go | 107 ++-- internal/lsp/server_template_test.go | 31 +- internal/lsp/server_test.go | 87 ++-- internal/lsp/store.go | 11 +- internal/lsp/store_test.go | 6 +- internal/novelty/holidays.go | 3 +- internal/update/update.go | 10 +- internal/update/update_test.go | 3 +- internal/util/util.go | 3 +- internal/util/util_test.go | 6 +- internal/web/server_test.go | 3 +- pkg/config/bundle.go | 3 +- pkg/config/config.go | 27 +- pkg/config/config_test.go | 6 +- pkg/config/filter.go | 3 +- pkg/config/global.go | 2 +- pkg/fixer/fileprovider/inmem.go | 6 +- pkg/fixer/fileprovider/inmem_test.go | 6 +- pkg/fixer/reporter_test.go | 6 +- pkg/linter/linter_test.go | 3 +- pkg/reporter/reporter.go | 15 +- pkg/reporter/reporter_test.go | 460 ++---------------- .../testdata/json/reporter-no-violations.json | 9 + pkg/reporter/testdata/json/reporter.json | 66 +++ pkg/reporter/testdata/junit/reporter.xml | 22 + .../testdata/pretty/reporter-long-text.txt | 8 + pkg/reporter/testdata/pretty/reporter.txt | 17 + .../testdata/sarif/reporter-no-region.json | 53 ++ .../testdata/sarif/reporter-no-violation.json | 16 + pkg/reporter/testdata/sarif/reporter.json | 114 +++++ 45 files changed, 539 insertions(+), 815 deletions(-) create mode 100644 pkg/reporter/testdata/json/reporter-no-violations.json create mode 100644 pkg/reporter/testdata/json/reporter.json create mode 100644 pkg/reporter/testdata/junit/reporter.xml create mode 100644 pkg/reporter/testdata/pretty/reporter-long-text.txt create mode 100644 pkg/reporter/testdata/pretty/reporter.txt create mode 100644 pkg/reporter/testdata/sarif/reporter-no-region.json create mode 100644 pkg/reporter/testdata/sarif/reporter-no-violation.json create mode 100644 pkg/reporter/testdata/sarif/reporter.json diff --git a/build/dprint.json b/build/dprint.json index 097fc123..50061f7e 100644 --- a/build/dprint.json +++ b/build/dprint.json @@ -3,7 +3,9 @@ "indentBlockSequenceInMap": true }, "json": {}, - "excludes": [], + "excludes": [ + "**/testdata/**" + ], "plugins": [ "https://plugins.dprint.dev/g-plane/pretty_yaml-v0.5.0.wasm", "https://plugins.dprint.dev/json-0.19.3.wasm" diff --git a/cmd/fix.go b/cmd/fix.go index cd1ca9a5..5aab3ded 100644 --- a/cmd/fix.go +++ b/cmd/fix.go @@ -98,8 +98,7 @@ The linter rules with automatic fixes available are currently: }, RunE: wrapProfiling(func(args []string) error { - err := fix(args, params) - if err != nil { + if err := fix(args, params); err != nil { log.SetOutput(os.Stderr) log.Println(err) @@ -339,8 +338,7 @@ func fix(args []string, params *fixCommandParams) error { } if fixReport.HasConflicts() { - err = r.Report(fixReport) - if err != nil { + if err = r.Report(fixReport); err != nil { return fmt.Errorf("failed to output fix report: %w", err) } @@ -425,8 +423,7 @@ please run fix from a clean state to support the use of git checkout for undo`, } for _, dir := range dirs { - err := os.Remove(dir) - if err != nil { + if err := os.Remove(dir); err != nil { return fmt.Errorf("failed to delete directory %s: %w", dir, err) } } @@ -445,20 +442,17 @@ please run fix from a clean state to support the use of git checkout for undo`, fileMode = fileInfo.Mode() } - err = os.MkdirAll(filepath.Dir(file), 0o755) - if err != nil { + if err = os.MkdirAll(filepath.Dir(file), 0o755); err != nil { return fmt.Errorf("failed to create directory for file %s: %w", file, err) } - err = os.WriteFile(file, fc, fileMode) - if err != nil { + if err = os.WriteFile(file, fc, fileMode); err != nil { return fmt.Errorf("failed to write file %s: %w", file, err) } } } - err = r.Report(fixReport) - if err != nil { + if err = r.Report(fixReport); err != nil { return fmt.Errorf("failed to output fix report: %w", err) } diff --git a/cmd/lint.go b/cmd/lint.go index 81bf998c..f4c4b80d 100644 --- a/cmd/lint.go +++ b/cmd/lint.go @@ -422,8 +422,7 @@ func formatError(format string, err error) error { buf := &bytes.Buffer{} - err := testSuites.WriteXML(buf) - if err != nil { + if err := testSuites.WriteXML(buf); err != nil { return fmt.Errorf("failed to format errors for output: %w", err) } diff --git a/cmd/new.go b/cmd/new.go index 7dff15fc..fe34ae9d 100644 --- a/cmd/new.go +++ b/cmd/new.go @@ -206,8 +206,7 @@ func addToDataYAML(params newRuleCommandParams) error { yamlEncoder := yaml.NewEncoder(&b) yamlEncoder.SetIndent(2) - err = yamlEncoder.Encode(existingConfig) - if err != nil { + if err = yamlEncoder.Encode(existingConfig); err != nil { return err } @@ -241,8 +240,7 @@ func scaffoldCustomRule(params newRuleCommandParams) error { return err } - err = ruleTmpl.Execute(ruleFile, templateValues(params)) - if err != nil { + if err = ruleTmpl.Execute(ruleFile, templateValues(params)); err != nil { return err } @@ -258,8 +256,7 @@ func scaffoldCustomRule(params newRuleCommandParams) error { return err } - err = testTmpl.Execute(testFile, templateValues(params)) - if err != nil { + if err = testTmpl.Execute(testFile, templateValues(params)); err != nil { return err } @@ -287,8 +284,7 @@ func scaffoldBuiltinRule(params newRuleCommandParams) error { return err } - err = ruleTmpl.Execute(ruleFile, templateValues(params)) - if err != nil { + if err = ruleTmpl.Execute(ruleFile, templateValues(params)); err != nil { return err } @@ -304,8 +300,7 @@ func scaffoldBuiltinRule(params newRuleCommandParams) error { return err } - err = testTmpl.Execute(testFile, templateValues(params)) - if err != nil { + if err = testTmpl.Execute(testFile, templateValues(params)); err != nil { return err } @@ -335,8 +330,7 @@ func createBuiltinDocs(params newRuleCommandParams) error { return err } - err = docTmpl.Execute(docFile, templateValues(params)) - if err != nil { + if err = docTmpl.Execute(docFile, templateValues(params)); err != nil { return err } diff --git a/cmd/version.go b/cmd/version.go index 3259a735..31d3b7f5 100644 --- a/cmd/version.go +++ b/cmd/version.go @@ -40,8 +40,7 @@ func init() { case formatJSON: e := encoding.JSON().NewEncoder(os.Stdout) e.SetIndent("", " ") - err := e.Encode(vi) - if err != nil { + if err := e.Encode(vi); err != nil { log.SetOutput(os.Stderr) log.Println(err) os.Exit(1) diff --git a/e2e/cli_test.go b/e2e/cli_test.go index 34bb5968..f62c0998 100644 --- a/e2e/cli_test.go +++ b/e2e/cli_test.go @@ -39,9 +39,7 @@ func readProvidedConfig(t *testing.T) config.Config { } var cfg config.Config - - err = yaml.Unmarshal(bs, &cfg) - if err != nil { + if err = yaml.Unmarshal(bs, &cfg); err != nil { t.Fatalf("failed to unmarshal config: %v", err) } @@ -103,14 +101,11 @@ func TestLintEmptyDir(t *testing.T) { }, }, } { - tc := tc t.Run(tc.format, func(t *testing.T) { t.Parallel() out := bytes.Buffer{} - - err := regal(&out)("lint", "--format", tc.format, t.TempDir()) - if err != nil { + if err := regal(&out)("lint", "--format", tc.format, t.TempDir()); err != nil { t.Fatalf("%v %[1]T", err) } @@ -126,8 +121,7 @@ func TestLintNonExistentDir(t *testing.T) { t.Parallel() - stdout := bytes.Buffer{} - stderr := bytes.Buffer{} + stdout, stderr := bytes.Buffer{}, bytes.Buffer{} td := t.TempDir() err := regal(&stdout, &stderr)("lint", td+filepath.FromSlash("/what/ever")) @@ -147,8 +141,7 @@ func TestLintNonExistentDir(t *testing.T) { func TestLintProposeToRunFix(t *testing.T) { t.Parallel() - stdout := bytes.Buffer{} - stderr := bytes.Buffer{} + stdout, stderr := bytes.Buffer{}, bytes.Buffer{} cwd := testutil.Must(os.Getwd())(t) @@ -171,9 +164,7 @@ func TestLintProposeToRunFix(t *testing.T) { func TestLintAllViolations(t *testing.T) { t.Parallel() - - stdout := bytes.Buffer{} - stderr := bytes.Buffer{} + stdout, stderr := bytes.Buffer{}, bytes.Buffer{} cwd := testutil.Must(os.Getwd())(t) cfg := readProvidedConfig(t) @@ -187,9 +178,7 @@ func TestLintAllViolations(t *testing.T) { } var rep report.Report - - err = json.Unmarshal(stdout.Bytes(), &rep) - if err != nil { + if err = json.Unmarshal(stdout.Bytes(), &rep); err != nil { t.Fatalf("expected JSON response, got %v", stdout.String()) } @@ -230,8 +219,7 @@ func TestLintAllViolations(t *testing.T) { func TestLintNotRegoV1Violations(t *testing.T) { t.Parallel() - stdout := bytes.Buffer{} - stderr := bytes.Buffer{} + stdout, stderr := bytes.Buffer{}, bytes.Buffer{} cwd := testutil.Must(os.Getwd())(t) @@ -246,9 +234,7 @@ func TestLintNotRegoV1Violations(t *testing.T) { } var rep report.Report - - err = json.Unmarshal(stdout.Bytes(), &rep) - if err != nil { + if err = json.Unmarshal(stdout.Bytes(), &rep); err != nil { t.Fatalf("expected JSON response, got %v", stdout.String()) } @@ -287,8 +273,7 @@ func TestLintFailsNonExistentConfigFile(t *testing.T) { cwd := testutil.Must(os.Getwd())(t) - stdout := bytes.Buffer{} - stderr := bytes.Buffer{} + stdout, stderr := bytes.Buffer{}, bytes.Buffer{} err := regal(&stdout, &stderr)("lint", "--config-file", cwd+filepath.FromSlash("/testdata/configs/non_existent_test_file.yaml"), @@ -306,8 +291,7 @@ func TestLintRuleIgnoreFiles(t *testing.T) { cwd := testutil.Must(os.Getwd())(t) - stdout := bytes.Buffer{} - stderr := bytes.Buffer{} + stdout, stderr := bytes.Buffer{}, bytes.Buffer{} err := regal(&stdout, &stderr)("lint", "--format", "json", "--config-file", cwd+filepath.FromSlash("/testdata/configs/ignore_files_prefer_snake_case.yaml"), @@ -320,9 +304,7 @@ func TestLintRuleIgnoreFiles(t *testing.T) { } var rep report.Report - - err = json.Unmarshal(stdout.Bytes(), &rep) - if err != nil { + if err = json.Unmarshal(stdout.Bytes(), &rep); err != nil { t.Fatalf("expected JSON response, got %v", stdout.String()) } @@ -341,9 +323,7 @@ func TestLintWithDebugOption(t *testing.T) { t.Parallel() cwd := testutil.Must(os.Getwd())(t) - - stdout := bytes.Buffer{} - stderr := bytes.Buffer{} + stdout, stderr := bytes.Buffer{}, bytes.Buffer{} err := regal(&stdout, &stderr)("lint", "--debug", "--config-file", cwd+filepath.FromSlash("/testdata/configs/ignore_files_prefer_snake_case.yaml"), @@ -360,9 +340,7 @@ func TestLintRuleNamingConventionFromCustomCategory(t *testing.T) { t.Parallel() cwd := testutil.Must(os.Getwd())(t) - - stdout := bytes.Buffer{} - stderr := bytes.Buffer{} + stdout, stderr := bytes.Buffer{}, bytes.Buffer{} err := regal(&stdout, &stderr)("lint", "--format", "json", "--config-file", cwd+filepath.FromSlash("/testdata/configs/custom_naming_convention.yaml"), @@ -375,9 +353,7 @@ func TestLintRuleNamingConventionFromCustomCategory(t *testing.T) { } var rep report.Report - - err = json.Unmarshal(stdout.Bytes(), &rep) - if err != nil { + if err = json.Unmarshal(stdout.Bytes(), &rep); err != nil { t.Fatalf("expected JSON response, got %v", stdout.String()) } @@ -403,8 +379,7 @@ func TestAggregatesAreCollectedAndUsed(t *testing.T) { basedir := cwd + filepath.FromSlash("/testdata/aggregates") t.Run("two policies — no violations expected", func(t *testing.T) { - stdout := bytes.Buffer{} - stderr := bytes.Buffer{} + stdout, stderr := bytes.Buffer{}, bytes.Buffer{} err := regal(&stdout, &stderr)("lint", "--format", "json", "--rules", basedir+filepath.FromSlash("/custom/regal/rules/testcase/aggregates/custom_rules_using_aggregates.rego"), @@ -418,8 +393,7 @@ func TestAggregatesAreCollectedAndUsed(t *testing.T) { }) t.Run("single policy — no aggregate violations expected", func(t *testing.T) { - stdout := bytes.Buffer{} - stderr := bytes.Buffer{} + stdout, stderr := bytes.Buffer{}, bytes.Buffer{} err := regal(&stdout, &stderr)("lint", "--format", "json", "--rules", basedir+filepath.FromSlash("/custom/regal/rules/testcase/aggregates/custom_rules_using_aggregates.rego"), @@ -433,8 +407,7 @@ func TestAggregatesAreCollectedAndUsed(t *testing.T) { }) t.Run("three policies - violation expected", func(t *testing.T) { - stdout := bytes.Buffer{} - stderr := bytes.Buffer{} + stdout, stderr := bytes.Buffer{}, bytes.Buffer{} err := regal(&stdout, &stderr)("lint", "--format", "json", "--rules", basedir+filepath.FromSlash("/custom/regal/rules/testcase/aggregates/custom_rules_using_aggregates.rego"), @@ -461,9 +434,7 @@ func TestAggregatesAreCollectedAndUsed(t *testing.T) { func TestLintAggregateIgnoreDirective(t *testing.T) { t.Parallel() - stdout := bytes.Buffer{} - stderr := bytes.Buffer{} - + stdout, stderr := bytes.Buffer{}, bytes.Buffer{} cwd := testutil.Must(os.Getwd())(t) err := regal(&stdout, &stderr)("lint", "--format", "json", @@ -477,8 +448,7 @@ func TestLintAggregateIgnoreDirective(t *testing.T) { var rep report.Report - err = json.Unmarshal(stdout.Bytes(), &rep) - if err != nil { + if err = json.Unmarshal(stdout.Bytes(), &rep); err != nil { t.Fatalf("expected JSON response, got %v", stdout.String()) } @@ -503,9 +473,7 @@ func TestLintAggregateIgnoreDirective(t *testing.T) { func TestTestRegalBundledBundle(t *testing.T) { t.Parallel() - stdout := bytes.Buffer{} - stderr := bytes.Buffer{} - + stdout, stderr := bytes.Buffer{}, bytes.Buffer{} cwd := testutil.Must(os.Getwd())(t) err := regal(&stdout, &stderr)("test", "--format", "json", cwd+filepath.FromSlash("/../bundle")) @@ -518,8 +486,7 @@ func TestTestRegalBundledBundle(t *testing.T) { var res []tester.Result - err = json.Unmarshal(stdout.Bytes(), &res) - if err != nil { + if err = json.Unmarshal(stdout.Bytes(), &res); err != nil { t.Fatalf("expected JSON response, got %v", stdout.String()) } } @@ -527,13 +494,10 @@ func TestTestRegalBundledBundle(t *testing.T) { func TestTestRegalBundledRules(t *testing.T) { t.Parallel() - stdout := bytes.Buffer{} - stderr := bytes.Buffer{} - + stdout, stderr := bytes.Buffer{}, bytes.Buffer{} cwd := testutil.Must(os.Getwd())(t) - err := regal(&stdout, &stderr)("test", "--format", "json", - cwd+filepath.FromSlash("/testdata/custom_rules")) + err := regal(&stdout, &stderr)("test", "--format", "json", cwd+filepath.FromSlash("/testdata/custom_rules")) expectExitCode(t, err, 0, &stdout, &stderr) @@ -542,9 +506,7 @@ func TestTestRegalBundledRules(t *testing.T) { } var res []tester.Result - - err = json.Unmarshal(stdout.Bytes(), &res) - if err != nil { + if err = json.Unmarshal(stdout.Bytes(), &res); err != nil { t.Fatalf("expected JSON response, got %v", stdout.String()) } } @@ -552,9 +514,7 @@ func TestTestRegalBundledRules(t *testing.T) { func TestTestRegalTestWithExtendedASTTypeChecking(t *testing.T) { t.Parallel() - stdout := bytes.Buffer{} - stderr := bytes.Buffer{} - + stdout, stderr := bytes.Buffer{}, bytes.Buffer{} cwd := testutil.Must(os.Getwd())(t) err := regal(&stdout, &stderr)("test", cwd+filepath.FromSlash("/testdata/ast_type_failure")) @@ -586,8 +546,7 @@ func TestCreateNewCustomRuleFromTemplate(t *testing.T) { t.Skip("temporarily skipping this test on Windows") } - stdout := bytes.Buffer{} - stderr := bytes.Buffer{} + stdout, stderr := bytes.Buffer{}, bytes.Buffer{} tmpDir := t.TempDir() expectExitCode(t, regal(&stdout, &stderr)( @@ -611,8 +570,7 @@ func TestCreateNewBuiltinRuleFromTemplate(t *testing.T) { t.Skip("temporarily skipping this test on Windows") } - stdout := bytes.Buffer{} - stderr := bytes.Buffer{} + stdout, stderr := bytes.Buffer{}, bytes.Buffer{} tmpDir := t.TempDir() expectExitCode(t, regal(&stdout, &stderr)( @@ -632,9 +590,7 @@ func TestCreateNewBuiltinRuleFromTemplate(t *testing.T) { func TestMergeRuleConfigWithoutLevel(t *testing.T) { t.Parallel() - stdout := bytes.Buffer{} - stderr := bytes.Buffer{} - + stdout, stderr := bytes.Buffer{}, bytes.Buffer{} cwd := testutil.Must(os.Getwd())(t) // No violations from the built-in configuration in the policy provided, but @@ -649,9 +605,7 @@ func TestMergeRuleConfigWithoutLevel(t *testing.T) { func TestConfigDefaultingWithDisableDirective(t *testing.T) { t.Parallel() - stdout := bytes.Buffer{} - stderr := bytes.Buffer{} - + stdout, stderr := bytes.Buffer{}, bytes.Buffer{} cwd := testutil.Must(os.Getwd())(t) err := regal(&stdout, &stderr)( @@ -686,9 +640,7 @@ func TestConfigDefaultingWithDisableDirective(t *testing.T) { func TestConfigDefaultingWithEnableDirective(t *testing.T) { t.Parallel() - stdout := bytes.Buffer{} - stderr := bytes.Buffer{} - + stdout, stderr := bytes.Buffer{}, bytes.Buffer{} cwd := testutil.Must(os.Getwd())(t) err := regal(&stdout, &stderr)( @@ -723,9 +675,7 @@ func TestConfigDefaultingWithEnableDirective(t *testing.T) { func TestLintWithCustomCapabilitiesAndUnmetRequirement(t *testing.T) { t.Parallel() - stdout := bytes.Buffer{} - stderr := bytes.Buffer{} - + stdout, stderr := bytes.Buffer{}, bytes.Buffer{} cwd := testutil.Must(os.Getwd())(t) // Test that the custom-has-key rule is skipped due to the custom capabilities provided where we @@ -750,9 +700,7 @@ func TestLintWithCustomCapabilitiesAndUnmetRequirement(t *testing.T) { func TestLintWithCustomCapabilitiesAndUnmetRequirementMultipleFiles(t *testing.T) { t.Parallel() - stdout := bytes.Buffer{} - stderr := bytes.Buffer{} - + stdout, stderr := bytes.Buffer{}, bytes.Buffer{} cwd := testutil.Must(os.Getwd())(t) // Test that the custom-has-key rule is skipped due to the custom capabilities provided where we @@ -779,9 +727,7 @@ func TestLintPprof(t *testing.T) { const pprofFile = "clock.pprof" - stdout := bytes.Buffer{} - stderr := bytes.Buffer{} - + stdout, stderr := bytes.Buffer{}, bytes.Buffer{} cwd := testutil.Must(os.Getwd())(t) t.Cleanup(func() { @@ -792,8 +738,7 @@ func TestLintPprof(t *testing.T) { expectExitCode(t, err, 3, &stdout, &stderr) - _, err = os.Stat(pprofFile) - if err != nil { + if _, err = os.Stat(pprofFile); err != nil { t.Fatalf("expected to find %s, got error: %v", pprofFile, err) } } @@ -801,8 +746,7 @@ func TestLintPprof(t *testing.T) { func TestFix(t *testing.T) { t.Parallel() - stdout := bytes.Buffer{} - stderr := bytes.Buffer{} + stdout, stderr := bytes.Buffer{}, bytes.Buffer{} td := t.TempDir() initialState := map[string]string{ @@ -922,10 +866,7 @@ test_allow := true } for file, expectedContent := range expectedState { - bs, err := os.ReadFile(filepath.Join(td, file)) - if err != nil { - t.Fatalf("failed to read %s: %v", file, err) - } + bs := testutil.Must(os.ReadFile(filepath.Join(td, file)))(t) if act := string(bs); expectedContent != act { t.Errorf("expected %s contents:\n%s\ngot\n%s", file, expectedContent, act) @@ -1003,10 +944,7 @@ Cannot move multiple files to: bar/bar.rego } for file, expectedContent := range initialState { - bs, err := os.ReadFile(filepath.Join(td, file)) - if err != nil { - t.Fatalf("failed to read %s: %v", file, err) - } + bs := testutil.Must(os.ReadFile(filepath.Join(td, file)))(t) if act := string(bs); expectedContent != act { t.Errorf("expected %s contents:\n%s\ngot\n%s", file, expectedContent, act) @@ -1224,8 +1162,7 @@ func mustWriteToFile(t *testing.T, path string, content string) { t.Fatalf("failed to create directory %s: %v", filepath.Dir(path), err) } - err := os.WriteFile(path, []byte(content), 0o644) - if err != nil { + if err := os.WriteFile(path, []byte(content), 0o644); err != nil { t.Fatalf("failed to write to %s: %v", path, err) } } diff --git a/internal/lsp/bundles/bundles_test.go b/internal/lsp/bundles/bundles_test.go index f63c9846..2f0d76d2 100644 --- a/internal/lsp/bundles/bundles_test.go +++ b/internal/lsp/bundles/bundles_test.go @@ -78,8 +78,7 @@ func TestLoadDataBundle(t *testing.T) { t.Fatalf("failed to create directory %s: %v", dir, err) } - err := os.WriteFile(filePath, []byte(contents), 0o600) - if err != nil { + if err := os.WriteFile(filePath, []byte(contents), 0o600); err != nil { t.Fatalf("failed to write file %s: %v", filePath, err) } } diff --git a/internal/lsp/bundles/cache_test.go b/internal/lsp/bundles/cache_test.go index 2068a7d4..01a87e39 100644 --- a/internal/lsp/bundles/cache_test.go +++ b/internal/lsp/bundles/cache_test.go @@ -28,8 +28,7 @@ func TestRefresh(t *testing.T) { t.Fatalf("failed to create directory %s: %v", dir, err) } - err := os.WriteFile(filePath, []byte(contents), 0o600) - if err != nil { + if err := os.WriteFile(filePath, []byte(contents), 0o600); err != nil { t.Fatalf("failed to write file %s: %v", filePath, err) } } @@ -149,13 +148,11 @@ func TestRefresh(t *testing.T) { } // remove the foo bundle - err = os.RemoveAll(filepath.Join(workspacePath, "foo")) - if err != nil { + if err = os.RemoveAll(filepath.Join(workspacePath, "foo")); err != nil { t.Fatalf("failed to remove foo bundle: %v", err) } - _, err = c.Refresh() - if err != nil { + if _, err = c.Refresh(); err != nil { t.Fatalf("failed to refresh cache: %v", err) } diff --git a/internal/lsp/completions/providers/policy.go b/internal/lsp/completions/providers/policy.go index 5e936bf2..08195b42 100644 --- a/internal/lsp/completions/providers/policy.go +++ b/internal/lsp/completions/providers/policy.go @@ -105,8 +105,7 @@ func (p *Policy) Run( completions := make([]types.CompletionItem, 8) - err = rio.JSONRoundTrip(result["completions"], &completions) - if err != nil { + if err = rio.JSONRoundTrip(result["completions"], &completions); err != nil { return nil, fmt.Errorf("failed converting completions: %w", err) } @@ -131,8 +130,7 @@ func prepareQuery(store storage.Store, query string) (*rego.PreparedEvalQuery, e return nil, fmt.Errorf("failed preparing query: %s, %w", query, err) } - err = store.Commit(context.Background(), txn) - if err != nil { + if err = store.Commit(context.Background(), txn); err != nil { return nil, fmt.Errorf("failed committing transaction: %w", err) } diff --git a/internal/lsp/config/watcher.go b/internal/lsp/config/watcher.go index 0c2637f8..fd8e1431 100644 --- a/internal/lsp/config/watcher.go +++ b/internal/lsp/config/watcher.go @@ -74,8 +74,7 @@ func (w *Watcher) loop(ctx context.Context) { } } - err := w.fsWatcher.Add(path) - if err != nil { + if err := w.fsWatcher.Add(path); err != nil { fmt.Fprintf(w.errorWriter, "failed to add watch: %v\n", err) } @@ -101,8 +100,7 @@ func (w *Watcher) loop(ctx context.Context) { case err := <-w.fsWatcher.Errors: fmt.Fprintf(w.errorWriter, "config watcher error: %v\n", err) case <-ctx.Done(): - err := w.Stop() - if err != nil { + if err := w.Stop(); err != nil { fmt.Fprintf(w.errorWriter, "failed to stop watcher: %v\n", err) } @@ -117,8 +115,7 @@ func (w *Watcher) Watch(configFilePath string) { func (w *Watcher) Stop() error { if w.fsWatcher != nil { - err := w.fsWatcher.Close() - if err != nil { + if err := w.fsWatcher.Close(); err != nil { return fmt.Errorf("failed to close fsnotify watcher: %w", err) } diff --git a/internal/lsp/config/watcher_test.go b/internal/lsp/config/watcher_test.go index d2d546e8..2b925440 100644 --- a/internal/lsp/config/watcher_test.go +++ b/internal/lsp/config/watcher_test.go @@ -18,8 +18,7 @@ func TestWatcher(t *testing.T) { foo: bar ` - err := os.WriteFile(configFilePath, []byte(configFileContents), 0o600) - if err != nil { + if err := os.WriteFile(configFilePath, []byte(configFileContents), 0o600); err != nil { t.Fatal(err) } @@ -29,8 +28,7 @@ foo: bar defer cancel() go func() { - err := watcher.Start(ctx) - if err != nil { + if err := watcher.Start(ctx); err != nil { t.Errorf("failed to start watcher: %v", err) } }() @@ -47,8 +45,7 @@ foo: bar foo: baz ` - err = os.WriteFile(configFilePath, []byte(newConfigFileContents), 0o600) - if err != nil { + if err := os.WriteFile(configFilePath, []byte(newConfigFileContents), 0o600); err != nil { t.Fatal(err) } @@ -58,8 +55,7 @@ foo: baz t.Fatal("timeout waiting for config event") } - err = os.Rename(configFilePath, configFilePath+".new") - if err != nil { + if err := os.Rename(configFilePath, configFilePath+".new"); err != nil { t.Fatal(err) } diff --git a/internal/lsp/eval.go b/internal/lsp/eval.go index 363ea088..cf01cf8a 100644 --- a/internal/lsp/eval.go +++ b/internal/lsp/eval.go @@ -76,8 +76,7 @@ func (l *LanguageServer) Eval( json := encoding.JSON() - err = json.Unmarshal(in, &inputMap) - if err != nil { + if err = json.Unmarshal(in, &inputMap); err != nil { return nil, fmt.Errorf("failed unmarshalling input: %w", err) } diff --git a/internal/lsp/eval_test.go b/internal/lsp/eval_test.go index 95b6d275..6c9b7dfe 100644 --- a/internal/lsp/eval_test.go +++ b/internal/lsp/eval_test.go @@ -78,8 +78,7 @@ func TestFindInput(t *testing.T) { t.Errorf("expected input at %s, got %s", content, res) } - err := os.Remove(tmpDir + "/workspace/foo/bar/input.json") - if err != nil { + if err := os.Remove(tmpDir + "/workspace/foo/bar/input.json"); err != nil { t.Fatal(err) } @@ -100,8 +99,7 @@ func createWithContent(t *testing.T, path string, content string) { defer f.Close() - _, err = f.WriteString(content) - if err != nil { + if _, err = f.WriteString(content); err != nil { t.Fatal(err) } } diff --git a/internal/lsp/lint.go b/internal/lsp/lint.go index 73d0d5c3..bd5e8151 100644 --- a/internal/lsp/lint.go +++ b/internal/lsp/lint.go @@ -45,8 +45,7 @@ func updateParse( cache.SetModule(fileURI, module) - err := PutFileMod(ctx, store, fileURI, module) - if err != nil { + if err := PutFileMod(ctx, store, fileURI, module); err != nil { return false, fmt.Errorf("failed to update rego store with parsed module: %w", err) } @@ -65,8 +64,7 @@ func updateParse( ruleRefs = append(ruleRefs, ref.Label) } - err = PutFileRefs(ctx, store, fileURI, ruleRefs) - if err != nil { + if err = PutFileRefs(ctx, store, fileURI, ruleRefs); err != nil { return false, fmt.Errorf("failed to update rego store with defined refs: %w", err) } diff --git a/internal/lsp/rego/rego.go b/internal/lsp/rego/rego.go index 9f5157c7..0b484cc2 100644 --- a/internal/lsp/rego/rego.go +++ b/internal/lsp/rego/rego.go @@ -201,8 +201,7 @@ func queryToValue[T any](ctx context.Context, pq *rego.PreparedEvalQuery, policy return toValue, err //nolint:wrapcheck } - err = rio.JSONRoundTrip(result.Expressions[0].Value, &toValue) - if err != nil { + if err := rio.JSONRoundTrip(result.Expressions[0].Value, &toValue); err != nil { return toValue, fmt.Errorf("failed unmarshaling code lenses: %w", err) } diff --git a/internal/lsp/server.go b/internal/lsp/server.go index 32122232..69d3cc37 100644 --- a/internal/lsp/server.go +++ b/internal/lsp/server.go @@ -191,8 +191,7 @@ func (l *LanguageServer) Handle( case "exit": // now we can close the channel, this will cause the program to exit and the // context for all workers to be cancelled - err := l.conn.Close() - if err != nil { + if err := l.conn.Close(); err != nil { return nil, fmt.Errorf("failed to close connection: %w", err) } @@ -224,23 +223,20 @@ func (l *LanguageServer) StartDiagnosticsWorker(ctx context.Context) { // updateParse will not return an error when the parsing failed, // but only when it was impossible - _, err := updateParse(ctx, l.cache, l.regoStore, evt.URI, bis) - if err != nil { + if _, err := updateParse(ctx, l.cache, l.regoStore, evt.URI, bis); err != nil { l.logError(fmt.Errorf("failed to update module for %s: %w", evt.URI, err)) continue } // lint the file and send the diagnostics - err = updateFileDiagnostics(ctx, l.cache, l.getLoadedConfig(), evt.URI, l.workspaceRootURI) - if err != nil { + if err := updateFileDiagnostics(ctx, l.cache, l.getLoadedConfig(), evt.URI, l.workspaceRootURI); err != nil { l.logError(fmt.Errorf("failed to update file diagnostics: %w", err)) continue } - err = l.sendFileDiagnostics(ctx, evt.URI) - if err != nil { + if err := l.sendFileDiagnostics(ctx, evt.URI); err != nil { l.logError(fmt.Errorf("failed to send diagnostic: %w", err)) continue @@ -254,15 +250,13 @@ func (l *LanguageServer) StartDiagnosticsWorker(ctx context.Context) { } case <-l.diagnosticRequestWorkspace: // results will be sent in response to the next workspace/diagnostics request - err := updateAllDiagnostics(ctx, l.cache, l.getLoadedConfig(), l.workspaceRootURI) - if err != nil { + if err := updateAllDiagnostics(ctx, l.cache, l.getLoadedConfig(), l.workspaceRootURI); err != nil { l.logError(fmt.Errorf("failed to update aggregate diagnostics (trigger): %w", err)) } // send diagnostics for all files for fileURI := range l.cache.GetAllFiles() { - err = l.sendFileDiagnostics(ctx, fileURI) - if err != nil { + if err := l.sendFileDiagnostics(ctx, fileURI); err != nil { l.logError(fmt.Errorf("failed to send diagnostic: %w", err)) } } @@ -302,15 +296,13 @@ func (l *LanguageServer) StartHoverWorker(ctx context.Context) { continue } - err = hover.UpdateBuiltinPositions(l.cache, fileURI, bis) - if err != nil { + if err = hover.UpdateBuiltinPositions(l.cache, fileURI, bis); err != nil { l.logError(fmt.Errorf("failed to update builtin positions: %w", err)) continue } - err = hover.UpdateKeywordLocations(ctx, l.cache, fileURI) - if err != nil { + if err = hover.UpdateKeywordLocations(ctx, l.cache, fileURI); err != nil { l.logError(fmt.Errorf("failed to update keyword positions: %w", err)) continue @@ -327,8 +319,7 @@ func (l *LanguageServer) getLoadedConfig() *config.Config { } func (l *LanguageServer) StartConfigWorker(ctx context.Context) { - err := l.configWatcher.Start(ctx) - if err != nil { + if err := l.configWatcher.Start(ctx); err != nil { l.logError(fmt.Errorf("failed to start config watcher: %w", err)) return @@ -348,8 +339,7 @@ func (l *LanguageServer) StartConfigWorker(ctx context.Context) { var userConfig config.Config - err = yaml.Unmarshal(configFileBs, &userConfig) - if err != nil && !errors.Is(err, io.EOF) { + if err = yaml.Unmarshal(configFileBs, &userConfig); err != nil && !errors.Is(err, io.EOF) { l.logError(fmt.Errorf("failed to reload config: %w", err)) return @@ -410,8 +400,7 @@ func (l *LanguageServer) StartConfigWorker(ctx context.Context) { l.cache.SetIgnoredFileContents(k, contents) } - err := RemoveFileMod(ctx, l.regoStore, k) - if err != nil { + if err := RemoveFileMod(ctx, l.regoStore, k); err != nil { l.logError(fmt.Errorf("failed to remove mod from store: %w", err)) } } @@ -431,8 +420,7 @@ func (l *LanguageServer) StartConfigWorker(ctx context.Context) { // updating the parse here will enable things like go-to definition // to start working right away without the need for a file content // update to run updateParse. - _, err = updateParse(ctx, l.cache, l.regoStore, k, bis) - if err != nil { + if _, err = updateParse(ctx, l.cache, l.regoStore, k, bis); err != nil { l.logError(fmt.Errorf("failed to update parse for previously ignored file %q: %w", k, err)) } } @@ -532,17 +520,15 @@ func (l *LanguageServer) StartCommandWorker(ctx context.Context) { // nolint:mai break } - err := l.conn.Call(ctx, methodWorkspaceApplyEdit, renameParams, nil) - if err != nil { + if err := l.conn.Call(ctx, methodWorkspaceApplyEdit, renameParams, nil); err != nil { l.logError(fmt.Errorf("failed %s notify: %v", methodWorkspaceApplyEdit, err.Error())) } - // clean up any left over empty edits dirs + // clean up any empty edits dirs left over if len(renameParams.Edit.DocumentChanges) > 0 { dir := filepath.Dir(uri.ToPath(l.clientIdentifier, renameParams.Edit.DocumentChanges[0].OldURI)) - err := util.DeleteEmptyDirs(dir) - if err != nil { + if err := util.DeleteEmptyDirs(dir); err != nil { l.logError(fmt.Errorf("failed to delete empty directories: %w", err)) } } @@ -591,8 +577,7 @@ func (l *LanguageServer) StartCommandWorker(ctx context.Context) { // nolint:mai responseResult := map[string]any{} - err = l.conn.Call(ctx, "regal/startDebugging", responseParams, &responseResult) - if err != nil { + if err = l.conn.Call(ctx, "regal/startDebugging", responseParams, &responseResult); err != nil { l.logError(fmt.Errorf("regal/startDebugging failed: %v", err.Error())) } case "regal.eval": @@ -679,14 +664,11 @@ func (l *LanguageServer) StartCommandWorker(ctx context.Context) { // nolint:mai cleanedMessage := strings.Replace(err.Error(), l.workspaceRootURI+"/", "", 1) - err := l.conn.Notify(ctx, "window/showMessage", types.ShowMessageParams{ + if err := l.conn.Notify(ctx, "window/showMessage", types.ShowMessageParams{ Type: 1, // error Message: cleanedMessage, - }) - if err != nil { + }); err != nil { l.logError(fmt.Errorf("failed to notify client of eval error: %w", err)) - - break } break @@ -710,8 +692,7 @@ func (l *LanguageServer) StartCommandWorker(ctx context.Context) { // nolint:mai responseResult := map[string]any{} - err = l.conn.Call(ctx, "regal/showEvalResult", responseParams, &responseResult) - if err != nil { + if err = l.conn.Call(ctx, "regal/showEvalResult", responseParams, &responseResult); err != nil { l.logError(fmt.Errorf("regal/showEvalResult failed: %v", err.Error())) } } else { @@ -747,22 +728,18 @@ func (l *LanguageServer) StartCommandWorker(ctx context.Context) { // nolint:mai if err != nil { l.logError(err) - err := l.conn.Notify(ctx, "window/showMessage", types.ShowMessageParams{ + if err := l.conn.Notify(ctx, "window/showMessage", types.ShowMessageParams{ Type: 1, // error Message: err.Error(), - }) - if err != nil { + }); err != nil { l.logError(fmt.Errorf("failed to notify client of command error: %w", err)) - - break } break } if fixed { - err = l.conn.Call(ctx, methodWorkspaceApplyEdit, editParams, nil) - if err != nil { + if err = l.conn.Call(ctx, methodWorkspaceApplyEdit, editParams, nil); err != nil { l.logError(fmt.Errorf("failed %s notify: %v", methodWorkspaceApplyEdit, err.Error())) } } @@ -795,8 +772,7 @@ func (l *LanguageServer) StartWorkspaceStateWorker(ctx context.Context) { l.cache.Delete(fileURI) // then send the diagnostics message based on the cleared cache - err = l.sendFileDiagnostics(ctx, fileURI) - if err != nil { + if err = l.sendFileDiagnostics(ctx, fileURI); err != nil { l.logError(fmt.Errorf("failed to send diagnostic: %w", err)) } } @@ -908,13 +884,12 @@ func (l *LanguageServer) StartTemplateWorker(ctx context.Context) { l.cache.Delete(renameParams.Edit.DocumentChanges[0].OldURI) } - err = l.conn.Call(ctx, methodWorkspaceApplyEdit, map[string]any{ + if err = l.conn.Call(ctx, methodWorkspaceApplyEdit, map[string]any{ "label": "Template new Rego file", "edit": map[string]any{ "documentChanges": edits, }, - }, nil) - if err != nil { + }, nil); err != nil { l.logError(fmt.Errorf("failed %s notify: %v", methodWorkspaceApplyEdit, err.Error())) } @@ -1149,13 +1124,11 @@ func (l *LanguageServer) processHoverContentUpdate(ctx context.Context, fileURI return nil } - err = hover.UpdateBuiltinPositions(l.cache, fileURI, bis) - if err != nil { + if err = hover.UpdateBuiltinPositions(l.cache, fileURI, bis); err != nil { return fmt.Errorf("failed to update builtin positions: %w", err) } - err = hover.UpdateKeywordLocations(ctx, l.cache, fileURI) - if err != nil { + if err = hover.UpdateKeywordLocations(ctx, l.cache, fileURI); err != nil { return fmt.Errorf("failed to update keyword locations: %w", err) } @@ -1790,8 +1763,7 @@ func (l *LanguageServer) handleTextDocumentDidSave( Message: "CRLF line ending detected. Please change editor setting to use LF for line endings.", } - err := l.conn.Notify(ctx, "window/showMessage", resp) - if err != nil { + if err := l.conn.Notify(ctx, "window/showMessage", resp); err != nil { l.logError(fmt.Errorf("failed to notify: %w", err)) return struct{}{}, nil @@ -2012,12 +1984,11 @@ func (l *LanguageServer) handleWorkspaceDidCreateFiles( } for _, createOp := range params.Files { - _, _, err = cache.UpdateCacheForURIFromDisk( + if _, _, err = cache.UpdateCacheForURIFromDisk( l.cache, uri.FromPath(l.clientIdentifier, createOp.URI), uri.ToPath(l.clientIdentifier, createOp.URI), - ) - if err != nil { + ); err != nil { return nil, fmt.Errorf("failed to update cache for uri %q: %w", createOp.URI, err) } @@ -2051,8 +2022,7 @@ func (l *LanguageServer) handleWorkspaceDidDeleteFiles( for _, deleteOp := range params.Files { l.cache.Delete(deleteOp.URI) - err := l.sendFileDiagnostics(ctx, deleteOp.URI) - if err != nil { + if err := l.sendFileDiagnostics(ctx, deleteOp.URI); err != nil { l.logError(fmt.Errorf("failed to send diagnostic: %w", err)) } } @@ -2092,8 +2062,7 @@ func (l *LanguageServer) handleWorkspaceDidRenameFiles( // clear the cache and send diagnostics for the old URI to clear the client l.cache.Delete(renameOp.OldURI) - err := l.sendFileDiagnostics(ctx, renameOp.OldURI) - if err != nil { + if err := l.sendFileDiagnostics(ctx, renameOp.OldURI); err != nil { l.logError(fmt.Errorf("failed to send diagnostic: %w", err)) } @@ -2251,8 +2220,7 @@ func (l *LanguageServer) handleInitialize( l.configWatcher.Watch(configFile.Name()) } - _, err = l.loadWorkspaceContents(ctx, false) - if err != nil { + if _, err = l.loadWorkspaceContents(ctx, false); err != nil { return nil, fmt.Errorf("failed to load workspace contents: %w", err) } @@ -2310,8 +2278,7 @@ func (l *LanguageServer) loadWorkspaceContents(ctx context.Context, newOnly bool bis := l.builtinsForCurrentCapabilities() - _, err = updateParse(ctx, l.cache, l.regoStore, fileURI, bis) - if err != nil { + if _, err = updateParse(ctx, l.cache, l.regoStore, fileURI, bis); err != nil { return fmt.Errorf("failed to update parse: %w", err) } @@ -2323,8 +2290,7 @@ func (l *LanguageServer) loadWorkspaceContents(ctx context.Context, newOnly bool } if l.bundleCache != nil { - _, err := l.bundleCache.Refresh() - if err != nil { + if _, err := l.bundleCache.Refresh(); err != nil { return nil, fmt.Errorf("failed to refresh the bundle cache: %w", err) } } @@ -2396,8 +2362,7 @@ func (l *LanguageServer) sendFileDiagnostics(ctx context.Context, fileURI string URI: fileURI, } - err := l.conn.Notify(ctx, methodTextDocumentPublishDiagnostics, resp) - if err != nil { + if err := l.conn.Notify(ctx, methodTextDocumentPublishDiagnostics, resp); err != nil { return fmt.Errorf("failed to notify: %w", err) } diff --git a/internal/lsp/server_template_test.go b/internal/lsp/server_template_test.go index 98892594..8921ff2f 100644 --- a/internal/lsp/server_template_test.go +++ b/internal/lsp/server_template_test.go @@ -89,13 +89,11 @@ func TestTemplateContentsForFile(t *testing.T) { for f, c := range tc.DiskContents { dir := filepath.Dir(f) - err := os.MkdirAll(filepath.Join(td, dir), 0o755) - if err != nil { + if err := os.MkdirAll(filepath.Join(td, dir), 0o755); err != nil { t.Fatalf("failed to create directory %s: %s", dir, err) } - err = os.WriteFile(filepath.Join(td, f), []byte(c), 0o600) - if err != nil { + if err := os.WriteFile(filepath.Join(td, f), []byte(c), 0o600); err != nil { t.Fatalf("failed to write file %s: %s", f, err) } } @@ -127,8 +125,6 @@ func TestTemplateContentsForFile(t *testing.T) { func TestNewFileTemplating(t *testing.T) { t.Parallel() - var err error - // set up the workspace content with some example rego and regal config tempDir := t.TempDir() @@ -142,13 +138,11 @@ func TestNewFileTemplating(t *testing.T) { } for f, fc := range files { - err := os.MkdirAll(filepath.Dir(filepath.Join(tempDir, f)), 0o755) - if err != nil { + if err := os.MkdirAll(filepath.Dir(filepath.Join(tempDir, f)), 0o755); err != nil { t.Fatalf("failed to create directory %s: %s", filepath.Dir(filepath.Join(tempDir, f)), err) } - err = os.WriteFile(filepath.Join(tempDir, f), []byte(fc), 0o600) - if err != nil { + if err := os.WriteFile(filepath.Join(tempDir, f), []byte(fc), 0o600); err != nil { t.Fatalf("failed to write file %s: %s", f, err) } } @@ -188,15 +182,13 @@ func TestNewFileTemplating(t *testing.T) { var response types.InitializeResult - err = connClient.Call(ctx, "initialize", request, &response) - if err != nil { + if err := connClient.Call(ctx, "initialize", request, &response); err != nil { t.Fatalf("failed to send initialize request: %s", err) } // 2. Client sends initialized notification no response to the call is // expected - err = connClient.Call(ctx, "initialized", struct{}{}, nil) - if err != nil { + if err := connClient.Call(ctx, "initialized", struct{}{}, nil); err != nil { t.Fatalf("failed to send initialized notification: %s", err) } @@ -220,23 +212,20 @@ func TestNewFileTemplating(t *testing.T) { newFileURI := uri.FromPath(clients.IdentifierGeneric, newFilePath) expectedNewFileURI := uri.FromPath(clients.IdentifierGeneric, filepath.Join(tempDir, "foo/bar_test/policy_test.rego")) - err = os.MkdirAll(filepath.Dir(newFilePath), 0o755) - if err != nil { + if err := os.MkdirAll(filepath.Dir(newFilePath), 0o755); err != nil { t.Fatalf("failed to create directory %s: %s", filepath.Dir(newFilePath), err) } - err = os.WriteFile(newFilePath, []byte(""), 0o600) - if err != nil { + if err := os.WriteFile(newFilePath, []byte(""), 0o600); err != nil { t.Fatalf("failed to write file %s: %s", newFilePath, err) } // 5. Client sends workspace/didCreateFiles notification - err = connClient.Call(ctx, "workspace/didCreateFiles", types.WorkspaceDidCreateFilesParams{ + if err := connClient.Call(ctx, "workspace/didCreateFiles", types.WorkspaceDidCreateFilesParams{ Files: []types.WorkspaceDidCreateFilesParamsCreatedFile{ {URI: newFileURI}, }, - }, nil) - if err != nil { + }, nil); err != nil { t.Fatalf("failed to send didChange notification: %s", err) } diff --git a/internal/lsp/server_test.go b/internal/lsp/server_test.go index 9b948949..f4bb58ab 100644 --- a/internal/lsp/server_test.go +++ b/internal/lsp/server_test.go @@ -69,14 +69,11 @@ const fileURIScheme = "file://" func TestLanguageServerSingleFile(t *testing.T) { t.Parallel() - var err error - // set up the workspace content with some example rego and regal config tempDir := t.TempDir() mainRegoURI := fileURIScheme + tempDir + mainRegoFileName - err = os.MkdirAll(filepath.Join(tempDir, ".regal"), 0o755) - if err != nil { + if err := os.MkdirAll(filepath.Join(tempDir, ".regal"), 0o755); err != nil { t.Fatalf("failed to create .regal directory: %s", err) } @@ -96,8 +93,7 @@ rules: } for f, fc := range files { - err = os.WriteFile(filepath.Join(tempDir, f), []byte(fc), 0o600) - if err != nil { + if err := os.WriteFile(filepath.Join(tempDir, f), []byte(fc), 0o600); err != nil { t.Fatalf("failed to write file %s: %s", f, err) } } @@ -117,8 +113,7 @@ rules: if req.Method == methodTextDocumentPublishDiagnostics { var requestData types.FileDiagnostics - err = encoding.JSON().Unmarshal(*req.Params, &requestData) - if err != nil { + if err := encoding.JSON().Unmarshal(*req.Params, &requestData); err != nil { t.Fatalf("failed to unmarshal diagnostics: %s", err) } @@ -144,8 +139,7 @@ rules: var response types.InitializeResult - err = connClient.Call(ctx, "initialize", request, &response) - if err != nil { + if err := connClient.Call(ctx, "initialize", request, &response); err != nil { t.Fatalf("failed to send initialize request: %s", err) } @@ -179,8 +173,7 @@ rules: // 2. Client sends initialized notification // no response to the call is expected - err = connClient.Call(ctx, "initialized", struct{}{}, nil) - if err != nil { + if err := connClient.Call(ctx, "initialized", struct{}{}, nil); err != nil { t.Fatalf("failed to send initialized notification: %s", err) } @@ -204,7 +197,7 @@ rules: // 3. Client sends textDocument/didChange notification with new contents for main.rego // no response to the call is expected - err = connClient.Call(ctx, "textDocument/didChange", types.TextDocumentDidChangeParams{ + if err := connClient.Call(ctx, "textDocument/didChange", types.TextDocumentDidChangeParams{ TextDocument: types.TextDocumentIdentifier{ URI: mainRegoURI, }, @@ -216,8 +209,7 @@ allow := true `, }, }, - }, nil) - if err != nil { + }, nil); err != nil { t.Fatalf("failed to send didChange notification: %s", err) } @@ -250,8 +242,7 @@ rules: level: ignore ` - err = os.WriteFile(filepath.Join(tempDir, ".regal/config.yaml"), []byte(newConfigContents), 0o600) - if err != nil { + if err := os.WriteFile(filepath.Join(tempDir, ".regal/config.yaml"), []byte(newConfigContents), 0o600); err != nil { t.Fatalf("failed to write new config file: %s", err) } @@ -300,8 +291,7 @@ capabilities: version: v1.23.0 ` - err = os.WriteFile(filepath.Join(tempDir, ".regal/config.yaml"), []byte(newConfigContents), 0o600) - if err != nil { + if err := os.WriteFile(filepath.Join(tempDir, ".regal/config.yaml"), []byte(newConfigContents), 0o600); err != nil { t.Fatalf("failed to write new config file: %s", err) } @@ -340,7 +330,7 @@ capabilities: // the start of an EOPA-specific call, so if the capabilities were // loaded correctly, we should see a completion later after we ask for // it. - err = connClient.Call(ctx, "textDocument/didChange", types.TextDocumentDidChangeParams{ + if err := connClient.Call(ctx, "textDocument/didChange", types.TextDocumentDidChangeParams{ TextDocument: types.TextDocumentIdentifier{ URI: mainRegoURI, }, @@ -352,8 +342,7 @@ allow := neo4j.q `, }, }, - }, nil) - if err != nil { + }, nil); err != nil { t.Fatalf("failed to send didChange notification: %s", err) } @@ -465,8 +454,6 @@ allow := neo4j.q func TestLanguageServerMultipleFiles(t *testing.T) { t.Parallel() - var err error - // set up the workspace content with some example rego and regal config tempDir := t.TempDir() authzRegoURI := fileURIScheme + tempDir + "/authz.rego" @@ -505,19 +492,16 @@ ignore: `, } - err = os.MkdirAll(filepath.Join(tempDir, ".regal"), 0o755) - if err != nil { + if err := os.MkdirAll(filepath.Join(tempDir, ".regal"), 0o755); err != nil { t.Fatalf("failed to create .regal directory: %s", err) } - err = os.MkdirAll(filepath.Join(tempDir, "ignored"), 0o755) - if err != nil { + if err := os.MkdirAll(filepath.Join(tempDir, "ignored"), 0o755); err != nil { t.Fatalf("failed to create ignored directory: %s", err) } for f, fc := range files { - err = os.WriteFile(filepath.Join(tempDir, f), []byte(fc), 0o600) - if err != nil { + if err := os.WriteFile(filepath.Join(tempDir, f), []byte(fc), 0o600); err != nil { t.Fatalf("failed to write file %s: %s", f, err) } } @@ -526,9 +510,7 @@ ignore: ctx, cancel := context.WithCancel(context.Background()) defer cancel() - ls := NewLanguageServer(&LanguageServerOptions{ - ErrorLog: newTestLogger(t), - }) + ls := NewLanguageServer(&LanguageServerOptions{ErrorLog: newTestLogger(t)}) go ls.StartDiagnosticsWorker(ctx) go ls.StartConfigWorker(ctx) @@ -543,9 +525,7 @@ ignore: } var requestData types.FileDiagnostics - - err = encoding.JSON().Unmarshal(*req.Params, &requestData) - if err != nil { + if err := encoding.JSON().Unmarshal(*req.Params, &requestData); err != nil { t.Fatalf("failed to unmarshal diagnostics: %s", err) } @@ -575,15 +555,13 @@ ignore: var response types.InitializeResult - err = connClient.Call(ctx, "initialize", request, &response) - if err != nil { + if err := connClient.Call(ctx, "initialize", request, &response); err != nil { t.Fatalf("failed to send initialize request: %s", err) } // 2. Client sends initialized notification // no response to the call is expected - err = connClient.Call(ctx, "initialized", struct{}{}, nil) - if err != nil { + if err := connClient.Call(ctx, "initialized", struct{}{}, nil); err != nil { t.Fatalf("failed to send initialized notification: %s", err) } @@ -625,7 +603,7 @@ ignore: // 3. Client sends textDocument/didChange notification with new contents for authz.rego // no response to the call is expected - err = connClient.Call(ctx, "textDocument/didChange", types.TextDocumentDidChangeParams{ + if err := connClient.Call(ctx, "textDocument/didChange", types.TextDocumentDidChangeParams{ TextDocument: types.TextDocumentIdentifier{ URI: authzRegoURI, }, @@ -643,8 +621,7 @@ allow if input.user in admins.users `, }, }, - }, nil) - if err != nil { + }, nil); err != nil { t.Fatalf("failed to send didChange notification: %s", err) } @@ -694,8 +671,7 @@ func TestProcessBuiltinUpdateExitsOnMissingFile(t *testing.T) { ErrorLog: newTestLogger(t), }) - err := ls.processHoverContentUpdate(context.Background(), "file://missing.rego", "foo") - if err != nil { + if err := ls.processHoverContentUpdate(context.Background(), "file://missing.rego", "foo"); err != nil { t.Fatal(err) } @@ -745,8 +721,7 @@ func TestFormatting(t *testing.T) { var response types.InitializeResult - err := connClient.Call(ctx, "initialize", request, &response) - if err != nil { + if err := connClient.Call(ctx, "initialize", request, &response); err != nil { t.Fatalf("failed to send initialize request: %s", err) } @@ -837,8 +812,7 @@ allow := true } for f, fc := range files { - err = os.WriteFile(filepath.Join(parentDir, f), []byte(fc), 0o600) - if err != nil { + if err := os.WriteFile(filepath.Join(parentDir, f), []byte(fc), 0o600); err != nil { t.Fatalf("failed to write file %s: %s", f, err) } } @@ -862,9 +836,8 @@ allow := true if req.Method == methodTextDocumentPublishDiagnostics { var requestData types.FileDiagnostics - err = encoding.JSON().Unmarshal(*req.Params, &requestData) - if err != nil { - t.Fatalf("failed to unmarshal diagnostics: %s", err) + if err2 := encoding.JSON().Unmarshal(*req.Params, &requestData); err2 != nil { + t.Fatalf("failed to unmarshal diagnostics: %s", err2) } receivedMessages <- requestData @@ -889,8 +862,7 @@ allow := true var response types.InitializeResult - err = connClient.Call(ctx, "initialize", request, &response) - if err != nil { + if err := connClient.Call(ctx, "initialize", request, &response); err != nil { t.Fatalf("failed to send initialize request: %s", err) } @@ -900,8 +872,7 @@ allow := true // Client sends initialized notification // the response to the call is expected to be empty and is ignored - err = connClient.Call(ctx, "initialized", struct{}{}, nil) - if err != nil { + if err := connClient.Call(ctx, "initialized", struct{}{}, nil); err != nil { t.Fatalf("failed to send initialized notification: %s", err) } @@ -933,8 +904,8 @@ allow := true level: ignore ` - err = os.WriteFile(filepath.Join(parentDir, ".regal/config.yaml"), []byte(newConfigContents), 0o600) - if err != nil { + path := filepath.Join(parentDir, ".regal/config.yaml") + if err := os.WriteFile(path, []byte(newConfigContents), 0o600); err != nil { t.Fatalf("failed to write new config file: %s", err) } diff --git a/internal/lsp/store.go b/internal/lsp/store.go index e9032a49..827b8048 100644 --- a/internal/lsp/store.go +++ b/internal/lsp/store.go @@ -60,15 +60,13 @@ func PutFileMod(ctx context.Context, store storage.Store, fileURI string, mod *a var modMap map[string]any - err := rio.JSONRoundTrip(mod, &modMap) - if err != nil { + if err := rio.JSONRoundTrip(mod, &modMap); err != nil { return fmt.Errorf("failed to marshal module to JSON: %w", err) } - err = store.Write(ctx, txn, storage.ReplaceOp, storage.Path{"workspace", "parsed", fileURI}, modMap) + err := store.Write(ctx, txn, storage.ReplaceOp, storage.Path{"workspace", "parsed", fileURI}, modMap) if errors.As(err, &stErr) && stErr.Code == storage.NotFoundErr { - err = store.Write(ctx, txn, storage.AddOp, storage.Path{"workspace", "parsed", fileURI}, modMap) - if err != nil { + if err = store.Write(ctx, txn, storage.AddOp, storage.Path{"workspace", "parsed", fileURI}, modMap); err != nil { return fmt.Errorf("failed to init module in store: %w", err) } } @@ -95,8 +93,7 @@ func RemoveFileMod(ctx context.Context, store storage.Store, fileURI string) err return fmt.Errorf("failed to read module from store: %w", err) } - err = store.Write(ctx, txn, storage.RemoveOp, storage.Path{"workspace", "parsed", fileURI}, nil) - if err != nil { + if err = store.Write(ctx, txn, storage.RemoveOp, storage.Path{"workspace", "parsed", fileURI}, nil); err != nil { return fmt.Errorf("failed to remove module from store: %w", err) } diff --git a/internal/lsp/store_test.go b/internal/lsp/store_test.go index b9e2cd45..af24b23b 100644 --- a/internal/lsp/store_test.go +++ b/internal/lsp/store_test.go @@ -19,8 +19,7 @@ func TestPutFileModStoresRoastRepresentation(t *testing.T) { fileURI := "file:///example.rego" module := parse.MustParseModule("package example\n\nrule := true") - err := PutFileMod(ctx, store, fileURI, module) - if err != nil { + if err := PutFileMod(ctx, store, fileURI, module); err != nil { t.Fatalf("PutFileMod failed: %v", err) } @@ -89,8 +88,7 @@ func TestPutFileRefs(t *testing.T) { ctx := context.Background() fileURI := "file:///example.rego" - err := PutFileRefs(ctx, store, fileURI, []string{"foo", "bar"}) - if err != nil { + if err := PutFileRefs(ctx, store, fileURI, []string{"foo", "bar"}); err != nil { t.Fatalf("PutFileRefs failed: %v", err) } diff --git a/internal/novelty/holidays.go b/internal/novelty/holidays.go index dc6543d7..d67a5dd8 100644 --- a/internal/novelty/holidays.go +++ b/internal/novelty/holidays.go @@ -15,8 +15,7 @@ func HappyHolidays() error { flakes := []string{flake1, flake2, flake3, flake4, flake5, flake6, flake7} - err := tm.Init() - if err != nil { + if err := tm.Init(); err != nil { return fmt.Errorf("termbox init failed: %w", err) } defer tm.Close() diff --git a/internal/update/update.go b/internal/update/update.go index c9787dff..4baae03a 100644 --- a/internal/update/update.go +++ b/internal/update/update.go @@ -119,8 +119,7 @@ func getLatestVersion(ctx context.Context, opts Options) (string, error) { json := encoding.JSON() - err = json.NewDecoder(file).Decode(&preExistingState) - if err != nil { + if err := json.NewDecoder(file).Decode(&preExistingState); err != nil { return "", fmt.Errorf("failed to decode existing version state file: %w", err) } @@ -169,9 +168,7 @@ func getLatestVersion(ctx context.Context, opts Options) (string, error) { } json := encoding.JSON() - - err = json.NewDecoder(resp.Body).Decode(&responseData) - if err != nil { + if err = json.NewDecoder(resp.Body).Decode(&responseData); err != nil { return "", fmt.Errorf("failed to decode response: %w", err) } @@ -183,8 +180,7 @@ func getLatestVersion(ctx context.Context, opts Options) (string, error) { return "", fmt.Errorf("failed to marshal state file: %w", err) } - err = os.WriteFile(opts.StateDir+"/latest_version.json", stateBs, 0o600) - if err != nil { + if err = os.WriteFile(opts.StateDir+"/latest_version.json", stateBs, 0o600); err != nil { return "", fmt.Errorf("failed to write state file: %w", err) } diff --git a/internal/update/update_test.go b/internal/update/update_test.go index 2e35634c..9e87bd66 100644 --- a/internal/update/update_test.go +++ b/internal/update/update_test.go @@ -16,8 +16,7 @@ func TestCheckAndWarn(t *testing.T) { localReleasesServer := httptest.NewServer( http.HandlerFunc(func(w http.ResponseWriter, _ *http.Request) { - _, err := w.Write([]byte(`{"tag_name": "v0.2.0"}`)) - if err != nil { + if _, err := w.Write([]byte(`{"tag_name": "v0.2.0"}`)); err != nil { t.Fatal(err) } diff --git a/internal/util/util.go b/internal/util/util.go index 2d1842de..d1afc10f 100644 --- a/internal/util/util.go +++ b/internal/util/util.go @@ -138,8 +138,7 @@ func FilepathJoiner(base string) func(string) string { func DeleteEmptyDirs(dir string) error { for { // os.Remove will only delete empty directories - err := os.Remove(dir) - if err != nil { + if err := os.Remove(dir); err != nil { if os.IsExist(err) { break } diff --git a/internal/util/util_test.go b/internal/util/util_test.go index 37decfdb..9d0d3f16 100644 --- a/internal/util/util_test.go +++ b/internal/util/util_test.go @@ -115,13 +115,11 @@ func TestDirCleanUpPaths(t *testing.T) { tempDir := t.TempDir() for k, v := range test.State { - err := os.MkdirAll(filepath.Dir(filepath.Join(tempDir, k)), 0o755) - if err != nil { + if err := os.MkdirAll(filepath.Dir(filepath.Join(tempDir, k)), 0o755); err != nil { t.Fatalf("unexpected error: %v", err) } - err = os.WriteFile(filepath.Join(tempDir, k), []byte(v), 0o600) - if err != nil { + if err := os.WriteFile(filepath.Join(tempDir, k), []byte(v), 0o600); err != nil { t.Fatalf("unexpected error: %v", err) } } diff --git a/internal/web/server_test.go b/internal/web/server_test.go index 4fc7f46d..b16559dd 100644 --- a/internal/web/server_test.go +++ b/internal/web/server_test.go @@ -13,8 +13,7 @@ func TestTemplateFoundAndParsed(t *testing.T) { st := state{Code: "package main\n\nimport rego.v1\n"} - err := tpl.ExecuteTemplate(&buf, mainTemplate, st) - if err != nil { + if err := tpl.ExecuteTemplate(&buf, mainTemplate, st); err != nil { t.Fatal(err) } diff --git a/pkg/config/bundle.go b/pkg/config/bundle.go index d761fcfd..2546aede 100644 --- a/pkg/config/bundle.go +++ b/pkg/config/bundle.go @@ -38,8 +38,7 @@ func LoadConfigWithDefaultsFromBundle(regalBundle *bundle.Bundle, userConfig *Co providedRuleLevels := providedConfLevels(&defaultConfig) - err = mergo.Merge(&defaultConfig, userConfig, mergo.WithOverride) - if err != nil { + if err = mergo.Merge(&defaultConfig, userConfig, mergo.WithOverride); err != nil { return Config{}, fmt.Errorf("failed to merge user config: %w", err) } diff --git a/pkg/config/config.go b/pkg/config/config.go index 32e2e0de..68e01e22 100644 --- a/pkg/config/config.go +++ b/pkg/config/config.go @@ -246,8 +246,7 @@ func rootsFromRegalDirectory(regalDir *os.File) ([]string, error) { if err == nil { var conf Config - err = yaml.Unmarshal(file, &conf) - if err != nil { + if err = yaml.Unmarshal(file, &conf); err != nil { return nil, fmt.Errorf("failed to unmarshal config file: %w", err) } @@ -285,8 +284,7 @@ func FindConfig(path string) (*os.File, error) { func FromMap(confMap map[string]any) (Config, error) { var conf Config - err := rio.JSONRoundTrip(confMap, &conf) - if err != nil { + if err := rio.JSONRoundTrip(confMap, &conf); err != nil { return conf, fmt.Errorf("failed to convert config map to config struct: %w", err) } @@ -296,8 +294,7 @@ func FromMap(confMap map[string]any) (Config, error) { func (config Config) MarshalYAML() (any, error) { var unstructuredConfig map[string]any - err := rio.JSONRoundTrip(config, &unstructuredConfig) - if err != nil { + if err := rio.JSONRoundTrip(config, &unstructuredConfig); err != nil { return nil, fmt.Errorf("failed to created unstructured config: %w", err) } @@ -372,13 +369,11 @@ func (config *Config) UnmarshalYAML(value *yaml.Node) error { } // this call will walk the rule config and load and defaults into the config - err := extractDefaults(config, &result) - if err != nil { + if err := extractDefaults(config, &result); err != nil { return fmt.Errorf("extracting defaults failed: %w", err) } - err = extractRules(config, &result) - if err != nil { + if err := extractRules(config, &result); err != nil { return fmt.Errorf("extracting rules failed: %w", err) } @@ -497,8 +492,7 @@ func extractRules(config *Config, result *marshallingIntermediary) error { var r Rule - err := r.mapToConfig(ruleData) - if err != nil { + if err := r.mapToConfig(ruleData); err != nil { return fmt.Errorf("unmarshalling rule failed: %w", err) } @@ -519,8 +513,7 @@ func extractDefaults(c *Config, result *marshallingIntermediary) error { rawGlobalDefault, ok := result.Rules["default"] if ok { - err := c.Defaults.Global.mapToConfig(rawGlobalDefault) - if err != nil { + if err := c.Defaults.Global.mapToConfig(rawGlobalDefault); err != nil { return fmt.Errorf("unmarshalling global defaults failed: %w", err) } } @@ -535,8 +528,7 @@ func extractDefaults(c *Config, result *marshallingIntermediary) error { if ok { var categoryDefault Default - err := categoryDefault.mapToConfig(rawCategoryDefault) - if err != nil { + if err := categoryDefault.mapToConfig(rawCategoryDefault); err != nil { return fmt.Errorf("unmarshalling category defaults failed: %w", err) } @@ -671,8 +663,7 @@ func (rule *Rule) mapToConfig(result any) error { if ignore, ok := ruleMap[keyIgnore]; ok { var dst Ignore - err := rio.JSONRoundTrip(ignore, &dst) - if err != nil { + if err := rio.JSONRoundTrip(ignore, &dst); err != nil { return fmt.Errorf("unmarshalling rule ignore failed: %w", err) } diff --git a/pkg/config/config_test.go b/pkg/config/config_test.go index 6216e579..1dd16eb6 100644 --- a/pkg/config/config_test.go +++ b/pkg/config/config_test.go @@ -30,8 +30,7 @@ func TestFindRegalDirectory(t *testing.T) { path := filepath.Join(root, "/foo/bar/baz") - _, err := FindRegalDirectory(path) - if err != nil { + if _, err := FindRegalDirectory(path); err != nil { t.Error(err) } }) @@ -62,8 +61,7 @@ func TestFindConfig(t *testing.T) { test.WithTempFS(fs, func(root string) { path := filepath.Join(root, "/foo/bar/baz") - _, err := FindConfig(path) - if err != nil { + if _, err := FindConfig(path); err != nil { t.Error(err) } }) diff --git a/pkg/config/filter.go b/pkg/config/filter.go index e6e4881a..4c3e970f 100644 --- a/pkg/config/filter.go +++ b/pkg/config/filter.go @@ -58,8 +58,7 @@ func walkPaths(paths []string, filter func(path string, info os.DirEntry, err er for _, path := range paths { // We need to stat the initial set of paths, as filepath.WalkDir // will panic on non-existent paths. - _, err := os.Stat(path) - if err != nil { + if _, err := os.Stat(path); err != nil { errs = errors.Join(errs, err) continue diff --git a/pkg/config/global.go b/pkg/config/global.go index f9d09409..06aaa1d7 100644 --- a/pkg/config/global.go +++ b/pkg/config/global.go @@ -5,7 +5,7 @@ import ( "path/filepath" ) -// Dir is the config directory that will be used for user-wide configuration. +// GlobalDir is the config directory that will be used for user-wide configuration. // This is different from the .regal directories that are searched for when // linting. func GlobalDir() string { diff --git a/pkg/fixer/fileprovider/inmem.go b/pkg/fixer/fileprovider/inmem.go index c6ab53b8..d7a0eee5 100644 --- a/pkg/fixer/fileprovider/inmem.go +++ b/pkg/fixer/fileprovider/inmem.go @@ -93,13 +93,11 @@ func (p *InMemoryFileProvider) Rename(from, to string) error { } } - err := p.Put(to, content) - if err != nil { + if err := p.Put(to, content); err != nil { return fmt.Errorf("failed to put file %s: %w", to, err) } - err = p.Delete(from) - if err != nil { + if err := p.Delete(from); err != nil { return fmt.Errorf("failed to delete file %s: %w", from, err) } diff --git a/pkg/fixer/fileprovider/inmem_test.go b/pkg/fixer/fileprovider/inmem_test.go index 504ce262..147f4ef6 100644 --- a/pkg/fixer/fileprovider/inmem_test.go +++ b/pkg/fixer/fileprovider/inmem_test.go @@ -20,14 +20,12 @@ func TestFromFS(t *testing.T) { for file, content := range files { /// make the dir - err := os.MkdirAll(filepath.Dir(file), 0o755) - if err != nil { + if err := os.MkdirAll(filepath.Dir(file), 0o755); err != nil { t.Fatal(err) } // write the file - err = os.WriteFile(file, []byte(content), 0o600) - if err != nil { + if err := os.WriteFile(file, []byte(content), 0o600); err != nil { t.Fatal(err) } } diff --git a/pkg/fixer/reporter_test.go b/pkg/fixer/reporter_test.go index 59d6ae52..0a65f23b 100644 --- a/pkg/fixer/reporter_test.go +++ b/pkg/fixer/reporter_test.go @@ -61,8 +61,7 @@ func TestPrettyReporterOutput(t *testing.T) { "/workspace/bundle2/policy1.rego", ) - err := reporter.Report(report) - if err != nil { + if err := reporter.Report(report); err != nil { t.Fatalf("unexpected error: %v", err) } @@ -135,8 +134,7 @@ func TestPrettyReporterOutputWithConflicts(t *testing.T) { "/workspace/bundle1/baz.rego", ) - err := reporter.Report(report) - if err != nil { + if err := reporter.Report(report); err != nil { t.Fatalf("unexpected error: %v", err) } diff --git a/pkg/linter/linter_test.go b/pkg/linter/linter_test.go index 46edb9ed..dd3527d7 100644 --- a/pkg/linter/linter_test.go +++ b/pkg/linter/linter_test.go @@ -513,8 +513,7 @@ func TestLintWithPrintHook(t *testing.T) { WithPrintHook(topdown.NewPrintHook(&bb)). WithInputModules(&input) - _, err := linter.Lint(context.Background()) - if err != nil { + if _, err := linter.Lint(context.Background()); err != nil { t.Fatal(err) } diff --git a/pkg/reporter/reporter.go b/pkg/reporter/reporter.go index 33e9821d..5410ac05 100644 --- a/pkg/reporter/reporter.go +++ b/pkg/reporter/reporter.go @@ -200,15 +200,12 @@ Hint: %d/%d violations can be automatically fixed (%s) // Publish prints a festive report to the configured output. func (tr FestiveReporter) Publish(ctx context.Context, r report.Report) error { if os.Getenv("CI") == "" && len(r.Violations) == 0 { - err := novelty.HappyHolidays() - if err != nil { + if err := novelty.HappyHolidays(); err != nil { return fmt.Errorf("novelty message display failed: %w", err) } } - pretty := NewPrettyReporter(tr.out) - - return pretty.Publish(ctx, r) + return NewPrettyReporter(tr.out).Publish(ctx, r) } func buildPrettyViolationsTable(violations []report.Violation) string { @@ -320,8 +317,7 @@ func (tr JSONReporter) Publish(_ context.Context, r report.Report) error { // to print the GitHub Actions annotations for each violation. Finally, it prints a summary of the report suitable // for the GitHub Actions UI. func (tr GitHubReporter) Publish(ctx context.Context, r report.Report) error { - err := NewPrettyReporter(tr.out).Publish(ctx, r) - if err != nil { + if err := NewPrettyReporter(tr.out).Publish(ctx, r); err != nil { return err } @@ -330,15 +326,14 @@ func (tr GitHubReporter) Publish(ctx context.Context, r report.Report) error { } for _, violation := range r.Violations { - _, err := fmt.Fprintf(tr.out, + if _, err := fmt.Fprintf(tr.out, "::%s file=%s,line=%d,col=%d::%s\n", violation.Level, violation.Location.File, violation.Location.Row, violation.Location.Column, fmt.Sprintf("%s. To learn more, see: %s", violation.Description, getDocumentationURL(violation)), - ) - if err != nil { + ); err != nil { return err } } diff --git a/pkg/reporter/reporter_test.go b/pkg/reporter/reporter_test.go index 890b95eb..9270ae85 100644 --- a/pkg/reporter/reporter_test.go +++ b/pkg/reporter/reporter_test.go @@ -3,6 +3,7 @@ package reporter import ( "bytes" "context" + "os" "strings" "testing" @@ -85,39 +86,13 @@ func TestPrettyReporterPublish(t *testing.T) { t.Parallel() var buf bytes.Buffer - - pr := NewPrettyReporter(&buf) - - err := pr.Publish(context.Background(), rep) - if err != nil { + if err := NewPrettyReporter(&buf).Publish(context.Background(), rep); err != nil { t.Fatal(err) } - // The actual output has trailing tabs, which go fmt strips out, - // so we'll need to compare line by line with the trailing tabs removed - expectLines := strings.Split(`Rule: breaking-the-law -Description: Rego must not break the law! -Category: legal -Location: a.rego:1:1 -Text: package illegal -Documentation: https://example.com/illegal - -Rule: questionable-decision -Description: Questionable decision found -Category: really? -Location: b.rego:22:18 -Text: default allow = true -Documentation: https://example.com/questionable - -3 files linted. 2 violations found in 2 files. 1 rule skipped: -- rule-missing-capability: Rule missing capability bar - -`, "\n") - - for i, line := range strings.Split(buf.String(), "\n") { - if strings.TrimRight(line, " \t") != expectLines[i] { - t.Errorf("expected %q, got %q", expectLines[i], line) - } + expect := MustReadFile(t, "testdata/pretty/reporter.txt") + if buf.String() != expect { + t.Errorf("expected %s, got %s", expect, buf.String()) } } @@ -154,29 +129,12 @@ func TestPrettyReporterPublishLongText(t *testing.T) { } var buf bytes.Buffer - pr := NewPrettyReporter(&buf) - - err := pr.Publish(context.Background(), longRep) - if err != nil { + if err := NewPrettyReporter(&buf).Publish(context.Background(), longRep); err != nil { t.Fatal(err) } - //nolint:lll - expectLines := strings.Split(`Rule: long-violation -Description: violation with a long description -Category: long -Location: b.rego:22:18 -Text: long,long,long,long,long,long,long,long,long,long,long,long,long,long,long,long,long,long,long,long,long,long,long,lo... -Documentation: https://example.com/to-long - -3 files linted. 1 violation found. - -`, "\n") - - for i, line := range strings.Split(buf.String(), "\n") { - if got, want := strings.TrimSpace(line), expectLines[i]; got != want { - t.Fatalf("expected\n%q\ngot\n%q", want, got) - } + if expect := MustReadFile(t, "testdata/pretty/reporter-long-text.txt"); expect != buf.String() { + t.Errorf("expected %s, got %s", expect, buf.String()) } } @@ -184,11 +142,7 @@ func TestPrettyReporterPublishNoViolations(t *testing.T) { t.Parallel() var buf bytes.Buffer - - pr := NewPrettyReporter(&buf) - - err := pr.Publish(context.Background(), report.Report{}) - if err != nil { + if err := NewPrettyReporter(&buf).Publish(context.Background(), report.Report{}); err != nil { t.Fatal(err) } @@ -201,11 +155,7 @@ func TestCompactReporterPublish(t *testing.T) { t.Parallel() var buf bytes.Buffer - - cr := NewCompactReporter(&buf) - - err := cr.Publish(context.Background(), rep) - if err != nil { + if err := NewCompactReporter(&buf).Publish(context.Background(), rep); err != nil { t.Fatal(err) } @@ -217,7 +167,6 @@ func TestCompactReporterPublish(t *testing.T) { +--------------+------------------------------+ ` - if buf.String() != expect { t.Errorf("expected %s, got %s", expect, buf.String()) } @@ -227,11 +176,7 @@ func TestCompactReporterPublishNoViolations(t *testing.T) { t.Parallel() var buf bytes.Buffer - - cr := NewCompactReporter(&buf) - - err := cr.Publish(context.Background(), report.Report{}) - if err != nil { + if err := NewCompactReporter(&buf).Publish(context.Background(), report.Report{}); err != nil { t.Fatal(err) } @@ -244,82 +189,11 @@ func TestJSONReporterPublish(t *testing.T) { t.Parallel() var buf bytes.Buffer - - jr := NewJSONReporter(&buf) - - err := jr.Publish(context.Background(), rep) - if err != nil { + if err := NewJSONReporter(&buf).Publish(context.Background(), rep); err != nil { t.Fatal(err) } - expect := `{ - "violations": [ - { - "title": "breaking-the-law", - "description": "Rego must not break the law!", - "category": "legal", - "level": "error", - "related_resources": [ - { - "description": "documentation", - "ref": "https://example.com/illegal" - } - ], - "location": { - "end": { - "row": 1, - "col": 14 - }, - "text": "package illegal", - "file": "a.rego", - "col": 1, - "row": 1 - } - }, - { - "title": "questionable-decision", - "description": "Questionable decision found", - "category": "really?", - "level": "warning", - "related_resources": [ - { - "description": "documentation", - "ref": "https://example.com/questionable" - } - ], - "location": { - "text": "default allow = true", - "file": "b.rego", - "col": 18, - "row": 22 - } - } - ], - "notices": [ - { - "title": "rule-made-obsolete", - "description": "Rule made obsolete by capability foo", - "category": "some-category", - "level": "notice", - "severity": "none" - }, - { - "title": "rule-missing-capability", - "description": "Rule missing capability bar", - "category": "some-category", - "level": "notice", - "severity": "warning" - } - ], - "summary": { - "files_scanned": 3, - "files_failed": 2, - "rules_skipped": 1, - "num_violations": 2 - } -} -` - if buf.String() != expect { + if expect := MustReadFile(t, "testdata/json/reporter.json"); expect != buf.String() { t.Errorf("expected %q, got %q", expect, buf.String()) } } @@ -328,45 +202,26 @@ func TestJSONReporterPublishNoViolations(t *testing.T) { t.Parallel() var buf bytes.Buffer - - jr := NewJSONReporter(&buf) - - err := jr.Publish(context.Background(), report.Report{}) - if err != nil { + if err := NewJSONReporter(&buf).Publish(context.Background(), report.Report{}); err != nil { t.Fatal(err) } - if buf.String() != `{ - "violations": [], - "summary": { - "files_scanned": 0, - "files_failed": 0, - "rules_skipped": 0, - "num_violations": 0 - } -} -` { - t.Errorf("expected %q, got %q", `{"violations":[]}`, buf.String()) + if expect := MustReadFile(t, "testdata/json/reporter-no-violations.json"); expect != buf.String() { + t.Errorf("expected %q, got %q", expect, buf.String()) } } -//nolint:paralleltest +// nolint:paralleltest func TestGitHubReporterPublish(t *testing.T) { // Can't use t.Parallel() here because t.Setenv() forbids that t.Setenv("GITHUB_STEP_SUMMARY", "") var buf bytes.Buffer - - cr := NewGitHubReporter(&buf) - - err := cr.Publish(context.Background(), rep) - if err != nil { + if err := NewGitHubReporter(&buf).Publish(context.Background(), rep); err != nil { t.Fatal(err) } - expectTable := "Rule: \tbreaking-the-law" - - if !strings.Contains(buf.String(), expectTable) { + if expectTable := "Rule: \tbreaking-the-law"; !strings.Contains(buf.String(), expectTable) { t.Errorf("expected table output %q, got %q", expectTable, buf.String()) } @@ -374,23 +229,18 @@ func TestGitHubReporterPublish(t *testing.T) { expectGithub := `::error file=a.rego,line=1,col=1::Rego must not break the law!. To learn more, see: https://example.com/illegal ::warning file=b.rego,line=22,col=18::Questionable decision found. To learn more, see: https://example.com/questionable ` - if !strings.Contains(buf.String(), expectGithub) { t.Errorf("expected workflow command output %q, got %q", expectGithub, buf.String()) } } -//nolint:paralleltest +// nolint:paralleltest func TestGitHubReporterPublishNoViolations(t *testing.T) { // Can't use t.Parallel() here because t.Setenv() forbids that t.Setenv("GITHUB_STEP_SUMMARY", "") var buf bytes.Buffer - - cr := NewGitHubReporter(&buf) - - err := cr.Publish(context.Background(), report.Report{}) - if err != nil { + if err := NewGitHubReporter(&buf).Publish(context.Background(), report.Report{}); err != nil { t.Fatal(err) } @@ -403,130 +253,11 @@ func TestSarifReporterPublish(t *testing.T) { t.Parallel() var buf bytes.Buffer - - sr := NewSarifReporter(&buf) - - err := sr.Publish(context.Background(), rep) - if err != nil { + if err := NewSarifReporter(&buf).Publish(context.Background(), rep); err != nil { t.Fatal(err) } - expect := `{ - "version": "2.1.0", - "$schema": "https://raw.githubusercontent.com/oasis-tcs/sarif-spec/main/sarif-2.1/schema/sarif-schema-2.1.0.json", - "runs": [ - { - "tool": { - "driver": { - "informationUri": "https://docs.styra.com/regal", - "name": "Regal", - "rules": [ - { - "id": "breaking-the-law", - "shortDescription": { - "text": "Rego must not break the law!" - }, - "helpUri": "https://example.com/illegal", - "properties": { - "category": "legal" - } - }, - { - "id": "questionable-decision", - "shortDescription": { - "text": "Questionable decision found" - }, - "helpUri": "https://example.com/questionable", - "properties": { - "category": "really?" - } - }, - { - "id": "rule-missing-capability", - "shortDescription": { - "text": "Rule missing capability bar" - }, - "properties": { - "category": "some-category" - } - } - ] - } - }, - "artifacts": [ - { - "location": { - "uri": "a.rego" - }, - "length": -1 - }, - { - "location": { - "uri": "b.rego" - }, - "length": -1 - } - ], - "results": [ - { - "ruleId": "breaking-the-law", - "ruleIndex": 0, - "level": "error", - "message": { - "text": "Rego must not break the law!" - }, - "locations": [ - { - "physicalLocation": { - "artifactLocation": { - "uri": "a.rego" - }, - "region": { - "startLine": 1, - "startColumn": 1, - "endLine": 1, - "endColumn": 14 - } - } - } - ] - }, - { - "ruleId": "questionable-decision", - "ruleIndex": 1, - "level": "warning", - "message": { - "text": "Questionable decision found" - }, - "locations": [ - { - "physicalLocation": { - "artifactLocation": { - "uri": "b.rego" - }, - "region": { - "startLine": 22, - "startColumn": 18 - } - } - } - ] - }, - { - "ruleId": "rule-missing-capability", - "ruleIndex": 2, - "kind": "informational", - "level": "none", - "message": { - "text": "Rule missing capability bar" - } - } - ] - } - ] -}` - - if buf.String() != expect { + if expect := MustReadFile(t, "testdata/sarif/reporter.json"); buf.String() != expect { t.Errorf("expected %s, got %s", expect, buf.String()) } } @@ -536,10 +267,7 @@ func TestSarifReporterViolationWithoutRegion(t *testing.T) { t.Parallel() var buf bytes.Buffer - - sr := NewSarifReporter(&buf) - - err := sr.Publish(context.Background(), report.Report{ + if err := NewSarifReporter(&buf).Publish(context.Background(), report.Report{ Violations: []report.Violation{ { Title: "opa-fmt", @@ -557,66 +285,11 @@ func TestSarifReporterViolationWithoutRegion(t *testing.T) { Level: "error", }, }, - }) - if err != nil { + }); err != nil { t.Fatal(err) } - expect := `{ - "version": "2.1.0", - "$schema": "https://raw.githubusercontent.com/oasis-tcs/sarif-spec/main/sarif-2.1/schema/sarif-schema-2.1.0.json", - "runs": [ - { - "tool": { - "driver": { - "informationUri": "https://docs.styra.com/regal", - "name": "Regal", - "rules": [ - { - "id": "opa-fmt", - "shortDescription": { - "text": "File should be formatted with ` + "`opa fmt`" + `" - }, - "helpUri": "https://docs.styra.com/regal/rules/style/opa-fmt", - "properties": { - "category": "style" - } - } - ] - } - }, - "artifacts": [ - { - "location": { - "uri": "policy.rego" - }, - "length": -1 - } - ], - "results": [ - { - "ruleId": "opa-fmt", - "ruleIndex": 0, - "level": "error", - "message": { - "text": "File should be formatted with ` + "`opa fmt`" + `" - }, - "locations": [ - { - "physicalLocation": { - "artifactLocation": { - "uri": "policy.rego" - } - } - } - ] - } - ] - } - ] -}` - - if buf.String() != expect { + if expect := MustReadFile(t, "testdata/sarif/reporter-no-region.json"); buf.String() != expect { t.Errorf("expected %s, got %s", expect, buf.String()) } } @@ -625,75 +298,26 @@ func TestSarifReporterPublishNoViolations(t *testing.T) { t.Parallel() var buf bytes.Buffer - - sr := NewSarifReporter(&buf) - - err := sr.Publish(context.Background(), report.Report{}) - if err != nil { + if err := NewSarifReporter(&buf).Publish(context.Background(), report.Report{}); err != nil { t.Fatal(err) } - expect := `{ - "version": "2.1.0", - "$schema": "https://raw.githubusercontent.com/oasis-tcs/sarif-spec/main/sarif-2.1/schema/sarif-schema-2.1.0.json", - "runs": [ - { - "tool": { - "driver": { - "informationUri": "https://docs.styra.com/regal", - "name": "Regal", - "rules": [] - } - }, - "results": [] - } - ] -}` - - if buf.String() != expect { + if expect := MustReadFile(t, "testdata/sarif/reporter-no-violation.json"); buf.String() != expect { t.Errorf("expected %s, got %s", expect, buf.String()) } } -//nolint:lll // the expected output is unfortunately longer than the allowed max line length +// nolint:lll // the expected output is unfortunately longer than the allowed max line length func TestJUnitReporterPublish(t *testing.T) { t.Parallel() var buf bytes.Buffer - - sr := NewJUnitReporter(&buf) - - err := sr.Publish(context.Background(), rep) - if err != nil { + if err := NewJUnitReporter(&buf).Publish(context.Background(), rep); err != nil { t.Fatal(err) } - expect := ` - - - - - - - - - - - -` - - if buf.String() != expect { - t.Errorf("expected \n%s, got \n%s", expect, buf.String()) + if expect := MustReadFile(t, "testdata/junit/reporter.xml"); buf.String() != expect { + t.Errorf("expected %s, got %s", expect, buf.String()) } } @@ -701,18 +325,22 @@ func TestJUnitReporterPublishNoViolations(t *testing.T) { t.Parallel() var buf bytes.Buffer + if err := NewJUnitReporter(&buf).Publish(context.Background(), report.Report{}); err != nil { + t.Fatal(err) + } - sr := NewJUnitReporter(&buf) + if expect := "\n"; buf.String() != expect { + t.Errorf("expected \n%s, got \n%s", expect, buf.String()) + } +} - err := sr.Publish(context.Background(), report.Report{}) +func MustReadFile(t *testing.T, path string) string { + t.Helper() + + bs, err := os.ReadFile(path) if err != nil { t.Fatal(err) } - expect := ` -` - - if buf.String() != expect { - t.Errorf("expected \n%s, got \n%s", expect, buf.String()) - } + return string(bs) } diff --git a/pkg/reporter/testdata/json/reporter-no-violations.json b/pkg/reporter/testdata/json/reporter-no-violations.json new file mode 100644 index 00000000..4b915e0c --- /dev/null +++ b/pkg/reporter/testdata/json/reporter-no-violations.json @@ -0,0 +1,9 @@ +{ + "violations": [], + "summary": { + "files_scanned": 0, + "files_failed": 0, + "rules_skipped": 0, + "num_violations": 0 + } +} diff --git a/pkg/reporter/testdata/json/reporter.json b/pkg/reporter/testdata/json/reporter.json new file mode 100644 index 00000000..61924517 --- /dev/null +++ b/pkg/reporter/testdata/json/reporter.json @@ -0,0 +1,66 @@ +{ + "violations": [ + { + "title": "breaking-the-law", + "description": "Rego must not break the law!", + "category": "legal", + "level": "error", + "related_resources": [ + { + "description": "documentation", + "ref": "https://example.com/illegal" + } + ], + "location": { + "end": { + "row": 1, + "col": 14 + }, + "text": "package illegal", + "file": "a.rego", + "col": 1, + "row": 1 + } + }, + { + "title": "questionable-decision", + "description": "Questionable decision found", + "category": "really?", + "level": "warning", + "related_resources": [ + { + "description": "documentation", + "ref": "https://example.com/questionable" + } + ], + "location": { + "text": "default allow = true", + "file": "b.rego", + "col": 18, + "row": 22 + } + } + ], + "notices": [ + { + "title": "rule-made-obsolete", + "description": "Rule made obsolete by capability foo", + "category": "some-category", + "level": "notice", + "severity": "none" + }, + { + "title": "rule-missing-capability", + "description": "Rule missing capability bar", + "category": "some-category", + "level": "notice", + "severity": "warning" + } + ], + "summary": { + "files_scanned": 3, + "files_failed": 2, + "rules_skipped": 1, + "num_violations": 2 + } +} diff --git a/pkg/reporter/testdata/junit/reporter.xml b/pkg/reporter/testdata/junit/reporter.xml new file mode 100644 index 00000000..52c579ad --- /dev/null +++ b/pkg/reporter/testdata/junit/reporter.xml @@ -0,0 +1,22 @@ + + + + + + + + + + + + diff --git a/pkg/reporter/testdata/pretty/reporter-long-text.txt b/pkg/reporter/testdata/pretty/reporter-long-text.txt new file mode 100644 index 00000000..62072cbe --- /dev/null +++ b/pkg/reporter/testdata/pretty/reporter-long-text.txt @@ -0,0 +1,8 @@ +Rule: long-violation +Description: violation with a long description +Category: long +Location: b.rego:22:18 +Text: long,long,long,long,long,long,long,long,long,long,long,long,long,long,long,long,long,long,long,long,long,long,long,lo... +Documentation: https://example.com/to-long + +3 files linted. 1 violation found. diff --git a/pkg/reporter/testdata/pretty/reporter.txt b/pkg/reporter/testdata/pretty/reporter.txt new file mode 100644 index 00000000..5036dce0 --- /dev/null +++ b/pkg/reporter/testdata/pretty/reporter.txt @@ -0,0 +1,17 @@ +Rule: breaking-the-law +Description: Rego must not break the law! +Category: legal +Location: a.rego:1:1 +Text: package illegal +Documentation: https://example.com/illegal + +Rule: questionable-decision +Description: Questionable decision found +Category: really? +Location: b.rego:22:18 +Text: default allow = true +Documentation: https://example.com/questionable + +3 files linted. 2 violations found in 2 files. 1 rule skipped: +- rule-missing-capability: Rule missing capability bar + diff --git a/pkg/reporter/testdata/sarif/reporter-no-region.json b/pkg/reporter/testdata/sarif/reporter-no-region.json new file mode 100644 index 00000000..22d2940e --- /dev/null +++ b/pkg/reporter/testdata/sarif/reporter-no-region.json @@ -0,0 +1,53 @@ +{ + "version": "2.1.0", + "$schema": "https://raw.githubusercontent.com/oasis-tcs/sarif-spec/main/sarif-2.1/schema/sarif-schema-2.1.0.json", + "runs": [ + { + "tool": { + "driver": { + "informationUri": "https://docs.styra.com/regal", + "name": "Regal", + "rules": [ + { + "id": "opa-fmt", + "shortDescription": { + "text": "File should be formatted with `opa fmt`" + }, + "helpUri": "https://docs.styra.com/regal/rules/style/opa-fmt", + "properties": { + "category": "style" + } + } + ] + } + }, + "artifacts": [ + { + "location": { + "uri": "policy.rego" + }, + "length": -1 + } + ], + "results": [ + { + "ruleId": "opa-fmt", + "ruleIndex": 0, + "level": "error", + "message": { + "text": "File should be formatted with `opa fmt`" + }, + "locations": [ + { + "physicalLocation": { + "artifactLocation": { + "uri": "policy.rego" + } + } + } + ] + } + ] + } + ] +} \ No newline at end of file diff --git a/pkg/reporter/testdata/sarif/reporter-no-violation.json b/pkg/reporter/testdata/sarif/reporter-no-violation.json new file mode 100644 index 00000000..2e6f2d1a --- /dev/null +++ b/pkg/reporter/testdata/sarif/reporter-no-violation.json @@ -0,0 +1,16 @@ +{ + "version": "2.1.0", + "$schema": "https://raw.githubusercontent.com/oasis-tcs/sarif-spec/main/sarif-2.1/schema/sarif-schema-2.1.0.json", + "runs": [ + { + "tool": { + "driver": { + "informationUri": "https://docs.styra.com/regal", + "name": "Regal", + "rules": [] + } + }, + "results": [] + } + ] +} \ No newline at end of file diff --git a/pkg/reporter/testdata/sarif/reporter.json b/pkg/reporter/testdata/sarif/reporter.json new file mode 100644 index 00000000..14228932 --- /dev/null +++ b/pkg/reporter/testdata/sarif/reporter.json @@ -0,0 +1,114 @@ +{ + "version": "2.1.0", + "$schema": "https://raw.githubusercontent.com/oasis-tcs/sarif-spec/main/sarif-2.1/schema/sarif-schema-2.1.0.json", + "runs": [ + { + "tool": { + "driver": { + "informationUri": "https://docs.styra.com/regal", + "name": "Regal", + "rules": [ + { + "id": "breaking-the-law", + "shortDescription": { + "text": "Rego must not break the law!" + }, + "helpUri": "https://example.com/illegal", + "properties": { + "category": "legal" + } + }, + { + "id": "questionable-decision", + "shortDescription": { + "text": "Questionable decision found" + }, + "helpUri": "https://example.com/questionable", + "properties": { + "category": "really?" + } + }, + { + "id": "rule-missing-capability", + "shortDescription": { + "text": "Rule missing capability bar" + }, + "properties": { + "category": "some-category" + } + } + ] + } + }, + "artifacts": [ + { + "location": { + "uri": "a.rego" + }, + "length": -1 + }, + { + "location": { + "uri": "b.rego" + }, + "length": -1 + } + ], + "results": [ + { + "ruleId": "breaking-the-law", + "ruleIndex": 0, + "level": "error", + "message": { + "text": "Rego must not break the law!" + }, + "locations": [ + { + "physicalLocation": { + "artifactLocation": { + "uri": "a.rego" + }, + "region": { + "startLine": 1, + "startColumn": 1, + "endLine": 1, + "endColumn": 14 + } + } + } + ] + }, + { + "ruleId": "questionable-decision", + "ruleIndex": 1, + "level": "warning", + "message": { + "text": "Questionable decision found" + }, + "locations": [ + { + "physicalLocation": { + "artifactLocation": { + "uri": "b.rego" + }, + "region": { + "startLine": 22, + "startColumn": 18 + } + } + } + ] + }, + { + "ruleId": "rule-missing-capability", + "ruleIndex": 2, + "kind": "informational", + "level": "none", + "message": { + "text": "Rule missing capability bar" + } + } + ] + } + ] +} \ No newline at end of file