diff --git a/CHANGELOG.md b/CHANGELOG.md index 3eb8b971c..042a086d5 100644 --- a/CHANGELOG.md +++ b/CHANGELOG.md @@ -304,6 +304,7 @@ and adheres to [Semantic Versioning](https://semver.org/spec/v2.0.0.html). - In React, a tag a component only mentions in a comment or a string no longer links it to that component: a JSDoc example, markup commented out like `{/* */}`, or an error message like react-router's "You cannot render a inside another " or shadcn's "useCarousel must be used within a ". Nothing in a TypeScript `.ts` file, which can't hold JSX, is read as a tag either, like the XML in outline's OpenSearch template. Before, these counted as renders, so react-router's hooks showed up among the callers of `Router`, `Routes` and `Route`, and `codegraph_explore`, callers and impact followed renders that never happen. A tag after an apostrophe, a URL or a backtick in a component's text, like `

Don't have an account? `, still links as before. Re-index React projects after upgrading. - In JavaScript and TypeScript, a name imported from a package your `package.json` lists no longer links to a project symbol that only shares its name when `tsconfig.json` or `jsconfig.json` has a catch-all path alias, like `"*": ["./typings/*"]` or `"*": ["src/*", "node_modules/*"]`, or an alias that points the package at a file in `node_modules`, like `"lit/decorators": ["./node_modules/lit/decorators.js"]`. Such an alias made every package look like part of the project, so `import { Typography } from '@mui/material'` was linked to the project's own `Typography` and every lit `@property()` decorator to an unrelated class's `property` field, and `codegraph callers`, impact and `codegraph affected` listed code that never used them. An import the alias does map to a file of your project, like a `.d.ts` you keep for an untyped package or `components/Button` through `"*": ["src/*"]`, links as before. Re-index affected projects after upgrading. - In JavaScript and TypeScript, a `tsconfig.json` or `jsconfig.json` path alias with text on both sides of its `*`, like `"@x/*/index": ["widgets/*/index"]`, no longer matches an import that leaves nothing for the `*`, like `@x/index`. Before, the one `/` in `@x/index` counted for both sides, so the import and every call through it linked to `widgets/index`, a file it never named; now the alias is skipped there, as TypeScript skips it, and a less specific alias can still match. Re-index affected projects after upgrading. Thanks @Dashsoap. +- A folder whose name starts with two dots, like `..generated/`, now counts as part of your project everywhere: a `tsconfig.json` or `jsconfig.json` path alias or `baseUrl` that points into it links its imports and the calls through them, the file watcher picks up edits made there, a gitignored one can be brought back with `include` in `codegraph.json`, a `.gitignore` rule for one applies when the project isn't a git repository, and `deprioritize` can name one. Before, each of these took such a folder for one outside the project. Re-index affected projects after upgrading. Thanks @amplifierplus. - In JavaScript and TypeScript, a function the file imports from a package and passes along as a value, like react-router's `Link` in `styled(Link)`, lodash's `noop` in `createContext({ onClose: noop })` or chart.js's `Legend` in `Chart.register(Legend)`, no longer links to a project function that only shares its name. Before, whenever the project had exactly one function of that name, every such value linked to it: outline's breadcrumbs, menus and document lists showed up among the callers of an email template's `Link`, and a test's own `noop` stub in cord-field among the code the app's contexts use, so `codegraph callers`, impact and `codegraph_explore` followed references that never happen. A function the file declares itself, including one that hides the import inside a function, and one imported from a file of your project link as before. Re-index JavaScript and TypeScript projects after upgrading. - In JavaScript and TypeScript, a function a file loads with a dynamic import from Node or a package, like `const { createServer } = await import('node:http')`, `const { default: chalk } = await import('chalk')` or `import('node:fs/promises').then(({ readFile }) => …)`, no longer links to a project function that only shares its name. Before, vite's call to Node's `createServer` showed up among the callers of vite's own `createServer`. The import counts only where it is in scope, so the rest of the file links as before, and a dynamic import of your own project's module still leads to that module. Thanks @Ancienttwo for the report. (#2491) - In JavaScript and TypeScript, code outside your tests no longer links to a Jest or Vitest manual mock under `__mocks__/`. Before, when an import of a workspace package couldn't be followed, a call like `formatLabel(name)` linked by name to the mock beside it instead of the package's own function, so the real function lost its callers, and the same happened to rendered components and event handlers. Production code is now matched as if the mocks weren't there, so it reaches the real function, and adding or deleting a mock leaves it alone. Tests, other mocks, and a file that imports a mock by its path, like a `jest.setup.ts`, still reach the mock. Thanks @Y-Osawa2022 for the report and @danusha2345. (#2455) diff --git a/__tests__/dotdot-folder-names.test.ts b/__tests__/dotdot-folder-names.test.ts new file mode 100644 index 000000000..a5e2f9e4e --- /dev/null +++ b/__tests__/dotdot-folder-names.test.ts @@ -0,0 +1,153 @@ +/** + * A folder whose name starts with two dots, like `..generated/`, is inside the + * project. + * + * Several places asked whether a relative path leaves the root with + * `rel.startsWith('..')`, which is also true of `..generated/client.ts`. So a + * `paths` alias into such a folder linked nothing (`"@generated/*": + * ["..generated/*"]`), the file watcher dropped every edit there, a gitignored + * folder of that name could not be brought back with `include`, a `.gitignore` + * rule for one was never applied when walking a folder that isn't a git + * repository, and `deprioritize` never matched one. A path leaves the root + * only through a `..` segment, or as an absolute path (another drive on + * Windows). + */ +import { describe, it, expect, afterEach, vi } from 'vitest'; +import { execFileSync } from 'child_process'; +import * as fs from 'fs'; +import * as os from 'os'; +import * as path from 'path'; +import { CodeGraph } from '../src'; +import { escapesRoot } from '../src/utils'; +import { applyAliases, aliasAndBaseUrlCandidates, loadProjectAliases } from '../src/resolution/path-aliases'; +import { scanDirectory } from '../src/extraction'; +import { clearProjectConfigCache } from '../src/project-config'; +import { FileWatcher, __emitWatchEventForTests } from '../src/sync/watcher'; + +let dirs: string[] = []; +let graphs: CodeGraph[] = []; +let watchers: FileWatcher[] = []; +afterEach(() => { + for (const watcher of watchers) watcher.stop(); + for (const graph of graphs) graph.close(); + for (const dir of dirs) fs.rmSync(dir, { recursive: true, force: true }); + watchers = []; + graphs = []; + dirs = []; + clearProjectConfigCache(); +}); + +const project = (files: Record): string => { + const dir = fs.mkdtempSync(path.join(os.tmpdir(), 'cg-dotdot-')); + dirs.push(dir); + for (const [rel, content] of Object.entries(files)) { + fs.mkdirSync(path.dirname(path.join(dir, rel)), { recursive: true }); + fs.writeFileSync(path.join(dir, rel), content); + } + return dir; +}; + +const gitCommitAll = (dir: string): void => { + execFileSync('git', ['init', '-q'], { cwd: dir }); + execFileSync('git', ['add', '-A'], { cwd: dir }); + execFileSync('git', ['-c', 'user.email=a@b.c', '-c', 'user.name=t', 'commit', '-qm', 'x'], { cwd: dir }); +}; + +const CLIENT = 'export function makeClient(): number { return 1; }\n'; + +describe('escapesRoot', () => { + it('leaves the root only through a `..` segment or an absolute path', () => { + for (const rel of ['..', '../x', '..\\x', '../..']) expect(escapesRoot(rel), rel).toBe(true); + expect(escapesRoot(path.resolve(os.tmpdir(), 'elsewhere'))).toBe(true); + for (const rel of ['..generated', '..generated/client.ts', '..generated\\client.ts', 'src/..x/y.ts', '.hidden/z.ts', '']) { + expect(escapesRoot(rel), rel).toBe(false); + } + }); +}); + +describe('a path alias into a folder named like `..generated`', () => { + it('maps into the folder, and still drops a target outside the project', () => { + const root = project({ + 'tsconfig.json': JSON.stringify({ compilerOptions: { baseUrl: '.', paths: { '@generated/*': ['..generated/*', '../outside/*'] } } }), + }); + expect(applyAliases('@generated/client', loadProjectAliases(root)!, root)).toEqual(['..generated/client']); + }); + + it('reads a specifier from a baseUrl that is such a folder', () => { + const root = project({ 'jsconfig.json': JSON.stringify({ compilerOptions: { baseUrl: '..generated' } }) }); + expect(aliasAndBaseUrlCandidates('client', loadProjectAliases(root)!, root)).toEqual(['..generated/client']); + }); + + it('links the import and the calls through it', async () => { + const root = project({ + 'tsconfig.json': JSON.stringify({ compilerOptions: { baseUrl: '.', paths: { '@generated/*': ['..generated/*'] } } }), + '..generated/client.ts': CLIENT, + 'src/app.ts': "import { makeClient as mk } from '@generated/client';\nexport function run(): number { return mk(); }\n", + }); + const cg = await CodeGraph.init(root, { index: true }); + graphs.push(cg); + const run = cg.getNodesByName('run')[0]!; + expect(cg.getCallees(run.id).map((c) => `${c.node.name}@${c.node.filePath}`)).toEqual(['makeClient@..generated/client.ts']); + }); +}); + +describe('the file watcher and a folder named like `..generated`', () => { + it('records an edit there, and still drops a path outside the project', async () => { + const root = project({ '..generated/client.ts': CLIENT, 'src/app.ts': 'export const app = 1;\n' }); + const syncFn = vi.fn().mockResolvedValue({ filesChanged: 0, durationMs: 0 }); + const watcher = new FileWatcher(root, syncFn, { inertForTests: true, debounceMs: 100 }); + watchers.push(watcher); + watcher.start(); + await watcher.waitUntilReady(); + + fs.writeFileSync(path.join(root, '..generated/client.ts'), 'export function makeClient(): number { return 2; }\n'); + __emitWatchEventForTests(root, '..generated/client.ts'); + __emitWatchEventForTests(root, '../outside.ts'); + expect(watcher.getPendingFiles().map((p) => p.path)).toEqual(['..generated/client.ts']); + }); + + it('skips such a folder in the per-folder watch tree when .gitignore lists it', async () => { + const root = project({ '..cache/x.ts': 'export const x = 1;\n', '.gitignore': '..cache/\n' }); + const watcher = new FileWatcher(root, vi.fn().mockResolvedValue({ filesChanged: 0, durationMs: 0 }), { inertForTests: true }); + watchers.push(watcher); + watcher.start(); + await watcher.waitUntilReady(); + expect((watcher as unknown as { shouldIgnoreDir(p: string): boolean }).shouldIgnoreDir(path.join(root, '..cache'))).toBe(true); + }); +}); + +describe('scanning a folder named like `..generated`', () => { + const scan = (dir: string) => scanDirectory(dir).map((f) => f.replace(/\\/g, '/')).sort(); + + it('brings a gitignored one back when `include` names it', () => { + const root = project({ + 'app/main.ts': 'export const main = 1;\n', + '..generated/client.ts': CLIENT, + '.gitignore': '..generated/\n', + 'codegraph.json': JSON.stringify({ include: ['..generated/'] }), + }); + gitCommitAll(root); + expect(scan(root)).toEqual(['..generated/client.ts', 'app/main.ts']); + }); + + it('applies a .gitignore rule for one when the folder is no git repository', () => { + const root = project({ 'app/main.ts': 'export const main = 1;\n', '..cache/x.ts': 'export const x = 1;\n', '.gitignore': '..cache/\n' }); + expect(scan(root)).toEqual(['app/main.ts']); + }); +}); + +describe('deprioritize and a folder named like `..generated`', () => { + it('ranks what such a folder holds lower when codegraph.json names it', async () => { + const files = { + '..generated/client.ts': 'export function makeClient(): number { return 1; }\n', + 'src/client.ts': 'export function makeClient(): number { return 2; }\n', + }; + const score = async (config: object | null): Promise => { + const root = project(config ? { ...files, 'codegraph.json': JSON.stringify(config) } : files); + const cg = await CodeGraph.init(root, { index: true }); + graphs.push(cg); + return cg.searchNodes('makeClient', { limit: 10 }).find((r) => r.node.filePath === '..generated/client.ts')!.score; + }; + expect(await score({ deprioritize: ['..generated/'] })).toBeLessThan(await score(null)); + }); +}); diff --git a/src/extraction/index.ts b/src/extraction/index.ts index 78b8bf214..20349209d 100644 --- a/src/extraction/index.ts +++ b/src/extraction/index.ts @@ -28,7 +28,7 @@ import { detectLanguage, isSourceFile, isLanguageSupported, isFileLevelOnlyLangu import { loadExtensionOverrides, loadIncludeIgnoredPatterns, loadExcludePatterns, loadIncludePatterns, PROJECT_CONFIG_FILENAME } from '../project-config'; import { isCodeGraphDataDir } from '../directory'; import { logDebug, logWarn } from '../errors'; -import { validatePathWithinRoot, normalizePath, isMissingPathError } from '../utils'; +import { validatePathWithinRoot, normalizePath, isMissingPathError, escapesRoot } from '../utils'; import ignore, { Ignore } from 'ignore'; import { detectFrameworks, getFrameworkResolver } from '../resolution/frameworks'; import { declaredDependencies } from '../resolution/frameworks/package-deps'; @@ -761,7 +761,7 @@ function collectIncludedFiles( if (entry.name === '.git' || isCodeGraphDataDir(entry.name)) continue; const abs = path.join(absDir, entry.name); const rel = normalizePath(path.relative(rootDir, abs)); - if (!rel || rel.startsWith('..')) continue; + if (!rel || escapesRoot(rel)) continue; if (entry.isSymbolicLink()) { try { const st = fs.statSync(fs.realpathSync(abs)); @@ -1878,7 +1878,7 @@ function scanDirectoryWalk( const isIgnored = (fullPath: string, isDir: boolean, matchers: ScopedIgnore[]): boolean => { for (const { dir, ig } of matchers) { let rel = normalizePath(path.relative(dir, fullPath)); - if (!rel || rel.startsWith('..')) continue; // not under this matcher's dir + if (!rel || escapesRoot(rel)) continue; // not under this matcher's dir if (isDir) rel += '/'; // dir-only rules (e.g. `build/`) only match with the slash if (ig.ignores(rel)) return true; } diff --git a/src/index.ts b/src/index.ts index 7f770b3dc..71849b7f3 100644 --- a/src/index.ts +++ b/src/index.ts @@ -58,7 +58,7 @@ import { createImportReader, reopenNameMatchedImports, reopenReferencesThroughAd import { GraphTraverser, GraphQueryManager } from './graph'; import { findNamedCopybooks, type NamedCopybook } from './graph/cobol-copybooks'; import { ContextBuilder, createContextBuilder } from './context'; -import { Mutex, FileLock } from './utils'; +import { Mutex, FileLock, escapesRoot } from './utils'; import { FileWatcher, WatchOptions, PendingFile, LockUnavailableError } from './sync'; import { EXTRACTION_VERSION } from './extraction/extraction-version'; import { getCodeGraphDir } from './directory'; @@ -252,7 +252,7 @@ export class CodeGraph { const rel = path.isAbsolute(filePath) ? path.relative(this.projectRoot, filePath) : filePath; - if (!rel || rel.startsWith('..')) return false; + if (!rel || escapesRoot(rel)) return false; return matcher.ignores(rel.split(path.sep).join('/')); }; const deprioritized: DeprioritizedPathMatcher = (filePath: string): boolean => { diff --git a/src/resolution/path-aliases.ts b/src/resolution/path-aliases.ts index 96d2b1748..3a6b1298a 100644 --- a/src/resolution/path-aliases.ts +++ b/src/resolution/path-aliases.ts @@ -31,6 +31,7 @@ import * as fs from 'fs'; import * as path from 'path'; import { logDebug } from '../errors'; +import { escapesRoot } from '../utils'; /** A single alias pattern from `compilerOptions.paths`. */ export interface AliasPattern { @@ -372,8 +373,9 @@ export function applyAliases( const absolute = path.resolve(aliases.baseUrl, filled); const relative = path.relative(projectRoot, absolute); // Skip if the rewrite escapes the project root (unsafe + can't - // be looked up via the file index anyway). - if (relative.startsWith('..')) continue; + // be looked up via the file index anyway). A folder named like + // `..generated` is inside it. + if (escapesRoot(relative)) continue; out.push(relative.replace(/\\/g, '/')); } return out; @@ -395,7 +397,7 @@ export function aliasAndBaseUrlCandidates( const out = applyAliases(importPath, aliases, projectRoot); if (aliases.declaresBaseUrl) { const relative = path.relative(projectRoot, path.resolve(aliases.baseUrl, importPath)); - if (relative && !relative.startsWith('..') && !path.isAbsolute(relative)) { + if (relative && !escapesRoot(relative)) { out.push(relative.replace(/\\/g, '/')); } } diff --git a/src/sync/watcher.ts b/src/sync/watcher.ts index 612c8596d..bae5d18c7 100644 --- a/src/sync/watcher.ts +++ b/src/sync/watcher.ts @@ -38,7 +38,7 @@ import * as path from 'path'; import { isSourceFile, buildScopeIgnore, type ScopeIgnore } from '../extraction'; import { loadExtensionOverrides, PROJECT_CONFIG_FILENAME } from '../project-config'; import { logDebug, logWarn } from '../errors'; -import { normalizePath } from '../utils'; +import { escapesRoot, normalizePath } from '../utils'; import { isCodeGraphDataDir } from '../directory'; import { watchDisabledReason } from './watch-policy'; @@ -659,7 +659,7 @@ export class FileWatcher { * — it drops node_modules/dist/.git churn before any sync is scheduled. */ private handleChange(rel: string): void { - if (!rel || rel === '.' || rel.startsWith('..')) return; + if (!rel || rel === '.' || escapesRoot(rel)) return; // `.git/info/exclude` is otherwise always-ignored with the rest of `.git/`, // but it feeds `buildScopeIgnore` — allow it through as a scope refresh // when the platform delivers the event (recursive watchers may; Linux @@ -847,7 +847,7 @@ export class FileWatcher { */ private shouldIgnoreDir(dirPath: string): boolean { const rel = normalizePath(path.relative(this.projectRoot, dirPath)); - if (!rel || rel === '.' || rel.startsWith('..')) return false; // root / outside + if (!rel || rel === '.' || escapesRoot(rel)) return false; // root / outside if (this.isAlwaysIgnored(rel)) return true; if (!this.ignoreMatcher) return false; return this.ignoreMatcher.ignores(rel + '/'); diff --git a/src/utils.ts b/src/utils.ts index a99180949..1f3c13210 100644 --- a/src/utils.ts +++ b/src/utils.ts @@ -124,6 +124,18 @@ export function lexicalPathWithinRoot(projectRoot: string, filePath: string): st return isWithinDir(resolved, path.resolve(projectRoot)) ? resolved : null; } +/** + * Whether a path made relative to a root (`path.relative(root, p)`, or a + * project-relative path) leaves the root: `..` itself, a first segment of + * `..` (`../x`, or `..\x` from Windows), or an absolute path, which + * `path.relative` gives on Windows for another drive. A folder whose NAME + * starts with two dots, like `..generated/`, is inside the root: + * `startsWith('..')` took it for a way out. + */ +export function escapesRoot(rel: string): boolean { + return rel === '..' || rel.startsWith('../') || rel.startsWith('..\\') || path.isAbsolute(rel); +} + /** * Whether a filesystem error means nothing is at the path: ENOENT, or ENOTDIR, * which POSIX reports instead when a folder on the way is a file (Windows