Skip to content
New issue

Have a question about this project? Sign up for a free GitHub account to open an issue and contact its maintainers and the community.

By clicking “Sign up for GitHub”, you agree to our terms of service and privacy statement. We’ll occasionally send you account related emails.

Already on GitHub? Sign in to your account

Remove negative check for nil #446

Open
wants to merge 3 commits into
base: main
Choose a base branch
from

Conversation

ashmaroli
Copy link
Contributor

Technically, if an object is not nil, it is either a Boolean false or a truthy.

I'm making an assumption here that false can be ignored because this check is used against values set by :parse method.
Therefore, the test for != nil is redundant.

@@ -2502,7 +2501,7 @@ def validate
unreserved = CharacterClasses::UNRESERVED
sub_delims = CharacterClasses::SUB_DELIMS
if !self.host.nil? && (self.host =~ /[<>{}\/\\\?\#\@"[[:space:]]]/ ||
(self.host[/^\[(.*)\]$/, 1] != nil && self.host[/^\[(.*)\]$/, 1] !~
(self.host[/^\[(.*)\]$/, 1] && self.host[/^\[(.*)\]$/, 1] !~
Copy link
Collaborator

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Style/RedundantSelf: Redundant self detected.

This comment was marked as spam.

raise InvalidURIError,
"Cannot have a relative path with an authority set: '#{self.to_s}'"
end
if self.path != nil && !self.path.empty? &&
if self.path && !self.path.empty? &&
Copy link
Collaborator

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Style/RedundantSelf: Redundant self detected.

if self.path != nil && !self.path.empty? && self.path[0..0] != SLASH &&
self.authority != nil
if self.path && !self.path.empty? && self.path[0..0] != SLASH &&
self.authority
Copy link
Collaborator

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Layout/MultilineOperationIndentation: Align the operands of a condition in an if statement spanning multiple lines.
Style/RedundantSelf: Redundant self detected.

raise InvalidURIError, "Hostname not supplied: '#{self.to_s}'"
end
end
if self.path != nil && !self.path.empty? && self.path[0..0] != SLASH &&
self.authority != nil
if self.path && !self.path.empty? && self.path[0..0] != SLASH &&
Copy link
Collaborator

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Style/RedundantSelf: Redundant self detected.

self.password != nil
if self.port ||
self.user ||
self.password
Copy link
Collaborator

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Layout/MultilineOperationIndentation: Align the operands of a condition in an if statement spanning multiple lines.
Style/RedundantSelf: Redundant self detected.

self.path =~ NORMPATH
raise InvalidURIError,
"Cannot assemble URI string with ambiguous path: '#{self.path}'"
end
@uri_string ||= begin
uri_string = String.new
uri_string << "#{self.scheme}:" if self.scheme != nil
uri_string << "//#{self.authority}" if self.authority != nil
uri_string << "#{self.scheme}:" if self.scheme
Copy link
Collaborator

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Style/RedundantSelf: Redundant self detected.

@@ -2359,18 +2358,18 @@ def empty?
#
# @return [String] The URI's <code>String</code> representation.
def to_s
if self.scheme == nil && self.path != nil && !self.path.empty? &&
if self.scheme == nil && self.path && !self.path.empty? &&
Copy link
Collaborator

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Style/RedundantSelf: Redundant self detected.
Style/NilComparison: Prefer the use of the nil? predicate.

@@ -1971,7 +1970,7 @@ def join(uri)

# If the base path is empty and an authority segment has been
# defined, use a base path of SLASH
if base_path.empty? && self.authority != nil
if base_path.empty? && self.authority
Copy link
Collaborator

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Style/RedundantSelf: Redundant self detected.

@@ -1947,7 +1946,7 @@ def join(uri)
else
if uri.path == nil || uri.path.empty?
joined_path = self.path
if uri.query != nil
if uri.query
Copy link
Collaborator

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Metrics/BlockNesting: Avoid more than 3 levels of block nesting.
Style/ConditionalAssignment: Use the return of the conditional for variable assignment and comparison.

site_string << "#{self.normalized_scheme}:"
end
if self.normalized_authority != nil
if self.normalized_authority
Copy link
Collaborator

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Style/RedundantSelf: Redundant self detected.

@@ -1498,10 +1497,10 @@ def normalized_site
return nil unless self.site
@normalized_site ||= begin
site_string = "".dup
if self.normalized_scheme != nil
if self.normalized_scheme
Copy link
Collaborator

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Style/RedundantSelf: Redundant self detected.

site_string << "#{self.scheme}:" if self.scheme != nil
site_string << "//#{self.authority}" if self.authority != nil
site_string << "#{self.scheme}:" if self.scheme
site_string << "//#{self.authority}" if self.authority
Copy link
Collaborator

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Style/RedundantSelf: Redundant self detected.

@@ -1479,8 +1478,8 @@ def default_port
def site
(self.scheme || self.authority) && @site ||= begin
site_string = "".dup
site_string << "#{self.scheme}:" if self.scheme != nil
site_string << "//#{self.authority}" if self.authority != nil
site_string << "#{self.scheme}:" if self.scheme
Copy link
Collaborator

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Style/RedundantSelf: Redundant self detected.

if new_port != nil && !(new_port.to_s =~ /^\d+$/)
raise InvalidURIError,
"Invalid port number: #{new_port.inspect}"
if new_port && !(new_port.to_s =~ /^\d+$/)
Copy link
Collaborator

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Style/InverseMethods: Use !~ instead of inverting =~.

@@ -1418,17 +1418,16 @@ def normalized_port
#
# @param [String, Integer, #to_s] new_port The new port component.
def port=(new_port)
if new_port != nil && new_port.respond_to?(:to_str)
if new_port && new_port.respond_to?(:to_str)
Copy link
Collaborator

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Style/SafeNavigation: Use safe navigation (&.) instead of checking if an object exists before calling the method.

authority << "#{self.normalized_userinfo}@"
end
authority << self.normalized_host
if self.normalized_port != nil
if self.normalized_port
Copy link
Collaborator

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Style/RedundantSelf: Redundant self detected.

@@ -1261,11 +1261,11 @@ def normalized_authority
return nil unless self.authority
@normalized_authority ||= begin
authority = String.new
if self.normalized_userinfo != nil
if self.normalized_userinfo
Copy link
Collaborator

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Style/RedundantSelf: Redundant self detected.

authority << "#{self.userinfo}@"
end
authority << self.host
if self.port != nil
if self.port
Copy link
Collaborator

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Style/RedundantSelf: Redundant self detected.

@@ -1242,11 +1242,11 @@ def domain
def authority
self.host && @authority ||= begin
authority = String.new
if self.userinfo != nil
if self.userinfo
Copy link
Collaborator

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Style/RedundantSelf: Redundant self detected.

@@ -686,7 +686,7 @@ def self.normalized_encode(uri, return_type=String)
:fragment => self.unencode_component(uri_object.fragment)
}
components.each do |key, value|
if value != nil
if value
Copy link
Collaborator

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Style/Next: Use next to skip iteration.

@@ -2502,7 +2501,7 @@ def validate
unreserved = CharacterClasses::UNRESERVED
sub_delims = CharacterClasses::SUB_DELIMS
if !self.host.nil? && (self.host =~ /[<>{}\/\\\?\#\@"[[:space:]]]/ ||
(self.host[/^\[(.*)\]$/, 1] != nil && self.host[/^\[(.*)\]$/, 1] !~
(self.host[/^\[(.*)\]$/, 1] && self.host[/^\[(.*)\]$/, 1] !~

This comment was marked as spam.

@dpep
Copy link
Contributor

dpep commented Dec 15, 2022

nice simplifications! in a similar vein, what do you think of swapping return nil if ... for return if ... ?

Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
None yet
Projects
None yet
Development

Successfully merging this pull request may close these issues.

None yet

6 participants