mirror of
https://github.com/colbymchenry/codegraph.git
synced 2026-10-17 05:07:37 +08:00
fix(resolution): Vue, Svelte and Astro calls follow the JS/TS rules (#2128)
Calls in `.vue`, `.svelte` and `.astro` files carry their own language, so the JS/TS rules from #2120 never applied to them: halo's `t('…')` from `useI18n()` went to an interface property `t` (562 callers) and `ref(…)` from `vue` to a local `ref` (523). "Outside the repository" is now a package the importing file's package.json chain (or a Deno import map) declares, a Node built-in, `@std/…`, or a framework's virtual module. An alias the resolver can't follow — SvelteKit's `$lib/…`, a nested Nuxt app's `~/…`, a nested app's own `@/…` — stays the project's, and a `workspace:`/`file:`/`link:` dependency is in the repo. SFC files also get the JS built-ins (`fetch`, `Error`, `Map`), and a file's own import of a built-in's name (`import Map from './Map.svelte'`) is the import. Untyped three-segment chains (`api.groupReports.getAll()`) keep resolving in SFCs, where they reach their API client class. Co-authored-by: Claude Opus 5.5 <noreply@anthropic.com>
This commit is contained in:
@@ -14,6 +14,7 @@ and adheres to [Semantic Versioning](https://semver.org/spec/v2.0.0.html).
|
||||
|
||||
### Fixes
|
||||
|
||||
- Calls in Vue, Svelte and Astro components now follow the same rules as JavaScript and TypeScript. A `t('…')` from `useI18n()` or a `ref(…)` from `vue` used to link to an interface property or a local variable of the same name elsewhere in the project, so on halo two unrelated symbols had over a thousand made-up callers each. A call imported from a package is also no longer mistaken for a project symbol when the package is declared in a nested app's `package.json` or a Deno import map. An import through an alias CodeGraph can't follow, like SvelteKit's `$lib/…` or a nested Nuxt app's `~/…`, is still treated as project code. Re-index Vue, Svelte, Astro and JavaScript/TypeScript projects after upgrading.
|
||||
- A bare Java call like `verify(mock)`, `assertThat(x)` or `hashCode()` now links only to a method of the class it's in, of that class's parents, or one the file imports statically. It used to link to any class's method of that name: in halo, Mockito's `verify` and `eq` gave one service's `verify` over a thousand made-up callers, and in retrofit Truth's `assertThat` did the same to a test helper. Calls to inherited methods and static imports from the project now link to the right one, like jsoup's `attr(…)` through `LeafNode` or halo's `and(…)` / `equal(…)` through `import static …Queries.*`. Re-index Java projects after upgrading.
|
||||
- Python calls like `User.objects.get(…)`, `self.client.login(…)` or `request.POST.get(…)` no longer link to whichever class in the project has a method of that name. Such a call now links only to a member of what its receiver names, like `helpers.slugify()` to `slugify` in `helpers.py`, and a bare `get(1)` never links to a method. A name the file imports from a package outside the project, like `from django.shortcuts import render`, no longer links to a same-named project function. In Django projects these made-up links had given single test or serializer methods thousands of callers, for example netbox's `.all()` calls on one `UserConfig.all`. Re-index Python projects after upgrading.
|
||||
- In Rust, a bare `Ok(…)`, `Some(…)`, `Err(…)`, `Result<…>`, `Option<…>`, `Vec<…>` or `drop(…)` now means the standard library's, unless the file defines or imports a project item of that name. These used to link to any same-named enum variant or struct in the project: every `Some(x)` in ripgrep to one enum's `Some` variant, and every `Ok` and `Result` in serde to structs its macro-hygiene tests declare. An enum variant is now reached by a bare name only where the file `use`s it or its enum's `*`, and a variant never stands for a type. Re-index Rust projects after upgrading.
|
||||
|
||||
@@ -12,6 +12,12 @@
|
||||
* interface's `it` property or a class's `describe` field.
|
||||
*
|
||||
* A default import of a local module's instance still resolves by method.
|
||||
*
|
||||
* Vue, Svelte and Astro files follow the same rules (halo's 562 `t(…)` calls
|
||||
* went to an interface's `t`). "Outside the repository" means a package the
|
||||
* file's package.json declares, a Node built-in or a framework's virtual
|
||||
* module: an alias the resolver doesn't know (SvelteKit's `$lib/…`, a nested
|
||||
* Nuxt app's `~/…`) is still the project's.
|
||||
*/
|
||||
import { describe, it, expect, afterAll } from 'vitest';
|
||||
import * as fs from 'fs';
|
||||
@@ -120,4 +126,103 @@ describe('JS/TS: a member picked by name alone', () => {
|
||||
cg.close();
|
||||
}
|
||||
});
|
||||
|
||||
it('applies to Vue and Svelte files, and an unknown alias is still the project', async () => {
|
||||
const root = fs.mkdtempSync(path.join(os.tmpdir(), 'cg-js-sfc-guess-'));
|
||||
roots.push(root);
|
||||
const files: Record<string, string> = {
|
||||
'frontend/package.json': JSON.stringify({ name: 'frontend', dependencies: { 'vue-i18n': '*', dayjs: '*', '@app/shared': 'workspace:*' } }),
|
||||
'shared/index.ts': `export function formatUrl(u: string) { return u; }
|
||||
`,
|
||||
'frontend/types/context.ts': `export interface ProvidersContext {
|
||||
t: (key: string) => string;
|
||||
}
|
||||
`,
|
||||
'frontend/test/mocks.ts': `export function dayjs() { return 0; }
|
||||
`,
|
||||
'frontend/composables/store.ts': `export function useLabelStore() { return {}; }
|
||||
`,
|
||||
'frontend/composables/use-labels.ts': `import { useLabelStore } from '~/composables/store';
|
||||
export function useLabels() { return useLabelStore(); }
|
||||
`,
|
||||
'frontend/pages/labels.vue': `<script setup lang="ts">
|
||||
import { useI18n } from 'vue-i18n';
|
||||
import dayjs from 'dayjs';
|
||||
import { useLabelStore } from '~/composables/store';
|
||||
import { formatUrl } from '@app/shared';
|
||||
const { t } = useI18n();
|
||||
const url = formatUrl('/labels');
|
||||
const title = t('labels.title');
|
||||
const today = dayjs();
|
||||
const store = useLabelStore();
|
||||
</script>
|
||||
<template><h1>{{ title }}</h1></template>
|
||||
`,
|
||||
'docs/package.json': JSON.stringify({ name: 'docs', devDependencies: { '@sveltejs/kit': '*' } }),
|
||||
'docs/src/lib/design.ts': `export function useDesignSystem() { return 1; }
|
||||
`,
|
||||
'docs/src/lib/router.ts': `export function goto(path: string) { return path; }
|
||||
`,
|
||||
'docs/src/lib/use.ts': `import { useDesignSystem } from '$lib/design';
|
||||
export const current = () => useDesignSystem();
|
||||
`,
|
||||
'docs/src/lib/instance.js': `export const DEV = true;
|
||||
if (DEV) {
|
||||
const fetch = globalThis.fetch;
|
||||
globalThis.fetch = async (info) => fetch(info);
|
||||
}
|
||||
`,
|
||||
'docs/src/lib/Map.svelte': `<div>map</div>
|
||||
`,
|
||||
'docs/src/routes/+page.svelte': `<script lang="ts">
|
||||
import Map from '../lib/Map.svelte';
|
||||
import { useDesignSystem } from '$lib/design';
|
||||
import { goto } from '$app/navigation';
|
||||
const ds = useDesignSystem();
|
||||
function go() { goto('/'); fetch('/api'); }
|
||||
</script>
|
||||
<button on:click={go}>{ds}</button>
|
||||
`,
|
||||
// Deno: the import map says `@std/assert` comes from JSR.
|
||||
'deno/deno.json': JSON.stringify({ imports: { '@std/assert': 'jsr:@std/assert@^1', 'app/': '../src/' } }),
|
||||
'deno/test/assert.ts': `export function assertEquals(a: unknown, b: unknown) { return a === b; }
|
||||
`,
|
||||
'deno/app.test.ts': `import { assertEquals } from '@std/assert';
|
||||
assertEquals(1, 1);
|
||||
`,
|
||||
};
|
||||
for (const [rel, content] of Object.entries(files)) {
|
||||
fs.mkdirSync(path.dirname(path.join(root, rel)), { recursive: true });
|
||||
fs.writeFileSync(path.join(root, rel), content);
|
||||
}
|
||||
const cg = await CodeGraph.init(root, { index: true });
|
||||
try {
|
||||
const callsFrom = (file: string): string[] => {
|
||||
const ids = cg.getNodesInFile(file).map((n) => n.id);
|
||||
return cg
|
||||
.getOutgoingEdgesFrom(ids, ['calls'])
|
||||
.map((e) => cg.getNode(e.target)?.qualifiedName ?? '?')
|
||||
.sort();
|
||||
};
|
||||
const fromVue = callsFrom('frontend/pages/labels.vue');
|
||||
expect(fromVue).not.toContain('ProvidersContext::t');
|
||||
expect(fromVue).not.toContain('dayjs');
|
||||
expect(fromVue).toContain('useLabelStore');
|
||||
// A `workspace:*` dependency is in the repository.
|
||||
expect(fromVue).toContain('formatUrl');
|
||||
expect(callsFrom('frontend/composables/use-labels.ts')).toContain('useLabelStore');
|
||||
const fromSvelte = callsFrom('docs/src/routes/+page.svelte');
|
||||
expect(fromSvelte).toContain('useDesignSystem');
|
||||
expect(fromSvelte).not.toContain('goto');
|
||||
expect(fromSvelte).not.toContain('fetch');
|
||||
expect(callsFrom('deno/app.test.ts')).not.toContain('assertEquals');
|
||||
// A built-in's name imported from the project is the import.
|
||||
const svelteIds = cg.getNodesInFile('docs/src/routes/+page.svelte').map((n) => n.id);
|
||||
const imported = cg.getOutgoingEdgesFrom(svelteIds, ['imports']).map((e) => cg.getNode(e.target)!.filePath);
|
||||
expect(imported).toContain('docs/src/lib/Map.svelte');
|
||||
expect(callsFrom('docs/src/lib/use.ts')).toContain('useDesignSystem');
|
||||
} finally {
|
||||
cg.close();
|
||||
}
|
||||
});
|
||||
});
|
||||
|
||||
+72
-17
@@ -41,6 +41,10 @@ import { lexicalPathWithinRoot } from '../utils';
|
||||
import type { ReExport } from './types';
|
||||
import { LRUCache } from './lru-cache';
|
||||
import { JS_BUILT_INS } from './js-builtins';
|
||||
import { builtinModules } from 'module';
|
||||
import { parse as parseJsonc } from 'jsonc-parser';
|
||||
|
||||
const NODE_BUILTINS = new Set(builtinModules);
|
||||
|
||||
/** Node kinds that can declare supertypes (extends/implements). */
|
||||
const SUPERTYPE_BEARING_KINDS = new Set<Node['kind']>([
|
||||
@@ -439,6 +443,7 @@ export class ReferenceResolver {
|
||||
this.supertypeGen++;
|
||||
this.nodesByKindCache.clear();
|
||||
this.fileExistsMemo.clear();
|
||||
this.manifestScopes.clear();
|
||||
this.knownNames = null;
|
||||
this.knownFiles = null;
|
||||
this.cachesWarmed = false;
|
||||
@@ -503,8 +508,8 @@ export class ReferenceResolver {
|
||||
resolveImport: (ref) => resolveViaImport(ref, this.context),
|
||||
isOutOfRepoImport: (source, fromFile, language) =>
|
||||
isExternalImport(source, language, this.context) &&
|
||||
!this.isOwnPackage(source) &&
|
||||
resolveImportPath(source, fromFile, language, this.context) === null,
|
||||
resolveImportPath(source, fromFile, language, this.context) === null &&
|
||||
this.isDeclaredOutsidePackage(source, fromFile),
|
||||
getNodesInFile: (filePath: string) => {
|
||||
if (!this.nodeCache.has(filePath)) {
|
||||
this.nodeCache.set(filePath, this.queries.getNodesByFile(filePath));
|
||||
@@ -2343,10 +2348,13 @@ export class ReferenceResolver {
|
||||
private isBuiltInOrExternal(ref: UnresolvedRef): boolean {
|
||||
const name = ref.referenceName;
|
||||
const isJsTs = ref.language === 'typescript' || ref.language === 'javascript'
|
||||
|| ref.language === 'tsx' || ref.language === 'jsx' || ref.language === 'arkts';
|
||||
|| ref.language === 'tsx' || ref.language === 'jsx' || ref.language === 'arkts'
|
||||
|| ref.language === 'vue' || ref.language === 'svelte' || ref.language === 'astro';
|
||||
|
||||
// JavaScript/TypeScript built-ins
|
||||
if (isJsTs && JS_BUILT_INS.has(name)) {
|
||||
// JavaScript/TypeScript built-ins — unless the file imports its own
|
||||
// binding of that name (`import Map from './Map.svelte'`).
|
||||
if (isJsTs && JS_BUILT_INS.has(name) &&
|
||||
!this.context.getImportMappings(ref.filePath, ref.language).some((m) => m.localName === name)) {
|
||||
return true;
|
||||
}
|
||||
|
||||
@@ -2873,20 +2881,67 @@ export class ReferenceResolver {
|
||||
}
|
||||
|
||||
/** The repository's own package name, from its root package.json; null without one. */
|
||||
private ownPackageName: string | null | undefined;
|
||||
/** Per directory: the package names its package.json and every enclosing one own and depend on. */
|
||||
private manifestScopes = new Map<string, { own: Set<string>; deps: Set<string> }>();
|
||||
|
||||
/** `axios` (or `axios/unsafe/…`) inside the axios repository: a package importing itself. */
|
||||
private isOwnPackage(source: string): boolean {
|
||||
if (this.ownPackageName === undefined) {
|
||||
let name: string | null = null;
|
||||
try {
|
||||
const json = JSON.parse(this.context.readFile('package.json') ?? 'null') as { name?: unknown } | null;
|
||||
if (json && typeof json.name === 'string' && json.name.length > 0) name = json.name;
|
||||
} catch { /* no or unreadable package.json */ }
|
||||
this.ownPackageName = name;
|
||||
/**
|
||||
* Is `source` a package from outside the repository? Only when the importing
|
||||
* file's package.json (or an enclosing one) declares it, or it is a Node
|
||||
* built-in or a runtime's virtual module. A specifier nothing declares is
|
||||
* an alias this resolver does not know — SvelteKit's `$lib/…`, Nuxt's
|
||||
* `~/…`, a nested app's own `@/…` — and stays the project's.
|
||||
*/
|
||||
private isDeclaredOutsidePackage(source: string, fromFile: string): boolean {
|
||||
// Deno's standard library is `@std/…` from JSR.
|
||||
if (/^(?:node|bun|jsr|npm|https?):/.test(source) || source.startsWith('@std/') || NODE_BUILTINS.has(source)) return true;
|
||||
const name = source.startsWith('@') ? source.split('/').slice(0, 2).join('/') : source.split('/')[0]!;
|
||||
const normalized = fromFile.replace(/\\/g, '/');
|
||||
const scope = this.manifestScope(normalized.includes('/') ? normalized.slice(0, normalized.lastIndexOf('/')) : '');
|
||||
if (scope.own.has(name)) return false;
|
||||
if (scope.deps.has(name)) return true;
|
||||
// SvelteKit's `$app/…` and Astro's `astro:…` belong to the framework —
|
||||
// unless this repository is that framework.
|
||||
const provider = source.startsWith('astro:') ? 'astro' : /^\$(?:app|env|service-worker)(?:\/|$)/.test(source) ? '@sveltejs/kit' : null;
|
||||
return provider !== null && !scope.own.has(provider) && !this.context.getWorkspacePackages?.()?.byName.has(provider);
|
||||
}
|
||||
|
||||
private manifestScope(dir: string): { own: Set<string>; deps: Set<string> } {
|
||||
const memo = this.manifestScopes.get(dir);
|
||||
if (memo) return memo;
|
||||
const parent = dir === '' ? null : this.manifestScope(dir.includes('/') ? dir.slice(0, dir.lastIndexOf('/')) : '');
|
||||
let scope = parent ?? { own: new Set<string>(), deps: new Set<string>() };
|
||||
try {
|
||||
const json = JSON.parse(this.context.readFile(dir ? `${dir}/package.json` : 'package.json') ?? 'null') as Record<string, unknown> | null;
|
||||
if (json && typeof json === 'object') {
|
||||
scope = { own: new Set(scope.own), deps: new Set(scope.deps) };
|
||||
if (typeof json.name === 'string' && json.name.length > 0) scope.own.add(json.name);
|
||||
for (const field of ['dependencies', 'devDependencies', 'peerDependencies', 'optionalDependencies']) {
|
||||
const deps = json[field];
|
||||
if (!deps || typeof deps !== 'object') continue;
|
||||
for (const [dep, version] of Object.entries(deps)) {
|
||||
// `workspace:*`, `file:../shared`, `link:…`: a package in this repository.
|
||||
if (typeof version === 'string' && /^(?:workspace|file|link|portal):/.test(version)) scope.own.add(dep);
|
||||
else scope.deps.add(dep);
|
||||
}
|
||||
}
|
||||
}
|
||||
} catch { /* no or unreadable package.json */ }
|
||||
// A Deno import map names packages the same way — an entry that maps to a
|
||||
// registry or a URL, not one that maps to a path in the repository.
|
||||
for (const file of ['deno.json', 'deno.jsonc']) {
|
||||
const raw = this.context.readFile(dir ? `${dir}/${file}` : file);
|
||||
if (!raw) continue;
|
||||
const json = parseJsonc(raw) as { imports?: Record<string, unknown> } | undefined;
|
||||
const imports = json && typeof json === 'object' ? json.imports : undefined;
|
||||
if (!imports || typeof imports !== 'object') continue;
|
||||
for (const [key, target] of Object.entries(imports)) {
|
||||
if (typeof target !== 'string' || !/^(?:jsr|npm|https?):/.test(target)) continue;
|
||||
if (scope === parent) scope = { own: new Set(scope.own), deps: new Set(scope.deps) };
|
||||
scope.deps.add(key.replace(/\/$/, ''));
|
||||
}
|
||||
}
|
||||
const own = this.ownPackageName;
|
||||
return own !== null && (source === own || source.startsWith(`${own}/`));
|
||||
this.manifestScopes.set(dir, scope);
|
||||
return scope;
|
||||
}
|
||||
|
||||
/** The one supertype-kind node a TypeScript value shares its name and file with. */
|
||||
|
||||
@@ -935,7 +935,13 @@ export function isVisibleAcrossFiles(candidate: Node, ref: UnresolvedRef, contex
|
||||
return isCrossFileReachable(candidate, ref, context);
|
||||
}
|
||||
|
||||
const JS_FAMILY = new Set<string>(['typescript', 'tsx', 'javascript', 'jsx']);
|
||||
/**
|
||||
* Languages whose calls are JS/TS calls — Vue, Svelte and Astro components'
|
||||
* scripts and template expressions included: a bare `t('key')` in a `.vue`
|
||||
* file resolves lexically exactly as in a `.ts` one.
|
||||
*/
|
||||
const JS_FAMILY = new Set<string>(['typescript', 'tsx', 'javascript', 'jsx', 'vue', 'svelte', 'astro']);
|
||||
const JS_TS = new Set<string>(['typescript', 'tsx', 'javascript', 'jsx']);
|
||||
|
||||
/**
|
||||
* Whether a JS/TS `calls` ref is a RECEIVER-LESS call — `serialize(x)`, not
|
||||
@@ -4283,9 +4289,11 @@ export function matchJsStoreBindingCall(ref: UnresolvedRef, context: ResolutionC
|
||||
}
|
||||
|
||||
/** A qualified untyped chain is useful source evidence, not permission to
|
||||
* infer a property type. Framework resolution runs before this guard. */
|
||||
* infer a property type. Framework resolution runs before this guard. Vue,
|
||||
* Svelte and Astro files keep resolving them: there `api.groupReports.getAll()`
|
||||
* reaches its API client class far more often than a wrong namesake. */
|
||||
export function isUnresolvedJsMemberCall(ref: UnresolvedRef): boolean {
|
||||
return ref.referenceKind === 'calls' && JS_FAMILY.has(ref.language) &&
|
||||
return ref.referenceKind === 'calls' && JS_TS.has(ref.language) &&
|
||||
!/^(?:this|window)\./.test(ref.referenceName) &&
|
||||
/^[A-Za-z_$][\w$]*(?:\.[A-Za-z_$][\w$]*){2,}$/.test(ref.referenceName);
|
||||
}
|
||||
|
||||
Reference in New Issue
Block a user