From f43432c86ad2ba578c11acf95fcbc6ab6ed4fc5d Mon Sep 17 00:00:00 2001 From: Colby Mchenry Date: Mon, 19 Oct 2026 20:45:40 +0000 Subject: [PATCH] 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 --- CHANGELOG.md | 1 + __tests__/fixtures/kernel-parity/torture.go | 2 + __tests__/go-doc-directives.test.ts | 177 ++++++++++++++++++++ codegraph-kernel/src/docstring.rs | 94 +++++++++-- codegraph-kernel/src/go.rs | 10 +- src/extraction/languages/go.ts | 3 + src/extraction/tree-sitter-helpers.ts | 29 +++- src/extraction/tree-sitter-types.ts | 6 + src/extraction/tree-sitter.ts | 3 +- 9 files changed, 299 insertions(+), 26 deletions(-) create mode 100644 __tests__/go-doc-directives.test.ts diff --git a/CHANGELOG.md b/CHANGELOG.md index b07bf3dbb..a9d902424 100644 --- a/CHANGELOG.md +++ b/CHANGELOG.md @@ -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) diff --git a/__tests__/fixtures/kernel-parity/torture.go b/__tests__/fixtures/kernel-parity/torture.go index b28b54eb2..115a1bdfc 100644 --- a/__tests__/fixtures/kernel-parity/torture.go +++ b/__tests__/fixtures/kernel-parity/torture.go @@ -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 } diff --git a/__tests__/go-doc-directives.test.ts b/__tests__/go-doc-directives.test.ts new file mode 100644 index 000000000..404797dda --- /dev/null +++ b/__tests__/go-doc-directives.test.ts @@ -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 = {}; + + 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', + }); + }); + } +}); diff --git a/codegraph-kernel/src/docstring.rs b/codegraph-kernel/src/docstring.rs index 9a9ee520d..85863d175 100644 --- a/codegraph-kernel/src/docstring.rs +++ b/codegraph-kernel/src/docstring.rs @@ -143,19 +143,21 @@ pub fn preceding_docstring_stepping_over( src: &str, step_over: &[&str], ) -> Option { - 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 { - 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 { + 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 { 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 = 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::>() @@ -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); + } } diff --git a/codegraph-kernel/src/go.rs b/codegraph-kernel/src/go.rs index b1ff1b613..b0e0d1cce 100644 --- a/codegraph-kernel/src/go.rs +++ b/codegraph-kernel/src/go.rs @@ -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 { - 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 ------------------------------------------------------------ diff --git a/src/extraction/languages/go.ts b/src/extraction/languages/go.ts index 138c4938c..7dae97d16 100644 --- a/src/extraction/languages/go.ts +++ b/src/extraction/languages/go.ts @@ -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, }; diff --git a/src/extraction/tree-sitter-helpers.ts b/src/extraction/tree-sitter-helpers.ts index 605f142e6..37a2d8dc4 100644 --- a/src/extraction/tree-sitter-helpers.ts +++ b/src/extraction/tree-sitter-helpers.ts @@ -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; diff --git a/src/extraction/tree-sitter-types.ts b/src/extraction/tree-sitter-types.ts index 22625081e..691ac0f1f 100644 --- a/src/extraction/tree-sitter-types.ts +++ b/src/extraction/tree-sitter-types.ts @@ -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 diff --git a/src/extraction/tree-sitter.ts b/src/extraction/tree-sitter.ts index 035dcdab1..61abda9a7 100644 --- a/src/extraction/tree-sitter.ts +++ b/src/extraction/tree-sitter.ts @@ -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}`;