Lint: warn on unattached /// doc comments (BT-980)

[jamesc]

Summary

• Detect /// doc comment trivia that is never captured by an AST node and emit a Warning diagnostic
• Two detection paths: cleared doc comments in collect_doc_comment() (blank line or // break), and pre-scanned doc comments before non-declaration tokens (post-parse sweep)
• Fix existing unattached doc comments in Actor.bt and TestCase.bt

Resolves: https://linear.app/beamtalk/issue/BT-980

Key Changes

• parser/mod.rs: Added unattached_doc_comment_indices: HashSet<usize> to Parser struct, pre-scanned in new(). collect_doc_comment() changed to &mut self to emit warnings when doc comments are cleared. Post-parse sweep warns for remaining unscanned indices.
• Actor.bt, TestCase.bt: Removed blank lines between /// doc comment blocks and class definitions (the exact pattern now warned about).
• Tests: 3 new tests + 2 updated existing tests covering blank line, // comment, before-expression, and no-warning cases.

Test plan

• unattached_doc_comment_blank_line_before_class_emits_warning — /// + blank line + class → warning
• attached_doc_comment_no_blank_line_no_warning — /// immediately before class → no warning
• unattached_doc_comment_before_expression_emits_warning — /// before expression → warning
• parse_doc_comment_resets_on_regular_comment — updated to verify warning
• parse_doc_comment_blank_line_resets — updated to verify warning
• just build — no warnings from stdlib
• just ci — all checks pass

🤖 Generated with Claude Code

Summary by CodeRabbit

Release Notes

• New Features

  • Parser now emits warnings when doc comments (///) are unattached to any declaration, helping developers identify misplaced documentation and ensure proper association with classes, methods, or state variables.

• Style

  • Removed unnecessary blank lines from standard library files.

[coderabbitai]

📝 Walkthrough

Walkthrough

Adds warning workflow for unattached doc comments in the parser by tracking comments that fail to attach to declarations and emitting warnings during parsing and post-parse sweeps. Includes minor whitespace cleanups in stdlib files.

Changes

Cohort / File(s)	Summary
Parser: Unattached Doc Comment Warnings crates/beamtalk-core/src/source_analysis/parser/mod.rs	Introduces warning detection for unattached doc comments. Adds unattached_doc_comment_indices HashSet field to Parser for tracking. Modifies collect_doc_comment to accept &mut self, immediately warn and remove indices when doc comments fail to attach. Adds post-parse sweep in parse() to emit warnings for remaining unattached doc comments. Includes comprehensive test coverage for attachment and warning edge cases.
Whitespace Cleanup stdlib/src/Actor.bt, stdlib/src/TestCase.bt	Removal of stray blank lines preceding declarations. No functional or behavioral changes.

Estimated code review effort

🎯 3 (Moderate) | ⏱️ ~20 minutes

Possibly related PRs

• Parser: attach comments to ClassDefinition, MethodDefinition, StateDeclaration (BT-975) #1011: Modifies parser comment-handling and tracking logic with similar state management for comment attachment
• Parser: attach comments to ExpressionStatement; handle Module.file_leading_comments (BT-976) #1013: Updates parser's comment/attachment logic and Parser state for comment handling in the same file
• Add CommentAttachment/ExpressionStatement types and comment fields to AST (BT-973) #1008: Extends parser's doc-comment handling with related AST field modifications

🚥 Pre-merge checks | ✅ 3

✅ Passed checks (3 passed)

Check name	Status	Explanation
Description Check	✅ Passed	Check skipped - CodeRabbit’s high-level summary is enabled.
Title check	✅ Passed	The title directly and concisely summarizes the main change: implementing a linting feature to warn on unattached doc comments, with a clear reference to the tracking issue BT-980.
Docstring Coverage	✅ Passed	Docstring coverage is 100.00% which is sufficient. The required threshold is 80.00%.

_{✏️ Tip: You can configure your own custom pre-merge checks in the settings.}

✨ Finishing Touches

• 📝 Generate docstrings (stacked PR)
• 📝 Generate docstrings (commit on current branch)

🧪 Generate unit tests (beta)

• Create PR with unit tests
• Post copyable unit tests in a comment
• Commit unit tests in branch worktree-BT-980

---

_{Comment @coderabbitai help to get the list of available commands and usage tips.}

[coderabbitai]

🧹 Nitpick comments (1)

crates/beamtalk-core/src/source_analysis/parser/mod.rs (1)

598-655: Avoid cloning full leading trivia in collect_doc_comment().

to_vec() copies all trivia for every call. A two-phase borrow pattern can keep the logic identical without the extra allocation.

♻️ Proposed refactor

 pub(super) fn collect_doc_comment(&mut self) -> Option<String> {
-    // Clone leading trivia to release the shared borrow of `self` so we
-    // can push to `self.diagnostics` inside the loop.
-    let leading_trivia = self.current_token().leading_trivia().to_vec();
     let current_span = self.current_token().span();
     let current_idx = self.current;
 
-    let mut lines: Vec<String> = Vec::new();
-    let mut had_unattached = false;
-
-    for trivia in &leading_trivia {
-        match trivia {
-            super::Trivia::DocComment(text) => {
-                let text = text.as_str();
-                // Strip `/// ` (with space) or `///` prefix
-                let stripped = text
-                    .strip_prefix("/// ")
-                    .unwrap_or_else(|| text.strip_prefix("///").unwrap_or(text));
-                lines.push(stripped.to_string());
-            }
-            super::Trivia::Whitespace(ws) => {
-                // A blank line (>1 newline) breaks consecutive doc comments
-                if ws.chars().filter(|&c| c == '\n').count() > 1 {
-                    if !lines.is_empty() {
-                        had_unattached = true;
-                    }
-                    lines.clear();
-                }
-            }
-            _ => {
-                // A non-doc-comment (e.g. `//`) resets collection
-                if !lines.is_empty() {
-                    had_unattached = true;
-                }
-                lines.clear();
-            }
-        }
-    }
+    let (lines, had_unattached) = {
+        let mut lines: Vec<String> = Vec::new();
+        let mut had_unattached = false;
+
+        for trivia in self.current_token().leading_trivia() {
+            match trivia {
+                super::Trivia::DocComment(text) => {
+                    let text = text.as_str();
+                    // Strip `/// ` (with space) or `///` prefix
+                    let stripped = text
+                        .strip_prefix("/// ")
+                        .unwrap_or_else(|| text.strip_prefix("///").unwrap_or(text));
+                    lines.push(stripped.to_string());
+                }
+                super::Trivia::Whitespace(ws) => {
+                    // A blank line (>1 newline) breaks consecutive doc comments
+                    if ws.chars().filter(|&c| c == '\n').count() > 1 {
+                        if !lines.is_empty() {
+                            had_unattached = true;
+                        }
+                        lines.clear();
+                    }
+                }
+                _ => {
+                    // A non-doc-comment (e.g. `//`) resets collection
+                    if !lines.is_empty() {
+                        had_unattached = true;
+                    }
+                    lines.clear();
+                }
+            }
+        }
+
+        (lines, had_unattached)
+    };

🤖 Prompt for AI Agents

Verify each finding against the current code and only fix it if needed.

In `@crates/beamtalk-core/src/source_analysis/parser/mod.rs` around lines 598 -
655, collect_doc_comment currently clones the entire leading_trivia with
to_vec(), causing an unnecessary allocation; instead, avoid cloning Trivia by
collecting references to the trivia items so you can drop the borrow on self
before pushing diagnostics. Replace the to_vec() usage with something like
collecting &super::Trivia references from
self.current_token().leading_trivia().iter() into a Vec<&super::Trivia> (keep
current_span and current_idx as before), then iterate over those references in
the for loop and leave the rest of the logic (lines, had_unattached, diagnostics
push, and unattached_doc_comment_indices removals) unchanged; this preserves
behavior in collect_doc_comment while removing the full-trivia clone.

🤖 Prompt for all review comments with AI agents

Verify each finding against the current code and only fix it if needed.

Nitpick comments:
In `@crates/beamtalk-core/src/source_analysis/parser/mod.rs`:
- Around line 598-655: collect_doc_comment currently clones the entire
leading_trivia with to_vec(), causing an unnecessary allocation; instead, avoid
cloning Trivia by collecting references to the trivia items so you can drop the
borrow on self before pushing diagnostics. Replace the to_vec() usage with
something like collecting &super::Trivia references from
self.current_token().leading_trivia().iter() into a Vec<&super::Trivia> (keep
current_span and current_idx as before), then iterate over those references in
the for loop and leave the rest of the logic (lines, had_unattached, diagnostics
push, and unattached_doc_comment_indices removals) unchanged; this preserves
behavior in collect_doc_comment while removing the full-trivia clone.

---

ℹ️ Review info

Configuration used: Organization UI

Review profile: CHILL

Plan: Pro

Disabled knowledge base sources:

• Linear integration is disabled

You can enable these sources in your CodeRabbit configuration.

📥 Commits

Reviewing files that changed from the base of the PR and between 213f695 and 4a48748.

📒 Files selected for processing (3)

• crates/beamtalk-core/src/source_analysis/parser/mod.rs
• stdlib/src/Actor.bt
• stdlib/src/TestCase.bt

💤 Files with no reviewable changes (2)

• stdlib/src/TestCase.bt
• stdlib/src/Actor.bt