mirror of
https://github.com/giancarloerra/socraticode.git
synced 2026-07-03 14:05:21 +02:00
fix(graph): allow Go resolution for projects with golang.org/* module paths
Address CodeRabbit review on PR #48. The early `isExternalModule` check in resolveImport was filtering out any import starting with `golang.org/` before the Go case had a chance to match it against the local module path. This blocked legitimate local imports for any project whose own module path starts with `golang.org/` (the Go team's own packages like golang.org/x/sync, golang.org/x/net, etc., where each one's go.mod declares `module golang.org/x/<name>`). Skip the early external check for Go specifically. The Go case in resolveImport already does its own module-path-aware classification and returns null for everything outside the local module, including stdlib and third-party deps. No regression in those cases. New regression test asserts that `module golang.org/x/custom` + `import "golang.org/x/custom/internal"` resolves to the local internal/ package. Confirmed the test fails without the fix and passes with it. Total: 752 unit tests pass. Co-authored-by: mrsuit92 <mrsuit92@users.noreply.github.com>
This commit is contained in:
@@ -215,8 +215,14 @@ export function resolveImport(
|
||||
csNamespaceMap?: Map<string, string[]>,
|
||||
goModuleInfo?: GoModuleInfo | null,
|
||||
): string | null {
|
||||
// Skip obvious external/stdlib modules
|
||||
if (isExternalModule(moduleSpecifier, language)) return null;
|
||||
// Skip obvious external/stdlib modules. Go is excluded from this
|
||||
// pre-check because its external classifier in `isExternalModule`
|
||||
// treats any `golang.org/...` import as external, which would block
|
||||
// valid local imports for projects whose own module path starts with
|
||||
// `golang.org/` (e.g. someone working on `golang.org/x/sync` itself).
|
||||
// The Go case below performs its own module-path-aware classification
|
||||
// and returns null for everything outside the local module.
|
||||
if (language !== "go" && isExternalModule(moduleSpecifier, language)) return null;
|
||||
|
||||
const sourceDir = path.dirname(sourceFile);
|
||||
|
||||
@@ -293,9 +299,10 @@ export function resolveImport(
|
||||
// (built by buildGoModuleInfo at graph-build time). When the import
|
||||
// starts with that prefix, strip it to get the package's directory
|
||||
// relative to the project root, then look up the representative
|
||||
// file for that directory. Imports outside the module path are
|
||||
// external dependencies (or stdlib already filtered upstream by
|
||||
// isExternalModule) and resolve to null.
|
||||
// file for that directory. Anything else (stdlib like "fmt",
|
||||
// third-party packages like "github.com/x/y", or sibling-module
|
||||
// paths that share a prefix textually but not structurally)
|
||||
// resolves to null.
|
||||
if (!goModuleInfo) return null;
|
||||
if (!moduleSpecifier.startsWith(goModuleInfo.modulePath)) return null;
|
||||
const rest = moduleSpecifier.slice(goModuleInfo.modulePath.length);
|
||||
|
||||
@@ -938,6 +938,34 @@ describe("graph-resolution", () => {
|
||||
expect(result).toBe("pkg/alpha.go");
|
||||
});
|
||||
|
||||
it("resolves local imports when the module path starts with golang.org/", () => {
|
||||
// Real-world case: someone working ON one of the Go-team packages
|
||||
// like golang.org/x/sync. Their go.mod declares
|
||||
// `module golang.org/x/sync` and internal subpackages must resolve
|
||||
// locally, even though isExternalModule treats `golang.org/...`
|
||||
// imports as external for non-local-module projects.
|
||||
project = createTempProject({
|
||||
"go.mod": "module golang.org/x/custom\n\ngo 1.21\n",
|
||||
"main.go": "",
|
||||
"internal/foo.go": "",
|
||||
});
|
||||
const goInfo = buildGoModuleInfo(project.fileSet, project.root);
|
||||
|
||||
const result = resolveImport(
|
||||
"golang.org/x/custom/internal",
|
||||
path.join(project.root, "main.go"),
|
||||
project.root,
|
||||
project.fileSet,
|
||||
"go",
|
||||
undefined,
|
||||
undefined,
|
||||
undefined,
|
||||
goInfo,
|
||||
);
|
||||
|
||||
expect(result).toBe("internal/foo.go");
|
||||
});
|
||||
|
||||
it("returns null for a similar-prefix import that is not a subpackage", () => {
|
||||
// `example.com/myapp-other/pkg` shares the prefix `example.com/myapp`
|
||||
// textually but is a separate module. Must not resolve to anything
|
||||
|
||||
Reference in New Issue
Block a user