ManageIQ/manageiq

View on GitHub

Showing 1,311 of 1,311 total issues

private (on line 183) does not make singleton methods private. Use private_class_method or private inside a class << self block instead.
Open

  def self.with_universal_newline(text)

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

Useless method definition detected.
Open

  def md5=(_md5)
    super
  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

Shadowing outer local variable - w.
Open

      w = workers.sort_by { |w| [w.memory_usage || -1, w.id] }.last

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

Call super to initialize state of the parent class.
Open

  def initialize(values, requester, resource_action, options = {})
    @settings        = {}
    @requester       = requester
    @target          = options[:target]
    @initiator       = options[:initiator]

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

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

    rescue Exception => err
      _log.error("#{err.message}, during reconnect!")
Severity: Minor
Found in app/models/miq_server.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

Use filter_map instead.
Open

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

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)

Use filter_map instead.
Open

    service_resources.where(:resource_type => 'VmOrTemplate').includes(:resource).collect(&:resource).compact
Severity: Minor
Found in app/models/service.rb by rubocop

Avoid more than 3 levels of block nesting.
Open

          unless field[:auto_select_single] == false
            @values[field_name] = field[:values].to_a.first
          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

          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.

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

        value = /array_/.match?(fld[:data_type]) ? values[f] : get_value(values[f])
Severity: Minor
Found in app/models/miq_request_workflow.rb by rubocop

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.

Use filter_map instead.
Open

        h[col] = arr.collect do |d|
          w = MiqWidget.find_by(:description => d)
          members << w if w
          w.try(:id)
        end.compact
Severity: Minor
Found in app/models/miq_widget_set.rb by rubocop

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

Use filter_map instead.
Open

    scsi_idx = svm.hardware.disks.collect { |d| d.location if d.controller_type == "scsi" }.compact

Use filter_map instead.
Open

    options.collect do |k, v|
      if skip_keys.include?(k)
        nil
      elsif v.kind_of?(Array)
        if v.length == 2

Use filter_map instead.
Open

    Array.wrap(vms).collect { |f| File.dirname(f) }.compact
Severity: Minor
Found in app/models/storage.rb by rubocop

Use filter_map instead.
Open

    miq_approvals.collect(&:stamper_name).compact.join(", ")
Severity: Minor
Found in app/models/miq_request.rb by rubocop

Use filter_map instead.
Open

    pxe_server.windows_images.collect do |p|
      build_ci_hash_struct(p, [:name, :description])
    end.compact
Severity: Minor
Found in app/models/miq_request_workflow.rb by rubocop

Redundant safe navigation detected.
Open

    elsif export_attributes["filter"]&.kind_of?(MiqExpression)

Checks for redundant safe navigation calls. instance_of?, kind_of?, is_a?, eql?, respond_to?, and equal? methods are checked by default. These are customizable with AllowedMethods option.

The AllowedMethods option specifies nil-safe methods, in other words, it is a method that is allowed to skip safe navigation. Note that the AllowedMethod option is not an option that specifies methods for which to suppress (allow) this cop's check.

In the example below, the safe navigation operator (&.) is unnecessary because NilClass has methods like respond_to? and is_a?.

Safety:

This cop is unsafe, because autocorrection can change the return type of the expression. An offending expression that previously could return nil will be autocorrected to never return nil.

Example:

# bad
do_something if attrs&.respond_to?(:[])

# good
do_something if attrs.respond_to?(:[])

# bad
while node&.is_a?(BeginNode)
  node = node.parent
end

# good
while node.is_a?(BeginNode)
  node = node.parent
end

# good - without `&.` this will always return `true`
foo&.respond_to?(:to_a)

Example: AllowedMethods: [nilsafemethod]

# bad
do_something if attrs&.nil_safe_method(:[])

# good
do_something if attrs.nil_safe_method(:[])
do_something if attrs&.not_nil_safe_method(:[])
Severity
Category
Status
Source
Language