Mutation bug: "Can't change the value of self"

[dkubb]

I just found a bug in how mutant mutates expressions like: self.attr ||= something, eg:

/Users/dkubb/.rvm/gems/ruby-2.0.0-p247@jettstream-api/gems/mutant-0.3.0.beta13/lib/mutant/loader.rb:43:in `eval': /Users/dkubb/Dropbox/Home/Programming/work/jettstream/api/app/models/child/questionnaire/answer.rb:37: Can't change the value of self (SyntaxError)
          self ||= option.questionnaire_id
                  ^

evil:Child::Questionnaire::Answer#set_default_questionnaire_id:/Users/dkubb/Dropbox/Home/Programming/work/jettstream/api/app/models/child/questionnaire/answer.rb:32:bce57
@@ -1,6 +1,6 @@
 def set_default_questionnaire_id
   if option
-    self.questionnaire_id ||= option.questionnaire_id
+    self ||= option.questionnaire_id
   end
 end

[mbj]

@dkubb Yeah, good catch an easy to fix. Will address it soon.

Interesting this one wasnt found more early. Most likely the amount of useing the op-assign operators was reduced a lot with dm2-rom-style.

[mbj]

@dkubb While generalizing this kind of error I think I should also exclude cases with literal lhs!

[dkubb]

@mbj yeah, that's a good idea. Are there notes that represent literal values and includes things like: 'string', 1, nil, true, false etc?

At first I was thinking you should only include nodes known to support assignment; but then I realized you could easily accidentally miss whole classes of nodes with that approach. I guess using an explicit exclude approach is better. While it might take some tweaking to get the list complete, mutating everything by default is probably a better approach.

[mbj]

@dkubb Jo, I'd only do blacklisting here. And yes identifying literals is easy just use Node#type.

:sym,:int,:nil,:true,:false,:self etc will be included.

[mbj]

Also detection of that case is easy. The "inner" nodes mutator, the mutator that mutates the "self.attribute" has to detect it is within an op assign parent. And it will NOT invoke the "emit receiver" mutation if the receiver is of that type list I gave above.

I really love mutants design here. We have access to the parent mutator and can easily branch on the parents Node#type.

Mutant is a recursive tree walker emitting mutations and tracking context.

[dkubb]

It would be kind of weird for the receiver to be one of those nodes listed, besides self of course, but I suppose it could happen if you were to do something like:

class String
  attr_accessor :extra
end

# will blow up when mutated to: '' = 42
''.extra = 42

Even though I would consider this a bit dumb, it is valid ruby.

[mbj]

@dkubb Do not confuse assignment with op-assign. = vs ||=.

[mbj]

My pref comment is not valid.

@dkubb My english limits me here, let the code that fixes the problem speek. Soon, still busy with other stuff.

[dkubb]

@mbj awesome, thanks! I'll test this out now

[dkubb]

@mbj works great. the code that triggered this bug originally works great with mutant now. Thanks!