Fix to reset visibility after insertion

* Before this change mutant would not in all cases ensure that the
  mutated method visibiltiy is staying exactly at unmutated visibility.
* With this change visibility is explicitly restored on insertion
  allowing to guarantee this property

[Fix #1242]

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.

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))

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

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

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<Integer>]

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
       #

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

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

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
 

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

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
 

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

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
 

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
 

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