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.

Sign up for GitHub

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

Jump to bottom

Allows for empty if blocks #6993

Merged
merged 3 commits into from Apr 30, 2019
Merged

Allows for empty if blocks #6993

Show file tree
Hide file tree
Changes from all commits
Commits
Show all changes
3 commits
Select commit Hold shift + click to select a range
263cc6d
Allows for empty if blocks
RicardoTrindade Apr 29, 2019
a2970bb
renames node match for if safe navigation candidate
RicardoTrindade Apr 29, 2019
7d5e159
update changelog
RicardoTrindade Apr 29, 2019
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
2 changes: 2 additions & 0 deletions CHANGELOG.md
View file
Open in desktop
Expand Up @@ -4,6 +4,7 @@

### Bug fixes

* [#6993](https://github.com/rubocop-hq/rubocop/pull/6993): Allowing for empty if blocks, preventing `Style/SafeNavigation` from crashing. ([@RicardoTrindade][])
* [#6995](https://github.com/rubocop-hq/rubocop/pull/6995): Fix an incorrect auto-correct for `Style/RedundantParentheses` when enclosed in parentheses at `while-post` or `until-post`. ([@koic][])
* [#6996](https://github.com/rubocop-hq/rubocop/pull/6996): Fix a false positive for `Style/RedundantFreeze` when freezing the result of `String#*`. ([@bquorning][])
* [#6998](https://github.com/rubocop-hq/rubocop/pull/6998): Fix autocorrect of `Naming/RescuedExceptionsVariableName` to also rename all references to the variable. ([@Darhazer][])
Expand Down Expand Up @@ -3985,3 +3986,4 @@
[@vfonic]: https://github.com/vfonic
[@andreaseger]: https://github.com/andreaseger
[@yakout]: https://github.com/yakout
[@RicardoTrindade]: https://github.com/RicardoTrindade
10 changes: 7 additions & 3 deletions lib/rubocop/cop/style/safe_navigation.rb
View file
Open in desktop
Expand Up @@ -72,7 +72,7 @@ class SafeNavigation < Cop

# if format: (if checked_variable body nil)
# unless format: (if checked_variable nil body)
def_node_matcher :modifier_if_safe_navigation_candidate?, <<-PATTERN
def_node_matcher :modifier_if_safe_navigation_candidate, <<-PATTERN
{
(if {
(send $_ {:nil? :!})
Expand Down Expand Up @@ -147,11 +147,15 @@ def extract_parts(node)

def extract_parts_from_if(node)
variable, receiver =
modifier_if_safe_navigation_candidate?(node)
modifier_if_safe_navigation_candidate(node)

checked_variable, matching_receiver, method =
extract_common_parts(receiver, variable)
matching_receiver = nil if LOGIC_JUMP_KEYWORDS.include?(receiver.type)

Copy link
Member

Choose a reason for hiding this comment

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

I would go first by renaming modifier_if_safe_navigation_candidate? to just modifier_if_safe_navigation_candidate as it doesn't return a boolean value. Next, if the receiver is returned, it is a Node for sure, so no need for the second check (receiver.type). So it may become matching_receiver = nil if receiver.nil? || LOGIC_JUMP_KEYWORDS.include?(receiver.type). Which isLOGIC_JUMP_KEYWORDS a bit long, so it may be extracted in a method

Copy link
Contributor Author

Choose a reason for hiding this comment

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

Alright, thanks! Made those changes, just not sure about the method extracted being called safeguarded?, maybe not the best of names.

if receiver && LOGIC_JUMP_KEYWORDS.include?(receiver.type)
matching_receiver = nil
end

[checked_variable, matching_receiver, receiver, method]
end

Expand Down
9 changes: 9 additions & 0 deletions spec/rubocop/cop/style/safe_navigation_spec.rb
View file
Open in desktop
Expand Up @@ -144,6 +144,15 @@
RUBY
end

it 'allows for empty if blocks with comments' do
expect_no_offenses(<<-RUBY.strip_indent)
if foo
# a random commnet
# TODO: Implement this before
end
RUBY
end

it 'allows a method call as a parameter when the parameter is ' \
'safe guarded with an object check' do
expect_no_offenses('foo(bar.baz) if bar')
Expand Down