From 26ce4eb0252a0d5a58d521d934ce991e08a64308 Mon Sep 17 00:00:00 2001 From: Titus Fortner Date: Wed, 8 Jun 2016 15:50:38 -0700 Subject: [PATCH] rb - Use a guard clause instead of wrapping the code inside a conditional expression --- rb/.rubocop_todo.yml | 26 ++------- rb/lib/selenium/server.rb | 10 ++-- rb/lib/selenium/webdriver/common/driver.rb | 5 +- .../common/html5/shared_web_storage.rb | 12 ++--- rb/lib/selenium/webdriver/common/keyboard.rb | 8 ++- rb/lib/selenium/webdriver/common/mouse.rb | 5 +- rb/lib/selenium/webdriver/common/platform.rb | 10 ++-- rb/lib/selenium/webdriver/common/proxy.rb | 5 +- .../selenium/webdriver/common/socket_lock.rb | 5 +- .../webdriver/common/socket_poller.rb | 7 +-- .../selenium/webdriver/common/touch_screen.rb | 5 +- rb/lib/selenium/webdriver/firefox/binary.rb | 5 +- rb/lib/selenium/webdriver/firefox/launcher.rb | 7 ++- rb/lib/selenium/webdriver/firefox/profile.rb | 5 +- .../webdriver/firefox/profiles_ini.rb | 5 +- rb/lib/selenium/webdriver/firefox/service.rb | 5 +- .../selenium/webdriver/phantomjs/service.rb | 5 +- .../selenium/webdriver/remote/http/default.rb | 53 +++++++++---------- rb/lib/selenium/webdriver/remote/response.rb | 9 ++-- .../webdriver/spec_support/rack_server.rb | 5 +- .../spec_support/test_environment.rb | 7 ++- 21 files changed, 78 insertions(+), 126 deletions(-) diff --git a/rb/.rubocop_todo.yml b/rb/.rubocop_todo.yml index b6aeb6820465d..facc8a6c4731c 100644 --- a/rb/.rubocop_todo.yml +++ b/rb/.rubocop_todo.yml @@ -1,6 +1,6 @@ # This configuration was generated by # `rubocop --auto-gen-config` -# on 2016-06-08 15:23:02 -0700 using RuboCop version 0.37.2. +# on 2016-06-08 15:50:57 -0700 using RuboCop version 0.37.2. # The point is for the user to remove these configuration records # one by one as the offenses are removed from the code base. # Note that changes in the inspected code, or installation of new @@ -11,7 +11,7 @@ Lint/AmbiguousRegexpLiteral: Exclude: - 'spec/integration/selenium/webdriver/firefox/marionette_spec.rb' -# Offense count: 13 +# Offense count: 12 # Configuration parameters: AllowSafeAssignment. Lint/AssignmentInCondition: Exclude: @@ -175,13 +175,12 @@ Style/Alias: - 'lib/selenium/webdriver/support/color.rb' - 'spec/integration/selenium/webdriver/spec_support/guards.rb' -# Offense count: 7 +# Offense count: 6 # Cop supports --auto-correct. # Configuration parameters: EnforcedStyle, SupportedStyles. # SupportedStyles: with_first_parameter, with_fixed_indentation Style/AlignParameters: Exclude: - - 'lib/selenium/webdriver/common/keyboard.rb' - 'lib/selenium/webdriver/remote/bridge.rb' - 'spec/integration/selenium/webdriver/element_spec.rb' - 'spec/integration/selenium/webdriver/spec_support/test_environment.rb' @@ -373,7 +372,7 @@ Style/EmptyLinesAroundMethodBody: Style/EmptyLinesAroundModuleBody: Enabled: false -# Offense count: 30 +# Offense count: 29 # Cop supports --auto-correct. # Configuration parameters: AllowForAlignment, ForceEqualSignAlignment. Style/ExtraSpacing: @@ -381,7 +380,6 @@ Style/ExtraSpacing: - 'lib/selenium/webdriver/chrome/profile.rb' - 'lib/selenium/webdriver/common/action_builder.rb' - 'lib/selenium/webdriver/common/error.rb' - - 'lib/selenium/webdriver/common/keyboard.rb' - 'lib/selenium/webdriver/common/log_entry.rb' - 'lib/selenium/webdriver/common/mouse.rb' - 'lib/selenium/webdriver/common/target_locator.rb' @@ -418,28 +416,14 @@ Style/FormatString: - 'lib/selenium/webdriver/safari/server.rb' - 'lib/selenium/webdriver/support/color.rb' -# Offense count: 7 -# Configuration parameters: MinBodyLength. -Style/GuardClause: - Exclude: - - 'lib/selenium/server.rb' - - 'lib/selenium/webdriver/common/**/*' - - 'lib/selenium/webdriver/firefox/*' - - 'lib/selenium/webdriver/phantomjs/service.rb' - - 'lib/selenium/webdriver/remote/*' - - 'spec/integration/selenium/webdriver/spec_support/*' - -# Offense count: 18 +# Offense count: 14 # Cop supports --auto-correct. # Configuration parameters: MaxLineLength. Style/IfUnlessModifier: Exclude: - 'lib/selenium/server.rb' - 'lib/selenium/webdriver/common/driver.rb' - - 'lib/selenium/webdriver/common/html5/shared_web_storage.rb' - 'lib/selenium/webdriver/edge/legacy_support.rb' - - 'lib/selenium/webdriver/firefox/service.rb' - - 'lib/selenium/webdriver/phantomjs/service.rb' - 'lib/selenium/webdriver/remote/bridge.rb' - 'lib/selenium/webdriver/remote/response.rb' - 'lib/selenium/webdriver/remote/w3c_bridge.rb' diff --git a/rb/lib/selenium/server.rb b/rb/lib/selenium/server.rb index 7604d59b79055..d0368d558196a 100644 --- a/rb/lib/selenium/server.rb +++ b/rb/lib/selenium/server.rb @@ -252,15 +252,13 @@ def process end def poll_for_service - unless socket.connected? - raise Error, "remote server not launched in #{@timeout} seconds" - end + return if socket.connected? + raise Error, "remote server not launched in #{@timeout} seconds" end def poll_for_shutdown - unless socket.closed? - raise Error, "remote server not stopped in #{@timeout} seconds" - end + return if socket.closed? + raise Error, "remote server not stopped in #{@timeout} seconds" end def socket diff --git a/rb/lib/selenium/webdriver/common/driver.rb b/rb/lib/selenium/webdriver/common/driver.rb index 53ba1eca1a9e8..82c97ce9f5a7d 100644 --- a/rb/lib/selenium/webdriver/common/driver.rb +++ b/rb/lib/selenium/webdriver/common/driver.rb @@ -93,9 +93,8 @@ def initialize(bridge) @bridge = bridge # TODO: refactor this away - unless bridge.driver_extensions.empty? - extend(*bridge.driver_extensions) - end + return if bridge.driver_extensions.empty? + extend(*bridge.driver_extensions) end def inspect diff --git a/rb/lib/selenium/webdriver/common/html5/shared_web_storage.rb b/rb/lib/selenium/webdriver/common/html5/shared_web_storage.rb index 5a0463b09a9f0..3dc3e7f907db9 100644 --- a/rb/lib/selenium/webdriver/common/html5/shared_web_storage.rb +++ b/rb/lib/selenium/webdriver/common/html5/shared_web_storage.rb @@ -31,15 +31,9 @@ def key?(key) alias_method :has_key?, :key? def fetch(key, &blk) - if self.key? key - return self[key] - end - - if block_given? - yield key - else - raise KeyError, "missing key #{key.inspect}" - end + return self[key] if self.key? key + return yield(key) if block_given? + raise KeyError, "missing key #{key.inspect}" end def empty? diff --git a/rb/lib/selenium/webdriver/common/keyboard.rb b/rb/lib/selenium/webdriver/common/keyboard.rb index c78e55580484b..4bf0331a09ce2 100644 --- a/rb/lib/selenium/webdriver/common/keyboard.rb +++ b/rb/lib/selenium/webdriver/common/keyboard.rb @@ -63,12 +63,10 @@ def release(key) MODIFIERS = [:control, :shift, :alt, :command, :meta] def assert_modifier(key) - unless MODIFIERS.include? key - raise ArgumentError, - "#{key.inspect} is not a modifier key, expected one of #{MODIFIERS.inspect}" - end + return if MODIFIERS.include? key + raise ArgumentError, "#{key.inspect} is not a modifier key, expected one of #{MODIFIERS.inspect}" end end # Keyboard end # WebDriver -end # Selenium +end # Selenium diff --git a/rb/lib/selenium/webdriver/common/mouse.rb b/rb/lib/selenium/webdriver/common/mouse.rb index 5e3fb4c1b9a1a..52d563ecbccf0 100755 --- a/rb/lib/selenium/webdriver/common/mouse.rb +++ b/rb/lib/selenium/webdriver/common/mouse.rb @@ -82,9 +82,8 @@ def move_if_needed(element) end def assert_element(element) - unless element.kind_of? Element - raise TypeError, "expected #{Element}, got #{element.inspect}:#{element.class}" - end + return if element.kind_of? Element + raise TypeError, "expected #{Element}, got #{element.inspect}:#{element.class}" end end # Mouse end # WebDriver diff --git a/rb/lib/selenium/webdriver/common/platform.rb b/rb/lib/selenium/webdriver/common/platform.rb index 0335033b79ab8..5757619453338 100644 --- a/rb/lib/selenium/webdriver/common/platform.rb +++ b/rb/lib/selenium/webdriver/common/platform.rb @@ -137,17 +137,15 @@ def make_writable(file) end def assert_file(path) - unless File.file? path - raise Error::WebDriverError, "not a file: #{path.inspect}" - end + return if File.file? path + raise Error::WebDriverError, "not a file: #{path.inspect}" end def assert_executable(path) assert_file(path) - unless File.executable? path - raise Error::WebDriverError, "not executable: #{path.inspect}" - end + return if File.executable? path + raise Error::WebDriverError, "not executable: #{path.inspect}" end def exit_hook(&blk) diff --git a/rb/lib/selenium/webdriver/common/proxy.rb b/rb/lib/selenium/webdriver/common/proxy.rb index f99ed535b2ec1..ce47eeec52dc4 100755 --- a/rb/lib/selenium/webdriver/common/proxy.rb +++ b/rb/lib/selenium/webdriver/common/proxy.rb @@ -53,9 +53,8 @@ def initialize(opts = {}) self.socks_username = opts.delete(:socks_username) if opts.has_key? :socks_username self.socks_password = opts.delete(:socks_password) if opts.has_key? :socks_password - unless opts.empty? - raise ArgumentError, "unknown option#{'s' if opts.size != 1}: #{opts.inspect}" - end + return if opts.empty? + raise ArgumentError, "unknown option#{'s' if opts.size != 1}: #{opts.inspect}" end def ==(other) diff --git a/rb/lib/selenium/webdriver/common/socket_lock.rb b/rb/lib/selenium/webdriver/common/socket_lock.rb index 255b8b855ad66..2c2a3c1fa6024 100644 --- a/rb/lib/selenium/webdriver/common/socket_lock.rb +++ b/rb/lib/selenium/webdriver/common/socket_lock.rb @@ -55,9 +55,8 @@ def lock sleep 0.1 end - unless did_lock? - raise Error::WebDriverError, "unable to bind to locking port #{@port} within #{@timeout} seconds" - end + return if did_lock? + raise Error::WebDriverError, "unable to bind to locking port #{@port} within #{@timeout} seconds" end def release diff --git a/rb/lib/selenium/webdriver/common/socket_poller.rb b/rb/lib/selenium/webdriver/common/socket_poller.rb index 1de53bf2de14f..6996403a52417 100644 --- a/rb/lib/selenium/webdriver/common/socket_poller.rb +++ b/rb/lib/selenium/webdriver/common/socket_poller.rb @@ -81,11 +81,8 @@ def listening? begin sock.connect_nonblock sockaddr rescue Errno::EINPROGRESS - if IO.select(nil, [sock], nil, CONNECT_TIMEOUT) - retry - else - raise Errno::ECONNREFUSED - end + retry if IO.select(nil, [sock], nil, CONNECT_TIMEOUT) + raise Errno::ECONNREFUSED rescue *CONNECTED_ERRORS # yay! end diff --git a/rb/lib/selenium/webdriver/common/touch_screen.rb b/rb/lib/selenium/webdriver/common/touch_screen.rb index b2f381e54f60b..5e66c74e25898 100755 --- a/rb/lib/selenium/webdriver/common/touch_screen.rb +++ b/rb/lib/selenium/webdriver/common/touch_screen.rb @@ -114,9 +114,8 @@ def coords_from(x, y) end def assert_element(element) - unless element.kind_of? Element - raise TypeError, "expected #{Element}, got #{element.inspect}:#{element.class}" - end + return if element.kind_of? Element + raise TypeError, "expected #{Element}, got #{element.inspect}:#{element.class}" end end # TouchScreen diff --git a/rb/lib/selenium/webdriver/firefox/binary.rb b/rb/lib/selenium/webdriver/firefox/binary.rb index 554eebbcb4157..dfb56c4bf3b72 100644 --- a/rb/lib/selenium/webdriver/firefox/binary.rb +++ b/rb/lib/selenium/webdriver/firefox/binary.rb @@ -93,9 +93,8 @@ def cope_with_mac_strangeness(args) # ensure we're ok sleep 0.3 - if @process.crashed? - raise Error::WebDriverError, "unable to start Firefox cleanly, args: #{args.inspect}" - end + return unless @process.crashed? + raise Error::WebDriverError, "unable to start Firefox cleanly, args: #{args.inspect}" end def modify_link_library_path(profile_path) diff --git a/rb/lib/selenium/webdriver/firefox/launcher.rb b/rb/lib/selenium/webdriver/firefox/launcher.rb index 1d54c21d0c6a3..e5397cb264e7c 100644 --- a/rb/lib/selenium/webdriver/firefox/launcher.rb +++ b/rb/lib/selenium/webdriver/firefox/launcher.rb @@ -85,10 +85,9 @@ def start def connect_until_stable poller = SocketPoller.new(@host, @port, STABLE_CONNECTION_TIMEOUT) - unless poller.connected? - @binary.quit - raise Error::WebDriverError, "unable to obtain stable firefox connection in #{STABLE_CONNECTION_TIMEOUT} seconds (#{@host}:#{@port})" - end + return if poller.connected? + @binary.quit + raise Error::WebDriverError, "unable to obtain stable firefox connection in #{STABLE_CONNECTION_TIMEOUT} seconds (#{@host}:#{@port})" end def fetch_profile diff --git a/rb/lib/selenium/webdriver/firefox/profile.rb b/rb/lib/selenium/webdriver/firefox/profile.rb index 66c4d122285f8..458c120a7a197 100644 --- a/rb/lib/selenium/webdriver/firefox/profile.rb +++ b/rb/lib/selenium/webdriver/firefox/profile.rb @@ -130,9 +130,8 @@ def log_file=(file) end def add_webdriver_extension - unless @extensions.has_key?(:webdriver) - add_extension(WEBDRIVER_EXTENSION_PATH, :webdriver) - end + return if @extensions.has_key?(:webdriver) + add_extension(WEBDRIVER_EXTENSION_PATH, :webdriver) end # diff --git a/rb/lib/selenium/webdriver/firefox/profiles_ini.rb b/rb/lib/selenium/webdriver/firefox/profiles_ini.rb index ed5652bc85ca2..0a436ba7b62e0 100644 --- a/rb/lib/selenium/webdriver/firefox/profiles_ini.rb +++ b/rb/lib/selenium/webdriver/firefox/profiles_ini.rb @@ -65,9 +65,8 @@ def parse end end - if p = path_for(name, is_relative, path) - @profile_paths[name] = p - end + return unless p = path_for(name, is_relative, path) + @profile_paths[name] = p end def path_for(name, is_relative, path) diff --git a/rb/lib/selenium/webdriver/firefox/service.rb b/rb/lib/selenium/webdriver/firefox/service.rb index f8bedcc6a29a1..fa7d27711278a 100644 --- a/rb/lib/selenium/webdriver/firefox/service.rb +++ b/rb/lib/selenium/webdriver/firefox/service.rb @@ -47,9 +47,8 @@ def start_process def stop_process super - if Platform.windows? && !$DEBUG - @process.io.close rescue nil - end + return unless Platform.windows? && !$DEBUG + @process.io.close rescue nil end def stop_server diff --git a/rb/lib/selenium/webdriver/phantomjs/service.rb b/rb/lib/selenium/webdriver/phantomjs/service.rb index c69431c6357e4..4618b90254250 100644 --- a/rb/lib/selenium/webdriver/phantomjs/service.rb +++ b/rb/lib/selenium/webdriver/phantomjs/service.rb @@ -46,9 +46,8 @@ def start_process def stop_process super - if Platform.jruby? && !$DEBUG - @process.io.close rescue nil - end + return unless Platform.jruby? && !$DEBUG + @process.io.close rescue nil end def stop_server diff --git a/rb/lib/selenium/webdriver/remote/http/default.rb b/rb/lib/selenium/webdriver/remote/http/default.rb index 000cf240b6c8a..dc041b7661f49 100644 --- a/rb/lib/selenium/webdriver/remote/http/default.rb +++ b/rb/lib/selenium/webdriver/remote/http/default.rb @@ -33,18 +33,18 @@ class Default < Common def http @http ||= ( - http = new_http_client - if server_url.scheme == "https" - http.use_ssl = true - http.verify_mode = OpenSSL::SSL::VERIFY_NONE - end + http = new_http_client + if server_url.scheme == "https" + http.use_ssl = true + http.verify_mode = OpenSSL::SSL::VERIFY_NONE + end - if @timeout - http.open_timeout = @timeout - http.read_timeout = @timeout - end + if @timeout + http.open_timeout = @timeout + http.read_timeout = @timeout + end - http + http ) end @@ -76,11 +76,8 @@ def request(verb, url, headers, payload, redirects = 0) retry rescue Errno::ECONNREFUSED => ex - if use_proxy? - raise ex.class, "using proxy: #{proxy.http}" - else - raise - end + raise ex.class, "using proxy: #{proxy.http}" if use_proxy? + raise end if response.kind_of? Net::HTTPRedirection @@ -124,13 +121,13 @@ def new_http_client def proxy @proxy ||= ( - proxy = ENV['http_proxy'] || ENV['HTTP_PROXY'] - no_proxy = ENV['no_proxy'] || ENV['NO_PROXY'] + proxy = ENV['http_proxy'] || ENV['HTTP_PROXY'] + no_proxy = ENV['no_proxy'] || ENV['NO_PROXY'] - if proxy - proxy = "http://#{proxy}" unless proxy.start_with?("http://") - Proxy.new(http: proxy, no_proxy: no_proxy) - end + if proxy + proxy = "http://#{proxy}" unless proxy.start_with?("http://") + Proxy.new(http: proxy, no_proxy: no_proxy) + end ) end @@ -140,13 +137,13 @@ def use_proxy? if proxy.no_proxy ignored = proxy.no_proxy.split(",").any? do |host| host == "*" || - host == server_url.host || ( - begin - IPAddr.new(host).include?(server_url.host) - rescue ArgumentError - false - end - ) + host == server_url.host || ( + begin + IPAddr.new(host).include?(server_url.host) + rescue ArgumentError + false + end + ) end diff --git a/rb/lib/selenium/webdriver/remote/response.rb b/rb/lib/selenium/webdriver/remote/response.rb index d0effd72a095e..48fd610996c64 100644 --- a/rb/lib/selenium/webdriver/remote/response.rb +++ b/rb/lib/selenium/webdriver/remote/response.rb @@ -67,11 +67,10 @@ def [](key) private def assert_ok - if e = error() - raise e - elsif @code.nil? || @code >= 400 - raise Error::ServerError, self - end + e = error() + raise e if e + return unless @code.nil? || @code >= 400 + raise Error::ServerError, self end def add_backtrace(ex) diff --git a/rb/spec/integration/selenium/webdriver/spec_support/rack_server.rb b/rb/spec/integration/selenium/webdriver/spec_support/rack_server.rb index a9e794b38d6e4..f4f46a87e1f7e 100644 --- a/rb/spec/integration/selenium/webdriver/spec_support/rack_server.rb +++ b/rb/spec/integration/selenium/webdriver/spec_support/rack_server.rb @@ -44,9 +44,8 @@ def start start_forked end - unless SocketPoller.new(@host, @port, START_TIMEOUT).connected? - raise "rack server not launched in #{START_TIMEOUT} seconds" - end + return if SocketPoller.new(@host, @port, START_TIMEOUT).connected? + raise "rack server not launched in #{START_TIMEOUT} seconds" end def run diff --git a/rb/spec/integration/selenium/webdriver/spec_support/test_environment.rb b/rb/spec/integration/selenium/webdriver/spec_support/test_environment.rb index dfb79ff5affff..ac393e916d000 100644 --- a/rb/spec/integration/selenium/webdriver/spec_support/test_environment.rb +++ b/rb/spec/integration/selenium/webdriver/spec_support/test_environment.rb @@ -59,10 +59,9 @@ def ensure_single_window end def quit_driver - if @driver_instance - @driver_instance.quit - @driver_instance = nil - end + return unless @driver_instance + @driver_instance.quit + @driver_instance = nil end def new_driver_instance