Extract Function: Improve rendering of error messages

idea/src/org/jetbrains/jet/plugin/debugger/evaluate/extractFunctionForDebuggerUtil.kt

@@ -47,8 +47,7 @@ fun getFunctionForExtractedFragment(
 ): JetNamedFunction? {
 
     fun getErrorMessageForExtractFunctionResult(analysisResult: AnalysisResult): String {
-        return analysisResult.messages.map {
+        return analysisResult.messages.map { errorMessage ->
-            errorMessage ->
             val message = when(errorMessage) {
                 ErrorMessage.NO_EXPRESSION -> "Cannot perform an action without an expression"
                 ErrorMessage.NO_CONTAINER -> "Cannot perform an action at this breakpoint ${breakpointFile.getName()}:${breakpointLine}"
@@ -60,8 +59,8 @@ fun getFunctionForExtractedFragment(
                 ErrorMessage.MULTIPLE_EXIT_POINTS,
                 ErrorMessage.DECLARATIONS_ARE_USED_OUTSIDE -> "Cannot perform an action for this expression"
             }
-            if (errorMessage.additionalInfo == null) message else "$message: ${errorMessage.additionalInfo?.makeString(", ")}"
+            errorMessage.additionalInfo?.let { "$message: ${it.joinToString(", ")}" } ?: message
-        }.makeString(", ")
+        }.joinToString(", ")
     }
 
     return ApplicationManager.getApplication()?.runReadAction(object: Computable<JetNamedFunction> {

idea/src/org/jetbrains/jet/plugin/refactoring/extractFunction/ExtractionDescriptor.kt

@@ -37,6 +37,7 @@ import org.jetbrains.jet.plugin.refactoring.extractFunction.AnalysisResult.Error
 import org.jetbrains.jet.lang.descriptors.DeclarationDescriptor
 import org.jetbrains.jet.lang.psi.JetProperty
 import org.jetbrains.jet.lang.psi.JetDeclaration
+import com.intellij.openapi.util.text.StringUtil
 
 trait Parameter {
     val argumentText: String
@@ -193,21 +194,21 @@ class AnalysisResult (
         }
 
         fun renderMessage(): String {
-            val message = JetRefactoringBundle.message(when(this) {
+            val message = JetRefactoringBundle.message(
-                NO_EXPRESSION -> "cannot.refactor.no.expression"
+                    when (this) {
-                NO_CONTAINER -> "cannot.refactor.no.container"
+                        NO_EXPRESSION -> "cannot.refactor.no.expression"
-                SUPER_CALL -> "cannot.extract.super.call"
+                        NO_CONTAINER -> "cannot.refactor.no.container"
-                DENOTABLE_TYPES -> "parameter.types.are.not.denotable"
+                        SUPER_CALL -> "cannot.extract.super.call"
-                MULTIPLE_OUTPUT -> "selected.code.fragment.has.multiple.output.values"
+                        DENOTABLE_TYPES -> "parameter.types.are.not.denotable"
-                OUTPUT_AND_EXIT_POINT -> "selected.code.fragment.has.output.values.and.exit.points"
+                        MULTIPLE_OUTPUT -> "selected.code.fragment.has.multiple.output.values"
-                MULTIPLE_EXIT_POINTS -> "selected.code.fragment.has.multiple.exit.points"
+                        OUTPUT_AND_EXIT_POINT -> "selected.code.fragment.has.output.values.and.exit.points"
-                DECLARATIONS_ARE_USED_OUTSIDE -> "declarations.are.used.outside.of.selected.code.fragment"
+                        MULTIPLE_EXIT_POINTS -> "selected.code.fragment.has.multiple.exit.points"
-                DECLARATIONS_OUT_OF_SCOPE -> "declarations.will.move.out.of.scope"
+                        DECLARATIONS_ARE_USED_OUTSIDE -> "declarations.are.used.outside.of.selected.code.fragment"
-            })!!
+                        DECLARATIONS_OUT_OF_SCOPE -> "declarations.will.move.out.of.scope"
-            if (additionalInfo != null) {
+                    }
-                return "$message\n${additionalInfo?.makeString("\n")}"
+            )
-            }
+
-            return message
+            return additionalInfo?.let { "$message\n\n${it.map { StringUtil.htmlEmphasize(it) }.joinToString("\n")}" } ?: message
         }
     }
 }

idea/src/org/jetbrains/jet/plugin/refactoring/extractFunction/extractFunctionUtils.kt

@@ -68,11 +68,21 @@ import org.jetbrains.jet.lang.resolve.OverridingUtil
 import org.jetbrains.jet.lang.psi.psiUtil.isAncestor
 import org.jetbrains.jet.plugin.intentions.declarations.DeclarationUtils
 import org.jetbrains.jet.lang.resolve.DescriptorToSourceUtils
+import com.intellij.openapi.util.text.StringUtil
 
 private val DEFAULT_FUNCTION_NAME = "myFun"
 private val DEFAULT_RETURN_TYPE = KotlinBuiltIns.getInstance().getUnitType()
 private val DEFAULT_PARAMETER_TYPE = KotlinBuiltIns.getInstance().getNullableAnyType()
 
+private fun DeclarationDescriptor.renderForMessage(): String =
+        DescriptorRenderer.SOURCE_CODE_SHORT_NAMES_IN_TYPES.render(this)
+
+private fun JetType.renderForMessage(): String =
+        DescriptorRenderer.SOURCE_CODE.renderType(this)
+
+private fun JetDeclaration.renderForMessage(bindingContext: BindingContext): String? =
+    bindingContext[BindingContext.DECLARATION_TO_DESCRIPTOR, this]?.renderForMessage()
+
 private fun JetType.isDefault(): Boolean = KotlinBuiltIns.getInstance().isUnit(this)
 
 private fun List<Instruction>.getModifiedVarDescriptors(bindingContext: BindingContext): Set<VariableDescriptor> {
@@ -203,7 +213,7 @@ private fun ExtractionData.analyzeControlFlow(
     val defaultControlFlow = DefaultControlFlow(if (returnValueType.isMeaningful()) returnValueType else typeOfDefaultFlow, declarationsToCopy)
 
     if (declarationsToReport.isNotEmpty()) {
-        val localVarStr = declarationsToReport.map { it.getName()!! }.sort()
+        val localVarStr = declarationsToReport.map { it.renderForMessage(bindingContext)!! }.distinct().sort()
         return Pair(defaultControlFlow, ErrorMessage.DECLARATIONS_ARE_USED_OUTSIDE.addAdditionalInfo(localVarStr))
     }
 
@@ -214,7 +224,9 @@ private fun ExtractionData.analyzeControlFlow(
     val outValuesCount = outDeclarations.size + outParameters.size
     when {
         outValuesCount > 1 -> {
-            val outValuesStr = (outParameters.map { it.argumentText } + outDeclarations.map { it.getName()!! }).sort()
+            val outValuesStr =
+                    (outParameters.map { it.originalDescriptor.renderForMessage() }
+                            + outDeclarations.map { it.renderForMessage(bindingContext)!! }).sort()
             return Pair(defaultControlFlow, ErrorMessage.MULTIPLE_OUTPUT.addAdditionalInfo(outValuesStr))
         }
 
@@ -532,7 +544,8 @@ private fun ExtractionData.inferParametersInfo(
 
 private fun ExtractionData.checkDeclarationsMovingOutOfScope(
         enclosingDeclaration: JetDeclaration,
-        controlFlow: ControlFlow
+        controlFlow: ControlFlow,
+        bindingContext: BindingContext
 ): ErrorMessage? {
     val declarationsOutOfScope = HashSet<JetNamedDeclaration>()
     if (controlFlow is JumpBasedControlFlow) {
@@ -551,7 +564,7 @@ private fun ExtractionData.checkDeclarationsMovingOutOfScope(
     }
 
     if (declarationsOutOfScope.isNotEmpty()) {
-        val declStr = declarationsOutOfScope.map { it.getName()!! }.sort()
+        val declStr = declarationsOutOfScope.map { it.renderForMessage(bindingContext)!! }.sort()
         return ErrorMessage.DECLARATIONS_OUT_OF_SCOPE.addAdditionalInfo(declStr)
     }
 
@@ -614,7 +627,7 @@ fun ExtractionData.performAnalysis(): AnalysisResult {
     controlFlow.returnType.processTypeIfExtractable(paramsInfo.typeParameters, paramsInfo.nonDenotableTypes)
 
     if (paramsInfo.nonDenotableTypes.isNotEmpty()) {
-        val typeStr = paramsInfo.nonDenotableTypes.map {DescriptorRenderer.HTML.renderType(it)}.sort()
+        val typeStr = paramsInfo.nonDenotableTypes.map {it.renderForMessage()}.sort()
         return AnalysisResult(
                 null,
                 Status.CRITICAL_ERROR,
@@ -622,7 +635,7 @@ fun ExtractionData.performAnalysis(): AnalysisResult {
         )
     }
 
-    checkDeclarationsMovingOutOfScope(enclosingDeclaration!!, controlFlow)?.let { messages.add(it) }
+    checkDeclarationsMovingOutOfScope(enclosingDeclaration!!, controlFlow, bindingContext)?.let { messages.add(it) }
 
     val functionNameValidator =
             JetNameValidatorImpl(
@@ -689,9 +702,9 @@ fun ExtractionDescriptor.validate(): ExtractionDescriptorWithConflicts {
         && parameters.any { it.mirrorVarName == currentDescriptor.getName().asString() }) continue
 
         if (diagnostics.any { it.getFactory() == Errors.UNRESOLVED_REFERENCE }
-        || (currentDescriptor != null
+                || (currentDescriptor != null
-        && !ErrorUtils.isError(currentDescriptor)
+                && !ErrorUtils.isError(currentDescriptor)
-        && !comparePossiblyOverridingDescriptors(currentDescriptor, resolveResult.descriptor))) {
+                && !comparePossiblyOverridingDescriptors(currentDescriptor, resolveResult.descriptor))) {
             conflicts.putValue(
                     resolveResult.originalRefExpr,
                     JetRefactoringBundle.message(

idea/testData/debugger/tinyApp/src/evaluate/singleBreakpoint/errors.kt.fragment

@@ -2,4 +2,4 @@ prop += 1
 prop2 +=2
 prop + prop2
 
-// RESULT: Cannot perform an action because this code fragment changes more than one variable: prop, prop2
+// RESULT: Cannot perform an action because this code fragment changes more than one variable: var prop2: Int, var prop: Int

idea/testData/debugger/tinyApp/src/evaluate/singleBreakpoint/errors.kt.fragment2

@@ -1,3 +1,3 @@
 object {}
 
-// RESULT: Cannot perform an action because following types are unavailable from debugger scope: errors.MyClass.baseFun.<no name provided>
+// RESULT: Cannot perform an action because following types are unavailable from debugger scope: errors.MyClass.baseFun.<no name provided>

idea/testData/refactoring/extractFunction/basic/localClassFunctionRef.kt.conflicts

@@ -1 +1 @@
-Cannot extract method since following types are not denotable in the target scope: foo.A
+Cannot extract method since following types are not denotable in the target scope:  foo.A

idea/testData/refactoring/extractFunction/basic/localClassPropertyRef.kt.conflicts

@@ -1 +1 @@
-Cannot extract method since following types are not denotable in the target scope: foo.A
+Cannot extract method since following types are not denotable in the target scope:  foo.A

idea/testData/refactoring/extractFunction/basic/localObjectRef.kt.conflicts

@@ -1 +1 @@
-Cannot extract method since following types are not denotable in the target scope: foo.A
+Cannot extract method since following types are not denotable in the target scope:  foo.A

idea/testData/refactoring/extractFunction/controlFlow/outputValues/multipleOutputValues.kt.conflicts

@@ -1 +1 @@
-Selected code fragment has multiple output values: b c
+Selected code fragment has multiple output values:  var b: Int var c: Int

idea/testData/refactoring/extractFunction/controlFlow/outputValues/multipleOutputValuesWithIf.kt.conflicts

@@ -1 +1 @@
-Selected code fragment has multiple output values: b c
+Selected code fragment has multiple output values:  var b: Int var c: Int

idea/testData/refactoring/extractFunction/controlFlow/outputValues/multipleOutputValuesWithWhen.kt.conflicts

@@ -1 +1 @@
-Selected code fragment has multiple output values: b c
+Selected code fragment has multiple output values:  var b: Int var c: Int

idea/testData/refactoring/extractFunction/controlFlow/unextractable/anonymousObject.kt.conflicts

@@ -1 +1 @@
-Cannot extract method since following types are not denotable in the target scope: foo.<no name provided>
+Cannot extract method since following types are not denotable in the target scope:  foo.<no name provided>

idea/testData/refactoring/extractFunction/controlFlow/unextractable/multipleInitalizersWithNonLocalUsages.kt.conflicts

@@ -1 +1 @@
-Selected code fragment has multiple output values: a b
+Selected code fragment has multiple output values:  val a: Int val b: Int

idea/testData/refactoring/extractFunction/controlFlow/unextractable/variablesOutOfScope.kt.conflicts

@@ -1 +1 @@
-Following declarations won't be available outside of extracted function body: x
+Following declarations won't be available outside of extracted function body:  val x: Int

idea/testData/refactoring/extractFunction/initializers/functions/localFunctionParameters.kt.conflicts

@@ -1 +1 @@
-Cannot extract method since following types are not denotable in the target scope: [ERROR : ]
+Cannot extract method since following types are not denotable in the target scope:  [ERROR : <ERROR FUNCTION RETURN TYPE>]

idea/testData/refactoring/extractFunction/initializers/functions/localFunctionParametersWithLambda.kt.conflicts

@@ -1 +1 @@
-Cannot extract method since following types are not denotable in the target scope: [ERROR : ]
+Cannot extract method since following types are not denotable in the target scope:  [ERROR : <ERROR FUNCTION RETURN TYPE>]

idea/testData/refactoring/extractFunction/parameters/nonDenotableTypes/anonymousObject.kt.conflicts

@@ -1 +1 @@
-Cannot extract method since following types are not denotable in the target scope: foo.<no name provided>
+Cannot extract method since following types are not denotable in the target scope:  foo.<no name provided>

idea/testData/refactoring/extractFunction/parameters/nonDenotableTypes/localClass.kt.conflicts

@@ -1 +1 @@
-Cannot extract method since following types are not denotable in the target scope: foo.T
+Cannot extract method since following types are not denotable in the target scope:  foo.T

idea/testData/refactoring/extractFunction/typeParameters/localClassInBound.kt.conflicts

@@ -1 +1 @@
-Cannot extract method since following types are not denotable in the target scope: foo.X
+Cannot extract method since following types are not denotable in the target scope:  foo.X

idea/testData/refactoring/extractFunction/typeParameters/localClassInTypeConstraint.kt.conflicts

@@ -1 +1 @@
-Cannot extract method since following types are not denotable in the target scope: foo.X
+Cannot extract method since following types are not denotable in the target scope:  foo.X