Skip to content
Merged
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 @@ -42,6 +42,7 @@ and adheres to [Semantic Versioning](https://semver.org/spec/v2.0.0.html).
- In Go, a type in a parameter, a result or a composite literal now always links to a type, never to a method or function that shares its name. Before, an unexported type like `keyIndex` in etcd's `func (ti *treeIndex) KeyIndex(keyi *keyIndex) *keyIndex` linked to the `keyIndex` method declared beside it, prometheus's `samples{…}` literals to a `samples` method, and a type from a package outside your project, like `config.URL` in `&config.URL{…}`, to whatever project method had that name. A type written without a package, including an unexported one or one named like `AppenderV2` or `Histogram_CountInt`, now links to its own package's type when there is one, and a generic method's type parameter, like `T` in `func (p *Pool[T]) Get() T`, no longer links to an unrelated `T`. Re-index Go projects after upgrading.
- In Go, a type declared on its own, like `type Point struct{…}` under a `// Point is …` comment, now keeps that comment as its documentation, as a type inside a `type ( … )` group already did, so searching for what a type does can find it. A comment at the end of a line of code, like `const sides = 4 // sides of a square.`, is no longer taken as the documentation of the declaration under it. Re-index Go projects after upgrading.
- In Go, a declaration's documentation is now just the comment written right above it, as `go doc` shows it. A section banner, a TODO or commented-out code that a blank line separates from a function, type, constant or variable is no longer added to the front of its documentation, or taken as its documentation when it has none of its own. Re-index Go projects after upgrading.
- In Go, a comment directive written above a declaration, like `//go:embed static`, `//go:generate stringer -type=Kind` or `//nolint:errcheck`, is no longer part of its documentation, as `go doc` shows it, so a variable whose only comment is a `//go:embed` line no longer has that line as its description. Re-index Go projects after upgrading.
- 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)
- Indexing a project that ships a large minified JavaScript file, such as the GraphiQL bundle inside go-ethereum, no longer stalls at "Resolving refs" when CodeGraph runs on Node 22, and linking such a file is faster on any Node version. The graph it builds is unchanged.
- The time `codegraph init` and `codegraph index` print beside the node and edge counts now covers the whole run, resolving references and linking included, and `codegraph sync` reports its whole run the same way. Before, it counted only reading and parsing the files, which can be a small part of an index, so a slow index looked fast. Thanks @bompus for the report. (#2334)
Expand Down
2 changes: 2 additions & 0 deletions __tests__/fixtures/kernel-parity/torture.go
Original file line number Diff line number Diff line change
Expand Up @@ -172,6 +172,8 @@ func shadowed() {
fmt.Println(MAX_ITEMS)
}

// reads is documented above a directive, which is no part of its doc.
//go:noinline
func reads() int {
return MAX_ITEMS
}
177 changes: 177 additions & 0 deletions __tests__/go-doc-directives.test.ts
Original file line number Diff line number Diff line change
@@ -0,0 +1,177 @@
/**
* A comment directive Go tools read, like `//go:embed testdata` above a
* variable or `//nolint:unparam` above a function, was stored as part of the
* declaration's docstring, or as the whole of it:
*
* //go:embed testdata
* var testsFs embed.FS
*
* gave prometheus's `testsFs` the docstring "go:embed testdata". go doc leaves
* directives out of a doc comment's text (go/ast's isDirective): a `//`
* comment with no space after the slashes that is `//line `, `//extern ` or
* `//export ` followed by text, or a lowercase word and a colon. The comments
* around a directive still form one doc comment with it.
*
* Runs against the native kernel (when built) and the wasm extractor, which
* must agree.
*/
import { describe, it, expect, beforeAll, beforeEach, afterEach } from 'vitest';
import * as fs from 'fs';
import * as path from 'path';
import { extractFromSource } from '../src/extraction';
import { initGrammars, loadGrammarsForLanguages } from '../src/extraction/grammars';
import { tryKernelExtract, resetKernelForTests } from '../src/extraction/kernel';
import type { ExtractionResult } from '../src/types';

const KERNEL_PATH = path.join(
__dirname,
'..',
'codegraph-kernel',
'prebuilds',
`${process.platform}-${process.arch}`,
'codegraph-kernel.node'
);
const kernelAvailable = fs.existsSync(KERNEL_PATH) || process.env.CODEGRAPH_KERNEL_EXPECT === '1';

const SOURCE = `// Package shapes is documented.
package shapes

import "embed"

//go:embed testdata
var testsFs embed.FS

//nolint:gosec
const secret = "x"

//go:generate stringer -type=Kind
type Kind int

// Area is documented above a directive.
//go:noinline
func Area() int { return 0 }

// Perimeter is documented above a lint directive.
//
//nolint:gocyclo // a long switch
func Perimeter() int { return 0 }

// Join keeps the comments on either side of a directive.
//go:generate stringer -type=Kind
// They are one doc comment.
func Join() {}

//revive:disable:var-naming
func JsonStats() {}

// Inline is documented a blank line above its directive.

//go:noinline
func Inline() {}

//line shapes.go:1
func Line() {}

//export Exported
func Exported() {}

// nolint:errcheck is not a directive when a space follows the slashes.
func Spaced() {}

//TODO: an uppercase word before the colon is not a directive.
func Todo() {}

//https://go.dev/doc/comment is not a directive either.
func Link() {}

/*nolint:all*/
func Block() {}
`;

const ENV_KEYS = ['CODEGRAPH_KERNEL', 'CODEGRAPH_KERNEL_LANGS'] as const;

describe('Go doc comments leave out comment directives', () => {
let savedEnv: Record<string, string | undefined> = {};

beforeAll(async () => {
await initGrammars();
await loadGrammarsForLanguages(['go']);
});

beforeEach(() => {
savedEnv = Object.fromEntries(ENV_KEYS.map((k) => [k, process.env[k]]));
resetKernelForTests();
});

afterEach(() => {
for (const k of ENV_KEYS) {
if (savedEnv[k] === undefined) delete process.env[k];
else process.env[k] = savedEnv[k];
}
resetKernelForTests();
});

function extract(backend: 'kernel' | 'wasm', source: string): ExtractionResult {
if (backend === 'wasm') {
process.env.CODEGRAPH_KERNEL = '0';
return extractFromSource('shapes/shapes.go', source, 'go');
}
delete process.env.CODEGRAPH_KERNEL;
process.env.CODEGRAPH_KERNEL_LANGS = 'all';
const result = tryKernelExtract('shapes/shapes.go', source, 'go');
expect(result, 'kernel extraction').not.toBeNull();
return result!;
}

const backends = kernelAvailable ? (['kernel', 'wasm'] as const) : (['wasm'] as const);

for (const crlf of [false, true]) {
it.each(backends)(`%s${crlf ? ' (CRLF)' : ''}`, (backend) => {
const result = extract(backend, crlf ? SOURCE.replace(/\n/g, '\r\n') : SOURCE);
const doc = (kind: string, name: string): string | null => {
const found = result.nodes.filter((n) => n.kind === kind && n.name === name);
expect(found, `${kind} ${name}`).toHaveLength(1);
return found[0].docstring ?? null;
};

expect({
// A declaration whose only comment is a directive has no doc.
testsFs: doc('variable', 'testsFs'),
secret: doc('constant', 'secret'),
Kind: doc('type_alias', 'Kind'),
JsonStats: doc('function', 'JsonStats'),
Line: doc('function', 'Line'),
Exported: doc('function', 'Exported'),
// A directive inside a doc comment is left out of it, and the
// comments around it stay one doc comment.
Area: doc('function', 'Area'),
Perimeter: doc('function', 'Perimeter'),
Join: doc('function', 'Join'),
// A directive still decides where the doc comment is: one a blank
// line separates from the declaration documents nothing.
Inline: doc('function', 'Inline'),
// A space after the slashes, an uppercase word, a colon not followed
// by a lowercase letter or digit, or a block comment: no directive.
Spaced: doc('function', 'Spaced'),
Todo: doc('function', 'Todo'),
Link: doc('function', 'Link'),
Block: doc('function', 'Block'),
}).toEqual({
testsFs: null,
secret: null,
Kind: null,
JsonStats: null,
Line: null,
Exported: null,
Area: 'Area is documented above a directive.',
Perimeter: 'Perimeter is documented above a lint directive.',
Join: 'Join keeps the comments on either side of a directive.\nThey are one doc comment.',
Inline: null,
Spaced: 'nolint:errcheck is not a directive when a space follows the slashes.',
Todo: 'TODO: an uppercase word before the colon is not a directive.',
Link: 'https://go.dev/doc/comment is not a directive either.',
Block: 'nolint:all',
});
});
}
});
94 changes: 79 additions & 15 deletions codegraph-kernel/src/docstring.rs
Original file line number Diff line number Diff line change
Expand Up @@ -143,19 +143,21 @@ pub fn preceding_docstring_stepping_over(
src: &str,
step_over: &[&str],
) -> Option<String> {
preceding_docstring_with(node, src, step_over, false, false)
preceding_docstring_with(node, src, step_over, false, false, false)
}

/// getPrecedingDocstring's `skipTrailing` and `skipDetached` (Go). The
/// comments the run opens with that belong to the line above are left out:
/// one written after code on its line (`const n = 4 // four.`), and any that
/// begins on the line such a comment ends. Of the rest, only the comments that
/// end on the line right above the declaration, together with the ones written
/// on the lines just before them, are kept: a blank line ends the docstring,
/// and a comment that ends on the declaration's own line documents nothing.
/// That is Go's doc comment, the one go doc shows.
pub fn preceding_docstring_skipping_trailing_and_detached(node: Node, src: &str) -> Option<String> {
preceding_docstring_with(node, src, &[], true, true)
/// getPrecedingDocstring's `skipTrailing`, `skipDetached` and `skipDirectives`,
/// which Go turns on together for the doc comment go doc shows. The comments
/// the run opens with that belong to the line above are left out: one written
/// after code on its line (`const n = 4 // four.`), and any that begins on the
/// line such a comment ends. Of the rest, only the comments that end on the
/// line right above the declaration, together with the ones written on the
/// lines just before them, are kept: a blank line ends the docstring, and a
/// comment that ends on the declaration's own line documents nothing. A
/// directive among them (`//go:embed static`, `//nolint:errcheck`) is left out
/// of the text.
pub fn preceding_go_doc_comment(node: Node, src: &str) -> Option<String> {
preceding_docstring_with(node, src, &[], true, true, true)
}

fn preceding_docstring_with(
Expand All @@ -164,6 +166,7 @@ fn preceding_docstring_with(
step_over: &[&str],
skip_trailing: bool,
skip_detached: bool,
skip_directives: bool,
) -> Option<String> {
let mut anchor = node;
while let Some(parent) = anchor.parent() {
Expand Down Expand Up @@ -219,11 +222,16 @@ fn preceding_docstring_with(
}
first = start;
}
if first == comments.len() {
let doc_comments: Vec<Node> = comments[first..]
.iter()
.copied()
.filter(|c| !skip_directives || !is_directive(&src[c.byte_range()]))
.collect();
if doc_comments.is_empty() {
return None;
}
Some(
comments[first..]
doc_comments
.iter()
.map(|c| clean_comment_markers(&src[c.byte_range()]))
.collect::<Vec<_>>()
Expand All @@ -233,6 +241,24 @@ fn preceding_docstring_with(
)
}

/// isDirective (tree-sitter-helpers.ts) — a comment directive Go tools read,
/// which go doc leaves out of a doc comment's text (go/ast's isDirective): a
/// `//` comment with no space after the slashes that is `//line `, `//extern `
/// or `//export ` followed by text, or that opens with a lowercase word, a
/// colon and a lowercase letter or digit (`//go:embed x`, `//nolint:errcheck`).
fn is_directive(comment: &str) -> bool {
let Some(text) = comment.strip_prefix("//") else {
return false;
};
if text.starts_with("line ") || text.starts_with("extern ") || text.starts_with("export ") {
return true;
}
let word_char = |b: &u8| b.is_ascii_lowercase() || b.is_ascii_digit();
let bytes = text.as_bytes();
let word = bytes.iter().take_while(|b| word_char(b)).count();
word > 0 && bytes.get(word) == Some(&b':') && bytes.get(word + 1).is_some_and(word_char)
}

/// followsCodeOnItsLine (tree-sitter-helpers.ts) — whether code comes before
/// `node` on the line it starts on.
fn follows_code_on_its_line(node: Node, src: &str) -> bool {
Expand Down Expand Up @@ -313,7 +339,7 @@ mod tests {
})
.expect(name)
};
let skipping_trailing = |n| preceding_docstring_with(n, src, &[], true, false);
let skipping_trailing = |n| preceding_docstring_with(n, src, &[], true, false, false);
assert_eq!(preceding_docstring(func("f"), src).as_deref(), Some("a\nb\nc"));
assert_eq!(skipping_trailing(func("f")).as_deref(), Some("c"));
assert_eq!(preceding_docstring(func("g"), src).as_deref(), Some("d"));
Expand Down Expand Up @@ -341,12 +367,50 @@ mod tests {
})
.expect(name)
};
let skipping_detached = |n| preceding_docstring_with(n, src, &[], false, true);
let skipping_detached = |n| preceding_docstring_with(n, src, &[], false, true, false);
assert_eq!(preceding_docstring(func("f"), src).as_deref(), Some("a\nb\nc\nd"));
assert_eq!(skipping_detached(func("f")).as_deref(), Some("b\nc\nd"));
assert_eq!(preceding_docstring(func("g"), src).as_deref(), Some("e"));
assert_eq!(skipping_detached(func("g")), None);
assert_eq!(preceding_docstring(func("h"), src).as_deref(), Some("h"));
assert_eq!(skipping_detached(func("h")), None);
}

/// The directive shapes go/ast leaves out of a doc comment's text, and
/// the near misses it keeps.
#[test]
fn recognizes_go_directives() {
for directive in ["//go:embed x", "//nolint:errcheck", "//a1:b", "//line f.go:1", "//extern f", "//export F"] {
assert!(is_directive(directive), "{directive}");
}
for comment in ["// nolint:errcheck", "//TODO: x", "//https://go.dev", "//go:", "//:x", "//line", "/*go:embed x*/", "//go-x:y"] {
assert!(!is_directive(comment), "{comment}");
}
}

/// Skipping directives leaves them out of the docstring's text but not
/// out of the comment run: one beside a doc comment drops out of it, and a
/// declaration whose only comment is one has no docstring.
#[test]
fn skips_directives_in_the_text_only() {
let grammar = crate::langs::grammar_for("go").expect("go grammar");
let mut parser = tree_sitter::Parser::new();
parser.set_language(&grammar).unwrap();
let src = "package p\n\n// a\n//go:noinline\n// b\nfunc f() {}\n\n//nolint:unparam\nfunc g() {}\n";
let tree = parser.parse(src, None).unwrap();
let root = tree.root_node();
let func = |name: &str| {
(0..root.named_child_count())
.filter_map(|i| root.named_child(i))
.find(|n| {
n.kind() == "function_declaration"
&& n.child_by_field_name("name").map(|id| &src[id.byte_range()]) == Some(name)
})
.expect(name)
};
assert_eq!(preceding_docstring(func("f"), src).as_deref(), Some("a\ngo:noinline\nb"));
assert_eq!(preceding_go_doc_comment(func("f"), src).as_deref(), Some("a\nb"));
assert_eq!(preceding_docstring(func("g"), src).as_deref(), Some("nolint:unparam"));
assert_eq!(preceding_go_doc_comment(func("g"), src), None);
}
}
10 changes: 4 additions & 6 deletions codegraph-kernel/src/go.rs
Original file line number Diff line number Diff line change
Expand Up @@ -16,7 +16,7 @@ use crate::buffers::{
build_meta, edge_kind_index, node_kind_index, Arena, BoolFlags, EdgeRow, EmitOut, NodeRow,
RefRow, StrRef, Tables, FLAG_IS_EXPORTED, FUNCTION_REF_CODE, NONE, NONE_STR,
};
use crate::docstring::preceding_docstring_skipping_trailing_and_detached;
use crate::docstring::preceding_go_doc_comment;
use crate::ids;
use crate::textutil as util;
use regex::Regex;
Expand Down Expand Up @@ -402,12 +402,10 @@ impl<'t> Walker<'t> {
/// from the declaration wrapper when there is one, without the comments
/// that trail the code above it (docstringSkipsTrailingComments) or that
/// don't end on the line right above it, past a blank line
/// (docstringSkipsDetachedComments).
/// (docstringSkipsDetachedComments), and without directives like
/// `//go:embed` in its text (docstringSkipsDirectives).
fn docstring_of(&self, node: Node<'t>) -> Option<String> {
preceding_docstring_skipping_trailing_and_detached(
self.declaration_wrapper(node).unwrap_or(node),
self.src,
)
preceding_go_doc_comment(self.declaration_wrapper(node).unwrap_or(node), self.src)
}

// --- visitNode ------------------------------------------------------------
Expand Down
3 changes: 3 additions & 0 deletions src/extraction/languages/go.ts
Original file line number Diff line number Diff line change
Expand Up @@ -261,4 +261,7 @@ export const goExtractor: LanguageExtractor = {
// line ends it, as go doc reads it: a banner or a note above that blank line
// documents nothing.
docstringSkipsDetachedComments: true,
// A directive Go tools read, like `//go:embed static` or
// `//nolint:errcheck`, is not part of the doc comment go doc shows.
docstringSkipsDirectives: true,
};
Loading