From bfac03f29d9beaac12271059bf4a0690d9587a9c Mon Sep 17 00:00:00 2001 From: Markus Schirp Date: Tue, 16 Sep 2014 18:39:27 +0000 Subject: [PATCH 1/2] Improve internals of method matcher * Fix name clash with Kernel#method * Match full paths of node enabling next changes to fix exclusion of evaled methods from direct matches --- config/flay.yml | 2 +- lib/mutant/ast.rb | 24 ++++++++++------- lib/mutant/matcher/method.rb | 25 +++++++----------- lib/mutant/matcher/method/instance.rb | 8 +++--- spec/unit/mutant/ast_spec.rb | 38 +++++++++++++++++++++++++++ 5 files changed, 68 insertions(+), 29 deletions(-) create mode 100644 spec/unit/mutant/ast_spec.rb diff --git a/config/flay.yml b/config/flay.yml index faf9491b..dda9d6d8 100644 --- a/config/flay.yml +++ b/config/flay.yml @@ -1,3 +1,3 @@ --- threshold: 18 -total_score: 1085 +total_score: 1094 diff --git a/lib/mutant/ast.rb b/lib/mutant/ast.rb index d8dccb2c..5936dec5 100644 --- a/lib/mutant/ast.rb +++ b/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] 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 - walk(node) do |candidate| - neddle = candidate if predicate.call(candidate, &predicate) + path = [] + walk(node, [node]) do |candidate, stack| + if predicate.call(candidate, &predicate) + path = stack.dup + end end - neddle + path end end # AST diff --git a/lib/mutant/matcher/method.rb b/lib/mutant/matcher/method.rb index 0ae04da8..d3117770 100644 --- a/lib/mutant/matcher/method.rb +++ b/lib/mutant/matcher/method.rb @@ -2,7 +2,7 @@ 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) # Methods within rbx kernel directory are precompiled and their source @@ -40,7 +40,7 @@ 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 matcher', target_method.inspect)) true else false @@ -54,7 +54,7 @@ module Mutant # @api private # def method_name - method.name + target_method.name end # Return context @@ -104,7 +104,7 @@ module Mutant # @api private # def source_location - method.source_location + target_method.source_location end # Return subject @@ -118,27 +118,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] - # if node could be found - # - # @return [nil] - # otherwise + # @return [Array] # # @api private # - def matched_node - AST.find_last(ast) do |node| - match?(node) - end + def matched_node_path + AST.find_last_path(ast, &method(:match?)) end + memoize :matched_node_path end # Method end # Matcher diff --git a/lib/mutant/matcher/method/instance.rb b/lib/mutant/matcher/method/instance.rb index 398ae288..fc2801b7 100644 --- a/lib/mutant/matcher/method/instance.rb +++ b/lib/mutant/matcher/method/instance.rb @@ -15,10 +15,10 @@ module Mutant # # @api private # - def self.build(env, scope, method) - name = method.name + def self.build(env, scope, target_method) + 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 diff --git a/spec/unit/mutant/ast_spec.rb b/spec/unit/mutant/ast_spec.rb new file mode 100644 index 00000000..713e8c99 --- /dev/null +++ b/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 From a2de0442f626e2ae5f1f3bb9b819965d6fa101a2 Mon Sep 17 00:00:00 2001 From: Markus Schirp Date: Tue, 16 Sep 2014 19:30:45 +0000 Subject: [PATCH 2/2] Skip method subjects defined in blocks * No easy way to insert mutants on these via current monkey patching loader * The blocks are typically part of a method building library when those libraries are mutated the def/defs nodes will be part of an mutated block that is included in a subject. [fix #254] --- config/flay.yml | 2 +- lib/mutant/matcher/method.rb | 7 +- spec/shared/method_matcher_behavior.rb | 2 +- .../mutant/matcher/method/instance_spec.rb | 110 +++++++----------- .../mutant/matcher/method/singleton_spec.rb | 68 +++-------- test_app/lib/test_app.rb | 94 +++++++++++++++ 6 files changed, 154 insertions(+), 129 deletions(-) diff --git a/config/flay.yml b/config/flay.yml index dda9d6d8..37c68199 100644 --- a/config/flay.yml +++ b/config/flay.yml @@ -1,3 +1,3 @@ --- threshold: 18 -total_score: 1094 +total_score: 1098 diff --git a/lib/mutant/matcher/method.rb b/lib/mutant/matcher/method.rb index d3117770..7007396c 100644 --- a/lib/mutant/matcher/method.rb +++ b/lib/mutant/matcher/method.rb @@ -3,7 +3,7 @@ module Mutant # Matcher for subjects that are a specific method class Method < self 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', target_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 diff --git a/spec/shared/method_matcher_behavior.rb b/spec/shared/method_matcher_behavior.rb index 1a76919d..c1cd7d65 100644 --- a/spec/shared/method_matcher_behavior.rb +++ b/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 diff --git a/spec/unit/mutant/matcher/method/instance_spec.rb b/spec/unit/mutant/matcher/method/instance_spec.rb index c26e2434..9269538e 100644 --- a/spec/unit/mutant/matcher/method/instance_spec.rb +++ b/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__ } - class self::Foo - def bar; end - end - - let(:method_line) { 2 } + let(:scope) { base::DefinedOnce } + let(:method_line) { 7 } it_should_behave_like 'a method matcher' end context 'when method is defined once with a memoizer' do - let(:base) { __LINE__ } - class self::Foo - def bar; end - include Adamantium - memoize :bar - end - - let(:method_line) { 2 } + let(:scope) { base::WithMemoizer } + let(:method_line) { 12 } it_should_behave_like 'a method matcher' end context 'when method is defined multiple times' do context 'on different lines' do - let(:base) { __LINE__ } - class self::Foo - def bar - end - - def bar(_arg) - end - end - - let(:method_line) { 5 } - let(:method_arity) { 1 } + let(:scope) { base::DefinedMultipleTimes::DifferentLines } + let(:method_line) { 21 } + let(:method_arity) { 1 } it_should_behave_like 'a method matcher' end context 'on the same line' do - let(:base) { __LINE__ } - class self::Foo - def bar; end; def bar(_arg); end - end - - let(:method_line) { 2 } - let(:method_arity) { 1 } + let(:scope) { base::DefinedMultipleTimes::SameLineSameScope } + let(:method_line) { 26 } + let(:method_arity) { 1 } it_should_behave_like 'a method matcher' end context 'on the same line with different scope' do - let(:base) { __LINE__ } - class self::Foo - def self.bar; end; def bar(_arg); end - end - - let(:method_line) { 2 } - let(:method_arity) { 1 } + let(:scope) { base::DefinedMultipleTimes::SameLineDifferentScope } + let(:method_line) { 30 } + let(:method_arity) { 1 } it_should_behave_like 'a method matcher' end - context 'when nested' do - let(:pattern) { 'Foo::Bar#baz' } + context 'in module eval' do + let(:scope) { base::InModuleEval } - context 'in class' do - let(:base) { __LINE__ } - class self::Foo - 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' + it 'does not emit matcher' do + subject + expect(yields.length).to be(0) end - context 'in module' do - let(:base) { __LINE__ } - module self::Foo - class Bar - def baz - end - end - 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 - let(:method_line) { 3 } - let(:method_name) { :baz } - let(:scope) { self.class::Foo::Bar } + context 'in class eval' do + let(:scope) { base::InClassEval } - 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 diff --git a/spec/unit/mutant/matcher/method/singleton_spec.rb b/spec/unit/mutant/matcher/method/singleton_spec.rb index 64d21969..629a0453 100644 --- a/spec/unit/mutant/matcher/method/singleton_spec.rb +++ b/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__ } - class self::Foo - a = Object.new - def a.bar; end; def self.bar; end - end - - let(:method_name) { :bar } - let(:method_line) { 3 } + let(:scope) { base::AlsoDefinedOnLvar } + let(:method_line) { 63 } 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__ } - class self::Foo - def self.bar; end - end - - let(:method_name) { :bar } - let(:method_line) { 2 } + let(:scope) { base::DefinedOnSelf } + let(:method_line) { 58 } 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__ } - module self::Namespace - class Foo - def Foo.bar - end - end - end - - let(:scope) { self.class::Namespace::Foo } - let(:method_name) { :bar } - let(:method_line) { 3 } + let(:scope) { base::DefinedOnConstant::InsideNamespace } + let(:method_line) { 68 } it_should_behave_like 'a method matcher' end context 'outside namespace' do - let(:base) { __LINE__ } - module self::Namespace - class Foo - end - - def Foo.bar - end - end - - let(:method_name) { :bar } - let(:method_line) { 5 } - let(:scope) { self.class::Namespace::Foo } + let(:method_line) { 75 } + let(:scope) { base::DefinedOnConstant::OutsideNamespace } 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__ } - module self::Namespace - module Foo; end - 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 } + let(:scope) { base::DefinedMultipleTimes::SameLine::DifferentScope } + let(:method_line) { 94 } + let(:method_arity) { 1 } it_should_behave_like 'a method matcher' end diff --git a/test_app/lib/test_app.rb b/test_app/lib/test_app.rb index 0df5bbc0..4d644757 100644 --- a/test_app/lib/test_app.rb +++ b/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'