From 2ce02af8aa6cd2dc838ff8ff1854adc01c81d782 Mon Sep 17 00:00:00 2001 From: Markus Schirp Date: Mon, 22 Dec 2014 01:28:30 +0000 Subject: [PATCH] Fix style --- .rubocop.yml | 2 + config/flay.yml | 2 +- config/rubocop.yml | 123 ++++++++++++------------ lib/mutant.rb | 2 + lib/mutant/ast.rb | 2 +- lib/mutant/cli.rb | 18 ++-- lib/mutant/env.rb | 2 +- lib/mutant/isolation.rb | 6 +- lib/mutant/loader.rb | 4 + lib/mutant/matcher/compiler.rb | 2 + lib/mutant/mutation.rb | 2 + lib/mutant/mutator/node/const.rb | 2 +- lib/mutant/mutator/node/generic.rb | 2 +- lib/mutant/parallel/master.rb | 2 + lib/mutant/reporter/cli.rb | 12 +-- lib/mutant/reporter/cli/printer.rb | 9 +- lib/mutant/zombifier/file.rb | 37 +++++-- spec/support/corpus.rb | 2 + spec/support/fake_actor.rb | 8 +- spec/support/mutation_verifier.rb | 2 + spec/support/shared_context.rb | 3 +- spec/unit/mutant/env_spec.rb | 6 +- spec/unit/mutant/expression_spec.rb | 5 +- spec/unit/mutant/warning_filter_spec.rb | 3 +- 24 files changed, 154 insertions(+), 104 deletions(-) diff --git a/.rubocop.yml b/.rubocop.yml index 125fba82..a1bb4d67 100644 --- a/.rubocop.yml +++ b/.rubocop.yml @@ -5,4 +5,6 @@ AllCops: - 'Gemfile.devtools' - 'vendor/**/*' - 'tmp/**/*' + - 'test_app/**/*' - 'benchmarks/**/*' + - 'bin/mutant' diff --git a/config/flay.yml b/config/flay.yml index 81378845..6f7b4ad8 100644 --- a/config/flay.yml +++ b/config/flay.yml @@ -1,3 +1,3 @@ --- threshold: 18 -total_score: 1191 +total_score: 1198 diff --git a/config/rubocop.yml b/config/rubocop.yml index 0a6c7d3e..22b0f942 100644 --- a/config/rubocop.yml +++ b/config/rubocop.yml @@ -1,18 +1,5 @@ inherit_from: ../.rubocop.yml -# General note about rubocop. -# It does NOT allow to silence a specific rule violation. -# For that reason I sometimes have to disable a whole cop where -# I just tried to whitelist a specific occurrence. - - -AllCops: - Include: - - '../**/*.rake' - - 'Gemfile' - - 'Gemfile.triage' - - 'mutant.gemspec' - # Avoid parameter lists longer than five parameters. ParameterLists: Max: 3 @@ -30,53 +17,31 @@ CollectionMethods: find: 'detect' find_all: 'select' -# Use square brackets for literal Array objects -PercentLiteralDelimiters: - PreferredDelimiters: - '%': () - '%i': '[]' - '%q': () - '%Q': () - '%r': '{}' - '%s': () - '%w': '[]' - '%W': '[]' - '%x': () - -MethodLength: - CountComments: false - Max: 17 # TODO: Bring down to 10 - -RegexpLiteral: # I do not agree %r(\A) is more readable than /\A/ +# Do not force public/protected/private keyword to be indented at the same +# level as the def keyword. My personal preference is to outdent these keywords +# because I think when scanning code it makes it easier to identify the +# sections of code and visually separate them. When the keyword is at the same +# level I think it sort of blends in with the def keywords and makes it harder +# to scan the code and see where the sections are. +AccessModifierIndentation: Enabled: false -Eval: - Enabled: false # Mutant must use Kernel#eval to inject mutated source - # Limit line length LineLength: - Max: 124 # TODO: lower to 79 + Max: 120 # Disable documentation checking until a class needs to be documented once Documentation: Enabled: false +# Do not always use &&/|| instead of and/or. +AndOr: + Enabled: false + # Do not favor modifier if/unless usage when you have a single-line body IfUnlessModifier: Enabled: false -# Mutant needs to define methods like def bar; end in specs -Semicolon: - Enabled: false - -# Mutant needs to define multiple methods on same line in specs -EmptyLineBetweenDefs: - Enabled: false - -# Mutant needs to define singleton methods like Foo.bar in specs -ClassMethods: - Enabled: false - # Allow case equality operator (in limited use within the specs) CaseEquality: Enabled: false @@ -89,33 +54,63 @@ ConstantName: TrivialAccessors: Enabled: false -# And also have a different opinion here -AndOr: +# Allow empty lines around class body +EmptyLinesAroundClassBody: Enabled: false -# I like my raise -SignalException: +# Allow empty lines around module body +EmptyLinesAroundModuleBody: Enabled: false -# I need to chain optparse builder, else it is more ugly -MultilineBlockChain: +# Allow empty lines around block body +EmptyLinesAroundBlockBody: Enabled: false -ClassLength: - Max: 119 - -# I align private keywords with class body -IndentationWidth: +# Allow multiple line operations to not require indentation +MultilineOperationIndentation: Enabled: false -# I like to have an empty line before closing the currently opened body -EmptyLinesAroundBody: +# Prefer String#% over Kernel#sprintf +FormatString: Enabled: false -# I test this style for a while +# Use square brackets for literal Array objects +PercentLiteralDelimiters: + PreferredDelimiters: + '%': '{}' + '%i': '[]' + '%q': () + '%Q': () + '%r': '{}' + '%s': () + '%w': '[]' + '%W': '[]' + '%x': () + +# Align if/else blocks with the variable assignment +EndAlignment: + AlignWith: variable + +# Do not always align parameters when it is easier to read +AlignParameters: + Exclude: + - spec/**/*_spec.rb + +# Prefer #kind_of? over #is_a? +ClassCheck: + EnforcedStyle: kind_of? + +# Do not prefer double quotes to be used when %q or %Q is more appropriate +UnneededPercentQ: + Enabled: false + +# Allow a maximum ABC score +Metrics/AbcSize: + Max: 18.00 + +# Do not prefer lambda.call(...) over lambda.(...) LambdaCall: Enabled: false -# I like my style more -AccessModifierIndentation: - Enabled: false +Style/RegexpLiteral: + MaxSlashes: 0 diff --git a/lib/mutant.rb b/lib/mutant.rb index 412c5f33..b4cc9c37 100644 --- a/lib/mutant.rb +++ b/lib/mutant.rb @@ -74,6 +74,8 @@ module Mutant # # @api private # + # rubocop:disable MethodLength + # def self.singleton_subclass_instance(name, superclass, &block) klass = Class.new(superclass) do def inspect diff --git a/lib/mutant/ast.rb b/lib/mutant/ast.rb index 7dfd78fe..2aacc648 100644 --- a/lib/mutant/ast.rb +++ b/lib/mutant/ast.rb @@ -15,7 +15,7 @@ module Mutant # @api private # def self.walk(node, stack, &block) - raise ArgumentError, 'block expected' unless block_given? + fail ArgumentError, 'block expected' unless block_given? block.call(node, stack) node.children.grep(Parser::AST::Node).each do |child| diff --git a/lib/mutant/cli.rb b/lib/mutant/cli.rb index 61be1b20..e0a4c106 100644 --- a/lib/mutant/cli.rb +++ b/lib/mutant/cli.rb @@ -74,7 +74,7 @@ module Mutant parse_match_expressions(opts.parse!(arguments)) rescue OptionParser::ParseError => error - fail(Error, error.message, error.backtrace) + raise(Error, error.message, error.backtrace) end # Parse matchers @@ -101,6 +101,8 @@ module Mutant # # @api private # + # rubocop:disable MethodLength + # def add_environment_options(opts) opts.separator('Environment:') opts.on('--zombie', 'Run mutant zombified') do @@ -129,7 +131,7 @@ module Mutant require "mutant/integration/#{name}" update(integration: Integration.lookup(name)) rescue LoadError - fail Error, "Could not load integration #{name.inspect} (you may want to try installing the gem mutant-#{name})" + raise Error, "Could not load integration #{name.inspect} (you may want to try installing the gem mutant-#{name})" end # Add options @@ -146,7 +148,8 @@ module Mutant opts.on('--score COVERAGE', 'Fail unless COVERAGE is not reached exactly') do |coverage| update(expected_coverage: Float(coverage)) - end.on('--use STRATEGY', 'Use STRATEGY for killing mutations', &method(:setup_integration)) + end + opts.on('--use STRATEGY', 'Use STRATEGY for killing mutations', &method(:setup_integration)) end # Add filter options @@ -177,12 +180,15 @@ module Mutant def add_debug_options(opts) opts.on('--fail-fast', 'Fail fast') do update(fail_fast: true) - end.on('--version', 'Print mutants version') do + end + opts.on('--version', 'Print mutants version') do puts("mutant-#{Mutant::VERSION}") Kernel.exit(EXIT_SUCCESS) - end.on('-d', '--debug', 'Enable debugging output') do + end + opts.on('-d', '--debug', 'Enable debugging output') do update(debug: true) - end.on_tail('-h', '--help', 'Show this message') do + end + opts.on_tail('-h', '--help', 'Show this message') do puts(opts.to_s) Kernel.exit(EXIT_SUCCESS) end diff --git a/lib/mutant/env.rb b/lib/mutant/env.rb index 7a8a4e33..3adda8bc 100644 --- a/lib/mutant/env.rb +++ b/lib/mutant/env.rb @@ -125,7 +125,7 @@ module Mutant def expression(scope) name = scope_name(scope) or return - unless name.is_a?(String) + unless name.instance_of?(String) warn("#{scope.class}#name from: #{scope.inspect} returned #{name.inspect}. #{SEMANTICS_MESSAGE}") return end diff --git a/lib/mutant/isolation.rb b/lib/mutant/isolation.rb index 35477e3e..a2242ee6 100644 --- a/lib/mutant/isolation.rb +++ b/lib/mutant/isolation.rb @@ -17,7 +17,7 @@ module Mutant def self.call(&block) block.call rescue => exception - fail Error, exception + raise Error, exception end end @@ -37,6 +37,8 @@ module Mutant # # @api private # + # rubocop:disable MethodLength + # def self.call(&block) reader, writer = IO.pipe.map(&:binmode) @@ -52,7 +54,7 @@ module Mutant writer.close Marshal.load(reader.read) rescue => exception - fail Error, exception + raise Error, exception ensure Process.waitpid(pid) if pid end diff --git a/lib/mutant/loader.rb b/lib/mutant/loader.rb index 0ddd097d..4744ffd9 100644 --- a/lib/mutant/loader.rb +++ b/lib/mutant/loader.rb @@ -12,6 +12,10 @@ module Mutant # # @api private # + # One off the very few valid uses of eval + # + # rubocop:disable Lint/Eval + # def call eval( source, diff --git a/lib/mutant/matcher/compiler.rb b/lib/mutant/matcher/compiler.rb index 4e560d09..8c7e8fa5 100644 --- a/lib/mutant/matcher/compiler.rb +++ b/lib/mutant/matcher/compiler.rb @@ -42,6 +42,8 @@ module Mutant # # @api private # + # rubocop:disable MethodLength + # def predicate if subject_selector && subject_rejector Morpher::Evaluator::Predicate::Boolean::And.new([ diff --git a/lib/mutant/mutation.rb b/lib/mutant/mutation.rb index 80e492cc..7976f7ef 100644 --- a/lib/mutant/mutation.rb +++ b/lib/mutant/mutation.rb @@ -16,6 +16,8 @@ module Mutant # # @api private # + # rubocop:disable MethodLength + # def kill(isolation, integration) start = Time.now tests = subject.tests diff --git a/lib/mutant/mutator/node/const.rb b/lib/mutant/mutator/node/const.rb index 90066a1a..476483b9 100644 --- a/lib/mutant/mutator/node/const.rb +++ b/lib/mutant/mutator/node/const.rb @@ -19,7 +19,7 @@ module Mutant emit_singletons unless parent_node && n_const?(parent_node) emit_type(nil, *children.drop(1)) children.each_with_index do |child, index| - mutate_child(index) if child.is_a?(Parser::AST::Node) + mutate_child(index) if child.instance_of?(Parser::AST::Node) end end diff --git a/lib/mutant/mutator/node/generic.rb b/lib/mutant/mutator/node/generic.rb index 7f38c225..5f8de327 100644 --- a/lib/mutant/mutator/node/generic.rb +++ b/lib/mutant/mutator/node/generic.rb @@ -26,7 +26,7 @@ module Mutant # def dispatch children.each_with_index do |child, index| - mutate_child(index) if child.is_a?(Parser::AST::Node) + mutate_child(index) if child.instance_of?(Parser::AST::Node) end end diff --git a/lib/mutant/parallel/master.rb b/lib/mutant/parallel/master.rb index a00dc5dc..1e941c47 100644 --- a/lib/mutant/parallel/master.rb +++ b/lib/mutant/parallel/master.rb @@ -43,6 +43,8 @@ module Mutant # # @api private # + # rubocop:disable MethodLength + # def run config.jobs.times do @workers += 1 diff --git a/lib/mutant/reporter/cli.rb b/lib/mutant/reporter/cli.rb index e2ff1c14..0483fe3c 100644 --- a/lib/mutant/reporter/cli.rb +++ b/lib/mutant/reporter/cli.rb @@ -14,13 +14,11 @@ module Mutant # def self.build(output) tty = output.respond_to?(:tty?) && output.tty? - format = - if !Mutant.ci? && tty && Tput::INSTANCE.available - Format::Framed.new(tty: tty, tput: Tput::INSTANCE) - else - Format::Progressive.new(tty: tty) - end - + format = if !Mutant.ci? && tty && Tput::INSTANCE.available + Format::Framed.new(tty: tty, tput: Tput::INSTANCE) + else + Format::Progressive.new(tty: tty) + end new(output, format) end diff --git a/lib/mutant/reporter/cli/printer.rb b/lib/mutant/reporter/cli/printer.rb index 7c325f13..063aa8c9 100644 --- a/lib/mutant/reporter/cli/printer.rb +++ b/lib/mutant/reporter/cli/printer.rb @@ -176,7 +176,7 @@ module Mutant # @api private # def active_subject_results - active_mutation_jobs = active_jobs.select { |job| job.payload.is_a?(Mutant::Mutation) } + active_mutation_jobs = active_jobs.select { |job| job.payload.kind_of?(Mutant::Mutation) } active_subjects = active_mutation_jobs.map(&:payload).flat_map(&:subject).to_set payload.subject_results.select do |subject_result| @@ -197,6 +197,8 @@ module Mutant # # @api private # + # rubocop:disable AbcSize + # def run info 'Mutant configuration:' info 'Matcher: %s', object.matcher_config.inspect @@ -231,6 +233,8 @@ module Mutant # # @api private # + # rubocop:disable MethodLength + # def run visit(Config, env.config) info 'Subjects: %s', amount_subjects @@ -480,7 +484,8 @@ module Mutant delegate :mutation, :test_result - DIFF_ERROR_MESSAGE = 'BUG: Mutation NOT resulted in exactly one diff hunk. Please report a reproduction!'.freeze + DIFF_ERROR_MESSAGE = + 'BUG: Mutation NOT resulted in exactly one diff hunk. Please report a reproduction!'.freeze MAP = { Mutant::Mutation::Evil => :evil_details, diff --git a/lib/mutant/zombifier/file.rb b/lib/mutant/zombifier/file.rb index d3796827..3d403e96 100644 --- a/lib/mutant/zombifier/file.rb +++ b/lib/mutant/zombifier/file.rb @@ -10,6 +10,10 @@ module Mutant # # @api private # + # Probably one of the only valid uses of eval. + # + # rubocop:disable Lint/Eval + # def zombify(namespace) $stderr.puts("Zombifying #{path}") eval( @@ -33,15 +37,7 @@ module Mutant # @api private # def self.find(logical_name) - file_name = - case ::File.extname(logical_name) - when '.so' - return - when '.rb' - logical_name - else - "#{logical_name}.rb" - end + file_name = expand_file_name(logical_name) $LOAD_PATH.each do |path| path = Pathname.new(path).join(file_name) @@ -52,6 +48,29 @@ module Mutant nil end + # Return expanded file name + # + # @param [String] logical_name + # + # @return [nil] + # if no expansion is possible + # + # @return [String] + # + # @api private + # + def self.expand_file_name(logical_name) + case ::File.extname(logical_name) + when '.so' + return + when '.rb' + logical_name + else + "#{logical_name}.rb" + end + end + private_class_method :expand_file_name + private # Return node diff --git a/spec/support/corpus.rb b/spec/support/corpus.rb index 1bd46875..ef76a9a5 100644 --- a/spec/support/corpus.rb +++ b/spec/support/corpus.rb @@ -47,6 +47,8 @@ module Corpus # otherwise # # rubocop:disable MethodLength + # rubocop:disable AbcSize + # def verify_mutation_generation checkout start = Time.now diff --git a/spec/support/fake_actor.rb b/spec/support/fake_actor.rb index 6882829d..d9296e2c 100644 --- a/spec/support/fake_actor.rb +++ b/spec/support/fake_actor.rb @@ -12,7 +12,7 @@ module FakeActor def verify(other) unless eql?(other) - raise "Got:\n#{other.inspect}\nExpected:\n#{inspect}" + fail "Got:\n#{other.inspect}\nExpected:\n#{inspect}" end block.call(other.message) if block end @@ -31,16 +31,16 @@ module FakeActor end def sending(expectation) - raise "Unexpected send: #{expectation.inspect}" if messages.empty? + fail "Unexpected send: #{expectation.inspect}" if messages.empty? expected = messages.shift expected.verify(expectation) self end def receiving(name) - raise "No message to read for #{name.inspect}" if messages.empty? + fail "No message to read for #{name.inspect}" if messages.empty? expected = messages.shift - raise "Unexpected message #{expected.inspect} for #{name.inspect}" unless expected.name.eql?(name) + fail "Unexpected message #{expected.inspect} for #{name.inspect}" unless expected.name.eql?(name) expected.message end diff --git a/spec/support/mutation_verifier.rb b/spec/support/mutation_verifier.rb index 00b2b74f..7e69aafa 100644 --- a/spec/support/mutation_verifier.rb +++ b/spec/support/mutation_verifier.rb @@ -43,6 +43,8 @@ private # # @api private # + # rubocop:disable AbcSize + # def mutation_report message = ['Original-AST:', original_node.inspect, 'Original-Source:', Unparser.unparse(original_node)] if missing.any? diff --git a/spec/support/shared_context.rb b/spec/support/shared_context.rb index 101c74c0..0de61864 100644 --- a/spec/support/shared_context.rb +++ b/spec/support/shared_context.rb @@ -8,12 +8,13 @@ module SharedContext def messages(&block) let(:message_sequence) do FakeActor::MessageSequence.new.tap do |sequence| - sequence.instance_eval(&block) + sequence.instance_eval(&block) end end end # rubocop:disable MethodLength + # rubocop:disable AbcSize def setup_shared_context let(:env) { double('env', config: config, subjects: [subject_a], mutations: mutations) } let(:job_a) { Mutant::Parallel::Job.new(index: 0, payload: mutation_a) } diff --git a/spec/unit/mutant/env_spec.rb b/spec/unit/mutant/env_spec.rb index 82310a24..0fce289a 100644 --- a/spec/unit/mutant/env_spec.rb +++ b/spec/unit/mutant/env_spec.rb @@ -8,11 +8,13 @@ RSpec.describe Mutant::Env do it 'warns via reporter' do klass = Class.new do def self.name - raise + fail end end - expected_warnings = ["Class#name from: #{klass} raised an error: RuntimeError. #{Mutant::Env::SEMANTICS_MESSAGE}"] + expected_warnings = [ + "Class#name from: #{klass} raised an error: RuntimeError. #{Mutant::Env::SEMANTICS_MESSAGE}" + ] expect { subject }.to change { config.reporter.warn_calls }.from([]).to(expected_warnings) diff --git a/spec/unit/mutant/expression_spec.rb b/spec/unit/mutant/expression_spec.rb index 9b6e23c7..e1623ab2 100644 --- a/spec/unit/mutant/expression_spec.rb +++ b/spec/unit/mutant/expression_spec.rb @@ -51,7 +51,10 @@ RSpec.describe Mutant::Expression do let(:input) { 'foo bar' } it 'raises an exception' do - expect { subject }.to raise_error(Mutant::Expression::InvalidExpressionError, 'Expression: "foo bar" is not valid') + expect { subject }.to raise_error( + Mutant::Expression::InvalidExpressionError, + 'Expression: "foo bar" is not valid' + ) end end diff --git a/spec/unit/mutant/warning_filter_spec.rb b/spec/unit/mutant/warning_filter_spec.rb index 1a1204d4..78c14bdf 100644 --- a/spec/unit/mutant/warning_filter_spec.rb +++ b/spec/unit/mutant/warning_filter_spec.rb @@ -54,7 +54,7 @@ RSpec.describe Mutant::WarningFilter do it 'executes block with warning filter enabled' do found = false object.use do - found = $stderr.is_a?(described_class) + found = $stderr.instance_of?(described_class) end expect(found).to be(true) end @@ -71,6 +71,7 @@ RSpec.describe Mutant::WarningFilter do it 'returns warnings generated within block' do warnings = object.use do + # rubocop:disable Lint/Eval eval(<<-RUBY) Class.new do def foo