Repository navigation
fix(go): a comment in an import declaration is not part of an import - #2374
Closed
danusha2345 wants to merge 1 commit into
Closed
danusha2345 wants to merge 1 commit into
danusha2345 wants to merge 1 commit into
Conversation
The Go import reader ran its regexes over the raw file text, so inside
import (
_ "example.com/app/lib/cache/memory" // memory cache
"example.com/app/lib/config"
)
the last word of the trailing comment was read as the alias of the import
on the next line: the file bound `cache` to `lib/config`, and every
`config.X()` in it lost its target or landed on a same-named method of
another package. A quoted word in a comment (a commented-out import, an
`import "x"` usage example in a doc comment) became an import of its own,
and a `)` in a comment ended the import block early.
Comments are now blanked before the import regexes run; string contents are
kept, so a `//` inside a quoted path is still part of the path.
Co-Authored-By: Claude Opus 5.5 <noreply@anthropic.com>
danusha2345
force-pushed
the
fix/go-import-comment-not-an-import
branch
from
October 6, 2026 11:15
5f246e9 to
ca09b91
Compare
danusha2345
pushed a commit
to danusha2345/codegraph
that referenced
this pull request
Oct 6, 2026
danusha2345
pushed a commit
to danusha2345/codegraph
that referenced
this pull request
Oct 6, 2026
…mport comments) into fork main
Contributor
Author
|
Closing in favour of #2410, which landed the same fix and goes further. #2410 blanks comments before the Go import regexes run, reads only the import section (so an Checked on main at ed199e6: this PR's Thanks for carrying it through, and for the credit in the changelog entry. |
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
The Go import reader runs its regexes over the raw file text, comments included. In
the alias pattern
(\w+)\s+"matches across the line break, so the last word of the trailing comment becomes the alias of the import on the next line: the file bindscachetolib/configand has noconfigat all. Everyconfig.X()in that file then loses its target, or lands on a same-named method of another package.Two more shapes come from the same cause:
// "example.com/app/pkg/legacy/store"shadows the realstoreimport below it, and animport "x"usage example in a doc comment is taken for an import;)in a comment ("example.com/app/lib/a" // (legacy)) ends the import block early, so the rest of the block is not read.Fix
Comments are blanked (with the existing Go comment stripper, which keeps string contents and offsets) before the import regexes run. A
//inside a quoted path is therefore still part of the path. Import mappings are computed at resolution time in TypeScript only, so the native and WASM extraction paths are unaffected and behave the same.Out of scope
/vNimport paths.example.com/errors/v2still gets the local namev2. I tried the usual "a trailing major-version element is not the package name" rule and dropped it: it is not safe without knowing whether the path is a module path. harbor has five project directories namedv1/v2whose package clause really ispackage v1/package v2(pkg/scan/rest/v1,pkg/token/claims/v2, …), and one file imports such a package without an alias; with the rule, itsv1.Artifact{…}lost a correct edge. Telling the two apart needs the module table (go.modmodule andrequirelines) at the point where the name is derived, which is a separate change.gopkg.in/yaml.v3is likewise unchanged.import `x/y`) were not read before and are not read now.import . "x/y") yields no mapping, as before; inside a block it yields one under the package name, as before.Tests
New
__tests__/go-import-comments.test.ts://and/* */comments, a comment between an alias and its path, comment lines with quoted words, a commented-out import, a)in a comment, a//inside the quoted path, Windows line endings, the cgo preamble aboveimport "C", and the other import forms (single line, alias, blank, dot, several declarations in one file);config.Load()after two commented blank imports, with a same-named method elsewhere), and a call through an import that sits under a commented-out import of a same-named package.8 of the 10 cases fail on
mainand all pass with the change, with the native kernel and withCODEGRAPH_KERNEL=0. The other Go suites and__tests__/resolution.test.tspass in both modes.Measured on goharbor/harbor
src/at f25e9da, 1,595 Go files, 38,243 nodes before and after.cache,driver,only,function,staticcheck) get their own name back, and 1 commented-out import is no longer listed.calls25,103 → 25,117,instantiates6,185 → 6,186,importsandreferencesunchanged.config.X()calls incore/main.go,metadata.Instance()in a test, and amodels.PostGreSQL{…}literal.config.Load(ctx)incore/main.goused to land on the methodConfigStore.Loadand now reaches the functionLoadoflib/config.The file from the original finding,
controller/scanner/base_controller.go, imports harbor's ownlib/errorsafter a commented blank import. Itserrors.X()calls additionally need #2368 (a project package named like a standard-library package); with both changes together the same run gains 42 edges, 26 of them from that file intolib/errors, and loses only the retargeted one above.Four other Go projects (10 to 99 Go files) show no change in import mappings or edges.
🤖 Generated with Claude Code