Sign upLogin

luislavena/gem-compiler

View on GitHub
Star

Showing 34 of 34 total issues

Gem::Compiler assumes too much for instance variable '@target_dir'
Open

class Gem::Compiler
Severity: Minor
Found in lib/rubygems/compiler.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.

Method simple_run has a Cognitive Complexity of 7 (exceeds 5 allowed). Consider refactoring.
Open

  def simple_run(command, command_name)
    begin
      output, status = Open3.capture2e(*command)
    rescue => error
      raise Gem::CompilerError, "#{command_name} failed#{error.message}"
Severity: Minor
Found in lib/rubygems/compiler.rb - 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

Gem::Compiler#prepare_installer calls 'installer.spec' 6 times
Open

    installer.spec.full_gem_path = @target_dir
    installer.spec.extension_dir = File.join(@target_dir, "lib")

    # Ensure Ruby version is met
    ensure_ruby_version_met(installer.spec)
Severity: Minor
Found in lib/rubygems/compiler.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.

Gem::Compiler#adjust_abi_lock calls 'gemspec.required_ruby_version = "~> #{ruby_abi}"' 2 times
Open

      gemspec.required_ruby_version = "~> #{ruby_abi}"
    when :strict
      cfg = RbConfig::CONFIG
      ruby_abi = "#{cfg["MAJOR"]}.#{cfg["MINOR"]}.#{cfg["TEENY"]}.0"
      gemspec.required_ruby_version = "~> #{ruby_abi}"
Severity: Minor
Found in lib/rubygems/compiler.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.

Gem::Compiler#unpack manually dispatches method call
Open

    if installer.respond_to?(:package)
Severity: Minor
Found in lib/rubygems/compiler.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)

Gem::Compiler#strip_artifacts calls 'options[:strip]' 2 times
Open

    return unless options[:strip]

    strip_cmd = options[:strip]
Severity: Minor
Found in lib/rubygems/compiler.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.

Gem::Commands::CompileCommand#initialize calls 'Dir.pwd' 2 times
Open

      output: Dir.pwd,
      abi_lock: :ruby
    }

    super "compile", "Create binary pre-compiled gem", defaults
Severity: Minor
Found in lib/rubygems/commands/compile_command.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.

Gem::Compiler has no descriptive comment
Open

class Gem::Compiler
Severity: Minor
Found in lib/rubygems/compiler.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

Gem::Compiler#build_extensions manually dispatches method call
Open

    if installer.respond_to?(:run_pre_install_hooks)
Severity: Minor
Found in lib/rubygems/compiler.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)

Gem::Compiler#adjust_gemspec_files calls 'gemspec.files' 2 times
Open

      gemspec.files.reject! { |f| !File.exist?("#{target_dir}/#{f}") }
    end

    # add discovered artifacts
    artifacts.each do |path|
Severity: Minor
Found in lib/rubygems/compiler.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.

Gem::Commands::CompileCommand#initialize performs a nil-check
Open

      if value.nil? || value.empty?
Severity: Minor
Found in lib/rubygems/commands/compile_command.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)

Gem::Compiler#platform_shared_ext doesn't depend on instance state (maybe move it to another class?)
Open

  def platform_shared_ext
Severity: Minor
Found in lib/rubygems/compiler.rb by reek

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

Gem::Compiler#initialize has the parameter name '_options'
Open

  def initialize(gemfile, _options = {})
Severity: Minor
Found in lib/rubygems/compiler.rb by reek

An Uncommunicative Parameter Name is a parameter name that doesn't communicate its intent well enough.

Poor names make it hard for the reader to build a mental picture of what's going on in the code. They can also be mis-interpreted; and they hurt the flow of reading, because the reader must slow down to interpret the names.

Gem::Compiler#adjust_gemspec_files has the variable name 'f'
Open

      gemspec.files.reject! { |f| !File.exist?("#{target_dir}/#{f}") }
Severity: Minor
Found in lib/rubygems/compiler.rb by reek

An Uncommunicative Variable Name is a variable name that doesn't communicate its intent well enough.

Poor names make it hard for the reader to build a mental picture of what's going on in the code. They can also be mis-interpreted; and they hurt the flow of reading, because the reader must slow down to interpret the names.

Severity
Category
Status
Source
Language