Skip to content
New issue

Have a question about this project? # for a free GitHub account to open an issue and contact its maintainers and the community.

By clicking “#”, you agree to our terms of service and privacy statement. We’ll occasionally send you account related emails.

Already on GitHub? # to your account

Separate the Spans of Messages and Tags in Doc Comments #670

Merged
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
18 changes: 8 additions & 10 deletions src/grammar/comments.rs
Original file line number Diff line number Diff line change
Expand Up @@ -5,20 +5,14 @@ use crate::slice_file::Span;

#[derive(Debug)]
pub struct DocComment {
pub overview: Option<Overview>,
pub overview: Option<Message>,
pub params: Vec<ParamTag>,
pub returns: Vec<ReturnsTag>,
pub throws: Vec<ThrowsTag>,
pub see: Vec<SeeTag>,
pub span: Span,
}

#[derive(Debug)]
pub struct Overview {
pub message: Message,
pub span: Span,
}

#[derive(Debug)]
pub struct ParamTag {
pub identifier: Identifier,
Expand Down Expand Up @@ -85,12 +79,14 @@ pub enum MessageComponent {
Link(LinkTag),
}

pub type Message = Vec<MessageComponent>;
#[derive(Debug)]
pub struct Message {
pub value: Vec<MessageComponent>,
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");
Expand All @@ -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);
25 changes: 11 additions & 14 deletions src/parsers/comments/grammar.lalrpop
Original file line number Diff line number Diff line change
Expand Up @@ -40,7 +40,7 @@ extern {
// Grammar Rules

pub DocComment: DocComment = {
<l: @L> <overview: Overview?> => create_doc_comment(overview, l, comment_parser.file_name),
<l: @L> <overview: MessageLines?> => create_doc_comment(overview, l, comment_parser.file_name),
<mut comment: DocComment> <param_block: ParamBlock> => {
append_tag_to_comment!(comment, params, param_block)
},
Expand All @@ -55,29 +55,22 @@ pub DocComment: DocComment = {
},
}

Overview: Overview = {
<l: @L> <message: MessageLines> <r: @R> => {
let span = Span::new(l, r, comment_parser.file_name);
Overview { message, span }
},
}

ParamBlock: ParamTag = {
<l: @L> param_keyword <identifier: Identifier> <message: Section> <r: @R> => {
<l: @L> param_keyword <identifier: Identifier> <r: @R> <message: Section> => {
let span = Span::new(l, r, comment_parser.file_name);
ParamTag { identifier, message, span }
},
}

ReturnsBlock: ReturnsTag = {
<l: @L> returns_keyword <identifier: Identifier?> <message: Section> <r: @R> => {
<l: @L> returns_keyword <identifier: Identifier?> <r: @R> <message: Section> => {
let span = Span::new(l, r, comment_parser.file_name);
ReturnsTag { identifier, message, span }
},
}

ThrowsBlock: ThrowsTag = {
<l: @L> throws_keyword <identifier: ScopedIdentifier> <message: Section> <r: @R> => {
<l: @L> throws_keyword <identifier: ScopedIdentifier> <r: @R> <message: Section> => {
let span = Span::new(l, r, comment_parser.file_name);
let thrown_type = TypeRefDefinition::Unpatched(identifier);
ThrowsTag { thrown_type, message, span }
Expand All @@ -99,13 +92,17 @@ InlineLink: LinkTag = {
}

Section: Message = {
<inline_message: (":" <Message?>)?> newline <message_lines: MessageLines?> => {
construct_section_message(inline_message.flatten(), message_lines)
<l: @L> <inline_message: (":" <Message?>)?> newline <message_lines: MessageLines?> <r: @R> => {
let span = Span::new(l, r, comment_parser.file_name);
construct_section_message(inline_message.flatten(), message_lines, span)
},
}

MessageLines: Message = {
(<Message?> newline)+ => sanitize_message_lines(<>),
<l: @L> <m: (<Message?> newline)+> <r: @R> => {
let span = Span::new(l, r, comment_parser.file_name);
sanitize_message_lines(m, span)
}
}

Message = MessageComponent+;
Expand Down
28 changes: 18 additions & 10 deletions src/parsers/comments/grammar.rs
Original file line number Diff line number Diff line change
Expand Up @@ -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;

Expand All @@ -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<Overview>, start: Location, file: &str) -> DocComment {
fn create_doc_comment(overview: Option<Message>, 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);
Expand Down Expand Up @@ -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>, message_lines: Option<Message>) -> Message {
let mut message_lines = message_lines.unwrap_or_default();
fn construct_section_message(
inline_message: Option<Vec<MessageComponent>>,
message_lines: Option<Message>,
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.
Expand All @@ -75,17 +79,19 @@ fn construct_section_message(inline_message: Option<Message>, 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<Option<Message>>) -> Message {
///
/// Note that the message's span is not updated to reflect the stripping of common leading whitespace.
fn sanitize_message_lines(lines: Vec<Option<Vec<MessageComponent>>>, 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 {
Expand All @@ -111,7 +117,7 @@ fn sanitize_message_lines(lines: Vec<Option<Message>>) -> 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.
Expand All @@ -126,5 +132,7 @@ fn sanitize_message_lines(lines: Vec<Option<Message>>) -> 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 }
}
8 changes: 4 additions & 4 deletions src/patchers/comment_link_patcher.rs
Original file line number Diff line number Diff line change
Expand Up @@ -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(&param_tag.message, commentable, ast);
Expand All @@ -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);
}
Expand Down Expand Up @@ -146,7 +146,7 @@ impl CommentLinkPatcher<'_> {
fn apply_patches(&mut self, scope: &str, comment: &mut Option<DocComment>) {
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);
Expand All @@ -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);
}
Expand Down
14 changes: 13 additions & 1 deletion src/slice_file.rs
Original file line number Diff line number Diff line change
Expand Up @@ -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 = " ";
Expand Down Expand Up @@ -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,
Expand Down
16 changes: 11 additions & 5 deletions src/validators/comments.rs
Original file line number Diff line number Diff line change
Expand Up @@ -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.
Expand All @@ -15,29 +16,34 @@ 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);
}
}
}

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);
}
}
}

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}",
Expand All @@ -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);
Expand Down
4 changes: 2 additions & 2 deletions src/validators/operations.rs
Original file line number Diff line number Diff line change
Expand Up @@ -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);
}
Expand Down Expand Up @@ -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);
}
Expand Down
Loading