You signed in with another tab or window. Reload to refresh your session.You signed out in another tab or window. Reload to refresh your session.You switched accounts on another tab or window. Reload to refresh your session.Dismiss alert
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 allexclude_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.
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.
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.
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 configuredexclude_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 callingreek
in a similar manner. An example:Piping to
reek
, with flags: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 usingSourceLocator
to filter out source files based on theexcluded_paths
configuration.reek/lib/reek/cli/application.rb
Lines 116 to 118 in add8959
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 ifstdin_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)
The text was updated successfully, but these errors were encountered: