Merge pull request #255 from mbj/fix-evaled-method-matchers

Fix evaled method matchers

config/flay.yml

@@ -1,3 +1,3 @@
 ---
 threshold: 18
-total_score: 1085
+total_score: 1098

lib/mutant/ast.rb

@@ -4,7 +4,8 @@ module Mutant
 
     # Walk all ast nodes
     #
-    # @param [Parser::AST::Node]
+    # @param [Parser::AST::Node] root
+    # @param [Array<Parser::AST::Node>] stack
     #
     # @yield [Parser::AST::Node]
     #   all nodes recursively including root
@@ -13,16 +14,19 @@ module Mutant
     #
     # @api private
     #
-    def self.walk(node, &block)
+    def self.walk(node, stack, &block)
       raise ArgumentError, 'block expected' unless block_given?
 
-      block.call(node)
+      block.call(node, stack)
       node.children.grep(Parser::AST::Node).each do |child|
-        walk(child, &block)
+        stack.push(child)
+        walk(child, stack, &block)
+        stack.pop
       end
 
       self
     end
+    private_class_method :walk
 
     # Find last node satisfying predicate (as block)
     #
@@ -39,13 +43,15 @@ module Mutant
     #
     # @api private
     #
-    def self.find_last(node, &predicate)
+    def self.find_last_path(node, &predicate)
       raise ArgumentError, 'block expected' unless block_given?
-      neddle = nil
+      path = []
-      walk(node) do |candidate|
+      walk(node, [node]) do |candidate, stack|
-        neddle = candidate if predicate.call(candidate, &predicate)
+        if predicate.call(candidate, &predicate)
+          path = stack.dup
+        end
       end
-      neddle
+      path
     end
 
   end # AST

lib/mutant/matcher/method.rb

@@ -2,8 +2,8 @@ module Mutant
   class Matcher
     # Matcher for subjects that are a specific method
     class Method < self
-      include Adamantium::Flat, Concord::Public.new(:env, :scope, :method)
+      include Adamantium::Flat, Concord::Public.new(:env, :scope, :target_method)
-      include Equalizer.new(:identification)
+      include AST::NodePredicates, Equalizer.new(:identification)
 
       # Methods within rbx kernel directory are precompiled and their source
       # cannot be accessed via reading source location. Same for methods created by eval.
@@ -40,7 +40,10 @@ module Mutant
       def skip?
         location = source_location
         if location.nil? || BLACKLIST.match(location.first)
-          env.warn(format('%s does not have valid source location unable to emit matcher', method.inspect))
+          env.warn(format('%s does not have valid source location unable to emit subject', target_method.inspect))
+          true
+        elsif matched_node_path.any?(&method(:n_block?))
+          env.warn(format('%s is defined from a 3rd party lib unable to emit subject', target_method.inspect))
           true
         else
           false
@@ -54,7 +57,7 @@ module Mutant
       # @api private
       #
       def method_name
-        method.name
+        target_method.name
       end
 
       # Return context
@@ -104,7 +107,7 @@ module Mutant
       # @api private
       #
       def source_location
-        method.source_location
+        target_method.source_location
       end
 
       # Return subject
@@ -118,27 +121,22 @@ module Mutant
       # @api private
       #
       def subject
-        node = matched_node
+        node = matched_node_path.last
         return unless node
         self.class::SUBJECT_CLASS.new(env.config, context, node)
       end
       memoize :subject
 
-      # Return matched node
+      # Return matched node path
       #
-      # @return [Parser::AST::Node]
+      # @return [Array<Parser::AST::Node>]
-      #   if node could be found
-      #
-      # @return [nil]
-      #   otherwise
       #
       # @api private
       #
-      def matched_node
+      def matched_node_path
-        AST.find_last(ast) do |node|
+        AST.find_last_path(ast, &method(:match?))
-          match?(node)
-        end
       end
+      memoize :matched_node_path
 
     end # Method
   end # Matcher

lib/mutant/matcher/method/instance.rb

@@ -15,10 +15,10 @@ module Mutant
         #
         # @api private
         #
-        def self.build(env, scope, method)
+        def self.build(env, scope, target_method)
-          name = method.name
+          name = target_method.name
           if scope.ancestors.include?(::Memoizable) && scope.memoized?(name)
-            return Memoized.new(env, scope, method)
+            return Memoized.new(env, scope, target_method)
           end
           super
         end
@@ -68,7 +68,7 @@ module Mutant
           # @api private
           #
           def source_location
-            scope.unmemoized_instance_method(method.name).source_location
+            scope.unmemoized_instance_method(method_name).source_location
           end
 
         end # Memoized

spec/shared/method_matcher_behavior.rb

@@ -17,7 +17,7 @@ RSpec.shared_examples_for 'a method matcher' do
   end
 
   it 'should have correct line number' do
-    expect(node.location.expression.line - base).to eql(method_line)
+    expect(node.location.expression.line).to eql(method_line)
   end
 
   it 'should have correct arity' do

spec/unit/mutant/ast_spec.rb

@@ -0,0 +1,38 @@
+RSpec.describe Mutant::AST do
+  let(:object) { described_class }
+
+  describe '.find_last_path' do
+    subject { object.find_last_path(root, &block) }
+
+    let(:root)    { s(:root, parent)             }
+    let(:child_a) { s(:child_a)                  }
+    let(:child_b) { s(:child_b)                  }
+    let(:parent)  { s(:parent, child_a, child_b) }
+
+    def path
+      subject.map(&:type)
+    end
+
+    context 'when no node matches' do
+      let(:block) { ->(_) { false } }
+
+      it { should eql([]) }
+    end
+
+    context 'when one node matches' do
+      let(:block) { ->(node) { node.equal?(child_a) } }
+
+      it 'returns the full path' do
+        expect(path).to eql([:root, :parent, :child_a])
+      end
+    end
+
+    context 'when two nodes match' do
+      let(:block) { ->(node) { node.equal?(child_a) || node.equal?(child_b) } }
+
+      it 'returns the last full path' do
+        expect(path).to eql([:root, :parent, :child_b])
+      end
+    end
+  end
+end

spec/unit/mutant/matcher/method/instance_spec.rb

@@ -11,10 +11,10 @@ RSpec.describe Mutant::Matcher::Method::Instance do
     let(:method)       { scope.instance_method(method_name)        }
     let(:yields)       { []                                        }
     let(:namespace)    { self.class                                }
-    let(:scope)        { self.class::Foo                           }
     let(:type)         { :def                                      }
-    let(:method_name)  { :bar                                      }
+    let(:method_name)  { :foo                                      }
     let(:method_arity) { 0                                         }
+    let(:base)         { TestApp::InstanceMethodTests              }
 
     def name
       node.children[0]
@@ -36,109 +36,79 @@ RSpec.describe Mutant::Matcher::Method::Instance do
       it 'does warn' do
         subject
         expect(reporter.warn_calls.last).to(
-          eql("#{method.inspect} does not have valid source location unable to emit matcher")
+          eql("#{method.inspect} does not have valid source location unable to emit subject")
         )
       end
     end
 
     context 'when method is defined once' do
-      let(:base) { __LINE__ }
+      let(:scope)       { base::DefinedOnce }
-      class self::Foo
+      let(:method_line) { 7                 }
-        def bar; end
-      end
-
-      let(:method_line) { 2 }
 
       it_should_behave_like 'a method matcher'
     end
 
     context 'when method is defined once with a memoizer' do
-      let(:base) { __LINE__ }
+      let(:scope)       { base::WithMemoizer }
-      class self::Foo
+      let(:method_line) { 12                 }
-        def bar; end
-        include Adamantium
-        memoize :bar
-      end
-
-      let(:method_line) { 2 }
 
       it_should_behave_like 'a method matcher'
     end
 
     context 'when method is defined multiple times' do
       context 'on different lines' do
-        let(:base) { __LINE__ }
+        let(:scope)        { base::DefinedMultipleTimes::DifferentLines }
-        class self::Foo
+        let(:method_line)  { 21                                         }
-          def bar
+        let(:method_arity) { 1                                          }
-          end
-
-          def bar(_arg)
-          end
-        end
-
-        let(:method_line)  { 5 }
-        let(:method_arity) { 1 }
 
         it_should_behave_like 'a method matcher'
       end
 
       context 'on the same line' do
-        let(:base) { __LINE__ }
+        let(:scope)        { base::DefinedMultipleTimes::SameLineSameScope }
-        class self::Foo
+        let(:method_line)  { 26                                            }
-          def bar; end; def bar(_arg); end
+        let(:method_arity) { 1                                             }
-        end
-
-        let(:method_line)  { 2 }
-        let(:method_arity) { 1 }
 
         it_should_behave_like 'a method matcher'
       end
 
       context 'on the same line with different scope' do
-        let(:base) { __LINE__ }
+        let(:scope)        { base::DefinedMultipleTimes::SameLineDifferentScope }
-        class self::Foo
+        let(:method_line)  { 30                                                 }
-          def self.bar; end; def bar(_arg); end
+        let(:method_arity) { 1                                                  }
-        end
-
-        let(:method_line)  { 2 }
-        let(:method_arity) { 1 }
 
         it_should_behave_like 'a method matcher'
       end
 
-      context 'when nested' do
+      context 'in module eval' do
-        let(:pattern) { 'Foo::Bar#baz' }
+        let(:scope) { base::InModuleEval }
 
-        context 'in class' do
+        it 'does not emit matcher' do
-          let(:base) { __LINE__ }
+          subject
-          class self::Foo
+          expect(yields.length).to be(0)
-            class Bar
-              def baz
-              end
-            end
-          end
-
-          let(:method_line) { 3                    }
-          let(:method_name) { :baz                 }
-          let(:scope)       { self.class::Foo::Bar }
-
-          it_should_behave_like 'a method matcher'
         end
 
-        context 'in module' do
+        it 'does warn' do
-          let(:base) { __LINE__ }
+          subject
-          module self::Foo
+          expect(reporter.warn_calls.last).to(
-            class Bar
+            eql("#{method.inspect} is defined from a 3rd party lib unable to emit subject")
-              def baz
+          )
-              end
+        end
-            end
+      end
-          end
 
-          let(:method_line) { 3                    }
+      context 'in class eval' do
-          let(:method_name) { :baz                 }
+        let(:scope) { base::InClassEval }
-          let(:scope)       { self.class::Foo::Bar }
 
-          it_should_behave_like 'a method matcher'
+        it 'does not emit matcher' do
+          subject
+          expect(yields.length).to be(0)
+        end
+
+        it 'does warn' do
+          subject
+          expect(reporter.warn_calls.last).to(
+            eql("#{method.inspect} is defined from a 3rd party lib unable to emit subject")
+          )
         end
       end
     end

spec/unit/mutant/matcher/method/singleton_spec.rb

@@ -6,10 +6,10 @@ RSpec.describe Mutant::Matcher::Method::Singleton, '#each' do
   let(:method)       { scope.method(method_name)               }
   let(:env)          { Fixtures::TEST_ENV                      }
   let(:yields)       { []                                      }
-  let(:namespace)    { self.class                              }
-  let(:scope)        { self.class::Foo                         }
   let(:type)         { :defs                                   }
+  let(:method_name)  { :foo                                    }
   let(:method_arity) { 0                                       }
+  let(:base)         { TestApp::SingletonMethodTests           }
 
   def name
     node.children[1]
@@ -22,14 +22,8 @@ RSpec.describe Mutant::Matcher::Method::Singleton, '#each' do
   context 'on singleton methods' do
 
     context 'when also defined on lvar' do
-      let(:base) { __LINE__ }
+      let(:scope)       { base::AlsoDefinedOnLvar }
-      class self::Foo
+      let(:method_line) { 63                      }
-        a = Object.new
-        def a.bar; end; def self.bar; end
-      end
-
-      let(:method_name) { :bar }
-      let(:method_line) { 3    }
 
       it_should_behave_like 'a method matcher'
 
@@ -42,13 +36,8 @@ RSpec.describe Mutant::Matcher::Method::Singleton, '#each' do
     end
 
     context 'when defined on self' do
-      let(:base) { __LINE__ }
+      let(:scope)       { base::DefinedOnSelf }
-      class self::Foo
+      let(:method_line) { 58                  }
-        def self.bar; end
-      end
-
-      let(:method_name) { :bar }
-      let(:method_line) { 2    }
 
       it_should_behave_like 'a method matcher'
     end
@@ -56,34 +45,15 @@ RSpec.describe Mutant::Matcher::Method::Singleton, '#each' do
     context 'when defined on constant' do
 
       context 'inside namespace' do
-        let(:base) { __LINE__ }
+        let(:scope)       { base::DefinedOnConstant::InsideNamespace }
-        module self::Namespace
+        let(:method_line) { 68                                       }
-          class Foo
-            def Foo.bar
-            end
-          end
-        end
-
-        let(:scope)       { self.class::Namespace::Foo }
-        let(:method_name) { :bar                       }
-        let(:method_line) { 3                          }
 
         it_should_behave_like 'a method matcher'
       end
 
       context 'outside namespace' do
-        let(:base) { __LINE__ }
+        let(:method_line) { 75                                        }
-        module self::Namespace
+        let(:scope)       { base::DefinedOnConstant::OutsideNamespace }
-          class Foo
-          end
-
-          def Foo.bar
-          end
-        end
-
-        let(:method_name) { :bar                       }
-        let(:method_line) { 5                          }
-        let(:scope)       { self.class::Namespace::Foo }
 
         it_should_behave_like 'a method matcher'
       end
@@ -91,21 +61,9 @@ RSpec.describe Mutant::Matcher::Method::Singleton, '#each' do
 
     context 'when defined multiple times in the same line' do
       context 'with method on different scope' do
-        let(:base) { __LINE__ }
+        let(:scope)        { base::DefinedMultipleTimes::SameLine::DifferentScope }
-        module self::Namespace
+        let(:method_line)  { 94                                                   }
-          module Foo; end
+        let(:method_arity) { 1                                                    }
-          module Bar
-            def self.baz
-            end
-            def Foo.baz(_arg)
-            end
-          end
-        end
-
-        let(:scope)        { self.class::Namespace::Bar }
-        let(:method_name)  { :baz                       }
-        let(:method_line)  { 4                          }
-        let(:method_arity) { 0                          }
 
         it_should_behave_like 'a method matcher'
       end

test_app/lib/test_app.rb

@@ -2,6 +2,100 @@
 
 # Namespace for test application
 module TestApp
+  module InstanceMethodTests
+    module DefinedOnce
+      def foo; end
+    end
+
+    class WithMemoizer
+      include Adamantium
+      def foo; end
+      memoize :foo
+    end
+
+    module DefinedMultipleTimes
+      class DifferentLines
+        def foo
+        end
+
+        def foo(_arg)
+        end
+      end
+
+      class SameLineSameScope
+        def foo; end; def foo(_arg); end
+      end
+
+      class SameLineDifferentScope
+        def self.foo; end; def foo(_arg); end
+      end
+    end
+
+    class InClassEval
+      class_eval do
+        def foo
+        end
+      end
+    end
+
+    class InModuleEval
+      module_eval do
+        def foo
+        end
+      end
+    end
+
+    class InInstanceEval
+      module_eval do
+        def foo
+        end
+      end
+    end
+  end
+
+  module SingletonMethodTests
+    module DefinedOnSelf
+      def self.foo; end
+    end
+
+    module AlsoDefinedOnLvar
+      a = Object.new
+      def a.foo; end; def self.foo; end
+    end
+
+    module DefinedOnConstant
+      module InsideNamespace
+        def InsideNamespace.foo
+        end
+      end
+
+      module OutsideNamespace
+      end
+
+      def OutsideNamespace.foo
+      end
+    end
+
+    module DefinedMultipleTimes
+      module DifferentLines
+        def self.foo
+        end
+
+        def self.foo(_arg)
+        end
+      end
+
+      module SameLine
+        module SameScope
+          def self.foo; end; def self.foo(_arg); end;
+        end
+
+        module DifferentScope
+          def self.foo; end; def DifferentScope.foo(_arg); end
+        end
+      end
+    end
+  end
 end
 
 require 'test_app/literal'