From 37ae88050241cc8d3397d3c9e2ae8d30c4a96c1e Mon Sep 17 00:00:00 2001 From: Kevin Newton Date: Tue, 19 Apr 2022 10:27:20 -0400 Subject: [PATCH 01/25] Fix up Gemfile.lock platforms --- Gemfile.lock | 4 ++++ 1 file changed, 4 insertions(+) diff --git a/Gemfile.lock b/Gemfile.lock index b226dd2e..bb6765ff 100644 --- a/Gemfile.lock +++ b/Gemfile.lock @@ -17,7 +17,11 @@ GEM simplecov_json_formatter (0.1.4) PLATFORMS + arm64-darwin-21 + ruby + x86_64-darwin-19 x86_64-darwin-21 + x86_64-linux DEPENDENCIES bundler From f1253f5b43cbf022fd7f77b8e7a1d9f47193a71c Mon Sep 17 00:00:00 2001 From: Vinicius Stock Date: Wed, 20 Apr 2022 14:32:02 -0400 Subject: [PATCH 02/25] Add convenient Formatter.format --- lib/syntax_tree/formatter.rb | 7 +++++++ test/formatting_test.rb | 5 +++++ 2 files changed, 12 insertions(+) diff --git a/lib/syntax_tree/formatter.rb b/lib/syntax_tree/formatter.rb index 643030ac..95c2adaf 100644 --- a/lib/syntax_tree/formatter.rb +++ b/lib/syntax_tree/formatter.rb @@ -17,6 +17,13 @@ def initialize(source, ...) @quote = "\"" end + def self.format(source, node) + formatter = new(source, []) + node.format(formatter) + formatter.flush + formatter.output.join + end + def format(node, stackable: true) stack << node if stackable doc = nil diff --git a/test/formatting_test.rb b/test/formatting_test.rb index 25059658..5f51d471 100644 --- a/test/formatting_test.rb +++ b/test/formatting_test.rb @@ -9,5 +9,10 @@ class FormattingTest < Minitest::Test assert_equal(fixture.formatted, SyntaxTree.format(fixture.source)) end end + + def test_format_class_level + source = "1+1" + assert_equal("1 + 1\n", SyntaxTree::Formatter.format(source, SyntaxTree.parse(source))) + end end end From 5b00be4954a4a9d679a1db29862a8eff1220187a Mon Sep 17 00:00:00 2001 From: Vinicius Stock Date: Wed, 20 Apr 2022 15:08:42 -0400 Subject: [PATCH 03/25] Make missing kwargs `nil` instead of `false` --- lib/syntax_tree/parser.rb | 6 +++++- 1 file changed, 5 insertions(+), 1 deletion(-) diff --git a/lib/syntax_tree/parser.rb b/lib/syntax_tree/parser.rb index 696a944c..f167aa83 100644 --- a/lib/syntax_tree/parser.rb +++ b/lib/syntax_tree/parser.rb @@ -2079,12 +2079,16 @@ def on_params( keyword_rest, block ) + # This is to make it so that required keyword arguments + # have a `nil` for the value instead of a `false`. + keywords&.map! { |(key, value)| [key, value || nil] } + parts = [ *requireds, *optionals&.flatten(1), rest, *posts, - *keywords&.flat_map { |(key, value)| [key, value || nil] }, + *keywords&.flatten(1), (keyword_rest if keyword_rest != :nil), (block if block != :&) ].compact From f537c00a376d6b6571e84c7a09c8edb5cd301bde Mon Sep 17 00:00:00 2001 From: Drew Bragg Date: Thu, 21 Apr 2022 09:15:18 -0400 Subject: [PATCH 04/25] Allow flow control operators to skip parens when they have single args --- lib/syntax_tree/node.rb | 18 +++++++++++++++++- test/fixtures/next.rb | 28 ++++++++++++++++++++++++++++ 2 files changed, 45 insertions(+), 1 deletion(-) diff --git a/lib/syntax_tree/node.rb b/lib/syntax_tree/node.rb index cdc8856b..b253d7c9 100644 --- a/lib/syntax_tree/node.rb +++ b/lib/syntax_tree/node.rb @@ -2066,7 +2066,12 @@ def format(q) part = arguments.parts.first if part.is_a?(Paren) - q.format(arguments) + if part.contents.body.length == 1 && skip_parens?(part.contents.body.first) + q.text(" ") + q.format(part.contents.body.first) + else + q.format(arguments) + end elsif part.is_a?(ArrayLiteral) q.text(" ") q.format(arguments) @@ -2091,6 +2096,17 @@ def format_arguments(q, opening, closing) q.breakable("") q.if_break { q.text(closing) } end + + def skip_parens?(node) + case node + in Int | FloatLiteral + true + in VarRef[value: GVar | IVar | CVar | Kw | Const] + true + else + false + end + end end # Break represents using the +break+ keyword. diff --git a/test/fixtures/next.rb b/test/fixtures/next.rb index ad5bac36..61d31027 100644 --- a/test/fixtures/next.rb +++ b/test/fixtures/next.rb @@ -25,3 +25,31 @@ foo bar ) +% +next(1) +- +next 1 +% +next(1.0) +- +next 1.0 +% +next($a) +- +next $a +% +next(@@a) +- +next @@a +% +next(self) +- +next self +% +next(@a) +- +next @a +% +next(A) +- +next A From 34f77fe050f67e1ceb4c6273c446c4dee18f9394 Mon Sep 17 00:00:00 2001 From: Drew Bragg Date: Thu, 21 Apr 2022 09:17:38 -0400 Subject: [PATCH 05/25] Do not create single line conditional if conditional is a single line conditional --- lib/syntax_tree/node.rb | 10 ++++++++++ test/fixtures/if.rb | 4 ++++ test/fixtures/unless.rb | 4 ++++ 3 files changed, 18 insertions(+) diff --git a/lib/syntax_tree/node.rb b/lib/syntax_tree/node.rb index b253d7c9..bd3f915f 100644 --- a/lib/syntax_tree/node.rb +++ b/lib/syntax_tree/node.rb @@ -4680,6 +4680,11 @@ def format(q) return end + if contains_conditional? + format_break(q, force: true) + return + end + if node.consequent || node.statements.empty? q.group { format_break(q, force: true) } else @@ -4716,6 +4721,11 @@ def format_break(q, force:) q.breakable(force: force) q.text("end") end + + def contains_conditional? + node.statements.body.length == 1 && + [If, IfMod, IfOp, Unless, UnlessMod].include?(node.statements.body.first.class) + end end # If represents the first clause in an +if+ chain. diff --git a/test/fixtures/if.rb b/test/fixtures/if.rb index 4331abb1..ed5e5a30 100644 --- a/test/fixtures/if.rb +++ b/test/fixtures/if.rb @@ -28,3 +28,7 @@ if (foo += 1) foo end +% +if foo + a ? b : c +end diff --git a/test/fixtures/unless.rb b/test/fixtures/unless.rb index 95a40c38..c66b16bf 100644 --- a/test/fixtures/unless.rb +++ b/test/fixtures/unless.rb @@ -28,3 +28,7 @@ unless (foo += 1) foo end +% +unless foo + a ? b : c +end From d0b80782cce9dbe7bf4d0a27f6141293377465f4 Mon Sep 17 00:00:00 2001 From: Drew Bragg Date: Thu, 21 Apr 2022 21:15:38 -0400 Subject: [PATCH 06/25] Add return example to fixture --- test/fixtures/return.rb | 4 ++++ 1 file changed, 4 insertions(+) diff --git a/test/fixtures/return.rb b/test/fixtures/return.rb index e1989cb4..243f14d2 100644 --- a/test/fixtures/return.rb +++ b/test/fixtures/return.rb @@ -25,3 +25,7 @@ foo bar ) +% +return([1, 2, 3]) +- +return 1, 2, 3 From fd435eff2462ce431f97c5fcd51b661fdeb13d83 Mon Sep 17 00:00:00 2001 From: Drew Bragg Date: Thu, 21 Apr 2022 21:16:22 -0400 Subject: [PATCH 07/25] Handle Array arg in parens --- lib/syntax_tree/node.rb | 4 +++- 1 file changed, 3 insertions(+), 1 deletion(-) diff --git a/lib/syntax_tree/node.rb b/lib/syntax_tree/node.rb index bd3f915f..390f8c9b 100644 --- a/lib/syntax_tree/node.rb +++ b/lib/syntax_tree/node.rb @@ -2068,7 +2068,9 @@ def format(q) if part.is_a?(Paren) if part.contents.body.length == 1 && skip_parens?(part.contents.body.first) q.text(" ") - q.format(part.contents.body.first) + contents = part.contents.body.first + contents = contents.contents if contents.is_a?(ArrayLiteral) + q.format(contents) else q.format(arguments) end From 30b22de3649832bb4b2f0f2477cb02b044aef629 Mon Sep 17 00:00:00 2001 From: Drew Bragg Date: Thu, 21 Apr 2022 21:16:53 -0400 Subject: [PATCH 08/25] Add return array w/ no parens examples to fixture --- test/fixtures/return.rb | 4 ++++ 1 file changed, 4 insertions(+) diff --git a/test/fixtures/return.rb b/test/fixtures/return.rb index 243f14d2..b57e5bad 100644 --- a/test/fixtures/return.rb +++ b/test/fixtures/return.rb @@ -29,3 +29,7 @@ return([1, 2, 3]) - return 1, 2, 3 +% +return [1, 2, 3] +- +return 1, 2, 3 From fe988e2b2646e30dbfd4c9a5abb0fc7456fec1ef Mon Sep 17 00:00:00 2001 From: Drew Bragg Date: Thu, 21 Apr 2022 21:17:25 -0400 Subject: [PATCH 09/25] Correctly handle `return Array` --- lib/syntax_tree/node.rb | 53 +++++++++++++++++++++-------------------- 1 file changed, 27 insertions(+), 26 deletions(-) diff --git a/lib/syntax_tree/node.rb b/lib/syntax_tree/node.rb index 390f8c9b..6b43d28e 100644 --- a/lib/syntax_tree/node.rb +++ b/lib/syntax_tree/node.rb @@ -2076,7 +2076,7 @@ def format(q) end elsif part.is_a?(ArrayLiteral) q.text(" ") - q.format(arguments) + q.format(part.contents) else format_arguments(q, "(", ")") end @@ -2089,6 +2089,7 @@ def format(q) private + def format_arguments(q, opening, closing) q.if_break { q.text(opening) } q.indent do @@ -2101,7 +2102,7 @@ def format_arguments(q, opening, closing) def skip_parens?(node) case node - in Int | FloatLiteral + in Int | FloatLiteral | ArrayLiteral true in VarRef[value: GVar | IVar | CVar | Kw | Const] true @@ -2525,7 +2526,7 @@ def child_nodes end alias deconstruct child_nodes - + def deconstruct_keys(keys) { value: value, location: location } end @@ -3810,7 +3811,7 @@ def child_nodes end alias deconstruct child_nodes - + def deconstruct_keys(keys) { value: value, location: location } end @@ -3840,7 +3841,7 @@ def child_nodes end alias deconstruct child_nodes - + def deconstruct_keys(keys) { value: value, location: location } end @@ -3872,7 +3873,7 @@ def child_nodes end alias deconstruct child_nodes - + def deconstruct_keys(keys) { value: value, location: location } end @@ -5313,7 +5314,7 @@ def child_nodes end alias deconstruct child_nodes - + def deconstruct_keys(keys) { value: value, location: location } end @@ -6438,7 +6439,7 @@ def child_nodes end alias deconstruct child_nodes - + def deconstruct_keys(keys) { value: value, location: location } end @@ -6538,7 +6539,7 @@ def child_nodes end alias deconstruct child_nodes - + def deconstruct_keys(keys) { value: value, location: location } end @@ -6599,7 +6600,7 @@ def child_nodes end alias deconstruct child_nodes - + def deconstruct_keys(keys) { value: value, location: location } end @@ -6624,7 +6625,7 @@ def child_nodes end alias deconstruct child_nodes - + def deconstruct_keys(keys) { value: value, location: location } end @@ -6695,7 +6696,7 @@ def child_nodes end alias deconstruct child_nodes - + def deconstruct_keys(keys) { beginning: beginning, parts: parts, location: location } end @@ -6728,7 +6729,7 @@ def child_nodes end alias deconstruct child_nodes - + def deconstruct_keys(keys) { value: value, location: location } end @@ -6762,7 +6763,7 @@ def child_nodes end alias deconstruct child_nodes - + def deconstruct_keys(keys) { value: value, location: location } end @@ -7248,7 +7249,7 @@ def initialize(value:, location:) @value = value @location = location end - + def accept(visitor) visitor.visit_rparen(self) end @@ -7258,7 +7259,7 @@ def child_nodes end alias deconstruct child_nodes - + def deconstruct_keys(keys) { value: value, location: location } end @@ -7513,7 +7514,7 @@ def child_nodes end alias deconstruct child_nodes - + def deconstruct_keys(keys) { parts: parts, location: location } end @@ -7810,7 +7811,7 @@ def child_nodes end alias deconstruct child_nodes - + def deconstruct_keys(keys) { value: value, location: location } end @@ -7840,7 +7841,7 @@ def child_nodes end alias deconstruct child_nodes - + def deconstruct_keys(keys) { value: value, location: location } end @@ -7971,7 +7972,7 @@ def child_nodes end alias deconstruct child_nodes - + def deconstruct_keys(keys) { value: value, location: location } end @@ -8000,7 +8001,7 @@ def child_nodes end alias deconstruct child_nodes - + def deconstruct_keys(keys) { value: value, location: location } end @@ -8030,7 +8031,7 @@ def child_nodes end alias deconstruct child_nodes - + def deconstruct_keys(keys) { value: value, location: location } end @@ -8141,7 +8142,7 @@ def child_nodes end alias deconstruct child_nodes - + def deconstruct_keys(keys) { value: value, location: location } end @@ -8219,7 +8220,7 @@ def child_nodes end alias deconstruct child_nodes - + def deconstruct_keys(keys) { value: value, location: location } end @@ -9244,7 +9245,7 @@ def child_nodes end alias deconstruct child_nodes - + def deconstruct_keys(keys) { value: value, location: location } end @@ -9273,7 +9274,7 @@ def child_nodes end alias deconstruct child_nodes - + def deconstruct_keys(keys) { parts: parts, location: location } end From aeee4d059ff8d443c9fb03c6d282c7ecc499f080 Mon Sep 17 00:00:00 2001 From: Drew Bragg Date: Thu, 21 Apr 2022 21:37:19 -0400 Subject: [PATCH 10/25] Add more examples to return fixture --- test/fixtures/return.rb | 4 ++++ 1 file changed, 4 insertions(+) diff --git a/test/fixtures/return.rb b/test/fixtures/return.rb index b57e5bad..8f7d0aa3 100644 --- a/test/fixtures/return.rb +++ b/test/fixtures/return.rb @@ -33,3 +33,7 @@ return [1, 2, 3] - return 1, 2, 3 +% +return [] +% +return [1] From a12a74a9020117048e3336320b2afeea404d727f Mon Sep 17 00:00:00 2001 From: Drew Bragg Date: Thu, 21 Apr 2022 21:37:46 -0400 Subject: [PATCH 11/25] Handle empty and single element Arrays --- lib/syntax_tree/node.rb | 6 +++++- 1 file changed, 5 insertions(+), 1 deletion(-) diff --git a/lib/syntax_tree/node.rb b/lib/syntax_tree/node.rb index 6b43d28e..af7ac42e 100644 --- a/lib/syntax_tree/node.rb +++ b/lib/syntax_tree/node.rb @@ -2076,7 +2076,11 @@ def format(q) end elsif part.is_a?(ArrayLiteral) q.text(" ") - q.format(part.contents) + if part.contents && part.contents.parts.length > 1 + q.format(part.contents) + else + q.format(arguments) + end else format_arguments(q, "(", ")") end From 0e7a01fcb09e217ba0122a860eb28de564e7d7fc Mon Sep 17 00:00:00 2001 From: Kevin Newton Date: Thu, 21 Apr 2022 10:52:17 -0400 Subject: [PATCH 12/25] Transform if/unless into ternary --- lib/syntax_tree/node.rb | 96 ++++++++++++++++++++++++++++++++++++++--- 1 file changed, 90 insertions(+), 6 deletions(-) diff --git a/lib/syntax_tree/node.rb b/lib/syntax_tree/node.rb index af7ac42e..7c06dcbf 100644 --- a/lib/syntax_tree/node.rb +++ b/lib/syntax_tree/node.rb @@ -4678,6 +4678,13 @@ def initialize(keyword, node) end def format(q) + # If we can transform this node into a ternary, then we're going to print + # a special version that uses the ternary operator if it fits on one line. + if can_ternary? + format_ternary(q) + return + end + # If the predicate of the conditional contains an assignment (in which # case we can't know for certain that that assignment doesn't impact the # statements inside the conditional) then we can't use the modifier form @@ -4687,12 +4694,7 @@ def format(q) return end - if contains_conditional? - format_break(q, force: true) - return - end - - if node.consequent || node.statements.empty? + if node.consequent || node.statements.empty? || contains_conditional? q.group { format_break(q, force: true) } else q.group do @@ -4729,10 +4731,92 @@ def format_break(q, force:) q.text("end") end + def format_ternary(q) + q.group do + q.if_break do + q.text("#{keyword} ") + q.nest(keyword.length + 1) { q.format(node.predicate) } + + q.indent do + q.breakable + q.format(node.statements) + end + + q.breakable + q.group do + q.format(node.consequent.keyword) + q.indent do + q.breakable + q.format(node.consequent.statements) + end + end + + q.breakable + q.text("end") + end.if_flat do + Parentheses.flat(q) do + q.format(node.predicate) + q.text(" ? ") + + statements = [node.statements, node.consequent.statements] + statements.reverse! if keyword == "unless" + + q.format(statements[0]) + q.text(" : ") + q.format(statements[1]) + end + end + end + end + def contains_conditional? node.statements.body.length == 1 && [If, IfMod, IfOp, Unless, UnlessMod].include?(node.statements.body.first.class) end + + # In order for an `if` or `unless` expression to be shortened to a ternary, + # there has to be one and only one consequent clause which is an Else. Both + # the body of the main node and the body of the Else node must have only one + # statement, and that statement must pass the `can_ternary_statements?` + # check. + def can_ternary? + case node + in { predicate: Assign | Command | CommandCall | MAssign | OpAssign } + false + in { consequent: Else[statements:] } + can_ternary_statements?(statements) && + can_ternary_statements?(node.statements) + else + false + end + end + + # Certain expressions cannot be reduced to a ternary without adding + # parentheses around them. In this case we say they cannot be ternaried and + # default instead to breaking them into multiple lines. + def can_ternary_statements?(statements) + return false if statements.body.length != 1 + statement = statements.body.first + + # This is a list of nodes that should not be allowed to be a part of a + # ternary clause. + no_ternary = [ + Alias, Assign, Break, Command, CommandCall, Heredoc, If, IfMod, IfOp, + Lambda, MAssign, Next, OpAssign, RescueMod, Return, Return0, Super, + Undef, Unless, UnlessMod, Until, UntilMod, VarAlias, VoidStmt, While, + WhileMod, Yield, Yield0, ZSuper + ] + + # Here we're going to check that the only statement inside the statements + # node is no a part of our denied list of nodes that can be ternaries. + # + # If the user is using one of the lower precedence "and" or "or" + # operators, then we can't use a ternary expression as it would break the + # flow control. + # + !no_ternary.include?(statement.class) && + !(statement.is_a?(Binary) && %i[and or].include?(statement.operator)) + end end # If represents the first clause in an +if+ chain. From a576f84c630e5d9c5ac66e0c77b115378e3a4f5c Mon Sep 17 00:00:00 2001 From: Kevin Newton Date: Thu, 21 Apr 2022 11:27:19 -0400 Subject: [PATCH 13/25] Don't format FCall parentheses unless you need them, add parentheses to not operator in ternaries if you need them --- lib/syntax_tree/node.rb | 133 ++++++++++++++++++++++++------------- test/fixtures/arg_paren.rb | 2 + test/fixtures/not.rb | 14 ++++ 3 files changed, 102 insertions(+), 47 deletions(-) diff --git a/lib/syntax_tree/node.rb b/lib/syntax_tree/node.rb index 7c06dcbf..5ecaff4c 100644 --- a/lib/syntax_tree/node.rb +++ b/lib/syntax_tree/node.rb @@ -4025,7 +4025,15 @@ def deconstruct_keys(keys) def format(q) q.format(value) - q.format(arguments) + + if arguments.is_a?(ArgParen) && arguments.arguments.nil? && !value.is_a?(Const) + # If you're using an explicit set of parentheses on something that looks + # like a constant, then we need to match that in order to maintain valid + # Ruby. For example, you could do something like Foo(), on which we + # would need to keep the parentheses to make it look like a method call. + else + q.format(arguments) + end end end @@ -4664,6 +4672,52 @@ def self.call(parent) end end + # In order for an `if` or `unless` expression to be shortened to a ternary, + # there has to be one and only one consequent clause which is an Else. Both + # the body of the main node and the body of the Else node must have only one + # statement, and that statement must not be on the denied list of potential + # statements. + module Ternaryable + class << self + def call(node) + case node + in { predicate: Assign | Command | CommandCall | MAssign | OpAssign } + false + in { statements: { body: [truthy] }, consequent: Else[statements: { body: [falsy] }] } + ternaryable?(truthy) && ternaryable?(falsy) + else + false + end + end + + private + + # Certain expressions cannot be reduced to a ternary without adding + # parentheses around them. In this case we say they cannot be ternaried and + # default instead to breaking them into multiple lines. + def ternaryable?(statement) + # This is a list of nodes that should not be allowed to be a part of a + # ternary clause. + no_ternary = [ + Alias, Assign, Break, Command, CommandCall, Heredoc, If, IfMod, IfOp, + Lambda, MAssign, Next, OpAssign, RescueMod, Return, Return0, Super, + Undef, Unless, UnlessMod, Until, UntilMod, VarAlias, VoidStmt, While, + WhileMod, Yield, Yield0, ZSuper + ] + + # Here we're going to check that the only statement inside the + # statements node is no a part of our denied list of nodes that can be + # ternaries. + # + # If the user is using one of the lower precedence "and" or "or" + # operators, then we can't use a ternary expression as it would break + # the flow control. + !no_ternary.include?(statement.class) && + !(statement.is_a?(Binary) && %i[and or].include?(statement.operator)) + end + end + end + # Formats an If or Unless node. class ConditionalFormatter # [String] the keyword associated with this conditional @@ -4680,7 +4734,7 @@ def initialize(keyword, node) def format(q) # If we can transform this node into a ternary, then we're going to print # a special version that uses the ternary operator if it fits on one line. - if can_ternary? + if Ternaryable.call(node) format_ternary(q) return end @@ -4746,7 +4800,13 @@ def format_ternary(q) q.group do q.format(node.consequent.keyword) q.indent do - q.breakable + # This is a very special case of breakable where we want to force + # it into the output but we _don't_ want to explicitly break the + # parent. If a break-parent shows up in the tree, then it's going + # to force it all the way up to the tree, which is going to negate + # the ternary. Maybe this should be an option in prettyprint? As + # in force: :no_break_parent or something. + q.target << PrettyPrint::Breakable.new(" ", 1, force: true) q.format(node.consequent.statements) end end @@ -4770,53 +4830,13 @@ def format_ternary(q) end def contains_conditional? - node.statements.body.length == 1 && - [If, IfMod, IfOp, Unless, UnlessMod].include?(node.statements.body.first.class) - end - - # In order for an `if` or `unless` expression to be shortened to a ternary, - # there has to be one and only one consequent clause which is an Else. Both - # the body of the main node and the body of the Else node must have only one - # statement, and that statement must pass the `can_ternary_statements?` - # check. - def can_ternary? case node - in { predicate: Assign | Command | CommandCall | MAssign | OpAssign } - false - in { consequent: Else[statements:] } - can_ternary_statements?(statements) && - can_ternary_statements?(node.statements) + in { statements: { body: [If | IfMod | IfOp | Unless | UnlessMod] } } + true else false end end - - # Certain expressions cannot be reduced to a ternary without adding - # parentheses around them. In this case we say they cannot be ternaried and - # default instead to breaking them into multiple lines. - def can_ternary_statements?(statements) - return false if statements.body.length != 1 - statement = statements.body.first - - # This is a list of nodes that should not be allowed to be a part of a - # ternary clause. - no_ternary = [ - Alias, Assign, Break, Command, CommandCall, Heredoc, If, IfMod, IfOp, - Lambda, MAssign, Next, OpAssign, RescueMod, Return, Return0, Super, - Undef, Unless, UnlessMod, Until, UntilMod, VarAlias, VoidStmt, While, - WhileMod, Yield, Yield0, ZSuper - ] - - # Here we're going to check that the only statement inside the statements - # node is no a part of our denied list of nodes that can be ternaries. - # - # If the user is using one of the lower precedence "and" or "or" - # operators, then we can't use a ternary expression as it would break the - # flow control. - # - !no_ternary.include?(statement.class) && - !(statement.is_a?(Binary) && %i[and or].include?(statement.operator)) - end end # If represents the first clause in an +if+ chain. @@ -8355,9 +8375,28 @@ def deconstruct_keys(keys) end def format(q) - q.text(parentheses ? "not(" : "not ") + parent = q.parents.take(2)[1] + ternary = (parent.is_a?(If) || parent.is_a?(Unless)) && Ternaryable.call(parent) + + q.text("not") + + if parentheses + q.text("(") + elsif ternary + q.if_break { q.text(" ") }.if_flat { q.text("(") } + else + q.text(" ") + end + q.format(statement) if statement - q.text(")") if parentheses + + if parentheses + q.text(")") + elsif ternary + q.if_break {}.if_flat { q.text(")") } + else + q.text(" ") + end end end diff --git a/test/fixtures/arg_paren.rb b/test/fixtures/arg_paren.rb index 0816af6a..0e01e208 100644 --- a/test/fixtures/arg_paren.rb +++ b/test/fixtures/arg_paren.rb @@ -2,6 +2,8 @@ foo(bar) % foo() +- +foo % foo(barrrrrrrrrrrrrrrrrrrrrrrrrrrrrrrrrrrrrrrrrrrrrrrrrrrrrrrrrrrrrrrrrrrrrrrrrr) - diff --git a/test/fixtures/not.rb b/test/fixtures/not.rb index 0204e345..6c0451ae 100644 --- a/test/fixtures/not.rb +++ b/test/fixtures/not.rb @@ -8,3 +8,17 @@ not(foo) % not (foo) +% +if foo + not bar +else + baz +end +- +foo ? not(bar) : baz +% +if foooooooooooooooooooooooooooooooooooooooooo + not bar +else + bazzzzzzzzzzzzzzzzzzzzzzzzzzzzzzzzzzzzzzzzzzz +end From 7fd2af359590bfbab5cb96ac6e8932da19c5aa97 Mon Sep 17 00:00:00 2001 From: Kevin Newton Date: Thu, 21 Apr 2022 11:40:12 -0400 Subject: [PATCH 14/25] Format comments on params even if they are empty --- lib/syntax_tree/node.rb | 18 +++++++++++------- 1 file changed, 11 insertions(+), 7 deletions(-) diff --git a/lib/syntax_tree/node.rb b/lib/syntax_tree/node.rb index 5ecaff4c..53f5b2b6 100644 --- a/lib/syntax_tree/node.rb +++ b/lib/syntax_tree/node.rb @@ -3060,7 +3060,10 @@ def format(q) q.group do q.text("def ") q.format(name) - q.format(params) if !params.is_a?(Params) || !params.empty? + + if !params.is_a?(Params) || !params.empty? || params.comments.any? + q.format(params) + end end unless bodystmt.empty? @@ -3280,7 +3283,10 @@ def format(q) q.format(target) q.format(CallOperatorFormatter.new(operator), stackable: false) q.format(name) - q.format(params) if !params.is_a?(Params) || !params.empty? + + if !params.is_a?(Params) || !params.empty? || params.comments.any? + q.format(params) + end end unless bodystmt.empty? @@ -6317,7 +6323,9 @@ def format(q) q.format(rest) if rest && rest.is_a?(ExcessedComma) end - if [Def, Defs, DefEndless].include?(q.parent.class) + if ![Def, Defs, DefEndless].include?(q.parent.class) || parts.empty? + q.nest(0, &contents) + else q.group(0, "(", ")") do q.indent do q.breakable("") @@ -6325,8 +6333,6 @@ def format(q) end q.breakable("") end - else - q.nest(0, &contents) end end end @@ -8394,8 +8400,6 @@ def format(q) q.text(")") elsif ternary q.if_break {}.if_flat { q.text(")") } - else - q.text(" ") end end end From 1f623a5bccb7728b575d7e32352aa55fa6e8620d Mon Sep 17 00:00:00 2001 From: Kevin Newton Date: Thu, 21 Apr 2022 11:51:23 -0400 Subject: [PATCH 15/25] Include %s symbols in no indent --- lib/syntax_tree/node.rb | 21 ++++++++++----------- 1 file changed, 10 insertions(+), 11 deletions(-) diff --git a/lib/syntax_tree/node.rb b/lib/syntax_tree/node.rb index 53f5b2b6..2ca7e8f4 100644 --- a/lib/syntax_tree/node.rb +++ b/lib/syntax_tree/node.rb @@ -1042,17 +1042,16 @@ def format(q) # Determins if the following value should be indented or not. module AssignFormatting def self.skip_indent?(value) - (value.is_a?(Call) && skip_indent?(value.receiver)) || - [ - ArrayLiteral, - HashLiteral, - Heredoc, - Lambda, - QSymbols, - QWords, - Symbols, - Words - ].include?(value.class) + case value + in ArrayLiteral | HashLiteral | Heredoc | Lambda | QSymbols | QWords | Symbols | Words + true + in Call[receiver:] + skip_indent?(receiver) + in DynaSymbol[quote:] + quote.start_with?("%s") + else + false + end end end From 6eb63e8a6378feecf34f67d421c20e1a48e98a8a Mon Sep 17 00:00:00 2001 From: Kevin Newton Date: Thu, 21 Apr 2022 13:35:38 -0400 Subject: [PATCH 16/25] Better formatting for empty hashes and arrays with interior comments --- lib/syntax_tree/formatter.rb | 2 +- lib/syntax_tree/node.rb | 89 ++++++++++++++++++++++++++++++++---- 2 files changed, 80 insertions(+), 11 deletions(-) diff --git a/lib/syntax_tree/formatter.rb b/lib/syntax_tree/formatter.rb index 95c2adaf..7015a837 100644 --- a/lib/syntax_tree/formatter.rb +++ b/lib/syntax_tree/formatter.rb @@ -50,7 +50,7 @@ def format(node, stackable: true) # Print all comments that were found after the node. trailing.each do |comment| line_suffix(priority: COMMENT_PRIORITY) do - text(" ") + comment.inline? ? text(" ") : breakable comment.format(self) break_parent end diff --git a/lib/syntax_tree/node.rb b/lib/syntax_tree/node.rb index 2ca7e8f4..5c3d4227 100644 --- a/lib/syntax_tree/node.rb +++ b/lib/syntax_tree/node.rb @@ -816,6 +816,29 @@ def format(q) end end + class EmptyWithCommentsFormatter + # [LBracket] the opening bracket + attr_reader :lbracket + + def initialize(lbracket) + @lbracket = lbracket + end + + def format(q) + q.group do + q.text("[") + q.indent do + lbracket.comments.each do |comment| + q.breakable(force: true) + comment.format(q) + end + end + q.breakable(force: true) + q.text("]") + end + end + end + # [LBracket] the bracket that opens this array attr_reader :lbracket @@ -867,6 +890,11 @@ def format(q) return end + if empty_with_comments? + EmptyWithCommentsFormatter.new(lbracket).format(q) + return + end + q.group do q.format(lbracket) @@ -919,6 +947,12 @@ def var_refs?(q) q.maxwidth * 2 ) end + + # If we have an empty array that contains only comments, then we're going + # to do some special printing to ensure they get indented correctly. + def empty_with_comments? + contents.nil? && lbracket.comments.any? && lbracket.comments.none?(&:inline?) + end end # AryPtn represents matching against an array pattern using the Ruby 2.7+ @@ -4311,6 +4345,29 @@ def format(q) # { key => value } # class HashLiteral < Node + class EmptyWithCommentsFormatter + # [LBrace] the opening brace + attr_reader :lbrace + + def initialize(lbrace) + @lbrace = lbrace + end + + def format(q) + q.group do + q.text("{") + q.indent do + lbrace.comments.each do |comment| + q.breakable(force: true) + comment.format(q) + end + end + q.breakable(force: true) + q.text("}") + end + end + end + # [LBrace] the left brace that opens this hash attr_reader :lbrace @@ -4355,7 +4412,18 @@ def format_key(q, key) private + # If we have an empty hash that contains only comments, then we're going + # to do some special printing to ensure they get indented correctly. + def empty_with_comments? + assocs.empty? && lbrace.comments.any? && lbrace.comments.none?(&:inline?) + end + def format_contents(q) + if empty_with_comments? + EmptyWithCommentsFormatter.new(lbrace).format(q) + return + end + q.format(lbrace) if assocs.empty? @@ -7584,19 +7652,20 @@ def attach_comments(start_char, end_char) comment = parser_comments[comment_index] location = comment.location - if !comment.inline? && (start_char <= location.start_char) && - (end_char >= location.end_char) && !comment.ignore? - parser_comments.delete_at(comment_index) - - while (node = body[body_index]) && - ( - node.is_a?(VoidStmt) || - node.location.start_char < location.start_char - ) + if !comment.inline? && (start_char <= location.start_char) && (end_char >= location.end_char) && !comment.ignore? + while (node = body[body_index]) && (node.is_a?(VoidStmt) || node.location.start_char < location.start_char) body_index += 1 end - body.insert(body_index, comment) + if body_index != 0 && body[body_index - 1].location.start_char < location.start_char && body[body_index - 1].location.end_char > location.start_char + # The previous node entirely encapsules the comment, so we don't + # want to attach it here since it will get attached normally. This + # is mostly in the case of hash and array literals. + comment_index += 1 + else + parser_comments.delete_at(comment_index) + body.insert(body_index, comment) + end else comment_index += 1 end From b38f7e17058b4edd571a53de64a7d4bf7c7853dd Mon Sep 17 00:00:00 2001 From: Kevin Newton Date: Thu, 21 Apr 2022 13:36:08 -0400 Subject: [PATCH 17/25] Only test long-running tests if CI env var --- test/idempotency_test.rb | 2 +- test/visitor_test.rb | 2 +- 2 files changed, 2 insertions(+), 2 deletions(-) diff --git a/test/idempotency_test.rb b/test/idempotency_test.rb index fde141f5..bb12bdbb 100644 --- a/test/idempotency_test.rb +++ b/test/idempotency_test.rb @@ -1,6 +1,6 @@ # frozen_string_literal: true -return if ENV["FAST"] +return unless ENV["CI"] require_relative "test_helper" module SyntaxTree diff --git a/test/visitor_test.rb b/test/visitor_test.rb index dd9f1e11..00cedda4 100644 --- a/test/visitor_test.rb +++ b/test/visitor_test.rb @@ -3,7 +3,7 @@ require_relative "test_helper" class VisitorTest < Minitest::Test - unless ENV["FAST"] + if ENV["CI"] def test_visit_all_nodes visitor = SyntaxTree::Visitor.new From e739d62689e2cff52fc3526d65dde4b9a33b94d2 Mon Sep 17 00:00:00 2001 From: Kevin Newton Date: Thu, 21 Apr 2022 14:30:11 -0400 Subject: [PATCH 18/25] Alignment on command and command call for ternaries --- lib/syntax_tree/node.rb | 30 +++++++++++++++++------------- lib/syntax_tree/prettyprint.rb | 13 +++++++++++++ 2 files changed, 30 insertions(+), 13 deletions(-) diff --git a/lib/syntax_tree/node.rb b/lib/syntax_tree/node.rb index 5c3d4227..d7bc250f 100644 --- a/lib/syntax_tree/node.rb +++ b/lib/syntax_tree/node.rb @@ -2614,26 +2614,24 @@ def deconstruct_keys(keys) def format(q) q.group do q.format(message) - q.text(" ") - - if align?(self) - q.nest(message.value.length + 1) { q.format(arguments) } - else - q.format(arguments) - end + align(q, self) { q.format(arguments) } end end private - def align?(node) + def align(q, node, &block) case node.arguments in Args[parts: [Def | Defs | DefEndless]] - false + q.text(" ") + yield + in Args[parts: [IfOp]] + yield in Args[parts: [Command => command]] - align?(command) + align(q, command, &block) else - true + q.text(" ") + q.nest(message.value.length + 1) { yield } end end end @@ -2705,9 +2703,15 @@ def format(q) q.format(message) end - if arguments + case arguments + in Args[parts: [IfOp]] + q.if_flat { q.text(" ") } + q.format(arguments) + in Args q.text(" ") q.nest(argument_alignment(q, doc)) { q.format(arguments) } + else + # If there are no arguments, print nothing. end end end @@ -8467,7 +8471,7 @@ def format(q) if parentheses q.text(")") elsif ternary - q.if_break {}.if_flat { q.text(")") } + q.if_flat { q.text(")") } end end end diff --git a/lib/syntax_tree/prettyprint.rb b/lib/syntax_tree/prettyprint.rb index 8ca5d555..4bd6fdf3 100644 --- a/lib/syntax_tree/prettyprint.rb +++ b/lib/syntax_tree/prettyprint.rb @@ -458,6 +458,10 @@ def if_break IfBreakBuilder.new end + # Also effectively unnecessary, but here for compatibility. + def if_flat + end + # A noop that immediately yields. def indent yield @@ -1011,6 +1015,15 @@ def if_break IfBreakBuilder.new(self, doc) end + # This is similar to if_break in that it also inserts an IfBreak node into the + # print tree, however it's starting from the flat contents, and cannot be used + # to build the break contents. + def if_flat + doc = IfBreak.new + + with_target(doc.flat_contents) { yield } + end + # Very similar to the #nest method, this indents the nested content by one # level by inserting an Indent node into the print tree. The contents of the # node are determined by the block. From b7c02329b10e95073d8a559f74507cddcf202b1f Mon Sep 17 00:00:00 2001 From: Kevin Newton Date: Thu, 21 Apr 2022 14:41:12 -0400 Subject: [PATCH 19/25] Format correctly for chained call nodes with arguments --- lib/syntax_tree/node.rb | 10 +++++++++- lib/syntax_tree/prettyprint.rb | 1 + 2 files changed, 10 insertions(+), 1 deletion(-) diff --git a/lib/syntax_tree/node.rb b/lib/syntax_tree/node.rb index d7bc250f..98e5f914 100644 --- a/lib/syntax_tree/node.rb +++ b/lib/syntax_tree/node.rb @@ -2297,7 +2297,15 @@ def format(q) q.format(message) if message != :call end - q.format(arguments) if arguments + case arguments + in ArgParen + q.format(arguments) + in Args + q.text(" ") + q.format(arguments) + else + # Do nothing if there are no arguments. + end end end end diff --git a/lib/syntax_tree/prettyprint.rb b/lib/syntax_tree/prettyprint.rb index 4bd6fdf3..a93a6f2d 100644 --- a/lib/syntax_tree/prettyprint.rb +++ b/lib/syntax_tree/prettyprint.rb @@ -1020,6 +1020,7 @@ def if_break # to build the break contents. def if_flat doc = IfBreak.new + target << doc with_target(doc.flat_contents) { yield } end From ffb6531ad6d1c3ce24728cc4edc9296fec77d322 Mon Sep 17 00:00:00 2001 From: Kevin Newton Date: Thu, 21 Apr 2022 15:52:19 -0400 Subject: [PATCH 20/25] Even better chained calls --- lib/syntax_tree/node.rb | 99 +++++++++++++++++++++++++++++++++++++---- 1 file changed, 90 insertions(+), 9 deletions(-) diff --git a/lib/syntax_tree/node.rb b/lib/syntax_tree/node.rb index 98e5f914..b2102129 100644 --- a/lib/syntax_tree/node.rb +++ b/lib/syntax_tree/node.rb @@ -2212,6 +2212,68 @@ def format(q) end end + class CallChainFormatter + # [Call] the top of the call chain + attr_reader :node + + def initialize(node) + @node = node + end + + def format(q) + children = [node] + children << children.last.receiver while children.last.receiver.is_a?(Call) + q.format(children.last.receiver) + + q.group do + format_child(q, children.pop) if attach_directly?(children.last) + + q.indent do + children.reverse_each do |child| + case child + in { receiver: Call[message: { value: "where" }], message: { value: "not" } } + # This is very specialized behavior wherein we group + # .where.not calls together because it looks better. For more + # information, see + # https://github.com/prettier/plugin-ruby/issues/862. + else + q.breakable("") + end + + format_child(q, child) + end + end + end + end + + private + + # For certain nodes, we want to attach directly to the end and don't + # want to indent the first call. So we'll pop off the first children and + # format it separately here. + def attach_directly?(child) + [ArrayLiteral, HashLiteral, Heredoc, If, Unless, XStringLiteral] + .include?(child.receiver.class) + end + + def format_child(q, child) + q.format(CallOperatorFormatter.new(child.operator)) + q.format(child.message) if child.message != :call + child.format_arguments(q) + + # If there are any comments on this node then we need to explicitly print + # them out here since we're bypassing the normal comment printing. + if child.comments.any? + child.comments.each do |comment| + comment.inline? ? q.text(" ") : q.breakable + comment.format(q) + end + + q.break_parent + end + end + end + # Call represents a method call. # # receiver.message @@ -2277,6 +2339,33 @@ def deconstruct_keys(keys) def format(q) call_operator = CallOperatorFormatter.new(operator) + # If we're at the top of a call chain, then we're going to do some + # specialized printing in case we can print it nicely. We _only_ do this + # at the top of the chain to avoid weird recursion issues. + if !q.parent.is_a?(Call) && receiver.is_a?(Call) + q.group { q.if_break { CallChainFormatter.new(self).format(q) }.if_flat { format_contents(q) } } + else + format_contents(q) + end + end + + def format_arguments(q) + case arguments + in ArgParen + q.format(arguments) + in Args + q.text(" ") + q.format(arguments) + else + # Do nothing if there are no arguments. + end + end + + private + + def format_contents(q) + call_operator = CallOperatorFormatter.new(operator) + q.group do q.format(receiver) @@ -2297,15 +2386,7 @@ def format(q) q.format(message) if message != :call end - case arguments - in ArgParen - q.format(arguments) - in Args - q.text(" ") - q.format(arguments) - else - # Do nothing if there are no arguments. - end + format_arguments(q) end end end From b75aa497200136cb882e2fd81bff0f6e9077febc Mon Sep 17 00:00:00 2001 From: Kevin Newton Date: Thu, 21 Apr 2022 16:36:28 -0400 Subject: [PATCH 21/25] Better call chain formatting with method add blocks factored in --- lib/syntax_tree/node.rb | 89 +++++++++++++++++++++++++++++++++++------ 1 file changed, 76 insertions(+), 13 deletions(-) diff --git a/lib/syntax_tree/node.rb b/lib/syntax_tree/node.rb index b2102129..3f985cef 100644 --- a/lib/syntax_tree/node.rb +++ b/lib/syntax_tree/node.rb @@ -2221,29 +2221,88 @@ def initialize(node) end def format(q) + q.group { q.if_break { format_chain(q) }.if_flat { node.format_contents(q) } } + end + + def format_chain(q) children = [node] - children << children.last.receiver while children.last.receiver.is_a?(Call) + + # First, walk down the chain until we get to the point where we're not + # longer at a chainable node. + while true + case children.last + in Call[receiver: Call] + children << children.last.receiver + in Call[receiver: MethodAddBlock[call: Call]] + children << children.last.receiver + in MethodAddBlock[call: Call] + children << children.last.call + else + break + end + end + + # We're going to have some specialized behavior for if it's an entire + # chain of calls without arguments except for the last one. This is common + # enough in Ruby source code that it's worth the extra complexity here. + empty_except_last = + children.drop(1).all? do |child| + child.is_a?(Call) && child.arguments.nil? + end + + # Here, we're going to add all of the children onto the stack of the + # formatter so it's as if we had descending normally into them. This is + # necessary so they can check their parents as normal. + q.stack.concat(children) q.format(children.last.receiver) q.group do - format_child(q, children.pop) if attach_directly?(children.last) + if attach_directly?(children.last) + format_child(q, children.pop) + q.stack.pop + end q.indent do - children.reverse_each do |child| + while child = children.pop case child - in { receiver: Call[message: { value: "where" }], message: { value: "not" } } + in Call[receiver: Call[message: { value: "where" }], message: { value: "not" }] # This is very specialized behavior wherein we group # .where.not calls together because it looks better. For more # information, see # https://github.com/prettier/plugin-ruby/issues/862. - else + in Call + # If we're at a Call node and not a MethodAddBlock node in the + # chain then we're going to add a newline so it indents properly. q.breakable("") + else end - format_child(q, child) + format_child(q, child, skip_attached: empty_except_last && children.empty?) + + # Pop off the formatter's stack so that it aligns with what would + # have happened if we had been formatting normally. + q.stack.pop end end end + + if empty_except_last + case node + in Call + node.format_arguments(q) + in MethodAddBlock[block:] + q.format(block) + end + end + end + + def self.chained?(node) + case node + in Call | MethodAddBlock[call: Call] + true + else + false + end end private @@ -2256,10 +2315,16 @@ def attach_directly?(child) .include?(child.receiver.class) end - def format_child(q, child) - q.format(CallOperatorFormatter.new(child.operator)) - q.format(child.message) if child.message != :call - child.format_arguments(q) + def format_child(q, child, skip_attached: false) + # First, format the actual contents of the child. + case child + in Call + q.format(CallOperatorFormatter.new(child.operator)) + q.format(child.message) if child.message != :call + child.format_arguments(q) unless skip_attached + in MethodAddBlock + q.format(child.block) unless skip_attached + end # If there are any comments on this node then we need to explicitly print # them out here since we're bypassing the normal comment printing. @@ -2342,7 +2407,7 @@ def format(q) # If we're at the top of a call chain, then we're going to do some # specialized printing in case we can print it nicely. We _only_ do this # at the top of the chain to avoid weird recursion issues. - if !q.parent.is_a?(Call) && receiver.is_a?(Call) + if !CallChainFormatter.chained?(q.parent) && CallChainFormatter.chained?(receiver) q.group { q.if_break { CallChainFormatter.new(self).format(q) }.if_flat { format_contents(q) } } else format_contents(q) @@ -2361,8 +2426,6 @@ def format_arguments(q) end end - private - def format_contents(q) call_operator = CallOperatorFormatter.new(operator) From 368889f70ba49218f5976146a6aa117a2c2dd466 Mon Sep 17 00:00:00 2001 From: Kevin Newton Date: Thu, 21 Apr 2022 16:49:02 -0400 Subject: [PATCH 22/25] Handle trailing operators if necessary --- lib/syntax_tree/node.rb | 46 ++++++++++++++++++++++++++++++++++------- 1 file changed, 38 insertions(+), 8 deletions(-) diff --git a/lib/syntax_tree/node.rb b/lib/syntax_tree/node.rb index 3f985cef..d7f62766 100644 --- a/lib/syntax_tree/node.rb +++ b/lib/syntax_tree/node.rb @@ -2213,7 +2213,7 @@ def format(q) end class CallChainFormatter - # [Call] the top of the call chain + # [Call | MethodAddBlock] the top of the call chain attr_reader :node def initialize(node) @@ -2221,10 +2221,6 @@ def initialize(node) end def format(q) - q.group { q.if_break { format_chain(q) }.if_flat { node.format_contents(q) } } - end - - def format_chain(q) children = [node] # First, walk down the chain until we get to the point where we're not @@ -2242,6 +2238,14 @@ def format_chain(q) end end + if children.length > 2 + q.group { q.if_break { format_chain(q, children) }.if_flat { node.format_contents(q) } } + else + node.format_contents(q) + end + end + + def format_chain(q, children) # We're going to have some specialized behavior for if it's an entire # chain of calls without arguments except for the last one. This is common # enough in Ruby source code that it's worth the extra complexity here. @@ -2263,6 +2267,11 @@ def format_chain(q) end q.indent do + # We track another variable that checks if you need to move the + # operator to the previous line in case there are trailing comments + # and a trailing operator. + skip_operator = false + while child = children.pop case child in Call[receiver: Call[message: { value: "where" }], message: { value: "not" }] @@ -2277,7 +2286,17 @@ def format_chain(q) else end - format_child(q, child, skip_attached: empty_except_last && children.empty?) + format_child(q, child, skip_operator: skip_operator, skip_attached: empty_except_last && children.empty?) + + # If the parent call node has a comment on the message then we need + # to print the operator trailing in order to keep it working. + case children.last + in Call[message: { comments: [_, *] }, operator:] + q.format(CallOperatorFormatter.new(operator)) + skip_operator = true + else + skip_operator = false + end # Pop off the formatter's stack so that it aligns with what would # have happened if we had been formatting normally. @@ -2315,11 +2334,11 @@ def attach_directly?(child) .include?(child.receiver.class) end - def format_child(q, child, skip_attached: false) + def format_child(q, child, skip_operator: false, skip_attached: false) # First, format the actual contents of the child. case child in Call - q.format(CallOperatorFormatter.new(child.operator)) + q.format(CallOperatorFormatter.new(child.operator)) unless skip_operator q.format(child.message) if child.message != :call child.format_arguments(q) unless skip_attached in MethodAddBlock @@ -5925,6 +5944,17 @@ def deconstruct_keys(keys) end def format(q) + # If we're at the top of a call chain, then we're going to do some + # specialized printing in case we can print it nicely. We _only_ do this + # at the top of the chain to avoid weird recursion issues. + if !CallChainFormatter.chained?(q.parent) && CallChainFormatter.chained?(call) + q.group { q.if_break { CallChainFormatter.new(self).format(q) }.if_flat { format_contents(q) } } + else + format_contents(q) + end + end + + def format_contents(q) q.format(call) q.format(block) end From b847e88343a17a79ed90a8f166fef70374a68748 Mon Sep 17 00:00:00 2001 From: Kevin Newton Date: Thu, 21 Apr 2022 22:00:18 -0400 Subject: [PATCH 23/25] Specialized formatting for sorbet --- lib/syntax_tree/node.rb | 35 +++++++++++++++++++++++++++++------ 1 file changed, 29 insertions(+), 6 deletions(-) diff --git a/lib/syntax_tree/node.rb b/lib/syntax_tree/node.rb index d7f62766..e32bdeb1 100644 --- a/lib/syntax_tree/node.rb +++ b/lib/syntax_tree/node.rb @@ -2222,6 +2222,7 @@ def initialize(node) def format(q) children = [node] + threshold = 3 # First, walk down the chain until we get to the point where we're not # longer at a chainable node. @@ -2238,7 +2239,27 @@ def format(q) end end - if children.length > 2 + # Here, we have very specialized behavior where if we're within a sig + # block, then we're going to assume we're creating a Sorbet type + # signature. In that case, we really want the threshold to be lowered so + # that we create method chains off of any two method calls within the + # block. For more details, see + # https://github.com/prettier/plugin-ruby/issues/863. + parents = q.parents.take(4) + if parent = parents[2] + # If we're at a do_block, then we want to go one more level up. This is + # because do blocks have BodyStmt nodes instead of just Statements + # nodes. + parent = parents[3] if parent.is_a?(DoBlock) + + case parent + in MethodAddBlock[call: FCall[value: { value: "sig" }]] + threshold = 2 + else + end + end + + if children.length >= threshold q.group { q.if_break { format_chain(q, children) }.if_flat { node.format_contents(q) } } else node.format_contents(q) @@ -2329,18 +2350,20 @@ def self.chained?(node) # For certain nodes, we want to attach directly to the end and don't # want to indent the first call. So we'll pop off the first children and # format it separately here. - def attach_directly?(child) + def attach_directly?(node) [ArrayLiteral, HashLiteral, Heredoc, If, Unless, XStringLiteral] - .include?(child.receiver.class) + .include?(node.receiver.class) end def format_child(q, child, skip_operator: false, skip_attached: false) # First, format the actual contents of the child. case child in Call - q.format(CallOperatorFormatter.new(child.operator)) unless skip_operator - q.format(child.message) if child.message != :call - child.format_arguments(q) unless skip_attached + q.group do + q.format(CallOperatorFormatter.new(child.operator)) unless skip_operator + q.format(child.message) if child.message != :call + child.format_arguments(q) unless skip_attached + end in MethodAddBlock q.format(child.block) unless skip_attached end From 752b49491f624484ec7fc89c89ca5f5ba83c2782 Mon Sep 17 00:00:00 2001 From: Kevin Newton Date: Fri, 22 Apr 2022 11:47:42 -0400 Subject: [PATCH 24/25] More random fixes and simplification --- lib/syntax_tree/node.rb | 185 ++++++++++++++++++++++++--------- lib/syntax_tree/prettyprint.rb | 24 ++--- test/fixtures/next.rb | 12 +++ 3 files changed, 160 insertions(+), 61 deletions(-) diff --git a/lib/syntax_tree/node.rb b/lib/syntax_tree/node.rb index e32bdeb1..7f477691 100644 --- a/lib/syntax_tree/node.rb +++ b/lib/syntax_tree/node.rb @@ -1585,12 +1585,12 @@ def format(q) q.text(" ") unless power if operator == :<< - q.text(operator) + q.text(operator.to_s) q.text(" ") q.format(right) else q.group do - q.text(operator) + q.text(operator.to_s) q.indent do q.breakable(power ? "" : " ") @@ -2075,12 +2075,12 @@ def format(q) end end - # Formats either a Break or Next node. + # Formats either a Break, Next, or Return node. class FlowControlFormatter # [String] the keyword to print attr_reader :keyword - # [Break | Next] the node being formatted + # [Break | Next | Return] the node being formatted attr_reader :node def initialize(keyword, node) @@ -2089,43 +2089,118 @@ def initialize(keyword, node) end def format(q) - arguments = node.arguments - q.group do q.text(keyword) - if arguments.parts.any? - if arguments.parts.length == 1 - part = arguments.parts.first - - if part.is_a?(Paren) - if part.contents.body.length == 1 && skip_parens?(part.contents.body.first) - q.text(" ") - contents = part.contents.body.first - contents = contents.contents if contents.is_a?(ArrayLiteral) - q.format(contents) - else - q.format(arguments) - end - elsif part.is_a?(ArrayLiteral) - q.text(" ") - if part.contents && part.contents.parts.length > 1 - q.format(part.contents) - else - q.format(arguments) - end - else - format_arguments(q, "(", ")") - end - else - format_arguments(q, " [", "]") - end + case node.arguments.parts + in [] + # Here there are no arguments at all, so we're not going to print + # anything. This would be like if we had: + # + # break + # + in [Paren[contents: { body: [ArrayLiteral[contents: { parts: [_, _, *] }] => array] }]] + # Here we have a single argument that is a set of parentheses wrapping + # an array literal that has at least 2 elements. We're going to print + # the contents of the array directly. This would be like if we had: + # + # break([1, 2, 3]) + # + # which we will print as: + # + # break 1, 2, 3 + # + q.text(" ") + format_array_contents(q, array) + in [Paren[contents: { body: [ArrayLiteral => statement] }]] + # Here we have a single argument that is a set of parentheses wrapping + # an array literal that has 0 or 1 elements. We're going to skip the + # parentheses but print the array itself. This would be like if we + # had: + # + # break([1]) + # + # which we will print as: + # + # break [1] + # + q.text(" ") + q.format(statement) + in [Paren[contents: { body: [statement] }]] if skip_parens?(statement) + # Here we have a single argument that is a set of parentheses that + # themselves contain a single statement. That statement is a simple + # value that we can skip the parentheses for. This would be like if we + # had: + # + # break(1) + # + # which we will print as: + # + # break 1 + # + q.text(" ") + q.format(statement) + in [Paren => part] + # Here we have a single argument that is a set of parentheses. We're + # going to print the parentheses themselves as if they were the set of + # arguments. This would be like if we had: + # + # break(foo.bar) + # + q.format(part) + in [ArrayLiteral[contents: { parts: [_, _, *] }] => array] + # Here there is a single argument that is an array literal with at + # least two elements. We skip directly into the array literal's + # elements in order to print the contents. This would be like if we + # had: + # + # break [1, 2, 3] + # + # which we will print as: + # + # break 1, 2, 3 + # + q.text(" ") + format_array_contents(q, array) + in [ArrayLiteral => part] + # Here there is a single argument that is an array literal with 0 or 1 + # elements. In this case we're going to print the array as it is + # because skipping the brackets would change the remaining. This would + # be like if we had: + # + # break [] + # break [1] + # + q.text(" ") + q.format(part) + in [_] + # Here there is a single argument that hasn't matched one of our + # previous cases. We're going to print the argument as it is. This + # would be like if we had: + # + # break foo + # + format_arguments(q, "(", ")") + else + # If there are multiple arguments, format them all. If the line is + # going to break into multiple, then use brackets to start and end the + # expression. + format_arguments(q, " [", "]") end end end private + def format_array_contents(q, array) + q.if_break { q.text("[") } + q.indent do + q.breakable("") + q.format(array.contents) + end + q.breakable("") + q.if_break { q.text("]") } + end def format_arguments(q, opening, closing) q.if_break { q.text(opening) } @@ -2139,9 +2214,9 @@ def format_arguments(q, opening, closing) def skip_parens?(node) case node - in Int | FloatLiteral | ArrayLiteral + in FloatLiteral | Imaginary | Int | RationalLiteral true - in VarRef[value: GVar | IVar | CVar | Kw | Const] + in VarRef[value: Const | CVar | GVar | IVar | Kw] true else false @@ -2307,7 +2382,13 @@ def format_chain(q, children) else end - format_child(q, child, skip_operator: skip_operator, skip_attached: empty_except_last && children.empty?) + format_child( + q, + child, + skip_comments: children.empty?, + skip_operator: skip_operator, + skip_attached: empty_except_last && children.empty? + ) # If the parent call node has a comment on the message then we need # to print the operator trailing in order to keep it working. @@ -2355,7 +2436,7 @@ def attach_directly?(node) .include?(node.receiver.class) end - def format_child(q, child, skip_operator: false, skip_attached: false) + def format_child(q, child, skip_comments: false, skip_operator: false, skip_attached: false) # First, format the actual contents of the child. case child in Call @@ -2370,7 +2451,7 @@ def format_child(q, child, skip_operator: false, skip_attached: false) # If there are any comments on this node then we need to explicitly print # them out here since we're bypassing the normal comment printing. - if child.comments.any? + if child.comments.any? && !skip_comments child.comments.each do |comment| comment.inline? ? q.text(" ") : q.breakable comment.format(q) @@ -2444,8 +2525,6 @@ def deconstruct_keys(keys) end def format(q) - call_operator = CallOperatorFormatter.new(operator) - # If we're at the top of a call chain, then we're going to do some # specialized printing in case we can print it nicely. We _only_ do this # at the top of the chain to avoid weird recursion issues. @@ -4950,14 +5029,24 @@ def self.call(parent) # statements. module Ternaryable class << self - def call(node) - case node - in { predicate: Assign | Command | CommandCall | MAssign | OpAssign } + def call(q, node) + case q.parents.take(2)[1] + in Paren[contents: Statements[body: [node]]] + # If this is a conditional inside of a parentheses as the only + # content, then we don't want to transform it into a ternary. + # Presumably the user wanted it to be an explicit conditional because + # there are parentheses around it. So we'll just leave it in place. false - in { statements: { body: [truthy] }, consequent: Else[statements: { body: [falsy] }] } - ternaryable?(truthy) && ternaryable?(falsy) else - false + # Otherwise, we're going to check the conditional for certain cases. + case node + in { predicate: Assign | Command | CommandCall | MAssign | OpAssign } + false + in { statements: { body: [truthy] }, consequent: Else[statements: { body: [falsy] }] } + ternaryable?(truthy) && ternaryable?(falsy) + else + false + end end end @@ -5005,7 +5094,7 @@ def initialize(keyword, node) def format(q) # If we can transform this node into a ternary, then we're going to print # a special version that uses the ternary operator if it fits on one line. - if Ternaryable.call(node) + if Ternaryable.call(q, node) format_ternary(q) return end @@ -5220,7 +5309,7 @@ def format(q) Yield0, ZSuper ] - if force_flat.include?(truthy.class) || force_flat.include?(falsy.class) + if q.parent.is_a?(Paren) || force_flat.include?(truthy.class) || force_flat.include?(falsy.class) q.group { format_flat(q) } return end @@ -8659,7 +8748,7 @@ def deconstruct_keys(keys) def format(q) parent = q.parents.take(2)[1] - ternary = (parent.is_a?(If) || parent.is_a?(Unless)) && Ternaryable.call(parent) + ternary = (parent.is_a?(If) || parent.is_a?(Unless)) && Ternaryable.call(q, parent) q.text("not") diff --git a/lib/syntax_tree/prettyprint.rb b/lib/syntax_tree/prettyprint.rb index a93a6f2d..0950ddfa 100644 --- a/lib/syntax_tree/prettyprint.rb +++ b/lib/syntax_tree/prettyprint.rb @@ -79,7 +79,7 @@ def initialize(indent:, contents: []) end def pretty_print(q) - q.group(2, "align([", "])") do + q.group(2, "align#{indent}([", "])") do q.seplist(contents) { |content| q.pp(content) } end end @@ -161,7 +161,7 @@ def break? end def pretty_print(q) - q.group(2, "group([", "])") do + q.group(2, break? ? "breakGroup([" : "group([", "])") do q.seplist(contents) { |content| q.pp(content) } end end @@ -763,9 +763,7 @@ def flush # This is a linear stack instead of a mutually recursive call defined on # the individual doc nodes for efficiency. - while commands.any? - indent, mode, doc = commands.pop - + while (indent, mode, doc = commands.pop) case doc when Text doc.objects.each { |object| buffer << object } @@ -793,10 +791,10 @@ def flush end end when IfBreak - if mode == MODE_BREAK - commands << [indent, mode, doc.break_contents] if doc.break_contents - elsif mode == MODE_FLAT - commands << [indent, mode, doc.flat_contents] if doc.flat_contents + if mode == MODE_BREAK && doc.break_contents.any? + commands << [indent, mode, doc.break_contents] + elsif mode == MODE_FLAT && doc.flat_contents.any? + commands << [indent, mode, doc.flat_contents] end when LineSuffix line_suffixes << [indent, mode, doc.contents, doc.priority] @@ -1130,10 +1128,10 @@ def fits?(next_command, rest_commands, remaining) when Group commands << [indent, doc.break? ? MODE_BREAK : mode, doc.contents] when IfBreak - if mode == MODE_BREAK - commands << [indent, mode, doc.break_contents] if doc.break_contents - else - commands << [indent, mode, doc.flat_contents] if doc.flat_contents + if mode == MODE_BREAK && doc.break_contents.any? + commands << [indent, mode, doc.break_contents] + elsif mode == MODE_FLAT && doc.flat_contents.any? + commands << [indent, mode, doc.flat_contents] end when Breakable if mode == MODE_FLAT && !doc.force? diff --git a/test/fixtures/next.rb b/test/fixtures/next.rb index 61d31027..be667951 100644 --- a/test/fixtures/next.rb +++ b/test/fixtures/next.rb @@ -53,3 +53,15 @@ next(A) - next A +% +next([]) +- +next [] +% +next([1]) +- +next [1] +% +next([1, 2]) +- +next 1, 2 From e536c2a5a1be25e9aaa78fdb1972dee1f5e1d737 Mon Sep 17 00:00:00 2001 From: Kevin Newton Date: Fri, 22 Apr 2022 12:21:06 -0400 Subject: [PATCH 25/25] Bump to v2.3.0 --- CHANGELOG.md | 24 +++++++++++++++++++++++- Gemfile.lock | 2 +- lib/syntax_tree/version.rb | 2 +- 3 files changed, 25 insertions(+), 3 deletions(-) diff --git a/CHANGELOG.md b/CHANGELOG.md index 94df9307..f8f0ee7d 100644 --- a/CHANGELOG.md +++ b/CHANGELOG.md @@ -6,6 +6,27 @@ The format is based on [Keep a Changelog](http://keepachangelog.com/en/1.0.0/) a ## [Unreleased] +## [2.3.0] - 2022-04-22 + +### Added + +- [#52](https://github.com/ruby-syntax-tree/syntax_tree/pull/52) - `SyntaxTree::Formatter.format` for formatting an already parsed node. +- [#56](https://github.com/ruby-syntax-tree/syntax_tree/pull/56) - `if` and `unless` can now be transformed into ternaries if they're simple enough. +- [#56](https://github.com/ruby-syntax-tree/syntax_tree/pull/56) - Nicely format call chains by one indentation. +- [#56](https://github.com/ruby-syntax-tree/syntax_tree/pull/56) - Handle trailing operators in call chains when they are necessary because of comments. +- [#56](https://github.com/ruby-syntax-tree/syntax_tree/pull/56) - Add some specialized formatting for Sorbet `sig` blocks to make them appear nicer. + +### Changed + +- [#53](https://github.com/ruby-syntax-tree/syntax_tree/pull/53) - Optional keyword arguments on method declarations have a value of `nil` now instead of `false`. This makes it easier to use the visitor. +- [#54](https://github.com/ruby-syntax-tree/syntax_tree/pull/54) - Flow control operators can now skip parentheses for simple, individual arguments. e.g., `break(1)` becomes `break 1`. +- [#54](https://github.com/ruby-syntax-tree/syntax_tree/pull/54) - Don't allow modifier conditionals to modify ternaries. +- [#55](https://github.com/ruby-syntax-tree/syntax_tree/pull/55) - Skip parentheses and brackets on arrays for flow control operators. e.g., `break([1, 2, 3])` becomes `break 1, 2, 3`. +- [#56](https://github.com/ruby-syntax-tree/syntax_tree/pull/56) - Don't add parentheses to method calls if you don't need them. +- [#56](https://github.com/ruby-syntax-tree/syntax_tree/pull/56) - Format comments on empty parameter sets. e.g., `def foo # bar` should keeps its comment. +- [#56](https://github.com/ruby-syntax-tree/syntax_tree/pull/56) - `%s[]` symbols on assignments should not indent to the next line. +- [#56](https://github.com/ruby-syntax-tree/syntax_tree/pull/56) - Empty hash and array literals with comments inside of them should be formatted correctly. + ## [2.2.0] - 2022-04-19 ### Added @@ -166,7 +187,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/v2.2.0...HEAD +[unreleased]: https://github.com/ruby-syntax-tree/syntax_tree/compare/v2.3.0...HEAD +[2.3.0]: https://github.com/ruby-syntax-tree/syntax_tree/compare/v2.2.0...v2.3.0 [2.2.0]: https://github.com/ruby-syntax-tree/syntax_tree/compare/v2.1.1...v2.2.0 [2.1.1]: https://github.com/ruby-syntax-tree/syntax_tree/compare/v2.1.0...v2.1.1 [2.1.0]: https://github.com/ruby-syntax-tree/syntax_tree/compare/v2.0.1...v2.1.0 diff --git a/Gemfile.lock b/Gemfile.lock index bb6765ff..36153378 100644 --- a/Gemfile.lock +++ b/Gemfile.lock @@ -1,7 +1,7 @@ PATH remote: . specs: - syntax_tree (2.2.0) + syntax_tree (2.3.0) GEM remote: https://rubygems.org/ diff --git a/lib/syntax_tree/version.rb b/lib/syntax_tree/version.rb index 16890604..60330ba1 100644 --- a/lib/syntax_tree/version.rb +++ b/lib/syntax_tree/version.rb @@ -1,5 +1,5 @@ # frozen_string_literal: true module SyntaxTree - VERSION = "2.2.0" + VERSION = "2.3.0" end