Protected in final inspection: protected modifier is effectively private in final classes #KT-6674 Fixed
This commit is contained in:
committed by
Mikhail Glukhikh
parent
20bac6178c
commit
a541aaafd8
@@ -174,7 +174,8 @@ fun KtBlockExpression.contentRange(): PsiChildRange {
|
|||||||
// ----------- Inheritance -----------------------------------------------------------------------------------------------------------------
|
// ----------- Inheritance -----------------------------------------------------------------------------------------------------------------
|
||||||
|
|
||||||
fun KtClass.isInheritable(): Boolean {
|
fun KtClass.isInheritable(): Boolean {
|
||||||
return isInterface() || hasModifier(KtTokens.OPEN_KEYWORD) || hasModifier(KtTokens.ABSTRACT_KEYWORD)
|
return isInterface() || hasModifier(KtTokens.OPEN_KEYWORD) ||
|
||||||
|
hasModifier(KtTokens.ABSTRACT_KEYWORD) || hasModifier(KtTokens.SEALED_KEYWORD)
|
||||||
}
|
}
|
||||||
|
|
||||||
fun KtDeclaration.isOverridable(): Boolean {
|
fun KtDeclaration.isOverridable(): Boolean {
|
||||||
|
|||||||
@@ -0,0 +1,6 @@
|
|||||||
|
<html>
|
||||||
|
<body>
|
||||||
|
This inspection reports <code>protected</code> visibility used inside of a non-inheritable class.
|
||||||
|
<code>protected</code> members is accessible only in the class itself in such a case, so they are effectively <code>private</code>.
|
||||||
|
</body>
|
||||||
|
</html>
|
||||||
@@ -1425,6 +1425,13 @@
|
|||||||
level="WEAK WARNING"
|
level="WEAK WARNING"
|
||||||
/>
|
/>
|
||||||
|
|
||||||
|
<localInspection implementationClass="org.jetbrains.kotlin.idea.inspections.ProtectedInFinalInspection"
|
||||||
|
displayName="'protected' visibility is effectively 'private' in a final class"
|
||||||
|
groupName="Kotlin"
|
||||||
|
enabledByDefault="true"
|
||||||
|
level="WEAK WARNING"
|
||||||
|
/>
|
||||||
|
|
||||||
<referenceImporter implementation="org.jetbrains.kotlin.idea.quickfix.KotlinReferenceImporter"/>
|
<referenceImporter implementation="org.jetbrains.kotlin.idea.quickfix.KotlinReferenceImporter"/>
|
||||||
|
|
||||||
<fileType.fileViewProviderFactory filetype="KJSM" implementationClass="com.intellij.psi.ClassFileViewProviderFactory"/>
|
<fileType.fileViewProviderFactory filetype="KJSM" implementationClass="com.intellij.psi.ClassFileViewProviderFactory"/>
|
||||||
|
|||||||
@@ -0,0 +1,77 @@
|
|||||||
|
/*
|
||||||
|
* Copyright 2010-2016 JetBrains s.r.o.
|
||||||
|
*
|
||||||
|
* Licensed under the Apache License, Version 2.0 (the "License");
|
||||||
|
* you may not use this file except in compliance with the License.
|
||||||
|
* You may obtain a copy of the License at
|
||||||
|
*
|
||||||
|
* http://www.apache.org/licenses/LICENSE-2.0
|
||||||
|
*
|
||||||
|
* Unless required by applicable law or agreed to in writing, software
|
||||||
|
* distributed under the License is distributed on an "AS IS" BASIS,
|
||||||
|
* WITHOUT WARRANTIES OR CONDITIONS OF ANY KIND, either express or implied.
|
||||||
|
* See the License for the specific language governing permissions and
|
||||||
|
* limitations under the License.
|
||||||
|
*/
|
||||||
|
|
||||||
|
package org.jetbrains.kotlin.idea.inspections
|
||||||
|
|
||||||
|
import com.intellij.codeInspection.*
|
||||||
|
import com.intellij.openapi.project.Project
|
||||||
|
import com.intellij.psi.PsiElementVisitor
|
||||||
|
import org.jetbrains.kotlin.lexer.KtTokens
|
||||||
|
import org.jetbrains.kotlin.psi.KtClass
|
||||||
|
import org.jetbrains.kotlin.psi.KtDeclaration
|
||||||
|
import org.jetbrains.kotlin.psi.KtModifierListOwner
|
||||||
|
import org.jetbrains.kotlin.psi.KtVisitorVoid
|
||||||
|
import org.jetbrains.kotlin.psi.addRemoveModifier.addModifier
|
||||||
|
import org.jetbrains.kotlin.psi.psiUtil.getParentOfType
|
||||||
|
import org.jetbrains.kotlin.psi.psiUtil.isInheritable
|
||||||
|
import org.jetbrains.kotlin.psi.psiUtil.visibilityModifier
|
||||||
|
|
||||||
|
class ProtectedInFinalInspection : AbstractKotlinInspection() {
|
||||||
|
override fun buildVisitor(holder: ProblemsHolder, isOnTheFly: Boolean, session: LocalInspectionToolSession): PsiElementVisitor {
|
||||||
|
return object : KtVisitorVoid() {
|
||||||
|
override fun visitDeclaration(declaration: KtDeclaration) {
|
||||||
|
val visibilityModifier = declaration.visibilityModifier() ?: return
|
||||||
|
val modifierType = visibilityModifier.node?.elementType
|
||||||
|
if (modifierType == KtTokens.PROTECTED_KEYWORD) {
|
||||||
|
val parentClass = declaration.getParentOfType<KtClass>(true) ?: return
|
||||||
|
if (!parentClass.isInheritable() && !parentClass.isEnum()) {
|
||||||
|
holder.registerProblem(visibilityModifier,
|
||||||
|
"'protected' visibility is effectively 'private' in a final class",
|
||||||
|
ProblemHighlightType.WEAK_WARNING,
|
||||||
|
MakePrivateFix(),
|
||||||
|
MakeOpenFix()
|
||||||
|
)
|
||||||
|
}
|
||||||
|
}
|
||||||
|
}
|
||||||
|
}
|
||||||
|
}
|
||||||
|
|
||||||
|
class MakePrivateFix : LocalQuickFix {
|
||||||
|
override fun getName(): String = "Make private"
|
||||||
|
|
||||||
|
override fun getFamilyName(): String = name
|
||||||
|
|
||||||
|
override fun applyFix(project: Project, descriptor: ProblemDescriptor) {
|
||||||
|
val modifierListOwner = descriptor.psiElement.getParentOfType<KtModifierListOwner>(true)
|
||||||
|
?: throw IllegalStateException("Can't find modifier list owner for modifier")
|
||||||
|
addModifier(modifierListOwner, KtTokens.PRIVATE_KEYWORD)
|
||||||
|
}
|
||||||
|
}
|
||||||
|
|
||||||
|
class MakeOpenFix : LocalQuickFix {
|
||||||
|
override fun getName(): String = "Make class open"
|
||||||
|
|
||||||
|
override fun getFamilyName(): String = name
|
||||||
|
|
||||||
|
override fun applyFix(project: Project, descriptor: ProblemDescriptor) {
|
||||||
|
val modifierListOwner = descriptor.psiElement.getParentOfType<KtModifierListOwner>(true)
|
||||||
|
?: throw IllegalStateException("Can't find modifier list owner for modifier")
|
||||||
|
val parentClass = modifierListOwner.getParentOfType<KtClass>(true) ?: return
|
||||||
|
addModifier(parentClass, KtTokens.OPEN_KEYWORD)
|
||||||
|
}
|
||||||
|
}
|
||||||
|
}
|
||||||
@@ -0,0 +1,26 @@
|
|||||||
|
<problems>
|
||||||
|
<problem>
|
||||||
|
<file>protectedInFinal.kt</file>
|
||||||
|
<line>2</line>
|
||||||
|
<module>light_idea_test_case</module>
|
||||||
|
<entry_point TYPE="file" FQNAME="temp:///src/protectedInFinal.kt" />
|
||||||
|
<problem_class severity="WARNING" attribute_key="WARNING_ATTRIBUTES">'protected' visibility is effectively 'private' in a final class</problem_class>
|
||||||
|
<description>'protected' visibility is effectively 'private' in a final class</description>
|
||||||
|
</problem>
|
||||||
|
<problem>
|
||||||
|
<file>protectedInFinal.kt</file>
|
||||||
|
<line>4</line>
|
||||||
|
<module>light_idea_test_case</module>
|
||||||
|
<entry_point TYPE="file" FQNAME="temp:///src/protectedInFinal.kt" />
|
||||||
|
<problem_class severity="WARNING" attribute_key="WARNING_ATTRIBUTES">'protected' visibility is effectively 'private' in a final class</problem_class>
|
||||||
|
<description>'protected' visibility is effectively 'private' in a final class</description>
|
||||||
|
</problem>
|
||||||
|
<problem>
|
||||||
|
<file>protectedInFinal.kt</file>
|
||||||
|
<line>6</line>
|
||||||
|
<module>light_idea_test_case</module>
|
||||||
|
<entry_point TYPE="file" FQNAME="temp:///src/protectedInFinal.kt" />
|
||||||
|
<problem_class severity="WARNING" attribute_key="WARNING_ATTRIBUTES">'protected' visibility is effectively 'private' in a final class</problem_class>
|
||||||
|
<description>'protected' visibility is effectively 'private' in a final class</description>
|
||||||
|
</problem>
|
||||||
|
</problems>
|
||||||
@@ -0,0 +1 @@
|
|||||||
|
// INSPECTION_CLASS: org.jetbrains.kotlin.idea.inspections.ProtectedInFinalInspection
|
||||||
@@ -0,0 +1,37 @@
|
|||||||
|
class F {
|
||||||
|
protected val foo: Int = 0
|
||||||
|
|
||||||
|
protected fun bar() {}
|
||||||
|
|
||||||
|
protected class Nested
|
||||||
|
}
|
||||||
|
|
||||||
|
class G {
|
||||||
|
interface H {
|
||||||
|
protected val foo: Int = 0
|
||||||
|
|
||||||
|
protected fun bar() {}
|
||||||
|
|
||||||
|
protected class Nested
|
||||||
|
}
|
||||||
|
}
|
||||||
|
|
||||||
|
sealed class S {
|
||||||
|
protected val foo: Int = 0
|
||||||
|
|
||||||
|
protected fun bar() {}
|
||||||
|
|
||||||
|
protected class Nested : S()
|
||||||
|
|
||||||
|
protected object Obj : S()
|
||||||
|
}
|
||||||
|
|
||||||
|
enum class E {
|
||||||
|
SINGLE {
|
||||||
|
override val x = foo()
|
||||||
|
};
|
||||||
|
|
||||||
|
abstract val x: Int
|
||||||
|
|
||||||
|
protected fun foo() = 42
|
||||||
|
}
|
||||||
@@ -0,0 +1 @@
|
|||||||
|
org.jetbrains.kotlin.idea.inspections.ProtectedInFinalInspection
|
||||||
@@ -0,0 +1,4 @@
|
|||||||
|
// "Make private" "true"
|
||||||
|
class C {
|
||||||
|
<caret>protected fun foo() {}
|
||||||
|
}
|
||||||
@@ -0,0 +1,4 @@
|
|||||||
|
// "Make private" "true"
|
||||||
|
class C {
|
||||||
|
private fun foo() {}
|
||||||
|
}
|
||||||
@@ -0,0 +1,4 @@
|
|||||||
|
// "Make class open" "true"
|
||||||
|
final class C {
|
||||||
|
<caret>protected fun foo() {}
|
||||||
|
}
|
||||||
@@ -0,0 +1,4 @@
|
|||||||
|
// "Make class open" "true"
|
||||||
|
open class C {
|
||||||
|
protected fun foo() {}
|
||||||
|
}
|
||||||
@@ -142,6 +142,12 @@ public class InspectionTestGenerated extends AbstractInspectionTest {
|
|||||||
doTest(fileName);
|
doTest(fileName);
|
||||||
}
|
}
|
||||||
|
|
||||||
|
@TestMetadata("protectedInFinal/inspectionData/inspections.test")
|
||||||
|
public void testProtectedInFinal_inspectionData_Inspections_test() throws Exception {
|
||||||
|
String fileName = KotlinTestUtils.navigationMetadata("idea/testData/inspections/protectedInFinal/inspectionData/inspections.test");
|
||||||
|
doTest(fileName);
|
||||||
|
}
|
||||||
|
|
||||||
@TestMetadata("redundantSamConstructor/inspectionData/inspections.test")
|
@TestMetadata("redundantSamConstructor/inspectionData/inspections.test")
|
||||||
public void testRedundantSamConstructor_inspectionData_Inspections_test() throws Exception {
|
public void testRedundantSamConstructor_inspectionData_Inspections_test() throws Exception {
|
||||||
String fileName = KotlinTestUtils.navigationMetadata("idea/testData/inspections/redundantSamConstructor/inspectionData/inspections.test");
|
String fileName = KotlinTestUtils.navigationMetadata("idea/testData/inspections/redundantSamConstructor/inspectionData/inspections.test");
|
||||||
|
|||||||
@@ -6053,6 +6053,27 @@ public class QuickFixTestGenerated extends AbstractQuickFixTest {
|
|||||||
}
|
}
|
||||||
}
|
}
|
||||||
|
|
||||||
|
@TestMetadata("idea/testData/quickfix/protectedInFinal")
|
||||||
|
@TestDataPath("$PROJECT_ROOT")
|
||||||
|
@RunWith(JUnit3RunnerWithInners.class)
|
||||||
|
public static class ProtectedInFinal extends AbstractQuickFixTest {
|
||||||
|
public void testAllFilesPresentInProtectedInFinal() throws Exception {
|
||||||
|
KotlinTestUtils.assertAllTestsPresentByMetadata(this.getClass(), new File("idea/testData/quickfix/protectedInFinal"), Pattern.compile("^([\\w\\-_]+)\\.kt$"), true);
|
||||||
|
}
|
||||||
|
|
||||||
|
@TestMetadata("protected.kt")
|
||||||
|
public void testProtected() throws Exception {
|
||||||
|
String fileName = KotlinTestUtils.navigationMetadata("idea/testData/quickfix/protectedInFinal/protected.kt");
|
||||||
|
doTest(fileName);
|
||||||
|
}
|
||||||
|
|
||||||
|
@TestMetadata("protected2.kt")
|
||||||
|
public void testProtected2() throws Exception {
|
||||||
|
String fileName = KotlinTestUtils.navigationMetadata("idea/testData/quickfix/protectedInFinal/protected2.kt");
|
||||||
|
doTest(fileName);
|
||||||
|
}
|
||||||
|
}
|
||||||
|
|
||||||
@TestMetadata("idea/testData/quickfix/redundantVisibilityModifier")
|
@TestMetadata("idea/testData/quickfix/redundantVisibilityModifier")
|
||||||
@TestDataPath("$PROJECT_ROOT")
|
@TestDataPath("$PROJECT_ROOT")
|
||||||
@RunWith(JUnit3RunnerWithInners.class)
|
@RunWith(JUnit3RunnerWithInners.class)
|
||||||
|
|||||||
Reference in New Issue
Block a user