Showing 12 of 12 total issues
Method vm_reconfigure
has a Cognitive Complexity of 21 (exceeds 11 allowed). Consider refactoring. Open
def vm_reconfigure(vm, options = {})
log_header = "EMS: [#{ext_management_system.name}] #{vm.class.name}: id [#{vm.id}], name [#{vm.name}], ems_ref [#{vm.ems_ref}]"
spec = options[:spec]
_log.info("#{log_header} Started...")
- Read upRead up
Cognitive Complexity
Cognitive Complexity is a measure of how difficult a unit of code is to intuitively understand. Unlike Cyclomatic Complexity, which determines how difficult your code will be to test, Cognitive Complexity tells you how difficult your code will be to read and comprehend.
A method's cognitive complexity is based on a few simple rules:
- Code is not considered more complex when it uses shorthand that the language provides for collapsing multiple statements into one
- Code is considered more complex for each "break in the linear flow of the code"
- Code is considered more complex when "flow breaking structures are nested"
Further reading
Cyclomatic complexity for vm_reconfigure is too high. [13/11] Open
def vm_reconfigure(vm, options = {})
log_header = "EMS: [#{ext_management_system.name}] #{vm.class.name}: id [#{vm.id}], name [#{vm.name}], ems_ref [#{vm.ems_ref}]"
spec = options[:spec]
_log.info("#{log_header} Started...")
- Read upRead up
- Exclude checks
Checks that the cyclomatic complexity of methods is not higher than the configured maximum. The cyclomatic complexity is the number of linearly independent paths through a method. The algorithm counts decision points and adds one.
An if statement (or unless or ?:) increases the complexity by one. An else branch does not, since it doesn't add a decision point. The && operator (or keyword and) can be converted to a nested if statement, and ||/or is shorthand for a sequence of ifs, so they also add one. Loops can be said to have an exit condition, so they add one. Blocks that are calls to builtin iteration methods (e.g. `ary.map{...}) also add one, others are ignored.
def each_child_node(*types) # count begins: 1
unless block_given? # unless: +1
return to_enum(__method__, *types)
children.each do |child| # each{}: +1
next unless child.is_a?(Node) # unless: +1
yield child if types.empty? || # if: +1, ||: +1
types.include?(child.type)
end
self
end # total: 6
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
- 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
metadata['rubygems_mfa_required']
must be set to 'true'
. Open
Gem::Specification.new do |spec|
spec.name = "manageiq-providers-red_hat_virtualization"
spec.version = ManageIQ::Providers::Redhat::VERSION
spec.authors = ["ManageIQ Authors"]
- Read upRead up
- 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
Call super
to initialize state of the parent class. Open
def initialize(args)
@keyed_futures_queue = []
@keyed_requests_queue = []
@parallel_processing_capacity = args[:batch_size] || DEFAULT_PARALLEL_PROCESSING_CAPACITY
@result_hash = {}
- Read upRead up
- Exclude checks
Checks for the presence of constructors and lifecycle callbacks
without calls to super
.
This cop does not consider method_missing
(and respond_to_missing?
)
because in some cases it makes sense to overtake what is considered a
missing method. In other cases, the theoretical ideal handling could be
challenging or verbose for no actual gain.
Autocorrection is not supported because the position of super
cannot be
determined automatically.
Object
and BasicObject
are allowed by this cop because of their
stateless nature. However, sometimes you might want to allow other parent
classes from this cop, for example in the case of an abstract class that is
not meant to be called with super
. In those cases, you can use the
AllowedParentClasses
option to specify which classes should be allowed
in addition to Object
and BasicObject
.
Example:
# bad
class Employee < Person
def initialize(name, salary)
@salary = salary
end
end
# good
class Employee < Person
def initialize(name, salary)
super(name)
@salary = salary
end
end
# bad
Employee = Class.new(Person) do
def initialize(name, salary)
@salary = salary
end
end
# good
Employee = Class.new(Person) do
def initialize(name, salary)
super(name)
@salary = salary
end
end
# bad
class Parent
def self.inherited(base)
do_something
end
end
# good
class Parent
def self.inherited(base)
super
do_something
end
end
# good
class ClassWithNoParent
def initialize
do_something
end
end
Example: AllowedParentClasses: [MyAbstractClass]
# good
class MyConcreteClass < MyAbstractClass
def initialize
do_something
end
end
Empty class detected. Open
class ManageIQ::Providers::Redhat::InfraManager::Scanning
end
- Read upRead up
- 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
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
- 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 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
- 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 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
- Exclude checks
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
- 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
Do not suppress exceptions. Open
rescue LoadError
- Read upRead up
- 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
Duplicate branch body detected. Open
else
- Read upRead up
- 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