diff --git a/compiler/frontend/src/org/jetbrains/kotlin/psi/psiUtil/ktPsiUtil.kt b/compiler/frontend/src/org/jetbrains/kotlin/psi/psiUtil/ktPsiUtil.kt index f9612b0b3bf..886ac10ba02 100644 --- a/compiler/frontend/src/org/jetbrains/kotlin/psi/psiUtil/ktPsiUtil.kt +++ b/compiler/frontend/src/org/jetbrains/kotlin/psi/psiUtil/ktPsiUtil.kt @@ -174,7 +174,8 @@ fun KtBlockExpression.contentRange(): PsiChildRange { // ----------- Inheritance ----------------------------------------------------------------------------------------------------------------- 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 { diff --git a/idea/resources/inspectionDescriptions/ProtectedInFinal.html b/idea/resources/inspectionDescriptions/ProtectedInFinal.html new file mode 100644 index 00000000000..f5a45e0897b --- /dev/null +++ b/idea/resources/inspectionDescriptions/ProtectedInFinal.html @@ -0,0 +1,6 @@ + + +This inspection reports protected visibility used inside of a non-inheritable class. +protected members is accessible only in the class itself in such a case, so they are effectively private. + + diff --git a/idea/src/META-INF/plugin.xml b/idea/src/META-INF/plugin.xml index 5847ea3ef12..70027e19650 100644 --- a/idea/src/META-INF/plugin.xml +++ b/idea/src/META-INF/plugin.xml @@ -1425,6 +1425,13 @@ level="WEAK WARNING" /> + + diff --git a/idea/src/org/jetbrains/kotlin/idea/inspections/ProtectedInFinalInspection.kt b/idea/src/org/jetbrains/kotlin/idea/inspections/ProtectedInFinalInspection.kt new file mode 100644 index 00000000000..2d608780802 --- /dev/null +++ b/idea/src/org/jetbrains/kotlin/idea/inspections/ProtectedInFinalInspection.kt @@ -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(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(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(true) + ?: throw IllegalStateException("Can't find modifier list owner for modifier") + val parentClass = modifierListOwner.getParentOfType(true) ?: return + addModifier(parentClass, KtTokens.OPEN_KEYWORD) + } + } +} diff --git a/idea/testData/inspections/protectedInFinal/inspectionData/expected.xml b/idea/testData/inspections/protectedInFinal/inspectionData/expected.xml new file mode 100644 index 00000000000..34bce6a9bca --- /dev/null +++ b/idea/testData/inspections/protectedInFinal/inspectionData/expected.xml @@ -0,0 +1,26 @@ + + + protectedInFinal.kt + 2 + light_idea_test_case + + 'protected' visibility is effectively 'private' in a final class + 'protected' visibility is effectively 'private' in a final class + + + protectedInFinal.kt + 4 + light_idea_test_case + + 'protected' visibility is effectively 'private' in a final class + 'protected' visibility is effectively 'private' in a final class + + + protectedInFinal.kt + 6 + light_idea_test_case + + 'protected' visibility is effectively 'private' in a final class + 'protected' visibility is effectively 'private' in a final class + + diff --git a/idea/testData/inspections/protectedInFinal/inspectionData/inspections.test b/idea/testData/inspections/protectedInFinal/inspectionData/inspections.test new file mode 100644 index 00000000000..12ccca6f1db --- /dev/null +++ b/idea/testData/inspections/protectedInFinal/inspectionData/inspections.test @@ -0,0 +1 @@ +// INSPECTION_CLASS: org.jetbrains.kotlin.idea.inspections.ProtectedInFinalInspection \ No newline at end of file diff --git a/idea/testData/inspections/protectedInFinal/protectedInFinal.kt b/idea/testData/inspections/protectedInFinal/protectedInFinal.kt new file mode 100644 index 00000000000..4b4e5cfbc16 --- /dev/null +++ b/idea/testData/inspections/protectedInFinal/protectedInFinal.kt @@ -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 +} diff --git a/idea/testData/quickfix/protectedInFinal/.inspection b/idea/testData/quickfix/protectedInFinal/.inspection new file mode 100644 index 00000000000..9f2140889c5 --- /dev/null +++ b/idea/testData/quickfix/protectedInFinal/.inspection @@ -0,0 +1 @@ +org.jetbrains.kotlin.idea.inspections.ProtectedInFinalInspection \ No newline at end of file diff --git a/idea/testData/quickfix/protectedInFinal/protected.kt b/idea/testData/quickfix/protectedInFinal/protected.kt new file mode 100644 index 00000000000..e9693c1ff5d --- /dev/null +++ b/idea/testData/quickfix/protectedInFinal/protected.kt @@ -0,0 +1,4 @@ +// "Make private" "true" +class C { + protected fun foo() {} +} diff --git a/idea/testData/quickfix/protectedInFinal/protected.kt.after b/idea/testData/quickfix/protectedInFinal/protected.kt.after new file mode 100644 index 00000000000..dfa43c64ae5 --- /dev/null +++ b/idea/testData/quickfix/protectedInFinal/protected.kt.after @@ -0,0 +1,4 @@ +// "Make private" "true" +class C { + private fun foo() {} +} diff --git a/idea/testData/quickfix/protectedInFinal/protected2.kt b/idea/testData/quickfix/protectedInFinal/protected2.kt new file mode 100644 index 00000000000..c01f2fbe6ac --- /dev/null +++ b/idea/testData/quickfix/protectedInFinal/protected2.kt @@ -0,0 +1,4 @@ +// "Make class open" "true" +final class C { + protected fun foo() {} +} diff --git a/idea/testData/quickfix/protectedInFinal/protected2.kt.after b/idea/testData/quickfix/protectedInFinal/protected2.kt.after new file mode 100644 index 00000000000..048a843b9ab --- /dev/null +++ b/idea/testData/quickfix/protectedInFinal/protected2.kt.after @@ -0,0 +1,4 @@ +// "Make class open" "true" +open class C { + protected fun foo() {} +} diff --git a/idea/tests/org/jetbrains/kotlin/idea/codeInsight/InspectionTestGenerated.java b/idea/tests/org/jetbrains/kotlin/idea/codeInsight/InspectionTestGenerated.java index 260a0448935..51049ace3df 100644 --- a/idea/tests/org/jetbrains/kotlin/idea/codeInsight/InspectionTestGenerated.java +++ b/idea/tests/org/jetbrains/kotlin/idea/codeInsight/InspectionTestGenerated.java @@ -142,6 +142,12 @@ public class InspectionTestGenerated extends AbstractInspectionTest { 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") public void testRedundantSamConstructor_inspectionData_Inspections_test() throws Exception { String fileName = KotlinTestUtils.navigationMetadata("idea/testData/inspections/redundantSamConstructor/inspectionData/inspections.test"); diff --git a/idea/tests/org/jetbrains/kotlin/idea/quickfix/QuickFixTestGenerated.java b/idea/tests/org/jetbrains/kotlin/idea/quickfix/QuickFixTestGenerated.java index 9042566ecb5..1ec6dccbaa1 100644 --- a/idea/tests/org/jetbrains/kotlin/idea/quickfix/QuickFixTestGenerated.java +++ b/idea/tests/org/jetbrains/kotlin/idea/quickfix/QuickFixTestGenerated.java @@ -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") @TestDataPath("$PROJECT_ROOT") @RunWith(JUnit3RunnerWithInners.class)