Visibility can be private: do not perform too expensive search

Also, additional test for usage via accessor was added
So #KT-18617 Fixed
This commit is contained in:
Mikhail Glukhikh
2017-06-23 16:22:11 +03:00
parent 3358f0ab69
commit c99db11ace
4 changed files with 54 additions and 6 deletions
@@ -16,18 +16,24 @@
package org.jetbrains.kotlin.idea.inspections package org.jetbrains.kotlin.idea.inspections
import com.intellij.codeInspection.* import com.intellij.codeInspection.IntentionWrapper
import com.intellij.codeInspection.ProblemHighlightType
import com.intellij.codeInspection.ProblemsHolder
import com.intellij.psi.PsiElementVisitor import com.intellij.psi.PsiElementVisitor
import com.intellij.psi.PsiNameIdentifierOwner import com.intellij.psi.PsiNameIdentifierOwner
import com.intellij.psi.PsiReference import com.intellij.psi.PsiReference
import com.intellij.psi.search.LocalSearchScope import com.intellij.psi.search.GlobalSearchScope
import com.intellij.psi.search.PsiSearchHelper
import com.intellij.psi.search.searches.ReferencesSearch import com.intellij.psi.search.searches.ReferencesSearch
import com.intellij.util.Processor import com.intellij.util.Processor
import org.jetbrains.kotlin.idea.quickfix.AddModifierFix import org.jetbrains.kotlin.idea.quickfix.AddModifierFix
import org.jetbrains.kotlin.idea.refactoring.isConstructorDeclaredProperty import org.jetbrains.kotlin.idea.refactoring.isConstructorDeclaredProperty
import org.jetbrains.kotlin.lexer.KtTokens import org.jetbrains.kotlin.lexer.KtTokens
import org.jetbrains.kotlin.psi.* import org.jetbrains.kotlin.psi.*
import org.jetbrains.kotlin.psi.psiUtil.* import org.jetbrains.kotlin.psi.psiUtil.containingClassOrObject
import org.jetbrains.kotlin.psi.psiUtil.getParentOfType
import org.jetbrains.kotlin.psi.psiUtil.isInheritable
import org.jetbrains.kotlin.psi.psiUtil.isOverridable
class MemberVisibilityCanPrivateInspection : AbstractKotlinInspection() { class MemberVisibilityCanPrivateInspection : AbstractKotlinInspection() {
@@ -57,20 +63,34 @@ class MemberVisibilityCanPrivateInspection : AbstractKotlinInspection() {
} }
private fun canBePrivate(declaration: KtDeclaration): Boolean { private fun canBePrivate(declaration: KtNamedDeclaration): Boolean {
if (declaration.hasModifier(KtTokens.PRIVATE_KEYWORD) || declaration.hasModifier(KtTokens.OVERRIDE_KEYWORD)) return false if (declaration.hasModifier(KtTokens.PRIVATE_KEYWORD) || declaration.hasModifier(KtTokens.OVERRIDE_KEYWORD)) return false
val classOrObject = declaration.containingClassOrObject ?: return false val classOrObject = declaration.containingClassOrObject ?: return false
val inheritable = classOrObject is KtClass && classOrObject.isInheritable() val inheritable = classOrObject is KtClass && classOrObject.isInheritable()
if (!inheritable && declaration.hasModifier(KtTokens.PROTECTED_KEYWORD)) return false //reported by ProtectedInFinalInspection if (!inheritable && declaration.hasModifier(KtTokens.PROTECTED_KEYWORD)) return false //reported by ProtectedInFinalInspection
if (declaration.isOverridable()) return false if (declaration.isOverridable()) return false
val psiSearchHelper = PsiSearchHelper.SERVICE.getInstance(declaration.project)
val useScope = declaration.useScope
val name = declaration.name ?: return false
if (useScope is GlobalSearchScope) {
when (psiSearchHelper.isCheapEnoughToSearch(name, useScope, null, null)) {
PsiSearchHelper.SearchCostResult.TOO_MANY_OCCURRENCES -> return false
PsiSearchHelper.SearchCostResult.ZERO_OCCURRENCES -> return false
PsiSearchHelper.SearchCostResult.FEW_OCCURRENCES -> {
}
}
}
var otherUsageFound = false var otherUsageFound = false
var inClassUsageFound = false var inClassUsageFound = false
ReferencesSearch.search(declaration, declaration.useScope).forEach(Processor<PsiReference> { ReferencesSearch.search(declaration, useScope).forEach(Processor<PsiReference> {
val usage = it.element val usage = it.element
if (classOrObject != usage.getParentOfType<KtClassOrObject>(false)) { if (classOrObject != usage.getParentOfType<KtClassOrObject>(false)) {
otherUsageFound = true otherUsageFound = true
false false
} else { }
else {
inClassUsageFound = true inClassUsageFound = true
true true
} }
@@ -0,0 +1,8 @@
// "Add 'private' modifier" "false"
// ACTION: Convert to secondary constructor
// ACTION: Create test
// ACTION: Move to class body
class My(val <caret>parameter: Int) {
val other = parameter
}
@@ -0,0 +1,5 @@
public class User {
public static int foo(My my) {
return my.getParameter();
}
}
@@ -1665,6 +1665,21 @@ public class QuickFixMultiFileTestGenerated extends AbstractQuickFixMultiFileTes
} }
} }
@TestMetadata("idea/testData/quickfix/memberVisibilityCanBePrivate")
@TestDataPath("$PROJECT_ROOT")
@RunWith(JUnit3RunnerWithInners.class)
public static class MemberVisibilityCanBePrivate extends AbstractQuickFixMultiFileTest {
public void testAllFilesPresentInMemberVisibilityCanBePrivate() throws Exception {
KotlinTestUtils.assertAllTestsPresentByMetadata(this.getClass(), new File("idea/testData/quickfix/memberVisibilityCanBePrivate"), Pattern.compile("^(\\w+)\\.((before\\.Main\\.\\w+)|(test))$"), TargetBackend.ANY, true);
}
@TestMetadata("getter.before.Main.kt")
public void testGetter() throws Exception {
String fileName = KotlinTestUtils.navigationMetadata("idea/testData/quickfix/memberVisibilityCanBePrivate/getter.before.Main.kt");
doTestWithExtraFile(fileName);
}
}
@TestMetadata("idea/testData/quickfix/migration") @TestMetadata("idea/testData/quickfix/migration")
@TestDataPath("$PROJECT_ROOT") @TestDataPath("$PROJECT_ROOT")
@RunWith(JUnit3RunnerWithInners.class) @RunWith(JUnit3RunnerWithInners.class)