Format most ERB block open statements

lib/erb/formatter.rb

@@ -39,7 +39,7 @@ class Error < StandardError; end
   ATTR = Regexp.union(SINGLE_QUOTE_ATTR, DOUBLE_QUOTE_ATTR, UNQUOTED_ATTR, UNQUOTED_VALUE)
   MULTILINE_ATTR_NAMES = %w[class data-action]
 
-  ERB_TAG = %r{(<%(?:==|=|-|))\s*(.*?)\s*(-?%>)}m
+  ERB_TAG = %r{(<%(?:=|-|#)*)(?:(?!\n)\s)*(.*?)\s*(-?%>)}m
   ERB_PLACEHOLDER = %r{erb[a-z0-9]+tag}
 
   TAG_NAME = /[a-z0-9_:-]+/
@@ -267,18 +267,22 @@ def format_ruby(code, autoclose: false)
     end
     p RUBY_IN_: code if @debug
 
-    SyntaxTree::Command.prepend SyntaxTreeCommandPatch
+    # SyntaxTree::Command.prepend SyntaxTreeCommandPatch
 
     code = begin
-      SyntaxTree.format(code, @line_width)
+      # TODO: For single-lines, 7 should be subtracted instead of 2: 3 for opening, 2 for closing and 2 surrounding spaces
+      # Subtract 2 for multiline indentation or for the surrounding tags
+      # Then subtract twice the tag_stack size to respect indentation
+      width = @line_width - 2 - tag_stack.size * 2
+      SyntaxTree.format(code, width)
     rescue SyntaxTree::Parser::ParseError => error
       p RUBY_PARSE_ERROR: error if @debug
       code
     end
 
     lines = code.strip.lines
     lines = lines[0...-1] if autoclose
-    code = lines.map { |l| indented(l.chomp("\n"), strip: false) }.join.strip
+    code = lines.map { |l| indented(l.chomp("\n"), strip: false) }.join
     p RUBY_OUT: code if @debug
     code
   end
@@ -304,31 +308,59 @@ def format_erb_tags(string)
         format_text(erb_pre_match)
 
         erb_open, ruby_code, erb_close = ERB_TAG.match(erb_code).captures
-        erb_open << ' ' unless ruby_code.start_with?('#')
+        ruby_code.strip!
+
+        block_type =
+          if erb_open.include?('#')
+            :comment
+          else
+            case ruby_code
+            when RUBY_STANDALONE_BLOCK then :standalone
+            when RUBY_CLOSE_BLOCK then :close
+            when RUBY_REOPEN_BLOCK then :reopen
+            when RUBY_OPEN_BLOCK then :open
+            else :other
+            end
+          end
 
-        case ruby_code
-        when RUBY_STANDALONE_BLOCK
+        # Format Ruby code, and indent if it's multiline
+        if block_type == :open
+          # Block openers aren't complete ruby scripts, so syntax_tree won't like them.
+          # These are two workarounds to help make most of them valid, so we can format them anyway.
+          if (match = ruby_code.match(/(?:\s+do|\s*\{)(?:\s*\|\s*\w+\s*(?:,\s*\w+\s*)*\|)?\s*\z/))
+            # If this is a block starter (ends with "do" or "{" followed by optional block parameters), it usually is a
+            #   valid statement without the suffix
+            suffix = match[0]
+            ruby_code = "#{format_ruby(ruby_code.chomp(suffix), autoclose: false)} #{suffix.strip}"
+          elsif ruby_code.start_with?('if ', 'unless ', 'while ', 'until ')
+            # If this is a condition or loop, it may be a valid expression without first word
+            keyword, rest = ruby_code.split(/\s+/, 2)
+            ruby_code = format_ruby(rest, autoclose: false).sub(/^(\s*)/, "\\1#{keyword} " )
+          end
+          ruby_code.gsub!(/^/, '  ') if ruby_code.strip.include?("\n")
+        elsif block_type == :standalone || block_type == :other
           ruby_code = format_ruby(ruby_code, autoclose: false)
-          full_erb_tag = "#{erb_open}#{ruby_code} #{erb_close}"
-          html << (erb_pre_match.match?(/\s+\z/) ? indented(full_erb_tag) : full_erb_tag)
-        when RUBY_CLOSE_BLOCK
-          full_erb_tag = "#{erb_open}#{ruby_code} #{erb_close}"
-          tag_stack_pop('%erb%', ruby_code)
-          html << (erb_pre_match.match?(/\s+\z/) ? indented(full_erb_tag) : full_erb_tag)
-        when RUBY_REOPEN_BLOCK
-          full_erb_tag = "#{erb_open}#{ruby_code} #{erb_close}"
-          tag_stack_pop('%erb%', ruby_code)
-          html << (erb_pre_match.match?(/\s+\z/) ? indented(full_erb_tag) : full_erb_tag)
-          tag_stack_push('%erb%', ruby_code)
-        when RUBY_OPEN_BLOCK
-          full_erb_tag = "#{erb_open}#{ruby_code} #{erb_close}"
-          html << (erb_pre_match.match?(/\s+\z/) ? indented(full_erb_tag) : full_erb_tag)
-          tag_stack_push('%erb%', ruby_code)
+          ruby_code.gsub!(/^/, '  ') if ruby_code.strip.include?("\n")
+        end
+
+        # Remove the first line if it only has whitespace
+        ruby_code.sub!(/\A((?!\n)\s)*\n/, '')
+
+        # Reset "common" indentation of multi-line comments
+        if block_type == :comment && ruby_code.strip.include?("\n")
+          # Leave comments intact, but if they're multiline, replace common indentation
+          ruby_code.gsub!(/^#{ruby_code.scan(/^ */).min_by(&:length)}/, '  ')
+        end
+
+        if ruby_code.strip.include?("\n")
+          full_erb_tag = "#{erb_open}\n#{ruby_code}#{indented(erb_close)}"
         else
-          ruby_code = format_ruby(ruby_code, autoclose: false)
-          full_erb_tag = "#{erb_open}#{ruby_code} #{erb_close}"
-          html << (erb_pre_match.match?(/\s+\z/) ? indented(full_erb_tag) : full_erb_tag)
+          full_erb_tag = "#{erb_open} #{ruby_code.strip} #{erb_close}"
         end
+
+        tag_stack_pop('%erb%', ruby_code) if %i[close reopen].include? block_type
+        html << (erb_pre_match.match?(/\s+\z/) ? indented(full_erb_tag) : full_erb_tag)
+        tag_stack_push('%erb%', ruby_code) if %i[reopen open].include? block_type
       else
         p ERB_REST: erb_scanner.rest if @debug
         rest = erb_scanner.rest.to_s

test/erb/test_formatter.rb

@@ -146,12 +146,16 @@ def test_tagnames_with_dashes
 
   def test_format_ruby
     assert_equal(
-      "<div>\n" \
-      "  <%= render MyComponent.new(\n" \
-      "    foo: barbarbarbarbarbarbarbar,\n" \
-      "    bar: bazbazbazbazbazbazbazbaz,\n" \
-      "  ) %>\n" \
-      "</div>\n",
+      <<~ERB,
+        <div>
+          <%=
+            render MyComponent.new(
+              foo: barbarbarbarbarbarbarbar,
+              bar: bazbazbazbazbazbazbazbaz,
+            )
+          %>
+        </div>
+      ERB
       ERB::Formatter.format("<div> <%=render MyComponent.new(foo:barbarbarbarbarbarbarbar,bar:bazbazbazbazbazbazbazbaz)%> </div>"),
     )
   end

test/fixtures/comments-2.html.erb

@@ -4,5 +4,5 @@
 <%-# link_to 'Approve', some_path, class: 'something', disabled: disabled %>
 
 <%# if smth %>
-<%#else %>
+<%# else %>
 <%# end %>

test/fixtures/comments-2.html.expected.erb

@@ -4,5 +4,5 @@
 <%-# link_to 'Approve', some_path, class: 'something', disabled: disabled %>
 
 <%# if smth %>
-<%#else %>
+<%# else %>
 <%# end %>

test/fixtures/comments.html.erb

@@ -72,4 +72,4 @@ This fails
 
 <%#
 hey
-%>
+%>

test/fixtures/comments.html.expected.erb

@@ -1,64 +1,74 @@
 <%#
-This fails
-hey
-hey
-hey
-hey %>
+  This fails
+    hey
+      hey
+        hey
+    hey
+%>
 
 <%#
-This fails
-hey
-hey
-hey
-hey %>
+  This fails
+  hey
+    hey
+      hey
+  hey
+%>
 
 <%#
-This fails
-hey
-hey
-hey
-hey %>
+  This fails
+  hey
+    hey
+      hey
+  hey
+%>
 
-<%# This fails
-This fails
-hey
-hey
-hey
-hey %>
-
-<%# This fails
-This fails
-hey
-hey
-hey
-hey %>
-
-<%# This fails
-This fails
-hey
-hey
-hey
-hey %>
-
-<%#This fails
-This fails
-hey
-hey
-hey
-hey %>
+<%#
+  This fails
+    This fails
+    hey
+      hey
+        hey
+    hey
+%>
 
-<%#    This fails
-This fails
-hey
-hey
-hey
-hey %>
+<%#
+  This fails
+  This fails
+    hey
+      hey
+        hey
+    hey
+%>
 
 <%#
-hey %>
+  This fails
+      This fails
+      hey
+        hey
+          hey
+      hey
+%>
 
 <%#
-hey %>
+  This fails
+    This fails
+    hey
+      hey
+        hey
+    hey
+%>
 
 <%#
-hey %>
+  This fails
+    This fails
+    hey
+      hey
+        hey
+    hey
+%>
+
+<%# hey %>
+
+<%# hey %>
+
+<%# hey %>

test/fixtures/complex_case_when.html.expected.erb

@@ -14,11 +14,13 @@
     <% when 'Foo::PaymentMethod' %>
       <span><%= t(".payment.stripe_invoice") %></span>
     <% else %>
-      <% Rails.logger.error.report(
-        StandardError.new(
-          "No human readable name found for payment method #{payment_method.class}",
-        ),
-      ) %>
+      <%
+        Rails.logger.error.report(
+          StandardError.new(
+            "No human readable name found for payment method #{payment_method.class}",
+          ),
+        )
+      %>
     <% end %>
   <% else %>
     <%= t(".payment.no_payment_method_found") %>

test/fixtures/formatted-2.html.expected.erb

@@ -5,12 +5,14 @@
   <%= react_component("HelloWorld", { greeting: "Hello from react-rails." }) %>
 
   <%= react_component("HelloWorld", { greeting: "Hello from react-rails." }) %>
-  <%= react_component(
-    "HelloWorld",
-    { greeting: "Hello from react-rails." },
-    { greeting: "Hello from react-rails." },
-    { greeting: "Hello from react-rails." },
-    { greeting: "Hello from react-rails." },
-  ) %>
+  <%=
+    react_component(
+      "HelloWorld",
+      { greeting: "Hello from react-rails." },
+      { greeting: "Hello from react-rails." },
+      { greeting: "Hello from react-rails." },
+      { greeting: "Hello from react-rails." },
+    )
+  %>
 
 </div>

test/fixtures/formatted.html.expected.erb

@@ -1,6 +1,8 @@
-<% link_to "Very long string here and there",
-very_very_very_long_long_long_pathhhhhh_here,
-opt: "212",
-options: "222sdasdasd",
-class: "  322 ",
-dis: diss %>
+<%
+  link_to "Very long string here and there",
+  very_very_very_long_long_long_pathhhhhh_here,
+  opt: "212",
+  options: "222sdasdasd",
+  class: "  322 ",
+  dis: diss
+%>

test/fixtures/if_then_else.html.expected.erb

@@ -1,17 +1,21 @@
 <% eeee ? "b" : c %>
 <% eeee ? a : c %>
 
-<% if longlonglonglonglonglonglonglonglonglonglonglonglonglonglonglonglonglonglonglonglonglonglonglonglonglonglonglong
-  a
-else
-  c
-end %>
+<%
+  if longlonglonglonglonglonglonglonglonglonglonglonglonglonglonglonglonglonglonglonglonglonglonglonglonglonglonglong
+    a
+  else
+    c
+  end
+%>
 
-<% if longlonglonglonglonglonglonglonglonglonglonglonglonglonglonglonglonglonglonglonglonglonglonglonglonglonglonglong
-  "a"
-else
-  "c"
-end %>
+<%
+  if longlonglonglonglonglonglonglonglonglonglonglonglonglonglonglonglonglonglonglonglonglonglonglonglonglonglonglong
+    "a"
+  else
+    "c"
+  end
+%>
 
 <div <% if eeee then "b" else c end %>></div>
 <div <% if eeee then a else c end %>></div>