From e8a86158694775caf5c9d402b65a7e39d6e3fe0a Mon Sep 17 00:00:00 2001 From: Juan Vasquez Date: Mon, 16 Oct 2023 16:31:31 -0600 Subject: [PATCH] Add max_allowed calls to FeatureEnvy smell detector This is adding a configuration `max_calls` key for the FeatureEnvy smell detector Fixes #1535 --- docs/Feature-Envy.md | 47 ++++++++++++++++++- docs/defaults.reek.yml | 1 + lib/reek/configuration/schema.yml | 4 +- lib/reek/smell_detectors/feature_envy.rb | 17 ++++++- .../reek/smell_detectors/feature_envy_spec.rb | 44 +++++++++++++++++ 5 files changed, 110 insertions(+), 3 deletions(-) diff --git a/docs/Feature-Envy.md b/docs/Feature-Envy.md index 625497648..9125e681d 100644 --- a/docs/Feature-Envy.md +++ b/docs/Feature-Envy.md @@ -61,7 +61,7 @@ Reek cannot reliably detect that each call's receiver is a different arg and wil ``` [4, 5, 6]:FeatureEnvy: Foo#initialize refers to 'arg' more than self (maybe move it to another class?) ``` - + If you're running into this problem you can disable this smell detector for this method either via configuration: @@ -91,3 +91,48 @@ _Feature Envy_ is only triggered if there are some references to self and _[Util ## Configuration _Feature Envy_ supports the [Basic Smell Options](Basic-Smell-Options.md). + +Option | Value | Effect +-------|-------|------- +`max_calls` | integer | The maximum number of duplicate calls allowed within a method. Defaults to 2. + + +## Example configuration + +### Adjusting `max_calls` + +Imagine code like this: + +```ruby +class Alfa + def bravo(charlie) + (charlie.delta - charlie.echo) * foxtrot + end +end +``` + +This would report: + +>> +src.rb -- 1 warnings: + [3, 3]:FeatureEnvy: Alfa#bravo refers to 'charlie' more than self (maybe move it to another class?) + +If you want to allow those double calls here you can disable it in 2 different ways: + +1.) Via source code comment: + +```ruby +class Alfa + # :reek:FeatureEnvy { max_calls: 3 } + def bravo(charlie) + (charlie.delta - charlie.echo) * foxtrot + end +end +``` + +2.) Via configuration file: + +```yaml +FeatureEnvy: + max_calls: 3 +``` diff --git a/docs/defaults.reek.yml b/docs/defaults.reek.yml index 7cb7d6168..0e968168f 100644 --- a/docs/defaults.reek.yml +++ b/docs/defaults.reek.yml @@ -25,6 +25,7 @@ detectors: FeatureEnvy: enabled: true exclude: [] + max_calls: 2 InstanceVariableAssumption: enabled: true exclude: [] diff --git a/lib/reek/configuration/schema.yml b/lib/reek/configuration/schema.yml index 9b28da121..9b13fee42 100644 --- a/lib/reek/configuration/schema.yml +++ b/lib/reek/configuration/schema.yml @@ -47,6 +47,8 @@ mapping: type: map mapping: <<: *detector_base + max_calls: + type: number InstanceVariableAssumption: type: map mapping: @@ -203,7 +205,7 @@ mapping: =: type: map mapping: *all_detectors - + "exclude_paths": type: seq sequence: diff --git a/lib/reek/smell_detectors/feature_envy.rb b/lib/reek/smell_detectors/feature_envy.rb index 005b9ffc7..244b3624b 100644 --- a/lib/reek/smell_detectors/feature_envy.rb +++ b/lib/reek/smell_detectors/feature_envy.rb @@ -36,6 +36,15 @@ module SmellDetectors # # See {file:docs/Feature-Envy.md} for details. class FeatureEnvy < BaseDetector + # The name of the config field that sets the maximum number of + # identical calls to be permitted within any single method. + MAX_ALLOWED_CALLS_KEY = 'max_calls' + DEFAULT_MAX_CALLS = 2 + + def self.default_config + super.merge(MAX_ALLOWED_CALLS_KEY => DEFAULT_MAX_CALLS) + end + # # Checks whether the given +context+ includes any code fragment that # might "belong" on another class. @@ -46,7 +55,9 @@ def sniff return [] if context.singleton_method? || context.module_function? return [] unless context.references_self? - envious_receivers.map do |name, lines| + envious_receivers.select do |_key, lines| + lines.length >= max_allowed_calls + end.map do |name, lines| smell_warning( lines: lines, message: "refers to '#{name}' more than self (maybe move it to another class?)", @@ -65,6 +76,10 @@ def envious_receivers refs.most_popular end + + def max_allowed_calls + @max_allowed_calls ||= value(MAX_ALLOWED_CALLS_KEY, context) + end end end end diff --git a/spec/reek/smell_detectors/feature_envy_spec.rb b/spec/reek/smell_detectors/feature_envy_spec.rb index 050dd7286..be9256f2c 100644 --- a/spec/reek/smell_detectors/feature_envy_spec.rb +++ b/spec/reek/smell_detectors/feature_envy_spec.rb @@ -292,4 +292,48 @@ def bravo(charlie) expect(src).not_to reek_of(:FeatureEnvy) end + + it 'does not reports if supressed with a preceding code comment' do + src = <<-RUBY + class Alfa + # :reek:FeatureEnvy + def bravo(charlie) + (charlie.delta - charlie.echo) * foxtrot + end + end + RUBY + + expect(src).not_to reek_of(:FeatureEnvy) + end + + it 'does not reports when increasing max_calls value with a comment' do + src = <<-RUBY + class Alfa + # :reek:FeatureEnvy { max_calls: 3 } + def bravo(charlie) + (charlie.delta - charlie.echo) * foxtrot + end + end + RUBY + + expect(src).not_to reek_of(:FeatureEnvy) + end + + context 'when allowing up to 3 calls' do + let(:config) do + { Reek::SmellDetectors::FeatureEnvy::MAX_ALLOWED_CALLS_KEY => 3 } + end + + it 'does not report double calls' do + src = <<-RUBY + class Alfa + def bravo(charlie) + (charlie.delta - charlie.echo) * foxtrot + end + end + RUBY + + expect(src).not_to reek_of(:FeatureEnvy).with_config(config) + end + end end