Skip to content
Merged
Show file tree
Hide file tree
Changes from all commits
Commits
File filter

Filter by extension

Filter by extension

Conversations
Failed to load comments.
Loading
Jump to
Jump to file
Failed to load files.
Loading
Diff view
Diff view
94 changes: 94 additions & 0 deletions scanner/jsimports_test.go
Original file line number Diff line number Diff line change
@@ -0,0 +1,94 @@
package scanner

import (
"context"
"sort"
"testing"
)

func sortedImporters(t *testing.T, graph *FileGraph, file string) []string {
t.Helper()
got := append([]string(nil), graph.Importers[file]...)
sort.Strings(got)
return got
}

func equalStrings(left, right []string) bool {
if len(left) != len(right) {
return false
}
for i := range left {
if left[i] != right[i] {
return false
}
}
return true
}

// The reported case: a CommonJS project written in the single quotes Prettier
// emits by default. js-imports matched only double-quoted literals, so every
// require in such a project was invisible and --importers answered a confident
// zero for a file with many requirers.
func TestCommonJSSingleQuoteImportersResolve(t *testing.T) {
graph, err := BuildFileGraph(context.Background(), "../testdata/commonjs-single-quotes", Filters{})
if err != nil {
t.Fatalf("build commonjs fixture graph: %v", err)
}

for _, tc := range []struct {
file string
want []string
why string
}{
{"services/layoutInputService.js", []string{"routes/admin.js", "routes/members.js"}, "single-quoted require, both ../ and ./../ forms"},
{"routes/members.js", []string{"app.js"}, "double-quoted require still resolves"},
{"routes/admin.js", []string{"app.js"}, "single-quoted ESM import in a .js file (already matched on main via jsx.yml's kind: import_statement; pinned so the rule rewrite cannot regress it)"},
{"app.js", nil, "entry point is imported by nothing"},
} {
got := sortedImporters(t, graph, tc.file)
if !equalStrings(got, tc.want) {
t.Errorf("%s importers = %v, want exactly %v (%s)", tc.file, got, tc.want, tc.why)
}
}
}

// A require whose argument is a variable cannot name a file. It has to resolve
// to nothing: a fabricated edge is worse than a missing one.
func TestDynamicRequireResolvesToNothing(t *testing.T) {
graph, err := BuildFileGraph(context.Background(), "../testdata/commonjs-single-quotes", Filters{})
if err != nil {
t.Fatalf("build commonjs fixture graph: %v", err)
}
got := append([]string(nil), graph.Imports["app.js"]...)
sort.Strings(got)
want := []string{"routes/admin.js", "routes/members.js"}
if !equalStrings(got, want) {
t.Fatalf("app.js imports = %v, want exactly %v — require(which) must add no edge", got, want)
}
}

// extractImportPath is what makes the structural rule quote-agnostic, since
// the rule binds no $PATH metavariable. Its fail-closed behavior on a
// non-literal argument is what keeps a dynamic require from inventing a path.
func TestExtractImportPathQuoteStyles(t *testing.T) {
for _, tc := range []struct {
name string
text string
want string
}{
{"double-quoted require", `require("./routes/members")`, "./routes/members"},
{"single-quoted require", `require('./routes/members')`, "./routes/members"},
{"template-literal require", "require(`./routes/members`)", "./routes/members"},
{"double-quoted esm import", `import x from "./mod";`, "./mod"},
{"single-quoted esm import", `import x from './mod';`, "./mod"},
{"bare esm import", `import './side-effect';`, "./side-effect"},
{"dynamic require yields nothing", `require(someVariable)`, ""},
{"computed require yields nothing", `require(base + '/x')`, "/x"},
} {
t.Run(tc.name, func(t *testing.T) {
if got := extractImportPath(tc.text); got != tc.want {
t.Fatalf("extractImportPath(%q) = %q, want %q", tc.text, got, tc.want)
}
})
}
}
11 changes: 9 additions & 2 deletions scanner/rustcargo.go
Original file line number Diff line number Diff line change
Expand Up @@ -100,6 +100,10 @@ func parseCargoMetadata(data []byte) (cargoMetadata, error) {
}

func buildRustWorkspaceIndex(ctx context.Context, root string, analyses []FileAnalysis, files []FileInfo, loader cargoMetadataLoader) (*rustWorkspaceIndex, *ScanSourceOutcome, error) {
return buildRustWorkspaceIndexWithTimeout(ctx, root, analyses, files, loader, cargoMetadataTimeout)
}

func buildRustWorkspaceIndexWithTimeout(ctx context.Context, root string, analyses []FileAnalysis, files []FileInfo, loader cargoMetadataLoader, metadataTimeout time.Duration) (*rustWorkspaceIndex, *ScanSourceOutcome, error) {
index := buildRustFallbackWorkspaceIndex(root, analyses)
manifestPaths, err := discoverCargoManifests(ctx, root, files)
if err != nil {
Expand All @@ -115,7 +119,7 @@ func buildRustWorkspaceIndex(ctx context.Context, root string, analyses []FileAn
outcome := cargoMetadataOutcome(0, len(manifestPaths))
return index, &outcome, nil
}
ctx, cancel := context.WithTimeout(ctx, cargoMetadataTimeout)
metadataCtx, cancel := context.WithTimeout(ctx, metadataTimeout)
defer cancel()

packagesByRoot := make(map[string]rustPackage, len(index.packages))
Expand All @@ -129,11 +133,14 @@ func buildRustWorkspaceIndex(ctx context.Context, root string, analyses []FileAn
if err := ctx.Err(); err != nil {
return nil, nil, err
}
if metadataCtx.Err() != nil {
break
}
manifestPath = filepath.Clean(manifestPath)
if handledManifests[manifestPath] {
continue
}
data, err := loader(ctx, manifestPath)
data, err := loader(metadataCtx, manifestPath)
if err != nil {
continue
}
Expand Down
12 changes: 9 additions & 3 deletions scanner/sg-rules/javascript.yml
Original file line number Diff line number Diff line change
Expand Up @@ -2,9 +2,15 @@ id: js-imports
language: javascript
rule:
any:
- pattern: import $$$_ from "$PATH"
- pattern: import "$PATH"
- pattern: require("$PATH")
# Structural, so quote style stops mattering: the literal-text patterns
# this replaces were hardcoded to double quotes, and single quotes are the
# Prettier default, so single-quoted imports and requires were invisible.
- kind: import_statement
# require() is a call, not an import_statement, so a kind rule alone would
# lose CommonJS entirely. $$$ binds no $PATH metavariable, which keeps
# path extraction on the quote-agnostic text path and lets a dynamic
# require(someVar) resolve to nothing rather than to a fabricated edge.
- pattern: require($$$)
---
id: js-functions
language: javascript
Expand Down
6 changes: 5 additions & 1 deletion scanner/sg-rules/jsx.yml
Original file line number Diff line number Diff line change
@@ -1,7 +1,11 @@
id: jsx-imports
language: jsx
rule:
kind: import_statement
any:
- kind: import_statement
# CommonJS require() is a call expression, so kind: import_statement alone
# never saw it, in either quote style.
- pattern: require($$$)
---
id: jsx-functions
language: jsx
Expand Down
6 changes: 5 additions & 1 deletion scanner/sg-rules/tsx.yml
Original file line number Diff line number Diff line change
@@ -1,7 +1,11 @@
id: tsx-imports
language: tsx
rule:
kind: import_statement
any:
- kind: import_statement
# CommonJS require() is a call expression, so kind: import_statement alone
# never saw it, in either quote style.
- pattern: require($$$)
---
id: tsx-functions
language: tsx
Expand Down
6 changes: 5 additions & 1 deletion scanner/sg-rules/typescript.yml
Original file line number Diff line number Diff line change
@@ -1,7 +1,11 @@
id: ts-imports
language: typescript
rule:
kind: import_statement
any:
- kind: import_statement
# CommonJS require() is a call expression, so kind: import_statement alone
# never saw it, in either quote style.
- pattern: require($$$)
---
id: ts-functions
language: typescript
Expand Down
10 changes: 10 additions & 0 deletions testdata/commonjs-single-quotes/app.js
Original file line number Diff line number Diff line change
@@ -0,0 +1,10 @@
// Double quotes still work, and an ESM-style single-quoted import is picked
// up too — both were broken for plain .js before.
const members = require("./routes/members");
import admin from './routes/admin';

// A dynamic require must resolve to nothing rather than a fabricated edge.
const which = process.env.ROUTE;
const dynamic = require(which);

module.exports = { members, admin, dynamic };
5 changes: 5 additions & 0 deletions testdata/commonjs-single-quotes/routes/admin.js
Original file line number Diff line number Diff line change
@@ -0,0 +1,5 @@
const layout = require('../services/layoutInputService');

module.exports = function admin() {
return layout.build();
};
5 changes: 5 additions & 0 deletions testdata/commonjs-single-quotes/routes/members.js
Original file line number Diff line number Diff line change
@@ -0,0 +1,5 @@
const layout = require('./../services/layoutInputService');

module.exports = function members() {
return layout.build();
};
Original file line number Diff line number Diff line change
@@ -0,0 +1,5 @@
// The shared module from the report: many files require it, all with the
// single quotes that Prettier writes by default.
module.exports.build = function build() {
return {};
};
Loading