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

Piping source ignores excluded_paths configuration #1429

Open
stevenharman opened this issue Oct 16, 2018 · 1 comment
Open

Piping source ignores excluded_paths configuration #1429

stevenharman opened this issue Oct 16, 2018 · 1 comment
Labels

Comments

@stevenharman
Copy link

I am working on the Reek + Ale integration which will pipe the source and pass the --stdin-filename argument. I noticed Reek was ignoring my configured exclude_paths and so errors were being reported via Ale. I tried passing the --force-exclusion flag, but that didn't change anything. I then confirmed all of this by calling reek in a similar manner. An example:

---
# Directories below will not be scanned at all
exclude_paths:
  - db/migrate

Piping to reek, with flags:

$ cat db/migrate/20181015212457_create_foos.rb | reek --config .reek.yml --force-exclusion  --stdin-filename 'db/migrate/20181015212457_create_foos.rb'
db/migrate/20181015212457_create_foos.rb -- 3 warnings:
  [6, 7, 8, 9, 11]:FeatureEnvy: CreateFoos#change refers to 't' more than self (maybe move it to another class?) [https://github.com/troessner/reek/blob/v5.1.0/docs/Feature-Envy.md]
  [4]:TooManyStatements: CreateFoos#change has approx 9 statements [https://github.com/troessner/reek/blob/v5.1.0/docs/Too-Many-Statements.md]
  [5]:UncommunicativeVariableName: CreateFoos#change has the variable name 't' [https://github.com/troessner/reek/blob/v5.1.0/docs/Uncommunicative-Variable-Name.md]

I would have expected Reek to ignore this file, per the configuration.

I started digging into Reek and it looks like Reek::CLI::Application#source_from_pipe directly loads the code, rather than using SourceLocator to filter out source files based on the excluded_paths configuration.

def source_from_pipe
[Source::SourceCode.from($stdin, origin: options.stdin_filename)]
end

I wonder if there's a way to either leverage, or exact, the bits of Source::SourceLocator which determine if we should exclude a source file. If so, we could check if stdin_filename is set, and if so, check if the file is excluded.

Thoughts or suggestions? I'd be happy to get a PR going, given a little guidance on a preferred approach.

Thank you!

See also: #1343 (comment)

@mvz
Copy link
Collaborator

mvz commented Oct 17, 2018

Hi @stevenharman, thank you for this detailed bug report. This combination of flags should indeed have the effect you expected.

Extracting the exclusion logic from SourceLocator sounds like a good approach, so if you'd like to take a stab at a PR, that would be much appreciated.

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

No branches or pull requests

2 participants