diff --git a/src/grammar/comments.rs b/src/grammar/comments.rs index 8d22a1f8..c5a069c4 100644 --- a/src/grammar/comments.rs +++ b/src/grammar/comments.rs @@ -5,7 +5,7 @@ use crate::slice_file::Span; #[derive(Debug)] pub struct DocComment { - pub overview: Option, + pub overview: Option, pub params: Vec, pub returns: Vec, pub throws: Vec, @@ -13,12 +13,6 @@ pub struct DocComment { pub span: Span, } -#[derive(Debug)] -pub struct Overview { - pub message: Message, - pub span: Span, -} - #[derive(Debug)] pub struct ParamTag { pub identifier: Identifier, @@ -85,12 +79,14 @@ pub enum MessageComponent { Link(LinkTag), } -pub type Message = Vec; +#[derive(Debug)] +pub struct Message { + pub value: Vec, + pub span: Span, +} implement_Element_for!(DocComment, "doc comment"); implement_Symbol_for!(DocComment); -implement_Element_for!(Overview, "overview"); -implement_Symbol_for!(Overview); implement_Element_for!(ParamTag, "param tag"); implement_Symbol_for!(ParamTag); implement_Element_for!(ReturnsTag, "returns tag"); @@ -101,3 +97,5 @@ implement_Element_for!(SeeTag, "see tag"); implement_Symbol_for!(SeeTag); implement_Element_for!(LinkTag, "link tag"); implement_Symbol_for!(LinkTag); +implement_Element_for!(Message, "doc message"); +implement_Symbol_for!(Message); diff --git a/src/parsers/comments/grammar.lalrpop b/src/parsers/comments/grammar.lalrpop index 2a1373b8..28b0c369 100644 --- a/src/parsers/comments/grammar.lalrpop +++ b/src/parsers/comments/grammar.lalrpop @@ -40,7 +40,7 @@ extern { // Grammar Rules pub DocComment: DocComment = { - => create_doc_comment(overview, l, comment_parser.file_name), + => create_doc_comment(overview, l, comment_parser.file_name), => { append_tag_to_comment!(comment, params, param_block) }, @@ -55,29 +55,22 @@ pub DocComment: DocComment = { }, } -Overview: Overview = { - => { - let span = Span::new(l, r, comment_parser.file_name); - Overview { message, span } - }, -} - ParamBlock: ParamTag = { - param_keyword => { + param_keyword => { let span = Span::new(l, r, comment_parser.file_name); ParamTag { identifier, message, span } }, } ReturnsBlock: ReturnsTag = { - returns_keyword => { + returns_keyword => { let span = Span::new(l, r, comment_parser.file_name); ReturnsTag { identifier, message, span } }, } ThrowsBlock: ThrowsTag = { - throws_keyword => { + throws_keyword => { let span = Span::new(l, r, comment_parser.file_name); let thrown_type = TypeRefDefinition::Unpatched(identifier); ThrowsTag { thrown_type, message, span } @@ -99,13 +92,17 @@ InlineLink: LinkTag = { } Section: Message = { - )?> newline => { - construct_section_message(inline_message.flatten(), message_lines) + )?> newline => { + let span = Span::new(l, r, comment_parser.file_name); + construct_section_message(inline_message.flatten(), message_lines, span) }, } MessageLines: Message = { - ( newline)+ => sanitize_message_lines(<>), + newline)+> => { + let span = Span::new(l, r, comment_parser.file_name); + sanitize_message_lines(m, span) + } } Message = MessageComponent+; diff --git a/src/parsers/comments/grammar.rs b/src/parsers/comments/grammar.rs index ee8121a9..6db34783 100644 --- a/src/parsers/comments/grammar.rs +++ b/src/parsers/comments/grammar.rs @@ -5,7 +5,7 @@ //! While many of these functions could be written directly into the parser rules, we implement them here instead, to //! keep the rules focused on grammar instead of implementation details, making the grammar easier to read and modify. -use crate::grammar::{DocComment, Message, MessageComponent, Overview}; +use crate::grammar::{DocComment, Message, MessageComponent}; use crate::slice_file::{Location, Span}; use lalrpop_util::lalrpop_mod; @@ -31,7 +31,7 @@ use append_tag_to_comment; // To let LALRPOP use the macro. // Grammar Rule Functions /// Creates a new doc comment with the specified overview and everything else empty. -fn create_doc_comment(overview: Option, start: Location, file: &str) -> DocComment { +fn create_doc_comment(overview: Option, start: Location, file: &str) -> DocComment { // We subtract 3 from the start of the comment to account for the leading "///" that is always present. // This span is automatically extended as more constructs are parsed. let mut span = Span::new(start, start, file); @@ -62,8 +62,12 @@ fn get_scoped_identifier_string<'a>(first: &'a str, mut others: Vec<&'a str>, is } /// Removes any leading whitespace from the inline part of the message, then combines it with any following lines. -fn construct_section_message(inline_message: Option, message_lines: Option) -> Message { - let mut message_lines = message_lines.unwrap_or_default(); +fn construct_section_message( + inline_message: Option>, + message_lines: Option, + span: Span, +) -> Message { + let mut value = message_lines.map(|m| m.value).unwrap_or_default(); if let Some(mut message) = inline_message { // Remove any leading whitespace from the inline portion of the message. @@ -75,17 +79,19 @@ fn construct_section_message(inline_message: Option, message_lines: Opt message.push(MessageComponent::Text("\n".to_owned())); // Combine the 2 messages together, with the inline portion at the front. - message.append(&mut message_lines); - message_lines = message; + message.append(&mut value); + value = message; } - message_lines + Message { value, span } } /// Removes any common leading whitespace from the provided lines and returns the result. /// Each element in the vector represents one line of the message. /// `None` means the line existed but was empty, `Some(message)` means the line had a message. -fn sanitize_message_lines(lines: Vec>) -> Message { +/// +/// Note that the message's span is not updated to reflect the stripping of common leading whitespace. +fn sanitize_message_lines(lines: Vec>>, span: Span) -> Message { // First compute the amount of leading whitespace that is common to every line. let mut common_leading_whitespace = usize::MAX; for line in &lines { @@ -111,7 +117,7 @@ fn sanitize_message_lines(lines: Vec>) -> Message { } // Now that we know the common leading whitespace, we iterate through the lines again and remove the whitespace. - lines + let value = lines .into_iter() .flat_map(|line| match line { // If the message had text, we remove the common leading whitespace and append a newline at the end. @@ -126,5 +132,7 @@ fn sanitize_message_lines(lines: Vec>) -> Message { // If the line was empty, we create a new message that only contains a newline character. None => vec![MessageComponent::Text("\n".to_owned())], }) - .collect() + .collect(); + + Message { value, span } } diff --git a/src/patchers/comment_link_patcher.rs b/src/patchers/comment_link_patcher.rs index 20f0bdbb..e1320677 100644 --- a/src/patchers/comment_link_patcher.rs +++ b/src/patchers/comment_link_patcher.rs @@ -75,7 +75,7 @@ impl CommentLinkPatcher<'_> { fn compute_patches_for(&mut self, commentable: &impl Commentable, ast: &Ast) { if let Some(comment) = commentable.comment() { if let Some(overview) = &comment.overview { - self.resolve_links_in(&overview.message, commentable, ast); + self.resolve_links_in(overview, commentable, ast); } for param_tag in &comment.params { self.resolve_links_in(¶m_tag.message, commentable, ast); @@ -94,7 +94,7 @@ impl CommentLinkPatcher<'_> { } fn resolve_links_in(&mut self, message: &Message, commentable: &impl Commentable, ast: &Ast) { - for component in message { + for component in &message.value { if let MessageComponent::Link(link_tag) = component { self.resolve_link(&link_tag.link, commentable, ast); } @@ -146,7 +146,7 @@ impl CommentLinkPatcher<'_> { fn apply_patches(&mut self, scope: &str, comment: &mut Option) { if let Some(comment) = comment { if let Some(overview) = &mut comment.overview { - self.patch_links_in(&mut overview.message); + self.patch_links_in(overview); } for param_tag in &mut comment.params { self.patch_links_in(&mut param_tag.message); @@ -165,7 +165,7 @@ impl CommentLinkPatcher<'_> { } fn patch_links_in(&mut self, message: &mut Message) { - for component in message { + for component in &mut message.value { if let MessageComponent::Link(link_tag) = component { patch_link!(self, link_tag); } diff --git a/src/slice_file.rs b/src/slice_file.rs index bbbdcb4e..2ba2320d 100644 --- a/src/slice_file.rs +++ b/src/slice_file.rs @@ -4,7 +4,7 @@ use crate::grammar::*; use crate::utils::ptr_util::WeakPtr; use console::style; use serde::Serialize; -use std::cmp::Ordering; +use std::cmp::{max, min, Ordering}; use std::fmt::{Display, Write}; const EXPANDED_TAB: &str = " "; @@ -54,6 +54,18 @@ impl Span { } } +impl std::ops::Add for &Span { + type Output = Span; + + fn add(self, rhs: Self) -> Self::Output { + Span { + start: min(self.start, rhs.start), + end: max(self.end, rhs.end), + file: self.file.clone(), + } + } +} + #[derive(Debug)] pub struct SliceFile { pub filename: String, diff --git a/src/validators/comments.rs b/src/validators/comments.rs index dfda2aa4..1447c385 100644 --- a/src/validators/comments.rs +++ b/src/validators/comments.rs @@ -2,6 +2,7 @@ use crate::diagnostics::{Diagnostic, Diagnostics, Lint}; use crate::grammar::*; +use crate::slice_file::Span; pub fn validate_common_doc_comments(commentable: &dyn Commentable, diagnostics: &mut Diagnostics) { // Only run this validation if a doc comment is present. @@ -15,7 +16,7 @@ pub fn validate_common_doc_comments(commentable: &dyn Commentable, diagnostics: fn only_operations_have_parameters(comment: &DocComment, entity: &dyn Commentable, diagnostics: &mut Diagnostics) { if !matches!(entity.concrete_entity(), Entities::Operation(_)) { for param_tag in &comment.params { - report_only_operation_error(param_tag, entity, diagnostics); + report_only_operation_error(param_tag, param_tag.message.span(), entity, diagnostics); } } } @@ -23,7 +24,7 @@ fn only_operations_have_parameters(comment: &DocComment, entity: &dyn Commentabl fn only_operations_can_return(comment: &DocComment, entity: &dyn Commentable, diagnostics: &mut Diagnostics) { if !matches!(entity.concrete_entity(), Entities::Operation(_)) { for returns_tag in &comment.returns { - report_only_operation_error(returns_tag, entity, diagnostics); + report_only_operation_error(returns_tag, returns_tag.message.span(), entity, diagnostics); } } } @@ -31,13 +32,18 @@ fn only_operations_can_return(comment: &DocComment, entity: &dyn Commentable, di fn only_operations_can_throw(comment: &DocComment, entity: &dyn Commentable, diagnostics: &mut Diagnostics) { if !matches!(entity.concrete_entity(), Entities::Operation(_)) { for throws_tag in &comment.throws { - report_only_operation_error(throws_tag, entity, diagnostics); + report_only_operation_error(throws_tag, throws_tag.message.span(), entity, diagnostics); } } } /// Helper function that reports an error if an operation-only comment-tag was used on something other than a comment. -fn report_only_operation_error(tag: &impl Symbol, entity: &dyn Commentable, diagnostics: &mut Diagnostics) { +fn report_only_operation_error( + tag: &impl Symbol, + message_span: &Span, + entity: &dyn Commentable, + diagnostics: &mut Diagnostics, +) { let entity_kind = entity.kind(); let note = format!( "'{identifier}' is {a} {entity_kind}", @@ -57,7 +63,7 @@ fn report_only_operation_error(tag: &impl Symbol, entity: &dyn Commentable, diag Diagnostic::new(Lint::IncorrectDocComment { message: format!("comment has a '{tag_kind}' tag, but only operations can {action_phrase}"), }) - .set_span(tag.span()) + .set_span(&(tag.span() + message_span)) .set_scope(entity.parser_scoped_identifier()) .add_note(note, Some(entity.span())) .push_into(diagnostics); diff --git a/src/validators/operations.rs b/src/validators/operations.rs index 6d60bfbd..34e5bf33 100644 --- a/src/validators/operations.rs +++ b/src/validators/operations.rs @@ -70,7 +70,7 @@ fn validate_returns_tags_for_operation_with_no_return_type( operation.identifier(), ), }) - .set_span(returns_tag.span()) + .set_span(&(returns_tag.span() + returns_tag.message.span())) .set_scope(operation.parser_scoped_identifier()) .push_into(diagnostics); } @@ -152,7 +152,7 @@ fn validate_throws_tags_for_operation_with_no_throws_clause( operation.identifier(), ), }) - .set_span(throws_tag.span()) + .set_span(&(throws_tag.span() + throws_tag.message.span())) .set_scope(operation.parser_scoped_identifier()) .push_into(diagnostics); } diff --git a/tests/comment_tests.rs b/tests/comment_tests.rs index de32f518..dce5c96e 100644 --- a/tests/comment_tests.rs +++ b/tests/comment_tests.rs @@ -33,7 +33,7 @@ mod comments { assert_eq!(overview.span.start, (4, 16).into()); assert_eq!(overview.span.end, (4, 51).into()); - let message = &overview.message; + let message = &overview.value; assert_eq!(message.len(), 2); let MessageComponent::Text(text) = &message[0] else { panic!() }; assert_eq!(text, "This is a single line doc comment."); @@ -66,7 +66,7 @@ mod comments { assert_eq!(overview.span.start, (4, 16).into()); assert_eq!(overview.span.end, (5, 39).into()); - let message = &overview.message; + let message = &overview.value; assert_eq!(message.len(), 4); let MessageComponent::Text(text) = &message[0] else { panic!() }; assert_eq!(text, "This is a"); @@ -101,7 +101,7 @@ mod comments { let param_tag = ¶m_tags[0]; assert_eq!(param_tag.span.start, (5, 21).into()); - assert_eq!(param_tag.span.end, (5, 52).into()); + assert_eq!(param_tag.span.end, (5, 37).into()); let identifier = ¶m_tag.identifier; assert_eq!(identifier.value, "testParam"); @@ -109,8 +109,12 @@ mod comments { assert_eq!(identifier.span.end, (5, 37).into()); let message = ¶m_tag.message; - assert_eq!(message.len(), 2); - let MessageComponent::Text(text) = &message[0] else { panic!() }; + assert_eq!(message.span.start, (5, 37).into()); + assert_eq!(message.span.end, (5, 52).into()); + + let components = &message.value; + assert_eq!(components.len(), 2); + let MessageComponent::Text(text) = &components[0] else { panic!() }; assert_eq!(text, "My test param"); } @@ -145,7 +149,9 @@ mod comments { assert_eq!(identifier.span.end, (5, 34).into()); let message = &returns_tag.message; - assert!(message.is_empty()); + assert!(message.value.is_empty()); + assert_eq!(message.span.start, (5, 34).into()); + assert_eq!(message.span.end, (5, 34).into()); } #[test] @@ -249,14 +255,18 @@ mod comments { let throws_tag = &throws_tags[0]; assert_eq!(throws_tag.span.start, (8, 21).into()); - assert_eq!(throws_tag.span.end, (8, 72).into()); + assert_eq!(throws_tag.span.end, (8, 40).into()); let thrown_type = throws_tag.thrown_type().unwrap(); assert_eq!(thrown_type.parser_scoped_identifier(), "tests::MyException"); let message = &throws_tag.message; - assert_eq!(message.len(), 2); - let MessageComponent::Text(text) = &message[0] else { panic!() }; + assert_eq!(message.span.start, (8, 40).into()); + assert_eq!(message.span.end, (8, 72).into()); + + let components = &message.value; + assert_eq!(components.len(), 2); + let MessageComponent::Text(text) = &components[0] else { panic!() }; assert_eq!(text, "Message about my thrown thing."); } @@ -351,7 +361,7 @@ mod comments { // Assert let struct_def = ast.find_element::("tests::TestStruct").unwrap(); let overview = &struct_def.comment().unwrap().overview; - let message = &overview.as_ref().unwrap().message; + let message = &overview.as_ref().unwrap().value; assert_eq!(message.len(), 3); let MessageComponent::Text(text) = &message[0] else { panic!() }; diff --git a/tests/diagnostic_output_tests.rs b/tests/diagnostic_output_tests.rs index 882231bb..85bf5937 100644 --- a/tests/diagnostic_output_tests.rs +++ b/tests/diagnostic_output_tests.rs @@ -38,7 +38,7 @@ mod output { // Assert let expected = concat!( - r#"{"message":"comment has a 'param' tag for 'x', but operation 'op' has no parameter with that name","severity":"warning","span":{"start":{"row":5,"col":17},"end":{"row":5,"col":39},"file":"string-0"},"notes":[],"error_code":"IncorrectDocComment"}"#, + r#"{"message":"comment has a 'param' tag for 'x', but operation 'op' has no parameter with that name","severity":"warning","span":{"start":{"row":5,"col":17},"end":{"row":5,"col":25},"file":"string-0"},"notes":[],"error_code":"IncorrectDocComment"}"#, "\n", r#"{"message":"invalid enum 'E': enums must contain at least one enumerator","severity":"error","span":{"start":{"row":9,"col":9},"end":{"row":9,"col":15},"file":"string-0"},"notes":[],"error_code":"E010"}"#, "\n", @@ -86,7 +86,7 @@ warning [IncorrectDocComment]: comment has a 'param' tag for 'x', but operation --> string-0:5:17 | 5 | /// @param x: this is an x - | ---------------------- + | -------- | error [E019]: invalid tag on member 'x': tagged members must be optional --> string-0:8:17 @@ -172,7 +172,7 @@ error [E010]: invalid enum 'E': enums must contain at least one enumerator // Assert: Only one of the two lints should be allowed. let expected = concat!( - r#"{"message":"comment has a 'param' tag for 'x', but operation 'op' has no parameter with that name","severity":"warning","span":{"start":{"row":6,"col":21},"end":{"row":6,"col":43},"file":"string-0"},"notes":[],"error_code":"IncorrectDocComment"}"#, + r#"{"message":"comment has a 'param' tag for 'x', but operation 'op' has no parameter with that name","severity":"warning","span":{"start":{"row":6,"col":21},"end":{"row":6,"col":29},"file":"string-0"},"notes":[],"error_code":"IncorrectDocComment"}"#, "\n", ); assert_eq!(expected, String::from_utf8(output).unwrap());