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 wrapper for KtLint rules #6037
Conversation
Codecov Report
@@ Coverage Diff @@
## main #6037 +/- ##
============================================
+ Coverage 84.84% 84.86% +0.01%
- Complexity 3944 3953 +9
============================================
Files 561 564 +3
Lines 13251 13266 +15
Branches 2313 2313
============================================
+ Hits 11243 11258 +15
Misses 868 868
Partials 1140 1140
Help us with your feedback. Take ten seconds to tell us how you rate us. Have a feature suggestion? Share it here. |
@@ -309,8 +309,7 @@ naming: | |||
active: false | |||
allowedPattern: '^(is|has|are)' | |||
ClassNaming: | |||
active: true | |||
classPattern: '[A-Z][a-zA-Z0-9]*' | |||
active: false |
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
Why this changed?
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
Yes, that was my question. Ideally, it should not be changed but it got changed automatically after running generateDocumentation
. This could be a bug in generateDocumentation
as I have also added the ClassNaming
rule from KtLint and there was one preexisting ClassNaming
rule as well
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
@cortinico here my only concern which is not related to this PR is how we handle the same name rules, should generateDocumentation
fail in cases like that? Or should we support the same name rules(if yes then when? Like should we allow the same name rules only when those are in different rule sections?) If we allow same name rules then the above change should also be addressed as generateDocumentation
wrongly updates the wrong rules config
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
In other words, in detekt
what is a unique identifier for a rule? Is this just rule name or combination of rule provider id and name?
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
how we handle the same name rules, should generateDocumentation fail in cases like that?
I think that's a bug and we should investigate here. Rules with same ID coudl occur, especially if coming from external packages.
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
Right now, I have reverted the unintentional change. I will create a separate issue for the bug
I believe these rules are all enabled by default in ktlint so to keep with convention we would enable them by default here too? Or do we make an exception and keep them disabled when detekt has an equivalent rule |
We should not make two different rules enabled. That can potentially cause problems due to conflicting analysis |
This avoids conflict with existing rule
Fixes #5627