Directory configurations changed incompatibly in v5.4.0

[rasaffie]

When updating from v5.3.2 to v5.4.0 running reek from CLI and Rake::Task has different outcomes:

My rake configuration is:

Reek::Rake::Task.new do |t|
  t.config_file   = '.reek.yml'
  t.source_files  = FileList['./**/*.rb'].exclude('./vendor/**/*.rb')
end

The problem is that the rake task finds the configuration file (changing its name causes a "file not found" error), but doesn't consider the directories rules, like:

directories:
  "db/migrate":
    FeatureEnvy:
      enabled: false
    UncommunicativeVariableName:
      enabled: false
    IrresponsibleModule:
      enabled: false
    UtilityFunction:
      enabled: false
    TooManyStatements:
      enabled: false

[troessner]

Just so we are triple clear on this, with 5.3.2 the output is exactly the same, right?

[rasaffie]

Thanks for your time. Yes, there is no difference using v5.3.2:

I also noticed that running my tests on CircleCI with v5.4.0 the folder vendor is not being excluded, so the task is ignoring that configuration.

The commit https://github.com/troessner/reek/commit/4f0b629a941dd02974f5bad136d5d0cca89d3dbe introduced the difference. @pbernery any thought on this?

[pbernery]

Interesting, I was not aware of this Rake task feature.

Looking rapidly at the code, I think the directives do not work because they are not applied relative to the root directory, which is mostly the case when running reek from the command line because it's usually called with reek ..

When using the Rake task, you're specifying a pattern which selects only *.rb files in subfolders. If I am not mistaken, the directives are interpreted relative to the given file, which will probably match nothing in your case.

I guess the issue can also be reproduced when calling Reek from the command line by passing the pattern as an option, like reek ./**/*.rb.

To help identify the issue, and this also could give you a workaround, could you try those two things (separately):

1. Run reek from the command line using reek ./**/*.rb
2. Update your Rake .source_files to FileList['.'] (if it's valid, I've never used FileList so far)

[mvz]

I think @pbernery's analysis is not entirely correct; The directives are applied to the file path as known to reek, which in the normal case does not include a leading ./. Your task configuration, however, does introduce such a leading ./ because of the parameters passed to FileList.

Can you try configuring the task with:

Reek::Rake::Task.new do |t|
  t.config_file   = '.reek.yml'
  t.source_files  = FileList['**/*.rb'].exclude('vendor/**/*.rb')
end

I think in the 5.3 version of the code we unintentially matched the excluded directory names anywhere in the path, so db/migrate would match db/migrate/..., ./db/migrate/..., and somewhere/else/db/migrate/....

[mvz]

Run reek from the command line using reek ./**/*.rb.

This is also a good thing to try to verify the analysis.

[rasaffie]

Thanks! With the configuration @mvz proposed it works as expected.

Running the CLI with reek ./**/*.rb reproduces the error.

[mvz]

@rasaffie I think that still means this is broken.

[mvz]

I don't think we will fix this in the 5.x series.