From 791120ce6d19137b8790f0335631b9d68ba3b222 Mon Sep 17 00:00:00 2001 From: Kevin Newton Date: Tue, 4 Oct 2022 07:37:57 -0400 Subject: [PATCH 1/3] Fix inline comments on loop and conditional predicates --- lib/syntax_tree/node.rb | 2 ++ lib/syntax_tree/parser.rb | 45 +++++++++++++++++++++++---------------- test/fixtures/elsif.rb | 5 +++++ test/fixtures/for.rb | 4 ++++ test/fixtures/if.rb | 4 ++++ test/fixtures/lambda.rb | 4 ++++ test/fixtures/unless.rb | 4 ++++ test/fixtures/until.rb | 4 ++++ test/fixtures/while.rb | 4 ++++ 9 files changed, 58 insertions(+), 18 deletions(-) diff --git a/lib/syntax_tree/node.rb b/lib/syntax_tree/node.rb index 47c534d1..e3d203a7 100644 --- a/lib/syntax_tree/node.rb +++ b/lib/syntax_tree/node.rb @@ -6004,6 +6004,8 @@ def format(q) q.group(0, "->") do if params.is_a?(Paren) q.format(params) unless params.contents.empty? + elsif params.empty? && params.comments.any? + q.format(params) elsif !params.empty? q.group do q.text("(") diff --git a/lib/syntax_tree/parser.rb b/lib/syntax_tree/parser.rb index ed9de499..7e46e856 100644 --- a/lib/syntax_tree/parser.rb +++ b/lib/syntax_tree/parser.rb @@ -302,8 +302,8 @@ def find_next_statement_start(position) def on_BEGIN(statements) lbrace = find_token(LBrace) rbrace = find_token(RBrace) - start_char = find_next_statement_start(lbrace.location.end_char) + start_char = find_next_statement_start(lbrace.location.end_char) statements.bind( start_char, start_char - line_counts[lbrace.location.start_line - 1].start, @@ -340,8 +340,8 @@ def on_CHAR(value) def on_END(statements) lbrace = find_token(LBrace) rbrace = find_token(RBrace) - start_char = find_next_statement_start(lbrace.location.end_char) + start_char = find_next_statement_start(lbrace.location.end_char) statements.bind( start_char, start_char - line_counts[lbrace.location.start_line - 1].start, @@ -831,8 +831,8 @@ def on_brace_block(block_var, statements) lbrace = find_token(LBrace) rbrace = find_token(RBrace) location = (block_var || lbrace).location - start_char = find_next_statement_start(location.end_char) + start_char = find_next_statement_start(location.end_char) statements.bind( start_char, start_char - line_counts[location.start_line - 1].start, @@ -1329,8 +1329,8 @@ def on_else(statements) node = tokens[index] ending = node.value == "end" ? tokens.delete_at(index) : node - start_char = find_next_statement_start(keyword.location.end_char) + start_char = find_next_statement_start(keyword.location.end_char) statements.bind( start_char, start_char - line_counts[keyword.location.start_line - 1].start, @@ -1355,9 +1355,10 @@ def on_elsif(predicate, statements, consequent) beginning = find_token(Kw, "elsif") ending = consequent || find_token(Kw, "end") + start_char = find_next_statement_start(predicate.location.end_char) statements.bind( - predicate.location.end_char, - predicate.location.end_column, + start_char, + start_char - line_counts[predicate.location.start_line - 1].start, ending.location.start_char, ending.location.start_column ) @@ -1598,9 +1599,12 @@ def on_for(index, collection, statements) tokens.delete(keyword) end + start_char = + find_next_statement_start((keyword || collection).location.end_char) statements.bind( - (keyword || collection).location.end_char, - (keyword || collection).location.end_column, + start_char, + start_char - + line_counts[(keyword || collection).location.end_line - 1].start, ending.location.start_char, ending.location.start_column ) @@ -1778,9 +1782,10 @@ def on_if(predicate, statements, consequent) beginning = find_token(Kw, "if") ending = consequent || find_token(Kw, "end") + start_char = find_next_statement_start(predicate.location.end_char) statements.bind( - predicate.location.end_char, - predicate.location.end_column, + start_char, + start_char - line_counts[predicate.location.end_line - 1].start, ending.location.start_char, ending.location.start_column ) @@ -2024,9 +2029,10 @@ def on_lambda(params, statements) closing = find_token(Kw, "end") end + start_char = find_next_statement_start(opening.location.end_char) statements.bind( - opening.location.end_char, - opening.location.end_column, + start_char, + start_char - line_counts[opening.location.end_line - 1].start, closing.location.start_char, closing.location.start_column ) @@ -3456,9 +3462,10 @@ def on_unless(predicate, statements, consequent) beginning = find_token(Kw, "unless") ending = consequent || find_token(Kw, "end") + start_char = find_next_statement_start(predicate.location.end_char) statements.bind( - predicate.location.end_char, - predicate.location.end_column, + start_char, + start_char - line_counts[predicate.location.end_line - 1].start, ending.location.start_char, ending.location.start_column ) @@ -3498,9 +3505,10 @@ def on_until(predicate, statements) end # Update the Statements location information + start_char = find_next_statement_start(predicate.location.end_char) statements.bind( - predicate.location.end_char, - predicate.location.end_column, + start_char, + start_char - line_counts[predicate.location.end_line - 1].start, ending.location.start_char, ending.location.start_column ) @@ -3633,9 +3641,10 @@ def on_while(predicate, statements) end # Update the Statements location information + start_char = find_next_statement_start(predicate.location.end_char) statements.bind( - predicate.location.end_char, - predicate.location.end_column, + start_char, + start_char - line_counts[predicate.location.end_line - 1].start, ending.location.start_char, ending.location.start_column ) diff --git a/test/fixtures/elsif.rb b/test/fixtures/elsif.rb index 2e4cd831..e0dd2bd6 100644 --- a/test/fixtures/elsif.rb +++ b/test/fixtures/elsif.rb @@ -17,3 +17,8 @@ else qyz end +% +if true +elsif false # comment1 + # comment2 +end diff --git a/test/fixtures/for.rb b/test/fixtures/for.rb index 62b207ee..1346a367 100644 --- a/test/fixtures/for.rb +++ b/test/fixtures/for.rb @@ -38,3 +38,7 @@ for foo, in [[foo, bar]] foo end +% +for foo in bar # comment1 + # comment2 +end diff --git a/test/fixtures/if.rb b/test/fixtures/if.rb index 1963d301..cfd6a882 100644 --- a/test/fixtures/if.rb +++ b/test/fixtures/if.rb @@ -63,3 +63,7 @@ if (x = x + 1).to_i x end +% +if true # comment1 + # comment2 +end diff --git a/test/fixtures/lambda.rb b/test/fixtures/lambda.rb index d0cc6f9b..5dba3be3 100644 --- a/test/fixtures/lambda.rb +++ b/test/fixtures/lambda.rb @@ -76,3 +76,7 @@ ) ) do end +% +-> do # comment1 + # comment2 +end diff --git a/test/fixtures/unless.rb b/test/fixtures/unless.rb index c66b16bf..2d5038c1 100644 --- a/test/fixtures/unless.rb +++ b/test/fixtures/unless.rb @@ -32,3 +32,7 @@ unless foo a ? b : c end +% +unless true # comment1 + # comment2 +end diff --git a/test/fixtures/until.rb b/test/fixtures/until.rb index 778e3fb0..f3ef5202 100644 --- a/test/fixtures/until.rb +++ b/test/fixtures/until.rb @@ -23,3 +23,7 @@ until (foo += 1) foo end +% +until true # comment1 + # comment2 +end diff --git a/test/fixtures/while.rb b/test/fixtures/while.rb index 1404f07d..9415135a 100644 --- a/test/fixtures/while.rb +++ b/test/fixtures/while.rb @@ -23,3 +23,7 @@ while (foo += 1) foo end +% +while true # comment1 + # comment2 +end From bcdf200aec51a2097d516d0cffd1a9ab17db3ab6 Mon Sep 17 00:00:00 2001 From: Kevin Newton Date: Tue, 4 Oct 2022 08:24:52 -0400 Subject: [PATCH 2/3] Handle formatting blocks outside of tree --- lib/syntax_tree/node.rb | 26 +++++++++++++++++++------- 1 file changed, 19 insertions(+), 7 deletions(-) diff --git a/lib/syntax_tree/node.rb b/lib/syntax_tree/node.rb index e3d203a7..7ecd69ff 100644 --- a/lib/syntax_tree/node.rb +++ b/lib/syntax_tree/node.rb @@ -1955,11 +1955,12 @@ def format(q) # If the receiver of this block a Command or CommandCall node, then there # are no parentheses around the arguments to that command, so we need to # break the block. - receiver = q.parent.call - if receiver.is_a?(Command) || receiver.is_a?(CommandCall) + case q.parent + in { call: Command | CommandCall } q.break_parent format_break(q, break_opening, break_closing) return + else end q.group do @@ -1978,16 +1979,26 @@ def unchangeable_bounds?(q) # If we hit a statements, then we're safe to use whatever since we # know for certain we're going to get split over multiple lines # anyway. - break false if parent.is_a?(Statements) || parent.is_a?(ArgParen) - - [Command, CommandCall].include?(parent.class) + case parent + in Statements | ArgParen + break false + in Command | CommandCall + true + else + false + end end end # If we're a sibling of a control-flow keyword, then we're going to have to # use the do..end bounds. def forced_do_end_bounds?(q) - [Break, Next, Return, Super].include?(q.parent.call.class) + case q.parent + in { call: Break | Next | Return | Super } + true + else + false + end end # If we're the predicate of a loop or conditional, then we're going to have @@ -2314,7 +2325,8 @@ def comments end def format(q) - if operator == :"::" || (operator.is_a?(Op) && operator.value == "::") + case operator + in :"::" | Op[value: "::"] q.text(".") else operator.format(q) From 99492653c851a40ea07d9be61c01d866bf3723cd Mon Sep 17 00:00:00 2001 From: Kevin Newton Date: Tue, 4 Oct 2022 12:00:48 -0400 Subject: [PATCH 3/3] Bump to version v3.6.2 --- CHANGELOG.md | 10 +++++++++- Gemfile.lock | 2 +- lib/syntax_tree/version.rb | 2 +- 3 files changed, 11 insertions(+), 3 deletions(-) diff --git a/CHANGELOG.md b/CHANGELOG.md index 91d1f2c1..79145063 100644 --- a/CHANGELOG.md +++ b/CHANGELOG.md @@ -6,6 +6,13 @@ The format is based on [Keep a Changelog](http://keepachangelog.com/en/1.0.0/) a ## [Unreleased] +## [3.6.2] - 2022-10-04 + +### Changed + +- [#165](https://github.com/ruby-syntax-tree/syntax_tree/pull/165) - Conditionals (`if`/`unless`), loops (`for`/`while`/`until`) and lambdas all had issues when comments immediately succeeded the declaration of the node where the comment could potentially be dropped. That has now been fixed. +- [#166](https://github.com/ruby-syntax-tree/syntax_tree/pull/166) - Blocks can now be formatted even if they are the top node of the tree. Previously they were looking to their parent for some additional metadata, so we now handle the case where the parent is nil. + ## [3.6.1] - 2022-09-28 ### Changed @@ -357,7 +364,8 @@ The format is based on [Keep a Changelog](http://keepachangelog.com/en/1.0.0/) a - 🎉 Initial release! 🎉 -[unreleased]: https://github.com/ruby-syntax-tree/syntax_tree/compare/v3.6.1...HEAD +[unreleased]: https://github.com/ruby-syntax-tree/syntax_tree/compare/v3.6.2...HEAD +[3.6.2]: https://github.com/ruby-syntax-tree/syntax_tree/compare/v3.6.1...v3.6.2 [3.6.1]: https://github.com/ruby-syntax-tree/syntax_tree/compare/v3.6.0...v3.6.1 [3.6.0]: https://github.com/ruby-syntax-tree/syntax_tree/compare/v3.5.0...v3.6.0 [3.5.0]: https://github.com/ruby-syntax-tree/syntax_tree/compare/v3.4.0...v3.5.0 diff --git a/Gemfile.lock b/Gemfile.lock index 9d60382c..62b8559b 100644 --- a/Gemfile.lock +++ b/Gemfile.lock @@ -1,7 +1,7 @@ PATH remote: . specs: - syntax_tree (3.6.1) + syntax_tree (3.6.2) prettier_print GEM diff --git a/lib/syntax_tree/version.rb b/lib/syntax_tree/version.rb index e7bf7655..850facb2 100644 --- a/lib/syntax_tree/version.rb +++ b/lib/syntax_tree/version.rb @@ -1,5 +1,5 @@ # frozen_string_literal: true module SyntaxTree - VERSION = "3.6.1" + VERSION = "3.6.2" end