mirror of
https://github.com/colbymchenry/codegraph.git
synced 2026-10-24 19:30:10 +08:00
fix(go): a project package named like a standard-library package is not the standard library
A Go reference written through a package qualifier was dropped as a standard-library reference whenever the qualifier matched a name in the built-in list (errors, log, types, parser, encoding, template, ...). The check looked at the name alone, never at what the file imports under that name, so a project with its own package of such a name lost nearly every call into it: harbor's errors.New(...) through its lib/errors, prometheus's parser.Inspect(...) through promql/parser. What the file imports now decides before the name does. A qualifier the file imports, by its last path segment, the name goimports assumes for an unaliased import, or an alias, from a package of one of the project's own modules is a project package, and the reference goes on to normal resolution. A file that imports the standard library's package of that name, and a qualifier the file doesn't import at all, are skipped as before. Validation: the new test file has 7 cases, 4 of which fail on main; all pass here, with the native kernel and with CODEGRAPH_KERNEL=0. A/B against main: harbor 296f304 (its Go tree, 1,611 Go files) +2,889 edges, prometheus a2fda0e +454, 0 removed and 0 retargeted on both, nodes unchanged. Every added edge was checked against its source: the line spells qualifier.Name and the file imports the target's package directory under that qualifier from inside the module (3,343 of 3,343), and a sample of 10 was read by eye. gin and cobra, and the controls express and flask, are byte-identical. Index CPU on harbor, median of 3 interleaved: 40.0 s on main, 39.9 s here. Original PR #2368 by @danusha2345. Co-Authored-By: Claude Opus 5.5 <noreply@anthropic.com>
This commit is contained in:
committed by
Colby Mchenry
co-authored by
Claude Opus 5.5
parent
449eefa7da
commit
16f2d357bc
@@ -234,6 +234,7 @@ and adheres to [Semantic Versioning](https://semver.org/spec/v2.0.0.html).
|
||||
- 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)
|
||||
- In Go, a call is now matched to the package or variable it is actually made on. Before, a call through a package, like `klog.Infof("Log using Infof")` or `digest.Digest(dig)` inside `&distribution.Descriptor{Digest: …}`, could link to a project method of the same name when the name appeared again on the line, and in `klog.Error(err.Error())` the `err.Error()` call was taken for a call into `klog`. A parameter, receiver or local variable named like an import, like etcd's `jwt, err := newTokenProviderJWT(…)` or the `suite` receiver in a testify test suite, now counts as that variable, so calls on it link to the methods of the type it is declared as, and to nothing when that type comes from outside your project. Re-index Go projects after upgrading.
|
||||
- In Go, a call through a parameter or local variable named like a standard-library package, such as `context`, `log`, `trace`, `printer` or `token`, now links to the method it calls. Before, every call written `context.…` or `printer.…` was taken for a call into the standard library and linked to nothing: gin's `context.AbortWithError(…)` inside `func(context *Context)`, harbor's `context.IsAuthenticated()` after `context := NewSecurityContext(…)` and `log.Errorf(…)` after `log := log.G(ctx)`, etcd's `printer.DBHashKV(…)` and `trace.Step(…)`. The variable's declaration decides the method: the type it is declared as, or the package of the project function its value comes from. A variable holding a value from outside your project, like `scanner := bufio.NewScanner(r)` or `url, err := url.Parse(raw)`, still links to nothing, and one whose type can't be told links only to a method no other type in the project has, never to one picked because it matches the variable's name. Re-index Go projects after upgrading.
|
||||
- In Go, a call into one of your own packages that shares its name with a standard-library package, like harbor's `lib/errors` and `lib/log` or prometheus's `promql/parser`, now links: `errors.New(…)` or `log.Infof(…)` in a file that imports your package reaches your function, and so does a struct built from such a package, like `types.Policy{…}`. Before, those calls were taken for the standard library's and dropped, so the package's callers and impact came back nearly empty. A file that imports the standard library's package of that name is unaffected. Re-index Go projects after upgrading. Thanks @danusha2345. (#2368)
|
||||
- In Go, a call of a method name that nearly every type has, like `err.Error()`, `id.String()`, `ctx.Err()` or `mu.Lock()`, now links to that method of the type its variable is declared as, and to nothing when that type is built into Go, like `error` or `string`, or comes from outside your project, like `context.Context`, `sync.Mutex` or `testing.TB`. Before, when the variable's type wasn't worked out, such a call was matched by a word its name shares with a type's, so kubernetes' `err.Error()` calls linked to an unrelated `notRegisteredErr` type and etcd's `leaderID.String()` to `MoveLeaderRequest`. A call on a variable whose type still can't be told now links only to a type the variable is named after, like `id` for an `ID`. Re-index Go projects after upgrading.
|
||||
- In Go, a call on a parameter or variable declared with a type from another package, like `client rest.Interface`, `app storage.Appender` or `pod *v1.Pod`, now links to that type's method, including one it gets from a type it embeds or one an alias names, and to nothing when the type comes from outside your project, like `w http.ResponseWriter`, `c clock.PassiveClock` or an alias of such a type. Before, the type was read only for a few method names that nearly every type has, so other calls were matched by name: `client.Patch(…)` linked to a `RESTClient` rather than the interface it is declared as, `c.Now()` to an unrelated `Clock`, and a `sync.Pool` or `http.Client` variable's calls to a project type of that name. Names declared together, like `a, b rest.Interface`, `var ( … )` blocks, named results and function literal parameters are read the same way, always from the declaration in scope rather than another variable of that name further up, like a test table's field. Re-index Go projects after upgrading. Thanks @danusha2345, whose #1954 found the same gap.
|
||||
- In Go, a method call on a variable or constant declared at the top level of a package, like `lg.Panic(…)` after `var lg *zap.Logger` or `RoleEndpoint.String()` after `RoleEndpoint Role = "endpoints"`, now links to that method of the type it is declared with, from any file of the package, and to nothing when that type, or the value it is set to, comes from outside your project. Before, nothing read such a declaration, so the call was matched by its method's name alone and could link to an unrelated type's method, like a project logger's `Panic` or another package's `Role`. Re-index Go projects after upgrading.
|
||||
|
||||
@@ -0,0 +1,172 @@
|
||||
/**
|
||||
* A project package may carry a standard-library name — harbor's
|
||||
* `src/lib/errors` and `src/lib/log`, a `pkg/types`, a `model/user`. What a
|
||||
* file imports under that name decides whose `errors.New(…)` it writes: the
|
||||
* project package's when the import path is inside one of the project's
|
||||
* modules, the standard library's otherwise.
|
||||
*/
|
||||
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-stdname-'));
|
||||
const files: Record<string, string> = {
|
||||
'go.mod': 'module example.com/app\n\ngo 1.22\n',
|
||||
'lib/errors/errors.go': `package errors
|
||||
|
||||
type Error struct {
|
||||
Code string
|
||||
}
|
||||
|
||||
func New(msg string) *Error {
|
||||
return &Error{Code: msg}
|
||||
}
|
||||
`,
|
||||
'lib/log/log.go': `package log
|
||||
|
||||
func Printf(format string, args ...any) {}
|
||||
`,
|
||||
'svc/project.go': `package svc
|
||||
|
||||
import (
|
||||
"example.com/app/lib/errors"
|
||||
"example.com/app/lib/log"
|
||||
)
|
||||
|
||||
func Create(name string) *errors.Error {
|
||||
log.Printf("create %s", name)
|
||||
return errors.New(name)
|
||||
}
|
||||
|
||||
func Literal() errors.Error {
|
||||
return errors.Error{Code: "x"}
|
||||
}
|
||||
`,
|
||||
'svc/renamed.go': `package svc
|
||||
|
||||
import (
|
||||
liberrors "example.com/app/lib/errors"
|
||||
)
|
||||
|
||||
func Renamed() *liberrors.Error {
|
||||
return liberrors.New("x")
|
||||
}
|
||||
`,
|
||||
'svc/both.go': `package svc
|
||||
|
||||
import (
|
||||
stderrors "errors"
|
||||
|
||||
errors "example.com/app/lib/errors"
|
||||
)
|
||||
|
||||
func Ours() *errors.Error {
|
||||
return errors.New("x")
|
||||
}
|
||||
|
||||
func Theirs() error {
|
||||
return stderrors.New("y")
|
||||
}
|
||||
`,
|
||||
'svc/std.go': `package svc
|
||||
|
||||
import (
|
||||
"errors"
|
||||
"log"
|
||||
)
|
||||
|
||||
func Std() error {
|
||||
log.Printf("std")
|
||||
return errors.New("std")
|
||||
}
|
||||
`,
|
||||
'lib/log/v2/log.go': `package log
|
||||
|
||||
func Infof(format string, args ...any) {}
|
||||
`,
|
||||
// Unaliased imports whose paths end in a major version are known as `log`
|
||||
// and `errors`; only the first is a package of this module.
|
||||
'svc/versioned.go': `package svc
|
||||
|
||||
import (
|
||||
"example.com/app/lib/log/v2"
|
||||
"github.com/acme/errors/v2"
|
||||
)
|
||||
|
||||
func Versioned() error {
|
||||
log.Infof("versioned")
|
||||
return errors.New("outside")
|
||||
}
|
||||
`,
|
||||
'svc/variable.go': `package svc
|
||||
|
||||
type bag struct{}
|
||||
|
||||
func (b *bag) Len() int { return 0 }
|
||||
|
||||
func Variable(errors *bag) int {
|
||||
errors.New("x")
|
||||
return errors.Len()
|
||||
}
|
||||
`,
|
||||
};
|
||||
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:name` of every edge leaving the function `fn` of `file`. */
|
||||
const edgesFrom = (file: string, fn: string) => {
|
||||
const ids = cg.getNodesInFile(file).filter((n) => n.name === fn).map((n) => n.id);
|
||||
return cg.getOutgoingEdgesFrom(ids).filter((e) => e.kind !== 'contains').map((e) => {
|
||||
const target = cg.getNode(e.target)!;
|
||||
return `${e.kind} ${target.filePath}:${target.qualifiedName}`;
|
||||
});
|
||||
};
|
||||
|
||||
describe('Go project package named like a standard-library package', () => {
|
||||
it('resolves a call written through its import', () => {
|
||||
const edges = edgesFrom('svc/project.go', 'Create');
|
||||
expect(edges).toContain('calls lib/errors/errors.go:New');
|
||||
expect(edges).toContain('calls lib/log/log.go:Printf');
|
||||
});
|
||||
|
||||
it('resolves a composite literal of its type', () => {
|
||||
expect(edgesFrom('svc/project.go', 'Literal')).toContain('instantiates lib/errors/errors.go:Error');
|
||||
});
|
||||
|
||||
it('resolves through an import alias', () => {
|
||||
expect(edgesFrom('svc/renamed.go', 'Renamed')).toContain('calls lib/errors/errors.go:New');
|
||||
});
|
||||
|
||||
it('resolves when the alias is the standard-library name itself', () => {
|
||||
expect(edgesFrom('svc/both.go', 'Ours')).toContain('calls lib/errors/errors.go:New');
|
||||
expect(edgesFrom('svc/both.go', 'Theirs')).toEqual([]);
|
||||
});
|
||||
|
||||
it('resolves through an unaliased import whose path ends in a major version', () => {
|
||||
expect(edgesFrom('svc/versioned.go', 'Versioned')).toEqual(['calls lib/log/v2/log.go:Infof']);
|
||||
});
|
||||
|
||||
it('leaves a file importing the standard library package alone', () => {
|
||||
expect(edgesFrom('svc/std.go', 'Std')).toEqual([]);
|
||||
});
|
||||
|
||||
it('leaves a variable of that name alone when the file imports no such package', () => {
|
||||
const edges = edgesFrom('svc/variable.go', 'Variable');
|
||||
expect(edges).not.toContain('calls lib/errors/errors.go:New');
|
||||
});
|
||||
});
|
||||
+13
-1
@@ -3307,6 +3307,16 @@ export class ReferenceResolver {
|
||||
!PYTHON_BUILT_IN_TYPES.has(ref.referenceName.slice(0, dot));
|
||||
}
|
||||
|
||||
/**
|
||||
* True when the Go file imports `qualifier` — by its last path segment or
|
||||
* an alias — from a package of one of the project's own modules.
|
||||
*/
|
||||
private isGoProjectPackage(ref: UnresolvedRef, qualifier: string): boolean {
|
||||
return this.context
|
||||
.getImportMappings(ref.filePath, ref.language)
|
||||
.some((imp) => imp.localName === qualifier && this.context.getGoPackageDir?.(imp.source, ref.filePath) != null);
|
||||
}
|
||||
|
||||
/**
|
||||
* Check if reference is to a built-in or external symbol
|
||||
*/
|
||||
@@ -3401,7 +3411,9 @@ export class ReferenceResolver {
|
||||
// Go standard library packages — refs like "fmt.Println", "http.ListenAndServe", etc.
|
||||
if (ref.language === 'go') {
|
||||
const dotIdx = name.indexOf('.');
|
||||
if (dotIdx > 0) {
|
||||
// What the file imports decides before the name does: a project package
|
||||
// may be called `errors`, `log` or `types` too.
|
||||
if (dotIdx > 0 && !this.isGoProjectPackage(ref, name.substring(0, dotIdx))) {
|
||||
const pkg = name.substring(0, dotIdx);
|
||||
// Not a local bound from a type assertion that is named like one:
|
||||
// grpc-go's `parser.ParseConfig(…)` after `parser, ok :=
|
||||
|
||||
Reference in New Issue
Block a user