Skip to content
Merged
Show file tree
Hide file tree
Changes from all commits
Commits
File filter

Filter by extension

Filter by extension

Conversations
Failed to load comments.
Loading
Jump to
Jump to file
Failed to load files.
Loading
Diff view
Diff view
20 changes: 14 additions & 6 deletions crates/beamtalk-core/src/source_analysis/parser/declarations.rs
Original file line number Diff line number Diff line change
Expand Up @@ -9,8 +9,8 @@
//! - Method definitions with optional `sealed` modifier

use crate::ast::{
ClassDefinition, CommentAttachment, Expression, ExpressionStatement, Identifier, KeywordPart,
MessageSelector, MethodDefinition, MethodKind, ParameterDefinition, StandaloneMethodDefinition,
ClassDefinition, Expression, ExpressionStatement, Identifier, KeywordPart, MessageSelector,
MethodDefinition, MethodKind, ParameterDefinition, StandaloneMethodDefinition,
StateDeclaration, TypeAnnotation,
};
use crate::source_analysis::{Span, TokenKind};
Expand Down Expand Up @@ -41,6 +41,7 @@ impl Parser {
pub(super) fn parse_class_definition(&mut self) -> ClassDefinition {
let start = self.current_token().span();
let doc_comment = self.collect_doc_comment();
let comments = self.collect_comment_attachment();
let mut is_abstract = false;
let mut is_sealed = false;
let mut is_typed = false;
Expand Down Expand Up @@ -119,6 +120,7 @@ impl Parser {
class_def.class_methods = class_methods;
class_def.class_variables = class_variables;
class_def.doc_comment = doc_comment;
class_def.comments = comments;
class_def
}

Expand Down Expand Up @@ -399,6 +401,8 @@ impl Parser {
/// - `state: fieldName: TypeName = defaultValue`
fn parse_state_declaration(&mut self) -> Option<StateDeclaration> {
let start = self.current_token().span();
let doc_comment = self.collect_doc_comment();
let comments = self.collect_comment_attachment();

// Consume `state:`
if !matches!(self.current_kind(), TokenKind::Keyword(k) if k == "state:") {
Expand Down Expand Up @@ -462,8 +466,8 @@ impl Parser {
name,
type_annotation,
default_value,
comments: CommentAttachment::default(),
doc_comment: None,
comments,
doc_comment,
span,
})
}
Expand All @@ -477,6 +481,8 @@ impl Parser {
/// - `classState: varName: TypeName = defaultValue`
fn parse_classvar_declaration(&mut self) -> Option<StateDeclaration> {
let start = self.current_token().span();
let doc_comment = self.collect_doc_comment();
let comments = self.collect_comment_attachment();

// Consume `classState:`
if !matches!(self.current_kind(), TokenKind::Keyword(k) if k == "classState:") {
Expand Down Expand Up @@ -529,8 +535,8 @@ impl Parser {
name,
type_annotation,
default_value,
comments: CommentAttachment::default(),
doc_comment: None,
comments,
doc_comment,
span,
})
}
Expand Down Expand Up @@ -593,6 +599,7 @@ impl Parser {
fn parse_method_definition(&mut self) -> Option<MethodDefinition> {
let start = self.current_token().span();
let doc_comment = self.collect_doc_comment();
let comments = self.collect_comment_attachment();
let method_kind = MethodKind::Primary;
let mut method_is_sealed = false;
let mut _is_class_method = false;
Expand Down Expand Up @@ -647,6 +654,7 @@ impl Parser {
span,
);
method.doc_comment = doc_comment;
method.comments = comments;
Some(method)
}

Expand Down
142 changes: 141 additions & 1 deletion crates/beamtalk-core/src/source_analysis/parser/mod.rs
Original file line number Diff line number Diff line change
Expand Up @@ -50,7 +50,9 @@
//! assert_eq!(module.expressions.len(), 1);
//! ```

use crate::ast::{Comment, CommentKind, Expression, ExpressionStatement, Module};
use crate::ast::{
Comment, CommentAttachment, CommentKind, Expression, ExpressionStatement, Module,
};
#[cfg(test)]
use crate::ast::{Literal, MessageSelector};
use crate::source_analysis::{Span, Token, TokenKind, Trivia, lex_with_eof};
Expand Down Expand Up @@ -582,6 +584,41 @@ impl Parser {
}
}

/// Extracts regular (non-doc) comments from the current token's leading trivia.
///
/// Collects `//` line comments and `/* */` block comments into
/// [`CommentAttachment::leading`]. `///` doc comments are intentionally
/// **skipped** — they are handled by [`collect_doc_comment`] and must not
/// be duplicated here. Call this *after* [`collect_doc_comment`] so both
/// functions operate on the same trivia snapshot before any token is
/// consumed.
///
/// # Ordering dependency
///
/// `collect_doc_comment()` must run first, then `collect_comment_attachment()`
/// on the same leading trivia. Both methods are `&self` / immutable reads,
/// so no token is consumed between the two calls.
pub(super) fn collect_comment_attachment(&self) -> CommentAttachment {
let token_span = self.current_token().span();
let mut leading = Vec::new();
for trivia in self.current_token().leading_trivia() {
match trivia {
super::Trivia::LineComment(text) => {
leading.push(Comment::line(text.clone(), token_span));
}
super::Trivia::BlockComment(text) => {
leading.push(Comment::block(text.clone(), token_span));
}
// DocComment is handled by collect_doc_comment — skip here.
super::Trivia::DocComment(_) | super::Trivia::Whitespace(_) => {}
}
}
CommentAttachment {
leading,
trailing: None,
}
}

/// Reports an error at the current token.
pub(super) fn error(&mut self, message: impl Into<EcoString>) {
let span = self.current_token().span();
Expand Down Expand Up @@ -4568,4 +4605,107 @@ Actor subclass: Counter
"expected no lints at module level, got: {lints:?}"
);
}

// ========================================================================
// Comment attachment tests (BT-975)
// ========================================================================

#[test]
fn class_line_comment_attached_to_leading() {
// A `//` comment immediately before a class definition should appear in
// `ClassDefinition.comments.leading`.
let module = parse_ok("// A useful class\nObject subclass: Foo\n");
assert_eq!(module.classes.len(), 1);
let class = &module.classes[0];
assert_eq!(
class.comments.leading.len(),
1,
"expected one leading comment"
);
assert_eq!(class.comments.leading[0].kind, CommentKind::Line);
assert!(
class.comments.leading[0].content.contains("A useful class"),
"content: {}",
class.comments.leading[0].content
);
}

#[test]
fn class_doc_comment_not_duplicated_in_attachment() {
// A `///` doc comment must NOT appear in `comments.leading`; it goes to
// `doc_comment` only.
let module = parse_ok("/// Doc text\nObject subclass: Foo\n");
assert_eq!(module.classes.len(), 1);
let class = &module.classes[0];
assert!(
class.doc_comment.is_some(),
"expected doc_comment to be populated"
);
assert!(
class.comments.leading.is_empty(),
"doc comment must not be duplicated into leading comments"
);
}

#[test]
fn method_mixed_doc_and_line_comment_separated() {
// `//` goes to `comments.leading`, `///` goes to `doc_comment`.
// The `//` must appear BEFORE `///` so the doc comment collection is
// not reset (a `//` after `///` would reset the doc-comment buffer).
let source = "Object subclass: Foo\n // Line comment\n /// Doc comment\n go => 42\n";
let module = parse_ok(source);
assert_eq!(module.classes.len(), 1);
let method = &module.classes[0].methods[0];
assert!(
method.doc_comment.is_some(),
"expected doc_comment on method"
);
assert_eq!(
method.comments.leading.len(),
1,
"expected one leading comment on method"
);
assert_eq!(method.comments.leading[0].kind, CommentKind::Line);
}

#[test]
fn state_declaration_doc_comment_populated() {
// A `///` doc comment before a state declaration is captured in `doc_comment`.
let source = "Object subclass: Foo\n /// The count\n state: count = 0\n";
let module = parse_ok(source);
assert_eq!(module.classes.len(), 1);
let state = &module.classes[0].state[0];
assert!(
state.doc_comment.is_some(),
"expected doc_comment on state declaration"
);
assert!(
state
.doc_comment
.as_deref()
.unwrap_or("")
.contains("The count"),
"doc_comment: {:?}",
state.doc_comment
);
}

#[test]
fn state_declaration_line_comment_attached() {
// A `//` comment before `state:` is attached to `comments.leading`.
let source = "Object subclass: Foo\n // The x field\n state: x = 1\n";
let module = parse_ok(source);
assert_eq!(module.classes.len(), 1);
let state = &module.classes[0].state[0];
assert_eq!(
state.comments.leading.len(),
1,
"expected one leading comment on state declaration"
);
assert!(
state.comments.leading[0].content.contains("The x field"),
"content: {}",
state.comments.leading[0].content
);
}
}
Original file line number Diff line number Diff line change
@@ -1,5 +1,6 @@
---
source: test-package-compiler/tests/compiler_tests.rs
assertion_line: 70
expression: output
---
AST:
Expand Down Expand Up @@ -78,7 +79,32 @@ Module {
class_methods: [],
class_variables: [],
comments: CommentAttachment {
leading: [],
leading: [
Comment {
content: "// Copyright 2026 James Casey",
span: Span {
start: 145,
end: 153,
},
kind: Line,
},
Comment {
content: "// SPDX-License-Identifier: Apache-2.0",
span: Span {
start: 145,
end: 153,
},
kind: Line,
},
Comment {
content: "// Test that instantiating an abstract class produces a diagnostic error.",
span: Span {
start: 145,
end: 153,
},
kind: Line,
},
],
trailing: None,
},
doc_comment: None,
Expand Down
Original file line number Diff line number Diff line change
@@ -1,5 +1,6 @@
---
source: test-package-compiler/tests/compiler_tests.rs
assertion_line: 70
expression: output
---
AST:
Expand Down Expand Up @@ -214,7 +215,32 @@ Module {
class_methods: [],
class_variables: [],
comments: CommentAttachment {
leading: [],
leading: [
Comment {
content: "// Copyright 2026 James Casey",
span: Span {
start: 104,
end: 109,
},
kind: Line,
},
Comment {
content: "// SPDX-License-Identifier: Apache-2.0",
span: Span {
start: 104,
end: 109,
},
kind: Line,
},
Comment {
content: "// Test class definition parsing",
span: Span {
start: 104,
end: 109,
},
kind: Line,
},
],
trailing: None,
},
doc_comment: None,
Expand Down
Original file line number Diff line number Diff line change
@@ -1,5 +1,6 @@
---
source: test-package-compiler/tests/compiler_tests.rs
assertion_line: 70
expression: output
---
AST:
Expand Down Expand Up @@ -411,7 +412,32 @@ Module {
},
],
comments: CommentAttachment {
leading: [],
leading: [
Comment {
content: "// Copyright 2026 James Casey",
span: Span {
start: 138,
end: 143,
},
kind: Line,
},
Comment {
content: "// SPDX-License-Identifier: Apache-2.0",
span: Span {
start: 138,
end: 143,
},
kind: Line,
},
Comment {
content: "// Test class method codegen (class-side methods, class variables)",
span: Span {
start: 138,
end: 143,
},
kind: Line,
},
],
trailing: None,
},
doc_comment: None,
Expand Down
Loading