New rule: RedundantWithIndex (#120)

spec/ameba/rule/lint/redundant_with_index_spec.cr

@@ -0,0 +1,163 @@
+require "../../../spec_helper"
+
+module Ameba::Rule::Lint
+  describe RedundantWithIndex do
+    subject = RedundantWithIndex.new
+
+    context "with_index" do
+      it "does not report if there is index argument" do
+        s = Source.new %(
+          collection.each.with_index do |e, i|
+            e += i
+          end
+        )
+        subject.catch(s).should be_valid
+      end
+
+      it "reports if there is not index argument" do
+        s = Source.new %(
+          collection.each.with_index do |e|
+            e += 1
+          end
+        )
+        subject.catch(s).should_not be_valid
+      end
+
+      it "reports if there is underscored index argument" do
+        s = Source.new %(
+          collection.each.with_index do |e, _|
+            e += 1
+          end
+        )
+        subject.catch(s).should_not be_valid
+      end
+
+      it "reports if there is no args" do
+        s = Source.new %(
+          collection.each.with_index do
+            puts :nothing
+          end
+        )
+        subject.catch(s).should_not be_valid
+      end
+
+      it "does not report if there is no block" do
+        s = Source.new %(
+          collection.each.with_index
+        )
+        subject.catch(s).should be_valid
+      end
+
+      it "does not report if first argument is underscored" do
+        s = Source.new %(
+          collection.each.with_index do |_, i|
+            puts i
+          end
+        )
+        subject.catch(s).should be_valid
+      end
+
+      it "does not report if there are more than 2 args" do
+        s = Source.new %(
+          tup.each.with_index do |key, value, index|
+            puts i
+          end
+        )
+        subject.catch(s).should be_valid
+      end
+
+      it "reports rule, location and message" do
+        s = Source.new %(
+          def valid?
+            collection.each.with_index do |e|
+            end
+          end
+        ), "source.cr"
+        subject.catch(s).should_not be_valid
+        issue = s.issues.first
+        issue.rule.should_not be_nil
+        issue.location.to_s.should eq "source.cr:2:19"
+        issue.end_location.to_s.should eq "source.cr:2:29"
+        issue.message.should eq "Remove redundant with_index"
+      end
+    end
+
+    context "each_with_index" do
+      it "does not report if there is index argument" do
+        s = Source.new %(
+          collection.each_with_index do |e, i|
+            e += i
+          end
+        )
+        subject.catch(s).should be_valid
+      end
+
+      it "reports if there is not index argument" do
+        s = Source.new %(
+          collection.each_with_index do |e|
+            e += 1
+          end
+        )
+        subject.catch(s).should_not be_valid
+      end
+
+      it "reports if there is underscored index argument" do
+        s = Source.new %(
+          collection.each_with_index do |e, _|
+            e += 1
+          end
+        )
+        subject.catch(s).should_not be_valid
+      end
+
+      it "reports if there is no args" do
+        s = Source.new %(
+          collection.each_with_index do
+            puts :nothing
+          end
+        )
+        subject.catch(s).should_not be_valid
+      end
+
+      it "does not report if there is no block" do
+        s = Source.new %(
+          collection.each_with_index(1)
+        )
+        subject.catch(s).should be_valid
+      end
+
+      it "does not report if first argument is underscored" do
+        s = Source.new %(
+          collection.each_with_index do |_, i|
+            puts i
+          end
+        )
+        subject.catch(s).should be_valid
+      end
+
+      it "does not report if there are more than 2 args" do
+        s = Source.new %(
+          tup.each_with_index do |key, value, index|
+            puts i
+          end
+        )
+        subject.catch(s).should be_valid
+      end
+
+      it "reports rule, location and message" do
+        s = Source.new %(
+          def valid?
+            collection.each_with_index do |e|
+            end
+          end
+        ), "source.cr"
+        subject.catch(s).should_not be_valid
+        issue = s.issues.first
+        issue.rule.should_not be_nil
+        issue.location.to_s.should eq "source.cr:2:14"
+        issue.end_location.to_s.should eq "source.cr:2:29"
+        issue.message.should eq "Use each instead of each_with_index"
+      end
+    end
+  end
+end

src/ameba/rule/lint/redundant_with_index.cr

@@ -0,0 +1,60 @@
+module Ameba::Rule::Lint
+  # A rule that disallows redundant `with_index` calls.
+  #
+  # For example, this is considered invalid:
+  # ```
+  # collection.each.with_index do |e|
+  #   # ...
+  # end
+  #
+  # collection.each_with_index do |e, _|
+  #   # ...
+  # end
+  # ```
+  #
+  # and it should be written as follows:
+  #
+  # ```
+  # collection.each do |e|
+  #   # ...
+  # end
+  # ```
+  #
+  # YAML configuration example:
+  #
+  # ```
+  # Lint/RedundantWithIndex:
+  #   Enabled: true
+  # ```
+  #
+  struct RedundantWithIndex < Base
+    properties do
+      description "Disallows redundant `with_index` calls"
+    end
+
+    def test(source)
+      AST::NodeVisitor.new self, source
+    end
+
+    def test(source, node : Crystal::Call)
+      args, block = node.args, node.block
+
+      return if args.size > 1 || block.nil? || with_index_arg?(block.not_nil!)
+
+      case node.name
+      when "with_index"
+        report source, node, "Remove redundant with_index"
+      when "each_with_index"
+        report source, node, "Use each instead of each_with_index"
+      end
+    end
+
+    private def with_index_arg?(block : Crystal::Block)
+      block.args.size >= 2 && block.args.last.name != "_"
+    end
+
+    private def report(source, node, msg)
+      issue_for node.name_location, node.name_end_location, msg
+    end
+  end
+end