From 23f728fac20908179af04ca9951b3ad99ff9f59b Mon Sep 17 00:00:00 2001 From: Colby Mchenry Date: Mon, 19 Oct 2026 20:28:12 +0000 Subject: [PATCH] fix(go): a comment a blank line separates from a declaration is not its doc (#2678) Go's doc comment, the one go doc shows (go/parser's lead comment), is the comment group that ends on the line right above a declaration: comments on consecutive lines form one group, and a blank line ends it. Both extractors joined every comment above a Go declaration instead, so a section banner, a TODO or commented-out code above a blank line became part of the declaration's docstring, or its whole docstring when the declaration had no doc of its own. Go now keeps only that group, through a Go-only extractor option beside the trailing-comment one (docstringSkipsDetachedComments in TS, preceding_docstring_skipping_trailing_and_detached in the kernel). A comment that ends on the declaration's own line documents nothing, as in go doc. Co-Authored-By: Claude Opus 5.5 --- CHANGELOG.md | 1 + __tests__/fixtures/kernel-parity/torture.go | 5 + __tests__/go-detached-doc-comments.test.ts | 208 ++++++++++++++++++++ codegraph-kernel/src/docstring.rs | 70 ++++++- codegraph-kernel/src/go.rs | 11 +- src/extraction/languages/go.ts | 4 + src/extraction/tree-sitter-helpers.ts | 22 ++- src/extraction/tree-sitter-types.ts | 8 + src/extraction/tree-sitter.ts | 3 +- 9 files changed, 317 insertions(+), 15 deletions(-) create mode 100644 __tests__/go-detached-doc-comments.test.ts diff --git a/CHANGELOG.md b/CHANGELOG.md index 64a62b086..b07bf3dbb 100644 --- a/CHANGELOG.md +++ b/CHANGELOG.md @@ -41,6 +41,7 @@ and adheres to [Semantic Versioning](https://semver.org/spec/v2.0.0.html). - In Go, a function or method passed as a value, like `sync.Pool{New: wm.new}`, now shows the function that passes it among its callers and impact even when it is named `new`, `nil`, `None`, `self` or a handful of similar words, or when a line break or comment sits beside the dot. Before, such a value was skipped, so a method used only that way looked unused. Re-index Go projects after upgrading. - 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. - 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 0485c331c..b28b54eb2 100644 --- a/__tests__/fixtures/kernel-parity/torture.go +++ b/__tests__/fixtures/kernel-parity/torture.go @@ -25,6 +25,8 @@ type Widget struct { name string } +// Containers: a banner a blank line separates from Stack, not its doc. + type Stack[T any] struct { items []T } @@ -160,6 +162,9 @@ func (w Widget) Render() string { return w.name } +// Callbacks: a section comment above TargetCb's own doc. + +// TargetCb is documented right above. func TargetCb(n int) {} func shadowed() { diff --git a/__tests__/go-detached-doc-comments.test.ts b/__tests__/go-detached-doc-comments.test.ts new file mode 100644 index 000000000..0f6e0e127 --- /dev/null +++ b/__tests__/go-detached-doc-comments.test.ts @@ -0,0 +1,208 @@ +/** + * A Go declaration's docstring took every comment written above it, including + * ones a blank line separates from it. In gin, `Context.Next` sits under a + * FLOW CONTROL banner of block comments, then a blank line, then its own doc + * comment, and its docstring began "/\nFLOW CONTROL\n/\n". A section banner, a + * TODO or commented-out code with nothing under it became the doc of whatever + * came next. + * + * Go's doc comment, the one go doc shows (go/parser's lead comment), is only + * the comment group that ends on the line right above the declaration: + * comments on consecutive lines form one group, and a blank line ends it. Go + * now reads its docstrings that way. + * + * 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 = `// Copyright 2026 The Shapes Authors. + +// Package shapes is documented. +package shapes + +/*** CONTENT NEGOTIATION ***/ + +// Negotiate is documented right above, under a banner. +func Negotiate() {} + +// TODO: a note a blank line separates from the function below. + +func Detached() {} + +// ==== Shapes START ==== + +type Bare struct{} + +// An orphaned copy of another declaration's doc. + +// Point is documented right above. +type Point struct{ X, Y int } + +// IsSafe is a hack, documented for something else. + +type ReadTx interface{} + +// Lengths. + +// Len is documented right above a method. +func (p Point) Len() int { return 0 } + +// interfaces for sorting + +func (p Point) Less() bool { return false } + +// Joined starts with a line comment, +/* +continues in a block comment +*/ +// and ends right above. +func Joined() {} + +/* +Box is documented in a block comment + +with a blank line inside it. +*/ +type Box struct{} + +// Para is documented in two paragraphs. +// +// The second paragraph. +func Para() {} + +/* Inline is written on the declaration's own line. */ func Inline() {} + +// Limits. + +// maxItems is documented right above. +const maxItems = 10 + +// A note about the registry. + +var registry = map[string]int{} + +type ( + // Circle is documented a blank line above. + + Circle struct{} + // Square is documented right above. + Square struct{} +) + +var origin = Point{} // the origin. +// Polygon is documented right below a commented line. +type Polygon struct{} +`; + +const ENV_KEYS = ['CODEGRAPH_KERNEL', 'CODEGRAPH_KERNEL_LANGS'] as const; + +describe('Go doc comments end at a blank line', () => { + 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; + }; + // A block comment keeps the line breaks written inside it. + const eol = (text: string) => (crlf ? text.replace(/\n/g, '\r\n') : text); + + expect({ + // A comment a blank line separates from the declaration is not its + // doc, whether or not a doc comment comes between them. + Negotiate: doc('function', 'Negotiate'), + Detached: doc('function', 'Detached'), + Bare: doc('struct', 'Bare'), + Point: doc('struct', 'Point'), + ReadTx: doc('interface', 'ReadTx'), + Len: doc('method', 'Len'), + Less: doc('method', 'Less'), + maxItems: doc('constant', 'maxItems'), + registry: doc('variable', 'registry'), + Circle: doc('struct', 'Circle'), + Square: doc('struct', 'Square'), + // Comments on consecutive lines are one doc comment, line and block + // comments alike, and a blank line inside a block comment or an empty + // `//` line does not end it. + Joined: doc('function', 'Joined'), + Box: doc('struct', 'Box'), + Para: doc('function', 'Para'), + // The doc comment ends on the line above the declaration, not on the + // declaration's own line. + Inline: doc('function', 'Inline'), + // A comment after code on its line is that line's, and the doc + // comment right below it starts after it. + Polygon: doc('struct', 'Polygon'), + }).toEqual({ + Negotiate: 'Negotiate is documented right above, under a banner.', + Detached: null, + Bare: null, + Point: 'Point is documented right above.', + ReadTx: null, + Len: 'Len is documented right above a method.', + Less: null, + maxItems: 'maxItems is documented right above.', + registry: null, + Circle: null, + Square: 'Square is documented right above.', + Joined: 'Joined starts with a line comment,\ncontinues in a block comment\nand ends right above.', + Box: eol('Box is documented in a block comment\n\nwith a blank line inside it.'), + Para: 'Para is documented in two paragraphs.\n\nThe second paragraph.', + Inline: null, + Polygon: 'Polygon is documented right below a commented line.', + }); + }); + } +}); diff --git a/codegraph-kernel/src/docstring.rs b/codegraph-kernel/src/docstring.rs index 22e499c23..9a9ee520d 100644 --- a/codegraph-kernel/src/docstring.rs +++ b/codegraph-kernel/src/docstring.rs @@ -143,16 +143,19 @@ pub fn preceding_docstring_stepping_over( src: &str, step_over: &[&str], ) -> Option { - preceding_docstring_with(node, src, step_over, false) + preceding_docstring_with(node, src, step_over, false, false) } -/// getPrecedingDocstring's `skipTrailing` (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. Go reads them as that line's comment, never the next -/// declaration's doc. -pub fn preceding_docstring_skipping_trailing(node: Node, src: &str) -> Option { - preceding_docstring_with(node, src, &[], true) +/// 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) } fn preceding_docstring_with( @@ -160,6 +163,7 @@ fn preceding_docstring_with( src: &str, step_over: &[&str], skip_trailing: bool, + skip_detached: bool, ) -> Option { let mut anchor = node; while let Some(parent) = anchor.parent() { @@ -200,6 +204,21 @@ fn preceding_docstring_with( first += 1; } } + if skip_detached { + // Walk back from the declaration: the last comment must end on the + // line above it, and each one before joins while it ends on the line + // the next one starts, or the line before that. + let mut start = comments.len(); + if start > first && comments[start - 1].end_position().row + 1 == anchor.start_position().row { + start -= 1; + while start > first + && comments[start - 1].end_position().row + 1 >= comments[start].start_position().row + { + start -= 1; + } + } + first = start; + } if first == comments.len() { return None; } @@ -294,9 +313,40 @@ mod tests { }) .expect(name) }; + let skipping_trailing = |n| preceding_docstring_with(n, src, &[], true, false); assert_eq!(preceding_docstring(func("f"), src).as_deref(), Some("a\nb\nc")); - assert_eq!(preceding_docstring_skipping_trailing(func("f"), src).as_deref(), Some("c")); + assert_eq!(skipping_trailing(func("f")).as_deref(), Some("c")); assert_eq!(preceding_docstring(func("g"), src).as_deref(), Some("d")); - assert_eq!(preceding_docstring_skipping_trailing(func("g"), src), None); + assert_eq!(skipping_trailing(func("g")), None); + } + + /// Skipping detached comments keeps the comments that end on the line + /// above the declaration and those on the lines just before them; a blank + /// line ends the run, and a comment on the declaration's own line is not + /// kept. Without it, every comment is. + #[test] + fn skips_the_comments_a_blank_line_detaches() { + 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\n// b\n/*\nc\n*/\n// d\nfunc f() {}\n\n// e\n\nfunc g() {}\n\n/* h */ func h() {}\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) + }; + let skipping_detached = |n| preceding_docstring_with(n, src, &[], false, true); + 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); } } diff --git a/codegraph-kernel/src/go.rs b/codegraph-kernel/src/go.rs index f13ee4429..b1ff1b613 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; +use crate::docstring::preceding_docstring_skipping_trailing_and_detached; use crate::ids; use crate::textutil as util; use regex::Regex; @@ -400,9 +400,14 @@ impl<'t> Walker<'t> { /// docstringFor (tree-sitter.ts) — the preceding comment run, looked up /// from the declaration wrapper when there is one, without the comments - /// that trail the code above it (docstringSkipsTrailingComments). + /// that trail the code above it (docstringSkipsTrailingComments) or that + /// don't end on the line right above it, past a blank line + /// (docstringSkipsDetachedComments). fn docstring_of(&self, node: Node<'t>) -> Option { - preceding_docstring_skipping_trailing(self.declaration_wrapper(node).unwrap_or(node), self.src) + preceding_docstring_skipping_trailing_and_detached( + 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 902f86103..138c4938c 100644 --- a/src/extraction/languages/go.ts +++ b/src/extraction/languages/go.ts @@ -257,4 +257,8 @@ export const goExtractor: LanguageExtractor = { // A comment after code on its line (`const sides = 4 // sides of a square.`) // is that line's, never the doc of the declaration below it. docstringSkipsTrailingComments: true, + // A doc comment ends on the line right above its declaration, and a blank + // line ends it, as go doc reads it: a banner or a note above that blank line + // documents nothing. + docstringSkipsDetachedComments: true, }; diff --git a/src/extraction/tree-sitter-helpers.ts b/src/extraction/tree-sitter-helpers.ts index 1177f2881..605f142e6 100644 --- a/src/extraction/tree-sitter-helpers.ts +++ b/src/extraction/tree-sitter-helpers.ts @@ -139,12 +139,19 @@ function cleanCommentMarkers(comment: string): string { * the line above: one written after code on its line (`const n = 4 // four.`), * and any that begins on the line such a comment ends. Go reads them as that * line's comment, never the next declaration's doc. Default: off. + * + * `skipDetached` keeps only the comments that end on the line right above the + * declaration, together with the ones written on the lines just before them: + * 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. */ export function getPrecedingDocstring( node: SyntaxNode, source: string, stepOver: readonly string[] = [], - skipTrailing = false + skipTrailing = false, + skipDetached = 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. @@ -186,6 +193,19 @@ export function getPrecedingDocstring( first++; } } + if (skipDetached) { + // Walk back from the declaration: the last comment must end on the line + // above it, and each one before joins while it ends on the line the next + // one starts, or the line before that. + let start = comments.length; + if (start > first && comments[start - 1]!.endPosition.row + 1 === anchor.startPosition.row) { + start--; + while (start > first && comments[start - 1]!.endPosition.row + 1 >= comments[start]!.startPosition.row) { + start--; + } + } + first = start; + } if (first === comments.length) return undefined; // Strip each comment's syntax markers (language-aware), then join. diff --git a/src/extraction/tree-sitter-types.ts b/src/extraction/tree-sitter-types.ts index 3a695d52d..22625081e 100644 --- a/src/extraction/tree-sitter-types.ts +++ b/src/extraction/tree-sitter-types.ts @@ -190,6 +190,14 @@ export interface LanguageExtractor { * line and never documents the declaration below it. */ docstringSkipsTrailingComments?: boolean; + /** + * Keep in a docstring only the comments that end on the line right above + * the declaration, together with the ones written on the lines just before + * them: a blank line ends it. That is Go's doc comment, the one go doc + * shows, so a section banner or a note a blank line separates from a + * declaration documents nothing. + */ + docstringSkipsDetachedComments?: 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 660511f43..035dcdab1 100644 --- a/src/extraction/tree-sitter.ts +++ b/src/extraction/tree-sitter.ts @@ -541,7 +541,8 @@ export class TreeSitterExtractor { anchor, this.source, this.extractor?.docstringStepOverTypes, - this.extractor?.docstringSkipsTrailingComments + this.extractor?.docstringSkipsTrailingComments, + this.extractor?.docstringSkipsDetachedComments ); const body = this.extractor?.getBodyDocstring?.(node, this.source); if (preceding && body) return `${preceding}\n\n${body}`;