phpro / grumphp

A PHP code-quality tool
MIT License
4.13k stars 430 forks source link

Replace blacklist/whitelist with better alternatives #888

Closed tomasnorre closed 3 years ago

tomasnorre commented 3 years ago

I allowed my self to delete the template as it wasn't relevant for my suggestion.

I would suggest replacing blacklist/whitelist with better alternatives

https://github.com/phpro/grumphp/search?q=blacklist https://github.com/phpro/grumphp/search?q=whitelist

I would suggest:

Allowlist and denylist.

veewee commented 3 years ago

Thanks for reporting @tomasnorre.

This would introduce a massive BC break. If it can be done in a backwards compatible way, I am willing to accept a pull request on this. It won't be as easy as replacing the occurrences though.

Feel free to give it a try.

tomasnorre commented 3 years ago

If it's a BC Break I can be done with a new major release ;) I'll see if I can figure something out.

veewee commented 3 years ago

I prefer not to tag a major for configuration rewording.

Besides that, grumphp has always tried to give meaningful errors. In this case, being BC compatible would mean that grumphp provides a message like: this config is deprecated, please use x instead.

This change will be quite big for the projects and packages that rely on grumphp. So it need to go as smooth as possible.

One last note: the config from grumphp Is mapped to the config from the tools it uses. Meaning that if that tool uses words like whitelist or blacklist, we won't be able to change it.

Taken all these considerations, it will be quite some thinking and work for a result that might not be 100% of what you want it to be

tomasnorre commented 3 years ago

Thanks for your input. I don't know the tool that well, so don't know the consequences of such a change, so valuable information to me.