Add a switch to disable return value propagation

[postmodern]

  def emit_statement(stmt)
    sql = emit_keyword(stmt.keyword)

    unless stmt.argument.nil?
      sql << @space << emit(stmt.argument)
    end

    unless stmt.clauses.empty?
      sql << @space << emit_clauses(stmt.clauses)
    end

-    return sql
+    sql
  end

[mbj]

This is far from beeing pointless.

return sql to sql is a valid mutation. In this case it does not change the behavior of the code, as it is the last expression in the method. So mutant found an uncovered mutation that cannot be fixed via spec adjustment. You have to adjust the code to just use sql as last expression.

The explicit return is redundant and should be removed. The same will happen for unneded explicit self receivers and some other cases.

Our policy is to not accept any redundant use of keywords. See

dkubb/stylegude.

On Sun, Jan 20, 2013 at 10:46:25PM -0800, Postmodern wrote:

      def emit_statement(stmt)
        sql = emit_keyword(stmt.keyword)

        unless stmt.argument.nil?
          sql 

Reply to this email directly or view it on GitHub:
https://github.com/mbj/mutant/issues/10

[postmodern]

Putting a return on the last line of a method is valid Ruby code. I don't think Mutant should be enforcing style :P Maybe it should check if the return is followed by an end token.

[mbj]

It is valid ruby code.

def foo(arg)
  if arg == 1
    return 2
  end
  3
end

Will be mutated to:

def foo(arg)
  if arg == 1
    2 # Same mutation but other occurence
  end
  3
end

This is a valid mutation that must be killed!

Mutant should also help to identify and remove redundant statements. Currently mutant is also context free, means it does not know if it is mutating the last expression of a block when recursing into Rubinius::AST::Return and one of the mutations of Rubinius::AST::Return is the emit of "just the return value".

This sometimes results in a mutation you have to kill from specs, and sometimes in a mutation you have to kill from code. This is a general problem of mutation testing called: Equivalent mutants. In your original case you found an equivalent mutant, me and @dkubb argree we should fight equivalent mutants via using code where these equivalent mutants do not occur. So for this case: Remove the redundant return!

This is more about reducing complexity and not about style.

EDIT: Spell, markdown and summary.

[dkubb]

@mbj yeah I was discussing this with @postmodern in #datamapper. It's my feeling that I would rather mutant nudge code towards having no redundant expressions. I mean, we could add a special case for this, but I would rather defer doing that as long as possible, ideally forever. I would rather have the mutation code work consistently than start to add special cases prematurely.

If it became clear that we must handle some other special cases (besides this) then we could consider making it configurable or something, but for now I would err on being consistent with this.

[mbj]

I'll track this issue as enhancement. (The enhancement is the addition of the option to skip this specific mutation).

[mbj]

@postmodern I just added the basic infrastructure to inject configuration into the mutators. See fc426bee869291af0c4e8472227ef85aed129c96. I think I can fix this issue soon.

I dunno how the CLI should look like. I'm open to suggestions. Pls keep in mind we'll soon have multiple switches. So maybe all the switches should have a common nameing scheme.

[mbj]

I personally think that adding CLI switches is not an option since this case is too specific. It'll be handled via the upcoming config file support.

[mbj]

I think that I'll never to this. Its a feature I'll never use by myself. And maintaining features I do not use is a bad idea.

My OSS time is limited so I need to focus on the features that help me to get a ROI out of it. For that reason I'm closing this.

[mbj]

Reopening it, but tagging on not-on-maintainers-time. Its a nice idea, that might help others. So worth keeping in the tracker. But I can only support this in the following cases:

a) I get payed for implementation + maintenance b) Someone does implementation + maintenance

Both is unlikely but still I want to track it.

[mbj]

Now as mutant is commercial, I'm planning to allow configuration of this nuance.