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

Bump KtLint to 0.44.0 and add UnnecessaryParenthesesBeforeTrailingLamda rule #4630

Merged
merged 8 commits into from
Mar 19, 2022

Conversation

kvn-stgl
Copy link
Contributor

@kvn-stgl kvn-stgl commented Mar 13, 2022

See https://github.com/pinterest/ktlint/releases/tag/0.44.0 for the full release note. I've also wrapped the new experimental UnnecessaryParenthesesBeforeTrailingLamda rule.

I also tried to implement the new VisitorModifier instead of the deprecated Rule.Modifier interfaces.

This should also fix the issue #4604

Sadly I get a lot of Unexpected indent of multiline string closing quotes [Indentation]-errors for the Unit-Tests. Maybe someone has some ideas how I can fix it.

@kvn-stgl kvn-stgl marked this pull request as ready for review March 13, 2022 15:20
@codecov
Copy link

codecov bot commented Mar 13, 2022

Codecov Report

Merging #4630 (6ca5949) into main (8bb4c85) will increase coverage by 0.00%.
The diff coverage is 100.00%.

@@            Coverage Diff            @@
##               main    #4630   +/-   ##
=========================================
  Coverage     84.52%   84.53%           
- Complexity     3358     3364    +6     
=========================================
  Files           481      482    +1     
  Lines         11193    11200    +7     
  Branches       2042     2043    +1     
=========================================
+ Hits           9461     9468    +7     
  Misses          698      698           
  Partials       1034     1034           
Impacted Files Coverage Δ
...b/arturbosch/detekt/rules/style/ForbiddenImport.kt 95.45% <ø> (ø)
...ekt/rules/style/RedundantVisibilityModifierRule.kt 98.24% <ø> (ø)
...lab/arturbosch/detekt/formatting/FormattingRule.kt 98.07% <100.00%> (+0.16%) ⬆️
...ab/arturbosch/detekt/formatting/KtLintMultiRule.kt 95.94% <100.00%> (+0.05%) ⬆️
...etekt/formatting/wrappers/ParameterListWrapping.kt 100.00% <100.00%> (ø)
...pers/UnnecessaryParenthesesBeforeTrailingLambda.kt 100.00% <100.00%> (ø)
...n/kotlin/io/github/detekt/report/html/HtmlUtils.kt 98.33% <100.00%> (ø)

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 8bb4c85...6ca5949. Read the comment docs.

Copy link
Member

@cortinico cortinico left a comment

Choose a reason for hiding this comment

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

Thanks for doing this @kvn-stgl, this is great!

I actually also had a branch with some of this work. I believe you should be able to integrate some of it in your PR (I would like to merge yours instead of mine as this looks like an amazing first-time contribution):

2a9b602

Comment on lines -24 to -25
@Configuration("indentation size")
private val indentSize by config(4)
Copy link
Member

Choose a reason for hiding this comment

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

This should be deprecated instead

@cortinico cortinico added this to the 1.20.0 milestone Mar 13, 2022
@cortinico cortinico added formatting notable changes Marker for notable changes in the changelog labels Mar 13, 2022
@kvn-stgl
Copy link
Contributor Author

Thanks @cortinico ! So you simply fixed the issues (or let the formatter do the work) .. Well, okay 😄

Can you just tell me a simple question about your changes: You wrote the following snippet: 2a9b602#diff-9cf77f9ffcbf036bf5044d1ef99728b56aad97854246f3473f69d54b4b929380R82-R86 . Is it really necessary to check the KtLint.EDITOR_CONFIG_USER_DATA_KEY? I'm not sure about it, it worked for already without it. But of course I will implement it if it is necessary.

@cortinico
Copy link
Member

cortinico commented Mar 16, 2022

Is it really necessary to check the KtLint.EDITOR_CONFIG_USER_DATA_KEY? I'm not sure about it, it worked for already without it.

I think it is. I tried to publish a new version of the Gradle plugin locally and KtLint was crashing while executing the Indentation rule for that. That's why I had to do this change.

That's the same reason why I had to touch the detekt-formatting gradle file to include the extra dependencies KtLint 0.44.0 carries over.

@kvn-stgl
Copy link
Contributor Author

That's the same reason why I had to touch the detekt-formatting gradle file to include the extra dependencies KtLint 0.44.0 carries over.

I was already wondering why you implemented the microutils logging framework. But now I see (and my code build locally). Thanks!

@@ -22,7 +23,10 @@ tasks.build { finalizedBy(":detekt-generator:generateDocumentation") }

val depsToPackage = setOf(
"org.ec4j.core",
"com.pinterest.ktlint"
"com.pinterest.ktlint",
"com.pinterest.ktlint",
Copy link
Member

Choose a reason for hiding this comment

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

Duplicated?

Copy link
Contributor Author

Choose a reason for hiding this comment

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

Fuuuu

@BraisGabin BraisGabin merged commit 14c0f11 into detekt:main Mar 19, 2022
@BraisGabin
Copy link
Member

😂 We just merged 0.44 but 0.45 is out

@BraisGabin
Copy link
Member

BraisGabin commented Mar 19, 2022

Oh! and thank you very much for the PR @kvn-stgl :)

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

Successfully merging this pull request may close these issues.

3 participants