fix(go): a comment directive like //go:embed is not part of a declaration's doc (#2679)

go doc leaves comment directives out of a doc comment's text: 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, like //go:embed static or //nolint:errcheck
(go/ast's isDirective). Both extractors kept them, so prometheus's
testsFs had the docstring "go:embed testdata", etcd's store.restore
"nolint:unparam", and a doc comment with a directive under it ended in
that directive.

Go now drops directives from the docstring's text through a Go-only
extractor option (docstringSkipsDirectives in TS; the kernel's Go
docstring helper becomes preceding_go_doc_comment, which applies all
three Go rules). A directive still counts as a comment when deciding
where the doc comment starts, as in go/parser, so a declaration whose
only comment is a directive has no docstring.

Co-Authored-By: Claude Opus 5.5 <noreply@anthropic.com>
This commit is contained in:
Colby Mchenry
2026-10-19 20:45:40 +00:00
committed by GitHub
parent 23f728fac2
commit f43432c86a
9 changed files with 299 additions and 26 deletions
+1
View File
@@ -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)
@@ -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
View File
@@ -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',
});
});
}
});
+79 -15
View File
@@ -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(
@@ -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() {
@@ -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<_>>()
@@ -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 {
@@ -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"));
@@ -341,7 +367,7 @@ 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"));
@@ -349,4 +375,42 @@ mod tests {
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);
}
}
+4 -6
View File
@@ -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;
@@ -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 ------------------------------------------------------------
+3
View File
@@ -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,
};
+25 -4
View File
@@ -145,13 +145,19 @@ function cleanCommentMarkers(comment: string): string {
* 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. Default: off.
*
* `skipDirectives` leaves the comment directives Go tools read, like
* `//go:embed static` or `//nolint:errcheck`, out of the docstring's text, as
* go doc does. They still count as comments when the other options decide
* where the docstring starts. Default: off.
*/
export function getPrecedingDocstring(
node: SyntaxNode,
source: string,
stepOver: readonly string[] = [],
skipTrailing = false,
skipDetached = false
skipDetached = false,
skipDirectives = false
): string | undefined {
// Climb out of any wrapper(s) so a comment preceding the WHOLE construct
// (export-, decorator-, or const-arrow-wrapped) is reachable as a sibling.
@@ -206,16 +212,31 @@ export function getPrecedingDocstring(
}
first = start;
}
if (first === comments.length) return undefined;
const docComments = comments
.slice(first)
.filter((c) => !skipDirectives || !isDirective(getNodeText(c, source)));
if (docComments.length === 0) return undefined;
// Strip each comment's syntax markers (language-aware), then join.
return comments
.slice(first)
return docComments
.map((c) => cleanCommentMarkers(getNodeText(c, source)))
.join('\n')
.trim();
}
/**
* Whether a comment is a 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, like `//go:embed static` or `//nolint:errcheck`.
*/
function isDirective(comment: string): boolean {
if (!comment.startsWith('//')) return false;
const text = comment.slice(2);
return /^(?:line|extern|export) /.test(text) || /^[a-z0-9]+:[a-z0-9]/.test(text);
}
/** Whether code comes before `node` on the line it starts on. */
function followsCodeOnItsLine(node: SyntaxNode, source: string): boolean {
let i = node.startIndex;
+6
View File
@@ -198,6 +198,12 @@ export interface LanguageExtractor {
* declaration documents nothing.
*/
docstringSkipsDetachedComments?: boolean;
/**
* Leave out of a docstring's text the comment directives Go tools read, like
* `//go:embed static`, `//nolint:errcheck` or `//export F`, as go doc does:
* a declaration whose only comment is a directive has no doc.
*/
docstringSkipsDirectives?: boolean;
/**
* Node types that may stand between a declaration and the decorators written
* before it without ending the scan for them. Dart writes comments there
+2 -1
View File
@@ -542,7 +542,8 @@ export class TreeSitterExtractor {
this.source,
this.extractor?.docstringStepOverTypes,
this.extractor?.docstringSkipsTrailingComments,
this.extractor?.docstringSkipsDetachedComments
this.extractor?.docstringSkipsDetachedComments,
this.extractor?.docstringSkipsDirectives
);
const body = this.extractor?.getBodyDocstring?.(node, this.source);
if (preceding && body) return `${preceding}\n\n${body}`;