Skip to content
Closed
Show file tree
Hide file tree
Changes from all commits
Commits
File filter

Filter by extension

Filter by extension

Conversations
Failed to load comments.
Loading
Jump to
Jump to file
Failed to load files.
Loading
Diff view
Diff view
1 change: 1 addition & 0 deletions CHANGELOG.md
Original file line number Diff line number Diff line change
Expand Up @@ -33,6 +33,7 @@ and adheres to [Semantic Versioning](https://semver.org/spec/v2.0.0.html).
- In COBOL, asking `codegraph_explore` (or `codegraph explore` and `codegraph context`) about a copybook by name, such as `CVACT01Y` or a member brought in with `EXEC SQL INCLUDE`, now answers with the copybook's own source when it is indexed and every `COPY` or `EXEC SQL INCLUDE` statement that includes it, and says so when the copybook's source isn't part of the project. Before, it answered "No relevant code found", or showed an unrelated program whose name was a letter or two away. Thanks @popolusiak for the report. (#2342)
- In Dart, reading a getter such as `box.area` or `status.label` now counts as calling it, so `codegraph callers` and impact list the code that reads it — whether the getter is declared on the class, inherited, or added by an extension — and a method an extension adds to an enum, like `shape.shout()`, now links as well. A plain field, a getter of a type from outside your project, or a value whose type isn't written down still links nothing. Re-index Dart projects after upgrading. Thanks @brookly255-lv for the report. (#2338)
- In Dart, a type is now linked from more of the places it is written, not only from parameters and return types: the type an `extension … on` targets, a field's type, a top-level variable's type, generic arguments such as `Future<Report?>.value(null)` or Riverpod's `final reportProvider = Family<Report?, String>()`, and a local variable's type, a cast or a type check inside a function. So `callers` and impact for a model class now reach the extensions, models and providers that use it. Thanks @mg-mg-mg for the report. (#2327)
- In Go, a comment inside an import block is no longer read as part of an import. A comment at the end of an import line, like `_ "example.com/app/cache/memory" // memory cache`, used to give its last word to the import on the next line as that import's name, so every call through the next package in that file, like `config.Load()`, lost its target or landed on a same-named method elsewhere. A commented-out import is no longer treated as imported, and a `)` in a comment no longer cuts the import block short. Re-index Go projects after upgrading. (#2374)
- In Go, calls through a method receiver or parameter of an unexported type, the usual shape of gRPC and HTTP handlers (`s.service.AddItem()` inside `func (s *server) Create()`), now resolve, and always within that type's own package: another package's `server` with a same-named method no longer takes the call, and a method the type gets from a struct or interface it embeds is found too. Re-index Go projects after upgrading. Thanks @GoDiao for the report and the fix. (#2323)
- In Go, calls into a module whose `go.mod` is not at the project root now resolve, whether it is a `server/` backend next to a `web/` frontend or one of several modules side by side as in etcd, so calls like `store.New()` and `s.db.CreateItem()` find their targets instead of being treated as calls into a third-party package. A name written through a package, like a `job.OPCommand` result type or a field of type `artifact.Manager`, now links to that package's symbol rather than a same-named one elsewhere, which also corrects links in projects with a single `go.mod`. Re-index Go projects after upgrading. Thanks @GoDiao for the report and @danusha2345 for the fix. (#2322)
- Indexing a project that includes large bundled JavaScript files, such as a copy of pdf.js or d3, is fast again: since 1.6.2, resolving the calls in a JavaScript or TypeScript file re-read the file's text above each call, so a single bundled library could add many seconds to an index. The graph it builds is unchanged. Thanks @bompus for the report. (#2334)
Expand Down
224 changes: 224 additions & 0 deletions __tests__/go-import-comments.test.ts
Original file line number Diff line number Diff line change
@@ -0,0 +1,224 @@
/**
* A comment inside a Go import declaration is not part of an import.
*
* harbor's `core/main.go` imports
*
* _ "github.com/goharbor/harbor/src/lib/cache/memory" // memory cache
* _ "github.com/goharbor/harbor/src/lib/cache/redis" // redis cache
* "github.com/goharbor/harbor/src/lib/config"
*
* and 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`, so every
* `config.X()` in it lost its target or landed on a same-named method. A
* quoted word in a comment was likewise read as an import of its own, and a
* `)` in a comment ended the import block early.
*/
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';

/** `localName=source` of every import the reader finds in `source`. */
const imports = (source: string) =>
extractImportMappings('main.go', source, 'go').map((m) => `${m.localName}=${m.source}`);

describe('Go import reader: comments', () => {
it('does not read a trailing line comment as the alias of the next import', () => {
expect(imports(`package main

import (
_ "example.com/app/lib/cache/memory" // memory cache
"example.com/app/lib/errors"
)
`)).toEqual(['_=example.com/app/lib/cache/memory', 'errors=example.com/app/lib/errors']);
});

it('does not read a trailing block comment as an alias', () => {
expect(imports(`package main

import (
"example.com/app/lib/cache" /* the cache */
"example.com/app/lib/errors"
al /* renamed */ "example.com/app/lib/log"
)
`)).toEqual([
'cache=example.com/app/lib/cache',
'errors=example.com/app/lib/errors',
'al=example.com/app/lib/log',
]);
});

it('does not read a quoted word in a comment as an import', () => {
expect(imports(`package main

// Usage:
//
// import "example.com/app/docs"
import (
"example.com/app/lib/a"
// was "example.com/app/lib/old" before the move
/* and "example.com/app/lib/older" before that */
// _ "example.com/app/lib/disabled"
"example.com/app/lib/b"
)
`)).toEqual(['a=example.com/app/lib/a', 'b=example.com/app/lib/b']);
});

it('reads the whole block when a comment contains a closing parenthesis', () => {
expect(imports(`package main

import (
"example.com/app/lib/a" // (legacy)
"example.com/app/lib/b"
)
`)).toEqual(['a=example.com/app/lib/a', 'b=example.com/app/lib/b']);
});

it('keeps a `//` inside the quoted path', () => {
expect(imports(`package main

import (
"example.com//app/lib/a"
b "example.com/app//lib/b" // trailing
)
`)).toEqual(['a=example.com//app/lib/a', 'b=example.com/app//lib/b']);
});

it('handles Windows line endings', () => {
expect(imports(
'package main\r\n\r\nimport (\r\n\t_ "example.com/app/lib/cache/memory" // memory cache\r\n\t"example.com/app/lib/errors"\r\n\tal "example.com/app/lib/log"\r\n)\r\n'
)).toEqual([
'_=example.com/app/lib/cache/memory',
'errors=example.com/app/lib/errors',
'al=example.com/app/lib/log',
]);
});

it('reads the other import forms as before', () => {
expect(imports('package main\n\nimport "example.com/app/lib/a"\n')).toEqual(['a=example.com/app/lib/a']);
expect(imports('package main\n\nimport al "example.com/app/lib/a"\n')).toEqual(['al=example.com/app/lib/a']);
expect(imports('package main\n\nimport _ "example.com/app/lib/a"\n')).toEqual(['_=example.com/app/lib/a']);
// Several declarations in one file, with a dot and a blank import.
expect(imports(`package main

import "example.com/app/lib/a"

import (
"example.com/app/lib/b"
)

import (
c "example.com/app/lib/d"
. "example.com/app/lib/e"
_ "example.com/app/lib/f"
)
`)).toEqual([
'a=example.com/app/lib/a',
'b=example.com/app/lib/b',
'c=example.com/app/lib/d',
'e=example.com/app/lib/e',
'_=example.com/app/lib/f',
]);
});

it('reads `import "C"` under its cgo preamble', () => {
expect(imports(`package main

/*
#include <stdio.h>
#include "bridge.h"
*/
import "C"

// #cgo LDFLAGS: -lm
// import "example.com/app/not/imported"
import "example.com/app/lib/a"
`)).toEqual(['C=C', 'a=example.com/app/lib/a']);
});
});

describe('Go calls through an import that follows a commented one', () => {
let root = '';
let cg: CodeGraph;

beforeAll(async () => {
root = fs.mkdtempSync(path.join(os.tmpdir(), 'cg-go-import-comments-'));
const files: Record<string, string> = {
'go.mod': 'module example.com/app\n\ngo 1.22\n',
'lib/cache/memory/memory.go': 'package memory\n\nfunc init() {}\n',
'lib/cache/redis/redis.go': 'package redis\n\nfunc init() {}\n',
'lib/config/config.go': `package config

func Load() error { return nil }
`,
// A same-named method elsewhere, as harbor's \`ConfigStore.Load\`.
'pkg/store/store.go': `package store

type ConfigStore struct{}

func (s *ConfigStore) Load() error { return nil }

func Open() *ConfigStore { return &ConfigStore{} }
`,
// The package a commented-out import names, with the same function.
'pkg/legacy/store/store.go': `package store

func Open() int { return 0 }
`,
'core/main.go': `package main

import (
_ "example.com/app/lib/cache/memory" // memory cache
_ "example.com/app/lib/cache/redis" // redis cache
"example.com/app/lib/config"
)

func main() {
if err := config.Load(); err != nil {
panic(err)
}
}
`,
'core/open.go': `package main

import (
// "example.com/app/pkg/legacy/store" until the migration (see the old layout)
"example.com/app/pkg/store"
)

func open() {
store.Open()
}
`,
};
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 });
});

/** `file:qualifiedName` of every call leaving the function `fn` of `file`. */
const callsFrom = (file: string, fn: string) => {
const ids = cg.getNodesInFile(file).filter((n) => n.name === fn).map((n) => n.id);
return cg.getOutgoingEdgesFrom(ids, ['calls']).map((e) => {
const target = cg.getNode(e.target)!;
return `${target.filePath}:${target.qualifiedName}`;
});
};

it('resolves a call into the package imported after a commented blank import', () => {
expect(callsFrom('core/main.go', 'main')).toEqual(['lib/config/config.go:Load']);
});

it('resolves a call into the imported package, not the one a comment names', () => {
expect(callsFrom('core/open.go', 'open')).toEqual(['pkg/store/store.go:Open']);
});
});
4 changes: 4 additions & 0 deletions src/resolution/import-resolver.ts
Original file line number Diff line number Diff line change
Expand Up @@ -1122,6 +1122,10 @@ function extractPythonImports(content: string): ImportMapping[] {
*/
function extractGoImports(content: string): ImportMapping[] {
const mappings: ImportMapping[] = [];
// A comment is not part of an import: the last word of `// memory cache`
// was read as the alias of the import on the next line, a quoted word in a
// comment as an import of its own, and a `)` in one ended the block early.
content = stripCommentsForRegex(content, 'go');

// import "path" or import alias "path"
const singleImportRegex = /import\s+(?:(\w+)\s+)?["']([^"']+)["']/g;
Expand Down