Nested class, interface or enum shouldn't be publicly visible #446
Conversation
|
|
||
| private fun isInternal(klass: KtClass): Boolean { | ||
| return klass.visibilityModifierType() == KtTokens.INTERNAL_KEYWORD | ||
| } |
There was a problem hiding this comment.
Move this to Junk.kt right under the following function.
fun KtModifierListOwner.isPublic()| private fun isPrivate(klass: KtClass): Boolean { | ||
| return klass.visibilityModifierType() == KtTokens.PRIVATE_KEYWORD | ||
| } | ||
|
|
| report(CodeSmell(issue, Entity.from(klass))) | ||
| } | ||
|
|
||
| private class VisibilityVisitor(val rule: NestedClassesVisibility) : DetektVisitor() { |
There was a problem hiding this comment.
You can also use an inner class here.
|
@schalkms hello, can you review again? |
| || this.hasModifier(KtTokens.INTERNAL_KEYWORD)) | ||
| } | ||
|
|
||
| fun KtModifierListOwner.isInternal(): Boolean { |
There was a problem hiding this comment.
let's do this?
fun KtModifierListOwner.isInternal() =
hasModifier(KtTokens.INTERNAL_KEYWORD)There was a problem hiding this comment.
@vanniktech Thx! done
|
|
||
| interface Test {} // report | ||
|
|
||
| private interface PrivateTest {} // valid |
There was a problem hiding this comment.
Please add some cases for nested classes (nest-level > 2)
There was a problem hiding this comment.
done + fix for enums
| } | ||
| } | ||
|
|
||
| private fun handleClass(klass: KtClass) { |
There was a problem hiding this comment.
This method could be inlined.
| } | ||
|
|
||
| private fun handleClass(klass: KtClass) { | ||
| report(CodeSmell(issue, Entity.from(klass))) |
There was a problem hiding this comment.
Should we report the issue with a concrete description? Something along the lines of: `issue.copy(description = "Nested types are often used for implementing private functionality. However the visiblilty of ${klass.name} makes it visible externally.")
That mentions the class name and might make the description of the user a bit clearer to the user.
|
Ok cool, thanks for the changes and contribution. I will wait for @schalkms to approve this now too and merge it :). |
Hello, Implementation for #444
Hacktoberfest!