Showing 37 of 37 total issues
Avoid hard coding large quantities of data in code. Prefer reading the data from an external source. Open
EVENT_CODES = {
# Code Description
0 => "UNASSIGNED",
1 => "VDC_START",
2 => "VDC_STOP",
- Read upRead up
- Create a ticketCreate a ticket
- Exclude checks
Checks for literals with extremely many entries. This is indicative of configuration or data that may be better extracted somewhere else, like a database, fetched from an API, or read from a non-code file (CSV, JSON, YAML, etc.).
Example:
# bad
# Huge Array literal
[1, 2, '...', 999_999_999]
# bad
# Huge Hash literal
{ 1 => 1, 2 => 2, '...' => '...', 999_999_999 => 999_999_999}
# bad
# Huge Set "literal"
Set[1, 2, '...', 999_999_999]
# good
# Reasonably sized Array literal
[1, 2, '...', 10]
# good
# Reading huge Array from external data source
# File.readlines('numbers.txt', chomp: true).map!(&:to_i)
# good
# Reasonably sized Hash literal
{ 1 => 1, 2 => 2, '...' => '...', 10 => 10}
# good
# Reading huge Hash from external data source
CSV.foreach('numbers.csv', headers: true).each_with_object({}) do |row, hash|
hash[row["key"].to_i] = row["value"].to_i
end
# good
# Reasonably sized Set "literal"
Set[1, 2, '...', 10]
# good
# Reading huge Set from external data source
SomeFramework.config_for(:something)[:numbers].to_set
Use ==
if you meant to do a comparison or wrap the expression in parentheses to indicate you meant to assign in a condition. Open
return unless content = customization_template_content
- 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 rescuing the Exception
class. Perhaps you meant to rescue StandardError
? Open
rescue Exception => err
msg = "#{log_header}: Connection to [#{ems_display_text}] failed for VM:[#{@vm_cfg_file}] with error [#{err}] after [#{Time.now - st}] seconds"
$log.error msg
raise
- Read upRead up
- Create a ticketCreate a ticket
- Exclude checks
Checks for rescue
blocks targeting the Exception class.
Example:
# bad
begin
do_something
rescue Exception
handle_exception
end
Example:
# good
begin
do_something
rescue ArgumentError
handle_exception
end
Use filter_map
instead. Open
references(:hosts).map do |ems_ref|
connection.system_service.hosts_service.host_service(uuid_from_ems_ref(ems_ref)).get
rescue OvirtSDK4::Error # when 404
nil
end.compact
- Create a ticketCreate a ticket
- Exclude checks
Avoid rescuing the Exception
class. Perhaps you meant to rescue StandardError
? Open
rescue Exception => e
self.class.handle_credentials_verification_error(e)
- Read upRead up
- Create a ticketCreate a ticket
- Exclude checks
Checks for rescue
blocks targeting the Exception class.
Example:
# bad
begin
do_something
rescue Exception
handle_exception
end
Example:
# good
begin
do_something
rescue ArgumentError
handle_exception
end
Use filter_map
instead. Open
vm.storages.collect(&:ems_ref).compact.each { |ems_ref| add_target!(:storagedomains, ems_ref) } unless vm.storages.nil?
- Create a ticketCreate a ticket
- Exclude checks
metadata['rubygems_mfa_required']
must be set to 'true'
. Open
Gem::Specification.new do |spec|
spec.name = "manageiq-providers-ovirt"
spec.version = ManageIQ::Providers::Ovirt::VERSION
spec.authors = ["ManageIQ Authors"]
- Read upRead up
- Create a ticketCreate a ticket
- Exclude checks
Requires a gemspec to have rubygems_mfa_required
metadata set.
This setting tells RubyGems that MFA (Multi-Factor Authentication) is required for accounts to be able perform privileged operations, such as (see RubyGems' documentation for the full list of privileged operations):
gem push
gem yank
gem owner --add/remove
- adding or removing owners using gem ownership page
This helps make your gem more secure, as users can be more confident that gem updates were pushed by maintainers.
Example:
# bad
Gem::Specification.new do |spec|
# no `rubygems_mfa_required` metadata specified
end
# good
Gem::Specification.new do |spec|
spec.metadata = {
'rubygems_mfa_required' => 'true'
}
end
# good
Gem::Specification.new do |spec|
spec.metadata['rubygems_mfa_required'] = 'true'
end
# bad
Gem::Specification.new do |spec|
spec.metadata = {
'rubygems_mfa_required' => 'false'
}
end
# good
Gem::Specification.new do |spec|
spec.metadata = {
'rubygems_mfa_required' => 'true'
}
end
# bad
Gem::Specification.new do |spec|
spec.metadata['rubygems_mfa_required'] = 'false'
end
# good
Gem::Specification.new do |spec|
spec.metadata['rubygems_mfa_required'] = 'true'
end
Useless method definition detected. Open
def set_or_default_hardware_field_values(vm)
super(vm)
end
- Read upRead up
- Create a ticketCreate a ticket
- Exclude checks
Checks for useless method definitions, specifically: empty constructors
and methods just delegating to super
.
Safety:
This cop is unsafe as it can register false positives for cases when an empty constructor just overrides the parent constructor, which is bad anyway.
Example:
# bad
def initialize
super
end
def method
super
end
# good - with default arguments
def initialize(x = Object.new)
super
end
# good
def initialize
super
initialize_internals
end
def method(*args)
super(:extra_arg, *args)
end
Wrap expressions with varying precedence with parentheses to avoid ambiguity. Open
host_os && host_os.type || host.type
- Read upRead up
- Create a ticketCreate a ticket
- Exclude checks
Looks for expressions containing multiple binary operators
where precedence is ambiguous due to lack of parentheses. For example,
in 1 + 2 * 3
, the multiplication will happen before the addition, but
lexically it appears that the addition will happen first.
The cop does not consider unary operators (ie. !a
or -b
) or comparison
operators (ie. a =~ b
) because those are not ambiguous.
NOTE: Ranges are handled by Lint/AmbiguousRange
.
Example:
# bad
a + b * c
a || b && c
a ** b + c
# good (different precedence)
a + (b * c)
a || (b && c)
(a ** b) + c
# good (same precedence)
a + b + c
a * b / c % d
Avoid rescuing the Exception
class. Perhaps you meant to rescue StandardError
? Open
rescue Exception => err
_log.warn("#{log_prefix} Event Monitor Stop errored because [#{err.message}]")
_log.warn("#{log_prefix} Error details: [#{err.details}]")
_log.log_backtrace(err)
- Read upRead up
- Create a ticketCreate a ticket
- Exclude checks
Checks for rescue
blocks targeting the Exception class.
Example:
# bad
begin
do_something
rescue Exception
handle_exception
end
Example:
# good
begin
do_something
rescue ArgumentError
handle_exception
end
Use ==
if you meant to do a comparison or wrap the expression in parentheses to indicate you meant to assign in a condition. Open
return unless content = customization_template_content
- 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
Duplicate branch body detected. Open
else
- 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
Use filter_map
instead. Open
disks_spec.collect do |disk_spec|
disk = prepare_disk_for_add(disk_spec)
_log.info("disk: #{disk.inspect}")
disk
end.compact
- Create a ticketCreate a ticket
- Exclude checks
Avoid rescuing the Exception
class. Perhaps you meant to rescue StandardError
? Open
rescue Exception => err
_log.error("#{log_header} Unhandled exception during perf data collection: [#{err}], class: [#{err.class}]")
_log.error("#{log_header} Timings at time of error: #{Benchmark.current_realtime.inspect}")
_log.log_backtrace(err)
raise
- Read upRead up
- Create a ticketCreate a ticket
- Exclude checks
Checks for rescue
blocks targeting the Exception class.
Example:
# bad
begin
do_something
rescue Exception
handle_exception
end
Example:
# good
begin
do_something
rescue ArgumentError
handle_exception
end
Use any?(ExtManagementSystem)
instead of block. Open
ems_in_list = targets.any? { |t| t.kind_of?(ExtManagementSystem) }
- Create a ticketCreate a ticket
- Exclude checks
Empty class detected. Open
class ManageIQ::Providers::Ovirt::InfraManager::Scanning
end
- Read upRead up
- Create a ticketCreate a ticket
- Exclude checks
Checks for classes and metaclasses without a body. Such empty classes and metaclasses are typically an oversight or we should provide a comment to be clearer what we're aiming for.
Example:
# bad
class Foo
end
class Bar
class << self
end
end
class << obj
end
# good
class Foo
def do_something
# ... code
end
end
class Bar
class << self
attr_reader :bar
end
end
class << obj
attr_reader :bar
end
Example: AllowComments: false (default)
# bad
class Foo
# TODO: implement later
end
class Bar
class << self
# TODO: implement later
end
end
class << obj
# TODO: implement later
end
Example: AllowComments: true
# good
class Foo
# TODO: implement later
end
class Bar
class << self
# TODO: implement later
end
end
class << obj
# TODO: implement later
end
Do not suppress exceptions. Open
rescue LoadError
- 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