diff --git a/CHANGELOG.md b/CHANGELOG.md index 4222048f5..dc63884d4 100644 --- a/CHANGELOG.md +++ b/CHANGELOG.md @@ -310,6 +310,7 @@ and adheres to [Semantic Versioning](https://semver.org/spec/v2.0.0.html). - In JavaScript and TypeScript, an import written from a jsconfig or tsconfig `baseUrl`, like `import MainLayout from 'layout/MainLayout'` under `"baseUrl": "src"`, or through a catch-all `"paths": { "*": ["./src/*"] }`, now links to the file it names, read with the config of the app the importing file belongs to, and so does an import of a single name like `'config'` or `'themes'`. Create React App and Vite admin templates such as Berry, Mantis and MatX write nearly every import this way. Before, each one was taken for an npm package, so it linked nothing, its names were matched by name alone, which could reach a same-named component in another app of the same repository, and a React Router route whose page loads through `lazy(() => import('views/dashboard/Default'))` pointed at that loader instead of the page and its layout. A package the app's `package.json` declares, or a Node built-in, never links to a project module of the same name, only to type declarations the project keeps for it, and only a file the index holds counts, never a folder or something in `node_modules`. `codegraph sync` and the file watcher link such an import too when the file it names is added later or gains what it imports. Re-index JavaScript and TypeScript projects after upgrading. - `codegraph sync` and the file watcher now relink an import that reaches a module through a barrel file passing it on under another name, like `import { MatxLoading } from './components'` when `components/index.js` says `export { default as MatxLoading } from './MatxLoading'`, once an edit gives that module what is imported. Before, the import and every call through it stayed unlinked, or stayed on something else in the project with the same name, until the importing file itself changed or the project was indexed again. - A folder named `build` inside `src/` is now indexed for its TypeScript, JSX, Vue, Svelte, Astro, Go, C# and Rust files, so a feature or package kept there, like the `src/build/` pages of luci-go's milo, Go's own `src/go/build` package or the `src/build/` module of dioxus's CLI, shows up in `codegraph_explore`, and the routes that lazily load its pages link to them. Before, every folder named `build` was skipped as build output. The JavaScript, declaration and other files such a folder holds are still left out, since a build may have written them, and so is every `build` folder outside `src/`. +- A folder git ignores at the top of your project no longer takes every folder of the same name further down with it: a multi-module Maven project whose root `src/` holds only a gitignored jib agent indexed no Java at all, and a root `lib/` or `static/` that a local build writes hid a package's own `lib/` or an app's `static/` from the index, `codegraph sync` and the file watcher. A gitignored folder whose name reads as a pattern, like `[draft]` or `scratch[1`, now stands for just that folder instead of matching others or making the file watcher fail, and the `includeIgnored` entry `codegraph init` suggests for a skipped nested repository now covers only that repository. Re-index affected projects after upgrading. Thanks @Victor-Rodriguez-Garcia for the report and the fix. (#2849) - On Windows, a Swift call written after `try!`, like `try! Message2(data: data)` or `try! Settings.Defaults.load()`, now links to what it calls, as it already did on macOS and Linux. Before, the `!` was read as a logical not, so the call linked to nothing, and a call made through a type lost the type it was made on. Re-index Swift projects on Windows after upgrading. - In JavaScript and TypeScript, a barrel's `export { A, B as C } from './x'` now links each name it passes on to what `./x` exports, following barrels that pass it on in turn. Before, each name was looked up by itself and could link to a same-named symbol elsewhere in the project, such as another folder's `Provider`, and a name passed on from a package, like `export { debounce } from 'lodash'`, could link to a project function of that name. Re-index JavaScript and TypeScript projects after upgrading. - In Swift, a call written after `!` or another prefix operator, or chained after `try` or `await`, now links to what it calls. `if !isReady()` links to `isReady`, `!store.contains(id)` and `try! JSONDecoder().decode(…)` link to the method of the type they're called on rather than to whichever `contains` or `decode` came first, and `!Settings.Defaults.isOn()` lands on the type it names. Re-index Swift projects after upgrading. diff --git a/__tests__/git-ignored-dirs-scope.test.ts b/__tests__/git-ignored-dirs-scope.test.ts new file mode 100644 index 000000000..4a8eacb59 --- /dev/null +++ b/__tests__/git-ignored-dirs-scope.test.ts @@ -0,0 +1,234 @@ +/** + * The folders git reports as ignored are scoped as literal paths (#2849). + * + * The scope matcher the indexer, sync and watcher share also takes in the + * folders `git ls-files -o -i --exclude-standard --directory` reports (#1728). + * They used to be added as gitignore patterns, where a path of one segment has + * no anchor: an ignored root `src/` (holding only a gitignored jib agent) hid + * every `src/` in a Maven monorepo, and the index came up empty. A pattern + * also reads a folder's name as syntax: `[draft]` as a character class, `a[b` + * as a regex that throws on the first match, `#tmp` as a comment and `!keep` + * as a negation. The CLI's `includeIgnored` hint wrote repo paths into + * codegraph.json the same unanchored way. + */ +import { describe, it, expect, beforeEach, 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 ignore from 'ignore'; +import CodeGraph from '../src/index'; +import { + buildScopeIgnore, + findUnindexedIgnoredRepos, + includeIgnoredPatternFor, + scanDirectory, + scanDirectoryAsync, +} from '../src/extraction'; +import { FileWatcher } from '../src/sync/watcher'; +import { clearProjectConfigCache } from '../src/project-config'; + +const BIN = path.resolve(__dirname, '../dist/bin/codegraph.js'); + +function git(cwd: string, ...args: string[]): string { + return execFileSync('git', args, { cwd, encoding: 'utf8', stdio: ['ignore', 'pipe', 'pipe'] }); +} + +function write(file: string, content: string): void { + fs.mkdirSync(path.dirname(file), { recursive: true }); + fs.writeFileSync(file, content); +} + +/** A repo at `dir` with `tracked` committed, then `untracked` written for `.gitignore` to hide. */ +function makeRepo(dir: string, tracked: Record, untracked: Record = {}): void { + for (const [rel, content] of Object.entries(tracked)) write(path.join(dir, rel), content); + git(dir, 'init', '-q'); + git(dir, 'add', '-A'); + git(dir, '-c', 'user.email=t@t', '-c', 'user.name=t', 'commit', '-qm', 'init'); + for (const [rel, content] of Object.entries(untracked)) write(path.join(dir, rel), content); +} + +/** What git reports as ignored-untracked, relative to `cwd`. */ +function gitIgnored(cwd: string): string[] { + return git(cwd, 'ls-files', '-z', '-o', '-i', '--exclude-standard', '--directory').split('\0').filter(Boolean); +} + +/** `rel` and every folder above it, in the forms the watcher asks about. */ +function pathAndFolders(rel: string): string[] { + const parts = rel.split('/'); + return [rel, ...parts.slice(0, -1).map((_, i) => parts.slice(0, i + 1).join('/') + '/')]; +} + +const sorted = (files: Iterable): string[] => [...files].sort(); + +describe('folders git reports as ignored are scoped as literal paths (#2849)', () => { + let root: string; + + beforeEach(() => { + root = fs.mkdtempSync(path.join(os.tmpdir(), 'cg-git-ignored-dirs-')); + clearProjectConfigCache(); + }); + + afterEach(() => { + clearProjectConfigCache(); + fs.rmSync(root, { recursive: true, force: true }); + }); + + const JAVA = 'services/example/src/main/java/Example.java'; + const javaClass = (method: string) => `public class Example { public int ${method}() { return 1; } }\n`; + + it('indexes and syncs a monorepo whose root src/ holds only a gitignored jib agent', async () => { + makeRepo(root, { '.gitignore': '*jib*\n**/opt/datadog\n', [JAVA]: javaClass('run') }, { + 'src/main/jib/opt/datadog/dd-java-agent.jar': 'jar\n', + }); + expect(gitIgnored(root)).toContain('src/'); + + expect(scanDirectory(root)).toEqual([JAVA]); + expect(await scanDirectoryAsync(root)).toEqual([JAVA]); + const scope = buildScopeIgnore(root); + for (const p of pathAndFolders(JAVA)) expect(scope.ignores(p), p).toBe(false); + expect(scope.ignores('src/')).toBe(true); + expect(scope.ignores('src/main/jib/opt/datadog/dd-java-agent.jar')).toBe(true); + + const cg = CodeGraph.initSync(root); + try { + expect((await cg.indexAll()).filesIndexed).toBe(1); + // A scoped sync of an edit there updates the file rather than dropping it. + write(path.join(root, JAVA), javaClass('stop')); + await cg.sync({ paths: [JAVA] }); + expect(cg.getFiles().map((f) => f.path)).toEqual([JAVA]); + expect(cg.getNodesInFile(JAVA).map((n) => n.name)).toContain('stop'); + } finally { + cg.close(); + } + }); + + it.each([ + { name: 'lib', gitignore: '/lib/\n', nested: 'packages/util/lib/strings.ts', output: 'lib/index.js' }, + // A feature folder kept in src/build/ is indexed (#2681). + { name: 'build', gitignore: '/build/\n', nested: 'app/src/build/feature.ts', output: 'build/out.js' }, + // A nested dist/ the project re-includes on purpose. + { name: 'dist', gitignore: '/dist/\n!packages/legacy/dist/\n', nested: 'packages/legacy/dist/legacy.js', output: 'dist/bundle.js' }, + ])('keeps a tracked $name/ below the root when git reports the root $name/ ignored', ({ name, gitignore, nested, output }) => { + makeRepo(root, { '.gitignore': gitignore, 'index.ts': 'export const main = 1;\n', [nested]: 'export const kept = 1;\n' }, { + [output]: 'module.exports = 1;\n', + }); + expect(gitIgnored(root)).toEqual([`${name}/`]); + + expect(sorted(scanDirectory(root))).toEqual(sorted(['index.ts', nested])); + const scope = buildScopeIgnore(root); + for (const p of pathAndFolders(nested)) expect(scope.ignores(p), p).toBe(false); + expect(scope.ignores(`${name}/`)).toBe(true); + expect(scope.ignores(output)).toBe(true); + }); + + it('reads a reported folder named like a character class as that one folder', () => { + makeRepo(root, { '.gitignore': '*draft*\n', 'src/a/alpha.ts': 'export const alpha = 1;\n', 'src/b.ts': 'export const b = 1;\n' }, { + '[draft]/notes.ts': 'export const draft = 1;\n', + }); + expect(gitIgnored(root)).toEqual(['[draft]/']); + + // As a pattern, `[draft]/` matched any folder named d, r, a, f or t. + expect(sorted(scanDirectory(root))).toEqual(['src/a/alpha.ts', 'src/b.ts']); + const scope = buildScopeIgnore(root); + for (const p of pathAndFolders('src/a/alpha.ts')) expect(scope.ignores(p), p).toBe(false); + expect(scope.ignores('[draft]/notes.ts')).toBe(true); + }); + + it('keeps the watcher on files under a nested namesake and off the reported folders, whatever their names', async () => { + makeRepo(root, { '.gitignore': '*jib*\nscratch*/\n', [JAVA]: javaClass('run') }, { + 'src/main/jib/Agent.java': 'public class Agent {}\n', + // An unterminated character class: as a pattern it threw on every match. + 'scratch[1/s.ts': 'export const s = 1;\n', + }); + expect(gitIgnored(root)).toEqual(expect.arrayContaining(['src/', 'scratch[1/'])); + + const syncFn = vi.fn().mockResolvedValue({ filesChanged: 0, durationMs: 0 }); + const watcher = new FileWatcher(root, syncFn, { inertForTests: true, debounceMs: 60_000 }); + try { + expect(watcher.start()).toBe(true); + await watcher.waitUntilReady(); + watcher.ingestEventForTests(JAVA); + watcher.ingestEventForTests('src/main/jib/Agent.java'); + watcher.ingestEventForTests('scratch[1/s.ts'); + expect(watcher.getPendingFiles().map((p) => p.path)).toEqual([JAVA]); + } finally { + watcher.stop(); + } + const scope = buildScopeIgnore(root); + for (const p of pathAndFolders(JAVA)) expect(scope.ignores(p), p).toBe(false); + expect(scope.ignores('scratch[1/')).toBe(true); + }); + + it('anchors the folders reported for a project in a subfolder of the repo to that subfolder', () => { + const java = 'services/a/src/main/java/A.java'; + makeRepo(root, { + '.gitignore': '*jib*\n/proj/#tmp/\n/proj/!keep/\n', + 'src/outside.ts': 'export const outside = 1;\n', + [`proj/${java}`]: 'public class A {}\n', + 'proj/lib/keep.ts': 'export const keep = 1;\n', + }, { + 'proj/src/main/jib/agent.jar': 'jar\n', + 'proj/#tmp/t.ts': 'export const t = 1;\n', + 'proj/!keep/k.ts': 'export const k = 1;\n', + }); + const proj = path.join(root, 'proj'); + // git prints them relative to its cwd, the project: `src/`, not `proj/src/`. + expect(gitIgnored(proj)).toEqual(expect.arrayContaining(['src/', '#tmp/', '!keep/'])); + + expect(sorted(scanDirectory(proj))).toEqual(['lib/keep.ts', java]); + const scope = buildScopeIgnore(proj); + for (const p of pathAndFolders(java)) expect(scope.ignores(p), p).toBe(false); + expect(scope.ignores('src/main/jib/agent.jar')).toBe(true); + // Neither the repo root's rules nor these names reach the matcher any other + // way; as patterns, they were a comment and a negation. + expect(scope.ignores('#tmp/t.ts')).toBe(true); + expect(scope.ignores('!keep/k.ts')).toBe(true); + }); + + it('suggests an includeIgnored entry that opts in only the repo it found', () => { + makeRepo(path.join(root, 'mtc-a'), { 'src/a.ts': 'export const a = 1;\n' }); + makeRepo(root, { '.gitignore': '/mtc-a/\n/archive/\n' }); + + // An empty index at a super-repo prints the opt-in snippet (#1156). + const out = execFileSync(process.execPath, [BIN, 'init', '-y', root], { + cwd: root, + encoding: 'utf8', + stdio: ['ignore', 'pipe', 'pipe'], + timeout: 120_000, + env: { + ...process.env, + CODEGRAPH_NO_DAEMON: '1', + CODEGRAPH_WASM_RELAUNCHED: '1', + CODEGRAPH_TELEMETRY: '0', + DO_NOT_TRACK: '1', + NO_COLOR: '1', + }, + }); + const snippet = out.match(/\{ "includeIgnored": \[[^\]]*\] \}/)?.[0]; + expect(snippet, out).toBeDefined(); + const { includeIgnored } = JSON.parse(snippet!) as { includeIgnored: string[] }; + expect(includeIgnored).toEqual(['/mtc-a/']); + fs.writeFileSync(path.join(root, 'codegraph.json'), JSON.stringify({ includeIgnored })); + clearProjectConfigCache(); + + // Later an old copy of the same repo lands in another gitignored folder. + // The opt-in is for the repo the hint named, not for every `mtc-a/`. + makeRepo(path.join(root, 'archive', 'mtc-a'), { 'src/old.ts': 'export const old = 1;\n' }); + expect(scanDirectory(root)).toEqual(['mtc-a/src/a.ts']); + expect(findUnindexedIgnoredRepos(root)).toEqual(['archive/mtc-a/']); + }, 120_000); + + it('includeIgnoredPatternFor anchors the path and escapes glob characters', () => { + const optsIn = (repo: string, dir: string) => ignore().add(includeIgnoredPatternFor(repo)).ignores(dir); + expect(includeIgnoredPatternFor('repos/a/')).toBe('/repos/a/'); + expect(optsIn('mtc-a/', 'mtc-a/')).toBe(true); + expect(optsIn('mtc-a/', 'archive/mtc-a/')).toBe(false); + expect(optsIn('[draft]/', '[draft]/')).toBe(true); + expect(optsIn('[draft]/', 'd/')).toBe(false); + expect(optsIn('a*b/', 'a*b/')).toBe(true); + expect(optsIn('a*b/', 'axxb/')).toBe(false); + expect(optsIn('#tmp/', '#tmp/')).toBe(true); + expect(optsIn('!keep/', '!keep/')).toBe(true); + }); +}); diff --git a/src/bin/codegraph.ts b/src/bin/codegraph.ts index aeae56d66..95a291a56 100644 --- a/src/bin/codegraph.ts +++ b/src/bin/codegraph.ts @@ -550,9 +550,11 @@ async function offerIndexIgnoredRepos( opts: { interactive: boolean }, ): Promise { let repos: string[]; + let patterns: string[]; try { - const { findUnindexedIgnoredRepos } = await import('../extraction'); + const { findUnindexedIgnoredRepos, includeIgnoredPatternFor } = await import('../extraction'); repos = findUnindexedIgnoredRepos(projectPath); + patterns = repos.map(includeIgnoredPatternFor); } catch { return; // detection is advisory — never let it break the command } @@ -563,7 +565,7 @@ async function offerIndexIgnoredRepos( const SHOWN = 6; const names = repos.slice(0, SHOWN).map((r) => r.replace(/\/$/, '')); const extra = repos.length > SHOWN ? ` (+${formatNumber(repos.length - SHOWN)} more)` : ''; - const snippet = `{ "includeIgnored": [${repos.map((p) => JSON.stringify(p)).join(', ')}] }`; + const snippet = `{ "includeIgnored": [${patterns.map((p) => JSON.stringify(p)).join(', ')}] }`; clack.log.warn( `Your .gitignore excludes ${isOne ? 'a nested git repository' : `${formatNumber(repos.length)} nested git repositories`} here, ` + @@ -594,7 +596,7 @@ async function offerIndexIgnoredRepos( let added: number; try { const { addIncludeIgnoredPatterns } = await import('../project-config'); - added = addIncludeIgnoredPatterns(projectPath, repos); + added = addIncludeIgnoredPatterns(projectPath, patterns); } catch (err) { clack.log.error(`Could not update ${PROJECT_CONFIG_FILENAME}: ${err instanceof Error ? err.message : String(err)}`); manualHint(); diff --git a/src/extraction/index.ts b/src/extraction/index.ts index 001648bd8..f437cfd09 100644 --- a/src/extraction/index.ts +++ b/src/extraction/index.ts @@ -517,9 +517,12 @@ function readGitExcludeExtraPatterns(rootDir: string): string { /** * Directories `git ls-files -o -i --exclude-standard --directory` reports as - * ignored-untracked. Seeded into {@link ScopeIgnore} so nested `.gitignore` - * effects (and any exclude-standard rule the flat matcher might miss) prune the - * watcher the same way the indexer skips them (#1728). + * ignored-untracked, relative to `rootDir` (git prints paths relative to its + * cwd, also when `rootDir` is a subfolder of the repo) and trailing-slashed. + * Seeded into {@link ScopeIgnore} so nested `.gitignore` effects (and any + * exclude-standard rule the flat matcher might miss) prune the watcher the + * same way the indexer skips them (#1728). Each one is a path, not a pattern: + * see {@link ScopeIgnore}. */ function listGitIgnoredDirectories(rootDir: string): string[] { try { @@ -535,8 +538,11 @@ function listGitIgnoredDirectories(rootDir: string): string[] { }, ); const dirs: string[] = []; - for (const entry of out.split('\0')) { - if (!entry) continue; + for (const raw of out.split('\0')) { + // `./` is git's "this whole directory" for a root that is itself ignored + // (#936); the root's own files stay in scope. + if (isWholeCwdEntry(raw)) continue; + const entry = normalizePath(raw); dirs.push(entry.endsWith('/') ? entry : `${entry}/`); } return dirs; @@ -958,11 +964,29 @@ export class ScopeIgnore { */ private include: Ignore | null = null, private includeRoots: string[] = [], + /** + * The directories `git ls-files --exclude-standard` reports as ignored + * (see listGitIgnoredDirectories): root-relative, trailing-slashed paths. + * Each names one directory and is matched as a literal path prefix, never + * as a gitignore pattern. As a pattern, `src/` matched every `src/` below + * the root too, hiding a whole Maven monorepo's sources (#2849), and a + * folder named `[draft]` or `a[b` read as a glob (or did not compile). + */ + private gitIgnoredDirs: ReadonlySet = new Set(), ) { // Longest root first so paths in nested embedded repos hit the innermost matcher. this.embedded = [...embedded].sort((a, b) => b.root.length - a.root.length); } + /** True when `rel` is one of {@link gitIgnoredDirs} or lies inside one. */ + private inGitIgnoredDir(rel: string): boolean { + if (this.gitIgnoredDirs.size === 0) return false; + for (let slash = rel.indexOf('/'); slash !== -1; slash = rel.indexOf('/', slash + 1)) { + if (this.gitIgnoredDirs.has(rel.slice(0, slash + 1))) return true; + } + return false; + } + ignores(rel: string): boolean { // User `exclude` (#999) is checked first and against the full root-relative // path: it must drop git-TRACKED paths (which `.gitignore` can't) and apply @@ -994,7 +1018,7 @@ export class ScopeIgnore { if (rel.endsWith('/') && this.embedded.some(({ root }) => root.startsWith(rel))) { return false; } - return this.rootMatcher.ignores(rel); + return this.rootMatcher.ignores(rel) || this.inGitIgnoredDir(rel); } } @@ -1006,19 +1030,17 @@ export class ScopeIgnore { export function buildScopeIgnore(rootDir: string, embeddedRoots?: Iterable): ScopeIgnore { const roots = embeddedRoots ? [...embeddedRoots] : discoverEmbeddedRepoRoots(rootDir); const include = loadIncludeMatcher(rootDir); - // Root matcher already has defaults + root `.gitignore` + info/exclude + - // core.excludesFile. Seed ignored-untracked directories from git so nested - // `.gitignore` effects prune the watcher identically to the indexer (#1728). - const rootMatcher = buildDefaultIgnore(rootDir); - for (const dir of listGitIgnoredDirectories(rootDir)) { - rootMatcher.add(dir); - } + // The root matcher has defaults + root `.gitignore` + info/exclude + + // core.excludesFile. The ignored-untracked directories git reports join + // them, so nested `.gitignore` effects prune the watcher identically to the + // indexer (#1728) — as literal paths, not patterns (#2849). return new ScopeIgnore( - rootMatcher, + buildDefaultIgnore(rootDir), roots.map((root) => ({ root, matcher: buildDefaultIgnore(path.join(rootDir, root)) })), loadExcludeMatcher(rootDir), include, include ? includeStaticRoots(loadIncludePatterns(rootDir)) : [], + new Set(listGitIgnoredDirectories(rootDir)), ); } @@ -1167,6 +1189,18 @@ export function findUnindexedIgnoredRepos(rootDir: string): string[] { return repos; } +/** + * The `codegraph.json` `includeIgnored` pattern that opts in exactly the repo + * at `repoDir`, a root-relative, trailing-slashed path from + * {@link findUnindexedIgnoredRepos}. Patterns are gitignore syntax, so the + * path is anchored to the root (a bare `child/` also matches every `child/` + * below it) and its `*` and `[` are escaped (#2849). The `ignore` matcher has + * no working escape for `?`, so a `?` stays a one-character wildcard. + */ +export function includeIgnoredPatternFor(repoDir: string): string { + return '/' + repoDir.replace(/[*[]/g, '\\$&'); +} + /** * Discover embedded repos hidden by `repoDir`'s OWN gitignore rules: for each * gitignored directory, search for nested `.git` roots. Returns repo paths