@@ -38,7 +38,7 @@ module Reek def initialize(outer, exp) super @name = Name.new('block') - @parameters = exp[0] || [] + @parameters = exp[2] || [] @parameters.extend(ParameterSet) end lib/reek/block_context.rb @@ -48,10 +48,6 @@ module Reek "#{@outer.outer_name}#{@name}#" end - def to_s - "#{@outer.outer_name}#{@name}" - end - def variable_names @instance_variables end lib/reek/class_context.rb @@ -23,7 +23,7 @@ module Reek @myself = nil end - def each(type, ignoring = [], &blk) + def each(type, ignoring, &blk) if block_given? @exp.look_for(type, ignoring, &blk) else lib/reek/code_context.rb @@ -1,4 +1,3 @@ -require 'rubygems' require 'sexp' require 'reek/block_context' require 'reek/class_context' @@ -73,7 +72,12 @@ module Reek def process_iter(exp) process(exp[1]) - handle_context(BlockContext, exp[0], exp[2..-1]) + scope = BlockContext.new(@element, exp) + push(scope) do + process_default(exp[2..-1]) + check_smells(exp[0]) + end + scope end def process_block(exp) lib/reek/code_parser.rb @@ -117,10 +117,6 @@ module Reek "#{@outer.outer_name}#{@name}/" end - def to_s - "#{@outer.outer_name}#{@name}" - end - def envious_receivers return [] if @refs.self_is_max? @refs.max_keys lib/reek/method_context.rb @@ -49,8 +49,8 @@ module Reek # SMELL: Duplication # This smell is reported once for each conditional that tests the # same parameter. Which means that the same smell can recur within - # a single sniffer. Which in turn means that the sniffer can't count - # its smells without knowing which are duplicates. + # a single detector. Which in turn means that SmellDetector must + # use a Set to hold smells found. found(cond, "is controlled by argument #{SexpFormatter.format(cond.if_expr)}") end end lib/reek/smells/control_couple.rb @@ -5,11 +5,11 @@ module Reek @refs = ObjectRefs.new @myself = Object end + def method_missing(method, *args) nil end - def count_statements(num) 0 end lib/reek/stop_context.rb @@ -69,10 +69,10 @@ describe 'Reek source code' do end it 'has no structural duplication' do - ['lib'].should_not flay(15) + ['lib'].should_not flay(20) end it 'has no textual duplication' do - ['lib'].should_not simian(2) + ['lib'].should_not simian(3) end it 'has no textual duplication in the tests' do ['spec/reek'].should_not simian(8) quality/reek_source_spec.rb @@ -9,26 +9,26 @@ describe BlockContext do it "should record single parameter" do element = StopContext.new - element = BlockContext.new(element, s(s(:lasgn, :x), nil)) + element = BlockContext.new(element, s(:iter, nil, s(:lasgn, :x), nil)) element.variable_names.should == [Name.new(:x)] end it "should record single parameter within a method" do element = StopContext.new element = MethodContext.new(element, s(:defn, :help)) - element = BlockContext.new(element, s(s(:lasgn, :x), nil)) + element = BlockContext.new(element, s(:iter, nil, s(:lasgn, :x), nil)) element.variable_names.should == [Name.new(:x)] end it "records multiple parameters" do element = StopContext.new - element = BlockContext.new(element, s(s(:masgn, s(:array, s(:lasgn, :x), s(:lasgn, :y))), nil)) + element = BlockContext.new(element, s(:iter, nil, s(:masgn, s(:array, s(:lasgn, :x), s(:lasgn, :y))), nil)) element.variable_names.should == [Name.new(:x), Name.new(:y)] end it "should not pass parameters upward" do mc = MethodContext.new(StopContext.new, s(:defn, :help, s(:args))) - element = BlockContext.new(mc, s(s(:lasgn, :x))) + element = BlockContext.new(mc, s(:iter, nil, s(:lasgn, :x))) mc.variable_names.should be_empty end @@ -39,7 +39,7 @@ describe BlockContext do end it 'copes with a yield to an ivar' do - scope = BlockContext.new(StopContext.new, [s(:iasgn, :@list), s(:self)]) + scope = BlockContext.new(StopContext.new, s(:iter, nil, s(:iasgn, :@list), s(:self))) scope.record_instance_variable(:@list) scope.variable_names.should == [:@list] end spec/reek/block_context_spec.rb @@ -78,7 +78,7 @@ describe CodeContext do it 'should recognise its fq name in a collection of names' do element = StopContext.new element = ModuleContext.new(element, Name.new(:mod), s(:module, :mod, nil)) - element = ClassContext.create(element, [0, :klass]) + element = ClassContext.create(element, s(:class, :klass)) element.matches?(['banana', 'mod']).should == true element.matches?(['banana', 'mod::klass']).should == true end @@ -86,7 +86,7 @@ describe CodeContext do it 'should recognise its fq name in a collection of names' do element = StopContext.new element = ModuleContext.new(element, Name.new(:mod), s(:module, :mod, nil)) - element = ClassContext.create(element, [0, :klass]) + element = ClassContext.create(element, s(:class, :klass)) element.matches?([/banana/, /mod/]).should == true element.matches?([/banana/, /mod::klass/]).should == true end @@ -101,20 +101,20 @@ describe CodeContext do @ctx = CodeContext.new(nil, ast) end it 'yields no calls' do - @ctx.each(:call) {|exp| raise "#{exp} yielded by empty module!"} + @ctx.each(:call, []) {|exp| raise "#{exp} yielded by empty module!"} end it 'yields one module' do mods = 0 - @ctx.each(:module) {|exp| mods += 1} + @ctx.each(:module, []) {|exp| mods += 1} mods.should == 1 end it "yields the module's full AST" do - @ctx.each(:module) {|exp| exp[1].should == @module_name.to_sym} + @ctx.each(:module, []) {|exp| exp[1].should == @module_name.to_sym} end context 'with no block' do it 'returns an empty array of ifs' do - @ctx.each(:if).should be_empty + @ctx.each(:if, []).should be_empty end end end @@ -128,19 +128,19 @@ describe CodeContext do @ctx = CodeContext.new(nil, ast) end it 'yields no ifs' do - @ctx.each(:if) {|exp| raise "#{exp} yielded by empty module!"} + @ctx.each(:if, []) {|exp| raise "#{exp} yielded by empty module!"} end it 'yields one module' do - @ctx.each(:module).length.should == 1 + @ctx.each(:module, []).length.should == 1 end it "yields the module's full AST" do - @ctx.each(:module) {|exp| exp[1].should == @module_name.to_sym} + @ctx.each(:module, []) {|exp| exp[1].should == @module_name.to_sym} end it 'yields one method' do - @ctx.each(:defn).length.should == 1 + @ctx.each(:defn, []).length.should == 1 end it "yields the method's full AST" do - @ctx.each(:defn) {|exp| exp[1].should == @method_name.to_sym} + @ctx.each(:defn, []) {|exp| exp[1].should == @method_name.to_sym} end context 'pruning the traversal' do @@ -169,7 +169,7 @@ EOS ast = src.to_reek_source.syntax_tree ctx = CodeContext.new(nil, ast) - ctx.each(:if).length.should == 3 + ctx.each(:if, []).length.should == 3 end end end spec/reek/code_context_spec.rb @@ -7,7 +7,7 @@ include Reek describe MethodContext, 'matching' do before :each do - @element = MethodContext.new(StopContext.new, [0, :mod], s(:module, :mod, nil)) + @element = MethodContext.new(StopContext.new, s(0, :mod), s(:module, :mod, nil)) end it 'should recognise itself in a collection of names' do @@ -26,7 +26,7 @@ describe MethodContext, 'matching fq names' do element = StopContext.new element = ModuleContext.new(element, Name.new(:mod), s(:module, :mod, nil)) element = ClassContext.new(element, Name.new(:klass)) - @element = MethodContext.new(element, [0, :meth]) + @element = MethodContext.new(element, s(0, :meth)) end it 'should recognise itself in a collection of names' do spec/reek/method_context_spec.rb @@ -17,7 +17,7 @@ describe Attribute do [ClassContext, ModuleContext].each do |klass| context "in a #{klass}" do before :each do - @ctx = klass.create(StopContext.new, "Fred") + @ctx = klass.create(StopContext.new, s(:null, :Fred)) end it_should_behave_like 'a variable detector' spec/reek/smells/attribute_spec.rb @@ -204,7 +204,7 @@ end describe FeatureEnvy, '#examine' do before :each do - @context = MethodContext.new(StopContext.new, [:defn, :cool]) + @context = MethodContext.new(StopContext.new, s(:defn, :cool)) @fe = FeatureEnvy.new end spec/reek/smells/feature_envy_spec.rb @@ -11,7 +11,7 @@ include Reek::Smells describe LargeClass, 'when exceptions are listed' do before(:each) do - @ctx = ClassContext.create(StopContext.new, [0, :Humungous]) + @ctx = ClassContext.create(StopContext.new, s(:null, :Humungous)) 30.times { |num| @ctx.record_method("method#{num}") } @config = LargeClass.default_config end spec/reek/smells/large_class_spec.rb @@ -7,4 +7,5 @@ describe Sniffer do dirty_file = Dir['spec/samples/two_smelly_files/*.rb'][0] File.new(dirty_file).sniff.should be_smelly end + end spec/reek/sniffer_spec.rb 2424950140475dd21cbe21b8b92727de45b5a17d Kevin Rutherford kevin@rutherford-software.com http://github.com/kevinrutherford/reek/commit/d543c7a35483adefa720aad89e4e6d0debb8215e d543c7a35483adefa720aad89e4e6d0debb8215e 2009-11-02T01:43:51-08:00 2009-11-02T01:43:51-08:00 Refactored to make line numbers available to every context 74bc97ef790ec883e3ddf9b8376bff0d2816b876 Kevin Rutherford kevin@rutherford-software.com