Skip to content

Commit 18d8549

Browse files
committed
fix(eslint-rules): make base-hook runtime reach cycle-safe
Cache in-progress reach results to preserve transitive closure across import cycles, normalize diagnostic paths cross-platform, and add cyclic regression fixtures/tests.
1 parent d8d42b4 commit 18d8549

6 files changed

Lines changed: 103 additions & 47 deletions

File tree

Lines changed: 7 additions & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -0,0 +1,7 @@
1+
import { useB } from './b';
2+
import { runHeavy } from 'heavy-runtime';
3+
4+
export function useA(): number {
5+
runHeavy();
6+
return useB() + 1;
7+
}
Lines changed: 6 additions & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -0,0 +1,6 @@
1+
import { useA } from './a';
2+
3+
export function useB(): number {
4+
// Keep the static import cycle while avoiding direct execution recursion.
5+
return (useA as unknown as () => number).length;
6+
}
Lines changed: 3 additions & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -0,0 +1,3 @@
1+
// Cyclic graph where `a` imports a forbidden runtime and `b` reaches it only transitively.
2+
export { useA } from './a';
3+
export { useB } from './b';

tools/eslint-rules/rules/__fixtures__/base-hook-no-forbidden-runtime/tsconfig.json

Lines changed: 2 additions & 1 deletion
Original file line numberDiff line numberDiff line change
@@ -14,7 +14,8 @@
1414
"watched-pkg/*": ["./stubs/watched-pkg/*"],
1515
"heavy-runtime": ["./stubs/heavy-runtime/index.ts"],
1616
"light-helper": ["./stubs/light-helper/index.ts"],
17-
"cyclic-pkg": ["./stubs/cyclic-pkg/index.ts"]
17+
"cyclic-pkg": ["./stubs/cyclic-pkg/index.ts"],
18+
"cyclic-heavy-pkg": ["./stubs/cyclic-heavy-pkg/index.ts"]
1819
}
1920
},
2021
"include": ["src/**/*.ts", "src/**/*.tsx", "stubs/**/*.ts"]

tools/eslint-rules/rules/base-hook-no-forbidden-runtime.spec.ts

Lines changed: 30 additions & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -241,6 +241,36 @@ typedRuleTester.run(`${RULE_NAME} (typed)`, rule, {
241241
},
242242
],
243243
},
244+
// Regression: when the defining symbol lives on one node of a cycle, forbidden runtime
245+
// imported by another node in that cycle must still appear in transitive reach.
246+
{
247+
languageOptions: typedLanguageOptions,
248+
filename: TYPED_FILENAME,
249+
options: [
250+
{
251+
watchedPackages: ['cyclic-heavy-pkg'],
252+
forbiddenRuntimes: ['heavy-runtime'],
253+
},
254+
],
255+
code: `
256+
import { useB } from 'cyclic-heavy-pkg';
257+
export const useThingBase_unstable = (props: { a: number }, ref) => {
258+
return { props, ref, value: useB() };
259+
};
260+
`,
261+
errors: [
262+
{
263+
messageId: 'forbiddenRuntimeReach',
264+
data: {
265+
hookName: 'useThingBase_unstable',
266+
importedName: 'useB',
267+
package: 'cyclic-heavy-pkg',
268+
runtime: 'heavy-runtime',
269+
viaFile: 'rules/__fixtures__/base-hook-no-forbidden-runtime/stubs/cyclic-heavy-pkg/b.ts',
270+
},
271+
},
272+
],
273+
},
244274
// Aliased import from a forbidden-runtime package is still flagged on the alias use site.
245275
{
246276
languageOptions: typedLanguageOptions,

tools/eslint-rules/rules/base-hook-no-forbidden-runtime.ts

Lines changed: 55 additions & 46 deletions
Original file line numberDiff line numberDiff line change
@@ -1,5 +1,6 @@
11
import type { TSESTree, TSESLint, ParserServicesWithTypeInformation } from '@typescript-eslint/utils';
22
import { ESLintUtils, AST_NODE_TYPES } from '@typescript-eslint/utils';
3+
import * as path from 'node:path';
34
import * as ts from 'typescript';
45

56
// NOTE: The rule will be available in ESLint configs as "@nx/workspace-base-hook-no-forbidden-runtime"
@@ -100,8 +101,6 @@ interface Reach {
100101
all: ReadonlySet<string>;
101102
}
102103

103-
const EMPTY_REACH: Reach = { value: new Set(), all: new Set() };
104-
105104
/**
106105
* Per-Program cache: source file path → reach sets transitively computed from that file.
107106
* Both `value` and `all` sets are filled in a single DFS pass to share resolution work.
@@ -497,58 +496,60 @@ function transitiveReach(program: ts.Program, sourceFile: ts.SourceFile): Reach
497496
/**
498497
* Recursive worker for `transitiveReach`. Walks the import graph DFS, recording every bare
499498
* specifier encountered (separately for value-only vs value+type follow modes) and recursing into
500-
* each resolved source file. Uses `inProgress` to break cycles (cycle hits return empty sets
501-
* without caching, so the originating call still commits the complete result).
499+
* each resolved source file. Uses `inProgress` to break cycles by returning the already-cached,
500+
* in-progress reach object for the cycle participant.
502501
*/
503502
function computeReach(
504503
program: ts.Program,
505504
sourceFile: ts.SourceFile,
506505
cache: Map<string, Reach>,
507506
inProgress: Set<string>,
508507
): Reach {
509-
const cached = cache.get(sourceFile.fileName);
508+
const fileName = sourceFile.fileName;
509+
const cached = cache.get(fileName);
510510
if (cached) {
511511
return cached;
512512
}
513-
if (inProgress.has(sourceFile.fileName)) {
514-
// Cycle: return empty sets without caching so the eventual full result is committed by the originator.
515-
return EMPTY_REACH;
516-
}
517-
inProgress.add(sourceFile.fileName);
518-
519513
const value = new Set<string>();
520514
const all = new Set<string>();
521-
for (const imp of collectImports(sourceFile)) {
522-
if (isBareSpecifier(imp.specifier)) {
523-
const pkg = packageNameOf(imp.specifier);
524-
all.add(pkg);
525-
if (!imp.typeOnly) {
526-
value.add(pkg);
515+
const result: Reach = { value, all };
516+
cache.set(fileName, result);
517+
if (inProgress.has(fileName)) {
518+
return result;
519+
}
520+
inProgress.add(fileName);
521+
522+
try {
523+
for (const imp of collectImports(sourceFile)) {
524+
if (isBareSpecifier(imp.specifier)) {
525+
const pkg = packageNameOf(imp.specifier);
526+
all.add(pkg);
527+
if (!imp.typeOnly) {
528+
value.add(pkg);
529+
}
527530
}
528-
}
529-
const resolved = resolveModule(program, sourceFile, imp.specifier, imp.literal);
530-
if (!resolved) {
531-
continue;
532-
}
533-
const childSourceFile = program.getSourceFile(resolved);
534-
if (!childSourceFile) {
535-
continue;
536-
}
537-
const childReach = computeReach(program, childSourceFile, cache, inProgress);
538-
for (const pkg of childReach.all) {
539-
all.add(pkg);
540-
}
541-
if (!imp.typeOnly) {
542-
// A type-only edge does not propagate runtime reach: it can only widen the `all` set.
543-
for (const pkg of childReach.value) {
544-
value.add(pkg);
531+
const resolved = resolveModule(program, sourceFile, imp.specifier, imp.literal);
532+
if (!resolved) {
533+
continue;
534+
}
535+
const childSourceFile = program.getSourceFile(resolved);
536+
if (!childSourceFile) {
537+
continue;
538+
}
539+
const childReach = computeReach(program, childSourceFile, cache, inProgress);
540+
for (const pkg of childReach.all) {
541+
all.add(pkg);
542+
}
543+
if (!imp.typeOnly) {
544+
// A type-only edge does not propagate runtime reach: it can only widen the `all` set.
545+
for (const pkg of childReach.value) {
546+
value.add(pkg);
547+
}
545548
}
546549
}
550+
} finally {
551+
inProgress.delete(fileName);
547552
}
548-
549-
inProgress.delete(sourceFile.fileName);
550-
const result: Reach = { value, all };
551-
cache.set(sourceFile.fileName, result);
552553
return result;
553554
}
554555

@@ -691,14 +692,22 @@ function packageNameOf(specifier: string): string {
691692
* path workspace-relative when inside the current working directory.
692693
*/
693694
function shortenPath(absolute: string): string {
694-
const marker = '/node_modules/';
695-
const idx = absolute.lastIndexOf(marker);
696-
if (idx !== -1) {
697-
return absolute.slice(idx + marker.length);
695+
const resolvedAbsolute = path.resolve(absolute);
696+
const normalizedAbsolute = toPosixPath(resolvedAbsolute);
697+
const segments = normalizedAbsolute.split('/');
698+
const nodeModulesIdx = segments.lastIndexOf('node_modules');
699+
if (nodeModulesIdx !== -1 && nodeModulesIdx + 1 < segments.length) {
700+
return segments.slice(nodeModulesIdx + 1).join('/');
698701
}
699-
const cwd = process.cwd();
700-
if (absolute.startsWith(cwd)) {
701-
return absolute.slice(cwd.length + 1);
702+
703+
const relative = path.relative(path.resolve(process.cwd()), resolvedAbsolute);
704+
if (relative.length > 0 && !relative.startsWith('..') && !path.isAbsolute(relative)) {
705+
return toPosixPath(relative);
702706
}
703-
return absolute;
707+
708+
return normalizedAbsolute;
709+
}
710+
711+
function toPosixPath(value: string): string {
712+
return value.replace(/\\/g, '/');
704713
}

0 commit comments

Comments
 (0)