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

Allow globally defined exclusion pattern #2581

Closed
jcornaz opened this issue Apr 5, 2020 · 8 comments
Closed

Allow globally defined exclusion pattern #2581

jcornaz opened this issue Apr 5, 2020 · 8 comments

Comments

@jcornaz
Copy link

jcornaz commented Apr 5, 2020

Expected Behavior

I would like to be able to globally edit the exclusion patterns, rather than chaning them one by one for each rules.

Current Behavior

Currently each rules has its own exclusion pattern. That leads to a lot of repetition in the config files, and it is especially annoying and error prone when it is time to change the patterns as we are forced to go through all of them one by one.

Context

I introduced new test source sets in a project and I have to tell detekt to ignore that sourceset for a bunch of rules. Which rules? Simple, all the rules that are already excluding the test sources.

But instead of having to change one line to tell detekt what are the patterns of test sources, I have to go through each rules and edit them one by one.

The configuration styled proposed by #775 would have been perfect.

@BraisGabin
Copy link
Member

I like the current way to exclude because it allows you to tune your config a lot. But I have the same problem as @jcornaz. I think that detekt should support the current exclusion method and the proposed in #775 or similar. My proposal is:

exclusion-patterns:
- patterns:
    - **/test/**
    - **/androidTest/**
    - **/*.Test.kt
    - **/*.Spec.kt
    - **/*.Spek.kt
  rules:
    - VariableNaming
    - FunctionNaming
- patterns:
    - *DTO.kt
  rules:
    - Rule4
    - Rule8
...

@BraisGabin
Copy link
Member

To give more context I'm going to explain my use case:

Every time that I update detekt I run --generate-config. This rewrites ALL my configurations so then I go change by change checking if I prefer the new defautl configuration or my old one. I do this because I like the have all the posible parameters in the configuration file so if I need to tune any rule I don't need to check any documentation.

This was not a big problem because I have just a few changes in my configuration file so the changes to check were few. But two month ago I added a new source route for my tests. And now I need to revert 24 lines each time that I update detekt.

With the proposed format I should just need to revert one removed line.

@BraisGabin
Copy link
Member

New format proposed in #2851

exclusion-patterns:
  - 'dont apply in tests':
    patterns:
      - '**/test/**'
      - '**/androidTest/**'
      - '**/commonTest/**'
    rules:
      - 'comments'
  - 'mappers are an special case too':
    patterns:
      - '**/jvmTest/**'
      - '**/jsTest/**'
      - '**/iosTest/**'
    rules:
      - 'potential-bugs>LateinitUsage'
      - 'style>MagicNumber'
      - 'style>WildcardImport'

@DcortezMeleth
Copy link
Contributor

Hi!

Any plans on implementing this in a near future? Maybe I can somehow help?

I would also benefit from such feature as we have some automatically generated class which break few rules and we don't won't modify n rules configs to skip it.
In our scenario actually something as simple as ignoring all rules for listed packages would do the job, but I guess we would like to be more flexible than that.

@cortinico
Copy link
Member

Any plans on implementing this in a near future? Maybe I can somehow help?

Not really. We had some attempts in the past, but none of them landed.

@BraisGabin
Copy link
Member

I think that the last comments about this are here: #3651 (comment) so 2.0 have this feature in its core.

If we want to implement this before 2.0, I think that the key should be a ReportingExtension. It have access to the configuration and the findings. Baseline uses the same hook to work.

It would not be perfect because it would be nice to avoid the run of that rule in those files but maybe it could help.

You can even give it a try by your own: https://detekt.github.io/detekt/extensions.html and if you find a general solution we could merge it.

@github-actions
Copy link

This issue is stale because it has been open 90 days with no activity. Please comment or this will be closed in 7 days.

@github-actions github-actions bot added the stale label Nov 10, 2021
@BraisGabin
Copy link
Member

#3251 (comment) should do the trick. Maybe we could add this to the documentation. But I think that we can close this for now.

Roadmap automation moved this from Backlog to Done Nov 10, 2021
@BraisGabin BraisGabin removed this from Done in Roadmap Nov 10, 2021
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Projects
None yet
Development

Successfully merging a pull request may close this issue.

5 participants