From a154db8699ee2328104f5d8a0fe0e6c3c66f9d6d Mon Sep 17 00:00:00 2001 From: Harman Singh Date: Mon, 8 Oct 2018 12:06:44 +0530 Subject: [PATCH 1/4] [rubocop] Resolves some instances of Metrics/MethodLength --- .rubocop_todo.yml | 7 +------ lib/rubycritic/analysers/helpers/ast_node.rb | 18 +++++++++++------- lib/rubycritic/command_factory.rb | 19 ++++++------------- lib/rubycritic/reporter.rb | 16 ++++++---------- 4 files changed, 24 insertions(+), 36 deletions(-) diff --git a/.rubocop_todo.yml b/.rubocop_todo.yml index 0968244f..0a30f9fb 100644 --- a/.rubocop_todo.yml +++ b/.rubocop_todo.yml @@ -4,17 +4,12 @@ Metrics/AbcSize: - 'lib/rubycritic/analysers/smells/flay.rb' - 'lib/rubycritic/cli/options.rb' -# Offense count: 6 +# Offense count: 2 # Configuration parameters: CountComments. Metrics/MethodLength: Exclude: - - 'lib/rubycritic/analysers/helpers/ast_node.rb' - 'lib/rubycritic/analysers/smells/flay.rb' - - 'lib/rubycritic/command_factory.rb' - 'lib/rubycritic/core/analysed_module.rb' - - 'lib/rubycritic/reporter.rb' - - 'lib/rubycritic/revision_comparator.rb' - - 'lib/rubycritic/platforms/base.rb' # Offense count: 4 Naming/AccessorMethodName: diff --git a/lib/rubycritic/analysers/helpers/ast_node.rb b/lib/rubycritic/analysers/helpers/ast_node.rb index 67961d5e..493434f6 100644 --- a/lib/rubycritic/analysers/helpers/ast_node.rb +++ b/lib/rubycritic/analysers/helpers/ast_node.rb @@ -29,13 +29,7 @@ def get_module_names children_modules = ast_node_children.flat_map(&:get_module_names) if MODULE_TYPES.include?(type) - if children_modules.empty? - [module_name] - else - children_modules.map do |children_module| - "#{module_name}::#{children_module}" - end - end + module_names_with_children children_modules else children_modules end @@ -43,6 +37,16 @@ def get_module_names private + def module_names_with_children(children_modules) + if children_modules.empty? + [module_name] + else + children_modules.map do |children_module| + "#{module_name}::#{children_module}" + end + end + end + def module_name name_segments = [] current_node = children[0] diff --git a/lib/rubycritic/command_factory.rb b/lib/rubycritic/command_factory.rb index 94be8742..2e47869b 100644 --- a/lib/rubycritic/command_factory.rb +++ b/lib/rubycritic/command_factory.rb @@ -4,25 +4,18 @@ module RubyCritic class CommandFactory + COMMAND_CLASS_MODES = %i[version help ci compare default].freeze + def self.create(options = {}) Config.set(options) command_class(Config.mode).new(options) end def self.command_class(mode) - case mode - when :version - require 'rubycritic/commands/version' - Command::Version - when :help - require 'rubycritic/commands/help' - Command::Help - when :ci - require 'rubycritic/commands/ci' - Command::Ci - when :compare_branches - require 'rubycritic/commands/compare' - Command::Compare + mode = mode.to_s.split('_').first.to_sym + if COMMAND_CLASS_MODES.include? mode + require "rubycritic/commands/#{mode}" + Command.const_get(mode.capitalize) else require 'rubycritic/commands/default' Command::Default diff --git a/lib/rubycritic/reporter.rb b/lib/rubycritic/reporter.rb index bc049acc..c9f67e10 100644 --- a/lib/rubycritic/reporter.rb +++ b/lib/rubycritic/reporter.rb @@ -2,21 +2,17 @@ module RubyCritic module Reporter + REPORT_GENERATOR_CLASS_FORMATS = %i[json console lint].freeze + def self.generate_report(analysed_modules) report_generator_class.new(analysed_modules).generate_report end def self.report_generator_class - case Config.format - when :json - require 'rubycritic/generators/json_report' - Generator::JsonReport - when :console - require 'rubycritic/generators/console_report' - Generator::ConsoleReport - when :lint - require 'rubycritic/generators/lint_report' - Generator::LintReport + config_format = Config.format + if REPORT_GENERATOR_CLASS_FORMATS.include? config_format + require "rubycritic/generators/#{config_format}_report" + Generator.const_get("#{config_format.capitalize}Report") else require 'rubycritic/generators/html_report' Generator::HtmlReport From 2565455d73a5804f813904f07054600de1819d2d Mon Sep 17 00:00:00 2001 From: Harman Singh Date: Mon, 8 Oct 2018 12:51:47 +0530 Subject: [PATCH 2/4] [rubocop] Resolves some instances of Naming/AccessorMethodName --- .rubocop_todo.yml | 4 +--- .todo.reek | 2 +- lib/rubycritic/analysers/helpers/ast_node.rb | 6 +++--- .../analysers/helpers/modules_locator.rb | 2 +- lib/rubycritic/command_factory.rb | 2 +- lib/rubycritic/commands/compare.rb | 2 +- lib/rubycritic/commands/default.rb | 2 +- lib/rubycritic/revision_comparator.rb | 21 +++++++++---------- .../rubycritic/revision_comparator_test.rb | 13 ++++++------ 9 files changed, 25 insertions(+), 29 deletions(-) diff --git a/.rubocop_todo.yml b/.rubocop_todo.yml index 0a30f9fb..641975d0 100644 --- a/.rubocop_todo.yml +++ b/.rubocop_todo.yml @@ -11,12 +11,10 @@ Metrics/MethodLength: - 'lib/rubycritic/analysers/smells/flay.rb' - 'lib/rubycritic/core/analysed_module.rb' -# Offense count: 4 +# Offense count: 1 Naming/AccessorMethodName: Exclude: - - 'lib/rubycritic/analysers/helpers/ast_node.rb' - 'lib/rubycritic/generators/html/base.rb' - - 'lib/rubycritic/revision_comparator.rb' # Offense count: 1 Style/ClassVars: diff --git a/.todo.reek b/.todo.reek index c23200da..4ba88597 100644 --- a/.todo.reek +++ b/.todo.reek @@ -89,7 +89,7 @@ TooManyStatements: exclude: - RubyCritic::RakeTask#initialize - RubyCritic::Analyser::Complexity#run - - Parser::AST::Node#get_module_names + - Parser::AST::Node#module_names - RubyCritic::Analyser::FlaySmells#run - RubyCritic::Cli::Application#execute - RubyCritic::Cli::Options#parse diff --git a/lib/rubycritic/analysers/helpers/ast_node.rb b/lib/rubycritic/analysers/helpers/ast_node.rb index 493434f6..f58dc0a5 100644 --- a/lib/rubycritic/analysers/helpers/ast_node.rb +++ b/lib/rubycritic/analysers/helpers/ast_node.rb @@ -21,12 +21,12 @@ def recursive_children end end - def get_module_names + def module_names ast_node_children = children.select do |child| child.is_a?(Parser::AST::Node) end - children_modules = ast_node_children.flat_map(&:get_module_names) + children_modules = ast_node_children.flat_map(&:module_names) if MODULE_TYPES.include?(type) module_names_with_children children_modules @@ -67,7 +67,7 @@ def count_nodes_of_type(*) 0 end - def get_module_names + def module_names [] end end diff --git a/lib/rubycritic/analysers/helpers/modules_locator.rb b/lib/rubycritic/analysers/helpers/modules_locator.rb index a4171982..aa7d8de2 100644 --- a/lib/rubycritic/analysers/helpers/modules_locator.rb +++ b/lib/rubycritic/analysers/helpers/modules_locator.rb @@ -13,7 +13,7 @@ def first_name end def names - names = node.get_module_names + names = node.module_names if names.empty? name_from_path else diff --git a/lib/rubycritic/command_factory.rb b/lib/rubycritic/command_factory.rb index 2e47869b..61f038bd 100644 --- a/lib/rubycritic/command_factory.rb +++ b/lib/rubycritic/command_factory.rb @@ -5,7 +5,7 @@ module RubyCritic class CommandFactory COMMAND_CLASS_MODES = %i[version help ci compare default].freeze - + def self.create(options = {}) Config.set(options) command_class(Config.mode).new(options) diff --git a/lib/rubycritic/commands/compare.rb b/lib/rubycritic/commands/compare.rb index 73b332c5..2ac556ab 100644 --- a/lib/rubycritic/commands/compare.rb +++ b/lib/rubycritic/commands/compare.rb @@ -98,7 +98,7 @@ def build_details def critique(branch) module_collection = AnalysersRunner.new(paths).run Config.send(:"#{branch}_collection=", module_collection) - RevisionComparator.new(paths).set_statuses(module_collection) + RevisionComparator.new(paths).statuses = module_collection end end end diff --git a/lib/rubycritic/commands/default.rb b/lib/rubycritic/commands/default.rb index 1e69611b..e8a40650 100644 --- a/lib/rubycritic/commands/default.rb +++ b/lib/rubycritic/commands/default.rb @@ -22,7 +22,7 @@ def execute def critique analysed_modules = AnalysersRunner.new(paths).run - RevisionComparator.new(paths).set_statuses(analysed_modules) + RevisionComparator.new(paths).statuses = analysed_modules end def report(analysed_modules) diff --git a/lib/rubycritic/revision_comparator.rb b/lib/rubycritic/revision_comparator.rb index d59675f9..7d1067a8 100644 --- a/lib/rubycritic/revision_comparator.rb +++ b/lib/rubycritic/revision_comparator.rb @@ -14,17 +14,16 @@ def initialize(paths) @paths = paths end - def set_statuses(analysed_modules_now) - if Config.source_control_system.revision? - analysed_modules_before = load_cached_analysed_modules - if analysed_modules_before - SmellsStatusSetter.set( - analysed_modules_before.flat_map(&:smells), - analysed_modules_now.flat_map(&:smells) - ) - end - end - analysed_modules_now + def statuses=(analysed_modules_now) + return unless Config.source_control_system.revision? + + analysed_modules_before = load_cached_analysed_modules + return unless analysed_modules_before + + SmellsStatusSetter.set( + analysed_modules_before.flat_map(&:smells), + analysed_modules_now.flat_map(&:smells) + ) end private diff --git a/test/lib/rubycritic/revision_comparator_test.rb b/test/lib/rubycritic/revision_comparator_test.rb index 9cf95b1f..85983bea 100644 --- a/test/lib/rubycritic/revision_comparator_test.rb +++ b/test/lib/rubycritic/revision_comparator_test.rb @@ -6,7 +6,7 @@ describe RubyCritic::RevisionComparator do subject { RubyCritic::RevisionComparator.new([]) } - describe '#set_statuses' do + describe '#statuses=' do context 'in a SCS with :revision? == false' do before do RubyCritic::Config.expects(:source_control_system) @@ -16,8 +16,7 @@ it 'does not attempt to compare with previous results' do subject.expects(:load_cached_analysed_modules).never - result = subject.set_statuses([]) - result.must_equal([]) + subject.statuses = [] end end @@ -36,12 +35,12 @@ it 'does not load them' do RubyCritic::Serializer.expects(:new).never - subject.set_statuses([]) + subject.statuses = [] end it 'does not invoke RubyCritic::SmellsStatusSetter' do RubyCritic::SmellsStatusSetter.expects(:set).never - subject.set_statuses([]) + subject.statuses = [] end end @@ -53,12 +52,12 @@ end it 'loads them' do - subject.set_statuses([]) + subject.statuses = [] end it 'invokes RubyCritic::SmellsStatusSetter' do RubyCritic::SmellsStatusSetter.expects(:set).once - subject.set_statuses([]) + subject.statuses = [] end end end From 8d8e6dd491f916db63b50c6f4e342a2655dc0334 Mon Sep 17 00:00:00 2001 From: Harman Singh Date: Mon, 8 Oct 2018 13:28:34 +0530 Subject: [PATCH 3/4] [rubocop] Resolves instances of Style/SpecialGlobalVars --- .rubocop_todo.yml | 7 ------- lib/rubycritic/source_control_systems/base.rb | 1 + lib/rubycritic/source_control_systems/git.rb | 4 ++-- lib/rubycritic/source_control_systems/mercurial.rb | 2 +- 4 files changed, 4 insertions(+), 10 deletions(-) diff --git a/.rubocop_todo.yml b/.rubocop_todo.yml index 641975d0..821846ec 100644 --- a/.rubocop_todo.yml +++ b/.rubocop_todo.yml @@ -21,13 +21,6 @@ Style/ClassVars: Exclude: - 'lib/rubycritic/source_control_systems/base.rb' -# Offense count: 3 -# Cop supports --auto-correct. -# Configuration parameters: EnforcedStyle, SupportedStyles. -# SupportedStyles: use_perl_names, use_english_names -Style/SpecialGlobalVars: - Enabled: false - # Offense count: 1 Style/MethodMissing: Exclude: diff --git a/lib/rubycritic/source_control_systems/base.rb b/lib/rubycritic/source_control_systems/base.rb index faa000cd..f333d2f6 100644 --- a/lib/rubycritic/source_control_systems/base.rb +++ b/lib/rubycritic/source_control_systems/base.rb @@ -1,5 +1,6 @@ # frozen_string_literal: true +require 'English' require 'shellwords' module RubyCritic diff --git a/lib/rubycritic/source_control_systems/git.rb b/lib/rubycritic/source_control_systems/git.rb index 54c7b7b6..5aa796e2 100644 --- a/lib/rubycritic/source_control_systems/git.rb +++ b/lib/rubycritic/source_control_systems/git.rb @@ -21,7 +21,7 @@ def git(arg) end def self.supported? - git('branch 2>&1') && $?.success? + git('branch 2>&1') && $CHILD_STATUS.success? end def self.to_s @@ -37,7 +37,7 @@ def date_of_last_commit(path) end def revision? - head_reference && $?.success? + head_reference && $CHILD_STATUS.success? end def head_reference diff --git a/lib/rubycritic/source_control_systems/mercurial.rb b/lib/rubycritic/source_control_systems/mercurial.rb index 807a135d..0303a355 100644 --- a/lib/rubycritic/source_control_systems/mercurial.rb +++ b/lib/rubycritic/source_control_systems/mercurial.rb @@ -6,7 +6,7 @@ class Mercurial < Base register_system def self.supported? - `hg verify 2>&1` && $?.success? + `hg verify 2>&1` && $CHILD_STATUS.success? end def self.to_s From 083c2283894c396c1fdf35ade40aa5a968e00694 Mon Sep 17 00:00:00 2001 From: Harman Singh Date: Mon, 8 Oct 2018 13:38:18 +0530 Subject: [PATCH 4/4] [tests] Updates require 'mocha/mini_test' --- test/test_helper.rb | 2 +- 1 file changed, 1 insertion(+), 1 deletion(-) diff --git a/test/test_helper.rb b/test/test_helper.rb index df34f9b3..1111cd4f 100644 --- a/test/test_helper.rb +++ b/test/test_helper.rb @@ -3,7 +3,7 @@ require 'minitest/autorun' require 'minitest/around/spec' require 'minitest/pride' -require 'mocha/mini_test' +require 'mocha/minitest' require 'ostruct' require 'diff/lcs'