Merge pull request #580 from nobodywasishere/nobody/heredoc-escape

Author: Sija

spec/ameba/ast/util_spec.cr

@@ -99,10 +99,10 @@ module Ameba::AST
       end
 
       it "does not report source of node which has incorrect location" do
-        s = <<-'CRYSTAL'
+        s = <<-CRYSTAL
           module MyModule
             macro conditional_error_for_inline_callbacks
-              \{%
+              {%
                 raise ""
               %}
             end

spec/ameba/rule/style/heredoc_escape_spec.cr

@@ -0,0 +1,123 @@
+require "../../../spec_helper"
+
+module Ameba::Rule::Style
+  describe HeredocEscape do
+    subject = HeredocEscape.new
+
+    it "passes if a heredoc doesn't contain interpolation" do
+      expect_no_issues subject, <<-CRYSTAL
+        <<-HEREDOC
+          foo
+          HEREDOC
+        CRYSTAL
+    end
+
+    it "passes if a heredoc contains interpolation" do
+      expect_no_issues subject, <<-'CRYSTAL'
+        <<-HEREDOC
+          foo #{:bar}
+          HEREDOC
+        CRYSTAL
+    end
+
+    it "passes if a heredoc contains normal and escaped interpolation" do
+      expect_no_issues subject, <<-'CRYSTAL'
+        <<-HEREDOC
+          foo \#{:bar} #{:baz}
+          HEREDOC
+        CRYSTAL
+    end
+
+    it "passes if a heredoc contains an escape sequence and escaped interpolation" do
+      expect_no_issues subject, <<-'CRYSTAL'
+        <<-HEREDOC
+          foo \377 \xFF \uFFFF \u{0} \t \n \#{:baz}
+          HEREDOC
+        CRYSTAL
+    end
+
+    it "passes if a heredoc contains an escaped escape sequence and interpolation" do
+      expect_no_issues subject, <<-'CRYSTAL'
+        <<-HEREDOC
+          foo \\377 \\xFF \\uFFFF \\u{0} \\t \\n #{:baz}
+          HEREDOC
+        CRYSTAL
+    end
+
+    it "fails if a heredoc contains escaped interpolation" do
+      expect_issue subject, <<-'CRYSTAL'
+        <<-HEREDOC
+        # ^^^^^^^^ error: Use an escaped heredoc marker: `<<-'HEREDOC'`
+          foo \#{:bar}
+          HEREDOC
+        CRYSTAL
+    end
+
+    it "fails if a heredoc contains escaped interpolation and escaped escape sequences" do
+      expect_issue subject, <<-'CRYSTAL'
+        <<-HEREDOC
+        # ^^^^^^^^ error: Use an escaped heredoc marker: `<<-'HEREDOC'`
+          foo \\t \#{:bar}
+          HEREDOC
+        CRYSTAL
+    end
+
+    it "passes if a heredoc contains normal and escaped escape sequences" do
+      expect_no_issues subject, <<-'CRYSTAL'
+        <<-HEREDOC
+          foo \t \n | \\t \\n
+          HEREDOC
+        CRYSTAL
+    end
+
+    it "fails if a heredoc contains escaped escape sequences" do
+      expect_issue subject, <<-'CRYSTAL'
+        <<-HEREDOC
+        # ^^^^^^^^ error: Use an escaped heredoc marker: `<<-'HEREDOC'`
+          \\t \\n
+          HEREDOC
+        CRYSTAL
+    end
+
+    it "passes if an escaped heredoc contains interpolation" do
+      expect_no_issues subject, <<-'CRYSTAL'
+        <<-'HEREDOC'
+          foo #{:bar}
+          HEREDOC
+        CRYSTAL
+    end
+
+    it "passes if an escaped heredoc contains escaped interpolation" do
+      expect_no_issues subject, <<-'CRYSTAL'
+        <<-'HEREDOC'
+          foo \#{:bar}
+          HEREDOC
+        CRYSTAL
+    end
+
+    it "passes if an escaped heredoc contains escape sequences" do
+      expect_no_issues subject, <<-'CRYSTAL'
+        <<-'HEREDOC'
+          foo \377 \xFF \uFFFF \u{0} \t \n
+          HEREDOC
+        CRYSTAL
+    end
+
+    it "passes if an escaped heredoc contains escaped escape sequences" do
+      expect_no_issues subject, <<-'CRYSTAL'
+        <<-'HEREDOC'
+          foo \\377 \\xFF \\uFFFF \\u{0} \\t \\n
+          HEREDOC
+        CRYSTAL
+    end
+
+    it "fails if an escaped heredoc doesn't contain interpolation" do
+      expect_issue subject, <<-CRYSTAL
+        <<-'HEREDOC'
+        # ^^^^^^^^^^ error: Use an unescaped heredoc marker: `<<-HEREDOC`
+          foo
+          HEREDOC
+        CRYSTAL
+    end
+  end
+end

spec/ameba/rule/style/redundant_self_spec.cr

@@ -249,23 +249,23 @@ module Ameba::Rule::Style
     end
 
     it "reports if there is redundant `self` used in a string interpolation" do
-      source = expect_issue subject, <<-CRYSTAL
+      source = expect_issue subject, <<-'CRYSTAL'
         class Foo
           def foo; end
 
           def foo!
-            "\#{self.foo || 42}"
+            "#{self.foo || 42}"
              # ^^^^ error: Redundant `self` detected
           end
         end
         CRYSTAL
 
-      expect_correction source, <<-CRYSTAL
+      expect_correction source, <<-'CRYSTAL'
         class Foo
           def foo; end
 
           def foo!
-            "\#{foo || 42}"
+            "#{foo || 42}"
           end
         end
         CRYSTAL

src/ameba/rule/style/heredoc_escape.cr

@@ -0,0 +1,78 @@
+module Ameba::Rule::Style
+  # A rule that enforces heredoc variant that escapes interpolation or control
+  # chars in a heredoc body. The opposite is enforced too - i.e. regular heredoc
+  # variant that doesn't escape interpolation or control chars in a heredoc body,
+  # when there is no need to escape it.
+  #
+  # For example, this is considered invalid:
+  #
+  # ```
+  # <<-DOC
+  #   This is an escaped \#{:interpolated} string \\n
+  #   DOC
+  # ```
+  #
+  # And should be written as:
+  #
+  # ```
+  # <<-'DOC'
+  #   This is an escaped #{:interpolated} string \n
+  #   DOC
+  # ```
+  #
+  # YAML configuration example:
+  #
+  # ```
+  # Style/HeredocEscape:
+  #   Enabled: true
+  # ```
+  class HeredocEscape < Base
+    include AST::Util
+
+    properties do
+      since_version "1.7.0"
+      description "Recommends using the heredoc variant that escapes interpolation or control chars in a heredoc body"
+    end
+
+    MSG_ESCAPE_NEEDED     = "Use an escaped heredoc marker: `<<-'%s'`"
+    MSG_ESCAPE_NOT_NEEDED = "Use an unescaped heredoc marker: `<<-%s`"
+
+    ESCAPE_SEQUENCE_PATTERN =
+      /\\(?:[abefnrtv]|[0-7]{1,3}|x[0-9a-fA-F]{2}|u[0-9a-fA-F]{4}|u\{[0-9a-fA-F]{1,6}\})/
+
+    def test(source, node : Crystal::StringInterpolation)
+      # Heredocs without interpolations have always size of 1
+      return unless node.expressions.size == 1
+      return unless expr = node.expressions.first.as?(Crystal::StringLiteral)
+
+      return unless code = node_source(node, source.lines)
+      return unless code.starts_with?("<<-")
+
+      body = code.lines[1..-2].join('\n')
+
+      if code.starts_with?("<<-'")
+        return if has_escape_sequence?(expr.value) || has_escaped_escape_sequence?(body)
+
+        marker = code.lchop("<<-'").match!(/^(\w+)/)[1]
+        msg = MSG_ESCAPE_NOT_NEEDED % marker
+      else
+        return if !has_escape_sequence?(expr.value) || has_escape_sequence?(body)
+
+        marker = code.lchop("<<-").match!(/^(\w+)/)[1]
+        msg = MSG_ESCAPE_NEEDED % marker
+      end
+
+      issue_for node, msg
+    end
+
+    private def has_escape_sequence?(value : String)
+      value.matches? /(?<!\\)(?:#\{|#{ESCAPE_SEQUENCE_PATTERN})/,
+        options: :no_utf_check
+    end
+
+    private def has_escaped_escape_sequence?(value : String)
+      value.matches? /(?<!\\)(?:\\)+(?:#\{|#{ESCAPE_SEQUENCE_PATTERN})/,
+        options: :no_utf_check
+    end
+  end
+end