Skip to content

Commit

Permalink
Added --[no-]show-all option
Browse files Browse the repository at this point in the history
  • Loading branch information
kevinrutherford committed Jul 6, 2009
1 parent 9ace989 commit 84222cc
Show file tree
Hide file tree
Showing 11 changed files with 110 additions and 41 deletions.
8 changes: 4 additions & 4 deletions config/defaults.reek
Expand Up @@ -7,8 +7,8 @@ LargeClass:
max_instance_variables: 9
LongParameterList:
max_params: 3
exclude: []

exclude:
- initialize
enabled: true
FeatureEnvy:
exclude:
Expand Down Expand Up @@ -46,6 +46,6 @@ ControlCouple:
enabled: true
LongYieldList:
max_params: 3
exclude: []

exclude:
- initialize
enabled: true
27 changes: 27 additions & 0 deletions features/masking_smells.feature
Expand Up @@ -40,3 +40,30 @@ Feature: Masking smells using config files
Dirty#a/block/block is nested (Nested Iterators)
"""

Scenario: switch off one smell but show all in the report
When I run reek --show-all spec/samples/masked/dirty.rb
Then it fails with exit status 2
And it reports:
"""
spec/samples/masked/dirty.rb -- 3 warnings (+3 masked):
(masked) Dirty has the variable name '@s' (Uncommunicative Name)
Dirty#a calls @s.title multiple times (Duplication)
Dirty#a calls puts(@s.title) multiple times (Duplication)
(masked) Dirty#a has the name 'a' (Uncommunicative Name)
(masked) Dirty#a/block has the variable name 'x' (Uncommunicative Name)
Dirty#a/block/block is nested (Nested Iterators)
"""

Scenario: switch off one smell and show hide them in the report
When I run reek --no-show-all spec/samples/masked/dirty.rb
Then it fails with exit status 2
And it reports:
"""
spec/samples/masked/dirty.rb -- 3 warnings (+3 masked):
Dirty#a calls @s.title multiple times (Duplication)
Dirty#a calls puts(@s.title) multiple times (Duplication)
Dirty#a/block/block is nested (Nested Iterators)
"""
5 changes: 3 additions & 2 deletions features/options.feature
Expand Up @@ -31,10 +31,11 @@ Feature: Reek can be controlled using command-line options
Options:
-a, --[no-]show-all Show all smells, including those masked by config settings
-h, --help Show this message
-f, --format FORMAT Specify the format of smell warnings
-c, --context-first Sort by context; sets the format string to "%c %w (%s)"
-s, --smell-first Sort by smell; sets the format string to "[%s] %c %w"
-c, --context-first Sort by context; sets the format string to "%m%c %w (%s)"
-s, --smell-first Sort by smell; sets the format string to "%m[%s] %c %w"
-v, --version Show version
"""
3 changes: 1 addition & 2 deletions features/samples.feature
Expand Up @@ -50,7 +50,7 @@ Feature: Basic smell detection
Then it fails with exit status 2
And it reports:
"""
spec/samples/optparse.rb -- 117 warnings:
spec/samples/optparse.rb -- 116 warnings:
OptionParser has at least 59 methods (Large Class)
OptionParser#CompletingHash#match/block/block is nested (Nested Iterators)
OptionParser#Completion::complete calls candidates.size multiple times (Duplication)
Expand Down Expand Up @@ -81,7 +81,6 @@ Feature: Basic smell detection
OptionParser#Switch#RequiredArgument#parse is controlled by argument arg (Control Couple)
OptionParser#Switch#add_banner has the variable name 's' (Uncommunicative Name)
OptionParser#Switch#conv_arg calls conv multiple times (Duplication)
OptionParser#Switch#initialize has 7 parameters (Long Parameter List)
OptionParser#Switch#parse_arg calls pattern multiple times (Duplication)
OptionParser#Switch#parse_arg calls s.length multiple times (Duplication)
OptionParser#Switch#parse_arg has approx 11 statements (Long Method)
Expand Down
26 changes: 19 additions & 7 deletions lib/reek/options.rb
Expand Up @@ -5,12 +5,13 @@ module Reek

class Options

CTX_SORT = '%c %w (%s)'
SMELL_SORT = '[%s] %c %w'
CTX_SORT = '%m%c %w (%s)'
SMELL_SORT = '%m[%s] %c %w'

def self.default_options
{
:format => CTX_SORT
:format => CTX_SORT,
:show_all => false
}
end

Expand All @@ -26,7 +27,7 @@ def self.parse_args(args)
parser.parse!(args)
result
end

def self.set_options(opts, config)
opts.banner = <<EOB
Usage: #{opts.program_name} [options] files...
Expand All @@ -36,9 +37,7 @@ def self.set_options(opts, config)
EOB

opts.separator "\nOptions:"
set_help_option(opts)
set_sort_option(config, opts)
set_version_option(opts)
set_all_options(opts, config)
end

def self.parse(args)
Expand All @@ -51,6 +50,13 @@ def self.parse(args)
end

private

def self.set_all_options(opts, config)
set_show_all_option(opts, config)
set_help_option(opts)
set_sort_option(config, opts)
set_version_option(opts)
end

def self.set_version_option(opts)
opts.on("-v", "--version", "Show version") do
Expand All @@ -59,6 +65,12 @@ def self.set_version_option(opts)
end
end

def self.set_show_all_option(opts, config)
opts.on("-a", "--[no-]show-all", "Show all smells, including those masked by config settings") do |opt|
config[:show_all] = opt
end
end

def self.set_help_option(opts)
opts.on("-h", "--help", "Show this message") do
puts opts
Expand Down
36 changes: 19 additions & 17 deletions lib/reek/report.rb
Expand Up @@ -7,73 +7,75 @@ class Report
include Enumerable

def initialize(sniffer = nil) # :nodoc:
@masked_smells = SortedSet.new
@report = SortedSet.new
@masked_warnings = SortedSet.new
@warnings = SortedSet.new
sniffer.report_on(self) if sniffer
end

#
# Yields, in turn, each SmellWarning in this report.
#
def each
@report.each { |smell| yield smell }
@warnings.each { |smell| yield smell }
end

#
# Checks this report for instances of +smell_class+, and returns +true+
# only if one of them has a report string matching all of the +patterns+.
#
def has_smell?(smell_class, patterns)
@report.any? { |smell| smell.matches?(smell_class, patterns) }
@warnings.any? { |smell| smell.matches?(smell_class, patterns) }
end

def <<(smell) # :nodoc:
@report << smell
@warnings << smell
true
end

def record_masked_smell(smell)
@masked_smells << smell
@masked_warnings << smell
end

def num_masked_smells
@masked_smells.length
@masked_warnings.length
end

def empty?
@report.empty?
@warnings.empty?
end

def length
@report.length
@warnings.length
end

alias size length

def [](index) # :nodoc:
@report.to_a[index]
end

# Creates a formatted report of all the +Smells::SmellWarning+ objects recorded in
# this report, with a heading.
def full_report(desc)
result = header(desc, @report.length)
result += ":\n#{to_s}" if length > 0
result = header(desc, @warnings.length)
result += ":\n#{to_s}" if should_report
result += "\n"
result
end

def should_report
@warnings.length > 0 or (Options[:show_all] and @masked_warnings.length > 0)
end

def header(desc, num_smells)
result = "#{desc} -- #{num_smells} warning"
result += 's' unless num_smells == 1
result += " (+#{@masked_smells.length} masked)" unless @masked_smells.empty?
result += " (+#{@masked_warnings.length} masked)" unless @masked_warnings.empty?
result
end

# Creates a formatted report of all the +Smells::SmellWarning+ objects recorded in
# this report.
def to_s
@report.map {|smell| " #{smell.report}"}.join("\n")
all = SortedSet.new(@warnings)
all.merge(@masked_warnings) if Options[:show_all]
all.map {|smell| " #{smell.report}"}.join("\n")
end
end

Expand Down
16 changes: 11 additions & 5 deletions lib/reek/smell_warning.rb
Expand Up @@ -8,18 +8,19 @@ module Reek
class SmellWarning
include Comparable

def initialize(smell, context, warning)
def initialize(smell, context, warning, is_masked = false)
@smell = smell
@context = context
@warning = warning
@is_masked = is_masked
end

def hash # :nodoc:
report.hash
basic_report.hash
end

def <=>(other)
report <=> other.report
basic_report <=> other.basic_report
end

alias eql? <=> # :nodoc:
Expand All @@ -34,16 +35,21 @@ def matches?(smell_class, patterns)
return patterns.all? {|exp| exp === rpt}
end

def basic_report
Options[:format].gsub(/\%s/, @smell.smell_name).gsub(/\%c/, @context.to_s).gsub(/\%w/, @warning)
end

#
# Returns a copy of the current report format (see +Options+)
# in which the following magic tokens have been substituted:
#
# * %s <-- the name of the smell that was detected
# * %c <-- a description of the +CodeContext+ containing the smell
# * %m <-- "(is_masked) " if this is a is_masked smell
# * %s <-- the name of the smell that was detected
# * %w <-- the specific problem that was detected
#
def report
Options[:format].gsub(/\%s/, @smell.smell_name).gsub(/\%c/, @context.to_s).gsub(/\%w/, @warning)
basic_report.gsub(/\%m/, @is_masked ? '(masked) ' : '')
end
end
end
5 changes: 4 additions & 1 deletion lib/reek/smells/long_parameter_list.rb
Expand Up @@ -19,7 +19,10 @@ class LongParameterList < SmellDetector
MAX_ALLOWED_PARAMS_KEY = 'max_params'

def self.default_config
super.adopt(MAX_ALLOWED_PARAMS_KEY => 3)
super.adopt(
MAX_ALLOWED_PARAMS_KEY => 3,
EXCLUDE_KEY => ['initialize']
)
end

def initialize(config = LongParameterList.default_config)
Expand Down
2 changes: 1 addition & 1 deletion lib/reek/smells/smell_detector.rb
Expand Up @@ -90,7 +90,7 @@ def exception?(context)
end

def found(scope, warning)
smell = SmellWarning.new(self, scope, warning)
smell = SmellWarning.new(self, scope, warning, @masked)
@smells_found << smell
smell
end
Expand Down
22 changes: 21 additions & 1 deletion spec/reek/smell_warning_spec.rb
Expand Up @@ -4,21 +4,41 @@

include Reek

describe SmellWarning, ' in comparisons' do
describe SmellWarning, 'equality' do
before :each do
@first = SmellWarning.new(Smells::FeatureEnvy.new, "self", "self")
@second = SmellWarning.new(Smells::FeatureEnvy.new, "self", "self")
@masked = SmellWarning.new(Smells::FeatureEnvy.new, "self", "self", true)
end

it 'should hash equal when the smell is the same' do
@first.hash.should == @second.hash
@first.hash.should == @masked.hash
end

it 'should compare equal when the smell is the same' do
@first.should == @second
@first.should == @masked
end

it 'should compare equal when using <=>' do
(@first <=> @second).should == 0
end

it 'should compare equal to masked smell' do
(@first <=> @masked).should == 0
end
end

describe SmellWarning, 'ordering' do
before :each do
@first = SmellWarning.new(Smells::FeatureEnvy.new, "aaa", "bbb")
@second = SmellWarning.new(Smells::FeatureEnvy.new, "ccc", "ddd")
@masked = SmellWarning.new(Smells::FeatureEnvy.new, "ccc", "ddd", true)
end

it 'should ignore masking in comparisons' do
(@first <=> @second).should == (@first <=> @masked)
(@second <=> @first).should == (@masked <=> @first)
end
end
1 change: 0 additions & 1 deletion spec/slow/optparse_spec.rb
Expand Up @@ -62,7 +62,6 @@
ruby.should reek_of(:LongMethod, /OptionParser#parse_in_order/)
ruby.should reek_of(:LongParameterList, /OptionParser#List#complete/)
ruby.should reek_of(:LongParameterList, /OptionParser#List#update/)
ruby.should reek_of(:LongParameterList, /OptionParser#Switch#initialize/)
ruby.should reek_of(:LongParameterList, /OptionParser#Switch#summarize/)
ruby.should reek_of(:LongParameterList, /OptionParser#complete/)
ruby.should reek_of(:LongParameterList, /OptionParser#summarize/)
Expand Down

0 comments on commit 84222cc

Please sign in to comment.