Support to exclude classes / packages by Regex pattern

[mikepenz]

The pull request introduced a new configuration option ignoredPatterns.

ignoredPatterns takes an array of regex patterns to exclude whole packages and classes based on this pattern. This configuration can be provided alongside the already existing ignoredPackages and ignoredClasses and offers much greater flexibility on ignoring sources from being included in the validation.

Motivation

The current offered APIs require absolute package names and class names, making them very verbose for large projects.
Additionally the nonPublicMarkers would be great for these situations, but we cannot attach them to generated sources from plugins, which we have no control over.

Using the regex provides much higher flexibility, and it is possible to for example exclude all packages including /internal/ in their package name.

Or similarly all generated classes by Androids navigation plugin which end in com.company.MyFragmentDirections

[qwwdfsad]

Thanks!

There is currently a big rework with help of @aSemy, I'll get back to this PR as soon as the work with the plugin itself is done

[mikepenz]

Thank you very much @qwwdfsad for the update. Happy to rebase and resolve conflicts on this after those reworks were done. (in case the PR is still relevant afterwards)

[RingerJK]

@qwwdfsad can the PR be moved forward?

[mikepenz]

It probably has to be re-created now given the time since it was created. I was waiting until the big rework happened.

[mikepenz]

Rebased this PR on top of the latest changes

[mikepenz]

Please let me know if there's anything I can do to help with this PR. :D

[fzhinkin]

@mikepenz, I'm really sorry for how slowly this is going. 😿

In general, the feature is extremely helpful, and I'd love to incorporate it into BCV.

What bothers me are the details:

• Accepting patterns as raw strings tends to be error-prone. Compiled REs look better as they can help by catching typos earlier, and the RE-syntax could also be highlighted by the IDE on the RE-construction site (although highlighting may not work for now). However, it's unclear what type should represent the RE: kotlinx.text.Regex is not available in Groovy builds scripts (build.grade), and writing java.util.regex.Pattern.compile(...) doesn't feel native in kts-scripts.
• The ignored property name in conjunction with its type (Set<String>) is confusing as it's unclear what will be ignored, and from the type, it's hard to tell if it accepts literal strings, REs or some Ant-style wildcards.
• It's probably OK to use the same RE for both packages and classes (at least, I can't come up with an example where separating them would be beneficial), but should we also allow using REs for other filters, like nonPublicMarkers? The latter could simplify configs like this one, although there are no so many examples out there.

I would love to hear your opinion regarding the listed (potential) problems and possible solutions.

---

Perhaps, instead of a new collection holding strings, old properties could be replaced with a DSL like this:

// disclaimer: names were not thought through very well
interface NameFilters {
    fun named(fqName: String): Unit
    fun matching(re: Regex): Unit
    fun matching(re: Pattern): Unit
}

class ApiValidationExtension { 
     ...
    // these could co-exist with old properties
    fun ignoredPackages(block: Action<NameFilters>) { ... }
    fun ignoredClasses(block: Action<NameFilters>) { ... }
    ...
}

And then configuration would look like this:

apiValidation {
    ignoredClasses {
        named("com.example.IgnoreMe")
        matching(Regex(".*\.ignore\.us\.all\.*"))
    }
}

I'm not proposing the particular API drafted above, just showing it as an example.

[mikepenz]

Thank you very much for your review and feedback. And I fully understand the need moving carefully on this project.

Accepting patterns as raw strings tends to be error-prone. Compiled REs look better as they can help by catching typos earlier, and the RE-syntax could also be highlighted by the IDE on the RE-construction site (although highlighting may not work for now). However, it's unclear what type should represent the RE: kotlinx.text.Regex is not available in Groovy builds scripts (build.grade), and writing java.util.regex.Pattern.compile(...) doesn't feel native in kts-scripts.

Agreeing with your observation that especially when used from Kotlin vs from Groovy that it is not as clear. Even more so as Groovy has its own syntax which allow you to write the regex pattern directly in place (e.g. ~/\d*/).
Providing java.util.regex.Pattern feels to be probably the best of both worlds.

The ignored property name in conjunction with its type (Set) is confusing as it's unclear what will be ignored, and from the type, it's hard to tell if it accepts literal strings, REs or some Ant-style wildcards.

Very good point. Changing away from string to Pattern partially resolves this question, however it wouldn't yet clarify that it's then matching on the fully qualified class name.

ignoredClassesPattern does appear far to verbose.

It's probably OK to use the same RE for both packages and classes (at least, I can't come up with an example where separating them would be beneficial), but should we also allow using REs for other filters, like nonPublicMarkers? The latter could simplify configs like this one, although there are no so many examples out there.

Overall I feel the same. Can't think of a specific example where we would benefit from splitting them, specifically as the regex would allow excluding either component.

If background compatibility isn't the major concern I really like your proposed idea of the significantly more clear DSL offering an easy ability to mix and match between the different forms. So it should become significantly more consistent throughout, also allowing the same logic to be working for packages, but also annotations as you noted in your prior message.

[fzhinkin]

If background compatibility isn't the major concern I really like your proposed idea of the significantly more clear DSL offering an easy ability to mix and match between the different forms.

Compatibility is always a concern. However, a DSL could be provided without breaking it (and then the old API could be gradually phased out).

[mikepenz]

Would you suggest we should update this branch to follow an API as brainstormed?
If so I might be able to work on updating this branch.

[fzhinkin]

Looping @shanshin into discussion. Privately, we discussed that the DSL needs to be idiomatic w.r.t. other Gradle plugins, especially the KGP. Sergey has some ideas on how such an API may look like, the main concern is providing a DSL that will fit KGP once #223 is done.

[mikepenz]

Rebased on top of the latest changes and changed ignored to ignoredPatterns