From 9e2d4f1856103a03f6c23bf410490b5f1919a5ed Mon Sep 17 00:00:00 2001 From: Vitalii Elenhaupt Date: Thu, 15 Apr 2021 17:26:49 +0300 Subject: [PATCH] Disable performance rules for spec files closes #220 --- spec/ameba/base_spec.cr | 11 +++++++++++ .../rule/performance/any_after_filter_spec.cr | 7 +++++++ .../performance/any_instead_of_empty_spec.cr | 7 +++++++ spec/ameba/rule/performance/base_spec.cr | 19 +++++++++++++++++++ .../chained_call_with_no_bang_spec.cr | 7 +++++++ .../performance/compact_after_map_spec.cr | 7 +++++++ .../first_last_after_filter_spec.cr | 7 +++++++ .../performance/flatten_after_map_spec.cr | 7 +++++++ .../performance/map_instead_of_block_spec.cr | 7 +++++++ .../performance/size_after_filter_spec.cr | 7 +++++++ spec/ameba/runner_spec.cr | 1 + spec/spec_helper.cr | 10 ++++++++++ src/ameba/rule/base.cr | 12 +++++++++++- .../rule/performance/any_after_filter.cr | 2 ++ .../rule/performance/any_instead_of_empty.cr | 2 ++ src/ameba/rule/performance/base.cr | 10 ++++++++++ .../performance/chained_call_with_no_bang.cr | 2 ++ .../rule/performance/compact_after_map.cr | 2 ++ .../performance/first_last_after_filter.cr | 2 ++ .../rule/performance/flatten_after_map.cr | 2 ++ .../rule/performance/map_instead_of_block.cr | 2 ++ .../rule/performance/size_after_filter.cr | 2 ++ 22 files changed, 134 insertions(+), 1 deletion(-) create mode 100644 spec/ameba/rule/performance/base_spec.cr create mode 100644 src/ameba/rule/performance/base.cr diff --git a/spec/ameba/base_spec.cr b/spec/ameba/base_spec.cr index 21970320..508091b5 100644 --- a/spec/ameba/base_spec.cr +++ b/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 diff --git a/spec/ameba/rule/performance/any_after_filter_spec.cr b/spec/ameba/rule/performance/any_after_filter_spec.cr index d78722c2..3c29740a 100644 --- a/spec/ameba/rule/performance/any_after_filter_spec.cr +++ b/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? diff --git a/spec/ameba/rule/performance/any_instead_of_empty_spec.cr b/spec/ameba/rule/performance/any_instead_of_empty_spec.cr index 2a5f8227..8606e309 100644 --- a/spec/ameba/rule/performance/any_instead_of_empty_spec.cr +++ b/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 %( diff --git a/spec/ameba/rule/performance/base_spec.cr b/spec/ameba/rule/performance/base_spec.cr new file mode 100644 index 00000000..7c9aa08b --- /dev/null +++ b/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 diff --git a/spec/ameba/rule/performance/chained_call_with_no_bang_spec.cr b/spec/ameba/rule/performance/chained_call_with_no_bang_spec.cr index 32504ce5..e5a31162 100644 --- a/spec/ameba/rule/performance/chained_call_with_no_bang_spec.cr +++ b/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 diff --git a/spec/ameba/rule/performance/compact_after_map_spec.cr b/spec/ameba/rule/performance/compact_after_map_spec.cr index ad34c79d..3d3a7625 100644 --- a/spec/ameba/rule/performance/compact_after_map_spec.cr +++ b/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 %( diff --git a/spec/ameba/rule/performance/first_last_after_filter_spec.cr b/spec/ameba/rule/performance/first_last_after_filter_spec.cr index d3320ccb..a4698c69 100644 --- a/spec/ameba/rule/performance/first_last_after_filter_spec.cr +++ b/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? diff --git a/spec/ameba/rule/performance/flatten_after_map_spec.cr b/spec/ameba/rule/performance/flatten_after_map_spec.cr index 71666c31..14d63814 100644 --- a/spec/ameba/rule/performance/flatten_after_map_spec.cr +++ b/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 %( diff --git a/spec/ameba/rule/performance/map_instead_of_block_spec.cr b/spec/ameba/rule/performance/map_instead_of_block_spec.cr index b3a14632..d51f8556 100644 --- a/spec/ameba/rule/performance/map_instead_of_block_spec.cr +++ b/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) diff --git a/spec/ameba/rule/performance/size_after_filter_spec.cr b/spec/ameba/rule/performance/size_after_filter_spec.cr index ccf958d8..a467d551 100644 --- a/spec/ameba/rule/performance/size_after_filter_spec.cr +++ b/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 diff --git a/spec/ameba/runner_spec.cr b/spec/ameba/runner_spec.cr index 16957e68..703f39ee 100644 --- a/spec/ameba/runner_spec.cr +++ b/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 diff --git a/spec/spec_helper.cr b/spec/spec_helper.cr index 3f106efb..e666aca3 100644 --- a/spec/spec_helper.cr +++ b/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)? diff --git a/src/ameba/rule/base.cr b/src/ameba/rule/base.cr index 414c469a..7777167c 100644 --- a/src/ameba/rule/base.cr +++ b/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 diff --git a/src/ameba/rule/performance/any_after_filter.cr b/src/ameba/rule/performance/any_after_filter.cr index bdc9c1e7..1bc57f4f 100644 --- a/src/ameba/rule/performance/any_after_filter.cr +++ b/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. # diff --git a/src/ameba/rule/performance/any_instead_of_empty.cr b/src/ameba/rule/performance/any_instead_of_empty.cr index 6dd7a2ba..4ccd8633 100644 --- a/src/ameba/rule/performance/any_instead_of_empty.cr +++ b/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. # diff --git a/src/ameba/rule/performance/base.cr b/src/ameba/rule/performance/base.cr new file mode 100644 index 00000000..405cf97a --- /dev/null +++ b/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 diff --git a/src/ameba/rule/performance/chained_call_with_no_bang.cr b/src/ameba/rule/performance/chained_call_with_no_bang.cr index 22a57c78..954200c4 100644 --- a/src/ameba/rule/performance/chained_call_with_no_bang.cr +++ b/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. diff --git a/src/ameba/rule/performance/compact_after_map.cr b/src/ameba/rule/performance/compact_after_map.cr index 63184498..4211ae30 100644 --- a/src/ameba/rule/performance/compact_after_map.cr +++ b/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`. # diff --git a/src/ameba/rule/performance/first_last_after_filter.cr b/src/ameba/rule/performance/first_last_after_filter.cr index f3368027..4b264b4f 100644 --- a/src/ameba/rule/performance/first_last_after_filter.cr +++ b/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. # diff --git a/src/ameba/rule/performance/flatten_after_map.cr b/src/ameba/rule/performance/flatten_after_map.cr index 18ee2f8a..6aa7f951 100644 --- a/src/ameba/rule/performance/flatten_after_map.cr +++ b/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`. # diff --git a/src/ameba/rule/performance/map_instead_of_block.cr b/src/ameba/rule/performance/map_instead_of_block.cr index 9ee872d5..c9c77a6e 100644 --- a/src/ameba/rule/performance/map_instead_of_block.cr +++ b/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`. diff --git a/src/ameba/rule/performance/size_after_filter.cr b/src/ameba/rule/performance/size_after_filter.cr index f7a2fbbb..b1a3e044 100644 --- a/src/ameba/rule/performance/size_after_filter.cr +++ b/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. #