diff --git a/cmd/capabilities.go b/cmd/capabilities.go index e3f389d0..4518c012 100644 --- a/cmd/capabilities.go +++ b/cmd/capabilities.go @@ -2,7 +2,6 @@ package cmd import ( "encoding/json" - "fmt" "os" "github.com/spf13/cobra" @@ -18,13 +17,11 @@ func init() { Long: "Show capabilities for Regal", RunE: func(*cobra.Command, []string) error { bs, err := json.MarshalIndent(io.Capabilities(), "", " ") - if err != nil { - return fmt.Errorf("failed marshalling capabilities: %w", err) + if err == nil { + _, err = os.Stdout.Write(append(bs, '\n')) } - fmt.Fprintln(os.Stdout, string(bs)) - - return nil + return err }, } diff --git a/cmd/parse.go b/cmd/parse.go index f01077e9..1cec47c6 100644 --- a/cmd/parse.go +++ b/cmd/parse.go @@ -62,12 +62,5 @@ func parse(args []string) error { return err } - output, err := encoding.JSON().MarshalIndent(enhancedAST, "", " ") - if err != nil { - return err - } - - _, err = os.Stdout.Write(output) - - return err + return encoding.NewIndentEncoder(os.Stdout, "", " ").Encode(enhancedAST) } diff --git a/internal/capabilities/capabilities.go b/internal/capabilities/capabilities.go index 87cc4577..de5edd05 100644 --- a/internal/capabilities/capabilities.go +++ b/internal/capabilities/capabilities.go @@ -221,9 +221,7 @@ func semverSort(stringVersions []string) { } if len(invalid) > 0 { - slices.Sort(invalid) - slices.Reverse(invalid) - copy(stringVersions[len(versions):], invalid) + copy(stringVersions[len(versions):], util.Reversed(util.Sorted(invalid))) } } diff --git a/internal/io/files/filter/filter.go b/internal/io/files/filter/filter.go index cca14717..b5e40e09 100644 --- a/internal/io/files/filter/filter.go +++ b/internal/io/files/filter/filter.go @@ -23,7 +23,7 @@ func Directories(_ string, info os.DirEntry) bool { return info.IsDir() } -// Filenames filters files by their exact name, not counting the path. +// Filenames filters files by their exact name, not including the path. func Filenames(names ...string) Func { return func(_ string, info os.DirEntry) bool { return slices.ContainsFunc(names, func(name string) bool { @@ -34,9 +34,9 @@ func Filenames(names ...string) Func { // Suffixes filters any path that has a suffix matching any of the provided suffixes. func Suffixes(suffixes ...string) Func { - return func(_ string, info os.DirEntry) bool { + return func(path string, _ os.DirEntry) bool { return slices.ContainsFunc(suffixes, func(suffix string) bool { - return strings.HasSuffix(info.Name(), suffix) + return strings.HasSuffix(path, suffix) }) } } @@ -49,6 +49,10 @@ func Not(filter Func) Func { } } -func RegoTests(_ string, info os.DirEntry) bool { - return strings.HasSuffix(info.Name(), "_test.rego") && info.Name() != "todo_test.rego" +func RegoTests(path string, info os.DirEntry) bool { + return strings.HasSuffix(path, "_test.rego") && info.Name() != "todo_test.rego" +} + +func NotRego(path string, _ os.DirEntry) bool { + return !strings.HasSuffix(path, ".rego") } diff --git a/internal/lsp/documentsymbol/documentsymbol.go b/internal/lsp/documentsymbol/documentsymbol.go index 938d199c..77303e66 100644 --- a/internal/lsp/documentsymbol/documentsymbol.go +++ b/internal/lsp/documentsymbol/documentsymbol.go @@ -92,16 +92,17 @@ func All(contents string, module *ast.Module, builtins map[string]*ast.Builtin) } func locationToRange(location *ast.Location) types.Range { - lines := bytes.Split(location.Text, []byte("\n")) - numLines := len(lines) startLine := util.SafeIntToUint(location.Row - 1) + numLines := bytes.Count(location.Text, []byte{'\n'}) + 1 endLine := startLine - if numLines != 1 { + if numLines > 1 { endLine += util.SafeIntToUint(numLines - 1) } - return types.RangeBetween(startLine, location.Col-1, endLine, len(lines[numLines-1])) + endLineContent := util.LineContents(location.Text, endLine-startLine) + + return types.RangeBetween(startLine, location.Col-1, endLine, len(endLineContent)) } func toWorkspaceSymbol(docSym types.DocumentSymbol, docURL string) types.WorkspaceSymbol { diff --git a/internal/lsp/lint.go b/internal/lsp/lint.go index 3294d56a..8bffa26d 100644 --- a/internal/lsp/lint.go +++ b/internal/lsp/lint.go @@ -72,12 +72,10 @@ func updateParse(ctx context.Context, opts updateParseOpts) (bool, error) { return false, fmt.Errorf("failed to get file contents for uri %q", opts.FileURI) } - lines := strings.Split(content, "\n") - numLines := len(lines) - options := rparse.ParserOptions() options.RegoVersion = opts.RegoVersion + numLines := strings.Count(content, "\n") + 1 presentedFileName := uri.ToRelativePath(opts.FileURI, opts.WorkspaceRootURI) module, err := rparse.ModuleWithOpts(presentedFileName, content, options) @@ -125,6 +123,7 @@ func updateParse(ctx context.Context, opts updateParseOpts) (bool, error) { } } + lines := strings.Split(content, "\n") diags := make([]types.Diagnostic, 0, len(astErrors)) for _, astError := range astErrors { diff --git a/internal/lsp/server.go b/internal/lsp/server.go index a971d388..52519d9c 100644 --- a/internal/lsp/server.go +++ b/internal/lsp/server.go @@ -346,7 +346,7 @@ func (l *LanguageServer) StartDiagnosticsWorker(ctx context.Context) { l.sendFileDiagnostics(ctx, job.URI) l.lintWorkspaceJobs <- lintWorkspaceJob{ - Reason: fmt.Sprintf("file %s %s", job.URI, job.Reason), + Reason: "file " + job.URI + " " + job.Reason, // this run is expected to used the cached aggregate state // for other files. // The aggregate state for this file will still be updated. @@ -724,18 +724,13 @@ func (l *LanguageServer) StartCommandWorker(ctx context.Context) { //nolint:main output := filepath.Join(l.workspacePath(), "output.json") var f *os.File - - f, err = os.OpenFile(output, os.O_CREATE|os.O_TRUNC|os.O_WRONLY, 0o755) - if err == nil { + if f, err = os.OpenFile(output, os.O_CREATE|os.O_TRUNC|os.O_WRONLY, 0o755); err == nil { value := result.Value if result.IsUndefined { value = emptyStringAnyMap // undefined displays as an empty object } - var jsonVal []byte - if jsonVal, err = encoding.JSON().MarshalIndent(value, "", " "); err == nil { - _, err = f.Write(jsonVal) - } + err = encoding.NewIndentEncoder(f, "", " ").Encode(value) rio.CloseIgnore(f) } diff --git a/internal/roast/encoding/array.go b/internal/roast/encoding/array.go index 8a945a46..8062714c 100644 --- a/internal/roast/encoding/array.go +++ b/internal/roast/encoding/array.go @@ -19,17 +19,13 @@ func (*arrayCodec) Encode(ptr unsafe.Pointer, stream *jsoniter.Stream) { stream.WriteArrayStart() - i := 0 - - arr.Foreach(func(term *ast.Term) { + for i := range arr.Len() { if i > 0 { stream.WriteMore() } - stream.WriteVal(term) - - i++ - }) + stream.WriteVal(arr.Elem(i)) + } stream.WriteArrayEnd() } diff --git a/internal/roast/encoding/head_test.go b/internal/roast/encoding/head_test.go index 78ccf1e3..dda1b402 100644 --- a/internal/roast/encoding/head_test.go +++ b/internal/roast/encoding/head_test.go @@ -14,37 +14,12 @@ func TestRuleHeadEncoding(t *testing.T) { head := ast.Head{ Name: "omitted", Reference: ast.Ref{ - { - Value: ast.Var("foo"), - Location: &ast.Location{ - Row: 1, - Col: 1, - Text: []byte("foo"), - }, - }, - { - Value: ast.String("bar"), - Location: &ast.Location{ - Row: 1, - Col: 5, // following "foo." - Text: []byte("bar"), - }, - }, - }, - Value: &ast.Term{ - Value: ast.Boolean(true), - Location: &ast.Location{ - Row: 1, - Col: 12, // following "foo.bar := " - Text: []byte("true"), - }, - }, - Assign: true, - Location: &ast.Location{ - Row: 1, - Col: 1, - Text: []byte("foo.bar := true"), + {Value: ast.Var("foo"), Location: &ast.Location{Row: 1, Col: 1, Text: []byte("foo")}}, + {Value: ast.String("bar"), Location: &ast.Location{Row: 1, Col: 5, Text: []byte("bar")}}, }, + Value: &ast.Term{Value: ast.Boolean(true), Location: &ast.Location{Row: 1, Col: 12, Text: []byte("true")}}, + Assign: true, + Location: &ast.Location{Row: 1, Col: 1, Text: []byte("foo.bar := true")}, } bs, err := jsoniter.ConfigFastest.MarshalIndent(head, "", " ") diff --git a/internal/roast/encoding/location.go b/internal/roast/encoding/location.go index c591b126..a4a38a7e 100644 --- a/internal/roast/encoding/location.go +++ b/internal/roast/encoding/location.go @@ -1,65 +1,23 @@ package encoding import ( - "bytes" - "strconv" - "strings" - "sync" "unsafe" jsoniter "github.com/json-iterator/go" "github.com/open-policy-agent/opa/v1/ast" + + "github.com/open-policy-agent/regal/pkg/roast/rast" ) type locationCodec struct{} -var newLine = []byte("\n") - -var sbPool = sync.Pool{ - New: func() any { - return new(strings.Builder) - }, -} - func (*locationCodec) IsEmpty(_ unsafe.Pointer) bool { return false } func (*locationCodec) Encode(ptr unsafe.Pointer, stream *jsoniter.Stream) { - location := *((*ast.Location)(ptr)) - - var endRow, endCol int - if location.Text == nil { - endRow = location.Row - endCol = location.Col - } else { - lines := bytes.Split(location.Text, newLine) - - numLines := len(lines) - - endRow = location.Row + numLines - 1 - - if numLines == 1 { - endCol = location.Col + len(location.Text) - } else { - lastLine := lines[numLines-1] - endCol = len(lastLine) + 1 - } - } - - sb := sbPool.Get().(*strings.Builder) //nolint:forcetypeassert - - sb.WriteString(strconv.Itoa(location.Row)) - sb.WriteByte(':') - sb.WriteString(strconv.Itoa(location.Col)) - sb.WriteByte(':') - sb.WriteString(strconv.Itoa(endRow)) - sb.WriteByte(':') - sb.WriteString(strconv.Itoa(endCol)) - - stream.WriteString(sb.String()) + location := (*ast.Location)(ptr) - sb.Reset() - sbPool.Put(sb) + stream.SetBuffer(append(rast.AppendLocation(append(stream.Buffer(), '"'), location), '"')) } diff --git a/internal/roast/encoding/location_test.go b/internal/roast/encoding/location_test.go index 49fd03b0..c8fe93da 100644 --- a/internal/roast/encoding/location_test.go +++ b/internal/roast/encoding/location_test.go @@ -16,41 +16,28 @@ func TestLocation(t *testing.T) { name string location ast.Location expected string - }{ - { - name: "multiple lines", - location: ast.Location{ - Row: 5, - Col: 2, - Text: []byte("allow if {\n input.foo == true\n}"), - }, - expected: "5:2:7:2", - }, - { - name: "single line", - location: ast.Location{ - Row: 1, - Col: 1, - Text: []byte("package example"), - }, - expected: "1:1:1:16", - }, - } - - json := jsoniter.ConfigFastest + }{{ + name: "multiple lines", + location: ast.Location{Row: 5, Col: 2, Text: []byte("allow if {\n input.foo == true\n}")}, + expected: "5:2:7:2", + }, { + name: "single line", + location: ast.Location{Row: 1, Col: 1, Text: []byte("package example")}, + expected: "1:1:1:16", + }} for _, tc := range cases { t.Run(tc.name, func(t *testing.T) { t.Parallel() - stream := json.BorrowStream(nil) - defer json.ReturnStream(stream) - + stream := jsoniter.ConfigFastest.BorrowStream(nil) stream.WriteVal(tc.location) if string(stream.Buffer()) != fmt.Sprintf("\"%s\"", tc.expected) { t.Fatalf("expected %s but got %s", tc.expected, string(stream.Buffer())) } + + jsoniter.ConfigFastest.ReturnStream(stream) }) } } @@ -60,10 +47,7 @@ func TestLocationHeadValue(t *testing.T) { // e.g. the end column would be presented as before the start column. t.Parallel() - module := ast.MustParseModule("package foo.bar\n\nrule := true") - json := jsoniter.ConfigFastest - - out, err := json.MarshalIndent(module, "", " ") + out, err := jsoniter.ConfigFastest.MarshalIndent(ast.MustParseModule("package foo.bar\n\nrule := true"), "", " ") if err != nil { t.Fatalf("failed to marshal module: %v", err) } diff --git a/internal/roast/encoding/module_test.go b/internal/roast/encoding/module_test.go index f6a43fc8..d0f8a9d3 100644 --- a/internal/roast/encoding/module_test.go +++ b/internal/roast/encoding/module_test.go @@ -228,8 +228,7 @@ func TestSerializedModuleSize(t *testing.T) { } } -// BenchmarkSerializeModule-16 3361 354640 ns/op 216756 B/op 9773 allocs/op - +// 285329 ns/op 125555 B/op 3094 allocs/op func BenchmarkSerializeModule(b *testing.B) { policy := mustReadTestFile(b, "testdata/policy.rego") module := ast.MustParseModuleWithOpts(string(policy), ast.ParserOptions{ProcessAnnotation: true}) diff --git a/internal/roast/transforms/module/module.go b/internal/roast/transforms/module/module.go index a560ead2..11c55308 100644 --- a/internal/roast/transforms/module/module.go +++ b/internal/roast/transforms/module/module.go @@ -1,10 +1,7 @@ package module import ( - "bytes" "encoding/base64" - "strconv" - "strings" "github.com/open-policy-agent/opa/v1/ast" outil "github.com/open-policy-agent/opa/v1/util" @@ -13,8 +10,6 @@ import ( "github.com/open-policy-agent/regal/pkg/roast/rast" ) -var newLine = []byte("\n") - // ToValue converts an AST module to RoAST value representation. // This is much more efficient than using a JSON encode/decode round trip. func ToValue(mod *ast.Module) (ast.Value, error) { @@ -97,41 +92,7 @@ func pathArray(terms []*ast.Term) *ast.Term { } func locationItem(location *ast.Location) [2]*ast.Term { - var endRow, endCol int - if location.Text == nil { - endRow = location.Row - endCol = location.Col - } else { - numLines := bytes.Count(location.Text, newLine) + 1 - - endRow = location.Row + numLines - 1 - - if numLines < 2 { - endCol = location.Col + len(location.Text) - } else { - endCol = len(location.Text) - bytes.LastIndexByte(location.Text, '\n') - } - } - - var sb strings.Builder - - sb.Grow( - outil.NumDigitsInt(location.Row) + - outil.NumDigitsInt(location.Col) + - outil.NumDigitsInt(endRow) + - outil.NumDigitsInt(endCol) + - 3, // 3 colons - ) - - sb.WriteString(strconv.Itoa(location.Row)) - sb.WriteByte(':') - sb.WriteString(strconv.Itoa(location.Col)) - sb.WriteByte(':') - sb.WriteString(strconv.Itoa(endRow)) - sb.WriteByte(':') - sb.WriteString(strconv.Itoa(endCol)) - - return item("location", ast.InternedTerm(sb.String())) + return item("location", ast.InternedTerm(outil.ByteSliceToString(rast.AppendLocation(nil, location)))) } func termToObjectLoc(term *ast.Term, includeLocation bool) *ast.Term { diff --git a/internal/roast/transforms/module/module_test.go b/internal/roast/transforms/module/module_test.go index 2e750757..b8197d6b 100644 --- a/internal/roast/transforms/module/module_test.go +++ b/internal/roast/transforms/module/module_test.go @@ -63,8 +63,8 @@ setcomp := {x | some x in input} } } -// BenchmarkModuleToValue/ToValue-12 26172 45026 ns/op 65514 B/op 1804 allocs/op -// BenchmarkModuleToValue/RoundTrip-12 9379 120571 ns/op 168927 B/op 4148 allocs/op +// BenchmarkModuleToValue/ToValue-16 25171 46355 ns/op 65489 B/op 1802 allocs/op +// BenchmarkModuleToValue/RoundTrip-16 10000 119018 ns/op 166038 B/op 3924 allocs/op func BenchmarkModuleToValue(b *testing.B) { policy := `# METADATA # title: p p p diff --git a/internal/util/util.go b/internal/util/util.go index 5518a3fa..3db0b86b 100644 --- a/internal/util/util.go +++ b/internal/util/util.go @@ -1,9 +1,11 @@ package util import ( + "bytes" "cmp" "errors" "fmt" + "iter" "math" "net" "os" @@ -330,3 +332,29 @@ func Reversed[T any](s []T) []T { return s } + +// LineContents returns the contents on line lineNum (0-indexed) from document. +// This function assumes the lineNum is known to be contained within the document,. +func LineContents(document []byte, lineNum uint) []byte { + for i, line := range Lines(document) { + if i == lineNum { + return bytes.TrimSuffix(line, []byte{'\n'}) + } + } + + return nil +} + +// Lines works like [bytes.Lines] but yields both the line number (0-indexed) and the line contents. +func Lines(s []byte) iter.Seq2[uint, []byte] { + return func(yield func(uint, []byte) bool) { + var lineNum uint + for line := range bytes.Lines(s) { + if !yield(lineNum, line) { + return + } + + lineNum++ + } + } +} diff --git a/internal/util/util_test.go b/internal/util/util_test.go index 571f0d9c..5a5b881f 100644 --- a/internal/util/util_test.go +++ b/internal/util/util_test.go @@ -2,6 +2,7 @@ package util import ( "slices" + "strconv" "testing" "github.com/open-policy-agent/opa/v1/ast" @@ -156,3 +157,45 @@ func BenchmarkSorted(b *testing.B) { b.Fatalf("expected %v, got %v", sorted, got) } } + +func TestLineContents(t *testing.T) { + t.Parallel() + + tests := []struct { + name string + line uint + expected string + }{ + {name: "first line", line: 0, expected: "line1"}, + {name: "middle line", line: 1, expected: "line2"}, + {name: "last line", line: 3, expected: "line4"}, + {name: "out of bounds (too high)", line: 5, expected: ""}, + } + + src := []byte("line1\nline2\nline3\nline4") + + for _, test := range tests { + t.Run(test.name, func(t *testing.T) { + t.Parallel() + + if got := LineContents(src, test.line); string(got) != test.expected { + t.Fatalf("expected %q, got %q", test.expected, string(got)) + } + }) + } +} + +// 9090 ns/op 24576 B/op 1 allocs/op // return bytes.Split(document, []byte{'\n'})[lineNum] +// 4726 ns/op 0 B/op 0 allocs/op // current implementation +func BenchmarkLineContents(b *testing.B) { + src := []byte{} + for i := range uint64(1000) { + src = append(strconv.AppendUint(append(src, "This is line number "...), i, 10), '\n') + } + + b.Run("LineContents", func(b *testing.B) { + for b.Loop() { + _ = LineContents(src, 500) + } + }) +} diff --git a/pkg/config/config.go b/pkg/config/config.go index e6c69bdc..7ead209f 100644 --- a/pkg/config/config.go +++ b/pkg/config/config.go @@ -366,12 +366,12 @@ func AllRegoVersions(root string, conf *Config) (map[string]ast.RegoVersion, err } for _, dir := range manifestLocations { - f, err := os.ReadFile(filepath.Join(root, dir, ".manifest")) + b, err := os.ReadFile(filepath.Join(root, dir, ".manifest")) if err != nil { return nil, fmt.Errorf("failed to read manifest file: %w", err) } - manifest, err := encoding.JSONUnmarshalTo[bundle.Manifest](f) + manifest, err := encoding.JSONUnmarshalTo[bundle.Manifest](b) if err != nil { return nil, fmt.Errorf("failed to unmarshal manifest file: %w", err) } diff --git a/pkg/config/filter.go b/pkg/config/filter.go index c8f65020..e92062a5 100644 --- a/pkg/config/filter.go +++ b/pkg/config/filter.go @@ -7,8 +7,6 @@ import ( "github.com/gobwas/glob" - "github.com/open-policy-agent/opa/v1/bundle" - "github.com/open-policy-agent/regal/internal/io/files" "github.com/open-policy-agent/regal/internal/io/files/filter" "github.com/open-policy-agent/regal/internal/util" @@ -16,14 +14,14 @@ import ( func FilterIgnoredPaths(paths, ignore []string, checkExists bool, pathPrefix string) (filtered []string, err error) { // - special case for stdin, return as is - if len(paths) == 1 && paths[0] == "-" || !checkExists && len(ignore) == 0 { + if len(paths) == 0 || len(paths) == 1 && paths[0] == "-" || !checkExists && len(ignore) == 0 { return paths, nil } if checkExists { for _, path := range paths { filtered, err = files.DefaultWalkReducer(path, filtered). - WithFilters(filter.Not(filter.Suffixes(bundle.RegoExt))). + WithFilters(filter.NotRego). WithStatBeforeWalk(true). Reduce(files.PathAppendReducer) if err != nil { @@ -31,8 +29,7 @@ func FilterIgnoredPaths(paths, ignore []string, checkExists bool, pathPrefix str } } - // Use forward slash since all paths are normalized to forward slashes for glob matching - return filterPaths(filtered, ignore, util.EnsureSuffix(pathPrefix, "/")) + paths = filtered } // Use forward slash since all paths are normalized to forward slashes for glob matching @@ -40,21 +37,17 @@ func FilterIgnoredPaths(paths, ignore []string, checkExists bool, pathPrefix str } func filterPaths(policyPaths []string, ignore []string, pathPrefix string) ([]string, error) { + patterns, err := compilePatterns(ignore) + if err != nil { + return nil, fmt.Errorf("failed to compile ignore patterns: %w", err) + } + filtered := make([]string, 0, len(policyPaths)) outer: for _, f := range policyPaths { - for _, pattern := range ignore { - if pattern == "" { - continue - } - - excluded, err := excludeFile(pattern, f, pathPrefix) - if err != nil { - return nil, fmt.Errorf("failed to check for exclusion using pattern %s: %w", pattern, err) - } - - if excluded { + for _, pattern := range patterns { + if excludeFile(pattern, f, pathPrefix) { continue outer } } @@ -67,59 +60,61 @@ outer: // excludeFile imitates the pattern matching of .gitignore files // See `exclusion.rego` for details on the implementation. -func excludeFile(pattern, filename, pathPrefix string) (bool, error) { - n := len(pattern) - +func excludeFile(pattern glob.Glob, filename, pathPrefix string) bool { // Normalize path separators to forward slashes for consistent glob matching filename = filepath.ToSlash(filename) - if pathPrefix != "" { - pathPrefix = filepath.ToSlash(pathPrefix) - filename = strings.TrimPrefix(filename, pathPrefix) - // Remove leading slash if present after trimming prefix - filename = strings.TrimPrefix(filename, "/") + filename = strings.TrimPrefix(strings.TrimPrefix(filename, filepath.ToSlash(pathPrefix)), "/") } - // Internal slashes means path is relative to root, otherwise it can - // appear anywhere in the directory (--> **/) - if !strings.Contains(pattern[:n-1], "/") { - pattern = "**/" + pattern - } + return pattern.Match(filename) +} - // Leading slash? - pattern = strings.TrimPrefix(pattern, "/") +func compilePatterns(patterns []string) ([]glob.Glob, error) { + compiled := make([]glob.Glob, 0, len(patterns)) - // Leading double-star? - ps := []string{pattern} - if noPrefix, ok := strings.CutPrefix(pattern, "**/"); ok { - ps = append(ps, noPrefix) - } + for _, pattern := range patterns { + if pattern == "" { + continue + } - var ps1 []string - - // trailing slash? - for _, p := range ps { - switch { - case strings.HasSuffix(p, "/"): - ps1 = append(ps1, p+"**") - case !strings.HasSuffix(p, "**"): - ps1 = append(ps1, p, p+"/**") - default: - ps1 = append(ps1, p) + n := len(pattern) + // Internal slashes means path is relative to root, otherwise it can + // appear anywhere in the directory (--> **/) + if !strings.Contains(pattern[:n-1], "/") { + pattern = "**/" + pattern } - } - // Loop through patterns and return true on first match - for _, p := range ps1 { - g, err := glob.Compile(p, '/') - if err != nil { - return false, fmt.Errorf("failed to compile pattern %s: %w", p, err) + pattern = strings.TrimPrefix(pattern, "/") + + ps := []string{pattern} + if noPrefix, ok := strings.CutPrefix(pattern, "**/"); ok { + ps = append(ps, noPrefix) } - if g.Match(filename) { - return true, nil + var ps1 []string + + for _, p := range ps { + switch { + case strings.HasSuffix(p, "/"): + ps1 = append(ps1, p+"**") + case !strings.HasSuffix(p, "**") && !strings.HasSuffix(p, ".rego"): + ps1 = append(ps1, p, p+"/**") + default: + ps1 = append(ps1, p) + } + } + + // Loop through patterns and return true on first match + for _, p := range ps1 { + g, err := glob.Compile(p, '/') + if err != nil { + return nil, fmt.Errorf("failed to compile pattern %s: %w", p, err) + } + + compiled = append(compiled, g) } } - return false, nil + return compiled, nil } diff --git a/pkg/config/filter_test.go b/pkg/config/filter_test.go index 2dcf4c13..7686bb42 100644 --- a/pkg/config/filter_test.go +++ b/pkg/config/filter_test.go @@ -5,6 +5,8 @@ import ( "testing" "github.com/google/go-cmp/cmp" + + "github.com/open-policy-agent/regal/internal/testutil" ) func TestFilterIgnoredPaths(t *testing.T) { @@ -105,3 +107,40 @@ func TestFilterIgnoredPaths(t *testing.T) { }) } } + +// 8680 ns/op 13816 B/op 373 allocs/op // original +// 1241 ns/op 2040 B/op 54 allocs/op // optimized +func BenchmarkFilterIgnoredPaths(b *testing.B) { + paths := []string{ + "foo/bar/baz/bax.rego", + "foo/baz/bar/bax.rego", + "bar/foo/regopkg/config/filter.go", + "bar/foo/regopkg/config/filter_test.go", + "bar/foo/main.rego", + } + ignore := []string{"foo/bar/**", "bar/*.rego"} + + for b.Loop() { + if _, err := FilterIgnoredPaths(paths, ignore, false, ""); err != nil { + b.Fatal(err) + } + } +} + +// 472 2292632 ns/op 1251673 B/op 28348 allocs/op +// 704 1688877 ns/op 312571 B/op 2969 allocs/op +func BenchmarkFilterIgnoredPathsBundleDir(b *testing.B) { + ignore := []string{"foo/bar/**", "bar/*.rego"} + + for b.Loop() { + testutil.Must(FilterIgnoredPaths([]string{"../../bundle"}, ignore, true, ""))(b) + } +} + +func BenchmarkFilterIgnoredPathsWorkspace(b *testing.B) { + ignore := []string{"foo/bar/**", "bar/*.rego"} + + for b.Loop() { + testutil.Must(FilterIgnoredPaths([]string{"../.."}, ignore, true, ""))(b) + } +} diff --git a/pkg/linter/linter.go b/pkg/linter/linter.go index 8de68e23..51cab8da 100644 --- a/pkg/linter/linter.go +++ b/pkg/linter/linter.go @@ -374,12 +374,7 @@ func (l Linter) Lint(ctx context.Context) (report.Report, error) { if l.inputModules != nil { l.startTimer(regalmetrics.RegalFilterIgnoredModules) - filteredPaths, err := config.FilterIgnoredPaths( - l.inputModules.FileNames, - ignore, - false, - l.pathPrefix, - ) + filteredPaths, err := config.FilterIgnoredPaths(l.inputModules.FileNames, ignore, false, l.pathPrefix) if err != nil { return report.Report{}, fmt.Errorf("failed to filter paths: %w", err) } diff --git a/pkg/report/report.go b/pkg/report/report.go index c57e655e..c96b9ee1 100644 --- a/pkg/report/report.go +++ b/pkg/report/report.go @@ -144,7 +144,7 @@ func (a Aggregate) IndexKey() string { return "" } - return fmt.Sprintf("%s/%s", cat, title) + return cat + "/" + title } func (r *Report) AddProfileEntries(prof map[string]ProfileEntry) { diff --git a/pkg/roast/encoding/json.go b/pkg/roast/encoding/json.go index 03402389..17e52233 100644 --- a/pkg/roast/encoding/json.go +++ b/pkg/roast/encoding/json.go @@ -1,6 +1,7 @@ package encoding import ( + "io" "log" jsoniter "github.com/json-iterator/go" @@ -72,3 +73,11 @@ func MustJSONRoundTripTo[T any](from any) T { return to } + +// NewIndentEncoder creates a new JSON encoder with the specified prefix and indent, encoding to w. +func NewIndentEncoder(w io.Writer, prefix, indent string) *jsoniter.Encoder { + enc := JSON().NewEncoder(w) + enc.SetIndent(prefix, indent) + + return enc +} diff --git a/pkg/roast/rast/rast.go b/pkg/roast/rast/rast.go index 20020dc9..342ad5a1 100644 --- a/pkg/roast/rast/rast.go +++ b/pkg/roast/rast/rast.go @@ -2,6 +2,7 @@ package rast import ( + "bytes" "fmt" "reflect" "slices" @@ -9,6 +10,7 @@ import ( "strings" "github.com/open-policy-agent/opa/v1/ast" + outil "github.com/open-policy-agent/opa/v1/util" "github.com/open-policy-agent/regal/internal/util" ) @@ -82,6 +84,36 @@ func ArrayTerm(a []string) *ast.Term { return ast.ArrayTerm(util.Map(a, ast.InternedTerm)...) } +func AppendLocation(buf []byte, location *ast.Location) []byte { + endRow, endCol := location.Row, location.Col + if textLen := len(location.Text); textLen > 0 { + numNewLines := util.SafeIntToUint(bytes.Count(location.Text, []byte("\n"))) + + endRow += util.SafeUintToInt(numNewLines) + if numNewLines == 0 { + endCol += textLen + } else { + endCol = textLen - bytes.LastIndexByte(location.Text, '\n') + } + } + + n := 3 + // 3 colons + outil.NumDigitsInt(location.Row) + outil.NumDigitsInt(location.Col) + + outil.NumDigitsInt(endRow) + outil.NumDigitsInt(endCol) + + if buf == nil { + buf = make([]byte, 0, n) + } else { + buf = slices.Grow(buf, n) + } + + buf = append(strconv.AppendInt(buf, int64(location.Row), 10), ':') + buf = append(strconv.AppendInt(buf, int64(location.Col), 10), ':') + buf = append(strconv.AppendInt(buf, int64(endRow), 10), ':') + + return strconv.AppendInt(buf, int64(endCol), 10) +} + func refHeadTerm(name string) *ast.Term { switch name { case "data": diff --git a/pkg/roast/rast/rast_test.go b/pkg/roast/rast/rast_test.go index 4cb54df8..24d12371 100644 --- a/pkg/roast/rast/rast_test.go +++ b/pkg/roast/rast/rast_test.go @@ -141,3 +141,37 @@ func TestRefStringToBody(t *testing.T) { } } } + +// BenchmarkAppendLocation/single_line_no_prealloc-16 34704147 34.05 ns/op 8 B/op 1 allocs/op +// BenchmarkAppendLocation/multi_line_no_prealloc-16 29631702 39.94 ns/op 16 B/op 1 allocs/op +// BenchmarkAppendLocation/single_line_with_prealloc-16 41071040 27.80 ns/op 0 B/op 0 allocs/op +// BenchmarkAppendLocation/multi_line_with_prealloc-16 30247112 40.32 ns/op 0 B/op 0 allocs/op +func BenchmarkAppendLocation(b *testing.B) { + cases := []struct { + name string + location *ast.Location + prealloc []byte + }{{ + name: "single line no prealloc", + location: &ast.Location{Row: 3, Col: 5, Text: []byte("example text")}, + }, { + name: "multi line no prealloc", + location: &ast.Location{Row: 2, Col: 10, Text: []byte("line one\nline two\nline three")}, + }, { + name: "single line with prealloc", + location: &ast.Location{Row: 1, Col: 1, Text: []byte("single line")}, + prealloc: make([]byte, 0, 10), + }, { + name: "multi line with prealloc", + location: &ast.Location{Row: 4, Col: 3, Text: []byte("first line\nsecond line\nthird line\nfourth line")}, + prealloc: make([]byte, 0, 20), + }} + + for _, tc := range cases { + b.Run(tc.name, func(b *testing.B) { + for b.Loop() { + _ = rast.AppendLocation(tc.prealloc, tc.location) + } + }) + } +} diff --git a/pkg/rules/rules.go b/pkg/rules/rules.go index 169922ac..34a810bd 100644 --- a/pkg/rules/rules.go +++ b/pkg/rules/rules.go @@ -5,7 +5,6 @@ import ( "io" "os" "path/filepath" - "slices" "strings" "sync" @@ -52,13 +51,6 @@ func InputFromPaths(paths []string, prefix string, versionsMap map[string]ast.Re return inputFromStdin() } - var versionedDirs []string - if len(versionsMap) > 0 { - // Sort directories by length, so that the most specific path is found first - versionedDirs = util.KeysSorted(versionsMap) - slices.Reverse(versionedDirs) - } - var wg sync.WaitGroup wg.Add(numPaths)