2.7 Syntax - Upgrade to unparser ~> 0.5.2 #1062

mbj · 2020-10-16T03:37:57Z

Remove all hacks for unparser normalization
Adjust verification to enforce the better unparser constraints
Add 2.7 syntax support

mbj · 2020-10-16T04:10:18Z

lib/mutant/subject/method/instance.rb

@@ -42,7 +42,7 @@ def prepare
        private

          def wrap_node(mutant)
-            s(:begin, mutant, s(:send, nil, :memoize, s(:args, s(:sym, name), *options)))


@snusnu All this years we where generating invalid AST that would silently be emitted as correct intended source.

mbj · 2020-10-16T04:10:38Z

meta/begin.rb


-  source s(:begin, s(:true))
-  # Mutation of each statement in block
-  mutation s(:begin, s(:false))


@dgollahon ^^ was part of the unparser preprocessor hack artifacts.

mbj · 2020-10-16T04:11:38Z

meta/float.rb

@@ -8,9 +8,9 @@
  # edge cases
  mutation '0.0'
  mutation '1.0'
-  mutation '(0.0 / 0.0)'
-  mutation '(1.0 / 0.0)'
-  mutation '(-1.0 / 0.0)'


@dgollahon ^^ Another one. This redundant begin was needed as unparser pre 0.5 was not able to infer or vs || based on AST structure and had to preserve semantic equivalency via extra parens. This extra parsens where an artifact of that.

This one in particular seems like a nice quality of life improvement.

Yeah, the unparser preprocessor hack was a bad shortcut of mine I took years ago when I had "nothing" and had to 80/20 a lot of subsystems to get something.

mbj · 2020-10-16T04:12:03Z

meta/lvar.rb

@@ -1,16 +1,10 @@
 # frozen_string_literal: true

 Mutant::Meta::Example.add :lvar do
-  source 'a = nil; a'
+  declare_lvar :a


Not really an unparser artifact. But correctly using the parser API to avoid redundancy.

mbj · 2020-10-16T04:12:21Z

meta/lvar.rb

-  # TODO: fix invalid AST
-  #   These ASTs are not valid and should NOT be emitted
-  #   Mutations of lvarasgn need to be special cased to avoid this.
-  mutation s(:begin, s(:lvasgn, :a__mutant__, s(:nil)), s(:lvar, :a))


No need anymore to specify AST with forwad lvar declaration.

* Remove all hacks for unparser normalization * Adjust verification to enforce the better unparser constraints * Add 2.7 syntax support

mbj · 2020-10-16T04:13:25Z

meta/op_assgn.rb

  mutation 'self.b += 1'
-  # TODO: fix invalid AST
-  #   This should not get emitted as invalid AST with valid unparsed source
-  mutation s(:op_asgn, s(:ivar, :@a), :+, s(:int, 1))


Side effect of better unparser we can remove that one now.

dgollahon · 2020-10-16T18:27:36Z

spec/unit/mutant/meta/example/dsl_spec.rb

+      let(:source) { 'false' }
+
+      let(:expected) do
+        [
+          Mutant::Meta::Example::Expected.new(
+            node:            s(:nil),
+            original_source: 'nil'
+          )
+        ]
+      end


This is kind of interesting. I'm not sure I fully understand the context of this spec, but does this mutate false -> nil somehow? I removed that mutation recently (I thought). Or is this just an artifact of test setup but wouldn't actually happen in practice?

This spec does not demonstrate the mutations. It never calls the mutation engine, AKA Mutant::Mutator::Node is never called. Instead it specifies on how mutations are specified. For this I run through the scenarios:

There was a mutation being generated that was not expected.

A mutation that was expected was not being generated.

A mutation that was generated unparses to invalid syntax

The original code that was being specified did not round trip.

And for each of these cases above I set-up an Mutant::Meta::Example object in that specific state. But I intentionally do NOT run the engine here. But still use AST nodes that can round trip to unparser at low complexity.

or TLDR: This code is the specification of the mutant mutation specifications and uses fake mutated nodes to not hard code the specification of properties of the specification against the mutation engine.

or TLDR: This code is the specification of the mutant mutation specifications and uses fake mutated nodes to not hard code the specification of properties of the specification against the mutation engine.

Ok, cool. I thought that might be the case but wasn't 100% sure at a glance. Might make sense to use a valid mutation for documentation consistency but probably doesn't matter much.

Might make sense to use a valid mutation for documentation consistency but probably doesn't matter much.

it is a valid mutation example to demonstrate the specification of the specification on the properties I listed above. And I do not wish to call the real mutation engine here. I only wish to demonstrate I can find specific expected, missing and invalid mutations.

Right, I just meant I usually like to have examples in my tests match "real" behavior as much as possible. It is clearly valid in this context to mutate to anything but I thought it might be slightly less confusing to match the "real" behavior. Doesn't actually matter though, feel free to ignore.

If I where to start having to manually have generated mutations in sync here, I'd enforce something that adds no value to the test. And even would break as mutations are about to change drastically.

I'm experimenting a lot with "tiers" of mutations to default to only the most useful etc.

Add Gemfile.shared to devloop event sources

e823048

mbj force-pushed the upgrade/unparser branch 3 times, most recently from e1611f5 to d12d1fa Compare October 16, 2020 04:05

mbj changed the title ~~Upgrade to unparser ~> 0.5.2~~ 2.7 Syntax - Upgrade to unparser ~> 0.5.2 Oct 16, 2020

mbj force-pushed the upgrade/unparser branch 3 times, most recently from d6c91fb to 2b896a0 Compare October 16, 2020 04:08

mbj commented Oct 16, 2020

View reviewed changes

Upgrade to unparser ~> 0.5.2

ccc9358

* Remove all hacks for unparser normalization * Adjust verification to enforce the better unparser constraints * Add 2.7 syntax support

mbj force-pushed the upgrade/unparser branch from 2b896a0 to ccc9358 Compare October 16, 2020 04:12

mbj commented Oct 16, 2020

View reviewed changes

mbj self-assigned this Oct 16, 2020

mbj added enhancement review-ready labels Oct 16, 2020

mbj merged commit 502aad0 into master Oct 16, 2020

mbj deleted the upgrade/unparser branch October 16, 2020 04:16

dgollahon reviewed Oct 16, 2020

View reviewed changes

Provide feedback

Saved searches

Use saved searches to filter your results more quickly

2.7 Syntax - Upgrade to unparser ~> 0.5.2 #1062

2.7 Syntax - Upgrade to unparser ~> 0.5.2 #1062

mbj commented Oct 16, 2020

mbj Oct 16, 2020

mbj Oct 16, 2020

mbj Oct 16, 2020

dgollahon Oct 16, 2020

mbj Oct 16, 2020

mbj Oct 16, 2020

mbj Oct 16, 2020

mbj Oct 16, 2020

dgollahon Oct 16, 2020

mbj Oct 16, 2020

mbj Oct 16, 2020

dgollahon Oct 16, 2020

mbj Oct 16, 2020

dgollahon Oct 18, 2020

mbj Oct 18, 2020

2.7 Syntax - Upgrade to unparser ~> 0.5.2 #1062

2.7 Syntax - Upgrade to unparser ~> 0.5.2 #1062

Conversation

mbj commented Oct 16, 2020

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment