ManageIQ/manageiq-providers-azure

View on GitHub

Showing 66 of 66 total issues

Method has too many optional parameters. [4/3]
Open

    def raw_connect(client_id, client_key, azure_tenant_id, subscription, service = nil, proxy_uri = nil, provider_region = nil, endpoint = nil)
      require 'azure-armrest'

      if subscription.blank?
        raise MiqException::MiqInvalidCredentialsError, _("Incorrect credentials - check your Azure Subscription ID")

Checks for methods with too many parameters.

The maximum number of parameters is configurable. Keyword arguments can optionally be excluded from the total count, as they add less complexity than positional or optional parameters.

Any number of arguments for initialize method inside a block of Struct.new and Data.define like this is always allowed:

Struct.new(:one, :two, :three, :four, :five, keyword_init: true) do
  def initialize(one:, two:, three:, four:, five:)
  end
end

This is because checking the number of arguments of the initialize method does not make sense.

NOTE: Explicit block argument &block is not counted to prevent erroneous change that is avoided by making block argument implicit.

Example: Max: 3

# good
def foo(a, b, c = 1)
end

Example: Max: 2

# bad
def foo(a, b, c = 1)
end

Example: CountKeywordArgs: true (default)

# counts keyword args towards the maximum

# bad (assuming Max is 3)
def foo(a, b, c, d: 1)
end

# good (assuming Max is 3)
def foo(a, b, c: 1)
end

Example: CountKeywordArgs: false

# don't count keyword args towards the maximum

# good (assuming Max is 3)
def foo(a, b, c, d: 1)
end

This cop also checks for the maximum number of optional parameters. This can be configured using the MaxOptionalParameters config option.

Example: MaxOptionalParameters: 3 (default)

# good
def foo(a = 1, b = 2, c = 3)
end

Example: MaxOptionalParameters: 2

# bad
def foo(a = 1, b = 2, c = 3)
end

Duplicate branch body detected.
Open

                    when "Microsoft.Compute/disks"
                      :__unused

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 match? instead of =~ when MatchData is not used.
Open

    return false unless ems_ref =~ /^\/subscriptions\//i

In Ruby 2.4, String#match?, Regexp#match? and Symbol#match? have been added. The methods are faster than match. Because the methods avoid creating a MatchData object or saving backref. So, when MatchData is not used, use match? instead of match.

Example:

# bad
def foo
  if x =~ /re/
    do_something
  end
end

# bad
def foo
  if x.match(/re/)
    do_something
  end
end

# bad
def foo
  if /re/ === x
    do_something
  end
end

# good
def foo
  if x.match?(/re/)
    do_something
  end
end

# good
def foo
  if x =~ /re/
    do_something(Regexp.last_match)
  end
end

# good
def foo
  if x.match(/re/)
    do_something($~)
  end
end

# good
def foo
  if /re/ === x
    do_something($~)
  end
end

Use match? instead of =~ when MatchData is not used.
Open

    if source.ems_ref =~ /.+:.+:.+:.+/

In Ruby 2.4, String#match?, Regexp#match? and Symbol#match? have been added. The methods are faster than match. Because the methods avoid creating a MatchData object or saving backref. So, when MatchData is not used, use match? instead of match.

Example:

# bad
def foo
  if x =~ /re/
    do_something
  end
end

# bad
def foo
  if x.match(/re/)
    do_something
  end
end

# bad
def foo
  if /re/ === x
    do_something
  end
end

# good
def foo
  if x.match?(/re/)
    do_something
  end
end

# good
def foo
  if x =~ /re/
    do_something(Regexp.last_match)
  end
end

# good
def foo
  if x.match(/re/)
    do_something($~)
  end
end

# good
def foo
  if /re/ === x
    do_something($~)
  end
end

Use filter_map instead.
Open

    instance.properties.network_profile.network_interfaces.map { |x| @indexed_network_ports[x.id] }.compact

metadata['rubygems_mfa_required'] must be set to 'true'.
Open

Gem::Specification.new do |spec|
  spec.name          = "manageiq-providers-azure"
  spec.version       = ManageIQ::Providers::Azure::VERSION
  spec.authors       = ["ManageIQ Authors"]

Severity: Minor
Found in manageiq-providers-azure.gemspec by rubocop

Requires a gemspec to have rubygems_mfa_required metadata set.

This setting tells RubyGems that MFA (Multi-Factor Authentication) is required for accounts to be able perform privileged operations, such as (see RubyGems' documentation for the full list of privileged operations):

  • gem push
  • gem yank
  • gem owner --add/remove
  • adding or removing owners using gem ownership page

This helps make your gem more secure, as users can be more confident that gem updates were pushed by maintainers.

Example:

# bad
Gem::Specification.new do |spec|
  # no `rubygems_mfa_required` metadata specified
end

# good
Gem::Specification.new do |spec|
  spec.metadata = {
    'rubygems_mfa_required' => 'true'
  }
end

# good
Gem::Specification.new do |spec|
  spec.metadata['rubygems_mfa_required'] = 'true'
end

# bad
Gem::Specification.new do |spec|
  spec.metadata = {
    'rubygems_mfa_required' => 'false'
  }
end

# good
Gem::Specification.new do |spec|
  spec.metadata = {
    'rubygems_mfa_required' => 'true'
  }
end

# bad
Gem::Specification.new do |spec|
  spec.metadata['rubygems_mfa_required'] = 'false'
end

# good
Gem::Specification.new do |spec|
  spec.metadata['rubygems_mfa_required'] = 'true'
end

Empty class detected.
Open

class ManageIQ::Providers::Azure::CloudManager::Scanning
end

Checks for classes and metaclasses without a body. Such empty classes and metaclasses are typically an oversight or we should provide a comment to be clearer what we're aiming for.

Example:

# bad
class Foo
end

class Bar
  class << self
  end
end

class << obj
end

# good
class Foo
  def do_something
    # ... code
  end
end

class Bar
  class << self
    attr_reader :bar
  end
end

class << obj
  attr_reader :bar
end

Example: AllowComments: false (default)

# bad
class Foo
  # TODO: implement later
end

class Bar
  class << self
    # TODO: implement later
  end
end

class << obj
  # TODO: implement later
end

Example: AllowComments: true

# good
class Foo
  # TODO: implement later
end

class Bar
  class << self
    # TODO: implement later
  end
end

class << obj
  # TODO: implement later
end

Use filter_map instead.
Open

      refs.map { |x| targeted_stacks_cache[x] }.compact

Use filter_map instead.
Open

      vm.floating_ips.collect(&:ems_ref).compact.each { |ems_ref| add_target!(:floating_ips, ems_ref) }

Use match? instead of =~ when MatchData is not used.
Open

    if err.to_s =~ /[D|d]eployment.+ could not be found/

In Ruby 2.4, String#match?, Regexp#match? and Symbol#match? have been added. The methods are faster than match. Because the methods avoid creating a MatchData object or saving backref. So, when MatchData is not used, use match? instead of match.

Example:

# bad
def foo
  if x =~ /re/
    do_something
  end
end

# bad
def foo
  if x.match(/re/)
    do_something
  end
end

# bad
def foo
  if /re/ === x
    do_something
  end
end

# good
def foo
  if x.match?(/re/)
    do_something
  end
end

# good
def foo
  if x =~ /re/
    do_something(Regexp.last_match)
  end
end

# good
def foo
  if x.match(/re/)
    do_something($~)
  end
end

# good
def foo
  if /re/ === x
    do_something($~)
  end
end

Use match? instead of =~ when MatchData is not used.
Open

          return ssa_snap_name if snap_state =~ /succe/i

In Ruby 2.4, String#match?, Regexp#match? and Symbol#match? have been added. The methods are faster than match. Because the methods avoid creating a MatchData object or saving backref. So, when MatchData is not used, use match? instead of match.

Example:

# bad
def foo
  if x =~ /re/
    do_something
  end
end

# bad
def foo
  if x.match(/re/)
    do_something
  end
end

# bad
def foo
  if /re/ === x
    do_something
  end
end

# good
def foo
  if x.match?(/re/)
    do_something
  end
end

# good
def foo
  if x =~ /re/
    do_something(Regexp.last_match)
  end
end

# good
def foo
  if x.match(/re/)
    do_something($~)
  end
end

# good
def foo
  if /re/ === x
    do_something($~)
  end
end

Use match? instead of =~ when MatchData is not used.
Open

    if ref_parts[3] =~ /resourceGroups/i

In Ruby 2.4, String#match?, Regexp#match? and Symbol#match? have been added. The methods are faster than match. Because the methods avoid creating a MatchData object or saving backref. So, when MatchData is not used, use match? instead of match.

Example:

# bad
def foo
  if x =~ /re/
    do_something
  end
end

# bad
def foo
  if x.match(/re/)
    do_something
  end
end

# bad
def foo
  if /re/ === x
    do_something
  end
end

# good
def foo
  if x.match?(/re/)
    do_something
  end
end

# good
def foo
  if x =~ /re/
    do_something(Regexp.last_match)
  end
end

# good
def foo
  if x.match(/re/)
    do_something($~)
  end
end

# good
def foo
  if /re/ === x
    do_something($~)
  end
end

Use filter_map instead.
Open

      vm.key_pairs.collect(&:name).compact.each do |name|

Use match? instead of =~ when MatchData is not used.
Open

    return nil unless ems_ref =~ /^\/subscriptions\//i

In Ruby 2.4, String#match?, Regexp#match? and Symbol#match? have been added. The methods are faster than match. Because the methods avoid creating a MatchData object or saving backref. So, when MatchData is not used, use match? instead of match.

Example:

# bad
def foo
  if x =~ /re/
    do_something
  end
end

# bad
def foo
  if x.match(/re/)
    do_something
  end
end

# bad
def foo
  if /re/ === x
    do_something
  end
end

# good
def foo
  if x.match?(/re/)
    do_something
  end
end

# good
def foo
  if x =~ /re/
    do_something(Regexp.last_match)
  end
end

# good
def foo
  if x.match(/re/)
    do_something($~)
  end
end

# good
def foo
  if /re/ === x
    do_something($~)
  end
end

Do not suppress exceptions.
Open

rescue LoadError
Severity: Minor
Found in Rakefile 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

Use filter_map instead.
Open

      network_port.floating_ips.collect(&:ems_ref).compact.each { |ems_ref| add_target!(:floating_ips, ems_ref) }

Duplicate branch body detected.
Open

                    when "Microsoft.Storage/storageAccounts"
                      :__unused

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

      vm.cloud_networks.collect(&:ems_ref).compact.each { |ems_ref| add_target!(:cloud_networks, ems_ref) }

Duplicate branch body detected.
Open

                    when "Microsoft.Compute/snapshots"
                      :__unused

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

      vm.network_ports.collect(&:ems_ref).compact.each do |ems_ref|
Severity
Category
Status
Source
Language