ManageIQ/manageiq

View on GitHub

Showing 1,311 of 1,311 total issues

Do not shadow rescued Exceptions.
Open

    rescue Timeout::Error, StandardError => scanErr
      last_err = scanErr
Severity: Minor
Found in app/models/mixins/scanning_mixin.rb by rubocop

Checks for a rescued exception that get shadowed by a less specific exception being rescued before a more specific exception is rescued.

An exception is considered shadowed if it is rescued after its ancestor is, or if it and its ancestor are both rescued in the same rescue statement. In both cases, the more specific rescue is unnecessary because it is covered by rescuing the less specific exception. (ie. rescue Exception, StandardError has the same behavior whether StandardError is included or not, because all StandardErrors are rescued by rescue Exception).

Example:

# bad

begin
  something
rescue Exception
  handle_exception
rescue StandardError
  handle_standard_error
end

# bad
begin
  something
rescue Exception, StandardError
  handle_error
end

# good

begin
  something
rescue StandardError
  handle_standard_error
rescue Exception
  handle_exception
end

# good, however depending on runtime environment.
#
# This is a special case for system call errors.
# System dependent error code depends on runtime environment.
# For example, whether `Errno::EAGAIN` and `Errno::EWOULDBLOCK` are
# the same error code or different error code depends on environment.
# This good case is for `Errno::EAGAIN` and `Errno::EWOULDBLOCK` with
# the same error code.
begin
  something
rescue Errno::EAGAIN, Errno::EWOULDBLOCK
  handle_standard_error
end

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 type state status message].each { |key| nh.delete(key) }
Severity: Minor
Found in app/models/service_template.rb by rubocop

Use filter_map instead.
Open

    build_id_to_name_hash(sources.collect { |h| find_cluster_above_ci(h) }.compact)
Severity: Minor
Found in app/models/miq_request_workflow.rb by rubocop

Avoid rescuing the Exception class. Perhaps you meant to rescue StandardError?
Open

    rescue Exception => err
      _log.error(err.to_s)
      _log.log_backtrace(err, :debug)
      job.signal(:abort_retry, err.to_s, "error", true)
      nil

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

Remove redundant sort.
Open

    Dir.glob(File.join(WIDGET_DIR, "*.yaml")).sort.each { |f| sync_from_file(f) }
Severity: Minor
Found in app/models/miq_widget_set.rb by rubocop

Sort globbed results by default in Ruby 3.0. This cop checks for redundant sort method to Dir.glob and Dir[].

Safety:

This cop is unsafe, in case of having a file and a directory with identical names, since directory will be loaded before the file, which will break exe/files.rb that rely on exe.rb file.

Example:

# bad
Dir.glob('./lib/**/*.rb').sort.each do |file|
end

Dir['./lib/**/*.rb'].sort.each do |file|
end

# good
Dir.glob('./lib/**/*.rb').each do |file|
end

Dir['./lib/**/*.rb'].each do |file|
end

Variable rel used in void context.
Open

    rel

Checks for operators, variables, literals, lambda, proc and nonmutating methods used in void context.

Example: CheckForMethodsWithNoSideEffects: false (default)

# bad
def some_method
  some_num * 10
  do_something
end

def some_method(some_var)
  some_var
  do_something
end

Example: CheckForMethodsWithNoSideEffects: true

# bad
def some_method(some_array)
  some_array.sort
  do_something(some_array)
end

# good
def some_method
  do_something
  some_num * 10
end

def some_method(some_var)
  do_something
  some_var
end

def some_method(some_array)
  some_array.sort!
  do_something(some_array)
end

Avoid using Marshal.load.
Open

      xml_file, data_type = Marshal.load(data_array)
Severity: Minor
Found in app/models/mixins/scanning_mixin.rb by rubocop

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({}))

Use filter_map instead.
Open

    @floating_ip_addresses ||= floating_ips.collect(&:address).compact.uniq
Severity: Minor
Found in app/models/network_port.rb by rubocop

Wrap complex range boundaries with parentheses to avoid ambiguity.
Open

          params[attr['dialog_param_'.size..-1]] = val

Checks for ambiguous ranges.

Ranges have quite low precedence, which leads to unexpected behavior when using a range with other operators. This cop avoids that by making ranges explicit by requiring parenthesis around complex range boundaries (anything that is not a literal: numerics, strings, symbols, etc.).

This cop can be configured with RequireParenthesesForMethodChains in order to specify whether method chains (including self.foo) should be wrapped in parens by this cop.

NOTE: Regardless of this configuration, if a method receiver is a basic literal value, it will be wrapped in order to prevent the ambiguity of 1..2.to_a.

Safety:

The cop autocorrects by wrapping the entire boundary in parentheses, which makes the outcome more explicit but is possible to not be the intention of the programmer. For this reason, this cop's autocorrect is unsafe (it will not change the behavior of the code, but will not necessarily match the intent of the program).

Example:

# bad
x || 1..2
(x || 1..2)
1..2.to_a

# good, unambiguous
1..2
'a'..'z'
:bar..:baz
MyClass::MIN..MyClass::MAX
@min..@max
a..b
-a..b

# good, ambiguity removed
x || (1..2)
(x || 1)..2
(x || 1)..(y || 2)
(1..2).to_a

Example: RequireParenthesesForMethodChains: false (default)

# good
a.foo..b.bar
(a.foo)..(b.bar)

Example: RequireParenthesesForMethodChains: true

# bad
a.foo..b.bar

# good
(a.foo)..(b.bar)

Avoid more than 3 levels of block nesting.
Open

              nh[:create_time] = e1.attributes['cdate_on_disk'] if e1.attributes['cdate_on_disk'].present?
Severity: Minor
Found in app/models/snapshot.rb by rubocop

Checks for excessive nesting of conditional and looping constructs.

You can configure if blocks are considered using the CountBlocks option. When set to false (the default) blocks are not counted towards the nesting level. Set to true to count blocks as well.

The maximum level of nesting allowed is configurable.

Wrap expressions with varying precedence with parentheses to avoid ambiguity.
Open

    hdr << "@page{@bottom-center{font-size: 75%;content: '" + _("Report date: %{report_date}") % {:report_date => run_date} + "'}}"
Severity: Minor
Found in app/models/miq_report_result.rb by rubocop

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

Use filter_map instead.
Open

    miq_approvals.collect { |i| i.stamper.try(:email) }.compact.join(", ")
Severity: Minor
Found in app/models/miq_request.rb by rubocop

This loop will have at most one iteration.
Open

    found = each_ems_metadata(nil, EmsCluster) { |ci| break(ci) }
Severity: Minor
Found in app/models/miq_request_workflow.rb by rubocop

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 Enumerables 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

    find_all_ems_of_type(Host).collect { |h| h if host_ids.include?(h.id) }.compact
Severity: Minor
Found in app/models/miq_request_workflow.rb by rubocop

Avoid using Marshal.load.
Open

    return Marshal.load(Base64.decode64(results.split("\n").join)) unless results.nil?
Severity: Minor
Found in app/models/miq_task.rb by rubocop

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({}))

Duplicate branch body detected.
Open

    else 'Unknown'
Severity: Minor
Found in app/models/miq_request.rb by rubocop

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 sum { ... } instead of collect { ... }.sum.
Open

    service_resources.collect(&:group_idx).uniq.collect { |idx| max_group_delay(idx, delay_type(action)) }.sum
Severity: Minor
Found in app/models/mixins/service_mixin.rb by rubocop

Use filter_map instead.
Open

    @cloud_subnets_names ||= cloud_subnets.collect(&:name).compact.uniq
Severity: Minor
Found in app/models/network_port.rb by rubocop

Do not return in begin..end blocks in assignment contexts.
Open

      return if src[:ems].nil?
Severity: Minor
Found in app/models/miq_request_workflow.rb by rubocop

Checks for the presence of a return inside a begin..end block in assignment contexts. In this situation, the return will result in an exit from the current method, possibly leading to unexpected behavior.

Example:

# bad

@some_variable ||= begin
  return some_value if some_condition_is_met

  do_something
end

Example:

# good

@some_variable ||= begin
  if some_condition_is_met
    some_value
  else
    do_something
  end
end

# good

some_variable = if some_condition_is_met
                  return if another_condition_is_met

                  some_value
                else
                  do_something
                end

private (on line 183) does not make singleton methods private. Use private_class_method or private inside a class << self block instead.
Open

  def self.calc_md5(text)

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
Severity
Category
Status
Source
Language