From 0abaef2f6e4f959e4da9fae454f37fa75f561f7c Mon Sep 17 00:00:00 2001 From: jiangbx Date: Fri, 28 Aug 2026 05:39:00 -0400 Subject: [PATCH] fix(go): resolve a package-qualified accessor chain on the factory's return type instead of a same-named method MIME-Version: 1.0 Content-Type: text/plain; charset=UTF-8 Content-Transfer-Encoding: 8bit `pkg.Factory().Method()` reached the resolver as a bare `Method` ref, so the name fallbacks matched it against any same-named method on an unrelated type — and, when the enclosing method shared the name, against itself (the #1496 shape). On hotgo, `service.SysTable().List(ctx, …)` inside `func (c *cTable) List(…)` resolved to `cTable::List`: a self-edge. Extraction re-encoded a chained receiver for Go only when the inner callee was a bare `identifier` (`New().Method()`). A package-qualified factory is a `selector_expression` — the same node type as an instance chain (`obj.Method().Other()`), whose receiver type is not recoverable and which must stay bare — so both were dropped. The file's import set separates them: re-encode only when the selector's operand names an imported package. Resolution then has to read the factory's declared return type, but Go package-level functions carry a bare qualifiedName (`Order`, not `service.Order`), so the `Class::method` lookup used by the dot-notation languages never matched. Look the factory up by name, map the call-site qualifier back through the file's imports (an alias does not name its directory), disambiguate by the import path's tail, and validate the method on the inferred type through `resolveMethodOnType`. Candidates that disagree on their return type yield no edge rather than a guess. An interface return lands on the interface's method, which the dynamic-dispatch pass already bridges to the implementation, closing route -> controller -> service -> implementation. Validated on the three public GoFrame repos from the coverage playbook, baseline vs. this build. Node count is identical on all three and the `route` edges from #747 are untouched: repo files nodes edges ctrl->svc->logic route edges gf-demo-user 38 248 474 -> 474 0 -> 7 7 -> 7 gfast 184 1,875 4,260 -> 4,236 0 -> 82 65 -> 65 hotgo/server 697 9,592 21,739 -> 21,609 0 -> 327 243 -> 243 Every removed edge sampled was wrong: controller self-edges as above, calls matched to a same-named TYPE (`service.SysTable().View(…)` -> `model.View`), and chains through a dependency's accessor (`gjson.New(req).String()` -> an unrelated `apiItem::String`, `g.Redis().Do(…)` -> a project function named `Do`). Those now resolve to nothing, which is the correct answer. --- CHANGELOG.md | 4 + __tests__/go-package-accessor-chain.test.ts | 265 ++++++++++++++++++++ src/extraction/tree-sitter.ts | 62 ++++- src/resolution/name-matcher.ts | 64 +++++ 4 files changed, 394 insertions(+), 1 deletion(-) create mode 100644 __tests__/go-package-accessor-chain.test.ts diff --git a/CHANGELOG.md b/CHANGELOG.md index 3ca5b9ff1..a08b83809 100644 --- a/CHANGELOG.md +++ b/CHANGELOG.md @@ -12,6 +12,10 @@ and adheres to [Semantic Versioning](https://semver.org/spec/v2.0.0.html). ## [Unreleased] +### Fixes + +- Go calls made through a package-level accessor — `service.Order().Create(...)`, the shape `gf gen service` generates for every GoFrame project — now resolve to the method the accessor's return type actually declares. Previously the chain was dropped while reading the code and only the method name was matched, so any same-named method on an unrelated type could win. In a layered Go app, where the controller, the service interface, and the implementation deliberately share one method name, the call landed on the controller: the path from a route down to the business logic was never joined up, and blast radius credited the implementation's tests to the wrong function. An accessor returning an interface now lands on the interface's method, which the dynamic-dispatch pass already bridges to the implementation. Instance chains (`obj.Method().Other()`), whose receiver type isn't recoverable, keep the name-based behaviour they had. + ## [1.6.0] - 2026-08-26 diff --git a/__tests__/go-package-accessor-chain.test.ts b/__tests__/go-package-accessor-chain.test.ts new file mode 100644 index 000000000..de0801dde --- /dev/null +++ b/__tests__/go-package-accessor-chain.test.ts @@ -0,0 +1,265 @@ +/** + * Go package-qualified accessor chains — `pkg.Factory().Method()`. + * + * The extractor re-encodes a chained receiver as `().` so + * resolution can infer the method's type from what the inner call RETURNS + * (the #645/#608 mechanism). Go used to re-encode only a BARE inner callee + * (`New().Method()`, an `identifier`), which left the package-qualified form + * `service.Order().Method()` — the `gf gen service` accessor every GoFrame app + * has — emitting a bare `Method` ref that the name fallbacks then matched + * against ANY same-named method on an unrelated type. + * + * A package-qualified inner callee is a `selector_expression`, the same node + * type as an instance chain (`obj.Method().Other()`), whose receiver type is not + * recoverable and which must therefore stay bare — re-encoding it would drop the + * edge instead. The file's import set separates the two, so the guards below pin + * both directions. + * + * Resolution then reads the factory's declared return type. Go package-level + * functions carry a BARE qualifiedName (`Order`, not `service.Order`), so the + * `Class::method` lookup used by the dot-notation languages can never match; + * the factory is looked up by name and disambiguated by its package directory. + * The method is validated on the inferred type through `resolveMethodOnType`, so + * a type that lacks the method yields no edge rather than a decoy. An interface + * return lands on the INTERFACE's method, which the dynamic-dispatch pass then + * bridges to the implementation — closing caller -> interface -> impl. + */ +import { describe, it, expect, beforeEach, afterEach } from 'vitest'; +import * as fs from 'node:fs'; +import * as path from 'node:path'; +import * as os from 'node:os'; +import { CodeGraph } from '../src'; + +describe('Go package-qualified accessor chains', () => { + let dir: string; + beforeEach(() => { dir = fs.mkdtempSync(path.join(os.tmpdir(), 'go-pkg-accessor-')); }); + afterEach(() => { fs.rmSync(dir, { recursive: true, force: true }); }); + + const write = (rel: string, body: string) => { + const p = path.join(dir, rel); + fs.mkdirSync(path.dirname(p), { recursive: true }); + fs.writeFileSync(p, body); + }; + + const load = async () => { + const cg = await CodeGraph.init(dir, { silent: true }); + await cg.indexAll(); + const db = (cg as any).db.db; + const calls: { src: string; tgt: string; tgtQn: string }[] = db + .prepare( + `SELECT s.name src, t.name tgt, t.qualified_name tgtQn + FROM edges e JOIN nodes s ON s.id = e.source JOIN nodes t ON t.id = e.target + WHERE e.kind = 'calls'`, + ) + .all(); + cg.close?.(); + return calls; + }; + const hasCall = (calls: any[], src: string, tgtQn: string) => + calls.some((e) => e.src === src && e.tgtQn === tgtQn); + /** Any resolved call `src` makes to something of the given bare name. */ + const callsNamed = (calls: any[], src: string, tgt: string) => + calls.some((e) => e.src === src && e.tgt === tgt); + + /** `svc` exposes an interface accessor; `impl` implements it; `ctrl` is a decoy. */ + const svc = `package svc + +type IAlpha interface{ Handle() string } + +var localAlpha IAlpha + +func RegisterAlpha(i IAlpha) { localAlpha = i } +func Alpha() IAlpha { return localAlpha } +`; + const impl = `package impl + +type SAlpha struct{} + +func (s *SAlpha) Handle() string { return "real" } +`; + // Same method name on an unrelated type, with a signature that does NOT + // satisfy IAlpha — so only name matching could ever reach it. + const decoy = `package ctrl + +type Req struct{ N int } + +type Ctrl struct{} + +func (c *Ctrl) Handle(req *Req) string { return "decoy" } +`; + + it('resolves an interface-returning accessor to the interface method, not a same-named decoy', async () => { + write('go.mod', 'module repro\n\ngo 1.25\n'); + write('svc/svc.go', svc); + write('impl/impl.go', impl); + write('ctrl/ctrl.go', decoy); + write('caller/caller.go', `package caller + +import "repro/svc" + +func RunA() string { return svc.Alpha().Handle() } +`); + const calls = await load(); + expect(hasCall(calls, 'RunA', 'IAlpha::Handle')).toBe(true); + expect(hasCall(calls, 'RunA', 'Ctrl::Handle')).toBe(false); + }); + + it('reaches the implementation through the interface (caller -> interface -> impl)', async () => { + write('go.mod', 'module repro\n\ngo 1.25\n'); + write('svc/svc.go', svc); + write('impl/impl.go', impl); + write('caller/caller.go', `package caller + +import "repro/svc" + +func RunA() string { return svc.Alpha().Handle() } +`); + const calls = await load(); + expect(hasCall(calls, 'RunA', 'IAlpha::Handle')).toBe(true); + expect(hasCall(calls, 'Handle', 'SAlpha::Handle')).toBe(true); + }); + + it('resolves a concrete-returning accessor to the returned type, not a same-named decoy', async () => { + write('go.mod', 'module repro\n\ngo 1.25\n'); + write('impl/impl.go', `package impl + +type SGamma struct{} + +func (s *SGamma) Execute() string { return "real" } +`); + write('fac/fac.go', `package fac + +import "repro/impl" + +func Gamma() *impl.SGamma { return &impl.SGamma{} } +`); + write('ctrl/ctrl.go', `package ctrl + +type Req struct{ N int } + +type Ctrl struct{} + +func (c *Ctrl) Execute(req *Req) string { return "decoy" } +`); + write('caller/caller.go', `package caller + +import "repro/fac" + +func RunC() string { return fac.Gamma().Execute() } +`); + const calls = await load(); + expect(hasCall(calls, 'RunC', 'SGamma::Execute')).toBe(true); + expect(hasCall(calls, 'RunC', 'Ctrl::Execute')).toBe(false); + }); + + it('honours an import alias as the package qualifier', async () => { + write('go.mod', 'module repro\n\ngo 1.25\n'); + write('svc/svc.go', svc); + write('impl/impl.go', impl); + write('ctrl/ctrl.go', decoy); + write('caller/caller.go', `package caller + +import svcx "repro/svc" + +func RunAlias() string { return svcx.Alpha().Handle() } +`); + const calls = await load(); + expect(hasCall(calls, 'RunAlias', 'IAlpha::Handle')).toBe(true); + expect(hasCall(calls, 'RunAlias', 'Ctrl::Handle')).toBe(false); + }); + + it('leaves an INSTANCE chain on the bare-name path — a variable receiver is not a package', async () => { + // `b.Inner().Value()` shares the `selector_expression` inner-callee shape + // with a package-qualified accessor. Re-encoding it would strip the edge + // (a variable's type is not recoverable here), so it must stay bare. + write('go.mod', 'module repro\n\ngo 1.25\n'); + write('box/box.go', `package box + +type Box struct{} + +func (b *Box) Inner() *Box { return b } +func (b *Box) Value() string { return "v" } + +func UseInstance() string { + var b Box + return b.Inner().Value() +} +`); + const calls = await load(); + expect(hasCall(calls, 'UseInstance', 'Box::Value')).toBe(true); + }); + + it('picks the aliased package the import path names, not another with the same function', async () => { + // Two packages export `Build()` returning different interfaces. The call site + // says `nope.Build()`, a name no directory carries — only the import path + // maps it back to `one`. + write('go.mod', 'module repro\n\ngo 1.25\n'); + write('one/one.go', `package one + +type IOne interface{ Run() string } + +func Build() IOne { var x IOne; return x } +`); + write('two/two.go', `package two + +type ITwo interface{ Run() string } + +func Build() ITwo { var x ITwo; return x } +`); + write('caller/caller.go', `package caller + +import nope "repro/one" + +func RunAliased() string { return nope.Build().Run() } +`); + const calls = await load(); + expect(hasCall(calls, 'RunAliased', 'IOne::Run')).toBe(true); + expect(hasCall(calls, 'RunAliased', 'ITwo::Run')).toBe(false); + }); + + it('makes no edge when the inferred type does not declare the method', async () => { + // Absent-method safety. The fixture is deliberately not type-correct — any + // call to a method the receiver lacks is a Go compile error, and extraction + // is syntactic — but it is the shape a stale or mistaken inference produces, + // and a same-named decoy must not be matched instead. + write('go.mod', 'module repro\n\ngo 1.25\n'); + write('svc/svc.go', svc); + write('impl/impl.go', impl); + write('ctrl/ctrl.go', `package ctrl + +type Ctrl struct{} + +func (c *Ctrl) Missing() string { return "decoy" } +`); + write('caller/caller.go', `package caller + +import "repro/svc" + +func RunMissing() string { return svc.Alpha().Missing() } +`); + const calls = await load(); + expect(callsNamed(calls, 'RunMissing', 'Missing')).toBe(false); + }); + + it('makes no edge when the factory belongs to a package outside the index', async () => { + // `g.Redis().Do(...)` — the accessor and its type live in a dependency, so + // nothing about the receiver is knowable. Previously the bare `Do` matched a + // project function of that name; it must now resolve to nothing. + write('go.mod', 'module repro\n\ngo 1.25\n'); + write('carrier/carrier.go', `package carrier + +func Do() string { return "unrelated project function" } +`); + write('caller/caller.go', `package caller + +import "github.com/gogf/gf/v2/frame/g" + +func RunExternal() string { + v, _ := g.Redis().Do(nil, "GET", "k") + return v.String() +} +`); + const calls = await load(); + expect(callsNamed(calls, 'RunExternal', 'Do')).toBe(false); + }); +}); diff --git a/src/extraction/tree-sitter.ts b/src/extraction/tree-sitter.ts index c34dc4716..c6ed1f2b7 100644 --- a/src/extraction/tree-sitter.ts +++ b/src/extraction/tree-sitter.ts @@ -396,6 +396,51 @@ export class TreeSitterExtractor { private nodes: Node[] = []; private edges: Edge[] = []; private unresolvedReferences: UnresolvedReference[] = []; + + /** + * Go: the package identifiers this file imports — the alias when one is given, + * otherwise the import path's last segment. Memoized; the extractor instance is + * per-file. Used to tell a package-qualified factory chain from an instance chain, + * which share the `selector_expression` inner-callee shape. + */ + private goImportedPkgsMemo: Set | null = null; + private goImportedPackages(from: SyntaxNode): Set { + if (this.goImportedPkgsMemo) return this.goImportedPkgsMemo; + const pkgs = new Set(); + let root: SyntaxNode = from; + while (root.parent) root = root.parent; + const walk = (n: SyntaxNode): void => { + if (n.type === 'import_spec') { + const alias = n.namedChildren.find( + (c) => c.type === 'package_identifier' || c.type === 'identifier', + ); + if (alias) { + pkgs.add(getNodeText(alias, this.source)); + return; + } + const path = n.namedChildren.find( + (c) => c.type === 'interpreted_string_literal' || c.type === 'raw_string_literal', + ); + if (path) { + const last = getNodeText(path, this.source).replace(/['"`]/g, '').split('/').pop(); + if (last) pkgs.add(last); + } + return; + } + for (const c of n.namedChildren) { + if ( + n.type === 'source_file' || + n.type === 'import_declaration' || + n.type === 'import_spec_list' + ) { + walk(c); + } + } + }; + walk(root); + this.goImportedPkgsMemo = pkgs; + return pkgs; + } // Value-reference edges (default ON; set CODEGRAPH_VALUE_REFS=0 to disable; see flushValueRefs). // Same-file reads of file-scope const/var symbols → `references` edges so impact analysis catches // value consumers ("change this constant/table, affect its readers"). @@ -4520,7 +4565,22 @@ export class TreeSitterExtractor { // the resolver can't recover a variable's type, so re-encoding would // only drop the edge. C/C++ re-encode any inner. if (this.language === 'rust') reencode = innerFn?.type === 'scoped_identifier'; - else if (this.language === 'go') reencode = innerFn?.type === 'identifier'; + else if (this.language === 'go') { + // Bare package-level factory (`New().Method()`): inner callee is an + // `identifier`. Package-qualified factory (`service.Order().Method()` — + // the `gf gen service` accessor every GoFrame app has): inner callee is + // a `selector_expression`, the SAME node type as an instance chain + // (`obj.Method().Other()`), which must stay bare because a variable's + // type isn't recoverable here. The file's import set separates the two. + if (innerFn?.type === 'identifier') reencode = true; + else if (innerFn?.type === 'selector_expression') { + const operand = getChildByField(innerFn, 'operand'); + reencode = + !!operand && + operand.type === 'identifier' && + this.goImportedPackages(innerFn).has(getNodeText(operand, this.source)); + } else reencode = false; + } // Scala: only a companion-factory / case-class-apply chain whose // receiver chain starts with a capitalized type (`Foo.create().bar()`, // `Foo(args).bar()`). An instance chain (`list.map().filter()`) has a diff --git a/src/resolution/name-matcher.ts b/src/resolution/name-matcher.ts index c74d8f272..faa357b9f 100644 --- a/src/resolution/name-matcher.ts +++ b/src/resolution/name-matcher.ts @@ -914,6 +914,54 @@ function lookupCalleeReturnType( return candidates.find((n) => n.kind === 'function')?.returnType ?? null; } +/** + * Go: the declared return type of a package-qualified factory — `pkg.Factory()`. + * + * Go package-level functions are indexed with a BARE qualifiedName (`Order`, not + * `service.Order`), so the `Class::method` lookup that serves the dot-notation + * languages can never match `service::Order`. A dotted prefix at a Go call site + * is a PACKAGE qualifier, not a receiver type (Go has no `Class.staticMethod()` + * form), so resolve it as one: among the package-level functions sharing the + * factory's name, prefer those declared in the qualifying package's directory. + * Ambiguity — several plausible candidates disagreeing on their return type — + * yields null, so a guess produces no edge rather than a wrong one (#750). + */ +function lookupGoPackageFuncReturnType( + pkg: string, + funcName: string, + ref: UnresolvedRef, + context: ResolutionContext, +): string | null { + const candidates = context + .getNodesByName(funcName) + .filter((n) => n.kind === 'function' && n.language === 'go' && !!n.returnType); + if (candidates.length === 0) return null; + // `pkg` is the name at the CALL SITE, which an alias detaches from the + // directory (`ctrlcart "app/internal/controller/order/cart"`). Map it back + // through the file's imports; a plain import maps to itself. A package whose + // name differs from its directory (legal) is covered too — the import PATH is + // what's matched, never the package clause. + const importPath = + context.getImportMappings(ref.filePath, ref.language).find((i) => i.localName === pkg) + ?.source ?? pkg; + // Go requires one package per directory, so the import path's tail IS the + // declaring directory. Match the longest tail available — the candidate's full + // directory path — which separates same-named packages under different parents. + const byDir = candidates.filter((n) => { + const dir = goDirOf(n.filePath); + return dir.length > 0 && (importPath === dir || importPath.endsWith(`/${dir}`)); + }); + const pool = byDir.length > 0 ? byDir : candidates; + const types = new Set(pool.map((n) => n.returnType!)); + return types.size === 1 ? (pool[0]!.returnType ?? null) : null; +} + +/** The directory path a file lives in — the package scope for Go. */ +function goDirOf(filePath: string): string { + const i = filePath.lastIndexOf('/'); + return i > 0 ? filePath.slice(0, i) : ''; +} + /** Does the graph contain an aggregate type named `name`'s last segment? */ function cppClassExists(name: string, ref: UnresolvedRef, context: ResolutionContext): boolean { const last = cppLastSegment(name); @@ -1110,6 +1158,22 @@ export function matchDottedCallChain( return resolveMethodOnType(inner, method, ref, context, 0.85, 'instance-method', importedFqnOf(inner, ref, context)); } + // Go: `pkg.Factory().Method()`. The dotted prefix is a package qualifier, so + // the `Class::method` lookup below can never match it; resolve the factory as a + // package-level function instead and VALIDATE the method on its declared return + // type. An interface return (`func Order() IOrder` — the `gf gen service` + // accessor every GoFrame app has) lands on the interface's method, which the + // dynamic-dispatch pass already bridges to the implementation, closing the + // caller -> interface -> impl chain. Without this the ref falls through to the + // bare-name fallback, which matches any same-named method on an unrelated type. + if (ref.language === 'go') { + const pkgName = inner.slice(0, lastDot).split('.').pop()!; + const factoryFn = inner.slice(lastDot + 1); + const goRet = lookupGoPackageFuncReturnType(pkgName, factoryFn, ref, context); + if (!goRet) return null; + return resolveMethodOnType(goRet, method, ref, context, 0.85, 'instance-method', importedFqnOf(goRet, ref, context)); + } + // Factory/fluent receiver `Receiver.factory(args).method()`: the receiver's // type is what `Receiver.factory` returns (its declared return type). const factoryClass = inner.slice(0, lastDot).split('.').pop(); // simple class name