diff --git a/CHANGELOG.md b/CHANGELOG.md index fe1144fe4..bfa4c1d08 100644 --- a/CHANGELOG.md +++ b/CHANGELOG.md @@ -75,6 +75,7 @@ and adheres to [Semantic Versioning](https://semver.org/spec/v2.0.0.html). - In Go, working out which structs implement an interface now counts the methods of the interfaces it embeds, and the methods a struct gets from the types it embeds. Before, only each type's own methods counted: etcd's `AuthReadTx`, which embeds `UnsafeAuthReader` and adds `RLock` and `RUnlock`, was listed as implemented by every read-write lock in the project, an interface that only embeds others had no implementations at all, and a struct that gets its methods from an embedded base, like prometheus's service discoveries embedding `refresh.Discovery` or gin's `Engine` embedding `RouterGroup`, was missing. `codegraph_explore`, impact and the type hierarchy now list the right implementations, and a call through the interface reaches a method the embedding struct overrides, like gin's `Engine.Use`. Re-index Go projects after upgrading. - In Angular templates, a property binding, an interpolation, a structural directive or a control-flow block that calls one of the component's own members, like `[name]="icon()"`, `{{ label() }}`, `*ngIf="isOpen()"` or `@if (loading()) {`, now links the component to that member, and so does reading a getter, like `[disabled]="!canSave"`, or handing a method to a child component, like `[displayWith]="displayFn"` or `trackBy: trackById`. Before, only event bindings such as `(click)="save()"` were read, so a signal, a `computed` value, a getter or a method used only from a template had no callers and looked unused. A pipe, a template variable, a call on another object such as `form.reset()`, and a plain field read like `[value]="title"` still link nothing. Re-index Angular projects after upgrading. - React Router routes kept in a table of their own are now indexed: the ASP.NET Core React template's `AppRoutes` array that `App.js` maps into ``, a `routes` array imported into `useRoutes(routes)` or `createBrowserRouter(routes)` or returned by a function as in `useRoutes(routes(isLoggedIn))`, route objects kept one per file and listed in `createBrowserRouter([MainRoutes, LoginRoutes])`, and routes written in place in `useRoutes([…])`. Before, route objects were read only from a file that itself creates a data router, so these apps had no routes, and their `navigate('/login')` calls and `` links led nowhere. Each route links to the page its `element` renders, past a guard like `` or ``, and through `lazy(() => import(…))` to the page it loads rather than a same-named page elsewhere in the repository. An `index: true` route is the page at its parent's address, and a parent route around others counts as their layout. A `{ path, element }` list that nothing hands to the router, such as a menu, still makes no routes. Re-index React projects after upgrading. +- In Go, a call or type written through an import whose path ends in a version, or in something other than the package's name, is now known to go through that import: `yaml.Unmarshal(…)` after `import "go.yaml.in/yaml/v3"` or `"gopkg.in/yaml.v3"`, `sqlite3.Error` after `import "github.com/mattn/go-sqlite3"`, `klog.V(2)` after `import "k8s.io/klog/v2"`. Before, only the last part of the path named such an import (`v3`, `yaml.v3`, `go-sqlite3`), so the name was matched on its own and could link to any project function, method or type that shared it: kubernetes' `klog.V(…)` calls were linked to a logging wrapper's `V` method, and etcd's `semver.Version` parameters to an unrelated `Version` function. A name from another module now links to nothing, and one through a package of your own project, like `kit.New()` after `import "example.com/kit/v2"`, links to that package's symbol. A comment in an import block is also no longer taken for the name of the import after it. Re-index Go projects after upgrading. Thanks @danusha2345 for the comment fix. (#2374) ## [1.6.2] - 2026-10-03 diff --git a/__tests__/go-import-package-names.test.ts b/__tests__/go-import-package-names.test.ts new file mode 100644 index 000000000..14365910e --- /dev/null +++ b/__tests__/go-import-package-names.test.ts @@ -0,0 +1,249 @@ +/** + * A Go import without a name written before its path is bound to the name of + * the package it imports, and the resolver took that to be the path's last + * element. For a versioned path it isn't: `go.yaml.in/yaml/v3` and + * `gopkg.in/yaml.v3` are package `yaml`, `github.com/mattn/go-sqlite3` is + * `sqlite3`. So `yaml.Node`, `klog.V(2)` or `semver.Version` matched none of + * the file's imports, and resolved by the bare name to whatever project symbol + * shared it: kubernetes' 1,561 `klog.V(…)` calls went to an etcd3 logger + * wrapper's `V` method, etcd's `*semver.Version` parameters to client/v3's + * `Version` function. An unaliased import now also takes the name goimports + * assumes for its path, while keeping the last element: + * `k8s.io/api/core/v1` really is package `v1`. + * + * And a word in a comment is no import name: `"fmt" // for printing` used to + * name the next import `printing`. + */ +import { describe, it, expect, beforeAll, afterAll } from 'vitest'; +import * as fs from 'fs'; +import * as os from 'os'; +import * as path from 'path'; +import { CodeGraph } from '../src'; +import { extractImportMappings } from '../src/resolution/import-resolver'; + +/** The names a Go file's imports are bound to, as `name <- path`. */ +const names = (source: string): string[] => + extractImportMappings('x.go', source, 'go').map((m) => `${m.localName} <- ${m.source}`).sort(); + +describe('Go import names', () => { + it('bind an unaliased versioned or go- import to its package name as well as its last element', () => { + expect(names(`package p + +import ( + "go.yaml.in/yaml/v3" + "github.com/mattn/go-sqlite3" + "k8s.io/api/core/v1" + "github.com/gin-gonic/gin" + "gopkg.in/natefinch/lumberjack.v2" +) +`)).toEqual([ + 'core <- k8s.io/api/core/v1', + 'gin <- github.com/gin-gonic/gin', + 'go-sqlite3 <- github.com/mattn/go-sqlite3', + 'lumberjack <- gopkg.in/natefinch/lumberjack.v2', + 'lumberjack.v2 <- gopkg.in/natefinch/lumberjack.v2', + 'sqlite3 <- github.com/mattn/go-sqlite3', + 'v1 <- k8s.io/api/core/v1', + 'v3 <- go.yaml.in/yaml/v3', + 'yaml <- go.yaml.in/yaml/v3', + ]); + expect(names('package p\n\nimport "gopkg.in/yaml.v3"\n')).toEqual(['yaml <- gopkg.in/yaml.v3', 'yaml.v3 <- gopkg.in/yaml.v3']); + }); + + it('take no assumed name another import holds, or two imports assume', () => { + // `core` is pkg/apis/core's own name, not the one assumed for api/core/v1. + expect(names(`package p + +import ( + "k8s.io/api/core/v1" + "k8s.io/kubernetes/pkg/apis/core" + yaml "sigs.k8s.io/yaml" + "go.yaml.in/yaml/v3" + "github.com/a/semver/v3" + "github.com/b/go-semver" +) +`)).toEqual([ + 'core <- k8s.io/kubernetes/pkg/apis/core', + 'go-semver <- github.com/b/go-semver', + 'v1 <- k8s.io/api/core/v1', + 'v3 <- github.com/a/semver/v3', + 'v3 <- go.yaml.in/yaml/v3', + 'yaml <- sigs.k8s.io/yaml', + ]); + }); + + it('keep a written name, assume none for a dot import, and read none from a comment', () => { + expect(names(`package p + +import ( + "fmt" // for printing + metav1 "k8s.io/apimachinery/pkg/apis/meta/v1" + _ "embed" + . "github.com/onsi/ginkgo/v2" + + // yaml decoding + "gopkg.in/yaml.v3" + /* the store */ "example.com/app/store" +) + +func run() {} +`)).toEqual([ + '_ <- embed', + 'fmt <- fmt', + 'metav1 <- k8s.io/apimachinery/pkg/apis/meta/v1', + 'store <- example.com/app/store', + 'v2 <- github.com/onsi/ginkgo/v2', + 'yaml <- gopkg.in/yaml.v3', + 'yaml.v3 <- gopkg.in/yaml.v3', + ]); + }); + + it('read only the import declarations, wherever a comment or a later string spells one', () => { + expect(names(`/* +Package p is a code generator. + +func Example() {} +*/ +package p + +// import "example.com/commented" +import ( + "text/template" // (for the body below) + "strings" +) + +func run() string { + return strings.TrimSpace(\` +import "example.com/generated" +\`) +} +`)).toEqual(['strings <- strings', 'template <- text/template']); + }); +}); + +describe('Go references through a versioned or go- import', () => { + let root = ''; + let cg: CodeGraph; + + beforeAll(async () => { + root = fs.mkdtempSync(path.join(os.tmpdir(), 'cg-go-import-names-')); + const files: Record = { + 'go.mod': 'module example.com/app\n\ngo 1.22\n', + // Project symbols named like what yaml, klog, semver and sqlite3 export. + 'other/other.go': [ + 'package other', + '', + 'type Node struct{}', + '', + 'type Version struct{}', + '', + 'type Error struct{}', + '', + 'func Unmarshal(b []byte, v any) error { return nil }', + '', + 'type klogWrapper struct{}', + '', + 'func (k *klogWrapper) V(level int) bool { return true }', + '', + 'func (k *klogWrapper) Infof(format string, args ...any) {}', + '', + ].join('\n'), + 'store/store.go': [ + 'package store', + '', + 'import (', + '\t"github.com/Masterminds/semver/v3"', + '\t"github.com/mattn/go-sqlite3"', + '\t"go.yaml.in/yaml/v3"', + '\t"k8s.io/klog/v2"', + ')', + '', + 'func Load(b []byte, root *yaml.Node, min *semver.Version) error {', + '\tif klog.V(2) {', + '\t\tklog.Infof("loading %d bytes", len(b))', + '\t}', + '\tvar e sqlite3.Error', + '\t_ = e', + '\treturn yaml.Unmarshal(b, root)', + '}', + '', + ].join('\n'), + // A project module whose path ends in a major version: package kit. + 'kit/go.mod': 'module example.com/kit/v2\n\ngo 1.22\n', + 'kit/kit.go': [ + 'package kit', + '', + 'type Kit struct{}', + '', + 'func New() *Kit { return &Kit{} }', + '', + ].join('\n'), + 'decoy/decoy.go': [ + 'package decoy', + '', + 'type Kit struct{}', + '', + 'func New() *Kit { return &Kit{} }', + '', + 'func Find(id string) string { return id }', + '', + ].join('\n'), + // k8s.io/api/core/v1 is package v1, beside a package named core. + 'api/core/v1/types.go': 'package v1\n\ntype Pod struct{}\n', + 'pkg/apis/core/types.go': 'package core\n\ntype Pod struct{}\n', + 'repo/repo.go': 'package repo\n\nfunc Find(id string) string { return id }\n', + 'app/app.go': [ + 'package app', + '', + 'import (', + '\t"fmt" // for printing', + '\t"example.com/app/repo"', + '\t"example.com/app/api/core/v1"', + '\t"example.com/app/pkg/apis/core"', + '\t"example.com/kit/v2"', + ')', + '', + 'func Run(p *v1.Pod, q *core.Pod) *kit.Kit {', + '\tfmt.Println(repo.Find("x"))', + '\treturn kit.New()', + '}', + '', + ].join('\n'), + }; + 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 }); + }, 60_000); + + afterAll(() => { + cg?.close(); + if (root) fs.rmSync(root, { recursive: true, force: true }); + }); + + const targetsFrom = (file: string) => { + const ids = cg.getNodesInFile(file).map((n) => n.id); + return cg.getOutgoingEdgesFrom(ids) + .filter((e) => e.kind !== 'contains') + .map((e) => { + const t = cg.getNode(e.target)!; + return `${e.kind} ${t.qualifiedName}@${t.filePath}`; + }) + .sort(); + }; + + it('never reach a project symbol that only shares the name', () => { + expect(targetsFrom('store/store.go').filter((t) => t.includes('@other/'))).toEqual([]); + }); + + it('reach the project package the import names', () => { + expect(targetsFrom('app/app.go')).toEqual([ + 'calls Find@repo/repo.go', + 'calls New@kit/kit.go', + 'references Kit@kit/kit.go', + 'references Pod@api/core/v1/types.go', + 'references Pod@pkg/apis/core/types.go', + ]); + }); +}); diff --git a/__tests__/go-interface-embedding.test.ts b/__tests__/go-interface-embedding.test.ts index 464748575..2883c6f50 100644 --- a/__tests__/go-interface-embedding.test.ts +++ b/__tests__/go-interface-embedding.test.ts @@ -25,8 +25,9 @@ * Resolution keeps an embedded type in its package. A bare name is its own * package's type, not another package's struct of that name, which the Go * framework heuristics preferred (promql/parser's `Node` interface, etcd's - * `Lease`). A package that is none of the file's imports as indexed (`yaml` - * under an unaliased `go.yaml.in/yaml/v3`) leaves the type unresolved. + * `Lease`). A package that is none of the file's imports as indexed + * (`clientv3` under an unaliased `go.etcd.io/etcd/client/v3`, which goimports + * would call `client`) leaves the type unresolved. */ import { describe, it, expect, beforeAll, beforeEach, afterEach } from 'vitest'; import * as fs from 'fs'; @@ -260,7 +261,7 @@ describe('the type hierarchy of an indexed Go module', () => { 'func (b *Base) Close() error { return nil }', '', ].join('\n'), - // Named like io.Closer, io.Reader, sync.Mutex and yaml.Node: never what those name. + // Named like io.Closer, io.Reader, sync.Mutex, yaml.Node and clientv3.KV: never what those name. 'other/other.go': [ 'package other', '', @@ -272,6 +273,8 @@ describe('the type hierarchy of an indexed Go module', () => { '', 'type Node struct{}', '', + 'type KV interface{ Get(key string) string }', + '', ].join('\n'), 'store/store.go': [ 'package store', @@ -281,15 +284,21 @@ describe('the type hierarchy of an indexed Go module', () => { '\t"sync"', '', '\t"example.com/app/storage"', + '\t"go.etcd.io/etcd/client/v3"', '\t"go.yaml.in/yaml/v3"', ')', '', - '// The package is yaml, though the index knows its import as v3.', + '// The package is yaml, though its import path ends in v3.', 'type RuleGroupNode struct {', '\tyaml.Node', '\tName string', '}', '', + '// The package is clientv3: not v3, nor the client goimports would assume.', + 'type kvPrefix struct {', + '\tclientv3.KV', + '}', + '', 'type Queryable interface{ Q() }', '', 'type Querier interface {', @@ -367,6 +376,7 @@ describe('the type hierarchy of an indexed Go module', () => { one('Number', 'store/store.go'), one('Store', 'store/store.go'), one('RuleGroupNode', 'store/store.go'), + one('kvPrefix', 'store/store.go'), one('Reader', 'wrap/wrap.go'), one('IRouter', 'gin/routergroup.go'), one('Expr', 'parser/ast.go'), @@ -381,8 +391,9 @@ describe('the type hierarchy of an indexed Go module', () => { .sort(); // An embedded interface of a struct resolves as `implements`; one of an // interface stays `extends`. io.Closer, sync.Mutex, io.Reader and yaml.Node - // are outside the project, so nothing named like them is linked. A bare - // name is its own package's type, whatever kind another package's is. + // are outside the project, and clientv3 is no import the index knows, so + // nothing named like them is linked. A bare name is its own package's + // type, whatever kind another package's is. expect(supertypes).toEqual([ 'Expr extends Node@parser/ast.go', 'IRouter extends IRoutes@gin/routergroup.go', @@ -405,6 +416,7 @@ describe('the type hierarchy of an indexed Go module', () => { expect(unresolved(one('Querier', 'store/store.go'))).toEqual(['Closer']); expect(unresolved(one('Store', 'store/store.go'))).toEqual(['Mutex']); expect(unresolved(one('RuleGroupNode', 'store/store.go'))).toEqual(['Node']); + expect(unresolved(one('kvPrefix', 'store/store.go'))).toEqual(['KV']); expect(unresolved(one('Reader', 'wrap/wrap.go'))).toEqual(['Reader']); // What the viewer's Type hierarchy and codegraph_explore read. Declared @@ -418,7 +430,7 @@ describe('the type hierarchy of an indexed Go module', () => { expect(subtypes(one('LabelQuerier', 'storage/storage.go'))).toEqual(['extends Querier@store/store.go']); expect(subtypes(one('Base', 'storage/storage.go'))).toEqual(['extends Store@store/store.go']); const declaredInto = graph - .getIncomingEdgesTo(['Closer', 'Mutex', 'Reader', 'Node'].map((name) => one(name, 'other/other.go').id), ['extends', 'implements']) + .getIncomingEdgesTo(['Closer', 'Mutex', 'Reader', 'Node', 'KV'].map((name) => one(name, 'other/other.go').id), ['extends', 'implements']) .filter((e) => e.provenance !== 'heuristic'); expect(declaredInto).toEqual([]); expect( diff --git a/src/resolution/import-resolver.ts b/src/resolution/import-resolver.ts index bf5dab41b..8847ef653 100644 --- a/src/resolution/import-resolver.ts +++ b/src/resolution/import-resolver.ts @@ -1145,50 +1145,85 @@ function extractPythonImports(content: string): ImportMapping[] { } /** - * Extract Go import mappings + * Extract Go import mappings. An import is bound to the name written before + * its path, or else to the name of the package it imports — which the path + * alone doesn't settle: `k8s.io/api/core/v1` is package `v1`, while + * `go.yaml.in/yaml/v3` and `gopkg.in/yaml.v3` are `yaml`, and + * `github.com/mattn/go-sqlite3` is `sqlite3`. So an unaliased import keeps its + * last path element and also takes the name goimports assumes for the path, + * unless another of the file's imports is bound to that name. */ function extractGoImports(content: string): ImportMapping[] { - const mappings: ImportMapping[] = []; - - // import "path" or import alias "path" - const singleImportRegex = /import\s+(?:(\w+)\s+)?["']([^"']+)["']/g; + const specs: Array<{ name: string | undefined; source: string }> = []; + // Only the imports, which Go puts before any other declaration, and without + // their comments: a word in one is no name — `"fmt" // for printing` used to + // name the next import `printing`. A name is on its path's own line. + const code = stripCommentsForRegex(content.slice(0, goImportsEnd(content)), 'go'); + + // import "path" or import name "path" + const singleImportRegex = /\bimport\s+(?:([A-Za-z_]\w*)[ \t]*)?["']([^"']+)["']/g; let match; - - while ((match = singleImportRegex.exec(content)) !== null) { - const [, alias, source] = match; - const packageName = source!.split('/').pop()!; - mappings.push({ - localName: alias || packageName, - exportedName: '*', - source: source!, - isDefault: false, - isNamespace: true, - }); + while ((match = singleImportRegex.exec(code)) !== null) { + specs.push({ name: match[1], source: match[2]! }); } // import ( ... ) block - const blockImportRegex = /import\s*\(\s*([^)]+)\s*\)/gs; - while ((match = blockImportRegex.exec(content)) !== null) { - const block = match[1]!; - const lineRegex = /(?:(\w+)\s+)?["']([^"']+)["']/g; + const blockImportRegex = /\bimport\s*\(\s*([^)]+)\s*\)/g; + while ((match = blockImportRegex.exec(code)) !== null) { + const lineRegex = /(?:([A-Za-z_]\w*|\.)[ \t]*)?["']([^"']+)["']/g; let lineMatch; - - while ((lineMatch = lineRegex.exec(block)) !== null) { - const [, alias, source] = lineMatch; - const packageName = source!.split('/').pop()!; - mappings.push({ - localName: alias || packageName, - exportedName: '*', - source: source!, - isDefault: false, - isNamespace: true, - }); + while ((lineMatch = lineRegex.exec(match[1]!)) !== null) { + specs.push({ name: lineMatch[1], source: lineMatch[2]! }); } } + const mapping = (localName: string, source: string): ImportMapping => + ({ localName, exportedName: '*', source, isDefault: false, isNamespace: true }); + // A dot import (`. "pkg"`) binds no name of its own; it stays listed under + // its last path element. + const mappings = specs.map(({ name, source }) => + mapping(name === undefined || name === '.' ? source.split('/').pop()! : name, source)); + const bound = new Set(mappings.map((m) => m.localName)); + const assumed = new Map(); + for (const { name, source } of specs) { + if (name !== undefined) continue; + const assumedName = goAssumedPackageName(source); + if (assumedName && !bound.has(assumedName)) assumed.set(assumedName, [...(assumed.get(assumedName) ?? []), source]); + } + // Two imports that assume one name can't both be that package. + for (const [localName, sources] of assumed) { + if (sources.length === 1) mappings.push(mapping(localName, sources[0]!)); + } return mappings; } +/** + * Where a Go file's imports end: at its first other top-level declaration, a + * line opening with `func`, `type`, `var` or `const` outside a block comment. + */ +function goImportsEnd(content: string): number { + const decl = /^(?:func|type|var|const)\b/gm; + for (let m; (m = decl.exec(content)) !== null;) { + if (content.lastIndexOf('/*', m.index) <= content.lastIndexOf('*/', m.index)) return m.index; + } + return content.length; +} + +/** + * The package name goimports assumes for an import path + * (ImportPathToAssumedName): its last element that isn't a major version + * (`go.yaml.in/yaml/v3` → `yaml`), without a `go-` prefix + * (`github.com/mattn/go-sqlite3` → `sqlite3`), cut where an identifier can't + * go on (`gopkg.in/yaml.v3` → `yaml`). + */ +function goAssumedPackageName(importPath: string): string { + const elements = importPath.split('/'); + let base = elements[elements.length - 1]!; + if (elements.length > 1 && /^v\d+$/.test(base)) base = elements[elements.length - 2]!; + if (base.startsWith('go-')) base = base.slice(3); + return /^[\p{L}\p{Nd}_]*/u.exec(base)![0]; +} + /** * Extract Java / Kotlin import mappings. * diff --git a/src/resolution/index.ts b/src/resolution/index.ts index d8eca46e4..f71e8c086 100644 --- a/src/resolution/index.ts +++ b/src/resolution/index.ts @@ -3125,10 +3125,10 @@ export class ReferenceResolver { result = { ...result, targetNodeId: type.id }; } if (isBoundToOutOfRepoImport(ref, this.context)) return null; - // A Go type embedded through a package (`yaml.Node`) is that package's. - // When the package is none of the file's imports as indexed, a type found - // by its bare name is some other package's namesake: prometheus's - // `RuleGroupNode` embeds yaml's `Node`, not discovery/kubernetes's. + // A Go type embedded through a package (`clientv3.KV`) is that package's. + // When the package is none of the file's imports as indexed (clientv3 + // imported unaliased as `go.etcd.io/etcd/client/v3`), a type found by its + // bare name is some other package's namesake. if (ref.language === 'go' && isGoUnknownQualified(ref, this.context)) return null; return result; } diff --git a/src/resolution/name-matcher.ts b/src/resolution/name-matcher.ts index 08dc5575f..69bebef53 100644 --- a/src/resolution/name-matcher.ts +++ b/src/resolution/name-matcher.ts @@ -1927,9 +1927,10 @@ export function goRefQualifier(ref: UnresolvedRef, context: ResolutionContext): /** * Whether a Go reference is written through a package that is none of its - * file's imports as the index knows them: `yaml` in `yaml.Node` under an - * unaliased `import "go.yaml.in/yaml/v3"`, which is recorded under its last - * path element, `v3`. Which package that is cannot be told from here. + * file's imports as the index knows them: `clientv3` in `clientv3.KV` under an + * unaliased `import "go.etcd.io/etcd/client/v3"`, a package named neither by + * its path's last element (`v3`) nor by the name goimports assumes for it + * (`client`). Which package that is cannot be told from here. */ export function isGoUnknownQualified(ref: UnresolvedRef, context: ResolutionContext): boolean { const { written, imported } = goRefQualification(ref, context);