ManageIQ/manageiq

View on GitHub

Showing 1,314 of 1,314 total issues

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

                t = filter_val + " " * (@line_len - filter_val.length - 2)

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 immutable Array literals in loops. It is better to extract it into a local variable or a constant.
Open

      next unless %w[websocket_closed ticket_invalid].include?(console.proxy_status)
Severity: Minor
Found in app/models/system_console.rb by rubocop

Useless method definition detected.
Open

  def power_state=(new_power_state)
    super
  end
Severity: Minor
Found in app/models/vm_or_template.rb by rubocop

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

Use filter_map instead.
Open

    !hardware.nil? ? hardware.nics.collect(&:lan).compact : []
Severity: Minor
Found in app/models/vm_or_template.rb by rubocop

Prefer using YAML.safe_load over YAML.load.
Open

        hash = r.send(column).kind_of?(Hash) ? r.send(column) : YAML.load(r.send(column))
Severity: Minor
Found in tools/fix_auth/auth_config_model.rb by rubocop

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)

Use count instead of select...count.
Open

duplicate_hosts = hosts_index.select { |_by, hosts| hosts.count == 2 && hosts.select(&:has_active_ems?).count == 1 }
Severity: Minor
Found in tools/reconnect_hosts.rb by rubocop

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 }

Remove redundant sort.
Open

Dir.glob(File.expand_path(File.join(__dir__, "ar_adapter", "*.rb"))).sort.each { |f| require f }
Severity: Minor
Found in lib/extensions/ar_adapter.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

Use filter_map instead.
Open

    tenant.children.includes(:tenant_quotas).map do |c|
      cq = c.tenant_quotas.send(name).take
      cq.value if cq
    end.compact.sum
Severity: Minor
Found in app/models/tenant_quota.rb by rubocop

Call super to initialize state of the parent class.
Open

    def initialize
      @vm = nil
      @all_busy_by_host_id_storage_id = {}
      @active_vm_scans_by_zone = nil
      @zone = nil
Severity: Minor
Found in app/models/vm_scan/dispatcher.rb by rubocop

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

Use atomic file operation method FileUtils.rm_f.
Open

  File.delete(zip_fname) if File.exist?(zip_fname)
Severity: Minor
Found in tools/evm_dump.rb by rubocop

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)

Avoid more than 3 levels of block nesting.
Open

          if opts[:dry_run]
            puts "  **** This is a dry-run, nothing updated, skipping. ****"
          else
            service.add_resource!(matching_stack)
            reconnected += 1

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 String#include? instead of a regex match with literal-only pattern.
Open

    raise unless /already exists/.match?(e.message)
Severity: Minor
Found in lib/container_orchestrator.rb by rubocop

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)
Severity: Minor
Found in app/models/time_profile.rb by rubocop

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

Shadowing outer local variable - col.
Open

    TREND_COLS[db.to_sym][col.to_sym][:limit_cols].each_with_object([]) do |col, arr|
Severity: Minor
Found in app/models/vim_performance_trend.rb by rubocop

Checks for the use of local variable names from an outer scope in block arguments or block-local variables. This mirrors the warning given by ruby -cw prior to Ruby 2.6: "shadowing outer local variable - foo".

NOTE: Shadowing of variables in block passed to Ractor.new is allowed because Ractor should not access outer variables. eg. following style is encouraged:

```ruby
worker_id, pipe = env
Ractor.new(worker_id, pipe) do |worker_id, pipe|
end
```

Example:

# bad

def some_method
  foo = 1

  2.times do |foo| # shadowing outer `foo`
    do_something(foo)
  end
end

Example:

# good

def some_method
  foo = 1

  2.times do |bar|
    do_something(bar)
  end
end

Use any?(folder) instead of block.
Open

    parent_blue_folders.any? { |f| f == folder }
Severity: Minor
Found in app/models/vm_or_template.rb by rubocop

Use filter_map instead.
Open

    perfs.collect(&:abs_max_mem_usage_absolute_average_value).compact.max

Use filter_map instead.
Open

    perfs.collect(&:abs_max_derived_memory_used_value).compact.max

Avoid more than 3 levels of block nesting.
Open

            if vm.respond_to?(:refresh_on_scan)
              begin
                vm.refresh_on_scan
              rescue => err
                _log.error("refreshing data from VIM: #{err.message}")
Severity: Minor
Found in app/models/vm_scan.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.

Do not suppress exceptions.
Open

rescue

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

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

    if %i[delete destroy].include?(delete_meth)
Severity
Category
Status
Source
Language