Repository navigation
fix(go): a dotted call is written through its own receiver, and a local named like an import is that variable - #2448
Merged
Conversation
…al named like an import is that variable goRefQualification read a dotted Go call's package qualifier from its line, as the line's only `X.Name` spelling. A second spelling of the name left it with none (`Digest:` beside `digest.Digest(dig)`, `"Log using Infof"`), and another selector before it was taken instead (`klog` for `err.Error` in `klog.Error(err.Error())`). It now takes the reference name's first segment, the only one that can be a package. A call through a parameter or local that takes an import's name (etcd's `jwt, err := newTokenProviderJWT(…)`, testify suites' `suite` receivers) is a call on that variable. A per-file scope reader finds the parameters, receivers, results, `:=`, `var` and `const` names in scope at the call, so such a call is no longer one into the package. The variable's declared type, when its declaration writes one, decides what it reaches: an outside package's type has none of the project's methods, a project package's type is resolved as the receiver's type. Co-Authored-By: Claude Opus 5.5 <noreply@anthropic.com>
…s it The Go scope reader keeps each file's declarations per resolution context. clearNameMatcherMemos now drops them with the other per-file memos, so a sync that removes a local named like an import stops treating the call as one on that local. Co-Authored-By: Claude Opus 5.5 <noreply@anthropic.com>
…own type `metadata.Type.String()` on a parameter `metadata prompb.MetricMetadata` calls String on the field, so the parameter's declared type only stands for a call made on the parameter itself. Co-Authored-By: Claude Opus 5.5 <noreply@anthropic.com>
…d like an import Co-Authored-By: Claude Opus 5.5 <noreply@anthropic.com>
…function, has no project method `clock := clocktesting.NewFakePassiveClock(…)` holds a value of a type that package declares, so `clock.Now()` is no call to a project `Clock`'s Now. Co-Authored-By: Claude Opus 5.5 <noreply@anthropic.com>
…ts braces by regex The Go scope reader built its blanked text by appending each piece and replacing every character of each comment and string. It now collects the pieces and joins them once, and skips from one comment or quote to the next. Same text on every Go file of gin, prometheus, etcd, harbor and kubernetes; about twice as fast. Co-Authored-By: Claude Opus 5.5 <noreply@anthropic.com>
This was referenced Oct 7, 2026
Merged
This file contains hidden or bidirectional Unicode text that may be interpreted or compiled differently than what appears below. To review, open the file in an editor that reveals hidden Unicode characters.
Learn more about bidirectional Unicode characters
Sign up for free
to join this conversation on GitHub.
Already have an account?
Sign in to comment
Add this suggestion to a batch that can be applied as a single commit.This suggestion is invalid because no changes were made to the code.Suggestions cannot be applied while the pull request is closed.Suggestions cannot be applied while viewing a subset of changes.Only one suggestion per line can be applied in a batch.Add this suggestion to a batch that can be applied as a single commit.Applying suggestions on deleted lines is not supported.You must change the existing code in this line in order to create a valid suggestion.Outdated suggestions cannot be applied.This suggestion has been applied or marked resolved.Suggestions cannot be applied from pending reviews.Suggestions cannot be applied on multi-line comments.Suggestions cannot be applied while the pull request is queued to merge.Suggestion cannot be applied right now. Please check back later.
Problem
goRefQualification(src/resolution/name-matcher.ts) decides which package a Go reference is written through, andisGoExternalQualified,isInGoQualifierPackageandresolveGoCrossPackageReferenceact on that answer (#2322, #2397). Two gaps were left after #2410.1. A dotted call's qualifier was read from its line. A call ref like
klog.Infofis recorded at its receiver's column, and the reader took the line's onlyX.Infofspelling. So:&distribution.Descriptor{Digest: digest.Digest(dig)}(theDigest:key; 9 calls →Suite::Digest), kubernetes'klog.Infof("Log using Infof, …")andklog.Errorf(…)incomponent-base/logs/example/example.go→ the etcd3 logger wrapper's methods, andcmp.Diffon a line that prints"Diff:"→ kubectl'sDiffer::Diff.klog.Error(err.Error()),err.Errorwas read as a call throughklog.2. A parameter or local named like an import was taken for the package.
isGoExternalQualifiedrejected every candidate of a call through it, methods included. Examples: etcd'sjwt, err := newTokenProviderJWT(…)thenjwt.assign(…), and harbor's testify suites, whosefunc (suite *DaoTestSuite) …receivers sit in files that import testify'ssuite.resolveGoCrossPackageReferencefollowed the import for it: kubernetes'cache := &atomic.Bool{}; cache.Store(false)linked to client-go'scache.Storeinterface, and harbor'slogger.Error(…)on a parameterlogger logger.Interfaceto the package-level functionlogger.Error.What changed
Resolver only (
name-matcher.ts, one line inimport-resolver.ts); extraction and the kernel are untouched.kloginklog.Infof,sins.cache.Get. Bare references are read from the line as before.funcwith a body (function literals included, function types excluded), and the names each:=,varandconstdeclares. Comments and string contents are blanked first.jwt, err := jwt.Parse(…)still calls the package on its right-hand side), anif/for/switchheader's names in that statement's blocks (elsechains included), aselectcase's in its clause, and a parameter in its function's body.quota.Name,args.PrefersProtobuf) from a method value; it is left as it was.clock clock.PassiveClock,cache := &atomic.Bool{}), or a value an outside package's function hands out (clock := clocktesting.NewFakePassiveClock(…),value := reflect.New(t)), has none of the project's methods. Such a call stays unlinked, as it was onmain.robot := &robot.Robot{…}reaches theToJSONthatcontroller/robot.Robotgets from the embeddedmodel.Robot). It also covers parameters declared through a package, likelogger logger.Interface, which Go's receiver patterns don't read. A field read through the local keeps its own type:metadata.Type.String()is notMetricMetadata::String.clearNameMatcherMemosdrops the scope reader's per-file cache with the other per-file memos, so a sync that edits the declaration is read afresh. There is a test for this.Validation
Tests
__tests__/go-ref-qualifier.test.ts, 6 tests:main(ed199e6). The sixth (the package stays the package elsewhere) passes there and pins that the scope reader doesn't overreach. On this branch, the sync test also fails if the new cache clear is taken out.CODEGRAPH_KERNEL=0.elsechains,selectclauses,var (…)groups holding multi-line literals,map[string]func(jwt string){…}types, generics, multi-line parameter lists, comments, strings and labels.tscpasses. 27 Go / resolution / extraction / framework suites (1,174 tests) pass withCODEGRAPH_KERNEL_EXPECT=1, plus the sync, sync-import-retry and dead-code suites (116 tests). The one exception isfunction-ref's Caller/impact graph misses methods passed as first-class references (callbacks), e.g. executor.submit(obj.method, …) #1820 Python case, which hit its own 60 s timeout at 100% CPU and passes with a longer limit.Before / after (full index, kernel path,
main@ed199e60 vs this branch, same kernel; natural-key edge diff,vendor/excluded; each moved or re-labelled edge counts once in removed and once in added)Re-run on the final base (
main@1fb8691d, after #2430, #2442, #2444, #2433, #2449 and #2434 landed during the merge): prometheus, etcd and harbor change exactly the same edges, edge for edge. Only the base totals moved: prometheus 97,315, etcd 63,177, harbor 98,475.Every changed edge was triaged at its site. A site was matched across arms by caller, line and column, and the declaration of the receiver was read where the receiver mattered.
zap.Error(err)→zapRaftLogger::Error, read through another selector on the line.digest.Digest→Suite::Digest, andconfig.ReadOnly(r), a func-typed field of the parameterconfig Configthat was linked tolib/config'sReadOnly.cmp.Diff→Differ::Diff;gomega.Expect→GomegaInstance::Expect;klog.Infof/klog.Errorf→klogWrapper;fwk.EnqueueExtensions()onfwk, err := newFramework(…)and 2cache.Store(false)oncache := &atomic.Bool{}, which had gone to the imports' interfaces;client.Doonclient := &http.Client{…}→HTTPClient::Do;server.Start()onserver := httptest.NewUnstartedServer(…)→ a kubeletserver::Start.rest rest.Interface,meta metav1.Object,clientset clientset.Interface,watch watch.Interface,version *version.Version,apps appsv1.AppsV1Interface,cadvisor cadvisor.Interface. On main these were receiver-word guesses (0.65 to 0.85) into unrelated packages, likeRESTClient::Patchor kubelet'sVersion::String.logger.Error/Infof/Debugfonlogger logger.Interfacenow reachInterface::*instead of the package's function.storage.Appenderonstorage *teststorage.TestStorage→DB::Appender, through the embedded*tsdb.DB;labels.String→ model/labels'Labels::Stringinstead of prompb's.client.Compact/client.Txnonclient *client.RecordingClient.logger.*incopy.go(logger := log.GetLogger(ctx), whose type nothing reads), and prometheus'sstorage := promqltest.LoadedStorage(…). They went from the import's function or interface to a guessedLoggerorStoragemethod.reg reg.Client,repository *model.RepoRecordand an allowlist local.config.UnmarshalYAMLonvar config SDConfig).jwt.assign/jwt.info(correct);hcn.*onhcn := (proxier.hcn).(*fakehcn.HcnMock)andstatus.FindContainerStatusByName(correct);status.GetInfoandclock.Nowonclock := testKubelet.fakeClock(wrong);api.QueryRange→ the project'squeryRangeAPIinterface, onapi, err := newAPI(…), which returns client_golang'sv1.API(a guess).http.Error(w, err.Error(), …),zap.String("id", c.cid.String())).Correct, 113: etcd's 102 two-hop field chains (0.85, e.g.
c.cid.String()on atypes.IDfield) and 10 typed calls, plus prometheus'q.stmt.Stringchain.Plausible, 20: etcd's 18
id.String()ontypes.IDparameters →ID::String, and 2reqStringer.String.Wrong, 196: guesses for a standard method name.
X.Error()→notRegisteredErr::Errorby a shared word, incl.maxinflight.go:53from the report, and 2 by a capitalized receiver.err.Error()and the like →ParseErr::Error.err.Error()→ErrKeepAliveHalted::Error, and 9String()calls → a protobuf request'sStringby a shared word, likeleaderID→MoveLeaderRequest.This is an existing guess class, not a new one. On main, kubernetes already has 2,374 guesses for a standard method name (1,995 for
Error), prometheus 193 and etcd 323. Main skipped these sites only because of the misreading this PR fixes. Follow-up below.Performance
goRefQualificationitself got cheaper, since a dotted ref no longer builds two regexes from its line.Limits and follow-ups (not in this PR)
Error()/String(). Strategy 3 accepts a project method of a standard name when the receiver shares a word with its owner, soerrmatchesnotRegisteredErrorParseErr. It also guesses through a receiver declared with a predeclared type (err erroratmaxinflight.go:53), because the typed path stops only at an undeclared capitalized type. Fixing it changes thousands of edges on main, so it deserves its own A/B.isBuiltInOrExternalskips any dotted Go call whose head is inGO_STDLIB_PACKAGES(log,user,token,parser,scanner,context, …) before resolution, whether or not it is a local. Examples: gin'scontext.AbortWithErroron acontext *Contextparameter, and harbor'suser.Username. The scope reader isn't consulted there. fix(go): a project package named like a standard-library package is not the standard library #2368 (@danusha2345, open) lets project packages named like the standard library through. That composes with this change but doesn't cover locals. Un-skipping locals also exposes calls on locals of standard-library types (scanner.Text(),url.String()) to the guess above, so it needs the guess fix first.c clock.PassiveClock,pod *v1.Pod) unless the parameter is named like an import, which this PR covers. Doing it for every receiver is a much wider change.:=locals whose value comes from a project function or a field (newAPI(…),testKubelet.fakeClock) keep the usual guesses.Overlap
No open PR touches these functions. #2368 edits
isBuiltInOrExternalinsrc/resolution/index.ts, with no textual overlap; see the follow-ups.🤖 Generated with Claude Code