Merge pull request #229 from crystal-ameba/fix/performance-in-specs

Disable performance rules for spec files

spec/ameba/base_spec.cr

@@ -8,6 +8,17 @@ module Ameba::Rule
         rules.should_not be_nil
         rules.should contain DummyRule
       end
+
+      it "contains rules across all the available groups" do
+        Rule.rules.map(&.group_name).uniq!.reject!(&.empty?).sort.should eq %w(
+          Ameba
+          Layout
+          Lint
+          Metrics
+          Performance
+          Style
+        )
+      end
     end
 
     context "properties" do

spec/ameba/rule/performance/any_after_filter_spec.cr

@@ -22,6 +22,13 @@ module Ameba::Rule::Performance
       subject.catch(source).should_not be_valid
     end
 
+    it "does not report if source is a spec" do
+      source = Source.new %(
+        [1, 2, 3].select { |e| e > 2 }.any?
+      ), "source_spec.cr"
+      subject.catch(source).should be_valid
+    end
+
     it "reports if there is reject followed by any? without a block" do
       source = Source.new %(
         [1, 2, 3].reject { |e| e > 2 }.any?

spec/ameba/rule/performance/any_instead_of_empty_spec.cr

@@ -21,6 +21,13 @@ module Ameba::Rule::Performance
       subject.catch(source).should_not be_valid
     end
 
+    it "does not report if source is a spec" do
+      source = Source.new %(
+        [1, 2, 3].any?
+      ), "source_spec.cr"
+      subject.catch(source).should be_valid
+    end
+
     context "macro" do
       it "reports in macro scope" do
         source = Source.new %(

spec/ameba/rule/performance/base_spec.cr

@@ -0,0 +1,19 @@
+require "../../../spec_helper"
+
+module Ameba::Rule::Performance
+  describe Base do
+    subject = PerfRule.new
+
+    describe "#catch" do
+      it "ignores spec files" do
+        source = Source.new("", "source_spec.cr")
+        subject.catch(source).should be_valid
+      end
+
+      it "reports perf issues for non-spec files" do
+        source = Source.new("", "source.cr")
+        subject.catch(source).should_not be_valid
+      end
+    end
+  end
+end

spec/ameba/rule/performance/chained_call_with_no_bang_spec.cr

@@ -23,6 +23,13 @@ module Ameba::Rule::Performance
       subject.catch(source).should_not be_valid
     end
 
+    it "does not report if source is a spec" do
+      source = Source.new %(
+        [1, 2, 3].select { |e| e > 1 }.reverse
+      ), "source_spec.cr"
+      subject.catch(source).should be_valid
+    end
+
     it "reports if there is select followed by reverse followed by other call" do
       source = Source.new %(
         [1, 2, 3].select { |e| e > 2 }.reverse.size

spec/ameba/rule/performance/compact_after_map_spec.cr

@@ -25,6 +25,13 @@ module Ameba::Rule::Performance
       subject.catch(source).should_not be_valid
     end
 
+    it "does not report if source is a spec" do
+      source = Source.new %(
+        (1..3).map(&.itself).compact
+      ), "source_spec.cr"
+      subject.catch(source).should be_valid
+    end
+
     context "macro" do
       it "doesn't report in macro scope" do
         source = Source.new %(

spec/ameba/rule/performance/first_last_after_filter_spec.cr

@@ -22,6 +22,13 @@ module Ameba::Rule::Performance
       subject.catch(source).should_not be_valid
     end
 
+    it "does not report if source is a spec" do
+      source = Source.new %(
+        [1, 2, 3].select { |e| e > 2 }.last
+      ), "source_spec.cr"
+      subject.catch(source).should be_valid
+    end
+
     it "reports if there is select followed by last?" do
       source = Source.new %(
         [1, 2, 3].select { |e| e > 2 }.last?

spec/ameba/rule/performance/flatten_after_map_spec.cr

@@ -18,6 +18,13 @@ module Ameba::Rule::Performance
       subject.catch(source).should_not be_valid
     end
 
+    it "does not report is source is a spec" do
+      source = Source.new %(
+        %w[Alice Bob].map(&.chars).flatten
+      ), "source_spec.cr"
+      subject.catch(source).should be_valid
+    end
+
     context "macro" do
       it "doesn't report in macro scope" do
         source = Source.new %(

spec/ameba/rule/performance/map_instead_of_block_spec.cr

@@ -19,6 +19,13 @@ module Ameba::Rule::Performance
       subject.catch(source).should_not be_valid
     end
 
+    it "does not report if source is a spec" do
+      source = Source.new %(
+        (1..3).map(&.to_s).join
+      ), "source_spec.cr"
+      subject.catch(source).should be_valid
+    end
+
     it "reports if there is map followed by sum without a block (with argument)" do
       source = Source.new %(
         (1..3).map(&.to_u64).sum(0)

spec/ameba/rule/performance/size_after_filter_spec.cr

@@ -24,6 +24,13 @@ module Ameba::Rule::Performance
       subject.catch(source).should_not be_valid
     end
 
+    it "does not report if source is a spec" do
+      source = Source.new %(
+        [1, 2, 3].select { |e| e > 2 }.size
+      ), "source_spec.cr"
+      subject.catch(source).should be_valid
+    end
+
     it "reports if there is a reject followed by size" do
       source = Source.new %(
         [1, 2, 3].reject { |e| e < 2 }.size

spec/ameba/runner_spec.cr

@@ -7,6 +7,7 @@ module Ameba
     config.globs = files
 
     config.update_rule ErrorRule.rule_name, enabled: false
+    config.update_rule PerfRule.rule_name, enabled: false
 
     Runner.new(config)
   end

spec/spec_helper.cr

@@ -107,6 +107,16 @@ module Ameba
     end
   end
 
+  class PerfRule < Rule::Performance::Base
+    properties do
+      description : String = "Sample performance rule"
+    end
+
+    def test(source)
+      issue_for({1, 1}, "Poor performance")
+    end
+  end
+
   class DummyFormatter < Formatter::BaseFormatter
     property started_sources : Array(Source)?
     property finished_sources : Array(Source)?

src/ameba/rule/base.cr

@@ -128,6 +128,16 @@ module Ameba::Rule
       {{ @type.subclasses }}
     end
 
+    protected def self.abstract?
+      {{ @type.abstract? }}
+    end
+
+    protected def self.inherited_rules
+      subclasses.each_with_object([] of Base.class) do |klass, obj|
+        klass.abstract? ? obj.concat(klass.inherited_rules) : (obj << klass)
+      end
+    end
+
     macro inherited
       protected def self.path_to_source_file
         __FILE__
@@ -188,6 +198,6 @@ module Ameba::Rule
   # Ameba::Rule.rules # => [Rule1, Rule2, ....]
   # ```
   def self.rules
-    Base.subclasses
+    Base.inherited_rules
   end
 end

src/ameba/rule/performance/any_after_filter.cr

@@ -1,3 +1,5 @@
+require "./base"
+
 module Ameba::Rule::Performance
   # This rule is used to identify usage of `any?` calls that follow filters.
   #

src/ameba/rule/performance/any_instead_of_empty.cr

@@ -1,3 +1,5 @@
+require "./base"
+
 module Ameba::Rule::Performance
   # This rule is used to identify usage of arg-less `Enumerable#any?` calls.
   #

src/ameba/rule/performance/base.cr

@@ -0,0 +1,10 @@
+require "../base"
+
+module Ameba::Rule::Performance
+  # A general base class for performance rules.
+  abstract class Base < Ameba::Rule::Base
+    def catch(source : Source)
+      source.spec? ? source : super
+    end
+  end
+end

src/ameba/rule/performance/chained_call_with_no_bang.cr

@@ -1,3 +1,5 @@
+require "./base"
+
 module Ameba::Rule::Performance
   # This rule is used to identify usage of chained calls not utilizing
   # the bang method variants.

src/ameba/rule/performance/compact_after_map.cr

@@ -1,3 +1,5 @@
+require "./base"
+
 module Ameba::Rule::Performance
   # This rule is used to identify usage of `compact` calls that follow `map`.
   #

src/ameba/rule/performance/first_last_after_filter.cr

@@ -1,3 +1,5 @@
+require "./base"
+
 module Ameba::Rule::Performance
   # This rule is used to identify usage of `first/last/first?/last?` calls that follow filters.
   #

src/ameba/rule/performance/flatten_after_map.cr

@@ -1,3 +1,5 @@
+require "./base"
+
 module Ameba::Rule::Performance
   # This rule is used to identify usage of `flatten` calls that follow `map`.
   #

src/ameba/rule/performance/map_instead_of_block.cr

@@ -1,3 +1,5 @@
+require "./base"
+
 module Ameba::Rule::Performance
   # This rule is used to identify usage of `sum/product` calls
   # that follow `map`.

src/ameba/rule/performance/size_after_filter.cr

@@ -1,3 +1,5 @@
+require "./base"
+
 module Ameba::Rule::Performance
   # This rule is used to identify usage of `size` calls that follow filter.
   #