Add new Style/FetchEnvVar cop

changelog/new_add_new_fetch_env_var.md

@@ -0,0 +1 @@
+* [#10502](https://github.com/rubocop/rubocop/pull/10502): Add new `Style/FetchEnvVar` cop. ([@johnny-miyake][])

config/default.yml

@@ -3526,6 +3526,16 @@ Style/ExponentialNotation:
     - engineering
     - integral
 
+Style/FetchEnvVar:
+  Description: >-
+                 This cop suggests `ENV.fetch` for the replacement of `ENV[]`.
+  Reference:
+    - https://rubystyle.guide/#hash-fetch-defaults
+  Enabled: pending
+  VersionAdded: '<<next>>'
+  # Environment variables to be excluded from the inspection.
+  AllowedVars: []
+
 Style/FileRead:
   Description: 'Favor `File.(bin)read` convenience methods.'
   StyleGuide: '#file-read'

lib/rubocop.rb

@@ -485,6 +485,7 @@
 require_relative 'rubocop/cop/style/expand_path_arguments'
 require_relative 'rubocop/cop/style/explicit_block_argument'
 require_relative 'rubocop/cop/style/exponential_notation'
+require_relative 'rubocop/cop/style/fetch_env_var'
 require_relative 'rubocop/cop/style/file_read'
 require_relative 'rubocop/cop/style/file_write'
 require_relative 'rubocop/cop/style/float_division'

lib/rubocop/cli/command/suggest_extensions.rb

@@ -42,7 +42,7 @@ def skip?
           # 1. On CI
           # 2. When given RuboCop options that it doesn't make sense for
           # 3. For all formatters except specified in `INCLUDED_FORMATTERS'`
-          ENV['CI'] ||
+          ENV.fetch('CI', nil) ||
             @options[:only] || @options[:debug] || @options[:list_target_files] ||
             @options[:out] || @options[:stdin] ||
             !INCLUDED_FORMATTERS.include?(current_formatter)

lib/rubocop/cop/style/fetch_env_var.rb

@@ -0,0 +1,76 @@
+# frozen_string_literal: true
+
+module RuboCop
+  module Cop
+    module Style
+      # This cop suggests `ENV.fetch` for the replacement of `ENV[]`.
+      # `ENV[]` silently fails and returns `nil` when the environment variable is unset,
+      # which may cause unexpected behaviors when the developer forgets to set it.
+      # On the other hand, `ENV.fetch` raises KeyError or returns the explicitly
+      # specified default value.
+      #
+      # @example
+      #   # bad
+      #   ENV['X']
+      #   ENV['X'] || z
+      #   x = ENV['X']
+      #
+      #   # good
+      #   ENV.fetch('X')
+      #   ENV.fetch('X', nil) || z
+      #   x = ENV.fetch('X')
+      #
+      #   # also good
+      #   !ENV['X']
+      #   ENV['X'].some_method # (e.g. `.nil?`)
+      #
+      class FetchEnvVar < Base
+        extend AutoCorrector
+
+        MSG = 'Use `ENV.fetch(%<key>s)` or `ENV.fetch(%<key>s, nil)` instead of `ENV[%<key>s]`.'
+
+        # @!method env_with_bracket?(node)
+        def_node_matcher :env_with_bracket?, <<~PATTERN
+          (send (const nil? :ENV) :[] $_)
+        PATTERN
+
+        def on_send(node)
+          env_with_bracket?(node) do |expression|
+            break if allowed_var?(expression)
+            break if allowable_use?(node)
+
+            add_offense(node, message: format(MSG, key: expression.source)) do |corrector|
+              corrector.replace(node, "ENV.fetch(#{expression.source}, nil)")
+            end
+          end
+        end
+
+        private
+
+        def allowed_var?(expression)
+          expression.str_type? && cop_config['AllowedVars'].include?(expression.value)
+        end
+
+        def used_as_flag?(node)
+          return false if node.root?
+
+          node.parent.if_type? || (node.parent.send_type? && node.parent.prefix_bang?)
+        end
+
+        # Check if the node is a receiver and receives a message with dot syntax.
+        def message_chained_with_dot?(node)
+          return false if node.root?
+
+          node.parent.send_type? && node.parent.children.first == node && node.parent.dot?
+        end
+
+        # Allow if used as a flag (e.g., `if ENV['X']` or `!ENV['X']`) because
+        # it simply checks whether the variable is set.
+        # Also allow if receiving a message with dot syntax, e.g. `ENV['X'].nil?`.
+        def allowable_use?(node)
+          used_as_flag?(node) || message_chained_with_dot?(node)
+        end
+      end
+    end
+  end
+end

lib/rubocop/result_cache.rb

@@ -66,12 +66,12 @@ def remove_files(files, dirs, remove_count)
     end
 
     def self.cache_root(config_store)
-      root = ENV['RUBOCOP_CACHE_ROOT']
+      root = ENV.fetch('RUBOCOP_CACHE_ROOT', nil)
       root ||= config_store.for_pwd.for_all_cops['CacheRootDirectory']
       root ||= if ENV.key?('XDG_CACHE_HOME')
                  # Include user ID in the path to make sure the user has write
                  # access.
-                 File.join(ENV['XDG_CACHE_HOME'], Process.uid.to_s)
+                 File.join(ENV.fetch('XDG_CACHE_HOME'), Process.uid.to_s)
                else
                  # On FreeBSD, the /home path is a symbolic link to /usr/home
                  # and the $HOME environment variable returns the /home path.
@@ -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['HOME']), '.cache')
+                 File.join(File.realpath(ENV.fetch('HOME')), '.cache')
                end
       File.join(root, 'rubocop_cache')
     end

lib/rubocop/rspec/shared_contexts.rb

@@ -5,8 +5,8 @@
 RSpec.shared_context 'isolated environment', :isolated_environment do
   around do |example|
     Dir.mktmpdir do |tmpdir|
-      original_home = ENV['HOME']
-      original_xdg_config_home = ENV['XDG_CONFIG_HOME']
+      original_home = ENV.fetch('HOME', nil)
+      original_xdg_config_home = ENV.fetch('XDG_CONFIG_HOME', nil)
 
       # Make sure to expand all symlinks in the path first. Otherwise we may
       # get mismatched pathnames when loading config files later on.

spec/rubocop/cli_spec.rb

@@ -1837,8 +1837,8 @@ def find_suggestions
 
       # Ensure that these specs works in CI, since the feature is generally
       # disabled in when ENV['CI'] is set.
-      allow(ENV).to receive(:[]).and_call_original
-      allow(ENV).to receive(:[]).with('CI').and_return(false)
+      allow(ENV).to receive(:fetch).and_call_original
+      allow(ENV).to receive(:fetch).with('CI', nil).and_return(false)
 
       # Mock the lockfile to be parsed by bundler
       allow(Bundler).to receive(:default_lockfile)
@@ -2024,7 +2024,7 @@ def find_suggestions
     end
 
     context 'when in CI mode' do
-      before { allow(ENV).to receive(:[]).with('CI').and_return(true) }
+      before { allow(ENV).to receive(:fetch).with('CI', nil).and_return(true) }
 
       it 'does not show the suggestion' do
         expect { cli.run(['example.rb']) }.not_to suggest_extensions