From 5d55e6ee554a3c0af6069b8a0845e9ef0951fbd8 Mon Sep 17 00:00:00 2001 From: Robert Fletcher Date: Fri, 19 Jun 2020 12:17:09 -0700 Subject: [PATCH] Add mixins folder This moves several files to a new `mixins/` folder. I was going to add some new macros, per [this comment][1], but thought it might clutter things up a bit to put them in the root folder. I thought of adding a `macros/` folder, but decided to emulate the [rubocop source][2] instead. [1]: https://github.com/rubocop-hq/rubocop-rspec/pull/934#issuecomment-643813427 [2]: https://github.com/rubocop-hq/rubocop/tree/master/lib/rubocop/cop/mixin --- lib/rubocop-rspec.rb | 11 ++-- lib/rubocop/cop/rspec/describe_class.rb | 2 +- lib/rubocop/cop/rspec/describe_method.rb | 2 +- .../cop/rspec/empty_line_after_example.rb | 2 +- .../rspec/empty_line_after_example_group.rb | 2 +- .../cop/rspec/empty_line_after_final_let.rb | 2 +- .../cop/rspec/empty_line_after_hook.rb | 2 +- .../cop/rspec/empty_line_after_subject.rb | 2 +- lib/rubocop/cop/rspec/file_path.rb | 2 +- lib/rubocop/cop/rspec/instance_variable.rb | 2 +- .../cop/rspec/mixin/empty_line_separation.rb | 51 ++++++++++++++++ .../cop/rspec/mixin/final_end_location.rb | 19 ++++++ .../cop/rspec/mixin/top_level_describe.rb | 54 ++++++++++++++++ .../cop/rspec/mixin/top_level_group.rb | 61 +++++++++++++++++++ lib/rubocop/cop/rspec/mixin/variable.rb | 18 ++++++ lib/rubocop/cop/rspec/multiple_describes.rb | 2 +- .../cop/rspec/multiple_memoized_helpers.rb | 2 +- lib/rubocop/cop/rspec/nested_groups.rb | 2 +- lib/rubocop/cop/rspec/subject_stub.rb | 2 +- lib/rubocop/cop/rspec/variable_definition.rb | 2 +- lib/rubocop/cop/rspec/variable_name.rb | 2 +- lib/rubocop/rspec/corrector/move_node.rb | 2 +- lib/rubocop/rspec/empty_line_separation.rb | 48 --------------- lib/rubocop/rspec/final_end_location.rb | 17 ------ lib/rubocop/rspec/top_level_group.rb | 57 ----------------- lib/rubocop/rspec/variable.rb | 16 ----- 26 files changed, 226 insertions(+), 158 deletions(-) create mode 100644 lib/rubocop/cop/rspec/mixin/empty_line_separation.rb create mode 100644 lib/rubocop/cop/rspec/mixin/final_end_location.rb create mode 100644 lib/rubocop/cop/rspec/mixin/top_level_describe.rb create mode 100644 lib/rubocop/cop/rspec/mixin/top_level_group.rb create mode 100644 lib/rubocop/cop/rspec/mixin/variable.rb delete mode 100644 lib/rubocop/rspec/empty_line_separation.rb delete mode 100644 lib/rubocop/rspec/final_end_location.rb delete mode 100644 lib/rubocop/rspec/top_level_group.rb delete mode 100644 lib/rubocop/rspec/variable.rb diff --git a/lib/rubocop-rspec.rb b/lib/rubocop-rspec.rb index 2491cc2f2..652c8eeb5 100644 --- a/lib/rubocop-rspec.rb +++ b/lib/rubocop-rspec.rb @@ -12,17 +12,20 @@ require_relative 'rubocop/rspec/wording' require_relative 'rubocop/rspec/language' require_relative 'rubocop/rspec/language/node_pattern' -require_relative 'rubocop/rspec/top_level_group' + +require_relative 'rubocop/cop/rspec/mixin/top_level_describe' +require_relative 'rubocop/cop/rspec/mixin/top_level_group' +require_relative 'rubocop/cop/rspec/mixin/variable' +require_relative 'rubocop/cop/rspec/mixin/final_end_location' +require_relative 'rubocop/cop/rspec/mixin/empty_line_separation' + require_relative 'rubocop/rspec/concept' require_relative 'rubocop/rspec/example_group' require_relative 'rubocop/rspec/example' require_relative 'rubocop/rspec/hook' -require_relative 'rubocop/rspec/variable' require_relative 'rubocop/cop/rspec/base' require_relative 'rubocop/rspec/align_let_brace' require_relative 'rubocop/rspec/factory_bot' -require_relative 'rubocop/rspec/final_end_location' -require_relative 'rubocop/rspec/empty_line_separation' require_relative 'rubocop/rspec/corrector/move_node' RuboCop::RSpec::Inject.defaults! diff --git a/lib/rubocop/cop/rspec/describe_class.rb b/lib/rubocop/cop/rspec/describe_class.rb index e007cb245..45bb737c4 100644 --- a/lib/rubocop/cop/rspec/describe_class.rb +++ b/lib/rubocop/cop/rspec/describe_class.rb @@ -35,7 +35,7 @@ module RSpec # describe "A feature example", type: :feature do # end class DescribeClass < Base - include RuboCop::RSpec::TopLevelGroup + include TopLevelGroup MSG = 'The first argument to describe should be '\ 'the class or module being tested.' diff --git a/lib/rubocop/cop/rspec/describe_method.rb b/lib/rubocop/cop/rspec/describe_method.rb index cfc1e8148..2ff64e760 100644 --- a/lib/rubocop/cop/rspec/describe_method.rb +++ b/lib/rubocop/cop/rspec/describe_method.rb @@ -17,7 +17,7 @@ module RSpec # describe MyClass, '.my_class_method' do # end class DescribeMethod < Base - include RuboCop::RSpec::TopLevelGroup + include TopLevelGroup MSG = 'The second argument to describe should be the method '\ "being tested. '#instance' or '.class'." diff --git a/lib/rubocop/cop/rspec/empty_line_after_example.rb b/lib/rubocop/cop/rspec/empty_line_after_example.rb index e0d783739..912225188 100644 --- a/lib/rubocop/cop/rspec/empty_line_after_example.rb +++ b/lib/rubocop/cop/rspec/empty_line_after_example.rb @@ -43,7 +43,7 @@ module RSpec # class EmptyLineAfterExample < Base extend AutoCorrector - include RuboCop::RSpec::EmptyLineSeparation + include EmptyLineSeparation MSG = 'Add an empty line after `%s`.' diff --git a/lib/rubocop/cop/rspec/empty_line_after_example_group.rb b/lib/rubocop/cop/rspec/empty_line_after_example_group.rb index e02fb73ba..e0f89f885 100644 --- a/lib/rubocop/cop/rspec/empty_line_after_example_group.rb +++ b/lib/rubocop/cop/rspec/empty_line_after_example_group.rb @@ -25,7 +25,7 @@ module RSpec # class EmptyLineAfterExampleGroup < Base extend AutoCorrector - include RuboCop::RSpec::EmptyLineSeparation + include EmptyLineSeparation MSG = 'Add an empty line after `%s`.' diff --git a/lib/rubocop/cop/rspec/empty_line_after_final_let.rb b/lib/rubocop/cop/rspec/empty_line_after_final_let.rb index 9bfc56391..1f91190ed 100644 --- a/lib/rubocop/cop/rspec/empty_line_after_final_let.rb +++ b/lib/rubocop/cop/rspec/empty_line_after_final_let.rb @@ -18,7 +18,7 @@ module RSpec # it { does_something } class EmptyLineAfterFinalLet < Base extend AutoCorrector - include RuboCop::RSpec::EmptyLineSeparation + include EmptyLineSeparation MSG = 'Add an empty line after the last `%s`.' diff --git a/lib/rubocop/cop/rspec/empty_line_after_hook.rb b/lib/rubocop/cop/rspec/empty_line_after_hook.rb index 4728ffdcb..bf2ae16f6 100644 --- a/lib/rubocop/cop/rspec/empty_line_after_hook.rb +++ b/lib/rubocop/cop/rspec/empty_line_after_hook.rb @@ -35,7 +35,7 @@ module RSpec # class EmptyLineAfterHook < Base extend AutoCorrector - include RuboCop::RSpec::EmptyLineSeparation + include EmptyLineSeparation MSG = 'Add an empty line after `%s`.' diff --git a/lib/rubocop/cop/rspec/empty_line_after_subject.rb b/lib/rubocop/cop/rspec/empty_line_after_subject.rb index bed4d1606..dd8beaaa1 100644 --- a/lib/rubocop/cop/rspec/empty_line_after_subject.rb +++ b/lib/rubocop/cop/rspec/empty_line_after_subject.rb @@ -16,7 +16,7 @@ module RSpec # let(:foo) { bar } class EmptyLineAfterSubject < Base extend AutoCorrector - include RuboCop::RSpec::EmptyLineSeparation + include EmptyLineSeparation MSG = 'Add an empty line after `%s`.' diff --git a/lib/rubocop/cop/rspec/file_path.rb b/lib/rubocop/cop/rspec/file_path.rb index 544095dce..9409ad4b9 100644 --- a/lib/rubocop/cop/rspec/file_path.rb +++ b/lib/rubocop/cop/rspec/file_path.rb @@ -57,7 +57,7 @@ module RSpec # my_class_spec.rb # describe MyClass, '#method' # class FilePath < Base - include RuboCop::RSpec::TopLevelGroup + include TopLevelGroup MSG = 'Spec path should end with `%s`.' diff --git a/lib/rubocop/cop/rspec/instance_variable.rb b/lib/rubocop/cop/rspec/instance_variable.rb index 4e1ad1b6f..76ebb7ad6 100644 --- a/lib/rubocop/cop/rspec/instance_variable.rb +++ b/lib/rubocop/cop/rspec/instance_variable.rb @@ -47,7 +47,7 @@ module RSpec # end # class InstanceVariable < Base - include RuboCop::RSpec::TopLevelGroup + include TopLevelGroup MSG = 'Avoid instance variables – use let, ' \ 'a method call, or a local variable (if possible).' diff --git a/lib/rubocop/cop/rspec/mixin/empty_line_separation.rb b/lib/rubocop/cop/rspec/mixin/empty_line_separation.rb new file mode 100644 index 000000000..5d48603f2 --- /dev/null +++ b/lib/rubocop/cop/rspec/mixin/empty_line_separation.rb @@ -0,0 +1,51 @@ +# frozen_string_literal: true + +module RuboCop + module Cop + module RSpec + # Helps determine the offending location if there is not an empty line + # following the node. Allows comments to follow directly after. + module EmptyLineSeparation + include FinalEndLocation + include RangeHelp + + def missing_separating_line_offense(node) + return if last_child?(node) + + missing_separating_line(node) do |location| + msg = yield(node.method_name) + add_offense(location, message: msg) do |corrector| + corrector.insert_after(location.end, "\n") + end + end + end + + def missing_separating_line(node) + line = final_end_location(node).line + + line += 1 while comment_line?(processed_source[line]) + + return if processed_source[line].blank? + + yield offending_loc(line) + end + + def offending_loc(last_line) + offending_line = processed_source[last_line - 1] + + content_length = offending_line.lstrip.length + start = offending_line.length - content_length + + source_range(processed_source.buffer, + last_line, start, content_length) + end + + def last_child?(node) + return true unless node.parent&.begin_type? + + node.equal?(node.parent.children.last) + end + end + end + end +end diff --git a/lib/rubocop/cop/rspec/mixin/final_end_location.rb b/lib/rubocop/cop/rspec/mixin/final_end_location.rb new file mode 100644 index 000000000..526049f5d --- /dev/null +++ b/lib/rubocop/cop/rspec/mixin/final_end_location.rb @@ -0,0 +1,19 @@ +# frozen_string_literal: true + +module RuboCop + module Cop + module RSpec + # Helps find the true end location of nodes which might contain heredocs. + module FinalEndLocation + def final_end_location(start_node) + heredoc_endings = + start_node.each_node(:str, :dstr, :xstr) + .select(&:heredoc?) + .map { |node| node.loc.heredoc_end } + + [start_node.loc.end, *heredoc_endings].max_by(&:line) + end + end + end + end +end diff --git a/lib/rubocop/cop/rspec/mixin/top_level_describe.rb b/lib/rubocop/cop/rspec/mixin/top_level_describe.rb new file mode 100644 index 000000000..e7cbe86d2 --- /dev/null +++ b/lib/rubocop/cop/rspec/mixin/top_level_describe.rb @@ -0,0 +1,54 @@ +# frozen_string_literal: true + +module RuboCop + module Cop + module RSpec + # Helper methods for top level describe cops + module TopLevelDescribe + extend NodePattern::Macros + + def on_send(node) + return unless respond_to?(:on_top_level_describe) + return unless top_level_describe?(node) + + on_top_level_describe(node, node.arguments) + end + + private + + def top_level_describe?(node) + return false unless node.method?(:describe) + + top_level_nodes.include?(node) + end + + def top_level_nodes + nodes = describe_statement_children(root_node) + # If we have no top level describe statements, we need to check any + # blocks on the top level (e.g. after a require). + if nodes.empty? + nodes = root_node.each_child_node(:block).flat_map do |child| + describe_statement_children(child) + end + end + + nodes + end + + def root_node + processed_source.ast + end + + def single_top_level_describe? + top_level_nodes.one? + end + + def describe_statement_children(node) + node.each_child_node(:send).select do |element| + element.method?(:describe) + end + end + end + end + end +end diff --git a/lib/rubocop/cop/rspec/mixin/top_level_group.rb b/lib/rubocop/cop/rspec/mixin/top_level_group.rb new file mode 100644 index 000000000..7407b0bb7 --- /dev/null +++ b/lib/rubocop/cop/rspec/mixin/top_level_group.rb @@ -0,0 +1,61 @@ +# frozen_string_literal: true + +module RuboCop + module Cop + module RSpec + # Helper methods for top level example group cops + module TopLevelGroup + extend RuboCop::NodePattern::Macros + include RuboCop::RSpec::Language + + def_node_matcher :example_or_shared_group?, + (ExampleGroups::ALL + SharedGroups::ALL).block_pattern + + def on_new_investigation + super + + return unless root_node + + top_level_groups.each do |node| + on_top_level_example_group(node) if example_group?(node) + on_top_level_group(node) + end + end + + def top_level_groups + @top_level_groups ||= + top_level_nodes(root_node).select do |node| + example_or_shared_group?(node) + end + end + + private + + # Dummy methods to be overridden in the consumer + def on_top_level_example_group(_node); end + + def on_top_level_group(_node); end + + def top_level_group?(node) + top_level_groups.include?(node) + end + + def top_level_nodes(node) + if node.nil? + [] + elsif node.begin_type? + node.children + elsif node.module_type? || node.class_type? + top_level_nodes(node.body) + else + [node] + end + end + + def root_node + processed_source.ast + end + end + end + end +end diff --git a/lib/rubocop/cop/rspec/mixin/variable.rb b/lib/rubocop/cop/rspec/mixin/variable.rb new file mode 100644 index 000000000..c06161690 --- /dev/null +++ b/lib/rubocop/cop/rspec/mixin/variable.rb @@ -0,0 +1,18 @@ +# frozen_string_literal: true + +module RuboCop + module Cop + module RSpec + # Helps check offenses with variable definitions + module Variable + include RuboCop::RSpec::Language + extend RuboCop::NodePattern::Macros + + def_node_matcher :variable_definition?, <<~PATTERN + (send nil? #{(Helpers::ALL + Subject::ALL).node_pattern_union} + $({sym str dsym dstr} ...) ...) + PATTERN + end + end + end +end diff --git a/lib/rubocop/cop/rspec/multiple_describes.rb b/lib/rubocop/cop/rspec/multiple_describes.rb index 9ca9fb9ae..14c535f45 100644 --- a/lib/rubocop/cop/rspec/multiple_describes.rb +++ b/lib/rubocop/cop/rspec/multiple_describes.rb @@ -23,7 +23,7 @@ module RSpec # end # end class MultipleDescribes < Base - include RuboCop::RSpec::TopLevelGroup + include TopLevelGroup MSG = 'Do not use multiple top-level example groups - '\ 'try to nest them.' diff --git a/lib/rubocop/cop/rspec/multiple_memoized_helpers.rb b/lib/rubocop/cop/rspec/multiple_memoized_helpers.rb index 4ad7a0cf2..2489a8632 100644 --- a/lib/rubocop/cop/rspec/multiple_memoized_helpers.rb +++ b/lib/rubocop/cop/rspec/multiple_memoized_helpers.rb @@ -85,7 +85,7 @@ module RSpec # class MultipleMemoizedHelpers < Base include ConfigurableMax - include RuboCop::RSpec::Variable + include Variable MSG = 'Example group has too many memoized helpers [%d/%d]' diff --git a/lib/rubocop/cop/rspec/nested_groups.rb b/lib/rubocop/cop/rspec/nested_groups.rb index 44301d789..c9a6e9e5f 100644 --- a/lib/rubocop/cop/rspec/nested_groups.rb +++ b/lib/rubocop/cop/rspec/nested_groups.rb @@ -87,7 +87,7 @@ module RSpec # class NestedGroups < Base include ConfigurableMax - include RuboCop::RSpec::TopLevelGroup + include TopLevelGroup MSG = 'Maximum example group nesting exceeded [%d/%d].' diff --git a/lib/rubocop/cop/rspec/subject_stub.rb b/lib/rubocop/cop/rspec/subject_stub.rb index 6d061ba4f..d2d247594 100644 --- a/lib/rubocop/cop/rspec/subject_stub.rb +++ b/lib/rubocop/cop/rspec/subject_stub.rb @@ -22,7 +22,7 @@ module RSpec # end # class SubjectStub < Base - include RuboCop::RSpec::TopLevelGroup + include TopLevelGroup MSG = 'Do not stub methods of the object under test.' diff --git a/lib/rubocop/cop/rspec/variable_definition.rb b/lib/rubocop/cop/rspec/variable_definition.rb index 91485d3f3..764168e94 100644 --- a/lib/rubocop/cop/rspec/variable_definition.rb +++ b/lib/rubocop/cop/rspec/variable_definition.rb @@ -24,7 +24,7 @@ module RSpec # let('user_name') { 'Adam' } class VariableDefinition < Base include ConfigurableEnforcedStyle - include RuboCop::RSpec::Variable + include Variable MSG = 'Use %