Skip to content

Commit

Permalink
Enable regex for directory directives (#1452)
Browse files Browse the repository at this point in the history
* Enable regex for directory directives

The code was already ready for that but escaped the potential regexp declared in
the reek configuration, actually disabling regexp.

This commit fixes this by removing the call to escape.

This allows this kind of declaration in the configuration file:

```yaml
---
[...]
  "engines/.*/test/.*":
    InstanceVariableAssumption:
      enabled: false
    UncommunicativeVariableName:
      enabled: false
```

* Support glob like syntax for directory patterns

This is a mix of currently supported patterns and `glob` patterns.

Currently supported patterns assumes that declared directories includes
sub-directories as well.

`glob` like patterns act like Dir.glob, but are actually backed by a
Regexp, this may lead to some different behaviour.

* Add documentation about Dir.glob pattern for `directory` directives

* Escape . characters

This make sure that the directive does not match an arbitrary character
that is not a `.`.

* Fix code alignment
  • Loading branch information
pbernery authored and troessner committed Apr 24, 2019
1 parent 0bf176f commit 4f0b629
Show file tree
Hide file tree
Showing 3 changed files with 78 additions and 6 deletions.
10 changes: 8 additions & 2 deletions README.md
Original file line number Diff line number Diff line change
Expand Up @@ -309,12 +309,18 @@ detectors:
# You can configure smells on a per-directory base.
# E.g. the classic Rails case: controllers smell of NestedIterators (see /docs/Nested-Iterators.md) and
# helpers smell of UtilityFunction (see docs/Utility-Function.md)
# Note that we only allow configuration on a directory level, not a file level, so all paths have to point to directories.
#
# Note that we only allow configuration on a directory level, not a file level,
# so all paths have to point to directories.
# A Dir.glob pattern can be used.
directories:
"web_app/app/controllers":
NestedIterators:
enabled: false
"web_app/app/helpers":
"web_app/app/helpers**:
UtilityFunction:
enabled: false
"web_app/lib/**/test/**":
UtilityFunction:
enabled: false

Expand Down
29 changes: 28 additions & 1 deletion lib/reek/configuration/directory_directives.rb
Original file line number Diff line number Diff line change
Expand Up @@ -53,10 +53,37 @@ def add(directory_config)
# @quality :reek:FeatureEnvy
def best_match_for(source_base_dir)
keys.
select { |pathname| source_base_dir.to_s.match(/#{Regexp.escape(pathname.to_s)}/) }.
select { |pathname| source_base_dir.to_s.match(glob_to_regexp(pathname.to_s)) }.
max_by { |pathname| pathname.to_s.length }
end

# Transform a glob pattern to a regexp.
#
# It changes:
# - /** to .*,
# - ** to .*,
# - * to [^\/]*.
#
# @quality :reek:FeatureEnvy
# @quality :reek:UtilityFunction
def glob_to_regexp(glob)
is_glob_pattern = glob.include?('*')

regexp = if is_glob_pattern
glob.
gsub(%r{/\*\*$}, '<<to_eol_wildcards>>').
gsub('**', '<<to_wildcards>>').
gsub('*', '[^/]*').
gsub('.', '\.').
gsub('<<to_eol_wildcards>>', '.*').
gsub('<<to_wildcards>>', '.*')
else
glob + '.*'
end

Regexp.new("^#{regexp}$", Regexp::IGNORECASE)
end

def error_message_for_invalid_smell_type(klass)
"You are trying to configure smell type #{klass} but we can't find one with that name.\n" \
"Please make sure you spelled it right. (See 'docs/defaults.reek' in the Reek\n" \
Expand Down
45 changes: 42 additions & 3 deletions spec/reek/configuration/directory_directives_spec.rb
Original file line number Diff line number Diff line change
Expand Up @@ -44,9 +44,12 @@
describe '#best_match_for' do
let(:directives) do
{
Pathname.new('foo/bar/baz') => {},
Pathname.new('foo/bar') => {},
Pathname.new('bar/boo') => {}
Pathname.new('foo/bar/baz') => {},
Pathname.new('foo/bar') => {},
Pathname.new('bar/boo') => {},
Pathname.new('bar/**/test/**') => {},
Pathname.new('bar/**/spec/*') => {},
Pathname.new('bar/**/.spec/*') => {}
}.extend(described_class)
end

Expand All @@ -62,6 +65,42 @@
expect(hit.to_s).to eq('foo/bar')
end

it 'returns the corresponding directory when source_base_dir matches the Dir.glob like pattern' do
source_base_dir = 'bar/something/test'
hit = directives.send :best_match_for, source_base_dir
expect(hit.to_s).to eq('bar/**/test/**')
end

it 'returns the corresponding directory when source_base_dir is a leaf of the Dir.glob like pattern' do
source_base_dir = 'bar/something/test/with/some/subdirectory'
hit = directives.send :best_match_for, source_base_dir
expect(hit.to_s).to eq('bar/**/test/**')
end

it 'returns the corresponding directory when source_base_dir is a direct leaf of the Dir.glob like pattern' do
source_base_dir = 'bar/something/spec/direct'
hit = directives.send :best_match_for, source_base_dir
expect(hit.to_s).to eq('bar/**/spec/*')
end

it 'returns the corresponding directory when source_base_dir contains a . character' do
source_base_dir = 'bar/something/.spec/direct'
hit = directives.send :best_match_for, source_base_dir
expect(hit.to_s).to eq('bar/**/.spec/*')
end

it 'does not match an arbitrary directory when source_base_dir contains a character that could match the .' do
source_base_dir = 'bar/something/aspec/direct'
hit = directives.send :best_match_for, source_base_dir
expect(hit.to_s).to eq('')
end

it 'returns nil when source_base_dir is a not direct leaf of the Dir.glob one-folder pattern' do
source_base_dir = 'bar/something/spec/with/some/subdirectory'
hit = directives.send :best_match_for, source_base_dir
expect(hit).to be_nil
end

it 'returns nil we are on top of the tree and all other directories are below' do
source_base_dir = 'foo'
hit = directives.send :best_match_for, source_base_dir
Expand Down

0 comments on commit 4f0b629

Please sign in to comment.