diff --git a/idea/resources/inspectionDescriptions/MemberVisibilityCanPrivate.html b/idea/resources/inspectionDescriptions/MemberVisibilityCanPrivate.html new file mode 100644 index 00000000000..8e0de6b18ad --- /dev/null +++ b/idea/resources/inspectionDescriptions/MemberVisibilityCanPrivate.html @@ -0,0 +1,5 @@ + + +This inspection reports class members which can be made private + + \ No newline at end of file diff --git a/idea/src/META-INF/plugin.xml b/idea/src/META-INF/plugin.xml index 45385c0a3db..44c0f6f6c31 100644 --- a/idea/src/META-INF/plugin.xml +++ b/idea/src/META-INF/plugin.xml @@ -2191,6 +2191,14 @@ language="kotlin" /> + + diff --git a/idea/src/org/jetbrains/kotlin/idea/inspections/MemberVisibilityCanPrivateInspection.kt b/idea/src/org/jetbrains/kotlin/idea/inspections/MemberVisibilityCanPrivateInspection.kt new file mode 100644 index 00000000000..8382b01643e --- /dev/null +++ b/idea/src/org/jetbrains/kotlin/idea/inspections/MemberVisibilityCanPrivateInspection.kt @@ -0,0 +1,93 @@ +/* + * Copyright 2010-2017 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.psi.PsiElementVisitor +import com.intellij.psi.PsiNameIdentifierOwner +import com.intellij.psi.PsiReference +import com.intellij.psi.search.LocalSearchScope +import com.intellij.psi.search.searches.ReferencesSearch +import com.intellij.util.Processor +import org.jetbrains.kotlin.idea.quickfix.AddModifierFix +import org.jetbrains.kotlin.idea.refactoring.isConstructorDeclaredProperty +import org.jetbrains.kotlin.lexer.KtTokens +import org.jetbrains.kotlin.psi.* +import org.jetbrains.kotlin.psi.psiUtil.* + +class MemberVisibilityCanPrivateInspection : AbstractKotlinInspection() { + + override fun buildVisitor(holder: ProblemsHolder, isOnTheFly: Boolean): PsiElementVisitor { + return object : KtVisitorVoid() { + override fun visitProperty(property: KtProperty) { + super.visitProperty(property) + if (!property.isLocal && canBePrivate(property)) { + registerProblem(holder, property) + } + } + + override fun visitNamedFunction(function: KtNamedFunction) { + super.visitNamedFunction(function) + if (canBePrivate(function)) { + registerProblem(holder, function) + } + } + + override fun visitParameter(parameter: KtParameter) { + super.visitParameter(parameter) + if (parameter.isConstructorDeclaredProperty() && canBePrivate(parameter)) { + registerProblem(holder, parameter) + } + } + } + } + + + private fun canBePrivate(declaration: KtDeclaration): Boolean { + if (declaration.hasModifier(KtTokens.PRIVATE_KEYWORD) || declaration.hasModifier(KtTokens.OVERRIDE_KEYWORD)) return false + val classOrObject = declaration.containingClassOrObject ?: return false + val inheritable = classOrObject is KtClass && classOrObject.isInheritable() + if (!inheritable && declaration.hasModifier(KtTokens.PROTECTED_KEYWORD)) return false //reported by ProtectedInFinalInspection + if (declaration.isOverridable()) return false + var otherUsageFound = false + var inClassUsageFound = false + ReferencesSearch.search(declaration, declaration.useScope).forEach(Processor { + val usage = it.element + if (classOrObject != usage.getParentOfType(false)) { + otherUsageFound = true + false + } else { + inClassUsageFound = true + true + } + }) + return inClassUsageFound && !otherUsageFound + } + + private fun registerProblem(holder: ProblemsHolder, declaration: KtDeclaration) { + val modifierListOwner = declaration.getParentOfType(false) ?: return + val member = when (declaration) { + is KtNamedFunction -> "Function" + else -> "Property" + } + val nameElement = (declaration as? PsiNameIdentifierOwner)?.nameIdentifier ?: return + holder.registerProblem(nameElement, + "$member '${declaration.name}' can be private", + ProblemHighlightType.WEAK_WARNING, + IntentionWrapper(AddModifierFix(modifierListOwner, KtTokens.PRIVATE_KEYWORD), declaration.containingFile)) + } +} \ No newline at end of file diff --git a/idea/testData/inspections/memberVisibilityCanBePrivate/inspectionData/expected.xml b/idea/testData/inspections/memberVisibilityCanBePrivate/inspectionData/expected.xml new file mode 100644 index 00000000000..43bc0617b61 --- /dev/null +++ b/idea/testData/inspections/memberVisibilityCanBePrivate/inspectionData/expected.xml @@ -0,0 +1,110 @@ + + + test.kt + 13 + light_idea_test_case + + Can have 'private' visibility + Property 'a' can be private + + + + + test.kt + 19 + light_idea_test_case + + Can have 'private' visibility + Function 'f1' can be private + + + + test.kt + 20 + light_idea_test_case + + Can have 'private' visibility + Function 'f2' can be private + + + + test.kt + 65 + light_idea_test_case + + Can have 'private' visibility + Property 'y' can be private + + + + test.kt + 68 + light_idea_test_case + + Can have 'private' visibility + Function 'f2' can be private + + + + test.kt + 78 + light_idea_test_case + + Can have 'private' visibility + Property 'a' can be private + + + + test.kt + 79 + light_idea_test_case + + Can have 'private' visibility + Property 'b' can be private + + + + test.kt + 80 + light_idea_test_case + + Can have 'private' visibility + Property 'c' can be private + + + + test.kt + 109 + light_idea_test_case + + Can have 'private' visibility + Property 'a' can be private + + + + test.kt + 110 + light_idea_test_case + + Can have 'private' visibility + Property 'b' can be private + + + + test.kt + 114 + light_idea_test_case + + Can have 'private' visibility + Function 'f1' can be private + + + + test.kt + 115 + light_idea_test_case + + Can have 'private' visibility + Function 'f2' can be private + + diff --git a/idea/testData/inspections/memberVisibilityCanBePrivate/inspectionData/inspections.test b/idea/testData/inspections/memberVisibilityCanBePrivate/inspectionData/inspections.test new file mode 100644 index 00000000000..e07c07e1dcd --- /dev/null +++ b/idea/testData/inspections/memberVisibilityCanBePrivate/inspectionData/inspections.test @@ -0,0 +1 @@ +// INSPECTION_CLASS: org.jetbrains.kotlin.idea.inspections.MemberVisibilityCanPrivateInspection \ No newline at end of file diff --git a/idea/testData/inspections/memberVisibilityCanBePrivate/test.kt b/idea/testData/inspections/memberVisibilityCanBePrivate/test.kt new file mode 100644 index 00000000000..4b497322359 --- /dev/null +++ b/idea/testData/inspections/memberVisibilityCanBePrivate/test.kt @@ -0,0 +1,129 @@ +val a = "a" + +fun f1() {} + +fun f2() { + println(a) + f1() + A().b +} + +class A { + val unused = "" + val a = "" + internal val b = "" + protected val c = "" + private val d = "" + + fun unused() {} + fun f1() {} + internal fun f2() {} + protected fun f3() {} + private fun f4() {} + + fun bar() { + println(a) + println(b) + println(c) + println(d) + f1() + f2() + f3() + f4() + } +} + +interface I { + val x: String + fun foo() +} + +class B : I { + override val x: String + get() = "" + + override fun foo() { + } + + fun bar() { + println(x) + foo() + } +} + +interface I2 { + val x: String + fun foo() + fun bar() { + println(x) + foo() + } +} + +open class C { + open val x = "" + protected val y = "" + + open fun f1() {} + protected fun f2() {} + + fun bar() { + println(x) + println(y) + f1() + f2() + } +} + +class D(val a: String = "", + var b: String = "", + internal val c: String = "", + protected val d: String = "", + private val e: String = "") { + fun foo() { + println(a) + println(b) + println(c) + println(d) + println(e) + } +} + +open class E(override val x: String = "", + open val a: String = "") : I { + override fun foo() {} + fun foo() { + println(x) + println(a) + } + + fun bar() { + var v1 = "" + val v2 = "" + println(v1) + println(v2) + } +} + +val x = object { + val a: String = "", + internal val b: String = "", + protected val c: String = "", + private val d: String = "" + + fun f1() {} + internal fun f2() {} + protected fun f3() {} + private fun f4() {} + + fun foo() { + println(a) + println(b) + println(c) + println(d) + f1() + f2() + f3() + f4() + } +} \ No newline at end of file diff --git a/idea/testData/quickfix/memberVisibilityCanBePrivate/.inspection b/idea/testData/quickfix/memberVisibilityCanBePrivate/.inspection new file mode 100644 index 00000000000..6365e001f56 --- /dev/null +++ b/idea/testData/quickfix/memberVisibilityCanBePrivate/.inspection @@ -0,0 +1 @@ +org.jetbrains.kotlin.idea.inspections.MemberVisibilityCanPrivateInspection \ No newline at end of file diff --git a/idea/testData/quickfix/memberVisibilityCanBePrivate/constructorParam.kt b/idea/testData/quickfix/memberVisibilityCanBePrivate/constructorParam.kt new file mode 100644 index 00000000000..81d0e6be0d2 --- /dev/null +++ b/idea/testData/quickfix/memberVisibilityCanBePrivate/constructorParam.kt @@ -0,0 +1,6 @@ +// "Add 'private' modifier" "true" +class A(internal val a: String = "") { + fun foo() { + a + } +} diff --git a/idea/testData/quickfix/memberVisibilityCanBePrivate/constructorParam.kt.after b/idea/testData/quickfix/memberVisibilityCanBePrivate/constructorParam.kt.after new file mode 100644 index 00000000000..d86fdda03d6 --- /dev/null +++ b/idea/testData/quickfix/memberVisibilityCanBePrivate/constructorParam.kt.after @@ -0,0 +1,6 @@ +// "Add 'private' modifier" "true" +class A(private val a: String = "") { + fun foo() { + a + } +} diff --git a/idea/testData/quickfix/memberVisibilityCanBePrivate/internal.kt b/idea/testData/quickfix/memberVisibilityCanBePrivate/internal.kt new file mode 100644 index 00000000000..0b51ec397fb --- /dev/null +++ b/idea/testData/quickfix/memberVisibilityCanBePrivate/internal.kt @@ -0,0 +1,8 @@ +// "Add 'private' modifier" "true" +class A { + internal val a = "" + + fun foo() { + a + } +} \ No newline at end of file diff --git a/idea/testData/quickfix/memberVisibilityCanBePrivate/internal.kt.after b/idea/testData/quickfix/memberVisibilityCanBePrivate/internal.kt.after new file mode 100644 index 00000000000..2b55d40e35e --- /dev/null +++ b/idea/testData/quickfix/memberVisibilityCanBePrivate/internal.kt.after @@ -0,0 +1,8 @@ +// "Add 'private' modifier" "true" +class A { + private val a = "" + + fun foo() { + a + } +} \ No newline at end of file diff --git a/idea/testData/quickfix/memberVisibilityCanBePrivate/noModifier.kt b/idea/testData/quickfix/memberVisibilityCanBePrivate/noModifier.kt new file mode 100644 index 00000000000..9ebbeb8aa10 --- /dev/null +++ b/idea/testData/quickfix/memberVisibilityCanBePrivate/noModifier.kt @@ -0,0 +1,8 @@ +// "Add 'private' modifier" "true" +class A { + val a = "" + + fun foo() { + a + } +} \ No newline at end of file diff --git a/idea/testData/quickfix/memberVisibilityCanBePrivate/noModifier.kt.after b/idea/testData/quickfix/memberVisibilityCanBePrivate/noModifier.kt.after new file mode 100644 index 00000000000..2b55d40e35e --- /dev/null +++ b/idea/testData/quickfix/memberVisibilityCanBePrivate/noModifier.kt.after @@ -0,0 +1,8 @@ +// "Add 'private' modifier" "true" +class A { + private val a = "" + + fun foo() { + a + } +} \ No newline at end of file diff --git a/idea/testData/quickfix/memberVisibilityCanBePrivate/protected.kt b/idea/testData/quickfix/memberVisibilityCanBePrivate/protected.kt new file mode 100644 index 00000000000..c5319138629 --- /dev/null +++ b/idea/testData/quickfix/memberVisibilityCanBePrivate/protected.kt @@ -0,0 +1,8 @@ +// "Add 'private' modifier" "true" +open class A { + protected val a = "" + + fun foo() { + a + } +} \ No newline at end of file diff --git a/idea/testData/quickfix/memberVisibilityCanBePrivate/protected.kt.after b/idea/testData/quickfix/memberVisibilityCanBePrivate/protected.kt.after new file mode 100644 index 00000000000..c6aed13e015 --- /dev/null +++ b/idea/testData/quickfix/memberVisibilityCanBePrivate/protected.kt.after @@ -0,0 +1,8 @@ +// "Add 'private' modifier" "true" +open class A { + private val a = "" + + fun foo() { + a + } +} \ No newline at end of file diff --git a/idea/testData/quickfix/memberVisibilityCanBePrivate/public.kt b/idea/testData/quickfix/memberVisibilityCanBePrivate/public.kt new file mode 100644 index 00000000000..a29e11b25b4 --- /dev/null +++ b/idea/testData/quickfix/memberVisibilityCanBePrivate/public.kt @@ -0,0 +1,8 @@ +// "Add 'private' modifier" "true" +class A { + public val a = "" + + fun foo() { + a + } +} \ No newline at end of file diff --git a/idea/testData/quickfix/memberVisibilityCanBePrivate/public.kt.after b/idea/testData/quickfix/memberVisibilityCanBePrivate/public.kt.after new file mode 100644 index 00000000000..2b55d40e35e --- /dev/null +++ b/idea/testData/quickfix/memberVisibilityCanBePrivate/public.kt.after @@ -0,0 +1,8 @@ +// "Add 'private' modifier" "true" +class A { + private val a = "" + + fun foo() { + a + } +} \ No newline at end of file diff --git a/idea/tests/org/jetbrains/kotlin/idea/codeInsight/InspectionTestGenerated.java b/idea/tests/org/jetbrains/kotlin/idea/codeInsight/InspectionTestGenerated.java index 4b239c2c9c5..672712b26b7 100644 --- a/idea/tests/org/jetbrains/kotlin/idea/codeInsight/InspectionTestGenerated.java +++ b/idea/tests/org/jetbrains/kotlin/idea/codeInsight/InspectionTestGenerated.java @@ -221,6 +221,12 @@ public class InspectionTestGenerated extends AbstractInspectionTest { doTest(fileName); } + @TestMetadata("memberVisibilityCanBePrivate/inspectionData/inspections.test") + public void testMemberVisibilityCanBePrivate_inspectionData_Inspections_test() throws Exception { + String fileName = KotlinTestUtils.navigationMetadata("idea/testData/inspections/memberVisibilityCanBePrivate/inspectionData/inspections.test"); + doTest(fileName); + } + @TestMetadata("overridingDeprecatedMember/inspectionData/inspections.test") public void testOverridingDeprecatedMember_inspectionData_Inspections_test() throws Exception { String fileName = KotlinTestUtils.navigationMetadata("idea/testData/inspections/overridingDeprecatedMember/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 c0292ae9a7c..bc3d86831e6 100644 --- a/idea/tests/org/jetbrains/kotlin/idea/quickfix/QuickFixTestGenerated.java +++ b/idea/tests/org/jetbrains/kotlin/idea/quickfix/QuickFixTestGenerated.java @@ -6327,6 +6327,45 @@ public class QuickFixTestGenerated extends AbstractQuickFixTest { } } + @TestMetadata("idea/testData/quickfix/memberVisibilityCanBePrivate") + @TestDataPath("$PROJECT_ROOT") + @RunWith(JUnit3RunnerWithInners.class) + public static class MemberVisibilityCanBePrivate extends AbstractQuickFixTest { + public void testAllFilesPresentInMemberVisibilityCanBePrivate() throws Exception { + KotlinTestUtils.assertAllTestsPresentByMetadata(this.getClass(), new File("idea/testData/quickfix/memberVisibilityCanBePrivate"), Pattern.compile("^([\\w\\-_]+)\\.kt$"), TargetBackend.ANY, true); + } + + @TestMetadata("constructorParam.kt") + public void testConstructorParam() throws Exception { + String fileName = KotlinTestUtils.navigationMetadata("idea/testData/quickfix/memberVisibilityCanBePrivate/constructorParam.kt"); + doTest(fileName); + } + + @TestMetadata("internal.kt") + public void testInternal() throws Exception { + String fileName = KotlinTestUtils.navigationMetadata("idea/testData/quickfix/memberVisibilityCanBePrivate/internal.kt"); + doTest(fileName); + } + + @TestMetadata("noModifier.kt") + public void testNoModifier() throws Exception { + String fileName = KotlinTestUtils.navigationMetadata("idea/testData/quickfix/memberVisibilityCanBePrivate/noModifier.kt"); + doTest(fileName); + } + + @TestMetadata("protected.kt") + public void testProtected() throws Exception { + String fileName = KotlinTestUtils.navigationMetadata("idea/testData/quickfix/memberVisibilityCanBePrivate/protected.kt"); + doTest(fileName); + } + + @TestMetadata("public.kt") + public void testPublic() throws Exception { + String fileName = KotlinTestUtils.navigationMetadata("idea/testData/quickfix/memberVisibilityCanBePrivate/public.kt"); + doTest(fileName); + } + } + @TestMetadata("idea/testData/quickfix/migration") @TestDataPath("$PROJECT_ROOT") @RunWith(JUnit3RunnerWithInners.class)