Skip to content
New issue

Have a question about this project? Sign up for a free GitHub account to open an issue and contact its maintainers and the community.

Sign up for GitHub

By clicking “Sign up for GitHub”, you agree to our terms of service and privacy statement. We’ll occasionally send you account related emails.

Already on GitHub? Sign in to your account

Disable UnusedPrivateMethod detector by default. #847

Merged
merged 1 commit into from
Feb 6, 2016
Merged

Disable UnusedPrivateMethod detector by default. #847

Show file tree
Hide file tree
Changes from all commits
Commits
Show all changes
1 commit
Select commit
File filter

Filter by extension

Filter by extension
Conversations
Failed to load comments.
Loading
Jump to
Jump to file
Failed to load files.
Loading
Diff view
Diff view
9 changes: 9 additions & 0 deletions README.md
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -172,6 +172,15 @@ Reek currently includes checks for some aspects of
and more. See the [Code Smells](docs/Code-Smells.md)
for up to date details of exactly what Reek will check in your code.

Note that [Unused Private Method](docs/Unused-Private-Method.md) is disabled by default
because it is [kind of controversial](https://github.com/troessner/reek/issues/844) which means
you have to explicitly activate in your configuration via

```Yaml
UnusedPrivateMethod:
enabled: true
```

## Configuration

### Command-line interface
Expand Down
2 changes: 1 addition & 1 deletion defaults.reek
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -107,7 +107,7 @@ UnusedParameters:
enabled: true
exclude: []
UnusedPrivateMethod:
enabled: true
enabled: false
exclude: []
UtilityFunction:
enabled: true
Expand Down
24 changes: 2 additions & 22 deletions features/samples.feature
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -177,7 +177,7 @@ Feature: Basic smell detection
UnusedParameters: OptionParser::Completion#convert has unused parameter 'opt' [https://github.com/troessner/reek/blob/master/docs/Unused-Parameters.md]
UnusedParameters: OptionParser::Switch::NoArgument#parse has unused parameter 'argv' [https://github.com/troessner/reek/blob/master/docs/Unused-Parameters.md]
UnusedParameters: OptionParser::Switch::OptionalArgument#parse has unused parameter 'argv' [https://github.com/troessner/reek/blob/master/docs/Unused-Parameters.md]
redcloth.rb -- 121 warnings:
redcloth.rb -- 101 warnings:
Attribute: RedCloth#filter_html is a writable attribute [https://github.com/troessner/reek/blob/master/docs/Attribute.md]
Attribute: RedCloth#filter_styles is a writable attribute [https://github.com/troessner/reek/blob/master/docs/Attribute.md]
Attribute: RedCloth#hard_breaks is a writable attribute [https://github.com/troessner/reek/blob/master/docs/Attribute.md]
Expand Down Expand Up @@ -269,26 +269,6 @@ Feature: Basic smell detection
UnusedParameters: RedCloth#textile_fn_ has unused parameter 'cite' [https://github.com/troessner/reek/blob/master/docs/Unused-Parameters.md]
UnusedParameters: RedCloth#textile_fn_ has unused parameter 'tag' [https://github.com/troessner/reek/blob/master/docs/Unused-Parameters.md]
UnusedParameters: RedCloth#textile_p has unused parameter 'cite' [https://github.com/troessner/reek/blob/master/docs/Unused-Parameters.md]
UnusedPrivateMethod: RedCloth has the unused private instance method `block_markdown_atx` [https://github.com/troessner/reek/blob/master/docs/Unused-Private-Method.md]
UnusedPrivateMethod: RedCloth has the unused private instance method `block_markdown_bq` [https://github.com/troessner/reek/blob/master/docs/Unused-Private-Method.md]
UnusedPrivateMethod: RedCloth has the unused private instance method `block_markdown_lists` [https://github.com/troessner/reek/blob/master/docs/Unused-Private-Method.md]
UnusedPrivateMethod: RedCloth has the unused private instance method `block_markdown_rule` [https://github.com/troessner/reek/blob/master/docs/Unused-Private-Method.md]
UnusedPrivateMethod: RedCloth has the unused private instance method `block_markdown_setext` [https://github.com/troessner/reek/blob/master/docs/Unused-Private-Method.md]
UnusedPrivateMethod: RedCloth has the unused private instance method `block_textile_lists` [https://github.com/troessner/reek/blob/master/docs/Unused-Private-Method.md]
UnusedPrivateMethod: RedCloth has the unused private instance method `block_textile_prefix` [https://github.com/troessner/reek/blob/master/docs/Unused-Private-Method.md]
UnusedPrivateMethod: RedCloth has the unused private instance method `block_textile_table` [https://github.com/troessner/reek/blob/master/docs/Unused-Private-Method.md]
UnusedPrivateMethod: RedCloth has the unused private instance method `inline_markdown_link` [https://github.com/troessner/reek/blob/master/docs/Unused-Private-Method.md]
UnusedPrivateMethod: RedCloth has the unused private instance method `inline_markdown_reflink` [https://github.com/troessner/reek/blob/master/docs/Unused-Private-Method.md]
UnusedPrivateMethod: RedCloth has the unused private instance method `inline_textile_code` [https://github.com/troessner/reek/blob/master/docs/Unused-Private-Method.md]
UnusedPrivateMethod: RedCloth has the unused private instance method `inline_textile_image` [https://github.com/troessner/reek/blob/master/docs/Unused-Private-Method.md]
UnusedPrivateMethod: RedCloth has the unused private instance method `inline_textile_link` [https://github.com/troessner/reek/blob/master/docs/Unused-Private-Method.md]
UnusedPrivateMethod: RedCloth has the unused private instance method `inline_textile_span` [https://github.com/troessner/reek/blob/master/docs/Unused-Private-Method.md]
UnusedPrivateMethod: RedCloth has the unused private instance method `refs_markdown` [https://github.com/troessner/reek/blob/master/docs/Unused-Private-Method.md]
UnusedPrivateMethod: RedCloth has the unused private instance method `refs_textile` [https://github.com/troessner/reek/blob/master/docs/Unused-Private-Method.md]
UnusedPrivateMethod: RedCloth has the unused private instance method `textile_bq` [https://github.com/troessner/reek/blob/master/docs/Unused-Private-Method.md]
UnusedPrivateMethod: RedCloth has the unused private instance method `textile_fn_` [https://github.com/troessner/reek/blob/master/docs/Unused-Private-Method.md]
UnusedPrivateMethod: RedCloth has the unused private instance method `textile_p` [https://github.com/troessner/reek/blob/master/docs/Unused-Private-Method.md]
UnusedPrivateMethod: RedCloth has the unused private instance method `textile_popup_help` [https://github.com/troessner/reek/blob/master/docs/Unused-Private-Method.md]
UtilityFunction: RedCloth#block_markdown_rule doesn't depend on instance state (maybe move it to another class?) [https://github.com/troessner/reek/blob/master/docs/Utility-Function.md]
UtilityFunction: RedCloth#clean_html doesn't depend on instance state (maybe move it to another class?) [https://github.com/troessner/reek/blob/master/docs/Utility-Function.md]
UtilityFunction: RedCloth#flush_left doesn't depend on instance state (maybe move it to another class?) [https://github.com/troessner/reek/blob/master/docs/Utility-Function.md]
Expand All @@ -299,5 +279,5 @@ Feature: Basic smell detection
UtilityFunction: RedCloth#lT doesn't depend on instance state (maybe move it to another class?) [https://github.com/troessner/reek/blob/master/docs/Utility-Function.md]
UtilityFunction: RedCloth#no_textile doesn't depend on instance state (maybe move it to another class?) [https://github.com/troessner/reek/blob/master/docs/Utility-Function.md]
UtilityFunction: RedCloth#v_align doesn't depend on instance state (maybe move it to another class?) [https://github.com/troessner/reek/blob/master/docs/Utility-Function.md]
285 total warnings
265 total warnings
"""
4 changes: 4 additions & 0 deletions lib/reek/smells/unused_private_method.rb
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -11,6 +11,10 @@ module Smells
# See {file:docs/Unused-Private-Method.md} for details.
#
class UnusedPrivateMethod < SmellDetector
def self.default_config
super.merge(SmellConfiguration::ENABLED_KEY => false)
end

# Class for storing `hits` which are unused private methods
# we found in the given context. `name` and `line` are then used to
# construct SmellWarnings.
Expand Down
14 changes: 10 additions & 4 deletions spec/reek/smells/unused_private_method_spec.rb
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -4,6 +4,12 @@
require_relative 'smell_detector_shared'

RSpec.describe Reek::Smells::UnusedPrivateMethod do
let(:configuration) do
Copy link
Owner Author

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Due to the way we load SmellDetectors and Examiners something like:

  before do
    allow(described_class).
      to receive(:default_config).
      and_return(described_class.superclass.default_config)
  end

or

  before do
    described_class.instance_eval { def default_config; super; end }
  end

doesn't work. But given that we don't do this every other week I'd consider this good enough given the task at hand.

test_configuration_for(
described_class =>
{ Reek::Smells::SmellConfiguration::ENABLED_KEY => true }
)
end
let(:detector) { build(:smell_detector, smell_type: :UnusedPrivateMethod) }

it_should_behave_like 'SmellDetector'
Expand All @@ -18,8 +24,8 @@ def drive; end
end
EOF

expect(source).to reek_of(:UnusedPrivateMethod, name: :start)
expect(source).to reek_of(:UnusedPrivateMethod, name: :drive)
expect(source).to reek_of(:UnusedPrivateMethod, { name: :start }, configuration)
expect(source).to reek_of(:UnusedPrivateMethod, { name: :drive }, configuration)
end

it 'creates warnings correctly' do
Expand All @@ -31,7 +37,7 @@ def drive; end
end
EOF

examiner = Reek::Examiner.new(source, 'UnusedPrivateMethod')
examiner = Reek::Examiner.new(source, 'UnusedPrivateMethod', configuration: configuration)

first_warning = examiner.smells.first
expect(first_warning.smell_category).to eq(Reek::Smells::UnusedPrivateMethod.smell_category)
Expand All @@ -58,7 +64,7 @@ def drive; end
end
EOF

examiner = Reek::Examiner.new(source, 'UnusedPrivateMethod')
examiner = Reek::Examiner.new(source, 'UnusedPrivateMethod', configuration: configuration)

expect(examiner.smells.size).to eq(1)
warning_for_drive = examiner.smells.first
Expand Down