From a4cf7a912b65bce96b831d6d66931367b24009d2 Mon Sep 17 00:00:00 2001 From: Ilya Kirillov Date: Tue, 17 Sep 2019 22:32:36 +0300 Subject: [PATCH] Split some non applicability based inspections into isApplicable and apply This is needed to fit new nj2k post-processing structure --- .../idea/inspections/CanBeValInspection.kt | 213 ++++++++++-------- .../inspections/ExplicitThisInspection.kt | 79 ++++--- .../LiftReturnOrAssignmentInspection.kt | 95 +++++--- .../RedundantCompanionReferenceInspection.kt | 79 ++++--- .../RedundantExplicitTypeInspection.kt | 54 +++-- .../inspections/RedundantGetterInspection.kt | 29 ++- .../RedundantSemicolonInspection.kt | 134 +++++------ .../inspections/RedundantSetterInspection.kt | 17 +- .../RedundantUnitReturnTypeInspection.kt | 27 ++- .../intentions/RemoveExplicitTypeIntention.kt | 12 +- 10 files changed, 423 insertions(+), 316 deletions(-) diff --git a/idea/src/org/jetbrains/kotlin/idea/inspections/CanBeValInspection.kt b/idea/src/org/jetbrains/kotlin/idea/inspections/CanBeValInspection.kt index ea6c28df2a5..6387b502821 100644 --- a/idea/src/org/jetbrains/kotlin/idea/inspections/CanBeValInspection.kt +++ b/idea/src/org/jetbrains/kotlin/idea/inspections/CanBeValInspection.kt @@ -40,113 +40,138 @@ import org.jetbrains.kotlin.resolve.BindingContext import org.jetbrains.kotlin.resolve.lazy.BodyResolveMode import java.util.* -class CanBeValInspection @JvmOverloads constructor(val ignoreNotUsedVals: Boolean = true) : AbstractKotlinInspection() { +class CanBeValInspection : AbstractKotlinInspection() { override fun buildVisitor(holder: ProblemsHolder, isOnTheFly: Boolean): PsiElementVisitor { - return object: KtVisitorVoid() { + return object : KtVisitorVoid() { private val pseudocodeCache = HashMap() - override fun visitDeclaration(declaration: KtDeclaration) { super.visitDeclaration(declaration) - - when (declaration) { - is KtProperty -> { - if (declaration.isVar && declaration.isLocal && !declaration.hasModifier(KtTokens.LATEINIT_KEYWORD) && - canBeVal(declaration, - declaration.hasInitializer() || declaration.hasDelegateExpression(), - listOf(declaration))) { - reportCanBeVal(declaration) - } - } - - is KtDestructuringDeclaration -> { - val entries = declaration.entries - if (declaration.isVar && entries.all { canBeVal(it, true, entries) }) { - reportCanBeVal(declaration) - } - } + if (declaration is KtValVarKeywordOwner && canBeVal(declaration, pseudocodeCache, ignoreNotUsedVals = true)) { + reportCanBeVal(declaration) } } - private fun canBeVal( - declaration: KtVariableDeclaration, - hasInitializerOrDelegate: Boolean, - allDeclarations: Collection - ): Boolean { - if (ignoreNotUsedVals && allDeclarations.all { ReferencesSearch.search(it, it.useScope).none() }) { - // do not report for unused var's (otherwise we'll get it highlighted immediately after typing the declaration - return false - } - - return if (hasInitializerOrDelegate) { - val hasWriteUsages = ReferencesSearch.search(declaration, declaration.useScope).any { - (it as? KtSimpleNameReference)?.element?.readWriteAccess(useResolveForReadWrite = true)?.isWrite == true - } - !hasWriteUsages - } - else { - val bindingContext = declaration.analyze(BodyResolveMode.FULL) - val pseudocode = pseudocode(declaration, bindingContext) ?: return false - val descriptor = bindingContext[BindingContext.DECLARATION_TO_DESCRIPTOR, declaration] ?: return false - - val writeInstructions = pseudocode.collectWriteInstructions(descriptor) - if (writeInstructions.isEmpty()) return false // incorrect code - do not report - - writeInstructions.none { it.owner !== pseudocode || canReach(it, writeInstructions) } - } - } - - private fun pseudocode(element: KtElement, bindingContext: BindingContext): Pseudocode? { - val declaration = element.containingDeclarationForPseudocode ?: return null - return pseudocodeCache.getOrPut(declaration) { PseudocodeUtil.generatePseudocode(declaration, bindingContext) } - } - - private fun Pseudocode.collectWriteInstructions(descriptor: DeclarationDescriptor): Set = - with (instructionsIncludingDeadCode) { - filterIsInstance() - .asSequence() - .filter { (it.target as? AccessTarget.Call)?.resolvedCall?.resultingDescriptor == descriptor } - .toSet() + - - filterIsInstance() - .map { it.body.collectWriteInstructions(descriptor) } - .flatten() - } - - private fun canReach(from: Instruction, targets: Set, visited: HashSet = HashSet()): Boolean { - // special algorithm for linear code to avoid too deep recursion - var instruction = from - while (instruction is InstructionWithNext) { - if (instruction is LocalFunctionDeclarationInstruction) { - if (canReach(instruction.body.enterInstruction, targets, visited)) return true - } - val next = instruction.next ?: return false - if (next in visited) return false - if (next in targets) return true - visited.add(next) - instruction = next - } - - for (next in instruction.nextInstructions) { - if (next in visited) continue - if (next in targets) return true - visited.add(next) - if (canReach(next, targets, visited)) return true - } - return false - } - private fun reportCanBeVal(declaration: KtValVarKeywordOwner) { val keyword = declaration.valOrVarKeyword!! val problemDescriptor = holder.manager.createProblemDescriptor( - keyword, - keyword, - "Variable is never modified and can be declared immutable using 'val'", - ProblemHighlightType.GENERIC_ERROR_OR_WARNING, - isOnTheFly, - IntentionWrapper(ChangeVariableMutabilityFix(declaration, false), declaration.containingFile) + keyword, + keyword, + "Variable is never modified and can be declared immutable using 'val'", + ProblemHighlightType.GENERIC_ERROR_OR_WARNING, + isOnTheFly, + IntentionWrapper(ChangeVariableMutabilityFix(declaration, false), declaration.containingFile) ) holder.registerProblem(problemDescriptor) } } } + + companion object { + fun canBeVal( + declaration: KtDeclaration, + pseudocodeCache: HashMap = HashMap(), + ignoreNotUsedVals: Boolean + ): Boolean { + when (declaration) { + is KtProperty -> { + if (declaration.isVar && declaration.isLocal && !declaration.hasModifier(KtTokens.LATEINIT_KEYWORD) && + canBeVal( + declaration, + declaration.hasInitializer() || declaration.hasDelegateExpression(), + listOf(declaration), + ignoreNotUsedVals, + pseudocodeCache + ) + ) { + return true + } + } + + is KtDestructuringDeclaration -> { + val entries = declaration.entries + if (declaration.isVar && entries.all { canBeVal(it, true, entries, ignoreNotUsedVals, pseudocodeCache) }) { + return true + } + } + } + return false + } + + private fun canBeVal( + declaration: KtVariableDeclaration, + hasInitializerOrDelegate: Boolean, + allDeclarations: Collection, + ignoreNotUsedVals: Boolean, + pseudocodeCache: MutableMap + ): Boolean { + if (ignoreNotUsedVals && allDeclarations.all { ReferencesSearch.search(it, it.useScope).none() }) { + // do not report for unused var's (otherwise we'll get it highlighted immediately after typing the declaration + return false + } + + return if (hasInitializerOrDelegate) { + val hasWriteUsages = ReferencesSearch.search(declaration, declaration.useScope).any { + (it as? KtSimpleNameReference)?.element?.readWriteAccess(useResolveForReadWrite = true)?.isWrite == true + } + !hasWriteUsages + } else { + val bindingContext = declaration.analyze(BodyResolveMode.FULL) + val pseudocode = pseudocode(declaration, bindingContext, pseudocodeCache) ?: return false + val descriptor = bindingContext[BindingContext.DECLARATION_TO_DESCRIPTOR, declaration] ?: return false + + val writeInstructions = pseudocode.collectWriteInstructions(descriptor) + if (writeInstructions.isEmpty()) return false // incorrect code - do not report + + writeInstructions.none { it.owner !== pseudocode || canReach(it, writeInstructions) } + } + } + + private fun pseudocode( + element: KtElement, + bindingContext: BindingContext, + pseudocodeCache: MutableMap + ): Pseudocode? { + val declaration = element.containingDeclarationForPseudocode ?: return null + return pseudocodeCache.getOrPut(declaration) { PseudocodeUtil.generatePseudocode(declaration, bindingContext) } + } + + private fun Pseudocode.collectWriteInstructions(descriptor: DeclarationDescriptor): Set = + with(instructionsIncludingDeadCode) { + filterIsInstance() + .asSequence() + .filter { (it.target as? AccessTarget.Call)?.resolvedCall?.resultingDescriptor == descriptor } + .toSet() + + + filterIsInstance() + .map { it.body.collectWriteInstructions(descriptor) } + .flatten() + } + + private fun canReach( + from: Instruction, + targets: Set, + visited: HashSet = HashSet() + ): Boolean { + // special algorithm for linear code to avoid too deep recursion + var instruction = from + while (instruction is InstructionWithNext) { + if (instruction is LocalFunctionDeclarationInstruction) { + if (canReach(instruction.body.enterInstruction, targets, visited)) return true + } + val next = instruction.next ?: return false + if (next in visited) return false + if (next in targets) return true + visited.add(next) + instruction = next + } + + for (next in instruction.nextInstructions) { + if (next in visited) continue + if (next in targets) return true + visited.add(next) + if (canReach(next, targets, visited)) return true + } + return false + } + } } \ No newline at end of file diff --git a/idea/src/org/jetbrains/kotlin/idea/inspections/ExplicitThisInspection.kt b/idea/src/org/jetbrains/kotlin/idea/inspections/ExplicitThisInspection.kt index 5d567433b4c..a27b5bc207f 100644 --- a/idea/src/org/jetbrains/kotlin/idea/inspections/ExplicitThisInspection.kt +++ b/idea/src/org/jetbrains/kotlin/idea/inspections/ExplicitThisInspection.kt @@ -21,13 +21,11 @@ import com.intellij.codeInspection.ProblemDescriptor import com.intellij.codeInspection.ProblemHighlightType.LIKE_UNUSED_SYMBOL import com.intellij.codeInspection.ProblemsHolder import com.intellij.openapi.project.Project -import org.jetbrains.kotlin.config.LanguageFeature.CallableReferencesToClassMembersWithEmptyLHS import org.jetbrains.kotlin.descriptors.FunctionDescriptor import org.jetbrains.kotlin.descriptors.VariableDescriptor import org.jetbrains.kotlin.descriptors.impl.LocalVariableDescriptor import org.jetbrains.kotlin.idea.caches.resolve.analyze import org.jetbrains.kotlin.idea.intentions.getCallableDescriptor -import org.jetbrains.kotlin.idea.project.languageVersionSettings import org.jetbrains.kotlin.idea.references.mainReference import org.jetbrains.kotlin.idea.util.* import org.jetbrains.kotlin.psi.* @@ -38,30 +36,39 @@ import org.jetbrains.kotlin.synthetic.SyntheticJavaPropertyDescriptor import org.jetbrains.kotlin.types.typeUtil.isSubtypeOf class ExplicitThisInspection : AbstractKotlinInspection() { - override fun buildVisitor(holder: ProblemsHolder, isOnTheFly: Boolean) = object : KtVisitorVoid() { - override fun visitCallableReferenceExpression(expression: KtCallableReferenceExpression) { - if (!expression.languageVersionSettings.supportsFeature(CallableReferencesToClassMembersWithEmptyLHS)) return + override fun visitExpression(expression: KtExpression) { + val thisExpression = expression.thisAsReceiverOrNull() ?: return + if (hasExplicitThis(expression)) { + holder.registerProblem( + thisExpression, + "Redundant explicit this", + LIKE_UNUSED_SYMBOL, + ExplicitThisExpressionFix(thisExpression.text) + ) + } + } + } - val thisExpression = expression.receiverExpression as? KtThisExpression ?: return - val selectorExpression = expression.callableReference - - handle(expression, thisExpression, selectorExpression) + companion object { + fun KtExpression.thisAsReceiverOrNull() = when (this) { + is KtCallableReferenceExpression -> receiverExpression as? KtThisExpression + is KtDotQualifiedExpression -> receiverExpression as? KtThisExpression + else -> null } - override fun visitDotQualifiedExpression(expression: KtDotQualifiedExpression) { - val thisExpression = expression.receiverExpression as? KtThisExpression ?: return - val selectorExpression = expression.selectorExpression as? KtReferenceExpression ?: return - - handle(expression, thisExpression, selectorExpression) - } - - private fun handle(expression: KtExpression, thisExpression: KtThisExpression, reference: KtReferenceExpression) { + fun hasExplicitThis(expression: KtExpression): Boolean { + val thisExpression = expression.thisAsReceiverOrNull() ?: return false + val reference = when (expression) { + is KtCallableReferenceExpression -> expression.callableReference + is KtDotQualifiedExpression -> expression.selectorExpression as? KtReferenceExpression + else -> null + } ?: return false val context = expression.analyze() - val scope = expression.getResolutionScope(context) ?: return + val scope = expression.getResolutionScope(context) ?: return false - val referenceExpression = reference as? KtNameReferenceExpression ?: reference.getChildOfType() ?: return - val receiverType = context[BindingContext.EXPRESSION_TYPE_INFO, thisExpression]?.type ?: return + val referenceExpression = reference as? KtNameReferenceExpression ?: reference.getChildOfType() ?: return false + val receiverType = context[BindingContext.EXPRESSION_TYPE_INFO, thisExpression]?.type ?: return false //we avoid overload-related problems by enforcing that there is only one candidate val name = referenceExpression.getReferencedNameAsName() @@ -74,21 +81,20 @@ class ExplicitThisInspection : AbstractKotlinInspection() { scope.getAllAccessibleVariables(name) } if (referenceExpression.getCallableDescriptor() is SyntheticJavaPropertyDescriptor) { - if (candidates.map { it.containingDeclaration }.distinct().size != 1) return + if (candidates.map { it.containingDeclaration }.distinct().size != 1) return false } else { - val candidate = candidates.singleOrNull() ?: return + val candidate = candidates.singleOrNull() ?: return false val extensionType = candidate.extensionReceiverParameter?.type - if (extensionType != null && extensionType != receiverType && receiverType.isSubtypeOf(extensionType)) return + if (extensionType != null && extensionType != receiverType && receiverType.isSubtypeOf(extensionType)) return false } - val expressionFactory = scope.getFactoryForImplicitReceiverWithSubtypeOf(receiverType) ?: return + val expressionFactory = scope.getFactoryForImplicitReceiverWithSubtypeOf(receiverType) ?: return false val label = thisExpression.getLabelName() ?: "" - if (!expressionFactory.matchesLabel(label)) return - - holder.registerProblem(thisExpression, "Redundant explicit this", LIKE_UNUSED_SYMBOL, Fix(thisExpression.text)) + if (!expressionFactory.matchesLabel(label)) return false + return true } - + private fun VariableDescriptor.canInvoke(): Boolean { val declarationDescriptor = this.type.constructor.declarationDescriptor as? LazyClassDescriptor ?: return false return declarationDescriptor.declaredCallableMembers.any { (it as? FunctionDescriptor)?.isOperator == true } @@ -99,16 +105,23 @@ class ExplicitThisInspection : AbstractKotlinInspection() { return label == implicitLabel || (label == "" && isImmediate) } } +} - private class Fix(private val text: String) : LocalQuickFix { - override fun getFamilyName(): String = "Remove redundant '$text'" +class ExplicitThisExpressionFix(private val text: String) : LocalQuickFix { + override fun getFamilyName(): String = "Remove redundant '$text'" - override fun applyFix(project: Project, descriptor: ProblemDescriptor) { - val thisExpression = descriptor.psiElement as? KtThisExpression ?: return + override fun applyFix(project: Project, descriptor: ProblemDescriptor) { + val thisExpression = descriptor.psiElement as? KtThisExpression ?: return + removeExplicitThisExpression(thisExpression) + } + + companion object { + fun removeExplicitThisExpression(thisExpression: KtThisExpression) { when (val parent = thisExpression.parent) { is KtDotQualifiedExpression -> parent.replace(parent.selectorExpression ?: return) is KtCallableReferenceExpression -> thisExpression.delete() } } } -} \ No newline at end of file +} + diff --git a/idea/src/org/jetbrains/kotlin/idea/inspections/LiftReturnOrAssignmentInspection.kt b/idea/src/org/jetbrains/kotlin/idea/inspections/LiftReturnOrAssignmentInspection.kt index f60f442e319..9f016104956 100644 --- a/idea/src/org/jetbrains/kotlin/idea/inspections/LiftReturnOrAssignmentInspection.kt +++ b/idea/src/org/jetbrains/kotlin/idea/inspections/LiftReturnOrAssignmentInspection.kt @@ -33,25 +33,20 @@ class LiftReturnOrAssignmentInspection @JvmOverloads constructor(private val ski override fun buildVisitor(holder: ProblemsHolder, isOnTheFly: Boolean, session: LocalInspectionToolSession) = object : KtVisitorVoid() { - private fun visitIfOrWhenOrTry(expression: KtExpression, keyword: PsiElement) { - if (skipLongExpressions && expression.lineCount() > LINES_LIMIT) return - if (expression.isElseIf()) return - - val foldableReturns = BranchedFoldingUtils.getFoldableReturns(expression) - if (foldableReturns?.isNotEmpty() == true) { - val hasOtherReturns = expression.anyDescendantOfType { it !in foldableReturns } - val isSerious = !hasOtherReturns && foldableReturns.size > 1 - registerProblem(expression, keyword, isSerious, LiftReturnOutFix(keyword.text)) - foldableReturns.forEach { - registerProblem(expression, keyword, isSerious, LiftReturnOutFix(keyword.text), it, INFORMATION) - } - return - } - - val assignmentNumber = BranchedFoldingUtils.getFoldableAssignmentNumber(expression) - if (assignmentNumber > 0) { - val isSerious = assignmentNumber > 1 - registerProblem(expression, keyword, isSerious, LiftAssignmentOutFix(keyword.text)) + override fun visitExpression(expression: KtExpression) { + val states = getState(expression, skipLongExpressions) ?: return + states.forEach { state -> + registerProblem( + expression, + state.keyword, + state.isSerious, + when (state.liftType) { + LiftType.LIFT_RETURN_OUT -> LiftReturnOutFix(state.keyword.text) + LiftType.LIFT_ASSIGNMENT_OUT -> LiftAssignmentOutFix(state.keyword.text) + }, + state.highlightElement, + state.highlightType + ) } } @@ -75,22 +70,6 @@ class LiftReturnOrAssignmentInspection @JvmOverloads constructor(private val ski ) } - override fun visitIfExpression(expression: KtIfExpression) { - super.visitIfExpression(expression) - visitIfOrWhenOrTry(expression, expression.ifKeyword) - } - - override fun visitWhenExpression(expression: KtWhenExpression) { - super.visitWhenExpression(expression) - visitIfOrWhenOrTry(expression, expression.whenKeyword) - } - - override fun visitTryExpression(expression: KtTryExpression) { - super.visitTryExpression(expression) - expression.tryKeyword?.let { - visitIfOrWhenOrTry(expression, it) - } - } } private class LiftReturnOutFix(private val keyword: String) : LocalQuickFix { @@ -116,5 +95,51 @@ class LiftReturnOrAssignmentInspection @JvmOverloads constructor(private val ski companion object { private const val LINES_LIMIT = 15 + + fun getState(expression: KtExpression, skipLongExpressions: Boolean) = when (expression) { + is KtWhenExpression -> getStateForWhenOrTry(expression, expression.whenKeyword, skipLongExpressions) + is KtIfExpression -> getStateForWhenOrTry(expression, expression.ifKeyword, skipLongExpressions) + is KtTryExpression -> expression.tryKeyword?.let { + getStateForWhenOrTry(expression, it, skipLongExpressions) + } + else -> null + } + + private fun getStateForWhenOrTry( + expression: KtExpression, + keyword: PsiElement, + skipLongExpressions: Boolean + ): List? { + if (skipLongExpressions && expression.lineCount() > LINES_LIMIT) return null + if (expression.isElseIf()) return null + + val foldableReturns = BranchedFoldingUtils.getFoldableReturns(expression) + if (foldableReturns?.isNotEmpty() == true) { + val hasOtherReturns = expression.anyDescendantOfType { it !in foldableReturns } + val isSerious = !hasOtherReturns && foldableReturns.size > 1 + return foldableReturns.map { + LiftState(keyword, isSerious, LiftType.LIFT_RETURN_OUT, it, INFORMATION) + } + LiftState(keyword, isSerious, LiftType.LIFT_RETURN_OUT) + } + + val assignmentNumber = BranchedFoldingUtils.getFoldableAssignmentNumber(expression) + if (assignmentNumber > 0) { + val isSerious = assignmentNumber > 1 + return listOf(LiftState(keyword, isSerious, LiftType.LIFT_ASSIGNMENT_OUT)) + } + return null + } + + enum class LiftType { + LIFT_RETURN_OUT, LIFT_ASSIGNMENT_OUT + } + + data class LiftState( + val keyword: PsiElement, + val isSerious: Boolean, + val liftType: LiftType, + val highlightElement: PsiElement = keyword, + val highlightType: ProblemHighlightType = if (isSerious) GENERIC_ERROR_OR_WARNING else INFORMATION + ) } } diff --git a/idea/src/org/jetbrains/kotlin/idea/inspections/RedundantCompanionReferenceInspection.kt b/idea/src/org/jetbrains/kotlin/idea/inspections/RedundantCompanionReferenceInspection.kt index f5c75a08400..41f32d4cd1f 100644 --- a/idea/src/org/jetbrains/kotlin/idea/inspections/RedundantCompanionReferenceInspection.kt +++ b/idea/src/org/jetbrains/kotlin/idea/inspections/RedundantCompanionReferenceInspection.kt @@ -33,53 +33,60 @@ import org.jetbrains.kotlin.resolve.scopes.utils.findVariable class RedundantCompanionReferenceInspection : AbstractKotlinInspection() { override fun buildVisitor(holder: ProblemsHolder, isOnTheFly: Boolean): PsiElementVisitor { return referenceExpressionVisitor(fun(expression) { - val parent = expression.parent as? KtDotQualifiedExpression ?: return + if (isRedundantCompanionReference(expression)) { + holder.registerProblem( + expression, + "Redundant Companion reference", + ProblemHighlightType.LIKE_UNUSED_SYMBOL, + RemoveRedundantCompanionReferenceFix() + ) + } + }) + } + + companion object { + fun isRedundantCompanionReference(reference: KtReferenceExpression): Boolean { + val parent = reference.parent as? KtDotQualifiedExpression ?: return false val selectorExpression = parent.selectorExpression - if (expression == selectorExpression && parent.parent !is KtDotQualifiedExpression) return - if (parent.getStrictParentOfType() != null) return + if (reference == selectorExpression && parent.parent !is KtDotQualifiedExpression) return false + if (parent.getStrictParentOfType() != null) return false - val objectDeclaration = expression.mainReference.resolve() as? KtObjectDeclaration ?: return - if (!objectDeclaration.isCompanion()) return - if (expression.text != objectDeclaration.name) return + val objectDeclaration = reference.mainReference.resolve() as? KtObjectDeclaration ?: return false + if (!objectDeclaration.isCompanion()) return false + if (reference.text != objectDeclaration.name) return false - val context = expression.analyze() + val context = reference.analyze() - val containingClass = objectDeclaration.containingClass() ?: return - if (expression.containingClass() != containingClass && expression == parent.receiverExpression) return - val containingClassDescriptor = containingClass.descriptor as? ClassDescriptor ?: return + val containingClass = objectDeclaration.containingClass() ?: return false + if (reference.containingClass() != containingClass && reference == parent.receiverExpression) return false + val containingClassDescriptor = containingClass.descriptor as? ClassDescriptor ?: return false val selectorDescriptor = selectorExpression?.getResolvedCall(context)?.resultingDescriptor when (selectorDescriptor) { is PropertyDescriptor -> { val name = selectorDescriptor.name - if (containingClassDescriptor.findMemberVariable(name) != null) return - val variable = expression.getResolutionScope().findVariable(name, NoLookupLocation.FROM_IDE) - if (variable != null && variable.isLocalOrExtension(containingClassDescriptor)) return + if (containingClassDescriptor.findMemberVariable(name) != null) return false + val variable = reference.getResolutionScope().findVariable(name, NoLookupLocation.FROM_IDE) + if (variable != null && variable.isLocalOrExtension(containingClassDescriptor)) return false } is FunctionDescriptor -> { val name = selectorDescriptor.name - if (containingClassDescriptor.findMemberFunction(name) != null) return - val function = expression.getResolutionScope().findFunction(name, NoLookupLocation.FROM_IDE) - if (function != null && function.isLocalOrExtension(containingClassDescriptor)) return + if (containingClassDescriptor.findMemberFunction(name) != null) return false + val function = reference.getResolutionScope().findFunction(name, NoLookupLocation.FROM_IDE) + if (function != null && function.isLocalOrExtension(containingClassDescriptor)) return false } } - (expression as? KtSimpleNameExpression)?.getReceiverExpression()?.getQualifiedElementSelector() + (reference as? KtSimpleNameExpression)?.getReceiverExpression()?.getQualifiedElementSelector() ?.mainReference?.resolveToDescriptors(context)?.firstOrNull() - ?.let { if (it != containingClassDescriptor) return } + ?.let { if (it != containingClassDescriptor) return false } val grandParent = parent.parent as? KtQualifiedExpression if (grandParent != null) { - val grandParentDescriptor = grandParent.getResolvedCall(context)?.resultingDescriptor ?: return - if (grandParentDescriptor is ConstructorDescriptor || grandParentDescriptor is FakeCallableDescriptorForObject) return + val grandParentDescriptor = grandParent.getResolvedCall(context)?.resultingDescriptor ?: return false + if (grandParentDescriptor is ConstructorDescriptor || grandParentDescriptor is FakeCallableDescriptorForObject) return false } - - holder.registerProblem( - expression, - "Redundant Companion reference", - ProblemHighlightType.LIKE_UNUSED_SYMBOL, - RemoveRedundantCompanionReferenceFix() - ) - }) + return true + } } } @@ -111,16 +118,22 @@ private fun CallableDescriptor.isLocalOrExtension(extensionClassDescriptor: Clas extensionReceiverParameter?.type?.constructor?.declarationDescriptor == extensionClassDescriptor } -private class RemoveRedundantCompanionReferenceFix : LocalQuickFix { +class RemoveRedundantCompanionReferenceFix : LocalQuickFix { override fun getName() = "Remove redundant Companion reference" override fun getFamilyName() = name override fun applyFix(project: Project, descriptor: ProblemDescriptor) { val expression = descriptor.psiElement as? KtReferenceExpression ?: return - val parent = expression.parent as? KtDotQualifiedExpression ?: return - val selector = parent.selectorExpression ?: return - val receiver = parent.receiverExpression - if (expression == receiver) parent.replace(selector) else parent.replace(receiver) + removeRedundantCompanionReference(expression) + } + + companion object { + fun removeRedundantCompanionReference(expression: KtReferenceExpression) { + val parent = expression.parent as? KtDotQualifiedExpression ?: return + val selector = parent.selectorExpression ?: return + val receiver = parent.receiverExpression + if (expression == receiver) parent.replace(selector) else parent.replace(receiver) + } } } \ No newline at end of file diff --git a/idea/src/org/jetbrains/kotlin/idea/inspections/RedundantExplicitTypeInspection.kt b/idea/src/org/jetbrains/kotlin/idea/inspections/RedundantExplicitTypeInspection.kt index 1963451740e..4da76fba803 100644 --- a/idea/src/org/jetbrains/kotlin/idea/inspections/RedundantExplicitTypeInspection.kt +++ b/idea/src/org/jetbrains/kotlin/idea/inspections/RedundantExplicitTypeInspection.kt @@ -18,55 +18,63 @@ import org.jetbrains.kotlin.types.AbbreviatedType class RedundantExplicitTypeInspection : AbstractKotlinInspection() { override fun buildVisitor(holder: ProblemsHolder, isOnTheFly: Boolean) = propertyVisitor(fun(property) { - if (!property.isLocal) return val typeReference = property.typeReference ?: return - val initializer = property.initializer ?: return + if (hasRedundantType(property)) { + holder.registerProblem( + typeReference, + "Explicitly given type is redundant here", + ProblemHighlightType.LIKE_UNUSED_SYMBOL, + IntentionWrapper(RemoveExplicitTypeIntention(), property.containingKtFile) + ) + } + }) - val type = property.resolveToDescriptorIfAny()?.type ?: return - if (type is AbbreviatedType) return + companion object { + fun hasRedundantType(property: KtProperty): Boolean { + if (!property.isLocal) return false + val typeReference = property.typeReference ?: return false + val initializer = property.initializer ?: return false + + val type = property.resolveToDescriptorIfAny()?.type ?: return false + if (type is AbbreviatedType) return false when (initializer) { is KtConstantExpression -> { when (initializer.node.elementType) { KtNodeTypes.BOOLEAN_CONSTANT -> { - if (!KotlinBuiltIns.isBoolean(type)) return + if (!KotlinBuiltIns.isBoolean(type)) return false } KtNodeTypes.INTEGER_CONSTANT -> { if (initializer.text.endsWith("L")) { - if (!KotlinBuiltIns.isLong(type)) return + if (!KotlinBuiltIns.isLong(type)) return false } else { - if (!KotlinBuiltIns.isInt(type)) return + if (!KotlinBuiltIns.isInt(type)) return false } } KtNodeTypes.FLOAT_CONSTANT -> { if (initializer.text.endsWith("f") || initializer.text.endsWith("F")) { - if (!KotlinBuiltIns.isFloat(type)) return + if (!KotlinBuiltIns.isFloat(type)) return false } else { - if (!KotlinBuiltIns.isDouble(type)) return + if (!KotlinBuiltIns.isDouble(type)) return false } } KtNodeTypes.CHARACTER_CONSTANT -> { - if (!KotlinBuiltIns.isChar(type)) return + if (!KotlinBuiltIns.isChar(type)) return false } - else -> return + else -> return false } } is KtStringTemplateExpression -> { - if (!KotlinBuiltIns.isString(type)) return + if (!KotlinBuiltIns.isString(type)) return false } is KtNameReferenceExpression -> { - if (typeReference.text != initializer.getReferencedName()) return + if (typeReference.text != initializer.getReferencedName()) return false } is KtCallExpression -> { - if (typeReference.text != initializer.calleeExpression?.text) return + if (typeReference.text != initializer.calleeExpression?.text) return false } - else -> return + else -> return false } - - holder.registerProblem( - typeReference, - "Explicitly given type is redundant here", - ProblemHighlightType.LIKE_UNUSED_SYMBOL, - IntentionWrapper(RemoveExplicitTypeIntention(), property.containingKtFile) - ) - }) + return true + } + } } \ No newline at end of file diff --git a/idea/src/org/jetbrains/kotlin/idea/inspections/RedundantGetterInspection.kt b/idea/src/org/jetbrains/kotlin/idea/inspections/RedundantGetterInspection.kt index a3a237fbe82..0698b75aa44 100644 --- a/idea/src/org/jetbrains/kotlin/idea/inspections/RedundantGetterInspection.kt +++ b/idea/src/org/jetbrains/kotlin/idea/inspections/RedundantGetterInspection.kt @@ -31,7 +31,7 @@ class RedundantGetterInspection : AbstractKotlinInspection(), CleanupLocalInspec } } -private fun KtPropertyAccessor.isRedundantGetter(): Boolean { +fun KtPropertyAccessor.isRedundantGetter(): Boolean { if (!isGetter) return false val expression = bodyExpression ?: return canBeCompletelyDeleted() if (expression.isBackingFieldReferenceTo(property)) return true @@ -61,23 +61,28 @@ fun KtPropertyAccessor.deleteBody() { deleteChildRange(leftParenthesis, lastChild) } -private class RemoveRedundantGetterFix : LocalQuickFix { +class RemoveRedundantGetterFix : LocalQuickFix { override fun getName() = "Remove redundant getter" override fun getFamilyName() = name override fun applyFix(project: Project, descriptor: ProblemDescriptor) { val accessor = descriptor.psiElement as? KtPropertyAccessor ?: return - val property = accessor.property + removeRedundantGetter(accessor) + } - val accessorTypeReference = accessor.returnTypeReference - if (accessorTypeReference != null && property.typeReference == null && property.initializer == null) { - property.typeReference = accessorTypeReference - } - if (accessor.canBeCompletelyDeleted()) { - accessor.delete() - } else { - accessor.deleteBody() + companion object { + fun removeRedundantGetter(getter: KtPropertyAccessor) { + val property = getter.property + val accessorTypeReference = getter.returnTypeReference + if (accessorTypeReference != null && property.typeReference == null && property.initializer == null) { + property.typeReference = accessorTypeReference + } + if (getter.canBeCompletelyDeleted()) { + getter.delete() + } else { + getter.deleteBody() + } } } -} \ No newline at end of file +} diff --git a/idea/src/org/jetbrains/kotlin/idea/inspections/RedundantSemicolonInspection.kt b/idea/src/org/jetbrains/kotlin/idea/inspections/RedundantSemicolonInspection.kt index deeba1cb7e2..3fc3e88e1ef 100644 --- a/idea/src/org/jetbrains/kotlin/idea/inspections/RedundantSemicolonInspection.kt +++ b/idea/src/org/jetbrains/kotlin/idea/inspections/RedundantSemicolonInspection.kt @@ -37,7 +37,7 @@ class RedundantSemicolonInspection : AbstractKotlinInspection(), CleanupLocalIns override fun visitElement(element: PsiElement) { super.visitElement(element) - if (element.node.elementType == KtTokens.SEMICOLON && isRedundant(element)) { + if (element.node.elementType == KtTokens.SEMICOLON && isRedundantSemicolon(element)) { holder.registerProblem( element, "Redundant semicolon", @@ -49,84 +49,86 @@ class RedundantSemicolonInspection : AbstractKotlinInspection(), CleanupLocalIns } } - private fun isRedundant(semicolon: PsiElement): Boolean { - val nextLeaf = semicolon.nextLeaf { it !is PsiWhiteSpace && it !is PsiComment || it.isLineBreak() } - val isAtEndOfLine = nextLeaf == null || nextLeaf.isLineBreak() - if (!isAtEndOfLine) { - //when there is no imports parser generates empty import list with no spaces - if (semicolon.parent is KtPackageDirective && (nextLeaf as? KtImportList)?.imports?.isEmpty() == true) { - return true + companion object { + fun isRedundantSemicolon(semicolon: PsiElement): Boolean { + val nextLeaf = semicolon.nextLeaf { it !is PsiWhiteSpace && it !is PsiComment || it.isLineBreak() } + val isAtEndOfLine = nextLeaf == null || nextLeaf.isLineBreak() + if (!isAtEndOfLine) { + //when there is no imports parser generates empty import list with no spaces + if (semicolon.parent is KtPackageDirective && (nextLeaf as? KtImportList)?.imports?.isEmpty() == true) { + return true + } + return false } - return false - } - if (semicolon.prevLeaf()?.node?.elementType == KtNodeTypes.ELSE) return false + if (semicolon.prevLeaf()?.node?.elementType == KtNodeTypes.ELSE) return false - if (semicolon.parent is KtEnumEntry) return false + if (semicolon.parent is KtEnumEntry) return false - (semicolon.parent.parent as? KtClass)?.let { - if (it.isEnum() && it.getChildrenOfType().isEmpty()) { - if (semicolon.prevLeaf { it !is PsiWhiteSpace && it !is PsiComment && !it.isLineBreak() }?.node?.elementType == KtTokens.LBRACE - && it.declarations.isNotEmpty() - ) { - //first semicolon in enum with no entries, but with some declarations - return false + (semicolon.parent.parent as? KtClass)?.let { + if (it.isEnum() && it.getChildrenOfType().isEmpty()) { + if (semicolon.prevLeaf { it !is PsiWhiteSpace && it !is PsiComment && !it.isLineBreak() }?.node?.elementType == KtTokens.LBRACE + && it.declarations.isNotEmpty() + ) { + //first semicolon in enum with no entries, but with some declarations + return false + } } } - } - (semicolon.prevLeaf()?.parent as? KtLoopExpression)?.let { - if (it !is KtDoWhileExpression && it.body == null) + (semicolon.prevLeaf()?.parent as? KtLoopExpression)?.let { + if (it !is KtDoWhileExpression && it.body == null) + return false + } + + semicolon.prevLeaf()?.parent?.safeAs()?.also { ifExpression -> + if (ifExpression.then == null) + return false + } + + if (nextLeaf?.nextLeaf { + it !is PsiWhiteSpace && it !is PsiComment && it.getStrictParentOfType() == null && + it.getStrictParentOfType() == null + }?.node?.elementType == KtTokens.LBRACE) { + return false // case with statement starting with '{' and call on the previous line + } + + if (isRequiredForCompanion(semicolon)) { return false + } + + val prevNameReference = semicolon.getPrevSiblingIgnoringWhitespaceAndComments() as? KtNameReferenceExpression + if (prevNameReference != null && prevNameReference.text in softModifierKeywords + && semicolon.getNextSiblingIgnoringWhitespaceAndComments() is KtDeclaration + ) return false + + return true } - semicolon.prevLeaf()?.parent?.safeAs()?.also { ifExpression -> - if (ifExpression.then == null) - return false + private fun isRequiredForCompanion(semicolon: PsiElement): Boolean { + val prev = semicolon.getPrevSiblingIgnoringWhitespaceAndComments() as? KtObjectDeclaration ?: return false + if (!prev.isCompanion()) return false + if (prev.nameIdentifier != null || prev.getChildOfType() != null) return false + + val next = semicolon.getNextSiblingIgnoringWhitespaceAndComments() ?: return false + val firstChildNode = next.firstChild?.node ?: return false + if (KtTokens.KEYWORDS.contains(firstChildNode.elementType)) return false + + return true } - if (nextLeaf?.nextLeaf { - it !is PsiWhiteSpace && it !is PsiComment && it.getStrictParentOfType() == null && - it.getStrictParentOfType() == null - }?.node?.elementType == KtTokens.LBRACE) { - return false // case with statement starting with '{' and call on the previous line + private fun PsiElement?.isLineBreak() = this is PsiWhiteSpace && textContains('\n') + + private object Fix : LocalQuickFix { + override fun getName() = "Remove redundant semicolon" + override fun getFamilyName() = name + + override fun applyFix(project: Project, descriptor: ProblemDescriptor) { + if (!FileModificationService.getInstance().preparePsiElementForWrite(descriptor.psiElement)) return + descriptor.psiElement.delete() + } } - if (isRequiredForCompanion(semicolon)) { - return false - } - - val prevNameReference = semicolon.getPrevSiblingIgnoringWhitespaceAndComments() as? KtNameReferenceExpression - if (prevNameReference != null && prevNameReference.text in softModifierKeywords - && semicolon.getNextSiblingIgnoringWhitespaceAndComments() is KtDeclaration - ) return false - - return true + private val softModifierKeywords = KtTokens.SOFT_KEYWORDS.types.mapNotNull { (it as? KtModifierKeywordToken)?.toString() } } - - private fun isRequiredForCompanion(semicolon: PsiElement): Boolean { - val prev = semicolon.getPrevSiblingIgnoringWhitespaceAndComments() as? KtObjectDeclaration ?: return false - if (!prev.isCompanion()) return false - if (prev.nameIdentifier != null || prev.getChildOfType() != null) return false - - val next = semicolon.getNextSiblingIgnoringWhitespaceAndComments() ?: return false - val firstChildNode = next.firstChild?.node ?: return false - if (KtTokens.KEYWORDS.contains(firstChildNode.elementType)) return false - - return true - } - - private fun PsiElement?.isLineBreak() = this is PsiWhiteSpace && textContains('\n') - - private object Fix : LocalQuickFix { - override fun getName() = "Remove redundant semicolon" - override fun getFamilyName() = name - - override fun applyFix(project: Project, descriptor: ProblemDescriptor) { - if (!FileModificationService.getInstance().preparePsiElementForWrite(descriptor.psiElement)) return - descriptor.psiElement.delete() - } - } - - private val softModifierKeywords = KtTokens.SOFT_KEYWORDS.types.mapNotNull { (it as? KtModifierKeywordToken)?.toString() } } diff --git a/idea/src/org/jetbrains/kotlin/idea/inspections/RedundantSetterInspection.kt b/idea/src/org/jetbrains/kotlin/idea/inspections/RedundantSetterInspection.kt index 316b7117c86..050b2299118 100644 --- a/idea/src/org/jetbrains/kotlin/idea/inspections/RedundantSetterInspection.kt +++ b/idea/src/org/jetbrains/kotlin/idea/inspections/RedundantSetterInspection.kt @@ -30,7 +30,7 @@ class RedundantSetterInspection : AbstractKotlinInspection(), CleanupLocalInspec } } -private fun KtPropertyAccessor.isRedundantSetter(): Boolean { +fun KtPropertyAccessor.isRedundantSetter(): Boolean { if (!isSetter) return false val expression = bodyExpression ?: return canBeCompletelyDeleted() if (expression is KtBlockExpression) { @@ -45,18 +45,23 @@ private fun KtPropertyAccessor.isRedundantSetter(): Boolean { } -private class RemoveRedundantSetterFix : LocalQuickFix { +class RemoveRedundantSetterFix : LocalQuickFix { override fun getName() = "Remove redundant setter" override fun getFamilyName() = name override fun applyFix(project: Project, descriptor: ProblemDescriptor) { val accessor = descriptor.psiElement as? KtPropertyAccessor ?: return + removeRedundantSetter(accessor) + } - if (accessor.canBeCompletelyDeleted()) { - accessor.delete() - } else { - accessor.deleteBody() + companion object { + fun removeRedundantSetter(setter: KtPropertyAccessor) { + if (setter.canBeCompletelyDeleted()) { + setter.delete() + } else { + setter.deleteBody() + } } } } \ No newline at end of file diff --git a/idea/src/org/jetbrains/kotlin/idea/inspections/RedundantUnitReturnTypeInspection.kt b/idea/src/org/jetbrains/kotlin/idea/inspections/RedundantUnitReturnTypeInspection.kt index 1fe8649dce7..7c58c9b999d 100644 --- a/idea/src/org/jetbrains/kotlin/idea/inspections/RedundantUnitReturnTypeInspection.kt +++ b/idea/src/org/jetbrains/kotlin/idea/inspections/RedundantUnitReturnTypeInspection.kt @@ -13,6 +13,7 @@ import com.intellij.psi.PsiElementVisitor import org.jetbrains.kotlin.idea.caches.resolve.resolveToDescriptorIfAny import org.jetbrains.kotlin.idea.intentions.RemoveExplicitTypeIntention import org.jetbrains.kotlin.psi.KtCodeFragment +import org.jetbrains.kotlin.psi.KtNamedFunction import org.jetbrains.kotlin.psi.namedFunctionVisitor import org.jetbrains.kotlin.types.typeUtil.isUnit @@ -21,17 +22,23 @@ class RedundantUnitReturnTypeInspection : AbstractKotlinInspection(), CleanupLoc return namedFunctionVisitor(fun(function) { if (function.containingFile is KtCodeFragment) return val typeElement = function.typeReference?.typeElement ?: return - val descriptor = function.resolveToDescriptorIfAny() ?: return - if (descriptor.returnType?.isUnit() == true) { - if (!function.hasBlockBody()) { - return - } - - holder.registerProblem(typeElement, - "Redundant 'Unit' return type", - ProblemHighlightType.LIKE_UNUSED_SYMBOL, - IntentionWrapper(RemoveExplicitTypeIntention(), function.containingKtFile)) + if (hasRedundantUnitReturnType(function)) { + holder.registerProblem( + typeElement, + "Redundant 'Unit' return type", + ProblemHighlightType.LIKE_UNUSED_SYMBOL, + IntentionWrapper(RemoveExplicitTypeIntention(), function.containingKtFile) + ) } }) } + + companion object { + fun hasRedundantUnitReturnType(function: KtNamedFunction): Boolean { + if (!function.hasBlockBody()) return false + if (function.typeReference?.typeElement == null) return false + val descriptor = function.resolveToDescriptorIfAny() ?: return false + return descriptor.returnType?.isUnit() == true + } + } } \ No newline at end of file diff --git a/idea/src/org/jetbrains/kotlin/idea/intentions/RemoveExplicitTypeIntention.kt b/idea/src/org/jetbrains/kotlin/idea/intentions/RemoveExplicitTypeIntention.kt index 421741116dd..923231f2e6e 100644 --- a/idea/src/org/jetbrains/kotlin/idea/intentions/RemoveExplicitTypeIntention.kt +++ b/idea/src/org/jetbrains/kotlin/idea/intentions/RemoveExplicitTypeIntention.kt @@ -38,13 +38,17 @@ class RemoveExplicitTypeIntention : SelfTargetingRangeIntention