ManageIQ/manageiq

View on GitHub

Showing 1,313 of 1,313 total issues

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

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

Use filter_map instead.
Open

    ws_tags.collect { |cat, tag| tags.fetch_path(cat.to_s.downcase, tag.downcase) }.compact
Severity: Minor
Found in app/models/miq_request_workflow.rb by rubocop

Use filter_map instead.
Open

      timezones.collect { |timezone| widget.generate_one_content_for_group(group, timezone) }.compact

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

    rescue Exception => e
      safe_log("Error in update_worker_record_at_exit: #{e.message}", :error)
Severity: Minor
Found in app/models/miq_worker/runner.rb by rubocop

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 return in begin..end blocks in assignment contexts.
Open

      return if email.blank?

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

Use count instead of find_all...length.
Open

        dups = current.uniq.find_all { |u| current.find_all { |c| c == u }.length > 1 }

This cop is used to identify usages of count on an Enumerable that follow calls to select or reject. Querying logic can instead be passed to the count call.

Example:

# bad
[1, 2, 3].select { |e| e > 2 }.size
[1, 2, 3].reject { |e| e > 2 }.size
[1, 2, 3].select { |e| e > 2 }.length
[1, 2, 3].reject { |e| e > 2 }.length
[1, 2, 3].select { |e| e > 2 }.count { |e| e.odd? }
[1, 2, 3].reject { |e| e > 2 }.count { |e| e.even? }
array.select(&:value).count

# good
[1, 2, 3].count { |e| e > 2 }
[1, 2, 3].count { |e| e < 2 }
[1, 2, 3].count { |e| e > 2 && e.odd? }
[1, 2, 3].count { |e| e < 2 && e.even? }
Model.select('field AS field_one').count
Model.select(:value).count

ActiveRecord compatibility: ActiveRecord will ignore the block that is passed to count. Other methods, such as select, will convert the association to an array and then run the block on the array. A simple work around to make count work with a block is to call to_a.count {...}.

Example: Model.where(id: [1, 2, 3].select { |m| m.method == true }.size

becomes:

Model.where(id: [1, 2, 3]).to_a.count { |m| m.method == true }

Avoid immutable Array literals in loops. It is better to extract it into a local variable or a constant.
Open

    return unless options.keys.any? { |key| [:date, :warn].include?(key) }

Do not suppress exceptions.
Open

    rescue
Severity: Minor
Found in app/models/mixins/scanning_mixin.rb by rubocop

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

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

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

Avoid more than 3 levels of block nesting.
Open

          unless currently_selected.nil? || field[:values].key?(currently_selected)
            @values[field_name] = [nil, nil]
          end
Severity: Minor
Found in app/models/miq_request_workflow.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.

Avoid more than 3 levels of block nesting.
Open

                   [found.id, found.name] if found
Severity: Minor
Found in app/models/miq_request_workflow.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.

Use atomic file operation method FileUtils.rm_f.
Open

    File.delete(pid_file) if File.exist?(pid_file)

Checks for non-atomic file operation. And then replace it with a nearly equivalent and atomic method.

These can cause problems that are difficult to reproduce, especially in cases of frequent file operations in parallel, such as test runs with parallel_rspec.

For examples: creating a directory if there is none, has the following problems

An exception occurs when the directory didn't exist at the time of exist?, but someone else created it before mkdir was executed.

Subsequent processes are executed without the directory that should be there when the directory existed at the time of exist?, but someone else deleted it shortly afterwards.

Safety:

This cop is unsafe, because autocorrection change to atomic processing. The atomic processing of the replacement destination is not guaranteed to be strictly equivalent to that before the replacement.

Example:

# bad - race condition with another process may result in an error in `mkdir`
unless Dir.exist?(path)
  FileUtils.mkdir(path)
end

# good - atomic and idempotent creation
FileUtils.mkdir_p(path)

# bad - race condition with another process may result in an error in `remove`
if File.exist?(path)
  FileUtils.remove(path)
end

# good - atomic and idempotent removal
FileUtils.rm_f(path)

Remove redundant sort.
Open

      Dir.glob(SCAN_ITEMS_DIR.join("*.{yml,yaml}")).sort + seed_plugin_files
Severity: Minor
Found in app/models/scan_item/seeding.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

Avoid immutable Array literals in loops. It is better to extract it into a local variable or a constant.
Open

            ['snapshot', 'mem', 'disk'].each do |a|
Severity: Minor
Found in app/models/storage.rb by rubocop

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

    hdr << "@page{@bottom-right{font-size: 75%;content: '" + _("Page %{page_number} of %{total_pages}") % {:page_number => " ' counter(page) '", :total_pages => " ' counter(pages)}}"}
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

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

      if node.name == klass_name && (datacenter == false || datacenter == true && ci.datacenter?)
Severity: Minor
Found in app/models/miq_request_workflow.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

Avoid more than 3 levels of block nesting.
Open

                if j.tags.any? { |t| t.to_s.starts_with?("miq_schedules_") }
                  _log.info("Removing user schedule with Tags: #{j.tags.inspect}")
                end

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.

Do not shadow rescued Exceptions.
Open

    rescue StandardError, Timeout::Error => err
      _log.error("#{log_prefix} Posting of #{log_type.downcase} logs failed for #{who_am_i} due to error: [#{err.class.name}] [#{err}]")
      task.update_status("Finished", "Error", "Posting of #{log_type.downcase} logs failed for #{who_am_i} due to error: [#{err.class.name}] [#{err}]")
      logfile.update(:state => "error")
      raise

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