ManageIQ/manageiq

View on GitHub

Showing 1,311 of 1,311 total issues

Use filter_map instead.
Open

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

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

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

    perfs.collect do |p|
      # Ignore any CPU bursts to 100% 15 minutes after VM booted
      next if (p.abs_max_cpu_usage_rate_average_value == 100.0) && boot_time && (p.abs_max_cpu_usage_rate_average_timestamp <= (boot_time + 15.minutes))

      p.abs_max_cpu_usage_rate_average_value

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
           class_name
           pid]

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 description].each { |key| dd.delete(key) }

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 assignment to variable - state. Use || instead of ||=.
Open

      state ||= obj.vim_performance_states.build(:timestamp => ts).capture_and_save
Severity: Minor
Found in app/models/vim_performance_state.rb by rubocop

Checks for every useless assignment to local variable in every scope. The basic idea for this cop was from the warning of ruby -cw:

assigned but unused variable - foo

Currently this cop has advanced logic that detects unreferenced reassignments and properly handles varied cases such as branch, loop, rescue, ensure, etc.

NOTE: Given the assignment foo = 1, bar = 2, removing unused variables can lead to a syntax error, so this case is not autocorrected.

Safety:

This cop's autocorrection is unsafe because removing assignment from operator assignment can cause NameError if this assignment has been used to declare local variable. For example, replacing a ||= 1 to a || 1 may cause "undefined local variable or method `a' for main:Object (NameError)".

Example:

# bad

def some_method
  some_var = 1
  do_something
end

Example:

# good

def some_method
  some_var = 1
  do_something(some_var)
end

Use filter_map instead.
Open

    coordinates = recs.collect do |r|
      next unless r.respond_to?(CHART_X_AXIS_COL) && r.respond_to?(col)

      [r.send(CHART_X_AXIS_COL).to_i, r.send(col).to_f]
    end.compact
Severity: Minor
Found in app/models/vim_performance_trend.rb by rubocop

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)

The use of eval is a serious security risk.
Open

  timings = eval(timings)

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

Example:

# bad

eval(something)
binding.eval(something)

Use Array.new(10) with a block instead of .times.map.
Open

  avg = 10.times.map { Benchmark.realtime { client.get("test") } }.inject(:+) / 10.0
Severity: Minor
Found in tools/memcached_ping.rb by rubocop

This cop checks for .times.map calls. In most cases such calls can be replaced with an explicit array creation.

Example:

# bad
9.times.map do |i|
  i.to_s
end

# good
Array.new(9) do |i|
  i.to_s
end

Remove redundant sort.
Open

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

Prefer using YAML.safe_load over YAML.load.
Open

        obj = YAML.load(data)

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)

Prefer using YAML.safe_load over YAML.load.
Open

        YAML.load(data)

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)

Useless method definition detected.
Open

        def push(datum)
          super(datum)
        end

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

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

Script file copy_reports_structure.rb doesn't have execute permission.
Open

#!/usr/bin/env ruby
Severity: Minor
Found in tools/copy_reports_structure.rb by rubocop

Checks if a file which has a shebang line as its first line is granted execute permission.

Example:

# bad

# A file which has a shebang line as its first line is not
# granted execute permission.

#!/usr/bin/env ruby
puts 'hello, world'

# good

# A file which has a shebang line as its first line is
# granted execute permission.

#!/usr/bin/env ruby
puts 'hello, world'

# good

# A file which has not a shebang line as its first line is not
# granted execute permission.

puts 'hello, world'

Duplicate branch body detected.
Open

      elsif old_application_name?(activity) && activity["application_name"].include?(" Server")
        return true

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 filter_map instead.
Open

activated_vms = duplicate_vms.map do |_by, vms|
  active_vms, inactive_vms = vms.partition(&:active)

  # There should only be one of each
  active_vm   = active_vms.first
Severity: Minor
Found in tools/reconnect_vms.rb by rubocop
Severity
Category
Status
Source
Language