Showing 1,311 of 1,311 total issues
This loop will have at most one iteration. Open
each_group_resource(grp_idx) { |_sr| return true }
- Read upRead up
- Create a ticketCreate a ticket
- Exclude checks
Checks for loops that will have at most one iteration.
A loop that can never reach the second iteration is a possible error in the code.
In rare cases where only one iteration (or at most one iteration) is intended behavior,
the code should be refactored to use if
conditionals.
NOTE: Block methods that are used with Enumerable
s are considered to be loops.
AllowedPatterns
can be used to match against the block receiver in order to allow
code that would otherwise be registered as an offense (eg. times
used not in an
Enumerable
context).
Example:
# bad
while node
do_something(node)
node = node.parent
break
end
# good
while node
do_something(node)
node = node.parent
end
# bad
def verify_list(head)
item = head
begin
if verify(item)
return true
else
return false
end
end while(item)
end
# good
def verify_list(head)
item = head
begin
if verify(item)
item = item.next
else
return false
end
end while(item)
true
end
# bad
def find_something(items)
items.each do |item|
if something?(item)
return item
else
raise NotFoundError
end
end
end
# good
def find_something(items)
items.each do |item|
if something?(item)
return item
end
end
raise NotFoundError
end
# bad
2.times { raise ArgumentError }
Example: AllowedPatterns: ['(exactly|atleast|atmost)(\d+).times'] (default)
# good
exactly(2).times { raise StandardError }
Use filter_map
instead. Open
contents.collect { |c| c.resource if c.resource.kind_of?(MiqAction) }.compact
- Create a ticketCreate a ticket
- Exclude checks
Avoid immutable Array literals in loops. It is better to extract it into a local variable or a constant. Open
name = obj.send([:name, :description, :object_id].detect { |m| obj.respond_to?(m) })
- Create a ticketCreate a ticket
- Exclude checks
Use ==
if you meant to do a comparison or wrap the expression in parentheses to indicate you meant to assign in a condition. Open
if p = parent_cluster_or_host
- Read upRead up
- Create a ticketCreate a ticket
- Exclude checks
Checks for assignments in the conditions of if/while/until.
AllowSafeAssignment
option for safe assignment.
By safe assignment we mean putting parentheses around
an assignment to indicate "I know I'm using an assignment
as a condition. It's not a mistake."
Safety:
This cop's autocorrection is unsafe because it assumes that the author meant to use an assignment result as a condition.
Example:
# bad
if some_var = true
do_something
end
# good
if some_var == true
do_something
end
Example: AllowSafeAssignment: true (default)
# good
if (some_var = true)
do_something
end
Example: AllowSafeAssignment: false
# bad
if (some_var = true)
do_something
end
Avoid using Marshal.load
. Open
Marshal.load(Base64.decode64(data.split("\n").join))
- Read upRead up
- Create a ticketCreate a ticket
- Exclude checks
Checks for the use of Marshal class methods which have potential security issues leading to remote code execution when loading from an untrusted source.
Example:
# bad
Marshal.load("{}")
Marshal.restore("{}")
# good
Marshal.dump("{}")
# okish - deep copy hack
Marshal.load(Marshal.dump({}))
private
(on line 157) does not make singleton methods private. Use private_class_method
or private
inside a class << self
block instead. Open
def self.profile_for_user_tz(user_id, user_tz)
- Read upRead up
- Create a ticketCreate a ticket
- Exclude checks
Checks for private
or protected
access modifiers which are
applied to a singleton method. These access modifiers do not make
singleton methods private/protected. private_class_method
can be
used for that.
Example:
# bad
class C
private
def self.method
puts 'hi'
end
end
Example:
# good
class C
def self.method
puts 'hi'
end
private_class_method :method
end
Example:
# good
class C
class << self
private
def method
puts 'hi'
end
end
end
Do not suppress exceptions. Open
rescue
- Read upRead up
- Create a ticketCreate a ticket
- Exclude checks
Checks for rescue
blocks with no body.
Example:
# bad
def some_method
do_something
rescue
end
# bad
begin
do_something
rescue
end
# good
def some_method
do_something
rescue
handle_exception
end
# good
begin
do_something
rescue
handle_exception
end
Example: AllowComments: true (default)
# good
def some_method
do_something
rescue
# do nothing
end
# good
begin
do_something
rescue
# do nothing
end
Example: AllowComments: false
# bad
def some_method
do_something
rescue
# do nothing
end
# bad
begin
do_something
rescue
# do nothing
end
Example: AllowNil: true (default)
# good
def some_method
do_something
rescue
nil
end
# good
begin
do_something
rescue
# do nothing
end
# good
do_something rescue nil
Example: AllowNil: false
# bad
def some_method
do_something
rescue
nil
end
# bad
begin
do_something
rescue
nil
end
# bad
do_something rescue nil
Use filter_map
instead. Open
perfs.collect(&:abs_max_cpu_usagemhz_rate_average_value).compact.max
- Create a ticketCreate a ticket
- Exclude checks
Script file Rakefile doesn't have execute permission. Open
#!/usr/bin/env rake
- Read upRead up
- Create a ticketCreate a ticket
- Exclude checks
Checks if a file which has a shebang line as its first line is granted execute permission.
Example:
# bad
# A file which has a shebang line as its first line is not
# granted execute permission.
#!/usr/bin/env ruby
puts 'hello, world'
# good
# A file which has a shebang line as its first line is
# granted execute permission.
#!/usr/bin/env ruby
puts 'hello, world'
# good
# A file which has not a shebang line as its first line is not
# granted execute permission.
puts 'hello, world'
Avoid immutable Array literals in loops. It is better to extract it into a local variable or a constant. Open
timings.delete_if { |k, _v| [:start_range, :end_range, :num_vim_queries, :num_vim_trips, :collect_metrics].include?(k) }
- Create a ticketCreate a ticket
- Exclude checks
Duplicate branch body detected. Open
elsif old_application_name?(activity) && !activity["application_name"].include?(" Server")
return true
- Read upRead up
- Create a ticketCreate a ticket
- Exclude checks
Checks that there are no repeated bodies
within if/unless
, case-when
, case-in
and rescue
constructs.
With IgnoreLiteralBranches: true
, branches are not registered
as offenses if they return a basic literal value (string, symbol,
integer, float, rational, complex, true
, false
, or nil
), or
return an array, hash, regexp or range that only contains one of
the above basic literal values.
With IgnoreConstantBranches: true
, branches are not registered
as offenses if they return a constant value.
Example:
# bad
if foo
do_foo
do_something_else
elsif bar
do_foo
do_something_else
end
# good
if foo || bar
do_foo
do_something_else
end
# bad
case x
when foo
do_foo
when bar
do_foo
else
do_something_else
end
# good
case x
when foo, bar
do_foo
else
do_something_else
end
# bad
begin
do_something
rescue FooError
handle_error
rescue BarError
handle_error
end
# good
begin
do_something
rescue FooError, BarError
handle_error
end
Example: IgnoreLiteralBranches: true
# good
case size
when "small" then 100
when "medium" then 250
when "large" then 1000
else 250
end
Example: IgnoreConstantBranches: true
# good
case size
when "small" then SMALL_SIZE
when "medium" then MEDIUM_SIZE
when "large" then LARGE_SIZE
else MEDIUM_SIZE
end
Do not mix named captures and numbered captures in a Regexp literal. Open
REGEX = /
(?<model_name>(?:[[:upper:]][[:alnum:]]*(?:::[[:upper:]][[:alnum:]]*)*)?)
\.?(?<associations>([a-z_]+\.)*)
(?<namespace>\bmanaged|user_tag\b)
-(?<column>[a-z]+[_:[:alnum:]]+)
- Read upRead up
- Create a ticketCreate a ticket
- Exclude checks
Do not mix named captures and numbered captures in a Regexp literal because numbered capture is ignored if they're mixed. Replace numbered captures with non-capturing groupings or named captures.
Example:
# bad
/(?<foo>FOO)(BAR)/
# good
/(?<foo>FOO)(?<bar>BAR)/
# good
/(?<foo>FOO)(?:BAR)/
# good
/(FOO)(BAR)/</foo></bar></foo></foo>
Avoid immutable Array literals in loops. It is better to extract it into a local variable or a constant. Open
%w[id created_on updated_on].each { |key| profile.delete(key) }
- Create a ticketCreate a ticket
- Exclude checks
Avoid immutable Array literals in loops. It is better to extract it into a local variable or a constant. Open
unless ["<compare>", "<drift>"].include?(mri.db)
- Create a ticketCreate a ticket
- Exclude checks
Use filter_map
instead. Open
resource.hosts.includes(:hardware).collect { |h| h.hardware.try(:cpu_sockets) }.compact.sum
- Create a ticketCreate a ticket
- Exclude checks
Use String#include?
instead of a regex match with literal-only pattern. Open
raise unless /already exists/.match?(e.message)
- Create a ticketCreate a ticket
- Exclude checks
Use String#include?
instead of a regex match with literal-only pattern. Open
raise unless /not found/.match?(e.message)
- Create a ticketCreate a ticket
- Exclude checks
Use filter_map
instead. Open
!hardware.nil? ? hardware.nics.collect(&:lan).compact : []
- Create a ticketCreate a ticket
- Exclude checks
Prefer using YAML.safe_load
over YAML.load
. Open
widgets = YAML.load(import_file_upload.uploaded_content)
- Read upRead up
- Create a ticketCreate a ticket
- Exclude checks
Checks for the use of YAML class methods which have potential security issues leading to remote code execution when loading from an untrusted source.
NOTE: Ruby 3.1+ (Psych 4) uses Psych.load
as Psych.safe_load
by default.
Safety:
The behavior of the code might change depending on what was
in the YAML payload, since YAML.safe_load
is more restrictive.
Example:
# bad
YAML.load("--- !ruby/object:Foo {}") # Psych 3 is unsafe by default
# good
YAML.safe_load("--- !ruby/object:Foo {}", [Foo]) # Ruby 2.5 (Psych 3)
YAML.safe_load("--- !ruby/object:Foo {}", permitted_classes: [Foo]) # Ruby 3.0- (Psych 3)
YAML.load("--- !ruby/object:Foo {}", permitted_classes: [Foo]) # Ruby 3.1+ (Psych 4)
YAML.dump(foo)
Avoid immutable Array literals in loops. It is better to extract it into a local variable or a constant. Open
%w[server_id
hostname
ipaddress]
- Create a ticketCreate a ticket
- Exclude checks