Skip to content

Commit

Permalink
[Fix #10568] Add new Style/EnvHome cop
Browse files Browse the repository at this point in the history
Fixes #10568.

This cop checks for consistent usage of `ENV['HOME']`. If `nil` is used as
the second argument of `ENV.fetch`, it is treated as a bad case like `ENV[]`.
This avoids conflicts with `Style/FetchEnvVar`'s autocorrection.

## Safety

The cop is unsafe because The result when `nil` is assigned to `ENV['HOME']` changes:

```ruby
ENV['HOME'] = nil
ENV['HOME'] # => nil
Dir.home    # => '/home/foo'
```

## Example

```ruby
# bad
ENV['HOME']
ENV.fetch('HOME', nil)

# good
Dir.home

# good
ENV.fetch('HOME', default)
```
  • Loading branch information
koic authored and bbatsov committed May 6, 2022
1 parent 0d23a0a commit 914551a
Show file tree
Hide file tree
Showing 7 changed files with 131 additions and 2 deletions.
1 change: 1 addition & 0 deletions changelog/new_add_new_style_env_home_cop.md
Original file line number Diff line number Diff line change
@@ -0,0 +1 @@
* [#10568](https://github.com/rubocop/rubocop/issues/10568): Add new `Style/EnvHome` cop. ([@koic][])
6 changes: 6 additions & 0 deletions config/default.yml
Original file line number Diff line number Diff line change
Expand Up @@ -3504,6 +3504,12 @@ Style/EndlessMethod:
- allow_always
- disallow

Style/EnvHome:
Description: "Checks for consistent usage of `ENV['HOME']`."
Enabled: pending
Safe: false
VersionAdded: '<<next>>'

Style/EvalWithLocation:
Description: 'Pass `__FILE__` and `__LINE__` to `eval` method, as they are used by backtraces.'
Enabled: true
Expand Down
1 change: 1 addition & 0 deletions lib/rubocop.rb
Original file line number Diff line number Diff line change
Expand Up @@ -480,6 +480,7 @@
require_relative 'rubocop/cop/style/endless_method'
require_relative 'rubocop/cop/style/encoding'
require_relative 'rubocop/cop/style/end_block'
require_relative 'rubocop/cop/style/env_home'
require_relative 'rubocop/cop/style/eval_with_location'
require_relative 'rubocop/cop/style/even_odd'
require_relative 'rubocop/cop/style/expand_path_arguments'
Expand Down
56 changes: 56 additions & 0 deletions lib/rubocop/cop/style/env_home.rb
Original file line number Diff line number Diff line change
@@ -0,0 +1,56 @@
# frozen_string_literal: true

module RuboCop
module Cop
module Style
# This cop checks for consistent usage of `ENV['HOME']`. If `nil` is used as
# the second argument of `ENV.fetch`, it is treated as a bad case like `ENV[]`.
#
# @safety
# The cop is unsafe because the result when `nil` is assigned to `ENV['HOME']` changes:
#
# [source,ruby]
# ----
# ENV['HOME'] = nil
# ENV['HOME'] # => nil
# Dir.home # => '/home/foo'
# ----
#
# @example
#
# # bad
# ENV['HOME']
# ENV.fetch('HOME', nil)
#
# # good
# Dir.home
#
# # good
# ENV.fetch('HOME', default)
#
class EnvHome < Base
extend AutoCorrector

MSG = 'Use `Dir.home` instead.'
RESTRICT_ON_SEND = %i[[] fetch].freeze

# @!method env_home?(node)
def_node_matcher :env_home?, <<~PATTERN
(send
(const {cbase nil?} :ENV) {:[] :fetch}
(str "HOME")
...)
PATTERN

def on_send(node)
return unless env_home?(node)
return if node.arguments.count == 2 && !node.arguments[1].nil_type?

add_offense(node) do |corrector|
corrector.replace(node, 'Dir.home')
end
end
end
end
end
end
2 changes: 1 addition & 1 deletion lib/rubocop/result_cache.rb
Original file line number Diff line number Diff line change
Expand Up @@ -81,7 +81,7 @@ def self.cache_root(config_store)
#
# To avoid raising warn log messages on FreeBSD, we retrieve
# the real path of the home folder.
File.join(File.realpath(ENV.fetch('HOME')), '.cache')
File.join(File.realpath(Dir.home), '.cache')
end
File.join(root, 'rubocop_cache')
end
Expand Down
2 changes: 1 addition & 1 deletion lib/rubocop/rspec/shared_contexts.rb
Original file line number Diff line number Diff line change
Expand Up @@ -5,7 +5,7 @@
RSpec.shared_context 'isolated environment', :isolated_environment do
around do |example|
Dir.mktmpdir do |tmpdir|
original_home = ENV.fetch('HOME', nil)
original_home = Dir.home
original_xdg_config_home = ENV.fetch('XDG_CONFIG_HOME', nil)

# Make sure to expand all symlinks in the path first. Otherwise we may
Expand Down
65 changes: 65 additions & 0 deletions spec/rubocop/cop/style/env_home_spec.rb
Original file line number Diff line number Diff line change
@@ -0,0 +1,65 @@
# frozen_string_literal: true

RSpec.describe RuboCop::Cop::Style::EnvHome, :config do
it "registers and corrects an offense when using `ENV['HOME']`" do
expect_offense(<<~RUBY)
ENV['HOME']
^^^^^^^^^^^ Use `Dir.home` instead.
RUBY

expect_correction(<<~RUBY)
Dir.home
RUBY
end

it "registers and corrects an offense when using `ENV.fetch('HOME')`" do
expect_offense(<<~RUBY)
ENV.fetch('HOME')
^^^^^^^^^^^^^^^^^ Use `Dir.home` instead.
RUBY

expect_correction(<<~RUBY)
Dir.home
RUBY
end

it "registers and corrects an offense when using `ENV.fetch('HOME', nil)`" do
expect_offense(<<~RUBY)
ENV.fetch('HOME', nil)
^^^^^^^^^^^^^^^^^^^^^^ Use `Dir.home` instead.
RUBY

expect_correction(<<~RUBY)
Dir.home
RUBY
end

it "registers and corrects an offense when using `::ENV['HOME']`" do
expect_offense(<<~RUBY)
::ENV['HOME']
^^^^^^^^^^^^^ Use `Dir.home` instead.
RUBY

expect_correction(<<~RUBY)
Dir.home
RUBY
end

it 'does not register an offense when using `Dir.home`' do
expect_no_offenses(<<~RUBY)
Dir.home
RUBY
end

it "does not register an offense when using `ENV['HOME'] = '/home/foo'`" do
expect_no_offenses(<<~RUBY)
ENV['HOME'] = '/home/foo'
RUBY
end

it "does not register an offense when using `ENV.fetch('HOME', default)`" do
expect_no_offenses(<<~RUBY)
ENV.fetch('HOME', default)
RUBY
end
end

0 comments on commit 914551a

Please sign in to comment.