From 89c4f44f22a9db76d39931a2f3a73d2e665bd9bb Mon Sep 17 00:00:00 2001 From: JohnnyMorganz Date: Thu, 30 Mar 2023 12:35:43 +0100 Subject: [PATCH 1/5] Add test cases --- tests/inputs/assignment-comments-3.lua | 8 ++++++++ tests/inputs/return-comments-2.lua | 5 +++++ 2 files changed, 13 insertions(+) create mode 100644 tests/inputs/assignment-comments-3.lua create mode 100644 tests/inputs/return-comments-2.lua diff --git a/tests/inputs/assignment-comments-3.lua b/tests/inputs/assignment-comments-3.lua new file mode 100644 index 00000000..560c4509 --- /dev/null +++ b/tests/inputs/assignment-comments-3.lua @@ -0,0 +1,8 @@ +-- https://github.com/JohnnyMorganz/StyLua/issues/662 +local a, b += 1 -- adoc +, 2 -- bdoc + +local a -- adoc +, b -- bdoc += 1, 2 diff --git a/tests/inputs/return-comments-2.lua b/tests/inputs/return-comments-2.lua new file mode 100644 index 00000000..8a12ea82 --- /dev/null +++ b/tests/inputs/return-comments-2.lua @@ -0,0 +1,5 @@ +-- https://github.com/JohnnyMorganz/StyLua/issues/662 +function f() + return a -- adoc + , b -- bdoc +end From 066c9120050b5f68987d7d6104b8d4e7b08f4145 Mon Sep 17 00:00:00 2001 From: JohnnyMorganz Date: Thu, 30 Mar 2023 13:04:57 +0100 Subject: [PATCH 2/5] Properly handle comments when formatting a punctuated list multiline --- src/formatters/assignment.rs | 6 +-- src/formatters/block.rs | 2 +- src/formatters/general.rs | 37 +++++++++----- src/formatters/trivia_util.rs | 90 ++++++++++++++++++++++++++++++++--- 4 files changed, 112 insertions(+), 23 deletions(-) diff --git a/src/formatters/assignment.rs b/src/formatters/assignment.rs index bd6c74f1..003e8a6f 100644 --- a/src/formatters/assignment.rs +++ b/src/formatters/assignment.rs @@ -181,7 +181,7 @@ fn attempt_assignment_tactics( format_expression, ); - if trivia_util::punctuated_expression_inline_comments(expressions) + if trivia_util::punctuated_inline_comments(expressions, true) || hanging_shape .take_first_line(&strip_trivia(&expr_list)) .over_budget() @@ -331,7 +331,7 @@ pub fn format_assignment_no_trivia( // Check if the assignment expressions or equal token contain comments. If they do, we bail out of determining any tactics // and format multiline let contains_comments = trivia_util::token_contains_comments(assignment.equal_token()) - || trivia_util::punctuated_expression_inline_comments(assignment.expressions()); + || trivia_util::punctuated_inline_comments(assignment.expressions(), true); // Firstly attempt to format the assignment onto a single line, using an infinite column width shape let mut var_list = try_format_punctuated( @@ -430,7 +430,7 @@ pub fn format_local_assignment_no_trivia( let contains_comments = assignment .equal_token() .map_or(false, trivia_util::token_contains_comments) - || trivia_util::punctuated_expression_inline_comments(assignment.expressions()); + || trivia_util::punctuated_inline_comments(assignment.expressions(), true); // Firstly attempt to format the assignment onto a single line, using an infinite column width shape let local_token = fmt_symbol!(ctx, assignment.local_token(), "local ", shape); diff --git a/src/formatters/block.rs b/src/formatters/block.rs index 94e089d4..b2eb0de9 100644 --- a/src/formatters/block.rs +++ b/src/formatters/block.rs @@ -55,7 +55,7 @@ pub fn format_return(ctx: &Context, return_node: &Return, shape: Shape) -> Retur ); let contains_comments = return_token_trailing_comments - || trivia_util::punctuated_expression_inline_comments(returns); + || trivia_util::punctuated_inline_comments(returns, true); let is_function_or_table = returns.iter().all(is_function_or_table_constructor); // See if we need to format multiline diff --git a/src/formatters/general.rs b/src/formatters/general.rs index 0c5828b3..a34f9f7e 100644 --- a/src/formatters/general.rs +++ b/src/formatters/general.rs @@ -2,7 +2,10 @@ use crate::{ context::{create_indent_trivia, create_newline_trivia, Context, FormatNode}, formatters::{ trivia::{FormatTriviaType, UpdateLeadingTrivia, UpdateTrailingTrivia, UpdateTrivia}, - trivia_util::{self, GetLeadingTrivia}, + trivia_util::{ + self, punctuated_inline_comments, GetLeadingTrivia, GetTrailingTrivia, + HasInlineComments, + }, }, shape::Shape, QuoteStyle, @@ -384,7 +387,7 @@ pub fn format_punctuated_multiline( hang_level: Option, ) -> Punctuated where - T: Node + GetLeadingTrivia + UpdateLeadingTrivia, + T: Node + GetLeadingTrivia + UpdateLeadingTrivia + GetTrailingTrivia + UpdateTrailingTrivia, F: Fn(&Context, &T, Shape) -> T, { let mut formatted: Punctuated = Punctuated::new(); @@ -412,7 +415,16 @@ where trivia_util::prepend_newline_indent(ctx, &value, hanging_shape) }; + // Handle any comments in between the value and the punctuation + // If they are present, then move them to after the punctuation + let mut trailing_comments = value.trailing_comments(); + let value = value.update_trailing_trivia(FormatTriviaType::Replace(vec![])); + let punctuation = fmt_symbol!(ctx, punctuation, ",", shape); + trailing_comments.append(&mut punctuation.trailing_trivia().cloned().collect()); + let punctuation = punctuation + .update_trailing_trivia(FormatTriviaType::Replace(trailing_comments)); + formatted.push(Pair::new(value, Some(punctuation))); } Pair::End(value) => { @@ -440,19 +452,18 @@ pub fn try_format_punctuated( hang_level: Option, ) -> Punctuated where - T: Node + GetLeadingTrivia + UpdateLeadingTrivia + std::fmt::Display, + T: Node + + GetLeadingTrivia + + UpdateLeadingTrivia + + GetTrailingTrivia + + UpdateTrailingTrivia + + HasInlineComments + + std::fmt::Display, F: Fn(&Context, &T, Shape) -> T, { - let mut format_multiline = false; - - for pair in old.pairs() { - if let Pair::Punctuated(_, punctuation) = pair { - if trivia_util::contains_comments(punctuation) { - format_multiline = true; - break; - } - } - } + // TODO: we do not check the leading comments of the punctuated list for determining multiline + // Maybe we should do later? + let format_multiline = punctuated_inline_comments(old, false); if format_multiline { format_punctuated_multiline(ctx, old, shape, value_formatter, hang_level) diff --git a/src/formatters/trivia_util.rs b/src/formatters/trivia_util.rs index 82a82220..e8bc4108 100644 --- a/src/formatters/trivia_util.rs +++ b/src/formatters/trivia_util.rs @@ -1307,17 +1307,39 @@ pub fn expression_contains_inline_comments(expression: &Expression) -> bool { } } -pub fn punctuated_expression_inline_comments(punctuated: &Punctuated) -> bool { - punctuated.pairs().any(|pair| { - pair.punctuation().map_or(false, token_contains_comments) - || !expression_leading_comments(pair.value()).is_empty() - || expression_contains_inline_comments(pair.value()) - }) +pub fn punctuated_inline_comments( + punctuated: &Punctuated, + include_leading: bool, +) -> bool { + let mut iter = punctuated.pairs().peekable(); + while let Some(pair) = iter.next() { + // Only check trailing comments on the expression if this is not the last pair + if iter.peek().is_some() && !pair.value().trailing_comments().is_empty() { + return true; + } + + if pair.punctuation().map_or(false, token_contains_comments) + || (include_leading && !pair.value().leading_comments().is_empty()) + || pair.value().has_inline_comments() + { + return true; + } + } + + false } // TODO: can we change this from returning a Vec to just a plain iterator? pub trait GetLeadingTrivia { fn leading_trivia(&self) -> Vec; + + fn leading_comments(&self) -> Vec { + self.leading_trivia() + .iter() + .filter(|token| trivia_is_comment(token)) + .cloned() + .collect() + } } impl GetLeadingTrivia for TokenReference { @@ -1376,6 +1398,62 @@ impl GetLeadingTrivia for Prefix { } } +pub trait GetTrailingTrivia { + fn trailing_trivia(&self) -> Vec; + + // Retrieves all the trailing comments from the token + // Prepends a space before each comment + fn trailing_comments(&self) -> Vec { + self.trailing_trivia() + .iter() + .filter(|token| trivia_is_comment(token)) + .flat_map(|x| { + // Prepend a single space beforehand + vec![Token::new(TokenType::spaces(1)), x.to_owned()] + }) + .collect() + } +} + +impl GetTrailingTrivia for TokenReference { + fn trailing_trivia(&self) -> Vec { + self.trailing_trivia().cloned().collect() + } +} + +impl GetTrailingTrivia for Suffix { + fn trailing_trivia(&self) -> Vec { + suffix_trailing_trivia(self) + } +} + +impl GetTrailingTrivia for Expression { + fn trailing_trivia(&self) -> Vec { + get_expression_trailing_trivia(self) + } +} + +impl GetTrailingTrivia for Var { + fn trailing_trivia(&self) -> Vec { + var_trailing_trivia(self) + } +} + +pub trait HasInlineComments { + fn has_inline_comments(&self) -> bool { + false + } +} + +impl HasInlineComments for Expression { + fn has_inline_comments(&self) -> bool { + expression_contains_inline_comments(self) + } +} + +impl HasInlineComments for Var {} +impl HasInlineComments for TokenReference {} + // Commonly, we update trivia to add in a newline and indent trivia to the leading trivia of a token/node. // An issue with this is if we do not properly take into account comments. This function also handles any comments present // by also interspersing them with the required newline and indentation, so they are aligned correctly. From fefbb95d1cf5f889317ef530f6e9c25aefd41168 Mon Sep 17 00:00:00 2001 From: JohnnyMorganz Date: Thu, 30 Mar 2023 13:05:09 +0100 Subject: [PATCH 3/5] Update snapshots --- .../tests__standard@assignment-comments-3.lua.snap | 11 +++++++++++ .../tests__standard@return-comments-2.lua.snap | 10 ++++++++++ 2 files changed, 21 insertions(+) create mode 100644 tests/snapshots/tests__standard@assignment-comments-3.lua.snap create mode 100644 tests/snapshots/tests__standard@return-comments-2.lua.snap diff --git a/tests/snapshots/tests__standard@assignment-comments-3.lua.snap b/tests/snapshots/tests__standard@assignment-comments-3.lua.snap new file mode 100644 index 00000000..b4384a93 --- /dev/null +++ b/tests/snapshots/tests__standard@assignment-comments-3.lua.snap @@ -0,0 +1,11 @@ +--- +source: tests/tests.rs +expression: format(&contents) +--- +-- https://github.com/JohnnyMorganz/StyLua/issues/662 +local a, b = + 1, -- adoc + 2 -- bdoc + +local a -- adoc, b -- bdoc = 1, 2 + diff --git a/tests/snapshots/tests__standard@return-comments-2.lua.snap b/tests/snapshots/tests__standard@return-comments-2.lua.snap new file mode 100644 index 00000000..3bdd8f6b --- /dev/null +++ b/tests/snapshots/tests__standard@return-comments-2.lua.snap @@ -0,0 +1,10 @@ +--- +source: tests/tests.rs +expression: format(&contents) +--- +-- https://github.com/JohnnyMorganz/StyLua/issues/662 +function f() + return a, -- adoc + b -- bdoc +end + From dc8db843ee2ad96bc678c796e5a1fa5843766a4b Mon Sep 17 00:00:00 2001 From: JohnnyMorganz Date: Thu, 30 Mar 2023 13:16:24 +0100 Subject: [PATCH 4/5] Hang equals token for comments --- src/formatters/assignment.rs | 26 +++++++++++++++++-- src/formatters/trivia_util.rs | 25 +++++++++++------- ...s__standard@assignment-comments-3.lua.snap | 4 ++- 3 files changed, 43 insertions(+), 12 deletions(-) diff --git a/src/formatters/assignment.rs b/src/formatters/assignment.rs index 003e8a6f..0d05a329 100644 --- a/src/formatters/assignment.rs +++ b/src/formatters/assignment.rs @@ -31,6 +31,8 @@ use crate::{ shape::Shape, }; +use super::trivia_util::{prepend_newline_indent, GetTrailingTrivia}; + /// Calculates the hanging level to use when hanging an expression. /// By default, we indent one further, but we DO NOT want to do this if the expression is just parentheses (or a unary operation on them) /// https://github.com/JohnnyMorganz/StyLua/issues/274 @@ -326,7 +328,7 @@ fn attempt_assignment_tactics( pub fn format_assignment_no_trivia( ctx: &Context, assignment: &Assignment, - shape: Shape, + mut shape: Shape, ) -> Assignment { // Check if the assignment expressions or equal token contain comments. If they do, we bail out of determining any tactics // and format multiline @@ -349,6 +351,16 @@ pub fn format_assignment_no_trivia( format_expression, ); + // If the var list ended with a comment, we need to hang the equals token + if var_list.has_trailing_comments(trivia_util::CommentSearch::Single) { + const EQUAL_TOKEN_LEN: usize = "= ".len(); + shape = shape + .reset() + .increment_additional_indent() + .add_width(EQUAL_TOKEN_LEN); + equal_token = prepend_newline_indent(ctx, &equal_token, shape); + } + // Test the assignment to see if its over width let singleline_shape = shape + (strip_leading_trivia(&var_list).to_string().len() @@ -420,7 +432,7 @@ fn format_local_no_assignment( pub fn format_local_assignment_no_trivia( ctx: &Context, assignment: &LocalAssignment, - shape: Shape, + mut shape: Shape, ) -> LocalAssignment { if assignment.expressions().is_empty() { format_local_no_assignment(ctx, assignment, shape) @@ -477,6 +489,16 @@ pub fn format_local_assignment_no_trivia( }); } + // If the var list ended with a comment, we need to hang the equals token + if name_list.has_trailing_comments(trivia_util::CommentSearch::Single) { + const EQUAL_TOKEN_LEN: usize = "= ".len(); + shape = shape + .reset() + .increment_additional_indent() + .add_width(EQUAL_TOKEN_LEN); + equal_token = prepend_newline_indent(ctx, &equal_token, shape); + } + // Test the assignment to see if its over width let singleline_shape = shape + (strip_leading_trivia(&name_list).to_string().len() diff --git a/src/formatters/trivia_util.rs b/src/formatters/trivia_util.rs index e8bc4108..74536df7 100644 --- a/src/formatters/trivia_util.rs +++ b/src/formatters/trivia_util.rs @@ -450,13 +450,6 @@ pub fn get_expression_leading_trivia(expression: &Expression) -> Vec { } } -pub fn punctuated_leading_trivia(punctuated: &Punctuated) -> Vec { - punctuated - .iter() - .next() - .map_or_else(Vec::new, get_expression_leading_trivia) -} - pub fn binop_leading_comments(binop: &BinOp) -> Vec { match binop { BinOp::And(token) @@ -1360,9 +1353,11 @@ impl GetLeadingTrivia for Expression { } } -impl GetLeadingTrivia for Punctuated { +impl GetLeadingTrivia for Punctuated { fn leading_trivia(&self) -> Vec { - punctuated_leading_trivia(self) + self.iter() + .next() + .map_or_else(Vec::new, GetLeadingTrivia::leading_trivia) } } @@ -1401,6 +1396,10 @@ impl GetLeadingTrivia for Prefix { pub trait GetTrailingTrivia { fn trailing_trivia(&self) -> Vec; + fn has_trailing_comments(&self, search: CommentSearch) -> bool { + trivia_contains_comments(self.trailing_trivia().iter(), search) + } + // Retrieves all the trailing comments from the token // Prepends a space before each comment fn trailing_comments(&self) -> Vec { @@ -1439,6 +1438,14 @@ impl GetTrailingTrivia for Var { } } +impl GetTrailingTrivia for Punctuated { + fn trailing_trivia(&self) -> Vec { + self.iter() + .last() + .map_or_else(Vec::new, GetTrailingTrivia::trailing_trivia) + } +} + pub trait HasInlineComments { fn has_inline_comments(&self) -> bool { false diff --git a/tests/snapshots/tests__standard@assignment-comments-3.lua.snap b/tests/snapshots/tests__standard@assignment-comments-3.lua.snap index b4384a93..f092bd01 100644 --- a/tests/snapshots/tests__standard@assignment-comments-3.lua.snap +++ b/tests/snapshots/tests__standard@assignment-comments-3.lua.snap @@ -7,5 +7,7 @@ local a, b = 1, -- adoc 2 -- bdoc -local a -- adoc, b -- bdoc = 1, 2 +local a, -- adoc + b -- bdoc + = 1, 2 From a441dee7a27bfcada3fe7fe66bbf55eed6b52dbd Mon Sep 17 00:00:00 2001 From: JohnnyMorganz Date: Thu, 30 Mar 2023 13:17:14 +0100 Subject: [PATCH 5/5] Update changelog --- CHANGELOG.md | 1 + 1 file changed, 1 insertion(+) diff --git a/CHANGELOG.md b/CHANGELOG.md index 3fb50cfb..e4153cf9 100644 --- a/CHANGELOG.md +++ b/CHANGELOG.md @@ -10,6 +10,7 @@ and this project adheres to [Semantic Versioning](https://semver.org/spec/v2.0.0 ### Fixed - Bumped internal parser dependency which should fix parsing problems for comments with Chinese characters, and multiline string escapes +- Fixed comments in punctuated lists for return statements or assignments being incorrectly formatted leading to syntax errors ([#662](https://github.com/JohnnyMorganz/StyLua/issues/662)) ## [0.17.0] - 2023-03-11