Skip to content
This repository was archived by the owner on Aug 23, 2020. It is now read-only.
Merged
Show file tree
Hide file tree
Changes from all commits
Commits
File filter

Filter by extension

Filter by extension

Conversations
Failed to load comments.
Loading
Jump to
Jump to file
Failed to load files.
Loading
Diff view
Diff view
2 changes: 1 addition & 1 deletion config/flay.yml
Original file line number Diff line number Diff line change
@@ -1,3 +1,3 @@
---
threshold: 16
total_score: 1315
total_score: 1309
2 changes: 2 additions & 0 deletions lib/mutest.rb
Original file line number Diff line number Diff line change
Expand Up @@ -74,6 +74,8 @@ def self.ci?
require 'mutest/actor/mailbox'
require 'mutest/actor/env'
require 'mutest/parser'
require 'mutest/source_file'
require 'mutest/ignores'
require 'mutest/isolation'
require 'mutest/isolation/none'
require 'mutest/isolation/fork'
Expand Down
6 changes: 5 additions & 1 deletion lib/mutest/context.rb
Original file line number Diff line number Diff line change
@@ -1,7 +1,7 @@
module Mutest
# An abstract context where mutations can be applied to.
class Context
include Adamantium::Flat, Concord::Public.new(:scope, :source_path)
include Adamantium::Flat, Concord::Public.new(:scope, :source_path, :ignores)
extend AST::Sexp

NAMESPACE_DELIMITER = '::'.freeze
Expand Down Expand Up @@ -77,6 +77,10 @@ def match_expressions
# @return [Module|Class]
attr_reader :scope

def ignore?(node)
ignores.ignored?(node)
end

private

# Nesting of names in scope
Expand Down
39 changes: 39 additions & 0 deletions lib/mutest/ignores.rb
Original file line number Diff line number Diff line change
@@ -0,0 +1,39 @@
module Mutest
class Ignores
include Equalizer.new(:disable_lines), Adamantium::Flat

COMMENT_TEXT = '# mutest:disable'.freeze

def initialize(comments)
@comments = comments
end

# TODO: Support multiple lines of comments preceeding a disable
# TODO: Support inline comment disable
def ignored?(node)
location = node.location
return false unless location.expression

disable_lines.include?(location.line)
end

protected

def disable_lines
disable_comments.map do |comment|
comment.location.line + 1
end
Copy link
Copy Markdown
Owner

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

This may be worth converting to a set. Duplicates serve no purpose and repeatedly scanning the list for every node mutated could add up.

Copy link
Copy Markdown
Collaborator Author

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Yeah I can do that

Copy link
Copy Markdown
Collaborator Author

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Heh hard to prove it is a set since it is a private method. If only we had inline comment disables!

Copy link
Copy Markdown
Owner

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Yeah, that occurred to me after I said it :P

Copy link
Copy Markdown
Collaborator Author

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Heh

Copy link
Copy Markdown
Owner

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

You could always initialize a set and add to it, of course.

Copy link
Copy Markdown
Collaborator Author

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

I'd prefer to just leave it for now I think and then add an inline disable when I can 😆

end
memoize :disable_lines

private

attr_reader :comments

def disable_comments
comments.select do |comment|
comment.text.eql?(COMMENT_TEXT)
end
end
end # Ignores
end # Mutest
14 changes: 12 additions & 2 deletions lib/mutest/matcher/method.rb
Original file line number Diff line number Diff line change
@@ -1,6 +1,8 @@
module Mutest
class Matcher
# Abstract base class for method matchers
#
# :reek:TooManyMethods { max_methods: 11 }
class Method < self
include AbstractType,
Adamantium::Flat,
Expand Down Expand Up @@ -71,16 +73,24 @@ def method_name
#
# @return [Context]
def context
Context.new(scope, source_path)
Context.new(scope, source_path, ignores)
Copy link
Copy Markdown
Owner

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

the term ignores seems better than the term comment used in Context

Copy link
Copy Markdown
Collaborator Author

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

k I'll rename to Ignores until I do that refactor I mentioned

end

# Root source node
#
# @return [Parser::AST::Node]
def ast
env.parser.call(source_path)
env.parser.parse(source_path)
end

# Ignores for source file
#
# @return [Array<Mutest::Ignore>]
def ignores
Ignores.new(env.parser.comments(source_path))
end
memoize :ignores

# Path to source
#
# @return [Pathname]
Expand Down
26 changes: 18 additions & 8 deletions lib/mutest/mutator.rb
Original file line number Diff line number Diff line change
Expand Up @@ -3,16 +3,21 @@ module Mutest
class Mutator
REGISTRY = Registry.new

include Adamantium::Flat, Concord.new(:input, :parent), AbstractType, Procto.call(:output)
include Adamantium::Flat,
Concord.new(:input, :filter, :parent),
AbstractType,
Procto.call(:output)

# Lookup and invoke dedicated AST mutator
#
# @param node [Parser::AST::Node]
# @param parent [nil,Mutest::Mutator::Node]
#
# @return [Set<Parser::AST::Node>]
def self.mutate(node, parent = nil)
self::REGISTRY.lookup(node.type).call(node, parent)
#
# :reek:LongParameterList
def self.mutate(node, filter = ->(_) {}, parent = nil)
self::REGISTRY.lookup(node.type).call(node, filter, parent)
end

# Register node class handler
Expand All @@ -35,16 +40,21 @@ def self.handle(*types)
# Initialize object
#
# @param [Object] input
# @param [#call] mutation filter
# @param [Object] parent
# @param [#call(node)] block
#
# @return [undefined]
def initialize(_input, _parent = nil)
def initialize(_input, _filter, _parent = nil)
Copy link
Copy Markdown
Owner

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

What's going on here? Is this because super is using them indirectly?

Copy link
Copy Markdown
Collaborator Author

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Yeah.

Copy link
Copy Markdown
Owner

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

weird ruby is weird. unfortunate that it's basically a false positive _. I wonder if we could not require underscored args in methods with zsuper in a semi-straightforward way.

Copy link
Copy Markdown
Collaborator Author

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Yeah that would be nice

super

@output = Set.new

dispatch
dispatch unless disabled?
end

def disabled?
filter.call(input)
end

# Test if generated object is not guarded from emitting
Expand Down Expand Up @@ -82,8 +92,8 @@ def dup_input
# Mutate child nodes within source path
#
# @return [Set<Parser::AST::Node>]
def mutate(*args)
self.class.mutate(*args)
def mutate(node, parent = nil)
self.class.mutate(node, filter, parent)
end

# Run input with mutator
Expand All @@ -101,7 +111,7 @@ def run(mutator)
def mutate_with(mutator, nodes, &block)
block ||= method(:emit)

mutator.call(nodes).each(&block)
mutator.call(nodes, filter).each(&block)
end
end # Mutator
end # Mutest
2 changes: 1 addition & 1 deletion lib/mutest/mutator/node.rb
Original file line number Diff line number Diff line change
Expand Up @@ -54,7 +54,7 @@ def children
# @return [undefined]
def mutate_child(index, &block)
block ||= TAUTOLOGY
Mutator.mutate(children.fetch(index), self).each do |mutation|
mutate(children.fetch(index), self).each do |mutation|
next unless block.call(mutation)
emit_child_update(index, mutation)
end
Expand Down
2 changes: 1 addition & 1 deletion lib/mutest/mutator/node/literal/regex.rb
Original file line number Diff line number Diff line change
Expand Up @@ -42,7 +42,7 @@ def mutate_body
return unless body.all?(&method(:n_str?))
return unless AST::Regexp.supported?(body_expression)

Mutator.mutate(body_ast).each do |mutation|
mutate(body_ast).each do |mutation|
source = AST::Regexp.to_expression(mutation).to_s
emit_type(s(:str, source), options)
end
Expand Down
4 changes: 4 additions & 0 deletions lib/mutest/mutator/util.rb
Original file line number Diff line number Diff line change
Expand Up @@ -2,6 +2,10 @@ module Mutest
class Mutator
# Namespace for utility mutators
class Util < self
private

def disabled?
end
end # Util
end # Mutator
end # Mutest
2 changes: 1 addition & 1 deletion lib/mutest/mutator/util/array.rb
Original file line number Diff line number Diff line change
Expand Up @@ -28,7 +28,7 @@ class Element < Util
# @return [undefined]
def dispatch
input.each_with_index do |element, index|
Mutator.mutate(element).each do |mutation|
mutate(element).each do |mutation|
Copy link
Copy Markdown
Owner

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Was this Mutator qualification always unnecessary?

Copy link
Copy Markdown
Collaborator Author

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

It was a result of that extraction PR you reviewed before

Copy link
Copy Markdown
Owner

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Ah. Yeah, I was just trying to see why that would have changed due to this PR.

dup = dup_input
dup[index] = mutation
emit(dup)
Expand Down
19 changes: 17 additions & 2 deletions lib/mutest/parser.rb
Original file line number Diff line number Diff line change
Expand Up @@ -15,8 +15,23 @@ def initialize
# @param [Pathname] path
#
# @return [AST::Node]
def call(path)
@cache[path] ||= ::Parser::CurrentRuby.parse(path.read)
def parse(path)
read(path).ast
end

# Extract comments from path
#
# @param [Pathname] path
#
# @return [Parser::Source::Comment]
def comments(path)
read(path).comments
end

private

def read(path)
@cache[path] ||= SourceFile.parse(path.read)
end
end # Parser
end # Mutest
13 changes: 13 additions & 0 deletions lib/mutest/source_file.rb
Original file line number Diff line number Diff line change
@@ -0,0 +1,13 @@
module Mutest
# A source file representation
class SourceFile
include Concord::Public.new(:ast, :comments)

# Read and parse file with comments
#
# @return [undefined]
def self.parse(source)
new(*::Parser::CurrentRuby.parse_with_comments(source))
end
end # SourceFile
end # Mutest
4 changes: 2 additions & 2 deletions lib/mutest/subject.rb
Original file line number Diff line number Diff line change
Expand Up @@ -10,8 +10,8 @@ class Subject
# @return [undefined]
def mutations
[neutral_mutation].concat(
Mutator.mutate(node).map do |mutest|
Mutation::Evil.new(self, wrap_node(mutest))
Mutator.mutate(node, context.method(:ignore?)).map do |mutant|
Mutation::Evil.new(self, wrap_node(mutant))
end
)
end
Expand Down
4 changes: 4 additions & 0 deletions spec/shared/method_matcher_behavior.rb
Original file line number Diff line number Diff line change
Expand Up @@ -23,6 +23,10 @@
expect(context.scope).to eql(scope)
end

it 'has the correct ignores in conext' do
expect(context.ignores).to eql(ignores)
end

it 'has source path in context' do
expect(context.source_path).to eql(source_path)
end
Expand Down
38 changes: 35 additions & 3 deletions spec/unit/mutest/context_spec.rb
Original file line number Diff line number Diff line change
Expand Up @@ -30,9 +30,12 @@
end
end

let(:object) { described_class.new(scope, source_path) }
let(:source_path) { instance_double(Pathname) }
let(:scope) { TestApp::Literal }
let(:source_path) { instance_double(Pathname) }
let(:scope) { TestApp::Literal }

let(:object) do
described_class.new(scope, source_path, Mutest::Ignores.new([]))
end

describe '#identification' do
subject { object.identification }
Expand Down Expand Up @@ -106,4 +109,33 @@ class Literal
end
end
end

describe '#ignore?' do
let(:object) do
described_class.new(scope, source_path, ignores)
end

let(:ignores) { instance_double(Mutest::Ignores) }
let(:node) { s(:node) }

before do
allow(ignores).to receive(:ignored?).with(node).and_return(ignore)
end

context 'when comment disables the node' do
let(:ignore) { true }

it 'ignores the node' do
expect(object.ignore?(node)).to be(true)
end
end

context 'when comment does not disable the node' do
let(:ignore) { false }

it 'ignores the node' do
expect(object.ignore?(node)).to be(false)
end
end
end
end
33 changes: 33 additions & 0 deletions spec/unit/mutest/ignores_spec.rb
Original file line number Diff line number Diff line change
@@ -0,0 +1,33 @@
RSpec.describe Mutest::Ignores do
subject(:comments) { described_class.new(parsed.last) }

let(:ast) { parsed.first }

let(:parsed) do
Parser::CurrentRuby.parse_with_comments(<<-RUBY)
# rubocop:disable
def foo(bar)
end

# mutest:disable
def bar
end
RUBY
end

it 'ignores the line after the the comment' do
foo_method, bar_method = *ast

aggregate_failures do
expect(comments.ignored?(foo_method)).to be(false)
expect(comments.ignored?(bar_method)).to be(true)
end
end

it 'ignores nodes that do not have a location' do
_, bar_method = *ast
_, bar_method_args, = *bar_method

expect(comments.ignored?(bar_method_args)).to be(false)
end
end
4 changes: 4 additions & 0 deletions spec/unit/mutest/matcher/method/instance_spec.rb
Original file line number Diff line number Diff line change
Expand Up @@ -17,6 +17,10 @@
)
end

let(:ignores) do
Mutest::Ignores.new([])
end

def name
node.children.fetch(0)
end
Expand Down
4 changes: 4 additions & 0 deletions spec/unit/mutest/matcher/method/singleton_spec.rb
Original file line number Diff line number Diff line change
Expand Up @@ -25,6 +25,10 @@ def arguments
node.children.fetch(2)
end

let(:ignores) do
Mutest::Ignores.new([])
end

context 'when also defined on lvar' do
let(:scope) { base::DefinedOnLvar }
let(:expected_warnings) do
Expand Down
2 changes: 1 addition & 1 deletion spec/unit/mutest/mutator/node_spec.rb
Original file line number Diff line number Diff line change
Expand Up @@ -28,7 +28,7 @@ def dispatch
end

def apply
klass.call(s(:and, s(:true), s(:true)))
klass.call(s(:and, s(:true), s(:true)), ->(_) {})
end

specify do
Expand Down
Loading