diff --git a/CHANGELOG.md b/CHANGELOG.md index 942a9ae63..4ec3ae232 100644 --- a/CHANGELOG.md +++ b/CHANGELOG.md @@ -194,6 +194,7 @@ and adheres to [Semantic Versioning](https://semver.org/spec/v2.0.0.html). - In Go, a type defined over a slice, map, function or basic type now counts as implementing the interfaces its methods satisfy, just as a struct does: prometheus's `type staticDiscoverer []*targetgroup.Group` is a `Discoverer`, gin's `type formSource map[string][]string` a `setter`, etcd's `type Int64Comparable int64` a `Comparable`, and an adapter like `type HandlerFunc func(…)` a `Handler`. Before, only structs were considered, so these types were missing from an interface's implementations in `codegraph_explore`, impact and the type hierarchy, and a call through the interface, like `Discoverer.Run` or `setter.TrySet`, stopped there instead of reaching their methods. Re-index Go projects after upgrading. - In Go, a type alias like `type Event = mvccpb.Event`, alone or inside a `type ( … )` group, is now indexed and links to the type it names, so impact on that type reaches the code that uses the alias. Before, aliases were missing: a use written through the alias's package, like `clientv3.Event` or prometheus's `storage.AOptions{…}`, linked to nothing, and one inside the package, like `&PrometheusVersion{}` in prometheus's `web` package, linked to whichever type of that name the project had, there a test helper's struct. A method called on a value of an alias's type now reaches the aliased type's method rather than a namesake's, and a generic alias like `type Items[T any] = List[T]` links to `List`, so `xs.Len()` on one reaches `List.Len` instead of nothing. Re-index Go projects after upgrading. - In Go, a struct that embeds another is no longer treated as overriding the methods it redeclares. gin's `Engine` embeds `RouterGroup` and has a `Use` of its own, but calling `Use` on a `RouterGroup` only ever runs `RouterGroup.Use`; before, `codegraph_explore`'s call path and impact went on from it into `Engine.Use` and the code only the engine runs. The same held for every struct or defined type embedded in another that redeclares one of its methods, like etcd's `Mutex` inside `lockerMutex`. A call through an interface still reaches each type that implements it, the embedding struct included. Re-index Go projects after upgrading. +- In JavaScript and TypeScript, a subclass's `static` method no longer counts as overriding an instance method of the same name in its base class, or the reverse, since calling one never runs the other. In webpack, `codegraph_explore`'s call path and impact went on from `Module`'s `deserialize(context)` into the `static deserialize(context)` of `NormalModule` and other modules. A static method still overrides its base's static method and an instance method its base's instance method, and JavaScript class members are now marked static, as TypeScript ones already were. Re-index JavaScript and TypeScript projects after upgrading. Thanks @inth3shadows for the report and @yang0228 for the pull request. (#2761) - In Go, a call through a struct field that holds a function, like prometheus's `d.refreshf(ctx)`, now reaches the functions put in that field, so `codegraph_explore` follows service discovery from `Discovery.Run` into each provider's `refresh` and names the line where each provider wires its method in, like `RefreshF: d.refresh`. The field can be set in a struct literal or by an assignment, copied from another struct's field (`refreshf: opts.RefreshF`), or given a method of an interface value, and two unrelated structs with a field of the same name, like `fn` or `handler`, stay apart. Before, such a flow stopped at the field. Re-index Go projects after upgrading. - In Go, a type defined from other types, like etcd's `type WatchChan <-chan WatchResponse` or gin's `type HandlerFunc func(*Context)` and `type HandlersChain []HandlerFunc`, now links to the types it is built from, so impact on `WatchResponse` or gin's `Context` reaches these types and the code that uses them. Before, such a type linked to nothing. One written through another package, like etcd's `type PutResponse pb.PutResponse`, links to that package's type, and a recursive one like `type stateFn func(*Lexer) stateFn` doesn't link to itself. Re-index Go projects after upgrading. - In Go, a struct now links to the types its fields are declared with, like etcd's `PutResponse` holding a `*ResponseHeader` or gin's `Context` holding a `HandlersChain`, so impact on `ResponseHeader` or `HandlersChain` reaches every struct that holds one and the code that uses those structs, and `codegraph_node` lists a struct's field types. Before, a struct linked only to the types it embeds. A field written through another package, like `*pb.ResponseHeader`, links to that package's type, one from outside your project, like `context.Context`, links to nothing, the types an embedded generic is filled in with, like the `Pod` in client-go's `listers.ResourceIndexer[*v1.Pod]`, link too, and a struct that holds itself, like a list node's `next *node`, doesn't link to itself. `codegraph_explore` still answers a question about a flow with the code that runs it, not with the structs that merely hold one of its types. Re-index Go projects after upgrading. diff --git a/__tests__/js-static-override-dispatch.test.ts b/__tests__/js-static-override-dispatch.test.ts new file mode 100644 index 000000000..6c7d9e5c4 --- /dev/null +++ b/__tests__/js-static-override-dispatch.test.ts @@ -0,0 +1,179 @@ +/** + * The override-dispatch edges (`synthesizedBy: interface-impl`) linked a base + * class's method to a subclass's method of the same name even when one was + * `static` and the other not (#2761). In JavaScript and TypeScript a static + * method is inherited and overridden by the subclass's static method, an + * instance method by an instance method, and a call to one never runs the + * other. JavaScript members also never recorded `static` at all, in the + * kernel or the wasm extractor, so the edges couldn't tell them apart there. + */ +import { describe, it, expect, beforeAll, beforeEach, afterEach } from 'vitest'; +import * as fs from 'fs'; +import * as os from 'os'; +import * as path from 'path'; +import { CodeGraph } from '../src'; +import { extractFromSource } from '../src/extraction'; +import { initGrammars, loadGrammarsForLanguages } from '../src/extraction/grammars'; +import { tryKernelExtract, resetKernelForTests } from '../src/extraction/kernel'; +import type { ExtractionResult, Language } from '../src/types'; + +const KERNEL_PATH = path.join( + __dirname, + '..', + 'codegraph-kernel', + 'prebuilds', + `${process.platform}-${process.arch}`, + 'codegraph-kernel.node' +); +const kernelAvailable = fs.existsSync(KERNEL_PATH) || process.env.CODEGRAPH_KERNEL_EXPECT === '1'; + +const MEMBERS = `export class Flags { + static create() {} + run() {} + static handler = () => {}; + callback = () => {}; + static count = 0; + value = 0; + static async *stream() {} + static get size() { return 1; } + static() {} +} +`; + +const ENV_KEYS = ['CODEGRAPH_KERNEL', 'CODEGRAPH_KERNEL_LANGS'] as const; + +describe('a JS/TS class member records whether it is static (#2761)', () => { + let savedEnv: Record = {}; + + beforeAll(async () => { + await initGrammars(); + await loadGrammarsForLanguages(['typescript', 'tsx', 'javascript', 'jsx']); + }); + + beforeEach(() => { + savedEnv = Object.fromEntries(ENV_KEYS.map((k) => [k, process.env[k]])); + resetKernelForTests(); + }); + + afterEach(() => { + for (const k of ENV_KEYS) { + if (savedEnv[k] === undefined) delete process.env[k]; + else process.env[k] = savedEnv[k]; + } + resetKernelForTests(); + }); + + function extract(backend: 'kernel' | 'wasm', file: string, source: string, language: Language): ExtractionResult { + if (backend === 'wasm') { + process.env.CODEGRAPH_KERNEL = '0'; + return extractFromSource(file, source, language); + } + delete process.env.CODEGRAPH_KERNEL; + process.env.CODEGRAPH_KERNEL_LANGS = 'all'; + const result = tryKernelExtract(file, source, language); + expect(result, `kernel extraction of ${file}`).not.toBeNull(); + return result!; + } + + const backends = kernelAvailable ? (['kernel', 'wasm'] as const) : (['wasm'] as const); + const variants = [['ts', 'typescript'], ['tsx', 'tsx'], ['js', 'javascript'], ['jsx', 'jsx']] as const; + + for (const backend of backends) { + it.each(variants)(`in %s, ${backend}`, (ext, language) => { + const members = extract(backend, `src/flags.${ext}`, MEMBERS, language).nodes + .filter((n) => n.kind === 'method' || n.kind === 'property') + .map((n) => `${n.name} ${n.kind} ${n.isStatic ? 'static' : 'instance'}`); + expect(members).toEqual([ + 'create method static', 'run method instance', 'handler method static', 'callback method instance', + 'count property static', 'value property instance', 'stream method static', 'size method static', + // A member named `static` is an instance method. + 'static method instance', + ]); + }); + } + + it.runIf(kernelAvailable)('the kernel and the wasm extractor agree on every node', () => { + for (const [ext, language] of variants) { + const canon = (r: ExtractionResult) => r.nodes.map(({ updatedAt: _u, ...n }) => JSON.stringify(n, Object.keys(n).sort())).sort(); + expect(canon(extract('kernel', `src/flags.${ext}`, MEMBERS, language)), ext) + .toEqual(canon(extract('wasm', `src/flags.${ext}`, MEMBERS, language))); + } + }); +}); + +describe('an override pairs a static method with a static one only (#2761)', () => { + let root = ''; + let graph: CodeGraph | null = null; + + afterEach(() => { + graph?.close(); + graph = null; + if (root) fs.rmSync(root, { recursive: true, force: true }); + root = ''; + }); + + /** `base -> override` for every interface-impl edge, by qualified name and kind of method. */ + function overrides(g: CodeGraph, file: string): string[] { + const out: string[] = []; + const label = (id: string) => { + const n = g.getNode(id)!; + return `${n.isStatic ? 'static ' : ''}${n.qualifiedName}`; + }; + for (const node of g.getNodesInFile(file)) { + for (const e of g.getOutgoingEdges(node.id)) { + if (e.kind === 'calls' && e.metadata?.synthesizedBy === 'interface-impl') out.push(`${label(e.source)} -> ${label(e.target)}`); + } + } + return out.sort(); + } + + it('in TypeScript and JavaScript, for methods and callable fields', async () => { + root = fs.mkdtempSync(path.join(os.tmpdir(), 'cg-static-override-')); + const classes = [ + // The issue's reproduction, and the other direction. + 'class B { s() {} }', + 'class A extends B { static s() {} }', + 'class StaticBase { static t() {} }', + 'class InstanceDerived extends StaticBase { t() {} }', + // A static overriding a static, an instance method an instance one. + 'class SB { static make() {} }', + 'class SD extends SB { static make() {} }', + 'class IB { go() {} }', + 'class ID extends IB { go() {} }', + // A base with both: each pairs with its own kind. + 'class Mixed { static m() {} m() {} }', + 'class StaticOnly extends Mixed { static m() {} }', + 'class InstanceOnly extends Mixed { m() {} }', + // A subclass with both, like webpack's modules with a `static + // deserialize()` beside the instance one: only that one overrides. + 'class Both extends IB { static go() {} go() {} }', + // Callable class fields. + 'class FieldBase { run = () => {}; static start = () => {}; }', + 'class FieldDerived extends FieldBase { static run = () => {}; start = () => {}; }', + '', + ].join('\n'); + fs.writeFileSync(path.join(root, 'package.json'), JSON.stringify({ name: 'app' })); + fs.mkdirSync(path.join(root, 'src')); + fs.writeFileSync(path.join(root, 'src/classes.ts'), classes); + fs.writeFileSync(path.join(root, 'src/classes.js'), classes); + fs.writeFileSync( + path.join(root, 'src/shapes.ts'), + 'export interface Shape { area(): number }\nexport class Square implements Shape { static area(): number { return 0; } }\nexport class Circle implements Shape { area(): number { return 1; } }\n', + ); + // Java, whose override edges this leaves as they were. + fs.writeFileSync(path.join(root, 'src/Base.java'), 'class Base { static void s() {} void r() {} }\nclass Derived extends Base { static void s() {} void r() {} }\n'); + graph = await CodeGraph.init(root, { index: true }); + + const expected = [ + 'IB::go -> Both::go', + 'IB::go -> ID::go', + 'Mixed::m -> InstanceOnly::m', + 'static Mixed::m -> static StaticOnly::m', + 'static SB::make -> static SD::make', + ]; + expect(overrides(graph, 'src/classes.ts')).toEqual(expected); + expect(overrides(graph, 'src/classes.js')).toEqual(expected); + expect(overrides(graph, 'src/shapes.ts')).toEqual(['Shape::area -> Circle::area']); + expect(overrides(graph, 'src/Base.java')).toEqual(['Base::r -> Derived::r', 'static Base::s -> static Derived::s']); + }, 120_000); +}); diff --git a/codegraph-kernel/src/tsjs/mod.rs b/codegraph-kernel/src/tsjs/mod.rs index 69860a015..12eea4be1 100644 --- a/codegraph-kernel/src/tsjs/mod.rs +++ b/codegraph-kernel/src/tsjs/mod.rs @@ -42,7 +42,7 @@ impl Variant { } } /// TS-family (typescript/tsx): type annotations, interfaces, enums, - /// aliases, visibility, isStatic. The JS family lacks all of those hooks. + /// aliases, visibility. The JS family lacks all of those hooks. fn is_ts(self) -> bool { matches!(self, Variant::Typescript | Variant::Tsx) } @@ -1072,13 +1072,10 @@ impl<'t> Walker<'t> { self.has_keyword_child(node, "async") } - /// TS has an isStatic hook; JS does not (None = field absent). + /// A member's own `static` keyword: the isStatic hook TS and JS share + /// (hasStaticModifier, #2761). fn is_static(&self, node: Node) -> Option { - if self.variant.is_ts() { - Some(self.has_keyword_child(node, "static")) - } else { - None - } + Some(self.has_keyword_child(node, "static")) } fn is_const_decl(&self, node: Node) -> bool { diff --git a/src/extraction/languages/javascript.ts b/src/extraction/languages/javascript.ts index 899e1530a..b96956e46 100644 --- a/src/extraction/languages/javascript.ts +++ b/src/extraction/languages/javascript.ts @@ -1,6 +1,6 @@ import { getNodeText, getChildByField } from '../tree-sitter-helpers'; import type { LanguageExtractor } from '../tree-sitter-types'; -import { classifyTsClassMember } from './typescript'; +import { classifyTsClassMember, hasStaticModifier } from './typescript'; export const javascriptExtractor: LanguageExtractor = { functionTypes: ['function_declaration', 'generator_function_declaration', 'arrow_function', 'function_expression', 'generator_function'], @@ -77,6 +77,9 @@ export const javascriptExtractor: LanguageExtractor = { } return false; }, + // A `static` method or field, as in TypeScript (#2761): without it every JS + // class member read as an instance one. + isStatic: hasStaticModifier, isConst: (node) => { if (node.type === 'lexical_declaration') { for (let i = 0; i < node.childCount; i++) { diff --git a/src/extraction/languages/typescript.ts b/src/extraction/languages/typescript.ts index eae4cc8df..40f5a6465 100644 --- a/src/extraction/languages/typescript.ts +++ b/src/extraction/languages/typescript.ts @@ -3,6 +3,18 @@ import type { LanguageExtractor } from '../tree-sitter-types'; import { hasFlowPragma } from '../grammars'; import type { Node as SyntaxNode } from 'web-tree-sitter'; +/** + * Whether a class member is written `static`: its own `static` keyword, never + * a member named `static()`. Shared by the TypeScript and JavaScript + * extractors, which parse class bodies alike. + */ +export function hasStaticModifier(node: SyntaxNode): boolean { + for (let i = 0; i < node.childCount; i++) { + if (node.child(i)?.type === 'static') return true; + } + return false; +} + /** * A TS/JS class field (`public_field_definition` / `field_definition`) is a * METHOD only when its value is callable — an arrow function, a function @@ -230,13 +242,7 @@ export const typescriptExtractor: LanguageExtractor = { } return false; }, - isStatic: (node) => { - for (let i = 0; i < node.childCount; i++) { - const child = node.child(i); - if (child?.type === 'static') return true; - } - return false; - }, + isStatic: hasStaticModifier, isConst: (node) => { // For lexical_declaration, check if it's 'const' or 'let' // For variable_declaration, it's always 'var' diff --git a/src/resolution/callback-synthesizer.ts b/src/resolution/callback-synthesizer.ts index 4c2755688..4727e6d00 100644 --- a/src/resolution/callback-synthesizer.ts +++ b/src/resolution/callback-synthesizer.ts @@ -822,6 +822,13 @@ const IFACE_OVERRIDE_LANGS = new Set([ 'java', 'kotlin', 'csharp', 'typescript', 'javascript', 'swift', 'scala', 'go', 'rust', 'arkts', ]); +/** + * Languages whose static and instance methods override only their own kind: + * a JS/TS class's statics are inherited and overridden by the subclass's + * statics, its instance methods by instance methods (#2761). tsx and jsx + * follow the same rule, though this pass doesn't walk their classes today. + */ +const STATIC_SPLIT_LANGS: ReadonlySet = new Set(['typescript', 'tsx', 'javascript', 'jsx']); /** * Whether a supertype edge out of a Go type is one of its embeddings (#2397): * declared, never synthesized (go-implements writes those), into a struct, an @@ -1671,6 +1678,11 @@ async function interfaceOverrideEdges(queries: QueryBuilder, onYield: MaybeYield for (const m of impls) { if (added >= MAX_CALLBACKS_PER_CHANNEL) break; if (bm.id === m.id) continue; + // A JS/TS static method overrides only a static one, and an + // instance method only an instance one (#2761): B's `s()` and the + // `static s()` of `class A extends B` are unrelated, and a call to + // one never runs the other. + if (STATIC_SPLIT_LANGS.has(m.language) && (bm.isStatic === true) !== (m.isStatic === true)) continue; const key = `${bm.id}>${m.id}`; if (seen.has(key)) continue; seen.add(key);