Fold IfMod into If and UnlessMod into Unless

kddnewton · kddnewton · commit 38a1b036593e · 2022-11-02T10:53:11.000-04:00
diff --git a/CHANGELOG.md b/CHANGELOG.md
@@ -6,6 +6,11 @@ The format is based on [Keep a Changelog](http://keepachangelog.com/en/1.0.0/) a
 
 ## [Unreleased]
 
+### Changed
+
+- Nodes no longer have a `comments:` keyword on their initializers. By default, they initialize to an empty array. If you were previously passing comments into the initializer, you should now create the node first, then call `node.comments.concat` to add your comments.
+- `IfMod` and `UnlessMod` are no longer nodes. Instead, they have been folded into `If` and `Unless`, respectively. The `If` and `Unless` nodes now have a `modifier?` method to tell you if they were original found in the modifier form.
+
 ## [4.3.0] - 2022-10-28
 
 ### Added
diff --git a/lib/syntax_tree/node.rb b/lib/syntax_tree/node.rb
@@ -2071,8 +2071,7 @@ def forced_brace_bounds?(q)
         when Paren, Statements
           # If we hit certain breakpoints then we know we're safe.
           return false
-        when If, IfMod, IfOp, Unless, UnlessMod, While, WhileMod, Until,
-             UntilMod
+        when If, IfOp, Unless, While, WhileMod, Until, UntilMod
           return true if parent.predicate == previous
         end
 
@@ -3884,7 +3883,7 @@ def format(q)
       q.format(left) if left
 
       case q.parent
-      when If, IfMod, Unless, UnlessMod
+      when If, Unless
         q.text(" #{operator} ")
       else
         q.text(operator)
@@ -5398,10 +5397,10 @@ def call(q, node)
       # and default instead to breaking them into multiple lines.
       def ternaryable?(statement)
         case statement
-        when 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
+        when Alias, Assign, Break, Command, CommandCall, Heredoc, If, IfOp,
+             Lambda, MAssign, Next, OpAssign, RescueMod, Return, Return0, Super,
+             Undef, Unless, Until, UntilMod, VarAlias, VoidStmt, While,
+             WhileMod, Yield, Yield0, ZSuper
           # This is a list of nodes that should not be allowed to be a part of a
           # ternary clause.
           false
@@ -5432,41 +5431,60 @@ 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 Ternaryable.call(q, node)
-        format_ternary(q)
-        return
-      end
+      if node.modifier?
+        statement = node.statements.body[0]
 
-      # 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
-      # and we must use the block form.
-      if ContainsAssignment.call(node.predicate)
-        format_break(q, force: true)
-        return
-      end
-
-      if node.consequent || node.statements.empty? || contains_conditional?
-        q.group { format_break(q, force: true) }
+        if ContainsAssignment.call(statement) || q.parent.is_a?(In)
+          q.group { format_flat(q) }
+        else
+          q.group { q.if_break { format_break(q, force: false) }.if_flat { format_flat(q) } }
+        end
       else
-        q.group do
-          q
-            .if_break { format_break(q, force: false) }
-            .if_flat do
-              Parentheses.flat(q) do
-                q.format(node.statements)
-                q.text(" #{keyword} ")
-                q.format(node.predicate)
+        # 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(q, node)
+          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
+        # and we must use the block form.
+        if ContainsAssignment.call(node.predicate)
+          format_break(q, force: true)
+          return
+        end
+
+        if node.consequent || node.statements.empty? || contains_conditional?
+          q.group { format_break(q, force: true) }
+        else
+          q.group do
+            q
+              .if_break { format_break(q, force: false) }
+              .if_flat do
+                Parentheses.flat(q) do
+                  q.format(node.statements)
+                  q.text(" #{keyword} ")
+                  q.format(node.predicate)
+                end
               end
-            end
+          end
         end
       end
     end
 
     private
 
+    def format_flat(q)
+      Parentheses.flat(q) do
+        q.format(node.statements.body[0])
+        q.text(" #{keyword} ")
+        q.format(node.predicate)
+      end
+    end
+
     def format_break(q, force:)
       q.text("#{keyword} ")
       q.nest(keyword.length + 1) { q.format(node.predicate) }
@@ -5537,7 +5555,7 @@ def contains_conditional?
       return false if statements.length != 1
 
       case statements.first
-      when If, IfMod, IfOp, Unless, UnlessMod
+      when If, IfOp, Unless
         true
       else
         false
@@ -5600,6 +5618,11 @@ def deconstruct_keys(_keys)
     def format(q)
       ConditionalFormatter.new("if", self).format(q)
     end
+
+    # Checks if the node was originally found in the modifier form.
+    def modifier?
+      predicate.location.start_char > statements.location.start_char
+    end
   end
 
   # IfOp represents a ternary clause.
@@ -5649,10 +5672,9 @@ def deconstruct_keys(_keys)
 
     def format(q)
       force_flat = [
-        Alias, Assign, Break, Command, CommandCall, Heredoc, If, IfMod, IfOp,
-        Lambda, MAssign, Next, OpAssign, RescueMod, Return, Return0, Super,
-        Undef, Unless, UnlessMod, UntilMod, VarAlias, VoidStmt, WhileMod, Yield,
-        Yield0, ZSuper
+        Alias, Assign, Break, Command, CommandCall, Heredoc, If, IfOp, Lambda,
+        MAssign, Next, OpAssign, RescueMod, Return, Return0, Super, Undef,
+        Unless, UntilMod, VarAlias, VoidStmt, WhileMod, Yield, Yield0, ZSuper
       ]
 
       if q.parent.is_a?(Paren) || force_flat.include?(truthy.class) ||
@@ -5704,94 +5726,6 @@ def format_flat(q)
     end
   end
 
-  # Formats an IfMod or UnlessMod node.
-  class ConditionalModFormatter
-    # [String] the keyword associated with this conditional
-    attr_reader :keyword
-
-    # [IfMod | UnlessMod] the node that is being formatted
-    attr_reader :node
-
-    def initialize(keyword, node)
-      @keyword = keyword
-      @node = node
-    end
-
-    def format(q)
-      if ContainsAssignment.call(node.statement) || q.parent.is_a?(In)
-        q.group { format_flat(q) }
-      else
-        q.group { q.if_break { format_break(q) }.if_flat { format_flat(q) } }
-      end
-    end
-
-    private
-
-    def format_break(q)
-      q.text("#{keyword} ")
-      q.nest(keyword.length + 1) { q.format(node.predicate) }
-      q.indent do
-        q.breakable_space
-        q.format(node.statement)
-      end
-      q.breakable_space
-      q.text("end")
-    end
-
-    def format_flat(q)
-      Parentheses.flat(q) do
-        q.format(node.statement)
-        q.text(" #{keyword} ")
-        q.format(node.predicate)
-      end
-    end
-  end
-
-  # IfMod represents the modifier form of an +if+ statement.
-  #
-  #     expression if predicate
-  #
-  class IfMod < Node
-    # [untyped] the expression to be executed
-    attr_reader :statement
-
-    # [untyped] the expression to be checked
-    attr_reader :predicate
-
-    # [Array[ Comment | EmbDoc ]] the comments attached to this node
-    attr_reader :comments
-
-    def initialize(statement:, predicate:, location:)
-      @statement = statement
-      @predicate = predicate
-      @location = location
-      @comments = []
-    end
-
-    def accept(visitor)
-      visitor.visit_if_mod(self)
-    end
-
-    def child_nodes
-      [statement, predicate]
-    end
-
-    alias deconstruct child_nodes
-
-    def deconstruct_keys(_keys)
-      {
-        statement: statement,
-        predicate: predicate,
-        location: location,
-        comments: comments
-      }
-    end
-
-    def format(q)
-      ConditionalModFormatter.new("if", self).format(q)
-    end
-  end
-
   # Imaginary represents an imaginary number literal.
   #
   #     1i
@@ -9443,50 +9377,10 @@ def deconstruct_keys(_keys)
     def format(q)
       ConditionalFormatter.new("unless", self).format(q)
     end
-  end
-
-  # UnlessMod represents the modifier form of an +unless+ statement.
-  #
-  #     expression unless predicate
-  #
-  class UnlessMod < Node
-    # [untyped] the expression to be executed
-    attr_reader :statement
-
-    # [untyped] the expression to be checked
-    attr_reader :predicate
-
-    # [Array[ Comment | EmbDoc ]] the comments attached to this node
-    attr_reader :comments
-
-    def initialize(statement:, predicate:, location:)
-      @statement = statement
-      @predicate = predicate
-      @location = location
-      @comments = []
-    end
-
-    def accept(visitor)
-      visitor.visit_unless_mod(self)
-    end
-
-    def child_nodes
-      [statement, predicate]
-    end
-
-    alias deconstruct child_nodes
-
-    def deconstruct_keys(_keys)
-      {
-        statement: statement,
-        predicate: predicate,
-        location: location,
-        comments: comments
-      }
-    end
 
-    def format(q)
-      ConditionalModFormatter.new("unless", self).format(q)
+    # Checks if the node was originally found in the modifier form.
+    def modifier?
+      predicate.location.start_char > statements.location.start_char
     end
   end
 
diff --git a/lib/syntax_tree/parser.rb b/lib/syntax_tree/parser.rb
@@ -1911,13 +1911,14 @@ def on_ifop(predicate, truthy, falsy)
     end
 
     # :call-seq:
-    #   on_if_mod: (untyped predicate, untyped statement) -> IfMod
+    #   on_if_mod: (untyped predicate, untyped statement) -> If
     def on_if_mod(predicate, statement)
       consume_keyword(:if)
 
-      IfMod.new(
-        statement: statement,
+      If.new(
         predicate: predicate,
+        statements: Statements.new(self, body: [statement], location: statement.location),
+        consequent: nil,
         location: statement.location.to(predicate.location)
       )
     end
@@ -3586,13 +3587,14 @@ def on_unless(predicate, statements, consequent)
     end
 
     # :call-seq:
-    #   on_unless_mod: (untyped predicate, untyped statement) -> UnlessMod
+    #   on_unless_mod: (untyped predicate, untyped statement) -> Unless
     def on_unless_mod(predicate, statement)
       consume_keyword(:unless)
 
-      UnlessMod.new(
-        statement: statement,
+      Unless.new(
         predicate: predicate,
+        statements: Statements.new(self, body: [statement], location: statement.location),
+        consequent: nil,
         location: statement.location.to(predicate.location)
       )
     end
diff --git a/lib/syntax_tree/visitor.rb b/lib/syntax_tree/visitor.rb
@@ -206,9 +206,6 @@ class Visitor < BasicVisitor
     # Visit an If node.
     alias visit_if visit_child_nodes
 
-    # Visit an IfMod node.
-    alias visit_if_mod visit_child_nodes
-
     # Visit an IfOp node.
     alias visit_if_op visit_child_nodes
 
@@ -431,9 +428,6 @@ class Visitor < BasicVisitor
     # Visit an Unless node.
     alias visit_unless visit_child_nodes
 
-    # Visit an UnlessMod node.
-    alias visit_unless_mod visit_child_nodes
-
     # Visit an Until node.
     alias visit_until visit_child_nodes
 
diff --git a/lib/syntax_tree/visitor/field_visitor.rb b/lib/syntax_tree/visitor/field_visitor.rb
diff --git a/test/node_test.rb b/test/node_test.rb
