Skip to content
New issue

Have a question about this project? Sign up for a free GitHub account to open an issue and contact its maintainers and the community.

By clicking “Sign up for GitHub”, you agree to our terms of service and privacy statement. We’ll occasionally send you account related emails.

Already on GitHub? Sign in to your account

Add exclusion filter for directories #1828

Open
wants to merge 8 commits into
base: main
Choose a base branch
from
Open

Conversation

lshala
Copy link
Contributor

@lshala lshala commented Nov 8, 2024

This PR adds the possibility to exclude folders or files from the analysis. Specifically, it should be possible to filter either by strings or by regex.

Copy link

codecov bot commented Nov 19, 2024

Codecov Report

Attention: Patch coverage is 63.15789% with 7 lines in your changes missing coverage. Please review.

Project coverage is 76.18%. Comparing base (2b0474a) to head (1d35c26).

Files with missing lines Patch % Lines
...e/fraunhofer/aisec/cpg/TranslationConfiguration.kt 63.63% 4 Missing ⚠️
...tlin/de/fraunhofer/aisec/cpg/TranslationManager.kt 66.66% 2 Missing ⚠️
...s/kotlin/de/fraunhofer/aisec/cpg/test/TestUtils.kt 50.00% 0 Missing and 1 partial ⚠️
Additional details and impacted files
Files with missing lines Coverage Δ
...s/kotlin/de/fraunhofer/aisec/cpg/test/TestUtils.kt 70.21% <50.00%> (-1.37%) ⬇️
...tlin/de/fraunhofer/aisec/cpg/TranslationManager.kt 71.64% <66.66%> (-0.16%) ⬇️
...e/fraunhofer/aisec/cpg/TranslationConfiguration.kt 89.71% <63.63%> (-1.46%) ⬇️

... and 2 files with indirect coverage changes

☔ View full report in Codecov by Sentry.
📢 Have feedback on the report? Share it here.


🚨 Try these New Features:

@lshala lshala marked this pull request as ready for review November 19, 2024 15:21
@@ -146,6 +146,13 @@ private constructor(

var sourceLocations: List<File> = ctx.config.softwareComponents[sc] ?: listOf()

sourceLocations =
Copy link
Member

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

I think this is not enough, I think you need to move the exclusion handling down to the file walk (or after the walk), because at this point here the locations are not "expanded". For example, if I set the source location "mypackage", which has a "tests" folder in it. In this case the exclusion filter filters based on mypackage, but not on mypackage/tests which is only expanded about 10 lines down.

Copy link
Contributor Author

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Done.

* exclusionPatterns(listOf(Regex(".*test(s)?")))
* ```
*/
fun exclusionPatternsByRegex(patterns: List<Regex>): Builder {
Copy link
Contributor Author

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Not sure if we should use two overloaded functions, e.g. “exclusionPatterns” or separate as here

Copy link
Member

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

You can overload them.

Copy link
Contributor Author

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Done. As far as I unterstand, the JvmName annotation is required because the two functions only differ by a generic parameter and the JVM erases that parameter during compilation

Copy link
Member

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Can we have varargs instead of a list? Then they should be different and varargs might be nice to work with

Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
None yet
Projects
None yet
Development

Successfully merging this pull request may close these issues.

2 participants