Ignore irrelevant cops when investigating a file #2175
Conversation
bquorning
force-pushed
the
dont-process-irrelevant-cops
branch
from
39c140c to
02fd49d
Compare
| @cops.select! { |cop| cop.relevant_file?(filename) } | ||
| end | ||
|
|
||
| # TODO: Bad design. |
There was a problem hiding this comment.
:-) That's one pretty generic remark.
|
0.00004% decreased code coverage is causing a red build? On the plus side, the coverage of lib/rubocop/cop/commissioner.rb increased to 102.04%! |
| @@ -64,6 +63,11 @@ def reset_errors | |||
| end | |||
|
|
|||
| # TODO: Bad design. | |||
| def remove_irrelevant_cops(filename) | |||
| @cops.select! { |cop| cop.relevant_file?(filename) } | |||
There was a problem hiding this comment.
This method name and the implementation kinda points in opposite directions. I thought of either naming the method #select_relevant_cops or changing the implementation to @cops.reject! { |cop| !cop.relevant_file?(filename) }. What do you think?
There was a problem hiding this comment.
You can also add Cop#irrelevant_file?. :-) (or excluded_file?, which is more in line with our terminology so far).
|
Overall the changes look good to me. @jonas054 Any comments? |
| @@ -46,14 +46,13 @@ def #{callback}(node) | |||
|
|
|||
| def investigate(processed_source) | |||
| reset_errors | |||
| remove_irrelevant_cops(processed_source.buffer.name) | |||
| prepare(processed_source) | |||
| invoke_custom_processing(@cops, processed_source) | |||
| invoke_custom_processing(@forces, processed_source) | |||
| process(processed_source.ast) if processed_source.ast | |||
| @cops.each_with_object([]) do |cop, offenses| | |||
There was a problem hiding this comment.
I guess this could be changed to a simple map now?
bquorning
force-pushed
the
dont-process-irrelevant-cops
branch
from
02fd49d to
dc45832
Compare
| # ignore files that are of no interest to the cop in question | ||
| offenses.concat(cop.offenses) if cop.relevant_file?(filename) | ||
| end | ||
| @cops.flat_map(&:offenses) |
There was a problem hiding this comment.
#flat_map and symbol-to-proc, how about that?
bquorning
force-pushed
the
dont-process-irrelevant-cops
branch
from
1e2f7ca to
1ddef56
Compare
Instead of checking a cop's relevance for the current file multiple times in the investigation process, we can filter the cops one time, at the very beginning of the investigation.
The default config for the Rails/ActionFilter cop tells it to operate
only on controllers (`{ 'Include' => ['app/controllers/**/*.rb'] }`),
causing it to *not* operate on the temporary files generated by the
spec. Nullifying the `Include` configuration option in the spec makes
the spec pass.
bquorning
force-pushed
the
dont-process-irrelevant-cops
branch
from
1ddef56 to
4646794
Compare
|
Looks good. I thought that this was going to be much more difficult to implement. |
Ignore irrelevant cops when investigating a file
Fixes #2142.
Instead of checking a cop's relevance for the current file multiple times in the investigation process, we can filter the cops one time, at the very beginning of the investigation.
The specs
arewere failing because ofRails/ActionFilters default configuration:The temporary file being tested of course doesn’t match the configured
Includepattern, and is therefore ignored.But I believe this is a problem with the spec, not with the implementation. Please help me: how can the specs be fixed?