Fix RSpec/FilePath when checking an empty class #1000
Conversation
Remember that `RSpec/FilePath` checks *all* files - not only **/*_spec.rb. And when some class defines e.g. an empty class, the `TopLevelGroup` module would raise an error. Fixes #999.
bquorning
changed the title
RSpec/FilePath when checking an empty class
|
Congratulations on the 1K pull requests (and issues) 🎉 |
|
I hope it’s OK to ignore Code Climate’s report on added complexity… |
| @@ -204,6 +204,14 @@ | |||
| RUBY | |||
| end | |||
|
|
|||
| # RSpec/FilePath runs on all files - not only **/*_spec.rb | |||
There was a problem hiding this comment.
Wondering, why so?
For some reason relevant_rubocop_rspec_file? evaluates to true for it.
There was a problem hiding this comment.
Ah, I see it
def relevant_rubocop_rspec_file?(_file)
true
end| @@ -38,7 +38,9 @@ def top_level_group?(node) | |||
| end | |||
|
|
|||
| def top_level_nodes(node) | |||
| if node.begin_type? | |||
| if node.nil? | |||
| [] | |||
There was a problem hiding this comment.
I remember we were discussing this with Maxim and settled on
return unless root_nodeDo you think both checks are necessary?
There was a problem hiding this comment.
As you mention, we already have return unless root_node inside #on_new_investigation. The problem happens when the root node is an empty module or class:
elsif node.module_type? || node.class_type?
top_level_nodes(node.body)There was a problem hiding this comment.
I mean, we could move the check into the elsif body:
elsif node.module_type? || node.class_type?
node.body.nil? ? [] : top_level_nodes(node.body)or
elsif node.module_type? || node.class_type?
if node.body
top_level_nodes(node.body)
else
[]
end
🎉 |
Remember that
RSpec/FilePathchecks all files - not only **/*_spec.rb. And when some class defines e.g. an empty class, theTopLevelGroupmodule would raise an error.Fixes #999.
Before submitting the PR make sure the following are checked:
master(if not - rebase it).CHANGELOG.mdif the new code introduces user-observable changes.bundle exec rake) passes (be sure to run this locally, since it may produce updated documentation that you will need to commit).