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 #8781] Change how comments are determined for Style/SafeNavigation autocorrection #8813

Merged
merged 1 commit into from Oct 17, 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.
Jump to
Jump to file
Failed to load files.
Diff view
Diff view
1 change: 1 addition & 0 deletions CHANGELOG.md
Expand Up @@ -9,6 +9,7 @@
### Bug fixes

* [#8892](https://github.com/rubocop-hq/rubocop/issues/8892): Fix an error for `Style/StringConcatenation` when correcting nested concatenable parts. ([@fatkodima][])
* [#8781](https://github.com/rubocop-hq/rubocop/issues/8781): Fix handling of comments in `Style/SafeNavigation` autocorrection. ([@dvandersluis][])

### Changes

Expand Down
20 changes: 16 additions & 4 deletions lib/rubocop/cop/style/safe_navigation.rb
Expand Up @@ -142,10 +142,22 @@ def handle_comments(corrector, node, method_call)
end

def comments(node)
processed_source.each_comment_in_lines(
node.loc.first_line...
node.loc.last_line
).to_a
relevant_comment_ranges(node).each.with_object([]) do |range, comments|
comments.concat(processed_source.each_comment_in_lines(range).to_a)
end
end

def relevant_comment_ranges(node)
# Get source lines ranges inside the if node that aren't inside an inner node
# Comments inside an inner node should remain attached to that node, and not
# moved.
begin_pos = node.loc.first_line
end_pos = node.loc.last_line

node.child_nodes.each.with_object([]) do |child, ranges|
ranges << (begin_pos...child.loc.first_line)
begin_pos = child.loc.last_line
end << (begin_pos...end_pos)
end

def allowed_if_condition?(node)
Expand Down
26 changes: 26 additions & 0 deletions spec/rubocop/cop/style/safe_navigation_spec.rb
Expand Up @@ -160,6 +160,32 @@
RUBY
end

# See https://github.com/rubocop-hq/rubocop/issues/8781
it 'does not move comments that are inside an inner block' do
expect_offense(<<~RUBY)
# Comment 1
if x
^^^^ Use safe navigation (`&.`) instead of checking if an object exists before calling the method.
# Comment 2
x.each do
# Comment 3
# Comment 4
end
# Comment 5
end
RUBY

expect_correction(<<~RUBY)
# Comment 1
# Comment 2
# Comment 5
x&.each do
# Comment 3
# Comment 4
end
RUBY
end

shared_examples 'all variable types' do |variable|
context 'modifier if' do
shared_examples 'safe guarding logical break keywords' do |keyword|
Expand Down