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 --auto-correct cmdline option to gradle tasks #4202

Merged
merged 3 commits into from
Nov 17, 2021

Conversation

evant
Copy link
Contributor

@evant evant commented Oct 25, 2021

This allows you to run ./gradlew detekt --auto-correct

I often find myself wanting to make this configurable particularly so that I can run it locally and not on ci. Also because auto-correct is a potentially destructive operation it's nice to have to be explicit to enable it on the cmdline instead of hard-coding it in the detekt task configuration. Having a standard option for this also makes it easier to autofix your code when working on various third-party projects using detekt.

This allows you to run ./gradlew detekt --auto-correct
@codecov
Copy link

codecov bot commented Oct 26, 2021

Codecov Report

Merging #4202 (3784f4d) into main (b36fa7d) will not change coverage.
The diff coverage is n/a.

Impacted file tree graph

@@            Coverage Diff            @@
##               main    #4202   +/-   ##
=========================================
  Coverage     84.22%   84.22%           
  Complexity     3258     3258           
=========================================
  Files           472      472           
  Lines         10309    10309           
  Branches       1820     1820           
=========================================
  Hits           8683     8683           
  Misses          666      666           
  Partials        960      960           

Continue to review full report at Codecov.

Legend - Click here to learn more
Δ = absolute <relative> (impact), ø = not affected, ? = missing data
Powered by Codecov. Last update b36fa7d...3784f4d. Read the comment docs.

@3flex
Copy link
Member

3flex commented Oct 27, 2021

This is a nice idea - but the same can be achieved by something like gradlew detekt -PdetektAutoCorrect=true

And then in the Gradle config:

val detektAutoCorrect: String? by project

tasks.withType<Detekt>().configureEach {
    autoCorrect = detektAutoCorrect.toBoolean()
}

It's a few more chars on the CLI but gets the same result.

I don't feel strongly about this though, but just thinking that if we add a CLI option for auto correct, then does it make sense to add it for other parameters. The Gradle plugin is fairly complicated for what it does, so it would be good to keep it simple where we can.

@evant
Copy link
Contributor Author

evant commented Oct 27, 2021

Yeah I've basically had to add that on every project I've worked on using detekt, and if a project I'm contributing to doesn't have it I have to tweak the build file to run it. Was hoping to save some time and add discoverability by having it built in. Don't know of another param in the same situation, but I feel like that could always be a follow-up if some is desired.

@evant
Copy link
Contributor Author

evant commented Oct 27, 2021

I think the thing sets auto-correct apart from other args is that it can modify your source so it really should be obvious when you are running with it imo. I'd argue that the gradle dsl shouldn't even have it, but I can live with keeping that param there.

@BraisGabin
Copy link
Member

I think that we declined already some PRs similar to this one but

and if a project I'm contributing to doesn't have it I have to tweak the build file to run it.

this seems a very legit use case. Not in all the projects you have the control to decide how detekt is setup so this is probably a legit use case. And, because we had this request multiple times, it seems like a feature that "a lot/some" users want.

I'd argue that the gradle dsl shouldn't even have it

I agree with this. I don't think it should be in the dsl. I don't know if the best way would be a flag or a new task detektFormat similar to what https://github.com/JLLeitschuh/ktlint-gradle does.

We know that our current gradle plugin api is a mess and we need to reframe it completely for 2.0 (more about this here: #4190 (comment)) but I think that until then this is a very clean way to give support to this use case.

Copy link
Member

@chao2zhang chao2zhang left a comment

Choose a reason for hiding this comment

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

Do we want to update docs?

@cortinico cortinico added this to the 1.19.0 milestone Oct 31, 2021
@cortinico cortinico added gradle-plugin notable changes Marker for notable changes in the changelog labels Oct 31, 2021
@BraisGabin
Copy link
Member

@evant can you update the documentation? I think that's the only thing missing for us to merge this.

@chao2zhang chao2zhang merged commit ec1f8b9 into detekt:main Nov 17, 2021
@gmazzo
Copy link

gmazzo commented Dec 5, 2022

Glad to see a change of ❤️ on this. I should provide better arguments time.

Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
gradle-plugin notable changes Marker for notable changes in the changelog
Projects
None yet
Development

Successfully merging this pull request may close these issues.

7 participants