[UndocumentedPublicProperty] Allow inline comments for properties in primary constructor as documentation #3722
Conversation
Codecov Report
@@ Coverage Diff @@
## main #3722 +/- ##
============================================
+ Coverage 78.04% 78.70% +0.65%
- Complexity 2883 2892 +9
============================================
Files 473 473
Lines 9302 9310 +8
Branches 1767 1705 -62
============================================
+ Hits 7260 7327 +67
- Misses 1078 1080 +2
+ Partials 964 903 -61 Continue to review full report at Codecov.
|
| @@ -22,6 +22,9 @@ import org.jetbrains.kotlin.psi.psiUtil.isPublic | |||
| * This also includes public properties defined in a primary constructor. | |||
| * If the codebase should have documentation on all public properties enable this rule to enforce this. | |||
| * Overridden properties are excluded by this rule. | |||
| * | |||
| * @configuration allowInlineConstructorPropertyComments - allow properties defined in a primary constructor to have | |||
| * inline documentation (default: `false`). | |||
There was a problem hiding this comment.
I'm not sure about this configuration. This rule checks if a public property is undocumented. This configuration is about if we like the kdoc there or not. I see the point of it but it's out of the scope of this rule. The property is correctly documented so this rule should not complain about where it was documented.
It we want to discorage that usage we could create another rule but I think that we should keep the focust of this rule.
There was a problem hiding this comment.
I see your point here to follow the name of the rule precisely, but I feel that this configuration is necessary to keep current behavior of Detekt.
There already maybe projects that rely on current behavior and change of this rule would be unexpected by them.
Also maybe the decision about what the correct way of documentation should be on users?
There was a problem hiding this comment.
I don't think that we should allow this to be configured. It is completely valid to document the property inline.
There was a problem hiding this comment.
Okay, let me reconfirm to be sure.
@BraisGabin agreed with the fact that inline documentation is valid and this PR is OK, just need to remove configuration option?
I will update PR after your confirmation
| !it.isPublicNotOverridden() || !it.hasValOrVar() -> false | ||
| isInlineConstructorPropertyCommentsAllowed && it.docComment != null -> false | ||
| else -> it.isUndocumented(comment) | ||
| } |
There was a problem hiding this comment.
TBH this is really hard to understand. Maybe it would be better to do something along the lines of
constructor.valueParameters
.filter { it.isPublicNotOverridden() }
.filter { it.hasValOrVar() }
.filter { it.isUndocumented(comment) }
.filter { it.docComment == null }
.forEach { report(it) }
- update tests
|
Thanks for the contribution 👍 |
cortinico
changed the title
…ntation (detekt#3722) * Allow inline comments for properties in primary constructor Fixes detekt#3677 * Revert blank line removal in UndocumentedPublicProperty * - remove configuration - update tests * - remove unused import
Fixes #3677
Allow inline comments for properties in primary constructor as a property documentation.
Changes:
Introduce new configuration parameter for<-- decided that configuration is not neededUndocumentedPublicPropertythat defaults to false to keep current behavior by default