Add Style/VerboseBlock rule

spec/ameba/rule/style/verbose_block_spec.cr

@@ -0,0 +1,131 @@
+require "../../../spec_helper"
+
+module Ameba::Rule::Style
+  subject = VerboseBlock.new
+
+  describe VerboseBlock do
+    it "passes if there is no potential performance improvements" do
+      source = Source.new %(
+        (1..3).any?(&.odd?)
+        (1..3).join('.', &.to_s)
+        (1..3).map { |i| typeof(i) }
+        (1..3).map { |i| i || 0 }
+      )
+      subject.catch(source).should be_valid
+    end
+
+    it "reports if there is a call with a collapsible block" do
+      source = Source.new %(
+        (1..3).any? { |i| i.odd? }
+      )
+      subject.catch(source).should_not be_valid
+    end
+
+    it "reports if there is a call with an argument + collapsible block" do
+      source = Source.new %(
+        (1..3).join('.') { |i| i.to_s }
+      )
+      subject.catch(source).should_not be_valid
+    end
+
+    it "reports if there is a call with a collapsible block (with chained call)" do
+      source = Source.new %(
+        (1..3).map { |i| i.to_s.split.reverse.join.strip }
+      )
+      subject.catch(source).should_not be_valid
+    end
+
+    context "properties" do
+      it "#exclude_calls_with_block" do
+        source = Source.new %(
+          (1..3).in_groups_of(1) { |i| i.map(&.to_s) }
+        )
+        rule = VerboseBlock.new
+        rule
+          .tap(&.exclude_calls_with_block = true)
+          .catch(source).should be_valid
+        rule
+          .tap(&.exclude_calls_with_block = false)
+          .catch(source).should_not be_valid
+      end
+
+      it "#exclude_multiple_line_blocks" do
+        source = Source.new %(
+          (1..3).any? do |i|
+            i.odd?
+          end
+        )
+        rule = VerboseBlock.new
+        rule
+          .tap(&.exclude_multiple_line_blocks = true)
+          .catch(source).should be_valid
+        rule
+          .tap(&.exclude_multiple_line_blocks = false)
+          .catch(source).should_not be_valid
+      end
+
+      it "#exclude_operators" do
+        source = Source.new %(
+          (1..3).sum { |i| i * 2 }
+        )
+        rule = VerboseBlock.new
+        rule
+          .tap(&.exclude_operators = true)
+          .catch(source).should be_valid
+        rule
+          .tap(&.exclude_operators = false)
+          .catch(source).should_not be_valid
+      end
+
+      it "#exclude_setters" do
+        source = Source.new %(
+          Char::Reader.new("abc").tap { |reader| reader.pos = 0 }
+        )
+        rule = VerboseBlock.new
+        rule
+          .tap(&.exclude_setters = true)
+          .catch(source).should be_valid
+        rule
+          .tap(&.exclude_setters = false)
+          .catch(source).should_not be_valid
+      end
+
+      it "#max_length" do
+        source = Source.new %(
+          (1..3).tap { |i| i.to_s.split.reverse.join.strip.blank? }
+        )
+        rule = VerboseBlock.new
+        rule
+          .tap(&.max_length = 30)
+          .catch(source).should be_valid
+        rule
+          .tap(&.max_length = nil)
+          .catch(source).should_not be_valid
+      end
+    end
+
+    context "macro" do
+      it "reports in macro scope" do
+        source = Source.new %(
+          {{ (1..3).any? { |i| i.odd? } }}
+        )
+        subject.catch(source).should_not be_valid
+      end
+    end
+
+    it "reports rule, pos and message" do
+      source = Source.new path: "source.cr", code: %(
+        (1..3).any? { |i| i.odd? }
+      )
+      subject.catch(source).should_not be_valid
+      source.issues.size.should eq 1
+
+      issue = source.issues.first
+      issue.rule.should_not be_nil
+      issue.location.to_s.should eq "source.cr:1:8"
+      issue.end_location.to_s.should eq "source.cr:1:26"
+
+      issue.message.should eq "Use short block notation instead: `any?(&.odd?)`"
+    end
+  end
+end

src/ameba/rule/style/verbose_block.cr

@@ -0,0 +1,150 @@
+module Ameba::Rule::Style
+  # This rule is used to identify usage of single expression blocks with
+  # argument as a receiver, that can be collapsed into a short form.
+  #
+  # For example, this is considered invalid:
+  #
+  # ```
+  # (1..3).any? { |i| i.odd? }
+  # ```
+  #
+  # And it should be written as this:
+  #
+  # ```
+  # (1..3).any?(&.odd?)
+  # ```
+  #
+  # YAML configuration example:
+  #
+  # ```
+  # Style/VerboseBlock:
+  #   Enabled: true
+  #   ExcludeMultipleLineBlocks: true
+  #   ExcludeCallsWithBlocks: false
+  #   ExcludeOperators: false
+  #   ExcludeSetters: false
+  #   MaxLength: 50 # use ~ to disable
+  # ```
+  class VerboseBlock < Base
+    properties do
+      description "Identifies usage of collapsible single expression blocks."
+
+      exclude_calls_with_block true
+      exclude_multiple_line_blocks false
+      exclude_operators false
+      exclude_setters false
+
+      max_length : Int32? = 50
+    end
+
+    MSG          = "Use short block notation instead: `%s`"
+    CALL_PATTERN = "%s(%s&.%s)"
+
+    protected def same_location_lines?(a, b)
+      return unless a_location = a.name_location
+      return unless b_location = b.location
+
+      a_location.line_number == b_location.line_number
+    end
+
+    private OPERATOR_CHARS =
+      {'[', ']', '!', '=', '>', '<', '~', '+', '-', '*', '/', '%', '^', '|', '&'}
+
+    protected def operator?(name)
+      name.each_char do |char|
+        return false unless char.in?(OPERATOR_CHARS)
+      end
+      !name.empty?
+    end
+
+    protected def setter?(name)
+      !name.empty? && name[0].letter? && name.ends_with?('=')
+    end
+
+    protected def valid_length?(code)
+      if max_length = self.max_length
+        return code.size <= max_length
+      end
+      true
+    end
+
+    protected def node_to_s(node : Crystal::Call)
+      case name = node.name
+      when "[]"
+        name = "[#{node.args.join ", "}]"
+      when "[]?"
+        name = "[#{node.args.join ", "}]?"
+      when "[]="
+        unless node.args.empty?
+          name = "[#{node.args[..-2].join ", "}]=(#{node.args.last})"
+        end
+      else
+        name += "(#{node.args.join ", "})" unless node.args.empty?
+        name += " {...}" if node.block
+      end
+      name
+    end
+
+    protected def call_code(call, body)
+      args = call.args.join ", " unless call.args.empty?
+      args += ", " if args
+
+      call_chain = %w[].tap do |arr|
+        obj = body.obj
+        while obj.is_a?(Crystal::Call)
+          arr << node_to_s(obj)
+          obj = obj.obj
+        end
+        arr.reverse!
+        arr << node_to_s(body)
+      end
+
+      name =
+        call_chain.join('.')
+
+      CALL_PATTERN % {call.name, args, name}
+    end
+
+    # ameba:disable Metrics/CyclomaticComplexity
+    def test(source, node : Crystal::Call)
+      # we are interested only in calls with block taking a single argument
+      #
+      # ```
+      # (1..3).any? { |i| i.to_i64.odd? }
+      #        ^---    ^  ^------------
+      #        block  arg  body
+      # ```
+      return unless (block = node.block) && block.args.size == 1
+
+      # we filter out the blocks that are of call type - `i.to_i64.odd?`
+      return unless (body = block.body).is_a?(Crystal::Call)
+
+      # we need to "unwind" the chain challs, so the final receiver object
+      # ends up being a variable - `i`
+      obj = body.obj
+      while obj.is_a?(Crystal::Call)
+        obj = obj.obj
+      end
+      return unless obj.is_a?(Crystal::Var)
+
+      # only calls with a first argument used as a receiver are the valid game
+      return unless (arg = block.args.first) == obj
+
+      # we skip auto-generated blocks - `(1..3).any?(&.odd?)`
+      return if arg.name.starts_with?("__arg")
+
+      return if exclude_calls_with_block && body.block
+      return if exclude_multiple_line_blocks && !same_location_lines?(node, body)
+      return if exclude_operators && operator?(body.name)
+      return if exclude_setters && setter?(body.name)
+
+      call_code =
+        call_code(node, body)
+
+      return unless valid_length?(call_code)
+
+      issue_for node.name_location, node.end_location,
+        MSG % call_code
+    end
+  end
+end