Skip to content
New issue

Have a question about this project? Sign up for a free GitHub account to open an issue and contact its maintainers and the community.

By clicking “Sign up for GitHub”, you agree to our terms of service and privacy statement. We’ll occasionally send you account related emails.

Already on GitHub? Sign in to your account

Add mixins folder #942

Closed
wants to merge 1 commit into from
Closed
Show file tree
Hide file tree
Changes from all commits
Commits
File filter

Filter by extension

Filter by extension

Conversations
Failed to load comments.
Jump to
Jump to file
Failed to load files.
Diff view
Diff view
12 changes: 7 additions & 5 deletions lib/rubocop-rspec.rb
Expand Up @@ -9,21 +9,23 @@
require_relative 'rubocop/rspec/version'
require_relative 'rubocop/rspec/inject'
require_relative 'rubocop/rspec/node'
require_relative 'rubocop/rspec/top_level_describe'
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'
Copy link
Collaborator

Choose a reason for hiding this comment

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

When we change the folder like this, shouldn’t we also change the namespace? E.g. RuboCop::RSpec::TopLevelDescribe -> RuboCop::Cop::RSpec::TopLevelDescribe.

Copy link
Member

Choose a reason for hiding this comment

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

Agree.

@mockdeep We've just renamed RuboCop::Cop::RSpec::Cop to RuboCop::Cop::RSpec::Base following RuboCop core's naming.
If we happen to move classes/modules around namespaces, and if it happens those are used by custom cops, we'll break them with a minor version update.
So for Base, we've decided to keep the old class, Cop as well until 2.0.
I don't see how it would be possible to release this change with namespace before we release 2.0 without similar breakage risks.

AFAI understand #863 depends on this pull request. But if this is going to be released in 2.0, that means that #863 will only be released as enabled in 3.0, until then it will be pending.

So I propose two options for #863:

  • untangle it from this pull request, and release separately
  • keep it dependent on this pull request and release post-2.0

Please let me know what you think.

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/cop'
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!
Expand Down
2 changes: 1 addition & 1 deletion lib/rubocop/cop/rspec/describe_class.rb
Expand Up @@ -22,7 +22,7 @@ module RSpec
# describe "A feature example", type: :feature do
# end
class DescribeClass < Cop
include RuboCop::RSpec::TopLevelDescribe
include TopLevelDescribe

MSG = 'The first argument to describe should be '\
'the class or module being tested.'
Expand Down
2 changes: 1 addition & 1 deletion lib/rubocop/cop/rspec/describe_method.rb
Expand Up @@ -17,7 +17,7 @@ module RSpec
# describe MyClass, '.my_class_method' do
# end
class DescribeMethod < Cop
include RuboCop::RSpec::TopLevelDescribe
include TopLevelDescribe

MSG = 'The second argument to describe should be the method '\
"being tested. '#instance' or '.class'."
Expand Down
2 changes: 1 addition & 1 deletion lib/rubocop/cop/rspec/empty_line_after_example.rb
Expand Up @@ -43,7 +43,7 @@ module RSpec
#
class EmptyLineAfterExample < Cop
extend AutoCorrector
include RuboCop::RSpec::EmptyLineSeparation
include EmptyLineSeparation

MSG = 'Add an empty line after `%<example>s`.'

Expand Down
2 changes: 1 addition & 1 deletion lib/rubocop/cop/rspec/empty_line_after_example_group.rb
Expand Up @@ -25,7 +25,7 @@ module RSpec
#
class EmptyLineAfterExampleGroup < Cop
extend AutoCorrector
include RuboCop::RSpec::EmptyLineSeparation
include EmptyLineSeparation

MSG = 'Add an empty line after `%<example_group>s`.'

Expand Down
2 changes: 1 addition & 1 deletion lib/rubocop/cop/rspec/empty_line_after_final_let.rb
Expand Up @@ -18,7 +18,7 @@ module RSpec
# it { does_something }
class EmptyLineAfterFinalLet < Cop
extend AutoCorrector
include RuboCop::RSpec::EmptyLineSeparation
include EmptyLineSeparation

MSG = 'Add an empty line after the last `%<let>s`.'

Expand Down
2 changes: 1 addition & 1 deletion lib/rubocop/cop/rspec/empty_line_after_hook.rb
Expand Up @@ -35,7 +35,7 @@ module RSpec
#
class EmptyLineAfterHook < Cop
extend AutoCorrector
include RuboCop::RSpec::EmptyLineSeparation
include EmptyLineSeparation

MSG = 'Add an empty line after `%<hook>s`.'

Expand Down
2 changes: 1 addition & 1 deletion lib/rubocop/cop/rspec/empty_line_after_subject.rb
Expand Up @@ -16,7 +16,7 @@ module RSpec
# let(:foo) { bar }
class EmptyLineAfterSubject < Cop
extend AutoCorrector
include RuboCop::RSpec::EmptyLineSeparation
include EmptyLineSeparation

MSG = 'Add an empty line after `%<subject>s`.'

Expand Down
2 changes: 1 addition & 1 deletion lib/rubocop/cop/rspec/file_path.rb
Expand Up @@ -57,7 +57,7 @@ module RSpec
# my_class_spec.rb # describe MyClass, '#method'
#
class FilePath < Cop
include RuboCop::RSpec::TopLevelDescribe
include TopLevelDescribe

MSG = 'Spec path should end with `%<suffix>s`.'

Expand Down
2 changes: 1 addition & 1 deletion lib/rubocop/cop/rspec/instance_variable.rb
Expand Up @@ -47,7 +47,7 @@ module RSpec
# end
#
class InstanceVariable < Cop
include RuboCop::RSpec::TopLevelGroup
include TopLevelGroup

MSG = 'Avoid instance variables – use let, ' \
'a method call, or a local variable (if possible).'
Expand Down
51 changes: 51 additions & 0 deletions 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
19 changes: 19 additions & 0 deletions 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
54 changes: 54 additions & 0 deletions 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_name == :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_name == :describe
end
end
end
end
end
end
46 changes: 46 additions & 0 deletions lib/rubocop/cop/rspec/mixin/top_level_group.rb
@@ -0,0 +1,46 @@
# 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_block(node)
return unless respond_to?(:on_top_level_group)
return unless top_level_group?(node)

on_top_level_group(node)
end

private

def top_level_group?(node)
top_level_groups.include?(node)
end

def top_level_groups
@top_level_groups ||=
top_level_nodes.select { |n| example_or_shared_group?(n) }
end

def top_level_nodes
if root_node.begin_type?
root_node.children
else
[root_node]
end
end

def root_node
processed_source.ast
end
end
end
end
end
18 changes: 18 additions & 0 deletions 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 #{RSPEC} #{(Helpers::ALL + Subject::ALL).node_pattern_union}
$({sym str dsym dstr} ...) ...)
PATTERN
end
end
end
end
2 changes: 1 addition & 1 deletion lib/rubocop/cop/rspec/multiple_describes.rb
Expand Up @@ -23,7 +23,7 @@ module RSpec
# end
# end
class MultipleDescribes < Cop
include RuboCop::RSpec::TopLevelDescribe
include TopLevelDescribe

MSG = 'Do not use multiple top level describes - '\
'try to nest them.'
Expand Down
2 changes: 1 addition & 1 deletion lib/rubocop/cop/rspec/nested_groups.rb
Expand Up @@ -87,7 +87,7 @@ module RSpec
#
class NestedGroups < Cop
include ConfigurableMax
include RuboCop::RSpec::TopLevelDescribe
include TopLevelDescribe

MSG = 'Maximum example group nesting exceeded [%<total>d/%<max>d].'

Expand Down
2 changes: 1 addition & 1 deletion lib/rubocop/cop/rspec/subject_stub.rb
Expand Up @@ -22,7 +22,7 @@ module RSpec
# end
#
class SubjectStub < Cop
include RuboCop::RSpec::TopLevelGroup
include TopLevelGroup

MSG = 'Do not stub methods of the object under test.'

Expand Down
2 changes: 1 addition & 1 deletion lib/rubocop/cop/rspec/variable_definition.rb
Expand Up @@ -24,7 +24,7 @@ module RSpec
# subject('user') { create_user }
class VariableDefinition < Cop
include ConfigurableEnforcedStyle
include RuboCop::RSpec::Variable
include Variable

MSG = 'Use %<style>s for variable names.'

Expand Down
2 changes: 1 addition & 1 deletion lib/rubocop/cop/rspec/variable_name.rb
Expand Up @@ -24,7 +24,7 @@ module RSpec
# subject(:userName) { 'Adam' }
class VariableName < Cop
include ConfigurableNaming
include RuboCop::RSpec::Variable
include Variable

MSG = 'Use %<style>s for variable names.'

Expand Down
2 changes: 1 addition & 1 deletion lib/rubocop/rspec/corrector/move_node.rb
Expand Up @@ -6,7 +6,7 @@ module Corrector
# Helper methods to move a node
class MoveNode
include RuboCop::Cop::RangeHelp
include RuboCop::RSpec::FinalEndLocation
include RuboCop::Cop::RSpec::FinalEndLocation

attr_reader :original, :corrector, :processed_source

Expand Down