ManageIQ/manageiq

View on GitHub

Showing 1,311 of 1,311 total issues

Use filter_map instead.
Open

      locks_owned_by_spid(spid).collect { |lock| lock["blocked_by"] }.compact.flatten.uniq

Use filter_map instead.
Open

activated_hosts = duplicate_hosts.map do |_by, hosts|
  active_hosts, inactive_hosts = hosts.partition(&:has_active_ems?)

  # There should only be one of each
  active_host   = active_hosts.first
Severity: Minor
Found in tools/reconnect_hosts.rb by rubocop

Prefer using YAML.safe_load over YAML.load.
Open

        YAML.load(f.read)
Severity: Minor
Found in lib/extensions/descendant_loader.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)

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

          "+" + "-" * (@line_len - 2) + "+" + CRLF

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

            if ["<compare>"].include?(mri.db) && r[0] == "% Match:"

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.

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)

Use filter_map instead.
Open

ids  = $2.split(',').collect do |id|
  id.strip!
  id.blank? ? nil : id.to_i
end.compact.uniq
Severity: Minor
Found in tools/create_evm_snapshot.rb by rubocop

Use filter_map instead.
Open

    vms = lan.guest_devices.collect { |gd| [gd.hardware.vm, gd.device_name] if gd.hardware && gd.hardware.vm }.compact
Severity: Minor
Found in tools/db_printers/print_network.rb by rubocop

Use atomic file operation method FileUtils.rm_f.
Open

  File.delete(yml_fname) if File.exist?(yml_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)

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

  if match = line.match(/Started\s(\w*)\s"([\/\w\d.-]*)"/)

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

Use filter_map instead.
Open

      domain = userid.split(",").collect { |p| p.split('dc=')[1] }.compact.join('.')

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

        %w[worker_id
           server_id
           datid
           datname
           spid

Use count instead of select...count.
Open

duplicate_vms = vms_index.select { |_by, vms| vms.count == 2 && vms.select(&:active).count == 1 }
Severity: Minor
Found in tools/reconnect_vms.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 }

Use String#include? instead of a regex match with literal-only pattern.
Open

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

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

Unused block argument - v. If it's necessary, use _ or _v as an argument name to indicate that it won't be used.
Open

    location_string = loc && loc.sort_by { |k, v| k.to_s }.inspect

Checks for unused block arguments.

Example:

# bad
do_something do |used, unused|
  puts used
end

do_something do |bar|
  puts :foo
end

define_method(:foo) do |bar|
  puts :baz
end

# good
do_something do |used, _unused|
  puts used
end

do_something do
  puts :foo
end

define_method(:foo) do |_bar|
  puts :baz
end

Example: IgnoreEmptyBlocks: true (default)

# good
do_something { |unused| }

Example: IgnoreEmptyBlocks: false

# bad
do_something { |unused| }

Example: AllowUnusedKeywordArguments: false (default)

# bad
do_something do |unused: 42|
  foo
end

Example: AllowUnusedKeywordArguments: true

# good
do_something do |unused: 42|
  foo
end

Use filter_map instead.
Open

        perf_cols = [:cpu, :vcpus, :memory, :storage].collect { |t| options.fetch_path(:vm_options, t, :metric) }.compact

Prefer using YAML.safe_load over YAML.load.
Open

        hash = old_value.kind_of?(Hash) ? old_value : YAML.load(old_value)
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)

Shadowing outer local variable - k.
Open

counts = Hash.new { |h, k| h[k] = Hash.new { |h, k| h[k] = [] } }

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