Const naming #3150
Const naming #3150
Conversation
Codecov Report
@@ Coverage Diff @@
## master #3150 +/- ##
=========================================
Coverage 80.04% 80.04%
Complexity 2653 2653
=========================================
Files 443 443
Lines 8090 8090
Branches 1540 1540
=========================================
Hits 6476 6476
Misses 790 790
Partials 824 824
Continue to review full report at Codecov.
|
| val code = """ | ||
| object O { | ||
| private val __NAME = "Artur" | ||
| private val MyNAME = "Artur" | ||
| private val n_a_m_e = "Artur" | ||
| private val _MY_NAME = "Artur" |
There was a problem hiding this comment.
Hm, as long as IntelliJ does not change it's own regex I would not like to change this.
All this cases are okay in IntelliJ when I paste them and would cause a lot of issues for our users
Stale issues without comments get locked by our bot integration.
I would really like to have IntelliJ change its default before doing such a change which will lead to new issues for our users.
|
|
Right now our regex allow names like this: I think that our users expect that if someone in their teams adds a property with a name like the one in my example detekt will complain. I don't think that flaging more code to our users is bad thing. They relay on detekt to find issues in their code. I understand that it will be better if IntelliJ implements it too and for that reason I waited nearly a year to reopen this issue. But if they don't fix it I don't see why we should keep this issue in our rule too. But, this change should be merged for a minor release and not a patch. |
|
Could we take a decision about this update @cortinico @schalkms what do you think about this? I think that it could be merged for the 1.15 relase if you agree. Or close the topic if disagree. |
|
In my opinion detekt should be compliant to IntelliJs rules. |
Agree. I've 👍 -d your YouTrack ticket. Other than that, I believe we should try to align as much as possible to commonly used RegExes (like the one used in IntelliJ) unless we have a strong argument/case to change one. So far it seems this is a minor concern, and users that are interested in altering the inspection behavior to adhere to the official docs, can still update the Regex. I believe if we end up waiting for JetBrains, we could still leave a note in the |
I added those comments but they look kind of strange there so I think that we should close this and if we get a notificacion that the issue was fixed we will update it. Maybe I try again to fix it in the intellij code but I have no idea how to run the test there. |
This is just a reopen for #2017 (I don't know why I can't reopen it). Extracted from there:
Why am I reopening this?
After @arturbosch closed that PR I opened this issue to IntelliJ: https://youtrack.jetbrains.com/issue/KT-34508 It was accepted as a valid issue but after one year it's not fixed.
I think that we should rethink about merging this.
We have another related issue:private val _HELLOshould not be valid and we are allowing it. I'm going to create another PR fixing it.