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

Allow documenting public fun name when same private variable is present #6165

Merged
merged 2 commits into from Jun 5, 2023

Conversation

atulgpt
Copy link
Contributor

@atulgpt atulgpt commented May 29, 2023

Extend the previous logic by traversing to visitNamedDeclaration

Fixes: #6162

Extend the previous logic by traversing to `visitNamedDeclaration`
@codecov
Copy link

codecov bot commented May 29, 2023

Codecov Report

Merging #6165 (8f6ade9) into main (9fb4353) will decrease coverage by 0.02%.
The diff coverage is 100.00%.

@@             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     
Impacted Files Coverage Δ
...s/documentation/KDocReferencesNonPublicProperty.kt 96.29% <100.00%> (-3.71%) ⬇️

Copy link
Member

@schalkms schalkms left a 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.

Comment on lines 203 to 220
/**
* [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()
}
}
Copy link
Member

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?

Copy link
Contributor Author

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?

Copy link
Member

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.

Copy link
Contributor Author

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)

Copy link
Member

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.

Copy link
Contributor Author

Choose a reason for hiding this comment

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

Updated

@schalkms schalkms added this to the 2.0.0 milestone Jun 1, 2023
@BraisGabin
Copy link
Member

Thank you!!

@BraisGabin BraisGabin merged commit 299c55f into detekt:main Jun 5, 2023
23 checks passed
@atulgpt atulgpt deleted the fixes/6162 branch June 5, 2023 19:44
@cortinico cortinico added the pick request Marker for PRs that should be ported to the 1.0 release branch label Jul 15, 2023
cortinico pushed a commit to cortinico/detekt that referenced this pull request Jul 15, 2023
…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
mgroth0 pushed a commit to mgroth0/detekt that referenced this pull request Feb 11, 2024
…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
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
pick request Marker for PRs that should be ported to the 1.0 release branch rules
Projects
None yet
Development

Successfully merging this pull request may close these issues.

false positive KDocReferencesNonPublicProperty when method and property with same name
5 participants