diff --git a/Changelog.md b/Changelog.md index e74baceac..976c0a958 100644 --- a/Changelog.md +++ b/Changelog.md @@ -1,5 +1,11 @@ # v0.11.5 unreleased +* [#1314](https://github.com/mbj/mutant/pull/1314) + + Fix visibility of mutated methods to retain original value. + + Fix: [#1242] + * [#1311](https://github.com/mbj/mutant/pull/1311) Change to fully enforced license. diff --git a/lib/mutant/matcher/method.rb b/lib/mutant/matcher/method.rb index f5cac90d3..e7d48f81c 100644 --- a/lib/mutant/matcher/method.rb +++ b/lib/mutant/matcher/method.rb @@ -99,16 +99,40 @@ def subject node = matched_node_path.last || return self.class::SUBJECT_CLASS.new( - context: context, - node: node + context: context, + node: node, + visibility: visibility ) end - memoize :subject def matched_node_path AST.find_last_path(ast, &method(:match?)) end memoize :matched_node_path + + def visibility + # This can be cleaned up once we are on >ruby-3.0 + # Method#{public,private,protected}? exists there. + # + # On Ruby 3.1 this can just be: + # + # if target_method.private? + # :private + # elsif target_method.protected? + # :protected + # else + # :public + # end + # + # Change to this once 3.0 is EOL. + if scope.private_methods.include?(method_name) + :private + elsif scope.protected_methods.include?(method_name) + :protected + else + :public + end + end end # Evaluator private_constant(*constants(false)) diff --git a/lib/mutant/matcher/method/instance.rb b/lib/mutant/matcher/method/instance.rb index 798b03bf8..b56c152ab 100644 --- a/lib/mutant/matcher/method/instance.rb +++ b/lib/mutant/matcher/method/instance.rb @@ -41,6 +41,16 @@ def match?(node) node.children.fetch(NAME_INDEX).equal?(method_name) end + def visibility + if scope.private_instance_methods.include?(method_name) + :private + elsif scope.protected_instance_methods.include?(method_name) + :protected + else + :public + end + end + # Evaluator specialized for memoized instance methods class Memoized < self SUBJECT_CLASS = Subject::Method::Instance::Memoized diff --git a/lib/mutant/mutation.rb b/lib/mutant/mutation.rb index 0c6f8fda2..608c25385 100644 --- a/lib/mutant/mutation.rb +++ b/lib/mutant/mutation.rb @@ -69,7 +69,10 @@ def insert(kernel) kernel: kernel, source: monkeypatch, subject: subject - ) + ).fmap do + subject.post_insert + nil + end end # Rendered mutation diff diff --git a/lib/mutant/subject.rb b/lib/mutant/subject.rb index 35817fbfd..8bbe79cb4 100644 --- a/lib/mutant/subject.rb +++ b/lib/mutant/subject.rb @@ -33,6 +33,13 @@ def prepare self end + # Perform post insert cleanup + # + # @return [self] + def post_insert + self + end + # Source line range # # @return [Range] diff --git a/lib/mutant/subject/method.rb b/lib/mutant/subject/method.rb index afffebb00..3d150d29b 100644 --- a/lib/mutant/subject/method.rb +++ b/lib/mutant/subject/method.rb @@ -4,6 +4,7 @@ module Mutant class Subject # Abstract base class for method subjects class Method < self + include anima.add(:visibility) # Method name # diff --git a/lib/mutant/subject/method/instance.rb b/lib/mutant/subject/method/instance.rb index b8138c055..528287519 100644 --- a/lib/mutant/subject/method/instance.rb +++ b/lib/mutant/subject/method/instance.rb @@ -17,6 +17,11 @@ def prepare self end + def post_insert + scope.__send__(visibility, name) + self + end + # Mutator for memoizable memoized instance methods class Memoized < self include AST::Sexp diff --git a/lib/mutant/subject/method/singleton.rb b/lib/mutant/subject/method/singleton.rb index 75ed29c20..353180ba5 100644 --- a/lib/mutant/subject/method/singleton.rb +++ b/lib/mutant/subject/method/singleton.rb @@ -17,6 +17,11 @@ def prepare self end + def post_insert + scope.singleton_class.__send__(visibility, name) + self + end + end # Singleton end # Method end # Subject diff --git a/spec/unit/mutant/cli_spec.rb b/spec/unit/mutant/cli_spec.rb index d9ee06cc1..7f0aec22a 100644 --- a/spec/unit/mutant/cli_spec.rb +++ b/spec/unit/mutant/cli_spec.rb @@ -496,11 +496,12 @@ def self.main_body let(:subject_a) do Mutant::Subject::Method::Instance.new( - context: Mutant::Context.new( + context: Mutant::Context.new( Object, 'subject.rb' ), - node: s(:def, :send, s(:args), nil) + node: s(:def, :send, s(:args), nil), + visibility: :public ) end diff --git a/spec/unit/mutant/matcher/descendants_spec.rb b/spec/unit/mutant/matcher/descendants_spec.rb index 39977f9b1..4ac5447b3 100644 --- a/spec/unit/mutant/matcher/descendants_spec.rb +++ b/spec/unit/mutant/matcher/descendants_spec.rb @@ -13,11 +13,12 @@ def apply let(:expected_subjects) do [ Mutant::Subject::Method::Instance.new( - context: Mutant::Context.new( + context: Mutant::Context.new( TestApp::Foo::Bar::Baz, TestApp::ROOT.join('lib/test_app.rb') ), - node: s(:def, :foo, s(:args), nil) + node: s(:def, :foo, s(:args), nil), + visibility: :public ) ] end diff --git a/spec/unit/mutant/matcher/method/instance_spec.rb b/spec/unit/mutant/matcher/method/instance_spec.rb index 2537150b1..86157cf1c 100644 --- a/spec/unit/mutant/matcher/method/instance_spec.rb +++ b/spec/unit/mutant/matcher/method/instance_spec.rb @@ -135,20 +135,33 @@ def arguments it_should_behave_like 'a method matcher' - it 'returns expected subjects' do - context = Mutant::Context.new( + let(:context) do + Mutant::Context.new( TestApp::InstanceMethodTests::WithMemoizer, MutantSpec::ROOT.join('test_app', 'lib', 'test_app.rb') ) + end - expect(subject).to eql( - [ - Mutant::Subject::Method::Instance.new( - context: context, - node: s(:def, :bar, s(:args), nil) - ) - ] - ) + let(:expected_subjects) do + [ + Mutant::Subject::Method::Instance.new( + context: context, + node: s(:def, :bar, s(:args), nil), + visibility: expected_visibility + ) + ] + end + + %i[public protected private].each do |visibility| + context 'with %s visibility' % visibility do + let(:expected_visibility) { visibility } + + before { context.scope.__send__(visibility, method_name) } + + it 'returns expected subjects' do + expect(subject).to eql(expected_subjects) + end + end end end diff --git a/spec/unit/mutant/matcher/method/singleton_spec.rb b/spec/unit/mutant/matcher/method/singleton_spec.rb index 15d019333..869804c7d 100644 --- a/spec/unit/mutant/matcher/method/singleton_spec.rb +++ b/spec/unit/mutant/matcher/method/singleton_spec.rb @@ -4,7 +4,7 @@ subject { object.call(env) } let(:object) { described_class.new(scope, method) } - let(:method) { scope.public_method(method_name) } + let(:method) { scope.method(method_name) } let(:type) { :defs } let(:method_name) { :foo } let(:method_arity) { 0 } @@ -50,7 +50,21 @@ def arguments let(:scope) { base::DefinedOnSelf } let(:method_line) { 61 } - it_should_behave_like 'a method matcher' + it_should_behave_like 'a method matcher' do + %i[public protected private].each do |visibility| + context 'with %s visibility' % visibility do + let(:expected_visibility) { visibility } + + before do + scope.singleton_class.__send__(visibility, method_name) + end + + it 'returns expected subjects' do + expect(subject).to eql([mutation_subject.with(visibility: visibility)]) + end + end + end + end end context 'when defined on constant' do diff --git a/spec/unit/mutant/mutation_spec.rb b/spec/unit/mutant/mutation_spec.rb index 101ecbff0..ba3c4d9e7 100644 --- a/spec/unit/mutant/mutation_spec.rb +++ b/spec/unit/mutant/mutation_spec.rb @@ -52,26 +52,66 @@ def apply end before do - expect(mutation_subject).to receive(:prepare) - .ordered + allow(mutation_subject).to receive(:prepare) .and_return(mutation_subject) - expect(Mutant::Loader).to receive(:call) - .ordered - .with( - binding: TOPLEVEL_BINDING, - kernel: kernel, - source: expected_source, - subject: mutation_subject - ) + allow(mutation_subject).to receive(:post_insert) + .and_return(mutation_subject) + + allow(Mutant::Loader).to receive(:call) .and_return(loader_result) end - let(:expected_source) { '1' } - let(:loader_result) { instance_double(Mutant::Either) } + let(:expected_source) { '1' } + + context 'on successful loader result' do + let(:loader_result) { Mutant::Loader::SUCCESS } + + it 'performs insertion with cleanup' do + apply + + expect(mutation_subject).to have_received(:prepare).ordered + + expect(Mutant::Loader).to have_received(:call) + .ordered + .with( + binding: TOPLEVEL_BINDING, + kernel: kernel, + source: expected_source, + subject: mutation_subject + ) + + expect(mutation_subject).to have_received(:post_insert).ordered + end + + it 'returns loader result' do + expect(apply).to eql(loader_result) + end + end + + context 'on failed loader result' do + let(:loader_result) { Mutant::Loader::VOID_VALUE } + + it 'does perform insertion without cleanup' do + apply + + expect(mutation_subject).to have_received(:prepare).ordered - it 'returns loader result' do - expect(apply).to be(loader_result) + expect(Mutant::Loader).to have_received(:call) + .ordered + .with( + binding: TOPLEVEL_BINDING, + kernel: kernel, + source: expected_source, + subject: mutation_subject + ) + + expect(mutation_subject).to_not have_received(:post_insert) + end + + it 'returns loader result' do + expect(apply).to eql(loader_result) + end end end diff --git a/spec/unit/mutant/subject/method/instance_spec.rb b/spec/unit/mutant/subject/method/instance_spec.rb index d1e82eaa7..aa73710e9 100644 --- a/spec/unit/mutant/subject/method/instance_spec.rb +++ b/spec/unit/mutant/subject/method/instance_spec.rb @@ -3,8 +3,9 @@ RSpec.describe Mutant::Subject::Method::Instance do let(:object) do described_class.new( - context: context, - node: node + context: context, + node: node, + visibility: :private ) end @@ -62,6 +63,19 @@ def self.name it_should_behave_like 'a command method' end + describe '#post_insert' do + subject { object.post_insert } + + it 'sets method visibility' do + expect { subject } + .to change { scope.private_instance_methods.include?(:foo) } + .from(false) + .to(true) + end + + it_should_behave_like 'a command method' + end + describe '#source' do subject { object.source } @@ -72,8 +86,9 @@ def self.name RSpec.describe Mutant::Subject::Method::Instance::Memoized do let(:object) do described_class.new( - context: context, - node: node + context: context, + node: node, + visibility: :public ) end diff --git a/spec/unit/mutant/subject/method/metaclass_spec.rb b/spec/unit/mutant/subject/method/metaclass_spec.rb index 080b36ba7..d85be04ca 100644 --- a/spec/unit/mutant/subject/method/metaclass_spec.rb +++ b/spec/unit/mutant/subject/method/metaclass_spec.rb @@ -3,8 +3,9 @@ RSpec.describe Mutant::Subject::Method::Metaclass do let(:object) do described_class.new( - context: context, - node: node + context: context, + node: node, + visibility: :public ) end diff --git a/spec/unit/mutant/subject/method/singleton_spec.rb b/spec/unit/mutant/subject/method/singleton_spec.rb index 96b9b57c1..30fab4e20 100644 --- a/spec/unit/mutant/subject/method/singleton_spec.rb +++ b/spec/unit/mutant/subject/method/singleton_spec.rb @@ -3,8 +3,9 @@ RSpec.describe Mutant::Subject::Method::Singleton do let(:object) do described_class.new( - context: context, - node: node + context: context, + node: node, + visibility: :private ) end @@ -51,6 +52,19 @@ def self.name it_should_behave_like 'a command method' end + describe '#post_insert' do + subject { object.post_insert } + + it 'sets method visibility' do + expect { subject } + .to change { scope.private_methods.include?(:foo) } + .from(false) + .to(true) + end + + it_should_behave_like 'a command method' + end + describe '#source' do subject { object.source } diff --git a/spec/unit/mutant/subject_spec.rb b/spec/unit/mutant/subject_spec.rb index 708aabb43..d46e1dca8 100644 --- a/spec/unit/mutant/subject_spec.rb +++ b/spec/unit/mutant/subject_spec.rb @@ -67,6 +67,12 @@ def foo it_should_behave_like 'a command method' end + describe '#post_insert' do + subject { object.post_insert } + + it_should_behave_like 'a command method' + end + describe '#node' do subject { object.node }