-
Notifications
You must be signed in to change notification settings - Fork 54
Various small fixes #1736
New issue
Have a question about this project? Sign up for a free GitHub account to open an issue and contact its maintainers and the community.
By clicking “Sign up for GitHub”, you agree to our terms of service and privacy statement. We’ll occasionally send you account related emails.
Already on GitHub? Sign in to your account
Various small fixes #1736
Changes from all commits
File filter
Filter by extension
Conversations
Jump to
Diff view
Diff view
There are no files selected for viewing
| Original file line number | Diff line number | Diff line change | ||||
|---|---|---|---|---|---|---|
|
|
@@ -106,16 +106,19 @@ func TestFindInputPath(t *testing.T) { | |||||
| t.Fatalf("did not expect to find input.%s", tc.fileExt) | ||||||
| } | ||||||
|
|
||||||
| createWithContent(t, tmpDir+"/workspace/foo/bar/input."+tc.fileExt, tc.fileContent) | ||||||
| inputPath := filepath.Join(workspacePath, "foo", "bar", "input."+tc.fileExt) | ||||||
|
Contributor
There was a problem hiding this comment. Choose a reason for hiding this commentThe reason will be displayed to describe this comment to others. Learn more.
Suggested change
|
||||||
| createWithContent(t, inputPath, tc.fileContent) | ||||||
|
|
||||||
| if path, exp := rio.FindInputPath(file, workspacePath), workspacePath+"/foo/bar/input."+tc.fileExt; path != exp { | ||||||
| if path, exp := rio.FindInputPath(file, workspacePath), inputPath; path != exp { | ||||||
| t.Errorf(`expected input at %s, got %s`, exp, path) | ||||||
| } | ||||||
|
|
||||||
| testutil.MustRemove(t, tmpDir+"/workspace/foo/bar/input."+tc.fileExt) | ||||||
| createWithContent(t, tmpDir+"/workspace/input."+tc.fileExt, tc.fileContent) | ||||||
| testutil.MustRemove(t, inputPath) | ||||||
|
|
||||||
| if path, exp := rio.FindInputPath(file, workspacePath), workspacePath+"/input."+tc.fileExt; path != exp { | ||||||
| workspaceInputPath := filepath.Join(workspacePath, "input."+tc.fileExt) | ||||||
| createWithContent(t, workspaceInputPath, tc.fileContent) | ||||||
|
|
||||||
| if path, exp := rio.FindInputPath(file, workspacePath), workspaceInputPath; path != exp { | ||||||
| t.Errorf(`expected input at %s, got %s`, exp, path) | ||||||
| } | ||||||
| }) | ||||||
|
|
@@ -141,18 +144,30 @@ func TestFindInput(t *testing.T) { | |||||
| t.Fatalf("did not expect to find input.%s", tc.fileType) | ||||||
| } | ||||||
|
|
||||||
| createWithContent(t, tmpDir+"/workspace/foo/bar/input."+tc.fileType, tc.fileContent) | ||||||
| inputPath := filepath.Join(workspacePath, "foo", "bar", "input."+tc.fileType) | ||||||
|
|
||||||
| createWithContent(t, inputPath, tc.fileContent) | ||||||
|
|
||||||
| path, content := rio.FindInput(file, workspacePath) | ||||||
| if path != workspacePath+"/foo/bar/input."+tc.fileType || !maps.Equal(content, map[string]any{"x": true}) { | ||||||
| t.Errorf(`expected input {"x": true} at, got %s`, content) | ||||||
| if path != inputPath { | ||||||
| t.Errorf(`expected input at %s, got %s`, inputPath, path) | ||||||
| } | ||||||
|
|
||||||
| if !maps.Equal(content, map[string]any{"x": true}) { | ||||||
| t.Errorf(`expected input {"x": true}, got %s`, content) | ||||||
| } | ||||||
|
|
||||||
| testutil.MustRemove(t, tmpDir+"/workspace/foo/bar/input."+tc.fileType) | ||||||
| createWithContent(t, tmpDir+"/workspace/input."+tc.fileType, tc.fileContent) | ||||||
| testutil.MustRemove(t, inputPath) | ||||||
|
|
||||||
| workspaceInputPath := filepath.Join(workspacePath, "input."+tc.fileType) | ||||||
| createWithContent(t, workspaceInputPath, tc.fileContent) | ||||||
|
|
||||||
| path, content = rio.FindInput(file, workspacePath) | ||||||
| if path != workspacePath+"/input."+tc.fileType || !maps.Equal(content, map[string]any{"x": true}) { | ||||||
| if path != workspaceInputPath { | ||||||
| t.Errorf(`expected input at %s, got %s`, workspaceInputPath, path) | ||||||
| } | ||||||
|
|
||||||
| if !maps.Equal(content, map[string]any{"x": true}) { | ||||||
| t.Errorf(`expected input {"x": true} at, got %s`, content) | ||||||
| } | ||||||
| }) | ||||||
|
|
||||||
| Original file line number | Diff line number | Diff line change |
|---|---|---|
|
|
@@ -869,27 +869,22 @@ func (l Linter) lintWithAggregateRules( | |
| ctx, cancel := context.WithCancel(ctx) | ||
| defer cancel() | ||
|
|
||
| input := map[string]any{ | ||
| // This will be replaced by the routing policy to provide each | ||
| // aggregate rule only the aggregated data from the same rule | ||
| "aggregates_internal": aggregates, | ||
| // There is no file provided in input here, but we'll provide *something* for | ||
| // consistency, and to avoid silently failing with undefined should someone | ||
| // refer to input.regal in an aggregate_report rule | ||
| "ignore_directives": ignoreDirectives, | ||
| "regal": map[string]any{ | ||
| "operations": []string{"aggregate"}, | ||
| "file": map[string]any{ | ||
| "name": "__aggregate_report__", | ||
| "lines": []string{}, | ||
| }, | ||
| }, | ||
| } | ||
| regal := ast.ObjectTerm( | ||
| ast.Item(ast.InternedTerm("operations"), ast.ArrayTerm(ast.InternedTerm("aggregate"))), | ||
| ast.Item(ast.InternedTerm("file"), ast.ObjectTerm( | ||
| ast.Item(ast.InternedTerm("name"), ast.InternedTerm("__aggregate_report__")), | ||
| ast.Item(ast.InternedTerm("lines"), ast.InternedEmptyArray), | ||
| )), | ||
| ) | ||
|
Comment on lines
+872
to
+878
Contributor
There was a problem hiding this comment. Choose a reason for hiding this commentThe reason will be displayed to describe this comment to others. Learn more. 🤔 I wonder if the compiler will raise this out of the function body, since it's constant... alternatively, you could do that, if this is called more than once, perhaps in the LSP scenario.
Member
Author
There was a problem hiding this comment. Choose a reason for hiding this commentThe reason will be displayed to describe this comment to others. Learn more. Yes, definitely for the LSP scenario! Will update 👍 |
||
|
|
||
| inputValue, err := transform.ToOPAInputValue(input) | ||
| if err != nil { | ||
| return report.Report{}, fmt.Errorf("failed to transform input value: %w", err) | ||
| } | ||
| aggParsed, _ := transform.ToOPAInputValue(aggregates) | ||
| dirParsed, _ := transform.ToOPAInputValue(ignoreDirectives) | ||
|
|
||
| inputValue := ast.NewObject( | ||
| ast.Item(ast.InternedTerm("aggregates_internal"), ast.NewTerm(aggParsed)), | ||
| ast.Item(ast.InternedTerm("ignore_directives"), ast.NewTerm(dirParsed)), | ||
| ast.Item(ast.InternedTerm("regal"), regal), | ||
| ) | ||
|
|
||
| evalArgs := []rego.EvalOption{ | ||
| rego.EvalParsedInput(inputValue), | ||
|
|
||
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.