kirushik/critical_chain

View on GitHub

Showing 212 of 215 total issues

Method has too many lines. [28/10]
Open

  def update
    @estimation = Estimation.find(params[:estimation_id]).decorate
    @estimation_item = EstimationItem.find(params[:id])

    authorize @estimation, :update?

Checks if the length of a method exceeds some maximum value. Comment lines can optionally be allowed. The maximum allowed length is configurable.

You can set constructs you want to fold with CountAsOne. Available are: 'array', 'hash', 'heredoc', and 'method_call'. Each construct will be counted as one line regardless of its actual size.

NOTE: The ExcludedMethods and IgnoredMethods configuration is deprecated and only kept for backwards compatibility. Please use AllowedMethods and AllowedPatterns instead. By default, there are no methods to allowed.

Example: CountAsOne: ['array', 'heredoc', 'method_call']

def m
  array = [       # +1
    1,
    2
  ]

  hash = {        # +3
    key: 'value'
  }

  <<~HEREDOC      # +1
    Heredoc
    content.
  HEREDOC

  foo(            # +1
    1,
    2
  )
end               # 6 points

Line is too long. [148/120]
Open

    helpers.editable object, field, url: helpers.estimation_estimation_item_path(object.estimation, object), mode: :inline, type: type, class: field

Checks the length of lines in the source code. The maximum length is configurable. The tab size is configured in the IndentationWidth of the Layout/IndentationStyle cop. It also ignores a shebang line by default.

This cop has some autocorrection capabilities. It can programmatically shorten certain long lines by inserting line breaks into expressions that can be safely split across lines. These include arrays, hashes, and method calls with argument lists.

If autocorrection is enabled, the following Layout cops are recommended to further format the broken lines. (Many of these are enabled by default.)

  • ArgumentAlignment
  • ArrayAlignment
  • BlockAlignment
  • BlockDelimiters
  • BlockEndNewline
  • ClosingParenthesisIndentation
  • FirstArgumentIndentation
  • FirstArrayElementIndentation
  • FirstHashElementIndentation
  • FirstParameterIndentation
  • HashAlignment
  • IndentationWidth
  • MultilineArrayLineBreaks
  • MultilineBlockLayout
  • MultilineHashBraceLayout
  • MultilineHashKeyLineBreaks
  • MultilineMethodArgumentLineBreaks
  • MultilineMethodParameterLineBreaks
  • ParameterAlignment

Together, these cops will pretty print hashes, arrays, method calls, etc. For example, let's say the max columns is 25:

Example:

# bad
{foo: "0000000000", bar: "0000000000", baz: "0000000000"}

# good
{foo: "0000000000",
bar: "0000000000", baz: "0000000000"}

# good (with recommended cops enabled)
{
  foo: "0000000000",
  bar: "0000000000",
  baz: "0000000000",
}

Method update has 28 lines of code (exceeds 25 allowed). Consider refactoring.
Open

  def update
    @estimation = Estimation.find(params[:estimation_id]).decorate
    @estimation_item = EstimationItem.find(params[:id])

    authorize @estimation, :update?
Severity: Minor
Found in app/controllers/estimation_items_controller.rb - About 1 hr to fix

    EstimationItemsController#create has approx 6 statements
    Open

      def create

    A method with Too Many Statements is any method that has a large number of lines.

    Too Many Statements warns about any method that has more than 5 statements. Reek's smell detector for Too Many Statements counts +1 for every simple statement in a method and +1 for every statement within a control structure (if, else, case, when, for, while, until, begin, rescue) but it doesn't count the control structure itself.

    So the following method would score +6 in Reek's statement-counting algorithm:

    def parse(arg, argv, &error)
      if !(val = arg) and (argv.empty? or /\A-/ =~ (val = argv[0]))
        return nil, block, nil                                         # +1
      end
      opt = (val = parse_arg(val, &error))[1]                          # +2
      val = conv_arg(*val)                                             # +3
      if opt and !arg
        argv.shift                                                     # +4
      else
        val[0] = nil                                                   # +5
      end
      val                                                              # +6
    end

    (You might argue that the two assignments within the first @if@ should count as statements, and that perhaps the nested assignment should count as +2.)

    EstimationItemsController#update has approx 10 statements
    Open

      def update

    A method with Too Many Statements is any method that has a large number of lines.

    Too Many Statements warns about any method that has more than 5 statements. Reek's smell detector for Too Many Statements counts +1 for every simple statement in a method and +1 for every statement within a control structure (if, else, case, when, for, while, until, begin, rescue) but it doesn't count the control structure itself.

    So the following method would score +6 in Reek's statement-counting algorithm:

    def parse(arg, argv, &error)
      if !(val = arg) and (argv.empty? or /\A-/ =~ (val = argv[0]))
        return nil, block, nil                                         # +1
      end
      opt = (val = parse_arg(val, &error))[1]                          # +2
      val = conv_arg(*val)                                             # +3
      if opt and !arg
        argv.shift                                                     # +4
      else
        val[0] = nil                                                   # +5
      end
      val                                                              # +6
    end

    (You might argue that the two assignments within the first @if@ should count as statements, and that perhaps the nested assignment should count as +2.)

    EstimationItemsController tests 'request.xhr?' at least 3 times
    Open

        redirect_to estimation_path(@estimation) unless request.xhr?
      end
    
      def destroy
        @estimation = Estimation.find(params[:estimation_id]).decorate

    Repeated Conditional is a special case of Simulated Polymorphism. Basically it means you are checking the same value throughout a single class and take decisions based on this.

    Example

    Given

    class RepeatedConditionals
      attr_accessor :switch
    
      def repeat_1
        puts "Repeat 1!" if switch
      end
    
      def repeat_2
        puts "Repeat 2!" if switch
      end
    
      def repeat_3
        puts "Repeat 3!" if switch
      end
    end

    Reek would emit the following warning:

    test.rb -- 4 warnings:
      [5, 9, 13]:RepeatedConditionals tests switch at least 3 times (RepeatedConditional)

    If you get this warning then you are probably not using the right abstraction or even more probable, missing an additional abstraction.

    EstimationItemsController assumes too much for instance variable '@estimation_item'
    Open

    class EstimationItemsController < ApplicationController

    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.

    Users::OmniauthCallbacksController assumes too much for instance variable '@user'
    Open

    class Users::OmniauthCallbacksController < Devise::OmniauthCallbacksController

    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.

    EstimationPolicy has no descriptive comment
    Open

    class EstimationPolicy < ApplicationPolicy
    Severity: Minor
    Found in app/policies/estimation_policy.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

    EstimationItemDecorator has no descriptive comment
    Open

    class EstimationItemDecorator < Draper::Decorator

    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

    ApplicationPolicy has no descriptive comment
    Open

    class ApplicationPolicy
    Severity: Minor
    Found in app/policies/application_policy.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

    ApplicationPolicy::Scope has no descriptive comment
    Open

      class Scope
    Severity: Minor
    Found in app/policies/application_policy.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

    WelcomeHelper has no descriptive comment
    Open

    module WelcomeHelper
    Severity: Minor
    Found in app/helpers/welcome_helper.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

    EstimationPolicy::Scope has no descriptive comment
    Open

      class Scope < Scope
    Severity: Minor
    Found in app/policies/estimation_policy.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

    Function activate_editables has a Cognitive Complexity of 7 (exceeds 5 allowed). Consider refactoring.
    Open

    var activate_editables = function () {
      $('.editable').editable({success: function(response, newValue) {
        if(!response.success) return response.msg; //msg will be shown in editable form
        var vals = response.additionalValues;
        if(vals) {
    Severity: Minor
    Found in app/assets/javascripts/application.js - About 35 mins to fix

    Cognitive Complexity

    Cognitive Complexity is a measure of how difficult a unit of code is to intuitively understand. Unlike Cyclomatic Complexity, which determines how difficult your code will be to test, Cognitive Complexity tells you how difficult your code will be to read and comprehend.

    A method's cognitive complexity is based on a few simple rules:

    • Code is not considered more complex when it uses shorthand that the language provides for collapsing multiple statements into one
    • Code is considered more complex for each "break in the linear flow of the code"
    • Code is considered more complex when "flow breaking structures are nested"

    Further reading

    ApplicationHelper has no descriptive comment
    Open

    module ApplicationHelper
    Severity: Minor
    Found in app/helpers/application_helper.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

    ApplicationController has no descriptive comment
    Open

    class ApplicationController < ActionController::Base

    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

    EstimationDecorator has no descriptive comment
    Open

    class EstimationDecorator < Draper::Decorator
    Severity: Minor
    Found in app/decorators/estimation_decorator.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

    EstimationsController assumes too much for instance variable '@estimation'
    Open

    class EstimationsController < ApplicationController

    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.

    Users::OmniauthCallbacksController has no descriptive comment
    Open

    class Users::OmniauthCallbacksController < Devise::OmniauthCallbacksController

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