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 documenting public fun name when same private variable is present #6165
Conversation
Extend the previous logic by traversing to `visitNamedDeclaration`
Codecov Report
@@ Coverage Diff @@
## main #6165 +/- ##
============================================
- Coverage 84.45% 84.44% -0.02%
+ Complexity 3994 3992 -2
============================================
Files 568 568
Lines 13428 13428
Branches 2372 2372
============================================
- Hits 11341 11339 -2
Misses 935 935
- Partials 1152 1154 +2
|
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.
See my comment.
Code adaption looks good.
/** | ||
* [peek] - public fun | ||
* [Inner.innerPeek] - public fun | ||
* [Object.objectPeek] - public fun | ||
*/ | ||
class Test { | ||
private var peek: Int = 0 | ||
private var innerPeek: Int = 0 | ||
private var objectPeek: Int = 0 | ||
private var companionPeek: Int = 0 | ||
fun peek() = System.currentTimeMillis() | ||
inner class Inner { | ||
fun innerPeek() = System.currentTimeMillis() | ||
} | ||
object Object { | ||
fun objectPeek() = System.currentTimeMillis() | ||
} | ||
} |
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.
I have the feeling this test snippet can be simplified to be easier understood for future readers.
What do you think?
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.
Hi, @schalkms I tried to have multiple similar TCs in a single code as that was possible here. In what respect do you want this to be changed? Will adding comments in the code make it clearer? Or do we want me to split the TC in multiple TCs under one nested class?
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.
My intention was to keep the code shorter if possible.
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.
I can replace System.currentTimeMillis()
with 0
will that make it shorter 😅. O/W code is already short and concise (it represents normal function, inner class
function, and object
function)
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.
Yep, that helps and removes unnecessary noise from test code. I had to read the test code twice to get the intention of it.
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.
Updated
Thank you!! |
…nt (detekt#6165) * Allow documenting public fun name when same private variable is present Extend the previous logic by traversing to `visitNamedDeclaration` * Update the TC to make it more shorter
…nt (detekt#6165) * Allow documenting public fun name when same private variable is present Extend the previous logic by traversing to `visitNamedDeclaration` * Update the TC to make it more shorter
Extend the previous logic by traversing to
visitNamedDeclaration
Fixes: #6162