diff --git a/CHANGELOG.md b/CHANGELOG.md index fb52ae289..4169a4227 100644 --- a/CHANGELOG.md +++ b/CHANGELOG.md @@ -35,6 +35,7 @@ and adheres to [Semantic Versioning](https://semver.org/spec/v2.0.0.html). - In Dart, a type is now linked from more of the places it is written, not only from parameters and return types: the type an `extension … on` targets, a field's type, a top-level variable's type, generic arguments such as `Future.value(null)` or Riverpod's `final reportProvider = Family()`, and a local variable's type, a cast or a type check inside a function. So `callers` and impact for a model class now reach the extensions, models and providers that use it. Thanks @mg-mg-mg for the report. (#2327) - In Go, calls through a method receiver or parameter of an unexported type, the usual shape of gRPC and HTTP handlers (`s.service.AddItem()` inside `func (s *server) Create()`), now resolve, and always within that type's own package: another package's `server` with a same-named method no longer takes the call, and a method the type gets from a struct or interface it embeds is found too. Re-index Go projects after upgrading. Thanks @GoDiao for the report and the fix. (#2323) - In Go, calls into a module whose `go.mod` is not at the project root now resolve, whether it is a `server/` backend next to a `web/` frontend or one of several modules side by side as in etcd, so calls like `store.New()` and `s.db.CreateItem()` find their targets instead of being treated as calls into a third-party package. A name written through a package, like a `job.OPCommand` result type or a field of type `artifact.Manager`, now links to that package's symbol rather than a same-named one elsewhere, which also corrects links in projects with a single `go.mod`. Re-index Go projects after upgrading. Thanks @GoDiao for the report and @danusha2345 for the fix. (#2322) +- In Go, an exported type written without a package, like `Node` in `func Walk(v Visitor, node Node)`, now links to the type of that name in its own package, whether it is a struct, an interface or another kind of type. Before, it could link to a struct of the same name in an unrelated package, so prometheus's PromQL parser functions pointed at the Kubernetes discovery `Node` struct instead of the parser's own `Node` interface, or, for a result type like `Appender` in `func (f *fanout) Appender(…) Appender`, to the method itself. A type from a package outside your project, like `apiv1.Node` or `http.Handler`, no longer links to a struct of the same name in the file that uses it; a variadic parameter like `...storage.Filter` now links to that package's type; a route handler written as a method value, like `h.Follow` in an Echo or Gin app, links to that method rather than a model struct named `Follow`; and a method called on another call's result or on an indexed value, like `err[i].Error()` or a chained `.String()`, is no longer recorded as creating a struct of the same name. Re-index Go projects after upgrading. - Indexing a project that includes large bundled JavaScript files, such as a copy of pdf.js or d3, is fast again: since 1.6.2, resolving the calls in a JavaScript or TypeScript file re-read the file's text above each call, so a single bundled library could add many seconds to an index. The graph it builds is unchanged. Thanks @bompus for the report. (#2334) - The time `codegraph init` and `codegraph index` print beside the node and edge counts now covers the whole run, resolving references and linking included, and `codegraph sync` reports its whole run the same way. Before, it counted only reading and parsing the files, which can be a small part of an index, so a slow index looked fast. Thanks @bompus for the report. (#2334) - In Rust, code that uses an enum only through its variants, like `mode::Mode::A` in an expression, a `Mode::B =>` match arm or `Self::A` inside the enum's own `impl`, now shows up in that enum's callers and impact, linked to the enum that actually declares the variant rather than to a same-named type elsewhere. Standard-library variants like `Ordering::Less` or `Option::Some` and associated items like `Foo::new()` or `Foo::MAX` don't count as using a project enum. Re-index Rust projects after upgrading. Thanks @mg-mg-mg for the report and @danusha2345. (#2328) diff --git a/__tests__/go-framework-own-package.test.ts b/__tests__/go-framework-own-package.test.ts new file mode 100644 index 000000000..137fa5880 --- /dev/null +++ b/__tests__/go-framework-own-package.test.ts @@ -0,0 +1,203 @@ +/** + * The Go framework resolver's name heuristics ("a `…Handler` is a handler + * function", "a PascalCase name is a model struct") outrank name matching on a + * tie, so where they guess wrong nothing corrects them. Go reads a name from + * one package only — one written bare from the reference's own, `pkg.Name` + * from that import's, `x[i].Name()` from whatever type `x[i]` has — so they + * guess only for a bare name, and only in its own package. Before, they took + * a struct of that name from any package: prometheus's promql/parser `Node` + * parameters went to discovery/kubernetes's struct `Node` instead of the + * package's own `Node` interface, and every `.String()` called through an + * expression became an instantiation of the struct `promql.String`. + */ +import { describe, it, expect, afterAll, beforeAll } from 'vitest'; +import * as fs from 'fs'; +import * as os from 'os'; +import * as path from 'path'; +import { CodeGraph } from '../src'; + +let root = ''; +let cg: CodeGraph; + +beforeAll(async () => { + root = fs.mkdtempSync(path.join(os.tmpdir(), 'cg-go-own-pkg-')); + const files: Record = { + 'go.mod': 'module example.com/app\n\ngo 1.22\n', + 'promql/parser/ast.go': `package parser + +type Node interface { + String() string +} + +type Visitor interface { + Visit(node Node, path []Node) (Visitor, error) +} + +func Walk(v Visitor, node Node, path []Node) error { + return nil +} +`, + 'discovery/kubernetes/node.go': `package kubernetes + +import apiv1 "k8s.io/api/core/v1" + +type Node struct { + name string +} + +func nodeSource(n *apiv1.Node) string { + return n.Name +} +`, + 'storage/interface.go': `package storage + +type Appender interface { + Append(ref uint64) error +} + +type Filter interface { + Accept(value string) bool +} +`, + 'storage/fanout.go': `package storage + +type fanout struct{} + +func (f *fanout) Appender() Appender { + return nil +} +`, + 'discovery/aws/aws.go': `package aws + +type Filter struct { + Name string +} +`, + 'web/api/v1/search.go': `package v1 + +import "example.com/app/storage" + +type ChainFilter struct{} + +func NewChainFilter(filters ...storage.Filter) *ChainFilter { + return &ChainFilter{} +} +`, + 'errors.go': `package gin + +type Error struct { + Err error +} + +func (msg *Error) Error() string { + return msg.Err.Error() +} + +var _ error = (*Error)(nil) + +func newError(err error) *Error { + return &Error{Err: err} +} +`, + 'binding/validator.go': `package binding + +import "strings" + +type SliceValidationError []error + +func (err SliceValidationError) Error() string { + var b strings.Builder + for i := range err { + b.WriteString(err[i].Error()) + } + return b.String() +} +`, + 'handler/handler.go': `package handler + +type Handler struct{} + +func (h *Handler) Follow(c Context) error { + return nil +} + +type Context interface{} +`, + 'handler/routes.go': `package handler + +type Group interface { + POST(path string, h func(Context) error) +} + +func (h *Handler) Register(g Group) { + g.POST("/:username/follow", h.Follow) +} +`, + 'model/user.go': `package model + +type Follow struct { + FollowerID uint +} +`, + }; + 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); + } + cg = await CodeGraph.init(root, { index: true }); +}); + +afterAll(() => { + cg?.close(); + if (root) fs.rmSync(root, { recursive: true, force: true }); +}); + +/** + * `kind file:qualifiedName` of each edge out of `file`'s nodes (of one kind, + * or named `from`) whose target is named `name`, deduplicated and sorted. + */ +const linksFrom = (file: string, name: string, from?: { kind?: string; name?: string }) => { + const sources = cg.getNodesInFile(file).filter((n) => (!from?.kind || n.kind === from.kind) && (!from?.name || n.name === from.name)); + const links = cg.getOutgoingEdgesFrom(sources.map((n) => n.id)) + .filter((e) => e.kind !== 'contains') + .map((e) => ({ kind: e.kind, target: cg.getNode(e.target)! })) + .filter(({ target }) => target.name === name) + .map(({ kind, target }) => `${kind} ${target.filePath}:${target.qualifiedName}`); + return [...new Set(links)].sort(); +}; + +describe('Go framework heuristics pick only in the package Go reads the name from', () => { + it('a bare type is its own package’s interface, not another package’s struct', () => { + expect(linksFrom('promql/parser/ast.go', 'Node')).toEqual(['references promql/parser/ast.go:Node']); + }); + + it('a type written through an outside import is not the same-named struct of its own file', () => { + expect(linksFrom('discovery/kubernetes/node.go', 'Node')).toEqual([]); + }); + + it('a method called through an expression is not an instantiation of a same-named struct', () => { + const links = linksFrom('binding/validator.go', 'Error'); + expect(links.filter((l) => l.startsWith('instantiates'))).toEqual([]); + expect(links).not.toContain('calls errors.go:Error'); + }); + + it('a result type is its own package’s type, not the method the line declares', () => { + expect(linksFrom('storage/fanout.go', 'Appender')).toEqual(['references storage/interface.go:Appender']); + }); + + it('a variadic `...pkg.T` is that import’s type', () => { + expect(linksFrom('web/api/v1/search.go', 'Filter')).toEqual(['references storage/interface.go:Filter']); + }); + + it('a conversion and a composite literal still reach their own package’s struct', () => { + expect(linksFrom('errors.go', 'Error', { name: '_' })).toEqual(['instantiates errors.go:Error']); + expect(linksFrom('errors.go', 'Error', { name: 'newError' })).toEqual([ + 'instantiates errors.go:Error', + 'references errors.go:Error', + ]); + }); + + it('a route’s handler written through a value is that value’s method, not a model struct', () => { + expect(linksFrom('handler/routes.go', 'Follow', { kind: 'route' })).toEqual(['references handler/handler.go:Handler::Follow']); + }); +}); diff --git a/src/resolution/frameworks/go.ts b/src/resolution/frameworks/go.ts index b90d20dea..58453a9ee 100644 --- a/src/resolution/frameworks/go.ts +++ b/src/resolution/frameworks/go.ts @@ -7,6 +7,7 @@ import { Node } from '../../types'; import { FrameworkResolver, UnresolvedRef, ResolvedRef, ResolutionContext } from '../types'; import { stripCommentsForRegex } from '../strip-comments'; +import { GO_TYPE_KINDS, isGoBareName } from '../name-matcher'; import { pickByNameAndKind } from './name-heuristic'; export const goResolver: FrameworkResolver = { @@ -26,18 +27,24 @@ export const goResolver: FrameworkResolver = { }, resolve(ref: UnresolvedRef, context: ResolutionContext): ResolvedRef | null { - // An embedded type is the one its package declares under that name: the - // same package's for `Node`, the import's for `pkg.Node`, which name - // matching and the import resolver work out. The patterns below pick among - // every same-named node by folder convention, and Pattern 4 sees structs - // only: `Node` embedded beside promql/parser's `type Node interface` went - // to discovery/kubernetes's struct `Node`, etcd's `Client` embedding its - // own `Lease` interface to the server's `Lease` struct. + // The patterns below guess a declaration from the shape of a name, but Go + // reads a name from one package only: one written bare from the + // reference's own, `pkg.Node` from that import's (the import resolver's + // job), `x[i].Error()` from whatever type `x[i]` has. So they guess only + // for a bare name, only in its own package, and never for an embedded + // type, which name matching and the import resolver work out. Past that, + // promql/parser's `Node` parameters and embeddings went to + // discovery/kubernetes's struct `Node` beside the package's own `Node` + // interface, `apiv1.Node` to the struct in the same file, etcd's `Client` + // embedding its own `Lease` interface to the server's `Lease` struct, and + // every `.String()` called through an expression to the struct + // `promql.String`, as an instantiation. if (ref.referenceKind === 'extends' || ref.referenceKind === 'implements') return null; + if (!isGoBareName(ref, context)) return null; // Pattern 1: Handler references if (ref.referenceName.endsWith('Handler') || ref.referenceName.startsWith('Handle')) { - const result = resolveByNameAndKind(ref, 'function', HANDLER_DIRS, context); + const result = resolveInOwnPackage(ref, FUNCTION_KINDS, context); if (result) { return { original: ref, @@ -50,7 +57,7 @@ export const goResolver: FrameworkResolver = { // Pattern 2: Service/Repository references if (ref.referenceName.endsWith('Service') || ref.referenceName.endsWith('Repository') || ref.referenceName.endsWith('Store')) { - const result = resolveByNameAndKind(ref, null, SERVICE_DIRS, context, SERVICE_KINDS); + const result = resolveInOwnPackage(ref, SERVICE_KINDS, context); if (result) { return { original: ref, @@ -63,7 +70,7 @@ export const goResolver: FrameworkResolver = { // Pattern 3: Middleware references if (ref.referenceName.endsWith('Middleware') || ref.referenceName.startsWith('Auth') || ref.referenceName.startsWith('Log')) { - const result = resolveByNameAndKind(ref, 'function', MIDDLEWARE_DIRS, context); + const result = resolveInOwnPackage(ref, FUNCTION_KINDS, context); if (result) { return { original: ref, @@ -74,9 +81,12 @@ export const goResolver: FrameworkResolver = { } } - // Pattern 4: Model/Entity references (typically PascalCase structs) + // Pattern 4: Model/Entity references — a PascalCase type of any kind: the + // package's `Node` interface or `Entry` type, not only its structs. Left + // to name matching, the result type of `func (f *fanout) Appender(…) + // Appender` linked to the method the line declares. if (/^[A-Z][a-zA-Z]+$/.test(ref.referenceName)) { - const result = resolveByNameAndKind(ref, 'struct', MODEL_DIRS, context); + const result = resolveInOwnPackage(ref, GO_TYPE_KINDS, context); if (result) { return { original: ref, @@ -175,24 +185,17 @@ function extractGoTailIdent(expr: string): string | null { return m ? m[1]! : null; } -// Directory patterns for framework resolution -const HANDLER_DIRS = ['handler', 'handlers', 'api', 'routes', 'controller', 'controllers']; -const SERVICE_DIRS = ['service', 'services', 'repository', 'store', 'pkg']; -const MIDDLEWARE_DIRS = ['middleware', 'middlewares']; -const MODEL_DIRS = ['model', 'models', 'entity', 'entities', 'domain', 'pkg']; -const SERVICE_KINDS = new Set(['struct', 'interface']); +const FUNCTION_KINDS: ReadonlySet = new Set(['function']); +const SERVICE_KINDS: ReadonlySet = new Set(['struct', 'interface']); -/** A framework name heuristic's pick (see name-heuristic.ts), preferring these folders. */ -function resolveByNameAndKind( - ref: UnresolvedRef, - kind: string | null, - preferredDirs: string[], - context: ResolutionContext, - kinds?: Set -): string | null { - const allowed: ReadonlySet = kinds ?? (kind ? new Set([kind]) : GO_NAMED_KINDS); - return pickByNameAndKind(ref, allowed, (f) => preferredDirs.some((d) => f.includes(`/${d}/`)), context); +/** + * A framework name heuristic's pick (see name-heuristic.ts) among the + * declarations of the reference's own package — its directory — its own + * file's first. No folder convention applies: Go never reads a bare name from + * another package, a dot import aside. + */ +function resolveInOwnPackage(ref: UnresolvedRef, kinds: ReadonlySet, context: ResolutionContext): string | null { + const dir = ref.filePath.slice(0, ref.filePath.lastIndexOf('/') + 1); + const inPackage = (n: Node) => n.filePath.startsWith(dir) && !n.filePath.slice(dir.length).includes('/'); + return pickByNameAndKind(ref, kinds, () => false, context, { accept: inPackage }); } - -/** Any declaration a name can be (the heuristic with no kind). */ -const GO_NAMED_KINDS: ReadonlySet = new Set(['function', 'method', 'struct', 'interface', 'type_alias', 'variable', 'constant']); diff --git a/src/resolution/index.ts b/src/resolution/index.ts index 7e6d2034a..6716adf5d 100644 --- a/src/resolution/index.ts +++ b/src/resolution/index.ts @@ -21,7 +21,7 @@ import { isImportableKind, CPP_DEFINE_SIGNATURE, } from './types'; -import { isPythonSelfCall, matchJsStoreBindingCall, isUnresolvedJsMemberCall, matchObjectPathCall, thisScopeCaller, isVisibleAcrossFiles, matchReference, matchFunctionRef, matchDottedCallChain, matchScopedCallChain, matchMethodCall, sameLanguageFamily, crossesCodeBoundary, gateLanguageMatch, dumpNameMatcherProfile, clearNameMatcherMemos, isRustNameInScope, CASE_INSENSITIVE_LANGUAGES, isDartMemberRead, matchDartMemberRead, isDartChainLink, matchDartChainLink, isDartAnnotation, matchDartAnnotation, isStdMethodName, isGoUnknownQualified } from './name-matcher'; +import { isPythonSelfCall, matchJsStoreBindingCall, isUnresolvedJsMemberCall, matchObjectPathCall, thisScopeCaller, isVisibleAcrossFiles, matchReference, matchFunctionRef, matchDottedCallChain, matchScopedCallChain, matchMethodCall, sameLanguageFamily, crossesCodeBoundary, gateLanguageMatch, dumpNameMatcherProfile, clearNameMatcherMemos, isRustNameInScope, CASE_INSENSITIVE_LANGUAGES, isDartMemberRead, matchDartMemberRead, isDartChainLink, matchDartChainLink, isDartAnnotation, matchDartAnnotation, isStdMethodName, isGoUnknownQualified, isGoBareName, GO_TYPE_KINDS } from './name-matcher'; import { isVisibleCppMacro, clearCppMacroVisibility } from './cpp-macro-visibility'; import { isCppConstructorRef, matchCppConstructor } from './cpp-constructor'; import { gateSwiftTypeTarget, clearSwiftTypeVisibility, swiftExtendedConformances } from './swift-type-visibility'; @@ -3036,6 +3036,12 @@ export class ReferenceResolver { if (ref.referenceKind === 'calls') { const target = this.nodeById(result.targetNodeId); if (target?.kind === 'constant' && CPP_DEFINE_SIGNATURE.test(target.signature ?? '')) return null; + // A Go call made through an expression — `h.Validate().Error()`, a + // `.Metrics()` chained onto the line above — calls a method of whatever + // the expression is. A type is called only bare (`T(x)`, `(*T)(x)`) or + // through its package; as a struct here it read as an instantiation. + if (target && ref.language === 'go' && GO_TYPE_KINDS.has(target.kind) && + !ref.referenceName.includes('.') && !isGoBareName(ref, this.context)) return null; } // An `imports` reference names something importable — never a member that diff --git a/src/resolution/name-matcher.ts b/src/resolution/name-matcher.ts index 023826641..045fe03fc 100644 --- a/src/resolution/name-matcher.ts +++ b/src/resolution/name-matcher.ts @@ -1905,9 +1905,10 @@ function goRefQualification(ref: UnresolvedRef, context: ResolutionContext): GoQ const line = context.getFileLines?.(ref.filePath)?.[ref.line - 1] ?? context.readFile(ref.filePath)?.split(/\r?\n/)[ref.line - 1] ?? ''; const at = Math.max(0, ref.column); // The qualifier right before the name at the reference's column, or the - // line's only spelling of the name. - const written = line.startsWith(name, at) ? /(?:^|[^\w.])([A-Za-z_]\w*)\.$/.exec(line.slice(0, at))?.[1] - : !new RegExp(`(? m.localName === written) : undefined; const found = { written, imported }; memo.set(key, found); @@ -2396,6 +2397,37 @@ function isBareGoCall(ref: UnresolvedRef, context: ResolutionContext): boolean { return ref.language === 'go' && isReceiverLessCall(ref, context); } +/** + * Whether a Go reference's name is written bare, so that Go reads it from the + * reference's own package: a type `Node` (variadic `...Node` too), a composite + * literal `&Event{}`, a route's handler `Index`, a call `Walk(v, n)` or a + * conversion `(*Block)(pb)`. Not `parser.Node` or `...chunks.Meta`, written + * through an import, nor a name reached through a value: `err[i].Error()`, a + * `.String()` chained onto the line above, a route's handler `h.Follow`. + */ +export function isGoBareName(ref: UnresolvedRef, context: ResolutionContext): boolean { + const name = ref.referenceName; + if (ref.language !== 'go' || !/^[A-Za-z_]\w*$/.test(name)) return false; + if (ref.referenceKind === 'calls' && isReceiverLessCall(ref, context)) return true; + const line = context.getFileLines?.(ref.filePath)?.[ref.line - 1] + ?? context.readFile(ref.filePath)?.split(/\r?\n/)[ref.line - 1]; + if (line === undefined) return false; + // A call's column is its expression's: only a conversion's parenthesized + // type is still bare there. + if (ref.referenceKind === 'calls') { + return line[ref.column] === '(' && new RegExp(`^\\(\\s*\\*?\\s*${name}\\s*\\)\\s*\\(`).test(line.slice(ref.column)); + } + if (line.startsWith(name, ref.column)) { + let end = ref.column; + while (end > 0 && WHITESPACE.test(line[end - 1]!)) end--; + return line[end - 1] !== '.' || (end >= 3 && line.slice(end - 3, end) === '...'); + } + // A route's handler is recorded at the start of its line: its spelling + // there, outside the path string. + const code = line.replace(/"(?:[^"\\]|\\.)*"|`[^`]*`/g, (s) => ' '.repeat(s.length)); + return new RegExp(`(? = new Set([ 'return', 'await', 'yield', 'typeof', 'void', 'new', 'else', 'case', 'throw', 'in', 'of', 'instanceof', 'go', 'defer', + 'if', 'switch', 'for', 'range', ]); /** @@ -10611,7 +10647,7 @@ function goDeclaredTypePackage(raw: string | undefined, filePath: string, contex } /** The node kinds a Go `type` declaration produces. */ -const GO_TYPE_KINDS: ReadonlySet = new Set(['struct', 'interface', 'type_alias']); +export const GO_TYPE_KINDS: ReadonlySet = new Set(['struct', 'interface', 'type_alias']); /** * `methodName` on the Go type `typeName` that the package in directory