Implement --force-exclusion flag
#1200
Conversation
|
I'm not sure why Travis is getting different test results than I am. Full test site continues to run fine for me, as does the ci task: |
| allow(options).to receive(:force_exclusion?).and_return(true) | ||
| end | ||
|
|
||
| it 'excludes files in ignored directories' do |
There was a problem hiding this comment.
I found this to be quite confusing. Above it says "when path is a file name". Ok, sounds good. Then it says down here "excludes files in ignored directories" and I'm thinking "wait, what? How can a file name exclude files in directories"?
How about
it 'excludes this file'
to make this clearer?
|
One small comment, rest looks great! |
|
@IanWhitney we're almost there ;) |
|
I'll poke at the CI problem on Wednesday, probably. Still not sure what's going on there as it seems to be specific to Travis. Rubocop should be good. Running it locally shows no problems and codeclimate has marked it as passed. Once the CI is passing I'll rebase and push. |
|
Ah, I hadn't looked at all of the Travis errors, just the first. My fault. So, it looks like Ruby 2.1 and 2.2 are failing due to a LocalJumpError. Ruby 2.3 and 2.4 are failing due to Rubocop's, mostly because of nesting. What's the thought on that? The test nesting seems entirely appropriate to me and my inclination is to override Rubocop in this case. |
|
a08c47e should fix the LocalJumpError in Ruby 2.1 and 2.2. Turned out to be kind of interesting, if you find new behaviors added to Ruby iterator methods interesting, I guess. I'll get to the Rubocop stuff in tomorrow or Wednesday. |
|
All that's left is Rubocop's flagging of test nesting. I think the nesting is appropriate, so I'm going to disable the |
|
Yay, all checks have passed. If everyone is good with this I'll rebase. |
|
Please squash, then lgtm ;) |
Fixes #1196 This introduces a --force-exclusion flag that mirrors the behavior of Rubocop's similarly-named flag. If the config contains an exclude_path for smelly_path and CLI is called with reek ./smelly_path/file.rb Then the behavior of Reek is unchanged and the file will be checked for smell violations But if the CLI is called with reek --force-exclusion ./smelly_path/file.rb Then the file will not be checked and Reek will exit with 0. Notes: - For the Source Locator to know about the option being set we have to pass the options in. I'm following the existing pattern here of named arguments with a default. - Choosing to ignore a file is different than ignoring a path, so there's a new method encapsulate it. If the force_exclusion option is in effect, the `ignore_file?' method looks to see if any of the file's parent paths are excluded. - Even though I changed code in `reek/cli/application` I did not add tests. I didn't see that there were any existing tests that cared how an instance of `Source::SourceLocator` was constructed (by mocking the call, or similar approaches). Since the tests of application continued to pass, I didn't see a reason to add new tests here. - The `ascend` method wasn't fully enumerable before Ruby 2.3 This commit uses the version of ascend in place since Ruby 1.9 for backwards compatibility with Ruby 2.1 and 2.2. Background: The 1.9.3 - 2.2 version of the docs: http://ruby-doc.org/stdlib-1.9.3/libdoc/pathname/rdoc/Pathname.html#method-i-ascend The 2.3 and up version http://ruby-doc.org/stdlib-2.3.1/libdoc/pathname/rdoc/Pathname.html#method-i-ascend Note the "Returns an Enumerator if no block was given" section that's been added.
|
Done. |
|
Quick question: is there a way for API consumers to set this option? pronto-reek uses Reek::Examiner directly, and I don't see a way to pass the CLI Options object to that. This option would be very useful for pronto-reek, because right now exclude_paths is effectively useless there. Related issue: prontolabs/pronto-reek#19 Thanks! |
|
I commented on prontolabs/pronto-reek#19. |
Closes #1196
There is probably work to do on this (such as creating a new scenario as @mvz suggests), but I wanted to get this up for review and feedback as soon as possible.
As discussed, this introduces a
--force-exclusionflag that mirrors the behavior of Rubocop's similarly-named flag. If the config contains anexclude_pathforsmelly_pathand CLI is called withreek ./smelly_path/file.rbThen the behavior of Reek is unchanged and the file will be checked for smell violations
But if the CLI is called with
reek --force-exclusion ./smelly_path/file.rbThen the file will not be checked and Reek will exit with 0.
There's no breaking changes in this commit and all existing tests -- Cucumber and RSpec -- continue to pass.