Bug hunt (#1020)

Fixes #1017 Which turned into a bug hunt.. Also fixed: - ast.resolved_imports crashing on duplicate identifiers - node_modules not excluded from fs walk, as @srenatus reported Opening the OPA repo now works as expected. The 500+ linter issues notwithstanding. Signed-off-by: Anders Eknert <[email protected]>
StyraInc · Aug 28, 2024 · a9d4e2f · a9d4e2f
1 parent 683f8de
commit a9d4e2f
Show file tree

Hide file tree

Showing 5 changed files with 18 additions and 12 deletions.
diff --git a/bundle/regal/ast/imports.rego b/bundle/regal/ast/imports.rego
@@ -19,19 +19,28 @@ imported_identifiers contains _imported_identifier(imp) if {
 	some imp in imports
 
 	imp.path.value[0].value in {"input", "data"}
+	count(imp.path.value) > 1
 }
 
 # METADATA
 # description: |
 #   map of all imported paths in the input module, keyed by their identifier or "namespace"
 resolved_imports[identifier] := path if {
-	some _import in imports
+	some identifier in imported_identifiers
 
-	_import.path.value[0].value == "data"
-	count(_import.path.value) > 1
+	# this should really be just a 1:1 mapping, but until OPA 1.0 we cannot
+	# trust that there are no duplicate imports, or imports shadowing other
+	# imports, which may render a runtime error here if two paths are written
+	# to the same identifier key ... simplify this post 1.0
+	paths := [path |
+		some imp in imports
 
-	identifier := _imported_identifier(_import)
-	path := [part.value | some part in _import.path.value]
+		_imported_identifier(imp) == identifier
+
+		path := [part.value | some part in imp.path.value]
+	]
+
+	path := paths[0]
 }
 
 # METADATA

diff --git a/internal/lsp/bundles/cache.go b/internal/lsp/bundles/cache.go
@@ -59,7 +59,7 @@ func (c *Cache) Refresh() ([]string, error) {
 	var foundBundleRoots []string
 
 	err := filepath.Walk(c.workspacePath, func(path string, info os.FileInfo, _ error) error {
-		if info.IsDir() && (info.Name() == ".git" || info.Name() == ".idea") {
+		if info.IsDir() && (info.Name() == ".git" || info.Name() == ".idea" || info.Name() == "node_modules") {
 			return filepath.SkipDir
 		}
 

diff --git a/internal/lsp/lint.go b/internal/lsp/lint.go
@@ -2,12 +2,11 @@ package lsp
 
 import (
 	"context"
+	"encoding/json"
 	"errors"
 	"fmt"
 	"strings"
 
-	"github.com/anderseknert/roast/pkg/encoding"
-
 	"github.com/open-policy-agent/opa/ast"
 	"github.com/open-policy-agent/opa/storage"
 
@@ -84,8 +83,6 @@ func updateParse(ctx context.Context, cache *cache.Cache, store storage.Store, f
 			Location: parseError.Location,
 		})
 	} else {
-		json := encoding.JSON()
-
 		jsonErrors, err := json.Marshal(unwrappedError)
 		if err != nil {
 			return false, fmt.Errorf("failed to marshal parse errors: %w", err)

diff --git a/internal/lsp/server.go b/internal/lsp/server.go
@@ -1859,7 +1859,7 @@ func (l *LanguageServer) loadWorkspaceContents(ctx context.Context, newOnly bool
 
 		// These directories often have thousands of items we don't care about,
 		// so don't even traverse them.
-		if d.IsDir() && (d.Name() == ".git" || d.Name() == ".idea") {
+		if d.IsDir() && (d.Name() == ".git" || d.Name() == ".idea" || d.Name() == "node_modules") {
 			return filepath.SkipDir
 		}
 

diff --git a/pkg/config/filter.go b/pkg/config/filter.go
@@ -22,7 +22,7 @@ func FilterIgnoredPaths(paths, ignore []string, checkFileExists bool, rootDir st
 		filtered := make([]string, 0, len(paths))
 
 		if err := walkPaths(paths, func(path string, info os.DirEntry, err error) error {
-			if info.IsDir() && (info.Name() == ".git" || info.Name() == ".idea") {
+			if info.IsDir() && (info.Name() == ".git" || info.Name() == ".idea" || info.Name() == "node_modules") {
 				return filepath.SkipDir
 			}
 			if !info.IsDir() && strings.HasSuffix(path, bundle.RegoExt) {