Skip to content
New issue

Have a question about this project? Sign up for a free GitHub account to open an issue and contact its maintainers and the community.

By clicking “Sign up for GitHub”, you agree to our terms of service and privacy statement. We’ll occasionally send you account related emails.

Already on GitHub? Sign in to your account

Handling exclude_paths in reek configuration #48

Open
abhinavmsra opened this issue Nov 18, 2020 · 0 comments
Open

Handling exclude_paths in reek configuration #48

abhinavmsra opened this issue Nov 18, 2020 · 0 comments

Comments

@abhinavmsra
Copy link

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
  1. 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.

Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
None yet
Projects
None yet
Development

No branches or pull requests

1 participant