blooper05/danger-reek

A Danger plugin to lint Ruby files through Reek.

Hi there 👋

danger-reek's Issues

Add support for older reek versions

Lint is executed for unexpected files

e.g. Gemfile.lock, README.md and YAML files

Add support for reading config files

Handling `exclude_paths` in reek configuration

Acknowledgement

I love danger & its plugin gems. They are always a part of my CI suite. I would like to express my gratitude to the maintainers of this project. 👏👏👏

Current Behavior

Danger::DangerReek

def lint
  files_to_lint = fetch_files_to_lint
  code_smells   = run_linter(files_to_lint)
  warn_each_line(code_smells)
end

Step Analysis

1. Builds a file list to examine, basically all changed files

fetch_files_to_lint = git.modified_files + git.added_files

2. Builds an examiner object for each files & evaluates it's smells.

run_linter(files_to_lint)

def run_linter(files_to_lint)
  configuration = ::Reek::Configuration::AppConfiguration.from_path(nil)
  files_to_lint.flat_map do |file|
    examiner = ::Reek::Examiner.new(file, configuration: configuration)
    examiner.smells
  end
end

examiner = ::Reek::Examiner.new(file, configuration: configuration)

This examiner does not check if the file path is in exclude_paths on Reek configuration. And as such, posts warning messages on PR.

Issue

exclude_paths is .reek.yml has no effect.

exclude_paths:
  - db/migrate

It gets particularly annoying if you add migration files. It posts FeatureEnvy message on each files.

Refers to 't' more than self

Proposed Enhancement

I made it work with the following change:

module MyDanger
  module ReekDecorator
    private

    def fetch_files_to_lint
      files = git.modified_files + git.added_files
      excluded_files = files.to_a.select { |file| file_excluded?(file) }
      files_to_lint = files - excluded_files
      ::Reek::Source::SourceLocator.new(files_to_lint).sources
    end

    def file_excluded?(filename)
      configuration = ::Reek::Configuration::AppConfiguration.from_path(nil)

      configuration.send(:excluded_paths).map(&:expand_path).map do |excluded_path|
        Pathname(filename).expand_path.fnmatch?(File.join(excluded_path, '**'))
      end.any?
    end
  end
end

Danger::DangerReek.prepend(MyDanger::ReekDecorator)

• It basically modifies fetch_files_to_lint method to discard files in excluded_paths path

It would be great to have some community feedback on this.

I can create a PR if my proposed solution is agreeable.

CI is failing due to Rubocop settings

Like this https://travis-ci.org/blooper05/danger-reek/jobs/290699063#L658

Show smell name as well as description

Currently, a comment left by danger-reek looks like this:

spec/rails_helper.rb#L49Doesn’t depend on instance state (maybe move it to another class?)

To find out which smell this is, you need to grep the reek source for the comment text. (It's also quite difficult to read with no space between the line number and the text…)

It's also not even particularly obvious that the comment was injected by danger-reek, and not something else in the Dangerfile.

Would it be possible for the comment to look like this instead?

spec/rails_helper.rb#L49 [Reek:UtilityFunction] Doesn’t depend on instance state (maybe move it to another class?)

Code Climate's badge is not working