Skip to content

Add CommentAttachment/ExpressionStatement types and comment fields to AST (BT-973)#1008

Merged
jamesc merged 3 commits intomainfrom
worktree-BT-973
Feb 28, 2026
Merged

Add CommentAttachment/ExpressionStatement types and comment fields to AST (BT-973)#1008
jamesc merged 3 commits intomainfrom
worktree-BT-973

Conversation

@jamesc
Copy link
Owner

@jamesc jamesc commented Feb 28, 2026

Summary

Phase 1 of ADR 0044 (Comments as First-Class AST Nodes), issue 1 of 6.

Adds the comment-carrying types and fields to ast.rs. Purely additive — no existing behavior changes, all downstream consumers continue to work unchanged.

Linear: https://linear.app/beamtalk/issue/BT-973

Key Changes

  • New types: CommentAttachment (leading/trailing comments) and ExpressionStatement (expression + comments wrapper)
  • Updated structs: ClassDefinition, MethodDefinition, and StateDeclaration gain comments: CommentAttachment field
  • StateDeclaration also gains doc_comment: Option<String> (always None until parser support in BT-975)
  • Convenience constructors: Comment::line(), Comment::block(), ExpressionStatement::bare()
  • All construction sites across codegen tests, semantic analysis, type checker, and parser updated
  • 20 insta snapshot tests regenerated

Test plan

  • Unit tests for CommentAttachment (default, leading, trailing, combined)
  • Unit tests for ExpressionStatement (bare constructor, with comments)
  • Unit tests for Comment constructors
  • All existing tests pass (no regressions)
  • just ci passes (build, clippy, fmt, test, test-stdlib, test-e2e)

🤖 Generated with Claude Code

Summary by CodeRabbit

  • Refactor

    • AST now preserves inline/block comments and optional documentation comments on declarations, ensuring comments are retained through analysis and generation.
  • New Features

    • Added lightweight statement wrapper to associate comments with expressions, and helper ways to create comment-less statements.
  • Tests

    • Expanded tests to cover comment creation, attachment, propagation, and default/empty states.

jamesc and others added 2 commits February 28, 2026 15:14
…lds to AST (BT-973)

Phase 1 of ADR 0044 - Comments as First-Class AST Nodes.

- Add `CommentAttachment` struct with `leading: Vec<Comment>` and
  `trailing: Option<Comment>` fields, `is_empty()` method, and
  `#[derive(Default)]`
- Add `ExpressionStatement` struct with `comments: CommentAttachment`
  and `expression: Expression` fields, plus `bare(expr)` constructor
- Add `comments: CommentAttachment` field to `ClassDefinition` and
  `MethodDefinition`; update all construction sites
- Add `comments: CommentAttachment` and `doc_comment: Option<String>`
  to `StateDeclaration`; update all construction sites
- Update parser/declarations.rs StateDeclaration struct literals
- Update snapshot tests to include new default-empty comment fields
- Add unit tests for CommentAttachment and ExpressionStatement

Co-Authored-By: Claude Sonnet 4.6 <noreply@anthropic.com>
…comment, and additional tests (BT-973)

From code review:
- Add Comment::line() and Comment::block() constructors to reduce
  struct literal fragility
- Document StateDeclaration.doc_comment as always None until BT-975
- Add test for CommentAttachment with both leading AND trailing
- Add test for Comment constructors

Co-Authored-By: Claude Sonnet 4.6 <noreply@anthropic.com>
@coderabbitai
Copy link

coderabbitai bot commented Feb 28, 2026

No actionable comments were generated in the recent review. 🎉

ℹ️ Recent 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 f53bda6 and 785ea5f.

📒 Files selected for processing (1)
  • crates/beamtalk-core/src/ast.rs

📝 Walkthrough

Walkthrough

Adds comment-support to the AST: new CommentAttachment and ExpressionStatement types, Comment::line/block constructors, and comments (plus doc_comment) fields on ClassDefinition, StateDeclaration, and MethodDefinition. Constructors, parser, semantic tests, and test fixtures updated to initialize these fields.

Changes

Cohort / File(s) Summary
AST Comment System
crates/beamtalk-core/src/ast.rs
Adds CommentAttachment (with is_empty()), ExpressionStatement (with bare()), Comment::line() and Comment::block(). Adds pub comments: CommentAttachment to ClassDefinition, StateDeclaration, MethodDefinition, and doc_comment: Option<String> placeholders; updates their constructors to initialize these fields. Includes unit tests for comment behavior.
Parser: attach parsed comments
crates/beamtalk-core/src/source_analysis/parser/declarations.rs
Imports CommentAttachment; sets comments: CommentAttachment::default() for parsed StateDeclaration and assigns parsed doc_comment to ClassDefinition/state declarations.
Semantic & Type-check tests / helpers
crates/beamtalk-core/src/semantic_analysis/..., crates/beamtalk-core/src/semantic_analysis/class_hierarchy/mod.rs, crates/beamtalk-core/src/semantic_analysis/mod.rs, crates/beamtalk-core/src/semantic_analysis/type_checker.rs
Updates test helpers and callers to construct ClassDefinition, MethodDefinition, and StateDeclaration with comments: CommentAttachment::default() (and doc_comment: None where applicable). Adds necessary imports. No runtime behavior changes.
Codegen tests
crates/beamtalk-core/src/codegen/core_erlang/tests.rs
Updates test fixtures to include the new comments and doc_comment fields on states, methods, and classes so test constructions match the extended AST signatures. Minor test adjustments only.

Sequence Diagram(s)

sequenceDiagram
    participant Parser
    participant AST
    participant SemanticAnalyzer
    participant Codegen

    Parser->>AST: parse source (produce nodes + comments)
    Note right of AST: nodes include CommentAttachment\nand doc_comment fields
    AST->>SemanticAnalyzer: provide annotated AST
    SemanticAnalyzer->>Codegen: emit analysis-ready structures
    Codegen->>Codegen: use comments/doc_comment where needed (tests/codegen)
Loading

Estimated code review effort

🎯 3 (Moderate) | ⏱️ ~20 minutes

Possibly related PRs

🚥 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 accurately and concisely describes the main changes: adding CommentAttachment and ExpressionStatement types plus comment fields to the AST, with the tracking issue BT-973 clearly referenced.
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-973

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

Copy link

@coderabbitai coderabbitai bot left a comment

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Actionable comments posted: 1

🤖 Prompt for all review comments with AI agents
Verify each finding against the current code and only fix it if needed.

Inline comments:
In `@crates/beamtalk-core/src/ast.rs`:
- Around line 214-218: The doc comment for ExpressionStatement overstates
adoption—update the comment text around the ExpressionStatement doc (the block
starting with "An expression in a statement position...") to avoid claiming that
method bodies, block bodies, and module-level expression sequences use
Vec<ExpressionStatement>; instead phrase it neutrally (e.g., note that
ExpressionStatement represents an expression with optional surrounding comments
and that some APIs or consumers may use Vec<ExpressionStatement> even though the
current fields remain Vec<Expression>) so consumers aren't misled; keep
references to ExpressionStatement and Expression to make intent clear.

ℹ️ 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 a422aac and f53bda6.

⛔ Files ignored due to path filters (20)
  • test-package-compiler/tests/snapshots/compiler_tests__abstract_class_spawn_parser.snap is excluded by !**/*.snap
  • test-package-compiler/tests/snapshots/compiler_tests__class_definition_parser.snap is excluded by !**/*.snap
  • test-package-compiler/tests/snapshots/compiler_tests__class_methods_parser.snap is excluded by !**/*.snap
  • test-package-compiler/tests/snapshots/compiler_tests__empty_method_body_parser.snap is excluded by !**/*.snap
  • test-package-compiler/tests/snapshots/compiler_tests__error_message_parser.snap is excluded by !**/*.snap
  • test-package-compiler/tests/snapshots/compiler_tests__intrinsic_keyword_parser.snap is excluded by !**/*.snap
  • test-package-compiler/tests/snapshots/compiler_tests__method_lookup_parser.snap is excluded by !**/*.snap
  • test-package-compiler/tests/snapshots/compiler_tests__sealed_class_violation_parser.snap is excluded by !**/*.snap
  • test-package-compiler/tests/snapshots/compiler_tests__sealed_method_override_parser.snap is excluded by !**/*.snap
  • test-package-compiler/tests/snapshots/compiler_tests__stdlib_class_dictionary_parser.snap is excluded by !**/*.snap
  • test-package-compiler/tests/snapshots/compiler_tests__stdlib_class_list_parser.snap is excluded by !**/*.snap
  • test-package-compiler/tests/snapshots/compiler_tests__stdlib_class_set_parser.snap is excluded by !**/*.snap
  • test-package-compiler/tests/snapshots/compiler_tests__stdlib_class_tuple_parser.snap is excluded by !**/*.snap
  • test-package-compiler/tests/snapshots/compiler_tests__typed_class_warnings_parser.snap is excluded by !**/*.snap
  • test-package-compiler/tests/snapshots/compiler_tests__typed_methods_parser.snap is excluded by !**/*.snap
  • test-package-compiler/tests/snapshots/compiler_tests__typed_value_type_parser.snap is excluded by !**/*.snap
  • test-package-compiler/tests/snapshots/compiler_tests__value_type_multi_expr_method_parser.snap is excluded by !**/*.snap
  • test-package-compiler/tests/snapshots/compiler_tests__value_type_param_collision_parser.snap is excluded by !**/*.snap
  • test-package-compiler/tests/snapshots/compiler_tests__workspace_binding_cascade_parser.snap is excluded by !**/*.snap
  • test-package-compiler/tests/snapshots/compiler_tests__workspace_binding_send_parser.snap is excluded by !**/*.snap
📒 Files selected for processing (6)
  • crates/beamtalk-core/src/ast.rs
  • crates/beamtalk-core/src/codegen/core_erlang/tests.rs
  • crates/beamtalk-core/src/semantic_analysis/class_hierarchy/mod.rs
  • crates/beamtalk-core/src/semantic_analysis/mod.rs
  • crates/beamtalk-core/src/semantic_analysis/type_checker.rs
  • crates/beamtalk-core/src/source_analysis/parser/declarations.rs

…-973

Address CodeRabbit review: doc comment overstated adoption since
statement-position fields haven't migrated to Vec<ExpressionStatement> yet.

Co-Authored-By: Claude Sonnet 4.6 <noreply@anthropic.com>
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment

Labels

None yet

Projects

None yet

Development

Successfully merging this pull request may close these issues.

1 participant