Escape Regexp-like Strings on CodeContext matches

[chastell]

I recently noticed a bug where a |-ending entry in an exclude config setting for a given smell would effectively exclude everything from the given smell; the below excludes all methods (not just Foo#|) from being checked aginst UncommunicativeVariableName:

UncommunicativeVariableName:
  exclude:
    - Foo#|

Today I noticed a bug where a []-ending entry blows up the whole tool; the below makes Reek not work at all:

UncommunicativeParameterName:
  exclude:
    - Bar#self.[]

I tracked down the culprit of both of these cases – the excludes were treated as regular expressions rather than straight strings.

[mvz]

There may be backwards-compatibility issues for people taking advantage of the fact that strings are interpreted as regexes. 😢

[chastell]

Hm, I think this borders on the classic ‘if someone potentially depending on a buggy behaviour means a bugfix is a breaking change then every bugfix would require a major version bump’ problem.

I looked at Reek docs and it seems Basic Smell Options explicitly suggests that if you want to use a Regexp then you should mark it as one:

ControlCouple: 
  exclude:
  - !ruby/regexp /write/

The problem with the current version is that – contrary to what the other examples suggest – you can’t use a plain string, because you have to explicitly escape it by hand. Also, exclude entries ending with | silently exclude the whole smell (which is a very surprising behaviour and masked a lot of smells in my case) and excludes ending with [] blow up Reek (which should never happen).

I think all of the above outweight the potential breakage (especially that if someone depends on regular expressions in the config they can still use regular expressions).

[mvz]

I agree. If the documentation explicitely mentions regexps like that we can safely make this change.

[troessner]

@chastell and 2.0.1 is released.:)

[chastell]

✨