Skip to content

Commit

Permalink
[Fix #10569] Fix a false positive for Style/FetchEnvVar
Browse files Browse the repository at this point in the history
Fixes #10569.

This PR fixes a false positive for `Style/FetchEnvVar` when using
the same value of `ENV` as `if` condition in the body.

The `ENV` var as a `if` condition is currently allowed.
So the `ENV` var already used in the condition will be accepted as well.

And the following (maybe) corner case is accepted in this PR.

```ruby
if ENV['foo']
  ENV.delete('foo')
  puts 'some code'
  puts ENV['foo']
  puts 'other code'
end
```

#10569 (comment)
  • Loading branch information
koic authored and bbatsov committed May 7, 2022
1 parent d392481 commit 4087bae
Show file tree
Hide file tree
Showing 3 changed files with 21 additions and 1 deletion.
1 change: 1 addition & 0 deletions changelog/fix_a_false_for_style_fetch_env_var.md
@@ -0,0 +1 @@
* [#10569](https://github.com/rubocop/rubocop/issues/10569): Fix a false positive for `Style/FetchEnvVar` when using the same `ENV` var as `if` condition in the body. ([@koic][])
4 changes: 3 additions & 1 deletion lib/rubocop/cop/style/fetch_env_var.rb
Expand Up @@ -97,7 +97,9 @@ def allowed_var?(node)

def used_as_flag?(node)
return false if node.root?
return true if node.parent.if_type?

if_node = node.ancestors.find(&:if_type?)
return true if if_node&.condition == node

node.parent.send_type? && (node.parent.prefix_bang? || node.parent.comparison_method?)
end
Expand Down
17 changes: 17 additions & 0 deletions spec/rubocop/cop/style/fetch_env_var_spec.rb
Expand Up @@ -296,6 +296,23 @@
end
RUBY
end

it 'registers no offenses when using the same `ENV` var as `if` condition in the body' do
expect_no_offenses(<<~RUBY)
if ENV['X']
puts ENV['X']
end
RUBY
end

it 'registers no offenses when using an `ENV` var that is different from `if` condition in the body' do
expect_offense(<<~RUBY)
if ENV['X']
puts ENV['Y']
^^^^^^^^ Use `ENV.fetch('Y')` or `ENV.fetch('Y', nil)` instead of `ENV['Y']`.
end
RUBY
end
end

context 'when the env val is excluded from the inspection by the config' do
Expand Down

0 comments on commit 4087bae

Please sign in to comment.