From 5cfb8bbf996f54f9a784caca37ca28667ddba661 Mon Sep 17 00:00:00 2001 From: Alexey Sedunov Date: Tue, 29 Apr 2014 15:07:59 +0400 Subject: [PATCH] Extract Function: Allow the user to proceed with extraction when non-critical errors are found #KT-4941 Fixed --- .../evaluate/KotlinEvaluationBuilder.kt | 7 +- .../ExtractKotlinFunctionHandler.kt | 75 ++++++++++++----- .../extractFunction/ExtractionDescriptor.kt | 14 ++++ .../extractFunction/extractFunctionUtils.kt | 84 +++++++++++-------- 4 files changed, 120 insertions(+), 60 deletions(-) diff --git a/idea/src/org/jetbrains/jet/plugin/debugger/evaluate/KotlinEvaluationBuilder.kt b/idea/src/org/jetbrains/jet/plugin/debugger/evaluate/KotlinEvaluationBuilder.kt index d446952b7b1..e7d01473382 100644 --- a/idea/src/org/jetbrains/jet/plugin/debugger/evaluate/KotlinEvaluationBuilder.kt +++ b/idea/src/org/jetbrains/jet/plugin/debugger/evaluate/KotlinEvaluationBuilder.kt @@ -62,6 +62,7 @@ import org.jetbrains.jet.plugin.caches.resolve.getAnalysisResults import org.jetbrains.jet.lang.psi.JetCodeFragment import org.jetbrains.jet.lang.psi.JetImportList import org.jetbrains.jet.lang.psi.JetExpression +import org.jetbrains.jet.plugin.refactoring.extractFunction.AnalysisResult.Status object KotlinEvaluationBuilder: EvaluatorBuilder { override fun build(codeFragment: PsiElement, position: SourcePosition?): ExpressionEvaluator { @@ -286,11 +287,11 @@ private fun getFunctionForExtractedFragment( if (nextSibling == null) return null val analysisResult = ExtractionData(tmpFile, Collections.singletonList(newDebugExpression), nextSibling).performAnalysis() - if (analysisResult is MaybeError) { - throw EvaluateExceptionUtil.createEvaluateException(analysisResult.error) + if (analysisResult.status != Status.SUCCESS) { + throw EvaluateExceptionUtil.createEvaluateException(analysisResult.messages.makeString("\n")) } - val validationResult = (analysisResult as MaybeValue).value.validate() + val validationResult = analysisResult.descriptor!!.validate() if (!validationResult.conflicts.isEmpty()) { throw EvaluateExceptionUtil.createEvaluateException("Some declarations are unavailable") } diff --git a/idea/src/org/jetbrains/jet/plugin/refactoring/extractFunction/ExtractKotlinFunctionHandler.kt b/idea/src/org/jetbrains/jet/plugin/refactoring/extractFunction/ExtractKotlinFunctionHandler.kt index 328babf15ba..c3ab99f394a 100644 --- a/idea/src/org/jetbrains/jet/plugin/refactoring/extractFunction/ExtractKotlinFunctionHandler.kt +++ b/idea/src/org/jetbrains/jet/plugin/refactoring/extractFunction/ExtractKotlinFunctionHandler.kt @@ -35,19 +35,21 @@ import org.jetbrains.jet.plugin.refactoring.getAllExtractionContainers import com.intellij.refactoring.IntroduceTargetChooser import com.intellij.openapi.util.Pass import com.intellij.openapi.application.ApplicationManager -import org.jetbrains.jet.lang.psi.JetPsiFactory import org.jetbrains.jet.lang.psi.psiUtil.getOutermostParentContainedIn import org.jetbrains.jet.plugin.refactoring.checkConflictsInteractively import org.jetbrains.jet.plugin.refactoring.executeWriteCommand -import org.jetbrains.jet.plugin.util.MaybeError -import org.jetbrains.jet.plugin.util.MaybeValue import org.jetbrains.jet.lang.psi.JetBlockExpression import org.jetbrains.jet.lang.psi.JetClassOrObject import org.jetbrains.jet.lang.psi.JetClassBody -import org.jetbrains.jet.lang.psi.JetNamedFunction -import org.jetbrains.jet.plugin.codeInsight.ShortenReferences import kotlin.test.fail import org.jetbrains.jet.lang.psi.JetDeclarationWithBody +import org.jetbrains.jet.plugin.refactoring.extractFunction.AnalysisResult.Status +import com.intellij.openapi.ui.popup.JBPopupFactory +import com.intellij.openapi.ui.MessageType +import javax.swing.event.HyperlinkEvent +import com.intellij.refactoring.BaseRefactoringProcessor.ConflictsInTestsException +import com.intellij.ui.awt.RelativePoint +import com.intellij.openapi.ui.popup.Balloon.Position public class ExtractKotlinFunctionHandler : RefactoringActionHandler { fun doInvoke( @@ -59,26 +61,59 @@ public class ExtractKotlinFunctionHandler : RefactoringActionHandler { val project = file.getProject() val analysisResult = ExtractionData(file, elements, nextSibling).performAnalysis() - if (analysisResult is MaybeError) { - showErrorHint(project, editor, analysisResult.error) - return + + if (ApplicationManager.getApplication()!!.isUnitTestMode() && analysisResult.status != Status.SUCCESS) { + throw ConflictsInTestsException(analysisResult.messages) } - val validationResult = (analysisResult as MaybeValue).value.validate() - if (!project.checkConflictsInteractively(validationResult.conflicts)) return + fun proceedWithExtraction() { + val validationResult = analysisResult.descriptor!!.validate() + if (!project.checkConflictsInteractively(validationResult.conflicts)) return - val descriptor = - if (ApplicationManager.getApplication()!!.isUnitTestMode()) { - validationResult.descriptor - } - else { - val dialog = KotlinExtractFunctionDialog(project, validationResult) - if (!dialog.showAndGet()) return + val descriptor = + if (ApplicationManager.getApplication()!!.isUnitTestMode()) { + validationResult.descriptor + } + else { + val dialog = KotlinExtractFunctionDialog(project, validationResult) + if (!dialog.showAndGet()) return - dialog.getCurrentDescriptor() - } + dialog.getCurrentDescriptor() + } - project.executeWriteCommand(EXTRACT_FUNCTION) { descriptor.generateFunction() } + project.executeWriteCommand(EXTRACT_FUNCTION) { descriptor.generateFunction() } + } + + val message = analysisResult.messages.makeString("\n") + when (analysisResult.status) { + Status.CRITICAL_ERROR -> { + showErrorHint(project, editor, message) + } + + Status.NON_CRITICAL_ERROR -> { + val anchorPoint = RelativePoint( + editor.getContentComponent(), + editor.visualPositionToXY(editor.getSelectionModel().getSelectionStartPosition()!!) + ) + JBPopupFactory.getInstance()!! + .createHtmlTextBalloonBuilder( + "$message

Proceed with extraction", + MessageType.WARNING, + { event -> + if (event?.getEventType() == HyperlinkEvent.EventType.ACTIVATED) { + proceedWithExtraction() + } + } + ) + .setHideOnClickOutside(true) + .setHideOnFrameResize(false) + .setHideOnLinkClick(true) + .createBalloon() + .show(anchorPoint, Position.below) + } + + Status.SUCCESS -> proceedWithExtraction() + } } override fun invoke(project: Project, editor: Editor, file: PsiFile, dataContext: DataContext?) { diff --git a/idea/src/org/jetbrains/jet/plugin/refactoring/extractFunction/ExtractionDescriptor.kt b/idea/src/org/jetbrains/jet/plugin/refactoring/extractFunction/ExtractionDescriptor.kt index 23e4c22ac77..fc03b1ec23d 100644 --- a/idea/src/org/jetbrains/jet/plugin/refactoring/extractFunction/ExtractionDescriptor.kt +++ b/idea/src/org/jetbrains/jet/plugin/refactoring/extractFunction/ExtractionDescriptor.kt @@ -31,6 +31,8 @@ import org.jetbrains.jet.lang.psi.psiUtil.replaced import org.jetbrains.jet.lang.psi.JetQualifiedExpression import org.jetbrains.jet.lang.psi.JetTypeParameter import org.jetbrains.jet.lang.psi.JetTypeConstraint +import kotlin.properties.Delegates +import org.jetbrains.jet.plugin.refactoring.extractFunction.AnalysisResult.Status data class Parameter( val argumentText: String, @@ -132,6 +134,18 @@ data class ExtractionDescriptor( val controlFlow: ControlFlow ) +class AnalysisResult ( + val descriptor: ExtractionDescriptor?, + val status: Status, + val messages: List +) { + enum class Status { + SUCCESS + NON_CRITICAL_ERROR + CRITICAL_ERROR + } +} + class ExtractionDescriptorWithConflicts( val descriptor: ExtractionDescriptor, val conflicts: MultiMap diff --git a/idea/src/org/jetbrains/jet/plugin/refactoring/extractFunction/extractFunctionUtils.kt b/idea/src/org/jetbrains/jet/plugin/refactoring/extractFunction/extractFunctionUtils.kt index 5da9af55e42..333eb1065d0 100644 --- a/idea/src/org/jetbrains/jet/plugin/refactoring/extractFunction/extractFunctionUtils.kt +++ b/idea/src/org/jetbrains/jet/plugin/refactoring/extractFunction/extractFunctionUtils.kt @@ -31,13 +31,10 @@ import org.jetbrains.jet.lang.descriptors.DeclarationDescriptor import org.jetbrains.jet.lang.descriptors.CallableDescriptor import org.jetbrains.jet.plugin.refactoring.JetRefactoringBundle import org.jetbrains.jet.lang.cfg.pseudocode.PseudocodeUtil -import org.jetbrains.jet.plugin.util.MaybeValue -import org.jetbrains.jet.plugin.util.Maybe import org.jetbrains.jet.lang.psi.psiUtil.isInsideOf import java.util.HashSet import org.jetbrains.jet.lang.cfg.pseudocode.WriteValueInstruction import org.jetbrains.jet.lang.descriptors.VariableDescriptor -import org.jetbrains.jet.plugin.util.MaybeError import org.jetbrains.jet.lang.psi.psiUtil.appendElement import org.jetbrains.jet.plugin.refactoring.createTempCopy import org.jetbrains.jet.lang.psi.psiUtil.getParentByType @@ -92,7 +89,6 @@ import org.jetbrains.jet.lang.descriptors.TypeParameterDescriptor import org.jetbrains.jet.utils.DFS import org.jetbrains.jet.utils.DFS.Neighbors import org.jetbrains.jet.utils.DFS.VisitedWithSet -import org.jetbrains.jet.utils.DFS.AbstractNodeHandler import org.jetbrains.jet.utils.DFS.CollectingNodeHandler import org.jetbrains.jet.lang.resolve.BindingContextUtils import org.jetbrains.jet.plugin.caches.resolve.getLazyResolveSession @@ -100,6 +96,7 @@ import org.jetbrains.jet.plugin.project.AnalyzerFacadeWithCache import org.jetbrains.jet.lang.diagnostics.Errors import org.jetbrains.jet.lang.psi.JetTypeReference import org.jetbrains.jet.lang.psi.JetTypeParameterListOwner +import org.jetbrains.jet.plugin.refactoring.extractFunction.AnalysisResult.Status import org.jetbrains.jet.lang.descriptors.FunctionDescriptor private val DEFAULT_FUNCTION_NAME = "myFun" @@ -147,13 +144,16 @@ private fun List.analyzeControlFlow( bindingContext: BindingContext, parameters: Set, inferredType: JetType? -): Maybe { +): Pair { val outParameters = parameters.filterTo(HashSet()) { it.mirrorVarName != null } if (outParameters.size > 1) { val outValuesStr = outParameters.map { it.argumentText }.sort().makeString(separator = "\n") - return MaybeError("${JetRefactoringBundle.message("selected.code.fragment.has.multiple.output.values")!!}\n$outValuesStr") + return Pair( + DefaultControlFlow, + "${JetRefactoringBundle.message("selected.code.fragment.has.multiple.output.values")!!}\n$outValuesStr" + ) } val exitPoints = getExitPoints() @@ -188,19 +188,21 @@ private fun List.analyzeControlFlow( if (outParameters.isNotEmpty()) { if (hasMeaningfulType || valuedReturnExits.isNotEmpty() || jumpExits.isNotEmpty()) { - return MaybeError(JetRefactoringBundle.message("selected.code.fragment.has.output.values.and.exit.points")!!) + return Pair(DefaultControlFlow, JetRefactoringBundle.message("selected.code.fragment.has.output.values.and.exit.points")!!) } - return MaybeValue(ParameterUpdate(outParameters.first())) + return Pair(ParameterUpdate(outParameters.first()), null) } - val multipleExitsError = - MaybeError(JetRefactoringBundle.message("selected.code.fragment.has.multiple.exit.points")!!) + val multipleExitsError = Pair( + DefaultControlFlow, + JetRefactoringBundle.message("selected.code.fragment.has.multiple.exit.points")!! + ) if (hasMeaningfulType) { if (valuedReturnExits.isNotEmpty() || jumpExits.isNotEmpty()) return multipleExitsError - return MaybeValue(ExpressionEvaluation(inferredType!!)) + return Pair(ExpressionEvaluation(inferredType!!), null) } if (valuedReturnExits.isNotEmpty()) { @@ -210,22 +212,22 @@ private fun List.analyzeControlFlow( if (valuedReturnExits.size != 1) return multipleExitsError val element = valuedReturnExits.first!!.getElement() - return MaybeValue(ConditionalJump(listOf(element), element)) + return Pair(ConditionalJump(listOf(element), element), null) } if (!valuedReturnExits.checkEquivalence(false)) return multipleExitsError - return MaybeValue(ExpressionEvaluationWithCallSiteReturn(valuedReturnExits.getResultType(bindingContext))) + return Pair(ExpressionEvaluationWithCallSiteReturn(valuedReturnExits.getResultType(bindingContext)), null) } if (jumpExits.isNotEmpty()) { if (!jumpExits.checkEquivalence(true)) return multipleExitsError val elements = jumpExits.map { it.getElement() } - if (defaultExits.isNotEmpty()) return MaybeValue(ConditionalJump(elements, elements.first!!)) - return MaybeValue(UnconditionalJump(elements, elements.first!!)) + if (defaultExits.isNotEmpty()) return Pair(ConditionalJump(elements, elements.first!!), null) + return Pair(UnconditionalJump(elements, elements.first!!), null) } - return MaybeValue(DefaultControlFlow) + return Pair(DefaultControlFlow, null) } private fun ExtractionData.createTemporaryCodeBlock(): JetBlockExpression { @@ -306,7 +308,7 @@ private fun ExtractionData.inferParametersInfo( replacementMap: MutableMap, parameters: MutableSet, typeParameters: MutableSet -): MaybeError? { +): String? { val varNameValidator = JetNameValidatorImpl( commonParent.getParentByType(javaClass()), originalElements.first, @@ -324,7 +326,7 @@ private fun ExtractionData.inferParametersInfo( val selector = (ref.getParent() as? JetCallExpression) ?: ref val superExpr = (selector.getParent() as? JetQualifiedExpression)?.getReceiverExpression() as? JetSuperExpression if (superExpr != null) { - return MaybeError(JetRefactoringBundle.message("cannot.extract.super.call")!!) + return JetRefactoringBundle.message("cannot.extract.super.call")!! } val receiverArgument = resolvedCall?.getReceiverArgument() @@ -343,7 +345,7 @@ private fun ExtractionData.inferParametersInfo( if (classObjectClassDescriptor != null) { if (!classObjectClassDescriptor.canBeReferencedViaImport()) { - return MaybeError(JetRefactoringBundle.message("cannot.extract.non.local.declaration.ref")!!) + return JetRefactoringBundle.message("cannot.extract.non.local.declaration.ref")!! } replacementMap[refInfo.offsetInBody] = FqNameReplacement(DescriptorUtils.getFqNameSafe(originalDescriptor)) @@ -367,6 +369,8 @@ private fun ExtractionData.inferParametersInfo( } else bindingContext[BindingContext.EXPRESSION_TYPE, originalRef] ?: DEFAULT_PARAMETER_TYPE + if (!parameterType.processTypeIfExtractable(bindingContext, typeParameters, nonDenotableTypes)) continue + val existingParameter = extractedDescriptorToParameter[descriptorToExtract] val parameter: Parameter = if (existingParameter != null) { @@ -431,7 +435,7 @@ private fun ExtractionData.inferParametersInfo( DescriptorRenderer.HTML.renderType(it) }.sort().makeString(separator = "\n") - return MaybeError("${JetRefactoringBundle.message("parameter.types.are.not.denotable")!!}\n$typeStr") + return "${JetRefactoringBundle.message("parameter.types.are.not.denotable")!!}\n$typeStr" } parameters.addAll(extractedDescriptorToParameter.values()) @@ -443,7 +447,7 @@ private fun ExtractionData.checkLocalDeclarationsWithNonLocalUsages( allInstructions: List, localInstructions: List, bindingContext: BindingContext -): MaybeError? { +): String? { // todo: non-locally used declaration can be turned into the output value val declarations = ArrayList() @@ -460,13 +464,13 @@ private fun ExtractionData.checkLocalDeclarationsWithNonLocalUsages( if (declarations.isNotEmpty()) { val localVarStr = declarations.map { it.getName()!! }.sort().makeString(separator = "\n") - return MaybeError("${JetRefactoringBundle.message("variables.are.used.outside.of.selected.code.fragment")!!}\n$localVarStr") + return "${JetRefactoringBundle.message("variables.are.used.outside.of.selected.code.fragment")!!}\n$localVarStr" } return null } -private fun ExtractionData.checkDeclarationsMovingOutOfScope(controlFlow: ControlFlow): MaybeError? { +private fun ExtractionData.checkDeclarationsMovingOutOfScope(controlFlow: ControlFlow): String? { val declarationsOutOfScope = HashSet() if (controlFlow is JumpBasedControlFlow) { controlFlow.elementToInsertAfterCall.accept( @@ -483,19 +487,23 @@ private fun ExtractionData.checkDeclarationsMovingOutOfScope(controlFlow: Contro if (declarationsOutOfScope.isNotEmpty()) { val declStr = declarationsOutOfScope.map { it.getName()!! }.sort().makeString(separator = "\n") - return MaybeError("${JetRefactoringBundle.message("declarations.will.move.out.of.scope")!!}\n$declStr") + return "${JetRefactoringBundle.message("declarations.will.move.out.of.scope")!!}\n$declStr" } return null } -fun ExtractionData.performAnalysis(): Maybe { - if (originalElements.empty) return MaybeError(JetRefactoringBundle.message("cannot.refactor.no.expresson")!!) +fun ExtractionData.performAnalysis(): AnalysisResult { + if (originalElements.empty) { + return AnalysisResult(null, Status.CRITICAL_ERROR, listOf(JetRefactoringBundle.message("cannot.refactor.no.expresson")!!)) + } val commonParent = PsiTreeUtil.findCommonParent(originalElements)!! val enclosingDeclaration = commonParent.getParentByType(javaClass()) - if (enclosingDeclaration == null) return MaybeError(JetRefactoringBundle.message("cannot.refactor.no.container")!!) + if (enclosingDeclaration == null) { + return AnalysisResult(null, Status.CRITICAL_ERROR, listOf(JetRefactoringBundle.message("cannot.refactor.no.container")!!)) + } val resolveSession = originalFile.getLazyResolveSession() val bindingContext = resolveSession.resolveToElement(enclosingDeclaration.getBodyExpression()) @@ -513,17 +521,17 @@ fun ExtractionData.performAnalysis(): Maybe { val parameterError = inferParametersInfo( commonParent, localInstructions, bindingContext, inferredResultType, replacementMap, parameters, typeParameters ) - if (parameterError != null) return parameterError + if (parameterError != null) { + return AnalysisResult(null, Status.CRITICAL_ERROR, listOf(parameterError)) + } - val controlFlowInfo = localInstructions.analyzeControlFlow(bindingContext, parameters, inferredResultType) - if (controlFlowInfo is MaybeError) return MaybeError(controlFlowInfo.error) - val controlFlow = (controlFlowInfo as MaybeValue).value + val messages = ArrayList() - val localDeclError = checkLocalDeclarationsWithNonLocalUsages(pseudocode.getInstructions(), localInstructions, bindingContext) - if (localDeclError != null) return localDeclError + val (controlFlow, controlFlowMessage) = localInstructions.analyzeControlFlow(bindingContext, parameters, inferredResultType) + controlFlowMessage?.let { messages.add(it) } - val outOfScopeError = checkDeclarationsMovingOutOfScope(controlFlow) - if (outOfScopeError != null) return outOfScopeError + checkLocalDeclarationsWithNonLocalUsages(pseudocode.getInstructions(), localInstructions, bindingContext)?.let { messages.add(it) } + checkDeclarationsMovingOutOfScope(controlFlow)?.let { messages.add(it) } val functionNameValidator = JetNameValidatorImpl( nextSibling.getParent(), @@ -536,7 +544,7 @@ fun ExtractionData.performAnalysis(): Maybe { val receiverParameter = if (receiverCandidates.size == 1) receiverCandidates.first() else null receiverParameter?.let { parameters.remove(it) } - return MaybeValue( + return AnalysisResult( ExtractionDescriptor( this, functionName, @@ -546,7 +554,9 @@ fun ExtractionData.performAnalysis(): Maybe { typeParameters.sortBy { it.originalDeclaration.getName()!! }, replacementMap, controlFlow - ) + ), + if (messages.empty) Status.SUCCESS else Status.NON_CRITICAL_ERROR, + messages ) }