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

Fix false positive for relative file path runs with long namespaces (fixes #1091) #1099

Merged
merged 1 commit into from
Dec 13, 2020
Merged
Show file tree
Hide file tree
Changes from all commits
Commits
File filter

Filter by extension

Filter by extension

Conversations
Failed to load comments.
Loading
Jump to
Jump to file
Failed to load files.
Loading
Diff view
Diff view
3 changes: 3 additions & 0 deletions CHANGELOG.md
Original file line number Diff line number Diff line change
Expand Up @@ -2,6 +2,8 @@

## Master (Unreleased)

* Fix `RSpec/FilePath` false positive for relative file path runs with long namespaces. ([@ahukkanen][])

## 2.0.1 (2020-12-02)

* Fixed infinite loop in `RSpec/ExpectActual` autocorrection when both expected and actual values are literals. ([@Darhazer][])
Expand Down Expand Up @@ -590,3 +592,4 @@ Compatibility release so users can upgrade RuboCop to 0.51.0. No new features.
[@Rafix02]: https://github.com/Rafix02
[@PhilCoggins]: https://github.com/PhilCoggins
[@sl4vr]: https://github.com/sl4vr
[@ahukkanen]: https://github.com/ahukkanen
39 changes: 23 additions & 16 deletions lib/rubocop/cop/rspec/file_path.rb
Original file line number Diff line number Diff line change
Expand Up @@ -82,30 +82,39 @@ def on_top_level_example_group(node)
private

def ensure_correct_file_path(send_node, described_class, arguments)
glob = glob_for(described_class, arguments.first)
return if filename_ends_with?(glob)

add_offense(send_node, message: format(MSG, suffix: glob))
pattern = pattern_for(described_class, arguments.first)
return if filename_ends_with?(pattern)

# For the suffix shown in the offense message, modify the regular
# expression pattern to resemble a glob pattern for clearer error
# messages.
offense_suffix = pattern.gsub('.*', '*').sub('[^/]', '')
.sub('\.', '.')
add_offense(send_node, message: format(MSG, suffix: offense_suffix))
end

def routing_spec?(args)
args.any?(&method(:routing_metadata?))
end

def glob_for(described_class, method_name)
return glob_for_spec_suffix_only? if spec_suffix_only?
def pattern_for(described_class, method_name)
return pattern_for_spec_suffix_only? if spec_suffix_only?

"#{expected_path(described_class)}#{name_glob(method_name)}*_spec.rb"
[
expected_path(described_class),
name_pattern(method_name),
'[^/]*_spec\.rb'
].join
end

def glob_for_spec_suffix_only?
'*_spec.rb'
def pattern_for_spec_suffix_only?
'.*_spec\.rb'
end

def name_glob(method_name)
def name_pattern(method_name)
return unless method_name&.str_type?

"*#{method_name.str_content.gsub(/\W/, '')}" unless ignore_methods?
".*#{method_name.str_content.gsub(/\W/, '')}" unless ignore_methods?
pirj marked this conversation as resolved.
Show resolved Hide resolved
end

def expected_path(constant)
Expand All @@ -131,11 +140,9 @@ def ignore_methods?
cop_config['IgnoreMethods']
end

def filename_ends_with?(glob)
filename =
RuboCop::PathUtil.relative_path(processed_source.buffer.name)
.gsub('../', '')
File.fnmatch?("*#{glob}", filename)
def filename_ends_with?(pattern)
filename = File.expand_path(processed_source.buffer.name)
filename.match?("#{pattern}$")
end

def relevant_rubocop_rspec_file?(_file)
Expand Down
40 changes: 25 additions & 15 deletions spec/rubocop/cop/rspec/file_path_spec.rb
Original file line number Diff line number Diff line change
Expand Up @@ -64,6 +64,13 @@
RUBY
end

it 'registers an offense for a file without the .rb extension' do
expect_offense(<<-RUBY, 'spec/models/user_specxrb')
describe User do; end
^^^^^^^^^^^^^ Spec path should end with `user*_spec.rb`.
RUBY
end

it 'ignores shared examples' do
expect_no_offenses(<<-RUBY, 'spec/models/user.rb')
shared_examples_for 'foo' do; end
Expand Down Expand Up @@ -183,30 +190,26 @@
RUBY
end

it 'uses relative path' do
allow(RuboCop::PathUtil)
.to receive(:relative_path).and_return('spec/models/bar_spec.rb')
it 'registers an offense for a path containing the class name' do
expect_offense(<<-RUBY, '/home/foo/spec/models/bar_spec.rb')
describe Foo do; end
^^^^^^^^^^^^ Spec path should end with `foo*_spec.rb`.
RUBY
end

it 'uses relative path for sibling directory project' do
allow(RuboCop::PathUtil)
.to receive(:relative_path)
.and_return('../ext-project/spec/models/bar_spec.rb')
expect_no_offenses(<<-RUBY, '/home/ext-project/spec/models/bar_spec.rb')
describe Bar do; end
it 'detects the path based on a class name with long module' do
expect_offense(<<-RUBY, '/home/foo/spec/very/my_class_spec.rb')
describe Very::Long::Namespace::MyClass do; end
^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^ Spec path should end with `very/long/namespace/my_class*_spec.rb`.
RUBY
end

it 'uses relative path for different path project' do
allow(RuboCop::PathUtil)
.to receive(:relative_path)
.and_return('../../opt/ext-project/spec/models/bar_spec.rb')
expect_no_offenses(<<-RUBY, '/opt/ext-project/spec/models/bar_spec.rb')
describe Bar do; end
it 'detects the path based the absolute path of the file' do
allow(File).to receive(:expand_path).with('my_class_spec.rb').and_return(
'/home/foo/spec/very/long/namespace/my_class_spec.rb'
)
expect_no_offenses(<<-RUBY, 'my_class_spec.rb')
describe Very::Long::Namespace::MyClass do; end
RUBY
end

Expand Down Expand Up @@ -259,5 +262,12 @@ class Foo
^^^^^^^^^^^^^^^^ Spec path should end with `*_spec.rb`.
RUBY
end

it 'registers an offense when the file extension is not .rb' do
expect_offense(<<-RUBY, 'whatever_specxrb')
describe MyClass do; end
^^^^^^^^^^^^^^^^ Spec path should end with `*_spec.rb`.
RUBY
end
end
end