Sign upLogin

kapost/kapost_deploy

View on GitHub
Star

Showing 116 of 116 total issues

KapostDeploy::Task#define_hook manually dispatches method call
Open

          plugin.send(kind) if plugin.respond_to?(kind)
Severity: Minor
Found in lib/kapost_deploy/task.rb by reek

Reek reports a Manual Dispatch smell if it finds source code that manually checks whether an object responds to a method before that method is called. Manual dispatch is a type of Simulated Polymorphism which leads to code that is harder to reason about, debug, and refactor.

Example

class MyManualDispatcher
  attr_reader :foo

  def initialize(foo)
    @foo = foo
  end

  def call
    foo.bar if foo.respond_to?(:bar)
  end
end

Reek would emit the following warning:

test.rb -- 1 warning:
  [9]: MyManualDispatcher manually dispatches method call (ManualDispatch)

KapostDeploy::Plugins::SlackGithubDiff#initialize calls 'config.options' 2 times
Open

                     notifier: KapostDeploy::Slack::Notifier.new(config.options.fetch(:slack_config, nil)),
                     behind_releases: KapostDeploy::Heroku::AppReleases.new(config.to, token: config.heroku_api_token),
                     ahead_releases: KapostDeploy::Heroku::AppReleases.new(config.app, token: config.heroku_api_token))

        self.ahead_app = config.app
Severity: Minor
Found in lib/kapost_deploy/plugins/slack_github_diff.rb by reek

Duplication occurs when two fragments of code look nearly identical, or when two fragments of code have nearly identical effects at some conceptual level.

Reek implements a check for Duplicate Method Call.

Example

Here's a very much simplified and contrived example. The following method will report a warning:

def double_thing()
  @other.thing + @other.thing
end

One quick approach to silence Reek would be to refactor the code thus:

def double_thing()
  thing = @other.thing
  thing + thing
end

A slightly different approach would be to replace all calls of double_thing by calls to @other.double_thing:

class Other
  def double_thing()
    thing + thing
  end
end

The approach you take will depend on balancing other factors in your code.

KapostDeploy::Seismograph::NullAdapter::Sensor has no descriptive comment
Open

      class Sensor
Severity: Minor
Found in lib/kapost_deploy/seismograph/notifier.rb by reek

Classes and modules are the units of reuse and release. It is therefore considered good practice to annotate every class and module with a brief comment outlining its responsibilities.

Example

Given

class Dummy
  # Do things...
end

Reek would emit the following warning:

test.rb -- 1 warning:
  [1]:Dummy has no descriptive comment (IrresponsibleModule)

Fixing this is simple - just an explaining comment:

# The Dummy class is responsible for ...
class Dummy
  # Do things...
end

KapostDeploy::Plugins::SlackGithubDiff#initialize calls 'config.heroku_api_token' 2 times
Open

                     behind_releases: KapostDeploy::Heroku::AppReleases.new(config.to, token: config.heroku_api_token),
                     ahead_releases: KapostDeploy::Heroku::AppReleases.new(config.app, token: config.heroku_api_token))
Severity: Minor
Found in lib/kapost_deploy/plugins/slack_github_diff.rb by reek

Duplication occurs when two fragments of code look nearly identical, or when two fragments of code have nearly identical effects at some conceptual level.

Reek implements a check for Duplicate Method Call.

Example

Here's a very much simplified and contrived example. The following method will report a warning:

def double_thing()
  @other.thing + @other.thing
end

One quick approach to silence Reek would be to refactor the code thus:

def double_thing()
  thing = @other.thing
  thing + thing
end

A slightly different approach would be to replace all calls of double_thing by calls to @other.double_thing:

class Other
  def double_thing()
    thing + thing
  end
end

The approach you take will depend on balancing other factors in your code.

KapostDeploy::Task assumes too much for instance variable '@shell'
Open

  class Task < Rake::TaskLib
Severity: Minor
Found in lib/kapost_deploy/task.rb by reek

Classes should not assume that instance variables are set or present outside of the current class definition.

Good:

class Foo
  def initialize
    @bar = :foo
  end

  def foo?
    @bar == :foo
  end
end

Good as well:

class Foo
  def foo?
    bar == :foo
  end

  def bar
    @bar ||= :foo
  end
end

Bad:

class Foo
  def go_foo!
    @bar = :foo
  end

  def foo?
    @bar == :foo
  end
end

Example

Running Reek on:

class Dummy
  def test
    @ivar
  end
end

would report:

[1]:InstanceVariableAssumption: Dummy assumes too much for instance variable @ivar

Note that this example would trigger this smell warning as well:

class Parent
  def initialize(omg)
    @omg = omg
  end
end

class Child < Parent
  def foo
    @omg
  end
end

The way to address the smell warning is that you should create an attr_reader to use @omg in the subclass and not access @omg directly like this:

class Parent
  attr_reader :omg

  def initialize(omg)
    @omg = omg
  end
end

class Child < Parent
  def foo
    omg
  end
end

Directly accessing instance variables is considered a smell because it breaks encapsulation and makes it harder to reason about code.

If you don't want to expose those methods as public API just make them private like this:

class Parent
  def initialize(omg)
    @omg = omg
  end

  private
  attr_reader :omg
end

class Child < Parent
  def foo
    omg
  end
end

Current Support in Reek

An instance variable must:

  • be set in the constructor
  • or be accessed through a method with lazy initialization / memoization.

If not, Instance Variable Assumption will be reported.

KapostDeploy::Task#to is a writable attribute
Open

    attr_accessor :to
Severity: Minor
Found in lib/kapost_deploy/task.rb by reek

A class that publishes a setter for an instance variable invites client classes to become too intimate with its inner workings, and in particular with its representation of state.

The same holds to a lesser extent for getters, but Reek doesn't flag those.

Example

Given:

class Klass
  attr_accessor :dummy
end

Reek would emit the following warning:

reek test.rb

test.rb -- 1 warning:
  [2]:Klass declares the writable attribute dummy (Attribute)

KapostDeploy::Plugins::NotifyHoneyBadgerAfterPromote#ci_sha doesn't depend on instance state (maybe move it to another class?)
Open

      def ci_sha
Severity: Minor
Found in lib/kapost_deploy/plugins/notify_honeybadger_after_promote.rb by reek

A Utility Function is any instance method that has no dependency on the state of the instance.

KapostDeploy::Task#name is a writable attribute
Open

    attr_accessor :name
Severity: Minor
Found in lib/kapost_deploy/task.rb by reek

A class that publishes a setter for an instance variable invites client classes to become too intimate with its inner workings, and in particular with its representation of state.

The same holds to a lesser extent for getters, but Reek doesn't flag those.

Example

Given:

class Klass
  attr_accessor :dummy
end

Reek would emit the following warning:

reek test.rb

test.rb -- 1 warning:
  [2]:Klass declares the writable attribute dummy (Attribute)

KapostDeploy::Task#validate performs a nil-check
Open

      fail "No 'pipeline' configured. Set config.pipeline to the name of your pipeline" if pipeline.nil?
      fail "No 'app' configured. Set config.app to the application to be promoted" if app.nil?
      fail "No 'to' configured. Set config.to to the downstream application to be promoted to" if to.nil?
Severity: Minor
Found in lib/kapost_deploy/task.rb by reek

A NilCheck is a type check. Failures of NilCheck violate the "tell, don't ask" principle.

Additionally, type checks often mask bigger problems in your source code like not using OOP and / or polymorphism when you should.

Example

Given

class Klass
  def nil_checker(argument)
    if argument.nil?
      puts "argument isn't nil!"
    end
  end
end

Reek would emit the following warning:

test.rb -- 1 warning:
  [3]:Klass#nil_checker performs a nil-check. (NilCheck)

KapostDeploy::Task#options is a writable attribute
Open

    attr_accessor :options
Severity: Minor
Found in lib/kapost_deploy/task.rb by reek

A class that publishes a setter for an instance variable invites client classes to become too intimate with its inner workings, and in particular with its representation of state.

The same holds to a lesser extent for getters, but Reek doesn't flag those.

Example

Given:

class Klass
  attr_accessor :dummy
end

Reek would emit the following warning:

reek test.rb

test.rb -- 1 warning:
  [2]:Klass declares the writable attribute dummy (Attribute)

KapostDeploy::Heroku::Shell#preflight_check_for_cli performs a nil-check
Open

        if Bundler.with_clean_env { system("heroku version").nil? }
Severity: Minor
Found in lib/kapost_deploy/heroku/shell.rb by reek

A NilCheck is a type check. Failures of NilCheck violate the "tell, don't ask" principle.

Additionally, type checks often mask bigger problems in your source code like not using OOP and / or polymorphism when you should.

Example

Given

class Klass
  def nil_checker(argument)
    if argument.nil?
      puts "argument isn't nil!"
    end
  end
end

Reek would emit the following warning:

test.rb -- 1 warning:
  [3]:Klass#nil_checker performs a nil-check. (NilCheck)

KapostDeploy::Plugins::NotifyHoneyBadgerAfterPromote#configured? performs a nil-check
Open

        !github_repo.nil? && !github_repo.empty?
Severity: Minor
Found in lib/kapost_deploy/plugins/notify_honeybadger_after_promote.rb by reek

A NilCheck is a type check. Failures of NilCheck violate the "tell, don't ask" principle.

Additionally, type checks often mask bigger problems in your source code like not using OOP and / or polymorphism when you should.

Example

Given

class Klass
  def nil_checker(argument)
    if argument.nil?
      puts "argument isn't nil!"
    end
  end
end

Reek would emit the following warning:

test.rb -- 1 warning:
  [3]:Klass#nil_checker performs a nil-check. (NilCheck)

KapostDeploy::Plugins::NotifyHoneyBadgerAfterPromote#github_repo performs a nil-check
Open

        git_config[:github_repo] unless git_config.nil?
Severity: Minor
Found in lib/kapost_deploy/plugins/notify_honeybadger_after_promote.rb by reek

A NilCheck is a type check. Failures of NilCheck violate the "tell, don't ask" principle.

Additionally, type checks often mask bigger problems in your source code like not using OOP and / or polymorphism when you should.

Example

Given

class Klass
  def nil_checker(argument)
    if argument.nil?
      puts "argument isn't nil!"
    end
  end
end

Reek would emit the following warning:

test.rb -- 1 warning:
  [3]:Klass#nil_checker performs a nil-check. (NilCheck)

KapostDeploy::Task#heroku_api_token is a writable attribute
Open

    attr_accessor :heroku_api_token
Severity: Minor
Found in lib/kapost_deploy/task.rb by reek

A class that publishes a setter for an instance variable invites client classes to become too intimate with its inner workings, and in particular with its representation of state.

The same holds to a lesser extent for getters, but Reek doesn't flag those.

Example

Given:

class Klass
  attr_accessor :dummy
end

Reek would emit the following warning:

reek test.rb

test.rb -- 1 warning:
  [2]:Klass declares the writable attribute dummy (Attribute)

KapostDeploy::Plugins::SlackGithubDiff#configured? performs a nil-check
Open

        !git_config.nil? && notifier.configured?
Severity: Minor
Found in lib/kapost_deploy/plugins/slack_github_diff.rb by reek

A NilCheck is a type check. Failures of NilCheck violate the "tell, don't ask" principle.

Additionally, type checks often mask bigger problems in your source code like not using OOP and / or polymorphism when you should.

Example

Given

class Klass
  def nil_checker(argument)
    if argument.nil?
      puts "argument isn't nil!"
    end
  end
end

Reek would emit the following warning:

test.rb -- 1 warning:
  [3]:Klass#nil_checker performs a nil-check. (NilCheck)

KapostDeploy::Task#app is a writable attribute
Open

    attr_accessor :app
Severity: Minor
Found in lib/kapost_deploy/task.rb by reek

A class that publishes a setter for an instance variable invites client classes to become too intimate with its inner workings, and in particular with its representation of state.

The same holds to a lesser extent for getters, but Reek doesn't flag those.

Example

Given:

class Klass
  attr_accessor :dummy
end

Reek would emit the following warning:

reek test.rb

test.rb -- 1 warning:
  [2]:Klass declares the writable attribute dummy (Attribute)

KapostDeploy::Plugins::NotifyDatadogAfterPromote#ci_sha doesn't depend on instance state (maybe move it to another class?)
Open

      def ci_sha
Severity: Minor
Found in lib/kapost_deploy/plugins/notify_datadog_after_promote.rb by reek

A Utility Function is any instance method that has no dependency on the state of the instance.

KapostDeploy::Slack::Notifier#configured? performs a nil-check
Open

        !slack_config.nil?
Severity: Minor
Found in lib/kapost_deploy/slack/notifier.rb by reek

A NilCheck is a type check. Failures of NilCheck violate the "tell, don't ask" principle.

Additionally, type checks often mask bigger problems in your source code like not using OOP and / or polymorphism when you should.

Example

Given

class Klass
  def nil_checker(argument)
    if argument.nil?
      puts "argument isn't nil!"
    end
  end
end

Reek would emit the following warning:

test.rb -- 1 warning:
  [3]:Klass#nil_checker performs a nil-check. (NilCheck)

KapostDeploy::Plugins::SlackAfterPromote#additional_message performs a nil-check
Open

        addl = slack_config.fetch(:additional_message, "") unless slack_config.nil?
Severity: Minor
Found in lib/kapost_deploy/plugins/slack_after_promote.rb by reek

A NilCheck is a type check. Failures of NilCheck violate the "tell, don't ask" principle.

Additionally, type checks often mask bigger problems in your source code like not using OOP and / or polymorphism when you should.

Example

Given

class Klass
  def nil_checker(argument)
    if argument.nil?
      puts "argument isn't nil!"
    end
  end
end

Reek would emit the following warning:

test.rb -- 1 warning:
  [3]:Klass#nil_checker performs a nil-check. (NilCheck)

KapostDeploy::Task#pipeline is a writable attribute
Open

    attr_accessor :pipeline
Severity: Minor
Found in lib/kapost_deploy/task.rb by reek

A class that publishes a setter for an instance variable invites client classes to become too intimate with its inner workings, and in particular with its representation of state.

The same holds to a lesser extent for getters, but Reek doesn't flag those.

Example

Given:

class Klass
  attr_accessor :dummy
end

Reek would emit the following warning:

reek test.rb

test.rb -- 1 warning:
  [2]:Klass declares the writable attribute dummy (Attribute)
Severity
Category
Status
Source
Language