[rb] fix new linting failures

titusfortner · titusfortner · commit 439da14d34e1 · 2022-04-25T12:20:50.000-05:00
diff --git a/rb/.rubocop.yml b/rb/.rubocop.yml
@@ -6,7 +6,6 @@ AllCops:
   TargetRubyVersion: 2.7
   Exclude:
     - 'lib/selenium/devtools/**/*'
-  NewCops: enable
 
 Layout/EmptyLinesAroundAttributeAccessor:
   Enabled: true
@@ -18,7 +17,7 @@ Layout/EmptyLinesAroundModuleBody:
   Enabled: false
 
 Layout/LineLength:
-  IgnoredPatterns:
+  AllowedPatterns:
       - '\s+# rubocop'
       - '^\s*#'
       - '^\s*it .*, except: \{.*\} do$'
@@ -190,3 +189,169 @@ Style/SignalException:
 
 RSpec/BeEq:
   Enabled: false
+
+# Additional things
+Gemspec/DateAssignment: # new in 1.10
+  Enabled: true
+Gemspec/RequireMFA: # new in 1.23
+  Enabled: true
+Layout/LineEndStringConcatenationIndentation: # new in 1.18
+  Enabled: true
+Layout/SpaceBeforeBrackets: # new in 1.7
+  Enabled: true
+Lint/AmbiguousAssignment: # new in 1.7
+  Enabled: true
+Lint/AmbiguousOperatorPrecedence: # new in 1.21
+  Enabled: true
+Lint/AmbiguousRange: # new in 1.19
+  Enabled: true
+Lint/DeprecatedConstants: # new in 1.8
+  Enabled: true
+Lint/DuplicateBranch: # new in 1.3
+  Enabled: true
+Lint/DuplicateRegexpCharacterClassElement: # new in 1.1
+  Enabled: true
+Lint/EmptyBlock: # new in 1.1
+  Enabled: true
+Lint/EmptyClass: # new in 1.3
+  Enabled: true
+Lint/EmptyInPattern: # new in 1.16
+  Enabled: true
+Lint/IncompatibleIoSelectWithFiberScheduler: # new in 1.21
+  Enabled: true
+Lint/LambdaWithoutLiteralBlock: # new in 1.8
+  Enabled: true
+Lint/NoReturnInBeginEndBlocks: # new in 1.2
+  Enabled: true
+Lint/NumberedParameterAssignment: # new in 1.9
+  Enabled: true
+Lint/OrAssignmentToConstant: # new in 1.9
+  Enabled: true
+Lint/RedundantDirGlobSort: # new in 1.8
+  Enabled: true
+Lint/RefinementImportMethods: # new in 1.27
+  Enabled: true
+Lint/RequireRelativeSelfPath: # new in 1.22
+  Enabled: true
+Lint/SymbolConversion: # new in 1.9
+  Enabled: true
+Lint/ToEnumArguments: # new in 1.1
+  Enabled: true
+Lint/TripleQuotes: # new in 1.9
+  Enabled: true
+Lint/UnexpectedBlockArity: # new in 1.5
+  Enabled: true
+Lint/UnmodifiedReduceAccumulator: # new in 1.1
+  Enabled: true
+Lint/UselessRuby2Keywords: # new in 1.23
+  Enabled: true
+Naming/BlockForwarding: # new in 1.24
+  Enabled: true
+Security/CompoundHash: # new in 1.28
+  Enabled: true
+Security/IoMethods: # new in 1.22
+  Enabled: true
+Style/ArgumentsForwarding: # new in 1.1
+  Enabled: true
+Style/CollectionCompact: # new in 1.2
+  Enabled: true
+Style/DocumentDynamicEvalDefinition: # new in 1.1
+  Enabled: true
+Style/EndlessMethod: # new in 1.8
+  Enabled: true
+Style/FetchEnvVar: # new in 1.28
+  Enabled: true
+Style/FileRead: # new in 1.24
+  Enabled: true
+Style/FileWrite: # new in 1.24
+  Enabled: true
+Style/HashConversion: # new in 1.10
+  Enabled: true
+Style/HashExcept: # new in 1.7
+  Enabled: true
+Style/IfWithBooleanLiteralBranches: # new in 1.9
+  Enabled: true
+Style/InPatternThen: # new in 1.16
+  Enabled: true
+Style/MapToHash: # new in 1.24
+  Enabled: true
+Style/MultilineInPatternThen: # new in 1.16
+  Enabled: true
+Style/NegatedIfElseCondition: # new in 1.2
+  Enabled: true
+Style/NestedFileDirname: # new in 1.26
+  Enabled: true
+Style/NilLambda: # new in 1.3
+  Enabled: true
+Style/NumberedParameters: # new in 1.22
+  Enabled: true
+Style/NumberedParametersLimit: # new in 1.22
+  Enabled: true
+Style/ObjectThen: # new in 1.28
+  Enabled: true
+Style/OpenStructUse: # new in 1.23
+  Enabled: true
+Style/QuotedSymbols: # new in 1.16
+  Enabled: true
+Style/RedundantArgument: # new in 1.4
+  Enabled: true
+Style/RedundantInitialize: # new in 1.27
+  Enabled: true
+Style/RedundantSelfAssignmentBranch: # new in 1.19
+  Enabled: true
+Style/SelectByRegexp: # new in 1.22
+  Enabled: true
+Style/StringChars: # new in 1.12
+  Enabled: true
+Style/SwapValues: # new in 1.1
+  Enabled: true
+Performance/AncestorsInclude: # new in 1.7
+  Enabled: true
+Performance/BigDecimalWithNumericArgument: # new in 1.7
+  Enabled: true
+Performance/BlockGivenWithExplicitBlock: # new in 1.9
+  Enabled: true
+Performance/CollectionLiteralInLoop: # new in 1.8
+  Enabled: true
+Performance/ConcurrentMonotonicTime: # new in 1.12
+  Enabled: true
+Performance/ConstantRegexp: # new in 1.9
+  Enabled: true
+Performance/MapCompact: # new in 1.11
+  Enabled: true
+Performance/MethodObjectAsBlock: # new in 1.9
+  Enabled: true
+Performance/RedundantEqualityComparisonBlock: # new in 1.10
+  Enabled: true
+Performance/RedundantSortBlock: # new in 1.7
+  Enabled: true
+Performance/RedundantSplitRegexpArgument: # new in 1.10
+  Enabled: true
+Performance/RedundantStringChars: # new in 1.7
+  Enabled: true
+Performance/ReverseFirst: # new in 1.7
+  Enabled: true
+Performance/SortReverse: # new in 1.7
+  Enabled: true
+Performance/Squeeze: # new in 1.7
+  Enabled: true
+Performance/StringIdentifierArgument: # new in 1.13
+  Enabled: true
+Performance/StringInclude: # new in 1.7
+  Enabled: true
+Performance/Sum: # new in 1.8
+  Enabled: true
+RSpec/BeNil: # new in 2.9.0
+  Enabled: true
+RSpec/ExcessiveDocstringSpacing: # new in 2.5
+  Enabled: true
+RSpec/IdenticalEqualityAssertion: # new in 2.4
+  Enabled: true
+RSpec/SubjectDeclaration: # new in 2.5
+  Enabled: true
+RSpec/VerifiedDoubleReference: # new in 2.10.0
+  Enabled: true
+RSpec/FactoryBot/SyntaxMethods: # new in 2.7
+  Enabled: true
+RSpec/Rails/AvoidSetupHook: # new in 2.4
+  Enabled: true
diff --git a/rb/lib/selenium/server.rb b/rb/lib/selenium/server.rb
@@ -122,7 +122,7 @@ def available_assets
       end
 
       def net_http_start(address, &block)
-        http_proxy = ENV['http_proxy'] || ENV['HTTP_PROXY']
+        http_proxy = ENV.fetch('http_proxy', nil) || ENV.fetch('HTTP_PROXY', nil)
         if http_proxy
           http_proxy = "http://#{http_proxy}" unless http_proxy.start_with?('http://')
           uri = URI.parse(http_proxy)
diff --git a/rb/lib/selenium/webdriver/common/element.rb b/rb/lib/selenium/webdriver/common/element.rb
@@ -46,7 +46,7 @@ def ==(other)
       alias_method :eql?, :==
 
       def hash
-        @id.hash ^ @bridge.hash
+        [@id, @bridge].hash
       end
 
       #
diff --git a/rb/lib/selenium/webdriver/common/platform.rb b/rb/lib/selenium/webdriver/common/platform.rb
@@ -177,9 +177,9 @@ def find_binary(*binary_names)
 
       def find_in_program_files(*binary_names)
         paths = [
-          ENV['PROGRAMFILES'] || '\\Program Files',
-          ENV['ProgramFiles(x86)'] || '\\Program Files (x86)',
-          ENV['ProgramW6432'] || '\\Program Files'
+          ENV.fetch('PROGRAMFILES', '\\Program Files'),
+          ENV.fetch('ProgramFiles(x86)', '\\Program Files (x86)'),
+          ENV.fetch('ProgramW6432', '\\Program Files')
         ]
 
         paths.each do |root|
diff --git a/rb/lib/selenium/webdriver/common/shadow_root.rb b/rb/lib/selenium/webdriver/common/shadow_root.rb
@@ -45,7 +45,7 @@ def ==(other)
       alias_method :eql?, :==
 
       def hash
-        @id.hash ^ @bridge.hash
+        [@id, @bridge].hash
       end
 
       #
diff --git a/rb/lib/selenium/webdriver/firefox/util.rb b/rb/lib/selenium/webdriver/firefox/util.rb
@@ -27,7 +27,7 @@ module Util
         def app_data_path
           case Platform.os
           when :windows
-            "#{ENV['APPDATA']}\\Mozilla\\Firefox"
+            "#{ENV.fetch('APPDATA')}\\Mozilla\\Firefox"
           when :macosx
             "#{Platform.home}/Library/Application Support/Firefox"
           when :unix, :linux
diff --git a/rb/lib/selenium/webdriver/remote/http/default.rb b/rb/lib/selenium/webdriver/remote/http/default.rb
@@ -136,8 +136,8 @@ def new_http_client
 
           def proxy
             @proxy ||= begin
-              proxy = ENV['http_proxy'] || ENV['HTTP_PROXY']
-              no_proxy = ENV['no_proxy'] || ENV['NO_PROXY']
+              proxy = ENV.fetch('http_proxy', nil) || ENV.fetch('HTTP_PROXY', nil)
+              no_proxy = ENV.fetch('no_proxy', nil) || ENV.fetch('NO_PROXY', nil)
 
               if proxy
                 proxy = "http://#{proxy}" unless proxy.start_with?('http://')
diff --git a/rb/lib/selenium/webdriver/support/color.rb b/rb/lib/selenium/webdriver/support/color.rb
@@ -125,7 +125,7 @@ def ==(other)
         alias_method :eql?, :==
 
         def hash
-          [red, green, blue, alpha].hash ^ self.class.hash
+          [red, green, blue, alpha, self.class].hash
         end
 
         def rgb
diff --git a/rb/lib/selenium/webdriver/support/guards.rb b/rb/lib/selenium/webdriver/support/guards.rb
@@ -48,7 +48,7 @@ def add_message(name, message)
         def disposition
           if !skipping_guard.nil?
             [:skip, skipping_guard.message]
-          elsif !pending_guard.nil? && ENV['SKIP_PENDING']
+          elsif !pending_guard.nil? && ENV.fetch('SKIP_PENDING', nil)
             [:skip, pending_guard.message]
           elsif !pending_guard.nil?
             [:pending, pending_guard.message]
diff --git a/rb/spec/integration/selenium/webdriver/chrome/profile_spec.rb b/rb/spec/integration/selenium/webdriver/chrome/profile_spec.rb
@@ -39,7 +39,7 @@ module Chrome
 
           profile.add_extension(ext_path)
 
-          ext_file = instance_double('file')
+          ext_file = instance_double(File)
           allow(File).to receive(:open).with(ext_path, 'rb').and_yield ext_file
           allow(ext_file).to receive(:read).and_return 'test'
 
diff --git a/rb/spec/integration/selenium/webdriver/edge/profile_spec.rb b/rb/spec/integration/selenium/webdriver/edge/profile_spec.rb
@@ -38,7 +38,7 @@ module Edge
 
           profile.add_extension(ext_path)
 
-          ext_file = instance_double('file')
+          ext_file = instance_double(File)
           allow(File).to receive(:open).with(ext_path, 'rb').and_yield ext_file
           allow(ext_file).to receive(:read).and_return 'test'
 
diff --git a/rb/spec/integration/selenium/webdriver/spec_helper.rb b/rb/spec/integration/selenium/webdriver/spec_helper.rb
@@ -37,7 +37,7 @@
   c.include(WebDriver::SpecSupport::Helpers)
 
   c.before(:suite) do
-    $DEBUG ||= ENV['DEBUG'] == 'true'
+    $DEBUG ||= ENV.fetch('DEBUG', nil) == 'true'
     GlobalTestEnv.remote_server.start if GlobalTestEnv.driver == :remote && ENV['WD_REMOTE_URL'].nil?
     GlobalTestEnv.print_env
   end
diff --git a/rb/spec/integration/selenium/webdriver/spec_support/rack_server.rb b/rb/spec/integration/selenium/webdriver/spec_support/rack_server.rb
@@ -30,7 +30,7 @@ def initialize(path, port = nil)
           @path = path
           @app  = TestApp.new(path)
 
-          @host = ENV['localhost'] || 'localhost'
+          @host = ENV.fetch('localhost', 'localhost')
           @port = Integer(port || PortProber.above(8180))
         end
 
diff --git a/rb/spec/integration/selenium/webdriver/spec_support/test_environment.rb b/rb/spec/integration/selenium/webdriver/spec_support/test_environment.rb
@@ -29,7 +29,7 @@ def initialize
 
           extract_browser_from_bazel_target_name
 
-          @driver = (ENV['WD_SPEC_DRIVER'] || :chrome).to_sym
+          @driver = ENV.fetch('WD_SPEC_DRIVER', 'chrome').to_sym
           @driver_instance = nil
         end
 
@@ -47,7 +47,7 @@ def print_env
         end
 
         def browser
-          driver == :remote ? (ENV['WD_REMOTE_BROWSER'] || :chrome).to_sym : driver
+          driver == :remote ? ENV.fetch('WD_REMOTE_BROWSER', 'chrome').to_sym : driver
         end
 
         def driver_instance
@@ -189,19 +189,19 @@ def create_remote_driver(opt = {})
           options = opt.delete(:capabilities)
           opt[:capabilities] = [WebDriver::Remote::Capabilities.send(browser)]
           opt[:capabilities] << options if options
-          opt[:url] = ENV['WD_REMOTE_URL'] || remote_server.webdriver_url
+          opt[:url] = ENV.fetch('WD_REMOTE_URL', remote_server.webdriver_url)
           opt[:http_client] ||= WebDriver::Remote::Http::Default.new
 
           WebDriver::Driver.for(:remote, opt)
         end
 
         def create_firefox_driver(opt = {})
-          WebDriver::Firefox.path = ENV['FIREFOX_BINARY'] if ENV['FIREFOX_BINARY']
+          WebDriver::Firefox.path = ENV.fetch('FIREFOX_BINARY', nil) if ENV.key?('FIREFOX_BINARY')
           WebDriver::Driver.for :firefox, opt
         end
 
         def create_firefox_nightly_driver(opt = {})
-          ENV['FIREFOX_BINARY'] = ENV['FIREFOX_NIGHTLY_BINARY']
+          ENV['FIREFOX_BINARY'] = ENV.fetch('FIREFOX_NIGHTLY_BINARY', nil)
           opt[:capabilities] = [
             WebDriver::Firefox::Options.new(debugger_address: true),
             WebDriver::Remote::Capabilities.firefox(web_socket_url: true)
@@ -215,7 +215,7 @@ def create_ie_driver(opt = {})
         end
 
         def create_chrome_driver(opt = {})
-          WebDriver::Chrome.path = ENV['CHROME_BINARY'] if ENV['CHROME_BINARY']
+          WebDriver::Chrome.path = ENV.fetch('CHROME_BINARY', nil) if ENV.key?('CHROME_BINARY')
           WebDriver::Driver.for :chrome, opt
         end
 
@@ -225,12 +225,12 @@ def create_safari_preview_driver(opt = {})
         end
 
         def create_edge_driver(opt = {})
-          WebDriver::Edge.path = ENV['EDGE_BINARY'] if ENV['EDGE_BINARY']
+          WebDriver::Edge.path = ENV.fetch('EDGE_BINARY', nil) if ENV.key?('EDGE_BINARY')
           WebDriver::Driver.for :edge, opt
         end
 
         def extract_browser_from_bazel_target_name
-          name = ENV['TEST_TARGET']
+          name = ENV.fetch('TEST_TARGET', nil)
           return unless name
 
           case name
diff --git a/rb/spec/unit/selenium/server_spec.rb b/rb/spec/unit/selenium/server_spec.rb
@@ -23,7 +23,7 @@
 module Selenium
   describe Server do
     let(:mock_process) { instance_double(ChildProcess::AbstractProcess).as_null_object }
-    let(:mock_poller) { instance_double('SocketPoller', connected?: true, closed?: true) }
+    let(:mock_poller) { instance_double(WebDriver::SocketPoller, connected?: true, closed?: true) }
     let(:repo) { 'https://api.github.com/repos/seleniumhq/selenium/releases' }
     let(:example_json) do
       [{url: "#{repo}/41272273",
@@ -191,7 +191,7 @@ module Selenium
     it 'raises Selenium::Server::Error if the server is not launched within the timeout' do
       allow(File).to receive(:exist?).with('selenium_server_deploy.jar').and_return(true)
 
-      poller = instance_double('SocketPoller')
+      poller = instance_double(WebDriver::SocketPoller)
       allow(poller).to receive(:connected?).and_return(false)
 
       server = Selenium::Server.new('selenium_server_deploy.jar', background: true)
diff --git a/rb/spec/unit/selenium/webdriver/common/action_builder_spec.rb b/rb/spec/unit/selenium/webdriver/common/action_builder_spec.rb
@@ -24,7 +24,7 @@ module WebDriver
     describe ActionBuilder do
       let(:keyboard) { Interactions.key('key') }
       let(:mouse) { Interactions.pointer(:mouse, name: 'mouse') }
-      let(:bridge) { instance_double('Bridge').as_null_object }
+      let(:bridge) { instance_double(Remote::Bridge).as_null_object }
       let(:builder) { ActionBuilder.new(bridge, devices: [mouse, keyboard]) }
       let(:async_builder) { ActionBuilder.new(bridge, devices: [mouse, keyboard], async: true) }
 
diff --git a/rb/spec/unit/selenium/webdriver/common/interactions/key_actions_spec.rb b/rb/spec/unit/selenium/webdriver/common/interactions/key_actions_spec.rb
diff --git a/rb/spec/unit/selenium/webdriver/common/interactions/pointer_actions_spec.rb b/rb/spec/unit/selenium/webdriver/common/interactions/pointer_actions_spec.rb
diff --git a/rb/spec/unit/selenium/webdriver/common/interactions/wheel_actions_spec.rb b/rb/spec/unit/selenium/webdriver/common/interactions/wheel_actions_spec.rb
diff --git a/rb/spec/unit/selenium/webdriver/search_context_spec.rb b/rb/spec/unit/selenium/webdriver/search_context_spec.rb
diff --git a/rb/spec/unit/selenium/webdriver/support/event_firing_spec.rb b/rb/spec/unit/selenium/webdriver/support/event_firing_spec.rb

Original file line number	Diff line number	Diff line change
`@@ -46,7 +46,7 @@ def ==(other)`
`46`	`46`	`alias_method :eql?, :==`
`47`	`47`
`48`	`48`	`def hash`
`49`		`- @id.hash ^ @bridge.hash`
	`49`	`+ [@id, @bridge].hash`
`50`	`50`	`end`
`51`	`51`
`52`	`52`	`#`
Original file line number	Diff line number	Diff line change
`@@ -45,7 +45,7 @@ def ==(other)`
`45`	`45`	`alias_method :eql?, :==`
`46`	`46`
`47`	`47`	`def hash`
`48`		`- @id.hash ^ @bridge.hash`
	`48`	`+ [@id, @bridge].hash`
`49`	`49`	`end`
`50`	`50`
`51`	`51`	`#`