(PUP-11767) Enable Style/FetchEnvVar

Author: tvpartytonight

.rubocop.yml

@@ -268,3 +268,6 @@ Style/EvenOdd:
 
 Style/ExpandPathArguments:
   Enabled: true
+
+Style/FetchEnvVar:
+  Enabled: true

ext/windows/service/daemon.rb

@@ -18,7 +18,7 @@ class WindowsDaemon < Puppet::Util::Windows::Daemon
   @run_thread = nil
   @LOG_TO_FILE = false
   @loglevel = 0
-  LOG_FILE =  File.expand_path(File.join(ENV['ALLUSERSPROFILE'], 'PuppetLabs', 'puppet', 'var', 'log', 'windows.log'))
+  LOG_FILE =  File.expand_path(File.join(ENV.fetch('ALLUSERSPROFILE', nil), 'PuppetLabs', 'puppet', 'var', 'log', 'windows.log'))
   LEVELS = [:debug, :info, :notice, :warning, :err, :alert, :emerg, :crit]
   LEVELS.each do |level|
     define_method("log_#{level}") do |msg|
@@ -203,10 +203,10 @@ def load_env(base_dir)
       ENV['Path'] = [
         File.join(base_dir, 'puppet', 'bin'),
         File.join(base_dir, 'bin'),
-      ].join(';').tr('/', '\\') + ';' + ENV['Path']
+      ].join(';').tr('/', '\\') + ';' + ENV.fetch('Path', nil)
 
       # ENV that uses forward slashes
-      ENV['RUBYLIB'] = "#{File.join(base_dir, 'puppet', 'lib')};#{ENV['RUBYLIB']}"
+      ENV['RUBYLIB'] = "#{File.join(base_dir, 'puppet', 'lib')};#{ENV.fetch('RUBYLIB', nil)}"
     rescue => e
       log_exception(e)
     end

lib/puppet/application/resource.rb

@@ -188,7 +188,7 @@ def handle_editing(text)
       tmpfile.puts text
 
       # edit the content
-      system(ENV["EDITOR"] || 'vi', tmpfile.path)
+      system(ENV.fetch("EDITOR", nil) || 'vi', tmpfile.path)
 
       # ...and, now, pass that file to puppet to apply.  Because
       # many editors rename or replace the original file we need to

lib/puppet/defaults.rb

@@ -49,7 +49,7 @@ def self.default_cadir
   def self.default_basemodulepath
     if Puppet::Util::Platform.windows?
       path = ['$codedir/modules']
-      installdir = ENV["FACTER_env_windows_installdir"]
+      installdir = ENV.fetch("FACTER_env_windows_installdir", nil)
       if installdir
         path << "#{installdir}/puppet/modules"
       end
@@ -61,7 +61,7 @@ def self.default_basemodulepath
 
   def self.default_vendormoduledir
     if Puppet::Util::Platform.windows?
-      installdir = ENV["FACTER_env_windows_installdir"]
+      installdir = ENV.fetch("FACTER_env_windows_installdir", nil)
       if installdir
         "#{installdir}\\puppet\\vendor_modules"
       else
@@ -373,7 +373,7 @@ def self.initialize_default_settings!(settings)
           Puppet::Util::Platform.default_paths.each do |path|
             next if paths.include?(path)
 
-            ENV['PATH'] = ENV['PATH'] + File::PATH_SEPARATOR + path
+            ENV['PATH'] = ENV.fetch('PATH', nil) + File::PATH_SEPARATOR + path
           end
           value
         end

lib/puppet/file_system/uniquefile.rb

@@ -153,7 +153,7 @@ def try_convert_to_hash(h)
 
   def tmpdir
     tmp = '.'
-    for dir in [ENV['TMPDIR'], ENV['TMP'], ENV['TEMP'], @@systmpdir, '/tmp']
+    for dir in [ENV.fetch('TMPDIR', nil), ENV.fetch('TMP', nil), ENV.fetch('TEMP', nil), @@systmpdir, '/tmp']
       stat = File.stat(dir) if dir
       if stat && stat.directory? && stat.writable?
         tmp = dir

lib/puppet/http/proxy.rb

@@ -18,7 +18,7 @@ def self.proxy(uri)
 
   def self.http_proxy_env
     # Returns a URI object if proxy is set, or nil
-    proxy_env = ENV["http_proxy"] || ENV["HTTP_PROXY"]
+    proxy_env = ENV.fetch("http_proxy", nil) || ENV.fetch("HTTP_PROXY", nil)
     begin
       return URI.parse(proxy_env) if proxy_env
     rescue URI::InvalidURIError
@@ -124,7 +124,7 @@ def self.http_proxy_password
   end
 
   def self.no_proxy
-    no_proxy_env = ENV["no_proxy"] || ENV["NO_PROXY"]
+    no_proxy_env = ENV.fetch("no_proxy", nil) || ENV.fetch("NO_PROXY", nil)
 
     if no_proxy_env
       return no_proxy_env

lib/puppet/node/environment.rb

@@ -554,7 +554,7 @@ def hash
   # not private so it can be called in tests
   def self.extralibs
     if ENV['PUPPETLIB']
-      split_path(ENV['PUPPETLIB'])
+      split_path(ENV.fetch('PUPPETLIB', nil))
     else
       []
     end

lib/puppet/provider/package/gem.rb

@@ -77,7 +77,7 @@ def self.execute_gem_command(command, command_options, custom_environment = {})
     validate_command(command)
     cmd = [command] << command_options
 
-    custom_environment = { 'HOME' => ENV['HOME'] }.merge(custom_environment)
+    custom_environment = { 'HOME' => ENV.fetch('HOME', nil) }.merge(custom_environment)
 
     if Puppet::Util::Platform.windows?
       custom_environment[:PATH] = windows_path_without_puppet_bin

lib/puppet/provider/package/pkgutil.rb

@@ -12,7 +12,7 @@
   confine 'os.family' => :solaris
 
   has_command(:pkguti, pkgutil_bin) do
-    environment :HOME => ENV['HOME']
+    environment :HOME => ENV.fetch('HOME', nil)
   end
 
   def self.healthcheck

lib/puppet/provider/package/puppet_gem.rb

@@ -9,9 +9,9 @@
   confine :true => Puppet.runtime[:facter].value(:aio_agent_version)
 
   def self.windows_gemcmd
-    puppet_dir = ENV['PUPPET_DIR']
+    puppet_dir = ENV.fetch('PUPPET_DIR', nil)
     if puppet_dir
-      File.join(ENV['PUPPET_DIR'].to_s, 'bin', 'gem.bat')
+      File.join(puppet_dir.to_s, 'bin', 'gem.bat')
     else
       File.join(Gem.default_bindir, 'gem.bat')
     end

lib/puppet/provider/package/puppetserver_gem.rb

@@ -33,7 +33,7 @@ def self.provider_command
   # CommandDefiner in provider.rb will set failonfail, combine, and environment.
 
   has_command(:puppetservercmd, '/opt/puppetlabs/bin/puppetserver') do
-    environment(HOME: ENV['HOME'])
+    environment(HOME: ENV.fetch('HOME', nil))
   end
 
   def self.gemlist(options)

lib/puppet/util.rb

@@ -44,7 +44,7 @@ def create_erb(content)
   # @deprecated Use ENV instead
   # @api private
   def get_env(name, mode = default_env)
-    ENV[name]
+    ENV.fetch(name, nil)
   end
   module_function :get_env
 
@@ -208,16 +208,16 @@ def which(bin)
     if absolute_path?(bin)
       return bin if FileTest.file? bin and FileTest.executable? bin
     else
-      exts = ENV['PATHEXT']
+      exts = ENV.fetch('PATHEXT', nil)
       exts = exts ? exts.split(File::PATH_SEPARATOR) : %w[.COM .EXE .BAT .CMD]
-      ENV['PATH'].split(File::PATH_SEPARATOR).each do |dir|
+      ENV.fetch('PATH').split(File::PATH_SEPARATOR).each do |dir|
         begin
           dest = File.expand_path(File.join(dir, bin))
         rescue ArgumentError => e
           # if the user's PATH contains a literal tilde (~) character and HOME is not set, we may get
           # an ArgumentError here.  Let's check to see if that is the case; if not, re-raise whatever error
           # was thrown.
-          if e.to_s =~ /HOME/ and (ENV['HOME'].nil? || ENV['HOME'] == "")
+          if e.to_s =~ /HOME/ and (ENV['HOME'].nil? || ENV.fetch('HOME', nil) == "")
             # if we get here they have a tilde in their PATH.  We'll issue a single warning about this and then
             # ignore this path element and carry on with our lives.
             # TRANSLATORS PATH and HOME are environment variables and should not be translated

lib/puppet/util/run_mode.rb

@@ -117,7 +117,7 @@ def log_dir
       private
 
       def windows_common_base(*extra)
-        [ENV['ALLUSERSPROFILE'], "PuppetLabs"] + extra
+        [ENV.fetch('ALLUSERSPROFILE', nil), "PuppetLabs"] + extra
       end
     end
   end

lib/puppet/util/windows/process.rb

@@ -54,7 +54,7 @@ def wait_process(handle)
       # and since it's read-only, we can't set it. But we can execute a
       # a shell that simply returns the desired exit status, which has the
       # desired effect.
-      %x{#{ENV['COMSPEC']} /c exit #{exit_status}}
+      %x{#{ENV.fetch('COMSPEC', nil)} /c exit #{exit_status}}
     end
 
     exit_status

spec/unit/defaults_spec.rb

@@ -101,7 +101,7 @@
       let(:installdir) { 'C:\Program Files\Puppet Labs\Puppet' }
 
       it 'includes user and system modules' do
-        allow(ENV).to receive(:[]).with("FACTER_env_windows_installdir").and_return(installdir)
+        allow(ENV).to receive(:fetch).with("FACTER_env_windows_installdir", anything).and_return(installdir)
 
         expect(
           Puppet.default_basemodulepath
@@ -129,7 +129,7 @@
       let(:installdir) { 'C:\Program Files\Puppet Labs\Puppet' }
 
       it 'includes the default vendormoduledir' do
-        allow(ENV).to receive(:[]).with("FACTER_env_windows_installdir").and_return(installdir)
+        allow(ENV).to receive(:fetch).with("FACTER_env_windows_installdir", anything).and_return(installdir)
 
         expect(
           Puppet.default_vendormoduledir

spec/unit/provider/package/puppet_gem_spec.rb

@@ -35,8 +35,8 @@
   describe '.windows_gemcmd' do
     context 'when PUPPET_DIR is not set' do
       before do
-        allow(ENV).to receive(:[]).and_call_original
-        allow(ENV).to receive(:[]).with('PUPPET_DIR').and_return(nil)
+        # allow(ENV).to receive(:fetch, anything).and_call_original
+        allow(ENV).to receive(:fetch).with('PUPPET_DIR', anything).and_return(nil)
         allow(Gem).to receive(:default_bindir).and_return('default_gem_bin')
       end
 
@@ -48,8 +48,7 @@
 
     context 'when PUPPET_DIR is set' do
       before do
-        allow(ENV).to receive(:[]).and_call_original
-        allow(ENV).to receive(:[]).with('PUPPET_DIR').and_return('puppet_dir')
+        allow(ENV).to receive(:fetch).with('PUPPET_DIR', anything).and_return('puppet_dir')
       end
 
       it 'uses Gem.default_bindir' do

spec/unit/util_spec.rb

@@ -706,8 +706,8 @@ def withenv_utf8(&block)
       end
 
       it "should walk the search PATH returning the first executable" do
-        allow(ENV).to receive(:[]).with('PATH').and_return(File.expand_path('/bin'))
-        allow(ENV).to receive(:[]).with('PATHEXT').and_return(nil)
+        allow(ENV).to receive(:fetch).with('PATH').and_return(File.expand_path('/bin'))
+        allow(ENV).to receive(:fetch).with('PATHEXT', anything).and_return(nil)
 
         expect(Puppet::Util.which('foo')).to eq(path)
       end
@@ -723,8 +723,8 @@ def withenv_utf8(&block)
 
       describe "when a file extension is specified" do
         it "should walk each directory in PATH ignoring PATHEXT" do
-          allow(ENV).to receive(:[]).with('PATH').and_return(%w[/bar /bin].map{|dir| File.expand_path(dir)}.join(File::PATH_SEPARATOR))
-          allow(ENV).to receive(:[]).with('PATHEXT').and_return('.FOOBAR')
+          allow(ENV).to receive(:fetch).with('PATH').and_return(%w[/bar /bin].map{|dir| File.expand_path(dir)}.join(File::PATH_SEPARATOR))
+          allow(ENV).to receive(:fetch).with('PATHEXT', anything).and_return('.FOOBAR')
 
           expect(FileTest).to receive(:file?).with(File.join(File.expand_path('/bar'), 'foo.CMD')).and_return(false)
 
@@ -735,8 +735,8 @@ def withenv_utf8(&block)
       describe "when a file extension is not specified" do
         it "should walk each extension in PATHEXT until an executable is found" do
           bar = File.expand_path('/bar')
-          allow(ENV).to receive(:[]).with('PATH').and_return("#{bar}#{File::PATH_SEPARATOR}#{base}")
-          allow(ENV).to receive(:[]).with('PATHEXT').and_return(".EXE#{File::PATH_SEPARATOR}.CMD")
+          allow(ENV).to receive(:fetch).with('PATH').and_return("#{bar}#{File::PATH_SEPARATOR}#{base}")
+          allow(ENV).to receive(:fetch).with('PATHEXT', anything).and_return(".EXE#{File::PATH_SEPARATOR}.CMD")
 
           expect(FileTest).to receive(:file?).ordered().with(File.join(bar, 'foo.EXE')).and_return(false)
           expect(FileTest).to receive(:file?).ordered().with(File.join(bar, 'foo.CMD')).and_return(false)
@@ -747,8 +747,8 @@ def withenv_utf8(&block)
         end
 
         it "should walk the default extension path if the environment variable is not defined" do
-          allow(ENV).to receive(:[]).with('PATH').and_return(base)
-          allow(ENV).to receive(:[]).with('PATHEXT').and_return(nil)
+          allow(ENV).to receive(:fetch).with('PATH').and_return(base)
+          allow(ENV).to receive(:fetch).with('PATHEXT', anything).and_return(nil)
 
           %w[.COM .EXE .BAT].each do |ext|
             expect(FileTest).to receive(:file?).ordered().with(File.join(base, "foo#{ext}")).and_return(false)
@@ -759,8 +759,8 @@ def withenv_utf8(&block)
         end
 
         it "should fall back if no extension matches" do
-          allow(ENV).to receive(:[]).with('PATH').and_return(base)
-          allow(ENV).to receive(:[]).with('PATHEXT').and_return(".EXE")
+          allow(ENV).to receive(:fetch).with('PATH').and_return(base)
+          allow(ENV).to receive(:fetch).with('PATHEXT', anything).and_return(".EXE")
 
           allow(FileTest).to receive(:file?).with(File.join(base, 'foo.EXE')).and_return(false)
           allow(FileTest).to receive(:file?).with(File.join(base, 'foo')).and_return(true)

util/rspec_grouper

@@ -20,7 +20,7 @@ module Parallel
 
       def initialize(group_size)
         config = ::RSpec::Core::Configuration.new
-        options = ::RSpec::Core::ConfigurationOptions.new((ENV['TEST'] || ENV['TESTS'] || 'spec').split(';'))
+        options = ::RSpec::Core::ConfigurationOptions.new((ENV.fetch('TEST', nil) || ENV.fetch('TESTS', nil) || 'spec').split(';'))
         options.configure config
 
         # This will scan and load all spec examples