ManageIQ/manageiq

View on GitHub

Showing 1,311 of 1,311 total issues

The use of eval is a serious security risk.
Open

    refresh_timings = eval(Regexp.last_match[2])

Checks for the use of Kernel#eval and Binding#eval.

Example:

# bad

eval(something)
binding.eval(something)

Use == if you meant to do a comparison or wrap the expression in parentheses to indicate you meant to assign in a condition.
Open

  elsif match = line[/Processing\sby\s([\d\w:.\#_-]*)/, 1]

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

The use of eval is a serious security risk.
Open

targets = eval(ARGV.first) rescue nil
Severity: Minor
Found in tools/vim_collect_perf_history.rb by rubocop

Checks for the use of Kernel#eval and Binding#eval.

Example:

# bad

eval(something)
binding.eval(something)

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

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.profiles_for_user(user_id, region_id)
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

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

Avoid more than 3 levels of block nesting.
Open

            rescue => err
              _log.warn("#{err.message}, unable to raise policy event: [vm_scan_complete]")
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.

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

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

Duplicate branch body detected.
Open

               else ""

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

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

        next if %w[consumption_admin admin].include?(u.userid)

Use filter_map instead.
Open

      scope_ids = scope_features.map do |id, feature|
        Rbac.role_allows?(:feature => feature, :any => true, :user => user_or_group) ? id : nil
      end.compact
Severity: Minor
Found in lib/rbac/filterer.rb by rubocop

Avoid more than 3 levels of block nesting.
Open

            rescue => err
              _log.error("saving VM drift state: #{err.message}")
              _log.log_backtrace(err)
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.

Avoid more than 3 levels of block nesting.
Open

              raise _("Unable to find Vm") if vm.nil?
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
Severity: Minor
Found in app/models/vm_scan/dispatcher.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

Remove unnecessary existence check File.exist?.
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)

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 }

Do not mix named captures and numbered captures in a Regexp literal.
Open

  REGEX = /
(?<model_name>([[:upper:]][[:alnum:]]*(::)?)+)
\.(?<associations>[a-z_.]+)
/x
Severity: Minor
Found in lib/miq_expression/count_field.rb by rubocop

Do not mix named captures and numbered captures in a Regexp literal because numbered capture is ignored if they're mixed. Replace numbered captures with non-capturing groupings or named captures.

Example:

# bad
/(?<foo>FOO)(BAR)/

# good
/(?<foo>FOO)(?<bar>BAR)/

# good
/(?<foo>FOO)(?:BAR)/

# good
/(FOO)(BAR)/</foo></bar></foo></foo>

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

                  tag_val1 = tag_val + " " * (@line_len - tag_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
Severity
Category
Status
Source
Language