* fix(resolution): gate extends/implements to real supertypes
An inheritance reference bound to whatever local symbol shared its name.
The name-matcher scores node kind as a bonus, never a filter, and awards
no bonus at all for inheritance refs, so `use std::error::Error;` +
`impl Error for MapperError {}` resolved to the local `MapperError::Error`
VARIANT — an implementation relationship absent from the source.
Two changes, both needed. Filtering by kind alone was measured and it
only RELOCATES the false edge: with enum members excluded, the same 7
refs moved onto an unrelated local `type Error` alias, which is a legal
supertype kind and therefore harder for a consumer to reject.
1. Eligibility before ranking. `matchByExactName` restricts its candidate
pool to kinds that can BE a supertype, so a legitimate trait outranks
a same-named variant instead of merely losing its edge. `resolveOne`
is wrapped by a gate that applies the same set to every other strategy
at one seam — filtering inside the name-matcher would have missed the
framework, import, chain and CFML paths.
2. Locality. A name imported from outside the repository has no in-repo
referent at all, so no candidate is correct. Only oracles that cannot
be wrong are consulted: Rust `use` paths rooted at a stdlib crate, and
`isExternalImport` for ES modules. Generalizing the Rust side to "the
module path doesn't resolve to a file" was tried and reverted — a
crate re-exporting a sibling's modules (`pub use pupil_core::ports;`)
has no directory to walk, and that version deleted 13 real trait
implementations.
Measured on a Rust/Tauri project (2,682 nodes): the 11 false inheritance
edges are gone, all 59 real trait relationships are preserved, and node
count is unchanged. On this repository as a control, the only edge
removed is a class recorded as extending a function. Synthesized-edge
counts are identical in both.
Co-Authored-By: Claude Opus 5 <noreply@anthropic.com>
* fix(resolution): an import never resolves to a member of a type
`import * as path from 'node:path'` is unresolvable — the module is
external — so the name-matcher fell back to finding any node called
`path`, and a common word like path/url/join/get matches a class property
or interface method somewhere in almost any repo. Nothing in any
supported language lets an import bind to a member that only exists
inside a type; you import the type.
Same shape as the inheritance gate that precedes it: eligibility applied
to the candidate pool before ranking, plus the resolveOne gate as the
backstop for every other strategy.
On this repository as a control: 19 imports pointing at methods and 4 at
properties are gone (all of them coincidences — `Walker::join`,
`Telemetry::events`), 3 refs now find the module constant they actually
name, node count unchanged.
Co-Authored-By: Claude Opus 5 <noreply@anthropic.com>
* fix(resolution): classify SFC script imports as ES module specifiers
`isExternalImport` had a TS/JS branch listing typescript/tsx/javascript/jsx/
arkts, so for Svelte, Vue and Astro it fell through every branch and returned
false — "not external" — for `import { Foo } from 'some-npm-pkg'`.
An SFC imports inside its `<script>` block (Astro: the `---` frontmatter) with
ordinary ES module syntax; `extractImportMappings` already routes all three
through the same `extractJSImports`. So the classifier disagreed with the
extractor about what those imports are.
Effect on the preceding commit: its locality check asks `isExternalImport`, so
it silently did nothing for SFCs. A class in a `.svelte`/`.vue`/`.astro` file
implementing a type imported from an npm package still bound to whatever local
class shared that name — verified against this branch before the fix, all three
languages.
The language set is now one constant used by both the classifier and the
locality check, so they cannot drift apart again. Relative and aliased
specifiers are unaffected: the branch returns "not external" for `./…`,
workspace members, tsconfig alias prefixes, `@/`, `~/` and `src/` exactly as it
does for `.ts`.
No edge changes on this repository as a control.
Co-Authored-By: Claude Opus 5 <noreply@anthropic.com>
---------
Co-authored-by: ctype_lab <cksgud1226@gmail.com>
Co-authored-by: Claude Opus 5 <noreply@anthropic.com>
This commit is contained in:
co-authored by
Claude Opus 5
ctype_lab
parent
040ba388da
commit
374b3b4209
@@ -316,6 +316,21 @@ const C_CPP_STDLIB_HEADERS = new Set([
|
||||
'version',
|
||||
]);
|
||||
|
||||
/**
|
||||
* Languages whose imports are ES-module specifiers, extracted by
|
||||
* `extractJSImports` and therefore classified by the same bare-specifier /
|
||||
* alias / workspace rules. Svelte, Vue and Astro belong here: an SFC imports
|
||||
* inside its `<script>` block (Astro: the `---` frontmatter) with exactly the
|
||||
* same syntax, and leaving them out made `isExternalImport` answer "not
|
||||
* external" for every npm specifier in an SFC.
|
||||
*/
|
||||
const ESM_IMPORT_LANGUAGES = new Set<Language>([
|
||||
'typescript', 'tsx', 'javascript', 'jsx', 'arkts', 'svelte', 'vue', 'astro',
|
||||
]);
|
||||
|
||||
/** Rust path roots that always name a standard-library crate. */
|
||||
const RUST_STDLIB_ROOTS = new Set(['std', 'core', 'alloc', 'proc_macro']);
|
||||
|
||||
/**
|
||||
* Check if an import is external (npm package, etc.)
|
||||
*
|
||||
@@ -344,7 +359,7 @@ function isExternalImport(
|
||||
}
|
||||
|
||||
// Common external patterns
|
||||
if (language === 'typescript' || language === 'javascript' || language === 'tsx' || language === 'jsx' || language === 'arkts') {
|
||||
if (ESM_IMPORT_LANGUAGES.has(language)) {
|
||||
// Node built-ins
|
||||
if (['fs', 'path', 'os', 'crypto', 'http', 'https', 'url', 'util', 'events', 'stream', 'child_process', 'buffer'].includes(importPath)) {
|
||||
return true;
|
||||
@@ -2487,3 +2502,123 @@ function resolveStaticMember(
|
||||
}
|
||||
return candidates[0];
|
||||
}
|
||||
|
||||
/**
|
||||
* Rust `use` declarations, flattened to `localName → full path`.
|
||||
*
|
||||
* Rust is the one supported language with NO `ImportMapping` extraction (see
|
||||
* `extractImportMappings`), so this is the only channel that can tell whether
|
||||
* a bare type name in a Rust file was brought in by a `use`. Handles nested
|
||||
* groups (`use a::{b::C, d as E}`), globs (skipped — they bind no single
|
||||
* name), and `as` aliases.
|
||||
*/
|
||||
function collectRustUseBindings(content: string): Map<string, string> {
|
||||
const out = new Map<string, string>();
|
||||
|
||||
// Expand one level of `{...}` at a time so `a::{b::{C, D}, E}` flattens.
|
||||
const expand = (spec: string): string[] => {
|
||||
const open = spec.indexOf('{');
|
||||
if (open === -1) return [spec.trim()];
|
||||
const prefix = spec.slice(0, open);
|
||||
let depth = 0;
|
||||
let close = -1;
|
||||
for (let i = open; i < spec.length; i++) {
|
||||
if (spec[i] === '{') depth++;
|
||||
else if (spec[i] === '}') {
|
||||
depth--;
|
||||
if (depth === 0) { close = i; break; }
|
||||
}
|
||||
}
|
||||
if (close === -1) return [];
|
||||
const suffix = spec.slice(close + 1);
|
||||
const inner = spec.slice(open + 1, close);
|
||||
const parts: string[] = [];
|
||||
let depth2 = 0;
|
||||
let start = 0;
|
||||
for (let i = 0; i <= inner.length; i++) {
|
||||
const ch = inner[i];
|
||||
if (ch === '{') depth2++;
|
||||
else if (ch === '}') depth2--;
|
||||
if (i === inner.length || (ch === ',' && depth2 === 0)) {
|
||||
const seg = inner.slice(start, i).trim();
|
||||
if (seg) parts.push(seg);
|
||||
start = i + 1;
|
||||
}
|
||||
}
|
||||
return parts.flatMap((p) => expand(prefix + p + suffix));
|
||||
};
|
||||
|
||||
// `use` items end at the first `;`. Attributes/visibility (`pub use`) are
|
||||
// irrelevant to the binding itself.
|
||||
const useRe = /(^|\n)\s*(?:pub(?:\([^)]*\))?\s+)?use\s+([^;]+);/g;
|
||||
let m: RegExpExecArray | null;
|
||||
while ((m = useRe.exec(content)) !== null) {
|
||||
for (const spec of expand(m[2]!.replace(/\s+/g, ' '))) {
|
||||
const aliasMatch = /^(.*?)\s+as\s+([A-Za-z_]\w*)$/.exec(spec);
|
||||
const rawPath = (aliasMatch ? aliasMatch[1]! : spec).trim();
|
||||
if (!rawPath || rawPath.endsWith('*')) continue;
|
||||
const segments = rawPath.split('::').map((s) => s.trim()).filter(Boolean);
|
||||
const leaf = segments[segments.length - 1];
|
||||
if (!leaf) continue;
|
||||
const local = aliasMatch ? aliasMatch[2]! : leaf;
|
||||
out.set(local, segments.join('::'));
|
||||
}
|
||||
}
|
||||
return out;
|
||||
}
|
||||
|
||||
/**
|
||||
* Is `name`, as used in `ref`'s file, bound by an import whose module lives
|
||||
* OUTSIDE the repository?
|
||||
*
|
||||
* When it is, no in-repo node can be the referent: the symbol is defined in a
|
||||
* third-party crate/package, and any same-named local symbol the name-matcher
|
||||
* finds is a coincidence. Rust `use std::error::Error;` + `impl Error for
|
||||
* MapperError {}` bound to a local `MapperError::Error` variant, and once
|
||||
* non-type kinds were filtered out it simply moved to an unrelated local
|
||||
* `type Error` alias — restricting kinds alone RELOCATES the false edge
|
||||
* instead of removing it, so locality has to be checked too.
|
||||
*
|
||||
* Answers only when it can be CERTAIN, because a false "yes" deletes a real
|
||||
* edge. Two languages qualify, each with an oracle that cannot be wrong:
|
||||
*
|
||||
* - **Rust** — the `use` path is rooted at a standard-library crate
|
||||
* (`std`/`core`/`alloc`/`proc_macro`), which by definition ships outside
|
||||
* any repository. Deliberately NOT generalized to "the module path doesn't
|
||||
* resolve to a file": a crate can re-export another workspace crate's
|
||||
* modules (`pub use pupil_core::{ports, domain};`), so `crate::ports::X`
|
||||
* has no `src/ports/` directory to walk yet is entirely in-repo — that
|
||||
* generalization measured 13 real trait implementations deleted.
|
||||
* - **ES modules** — `isExternalImport`, which already accounts for tsconfig
|
||||
* path aliases and monorepo workspace packages.
|
||||
*
|
||||
* Everything else returns false and resolves exactly as before. JVM and Python
|
||||
* imports notably do NOT go through `resolveImportPath` (they have dedicated
|
||||
* FQN/module matchers), so there is no trustworthy oracle to consult here.
|
||||
*/
|
||||
export function isBoundToOutOfRepoImport(
|
||||
ref: UnresolvedRef,
|
||||
context: ResolutionContext
|
||||
): boolean {
|
||||
const name = ref.referenceName;
|
||||
if (name.includes('::') || name.includes('.')) return false; // qualified refs resolve by path
|
||||
|
||||
if (ref.language === 'rust') {
|
||||
const content = context.readFile(ref.filePath);
|
||||
if (!content) return false;
|
||||
const usePath = collectRustUseBindings(content).get(name);
|
||||
if (!usePath) return false;
|
||||
const segments = usePath.split('::');
|
||||
if (segments.length < 2 || !RUST_STDLIB_ROOTS.has(segments[0]!)) return false;
|
||||
// 2015-edition crate-relative paths can shadow a stdlib root with a local
|
||||
// module of the same name — if the path walks to a real file, it's local.
|
||||
return resolveRustModuleFile(segments.slice(0, -1), ref.filePath, context) === null;
|
||||
}
|
||||
|
||||
if (!ESM_IMPORT_LANGUAGES.has(ref.language)) return false;
|
||||
for (const imp of context.getImportMappings(ref.filePath, ref.language)) {
|
||||
if (imp.localName !== name) continue;
|
||||
return isExternalImport(imp.source, ref.language, context);
|
||||
}
|
||||
return false;
|
||||
}
|
||||
|
||||
+58
-2
@@ -15,9 +15,12 @@ import {
|
||||
ResolutionContext,
|
||||
FrameworkResolver,
|
||||
ImportMapping,
|
||||
SUPERTYPE_TARGET_KINDS,
|
||||
isInheritanceRef,
|
||||
isImportableKind,
|
||||
} from './types';
|
||||
import { isVisibleAcrossFiles, matchReference, matchFunctionRef, matchDottedCallChain, matchScopedCallChain, matchMethodCall, sameLanguageFamily, crossesKnownFamily, dumpNameMatcherProfile, clearNameMatcherMemos } from './name-matcher';
|
||||
import { resolveViaImport, resolvePhpImportedStaticCall, resolveJvmImport, extractImportMappings, extractReExports, loadCppIncludeDirs, isPhpIncludePathRef, isCobolCopybookRef, isNixPathImportRef, clearImportResolverMemos, resolveImportPath } from './import-resolver';
|
||||
import { resolveViaImport, resolvePhpImportedStaticCall, resolveJvmImport, extractImportMappings, extractReExports, loadCppIncludeDirs, isPhpIncludePathRef, isCobolCopybookRef, isNixPathImportRef, isBoundToOutOfRepoImport, clearImportResolverMemos, resolveImportPath } from './import-resolver';
|
||||
import { ResolverPool, minRefsForPool } from './resolver-pool';
|
||||
import { detectFrameworks } from './frameworks';
|
||||
import { synthesizeCallbackEdges } from './callback-synthesizer';
|
||||
@@ -36,6 +39,11 @@ const SUPERTYPE_BEARING_KINDS = new Set<Node['kind']>([
|
||||
'class', 'struct', 'interface', 'trait', 'protocol', 'enum',
|
||||
]);
|
||||
|
||||
// SUPERTYPE_TARGET_KINDS (the kinds an extends/implements edge may TARGET)
|
||||
// lives in ./types — the name-matcher needs the same set to restrict its
|
||||
// candidate pool before ranking. It is deliberately wider than
|
||||
// SUPERTYPE_BEARING_KINDS above, which is about the DECLARING side.
|
||||
|
||||
/**
|
||||
* Languages whose chained static-factory/fluent calls defer to the conformance
|
||||
* second pass. Dotted-receiver languages resolve via matchDottedCallChain; the
|
||||
@@ -857,9 +865,18 @@ export class ReferenceResolver {
|
||||
}
|
||||
|
||||
/**
|
||||
* Resolve a single reference
|
||||
* Resolve a single reference.
|
||||
*
|
||||
* Thin decorator over `resolveOneInner` so every strategy — framework,
|
||||
* import, name-match, chain, CFML component path — passes through the
|
||||
* inheritance target-kind gate at ONE seam. Filtering inside the
|
||||
* name-matcher would have covered `matchByExactName` only.
|
||||
*/
|
||||
resolveOne(ref: UnresolvedRef): ResolvedRef | null {
|
||||
return this.gateTargetKind(this.resolveOneInner(ref), ref);
|
||||
}
|
||||
|
||||
private resolveOneInner(ref: UnresolvedRef): ResolvedRef | null {
|
||||
// Skip built-in/external references
|
||||
if (this.isBuiltInOrExternal(ref)) {
|
||||
return null;
|
||||
@@ -2582,6 +2599,45 @@ export class ReferenceResolver {
|
||||
return this.persistDeferredReferences(deferred, resolved);
|
||||
}
|
||||
|
||||
/**
|
||||
* Drop a resolution whose target cannot be what the reference names.
|
||||
* Applied at the `resolveOne` seam so it covers every strategy uniformly —
|
||||
* framework, import, name-match, chain, CFML component path.
|
||||
*
|
||||
* For `imports`: the target must be importable. A member that only exists
|
||||
* inside a type never is.
|
||||
*
|
||||
* For `extends`/`implements`, it cannot be describing a real supertype when:
|
||||
*
|
||||
* 1. The target's kind can never be a supertype (an enum member, a method,
|
||||
* a variable). `matchByExactName` additionally narrows its candidate
|
||||
* pool by the same set, so a legitimate supertype outranks a same-named
|
||||
* non-type rather than merely losing its edge.
|
||||
* 2. The name is imported from outside the repo, so NO local node is the
|
||||
* referent. Without this, filtering by kind alone just relocates the
|
||||
* false edge onto the next same-named local type.
|
||||
*
|
||||
* Direction is one-way: this only ever REMOVES an edge, never adds one. A
|
||||
* dropped ref stays in `unresolved_refs` as `failed`, which is the honest
|
||||
* record for a supertype that lives outside the repo — silent beats wrong.
|
||||
*/
|
||||
private gateTargetKind(result: ResolvedRef | null, ref: UnresolvedRef): ResolvedRef | null {
|
||||
if (!result) return result;
|
||||
|
||||
// An `imports` reference names something importable — never a member that
|
||||
// only exists inside a type.
|
||||
if (ref.referenceKind === 'imports') {
|
||||
const target = this.queries.getNodeById(result.targetNodeId);
|
||||
return target && !isImportableKind(target.kind) ? null : result;
|
||||
}
|
||||
|
||||
if (!isInheritanceRef(ref)) return result;
|
||||
const target = this.queries.getNodeById(result.targetNodeId);
|
||||
if (target && !SUPERTYPE_TARGET_KINDS.has(target.kind)) return null;
|
||||
if (isBoundToOutOfRepoImport(ref, this.context)) return null;
|
||||
return result;
|
||||
}
|
||||
|
||||
private gateLanguage(result: ResolvedRef | null, ref: UnresolvedRef): ResolvedRef | null {
|
||||
if (!result) return result;
|
||||
const tgt = this.getLanguageFromNodeId(result.targetNodeId);
|
||||
|
||||
@@ -6,7 +6,7 @@
|
||||
|
||||
import * as path from 'path';
|
||||
import { Language, Node } from '../types';
|
||||
import { UnresolvedRef, ResolvedRef, ResolutionContext } from './types';
|
||||
import { UnresolvedRef, ResolvedRef, ResolutionContext, SUPERTYPE_TARGET_KINDS, isInheritanceRef, isImportableKind } from './types';
|
||||
import { blankStringContents, stripCommentsForRegex } from './strip-comments';
|
||||
import { JS_BUILT_INS } from './js-builtins';
|
||||
|
||||
@@ -762,7 +762,19 @@ export function matchByExactName(
|
||||
.filter((n) => !(bareJs && n.kind === 'method'))
|
||||
// A name the file binds itself (a parameter, a const) shadows every other
|
||||
// file's symbol of that name, so a bare call has no cross-file candidate.
|
||||
.filter((n) => !(bareJs && n.filePath !== ref.filePath && isLocallyBoundJsName(ref.referenceName, ref.filePath, context)));
|
||||
.filter((n) => !(bareJs && n.filePath !== ref.filePath && isLocallyBoundJsName(ref.referenceName, ref.filePath, context)))
|
||||
// An `extends`/`implements` ref names a supertype, so anything that can't
|
||||
// BE one is not a candidate at all. This is eligibility, not
|
||||
// ranking: kind is only a scoring bonus below (and none is awarded for
|
||||
// inheritance refs), so without this a same-named `enum_member` outranked
|
||||
// the real `trait`, and as the sole candidate was adopted outright by the
|
||||
// single-match shortcut. Restricting the pool BEFORE ranking lets the
|
||||
// legitimate supertype win instead of merely dropping the false edge.
|
||||
.filter((n) => !isInheritanceRef(ref) || SUPERTYPE_TARGET_KINDS.has(n.kind))
|
||||
// Likewise for `imports`: a member that only exists inside a type is not
|
||||
// importable, so it is not a candidate. Without this a `path`/`id`/`url`
|
||||
// import resolved to some interface's same-named property.
|
||||
.filter((n) => ref.referenceKind !== 'imports' || isImportableKind(n.kind));
|
||||
|
||||
if (candidates.length === 0) {
|
||||
return null;
|
||||
|
||||
@@ -295,3 +295,54 @@ export type ReExport =
|
||||
/** Module specifier of the upstream module. */
|
||||
source: string;
|
||||
};
|
||||
|
||||
/**
|
||||
* Node kinds an `extends`/`implements` edge may legally TARGET — the things a
|
||||
* type can actually inherit from or conform to.
|
||||
*
|
||||
* Kept deliberately wide: `type_alias` because TS `class X implements
|
||||
* SomeAliasedObjectType` is valid, `component` because a framework component
|
||||
* node stands in for a class, and `module`/`namespace` because whole
|
||||
* languages inherit from one — Ruby `include Trackable` targets a `module`,
|
||||
* Erlang `-behaviour(gen_server)` targets the behaviour module, which Erlang
|
||||
* extraction indexes as a `namespace` (the conformance pass in
|
||||
* `resolution/index.ts` makes the same `module` allowance).
|
||||
*
|
||||
* Everything omitted (`enum_member`, `method`, `field`, `property`,
|
||||
* `variable`, `constant`, `function`, `parameter`, `import`, `export`,
|
||||
* `file`, `route`) can never be a supertype in any supported language, so an
|
||||
* inheritance edge pointing at one is false data.
|
||||
*
|
||||
* Why this is needed: the name-matcher scores node kind as a BONUS,
|
||||
* never a filter, and awards no bonus at all for inheritance refs — so a
|
||||
* same-named non-type outranked (or, as the sole candidate, was adopted
|
||||
* outright as) the real supertype. Rust `use std::error::Error;` + `impl Error
|
||||
* for MapperError {}` bound to the local `MapperError::Error` VARIANT. The
|
||||
* supertype is out-of-repo and simply unresolvable; a failed ref is correct.
|
||||
*/
|
||||
export const SUPERTYPE_TARGET_KINDS = new Set<Node['kind']>([
|
||||
'class', 'struct', 'interface', 'trait', 'protocol', 'enum', 'union',
|
||||
'type_alias', 'component', 'module', 'namespace',
|
||||
]);
|
||||
|
||||
/** True for the reference kinds that assert an inheritance/conformance relation. */
|
||||
export function isInheritanceRef(ref: UnresolvedRef): boolean {
|
||||
return ref.referenceKind === 'extends' || ref.referenceKind === 'implements';
|
||||
}
|
||||
|
||||
/**
|
||||
* Node kinds an `imports` edge may never TARGET: members that only exist
|
||||
* INSIDE a type. No language lets you import a class's property, an
|
||||
* interface's method or an enum's variant — you import the type that
|
||||
* contains it. The name-matcher has no kind filter, so a bare
|
||||
* `import path from 'node:path'` (unresolvable, since the module is external)
|
||||
* name-matched an interface property called `path` in an unrelated file.
|
||||
*/
|
||||
const NON_IMPORTABLE_KINDS = new Set<Node['kind']>([
|
||||
'property', 'field', 'method', 'enum_member', 'parameter',
|
||||
]);
|
||||
|
||||
/** Can an `imports` reference legally resolve to this node kind? */
|
||||
export function isImportableKind(kind: Node['kind']): boolean {
|
||||
return !NON_IMPORTABLE_KINDS.has(kind);
|
||||
}
|
||||
|
||||
Reference in New Issue
Block a user