From a7519853cb816419f3e3fdaedc6098ab6725032c Mon Sep 17 00:00:00 2001 From: Nikolay Krasko Date: Mon, 19 Oct 2015 15:20:18 +0300 Subject: [PATCH] Fixes after review in creating single action for fixing several problems --- .../kotlin/idea/highlighter/JetPsiChecker.kt | 412 +++++++++--------- .../quickfix/JetIntentionActionsFactory.kt | 22 +- .../inspections/KotlinCleanupInspection.kt | 2 +- 3 files changed, 215 insertions(+), 221 deletions(-) diff --git a/idea/idea-analysis/src/org/jetbrains/kotlin/idea/highlighter/JetPsiChecker.kt b/idea/idea-analysis/src/org/jetbrains/kotlin/idea/highlighter/JetPsiChecker.kt index 15b98e0356c..99b940e6085 100644 --- a/idea/idea-analysis/src/org/jetbrains/kotlin/idea/highlighter/JetPsiChecker.kt +++ b/idea/idea-analysis/src/org/jetbrains/kotlin/idea/highlighter/JetPsiChecker.kt @@ -32,6 +32,7 @@ import com.intellij.openapi.util.TextRange import com.intellij.psi.MultiRangeReference import com.intellij.psi.PsiElement import com.intellij.psi.PsiFile +import com.intellij.util.containers.MultiMap import com.intellij.xml.util.XmlStringUtil import org.jetbrains.kotlin.descriptors.DeclarationDescriptor import org.jetbrains.kotlin.diagnostics.Diagnostic @@ -40,7 +41,6 @@ import org.jetbrains.kotlin.diagnostics.Severity import org.jetbrains.kotlin.diagnostics.rendering.DefaultErrorMessages import org.jetbrains.kotlin.idea.actions.internal.KotlinInternalMode import org.jetbrains.kotlin.idea.caches.resolve.analyzeFullyAndGetResult -import org.jetbrains.kotlin.idea.quickfix.JetIntentionActionsFactory import org.jetbrains.kotlin.idea.quickfix.QuickFixes import org.jetbrains.kotlin.idea.references.mainReference import org.jetbrains.kotlin.idea.util.ProjectRootsUtil @@ -50,6 +50,7 @@ import org.jetbrains.kotlin.psi.KtParameter import org.jetbrains.kotlin.psi.KtReferenceExpression import org.jetbrains.kotlin.resolve.BindingContext import org.jetbrains.kotlin.resolve.diagnostics.Diagnostics +import org.jetbrains.kotlin.utils.singletonOrEmptyList import java.lang.reflect.* import java.util.* @@ -80,184 +81,11 @@ public open class JetPsiChecker : Annotator, HighlightRangeExtension { fun annotateElement(element: PsiElement, holder: AnnotationHolder, diagnostics: Diagnostics) { if (ProjectRootsUtil.isInProjectSource(element) || element.getContainingFile() is KtCodeFragment) { - ElementAnnotator(element, holder).registerDiagnosticsAnnotations(diagnostics.forElement(element)) - } - } - - private inner class ElementAnnotator(private val element: PsiElement, private val holder: AnnotationHolder) { - fun registerDiagnosticsAnnotations(diagnostics: Collection) { - diagnostics.groupBy { diagnostic -> diagnostic.factory }.forEach { group -> registerDiagnosticAnnotations(group.getValue()) } - } - - private fun registerDiagnosticAnnotations(diagnostics: List) { - assert(diagnostics.isNotEmpty()) - - val validDiagnostics = diagnostics.filter { it.isValid } - if (validDiagnostics.isEmpty()) return - - val diagnostic = diagnostics.first() - val factory = diagnostic.getFactory() - - assert(diagnostics.all { it.getPsiElement() == element && it.factory == factory }) - - val presentationInfo: AnnotationPresentationInfo = when (factory.severity) { - Severity.ERROR -> { - when (factory) { - in Errors.UNRESOLVED_REFERENCE_DIAGNOSTICS -> { - val referenceExpression = element as KtReferenceExpression - val reference = referenceExpression.mainReference - if (reference is MultiRangeReference) { - AnnotationPresentationInfo(diagnostic, - ranges = reference.getRanges().map { it.shiftRight(referenceExpression.getTextOffset()) }, - highlightType = ProblemHighlightType.LIKE_UNKNOWN_SYMBOL) - } - else { - AnnotationPresentationInfo(diagnostic, highlightType = ProblemHighlightType.LIKE_UNKNOWN_SYMBOL) - } - } - - Errors.ILLEGAL_ESCAPE -> AnnotationPresentationInfo(diagnostic, textAttributes = JetHighlightingColors.INVALID_STRING_ESCAPE) - - Errors.REDECLARATION -> AnnotationPresentationInfo( - diagnostic, ranges = listOf(diagnostic.getTextRanges().first()), defaultMessage = "") - - else -> { - AnnotationPresentationInfo(diagnostic, - highlightType = if (factory == Errors.INVISIBLE_REFERENCE) - ProblemHighlightType.LIKE_UNKNOWN_SYMBOL - else - null) - } - } - } - Severity.WARNING -> { - if (factory == Errors.UNUSED_PARAMETER && shouldSuppressUnusedParameter(element as KtParameter)) { - return - } - - AnnotationPresentationInfo(diagnostic, - textAttributes = if (factory == Errors.DEPRECATION) CodeInsightColors.DEPRECATED_ATTRIBUTES else null, - highlightType = if (factory in Errors.UNUSED_ELEMENT_DIAGNOSTICS) - ProblemHighlightType.LIKE_UNUSED_SYMBOL - else - null - ) - } - Severity.INFO -> return // Do nothing - } - - setUpAnnotations(diagnostics, presentationInfo) - } - - private fun setUpAnnotations(diagnostics: List, data: AnnotationPresentationInfo) { - for (range in data.ranges) { - registerQuickFixes(diagnostics, range, data) - } - } - - fun registerQuickFixes(diagnostics: List, range: TextRange, data: AnnotationPresentationInfo) { - val (multiFixes, processedFactories) = createQuickFixesForSameTypeDiagnostics(diagnostics) - - val annotations = diagnostics.map { diagnostic -> - val annotation = data.create(range, holder) - - createQuickfixes(diagnostic, processedFactories).forEach { annotation.registerFix(it) } - - // Making warnings suppressable - if (diagnostic.getSeverity() == Severity.WARNING) { - annotation.setProblemGroup(KotlinSuppressableWarningProblemGroup(diagnostic.getFactory())) - - val fixes = annotation.getQuickFixes() - if (fixes == null || (fixes.isEmpty() && multiFixes.isEmpty())) { - // if there are no quick fixes we need to register an EmptyIntentionAction to enable 'suppress' actions - annotation.registerFix(EmptyIntentionAction(diagnostic.getFactory().getName())) - } - } - - annotation - } - - // Always register all group fixes on the same annotation - val firstAnnotation = annotations.minBy { it.message }!! - multiFixes.forEach { fix -> firstAnnotation.registerFix(fix) } - } - - private fun createQuickFixesForSameTypeDiagnostics(diagnostics: List): - Pair, Collection> { - val factory = diagnostics.first().factory - val sameProblemsFixesFactories = QuickFixes.getInstance().getActionFactories(factory).filter { it.canFixSeveralSameProblems() } - - val processedFactories = hashSetOf() - val fixActions = arrayListOf() - - for (actionFactory in sameProblemsFixesFactories) { - val actions = actionFactory.createActions(diagnostics) - if (actions.isNotEmpty()) { - processedFactories.add(actionFactory) - fixActions.addAll(actions) - } - } - - return Pair(fixActions, processedFactories) + ElementAnnotator(element, holder, { param -> shouldSuppressUnusedParameter(param) }).registerDiagnosticsAnnotations(diagnostics.forElement(element)) } } companion object { - private fun getMessage(diagnostic: Diagnostic): String { - var message = IdeErrorMessages.render(diagnostic) - if (KotlinInternalMode.enabled || ApplicationManager.getApplication().isUnitTestMode()) { - val factoryName = diagnostic.getFactory().getName() - if (message.startsWith("")) { - message = "[$factoryName] ${message.substring("".length())}" - } - else { - message = "[$factoryName] $message" - } - } - if (!message.startsWith("")) { - message = "${XmlStringUtil.escapeString(message)}" - } - return message - } - - private fun getDefaultMessage(diagnostic: Diagnostic): String { - val message = DefaultErrorMessages.render(diagnostic) - if (KotlinInternalMode.enabled || ApplicationManager.getApplication().isUnitTestMode()) { - return "[${diagnostic.getFactory().getName()}] $message" - } - return message - } - - class AnnotationPresentationInfo( - diagnostic: Diagnostic, - val severity: Severity = diagnostic.severity, - val ranges: List = diagnostic.textRanges, - val defaultMessage: String = getDefaultMessage(diagnostic), - val tooltip: String = getMessage(diagnostic), - val highlightType: ProblemHighlightType? = null, - val textAttributes: TextAttributesKey? = null) { - - public fun create(range: TextRange, holder: AnnotationHolder): Annotation { - val annotation = when (severity) { - Severity.ERROR -> holder.createErrorAnnotation(range, defaultMessage) - Severity.WARNING -> holder.createWarningAnnotation(range, defaultMessage) - else -> throw IllegalArgumentException("Only ERROR and WARNING diagnostics are supported") - } - - annotation.tooltip = tooltip - - if (highlightType != null) { - annotation.highlightType = highlightType - } - - if (textAttributes != null) { - annotation.textAttributes = textAttributes - } - - return annotation - } - } - private fun getAfterAnalysisVisitor(holder: AnnotationHolder, bindingContext: BindingContext) = arrayOf( PropertiesHighlightingVisitor(holder, bindingContext), FunctionsHighlightingVisitor(holder, bindingContext), @@ -265,53 +93,219 @@ public open class JetPsiChecker : Annotator, HighlightRangeExtension { TypeKindHighlightingVisitor(holder, bindingContext) ) - public fun createQuickfixes( - diagnostic: Diagnostic, - excludedFactories : Collection = emptySet()): Collection { - val result = arrayListOf() - val intentionActionsFactories = QuickFixes.getInstance().getActionFactories(diagnostic.getFactory()) - excludedFactories - for (intentionActionsFactory in intentionActionsFactories.filterNotNull()) { - result.addAll(intentionActionsFactory.createActions(diagnostic)) + public fun createQuickFixes(diagnostic: Diagnostic): Collection = + createQuickFixes(diagnostic.singletonOrEmptyList())[diagnostic] + } +} + +private fun createQuickFixes(similarDiagnostics: Collection): MultiMap { + val first = similarDiagnostics.minBy { it.toString() } + val factory = similarDiagnostics.first().factory + + val actions = MultiMap() + + val intentionActionsFactories = QuickFixes.getInstance().getActionFactories(factory) + for (intentionActionsFactory in intentionActionsFactories.filterNotNull()) { + val allProblemsActions = intentionActionsFactory.createActionsForAllProblems(similarDiagnostics) + if (!allProblemsActions.isEmpty()) { + actions.putValues(first, allProblemsActions) + } + else { + for (diagnostic in similarDiagnostics) { + actions.putValues(diagnostic, intentionActionsFactory.createActions(diagnostic)) } - result.addAll(QuickFixes.getInstance().getActions(diagnostic.factory)) + } + } - result.forEach { check(it.javaClass) } + for (diagnostic in similarDiagnostics) { + actions.putValues(diagnostic, QuickFixes.getInstance().getActions(diagnostic.getFactory())) + } - return result + actions.values().forEach { NoDeclarationDescriptorsChecker.check(it.javaClass) } + + return actions +} + +private object NoDeclarationDescriptorsChecker { + private val LOG = Logger.getInstance(NoDeclarationDescriptorsChecker::class.java) + + private val checkedQuickFixClasses = Collections.synchronizedSet(HashSet>()) + + fun check(quickFixClass: Class<*>) { + if (!checkedQuickFixClasses.add(quickFixClass)) return + + for (field in quickFixClass.declaredFields) { + checkType(field.genericType, field) } - private val LOG = Logger.getInstance(JetPsiChecker::class.java) + @Suppress("UNNECESSARY_SAFE_CALL") // Wrong UNNECESSARY_SAFE_CALL + quickFixClass.superclass?.let { check(it) } + } - private val checkedQuickFixClasses = Collections.synchronizedSet(HashSet>()) - - private fun check(quickFixClass: Class<*>) { - if (!checkedQuickFixClasses.add(quickFixClass)) return - - for (field in quickFixClass.declaredFields) { - checkType(field.genericType, field) + private fun checkType(type: Type, field: Field) { + when (type) { + is Class<*> -> { + if (DeclarationDescriptor::class.java.isAssignableFrom(type)) { + LOG.error("QuickFix class ${field.declaringClass.name} contains field ${field.name} that holds DeclarationDescriptor") + } } - quickFixClass.superclass?.let { check(it) } + is GenericArrayType -> checkType(type.genericComponentType, field) + + is ParameterizedType -> { + if (Collection::class.java.isAssignableFrom(type.rawType as Class<*>)) { + type.actualTypeArguments.forEach { checkType(it, field) } + } + } + + is WildcardType -> type.upperBounds.forEach { checkType(it, field) } + } + } +} + +private class ElementAnnotator(private val element: PsiElement, + private val holder: AnnotationHolder, + private val shouldSuppressUnusedParameter: (KtParameter) -> Boolean) { + fun registerDiagnosticsAnnotations(diagnostics: Collection) { + diagnostics.groupBy { it.factory }.forEach { group -> registerDiagnosticAnnotations(group.value) } + } + + private fun registerDiagnosticAnnotations(diagnostics: List) { + assert(diagnostics.isNotEmpty()) + + val validDiagnostics = diagnostics.filter { it.isValid } + if (validDiagnostics.isEmpty()) return + + val diagnostic = diagnostics.first() + val factory = diagnostic.getFactory() + + assert(diagnostics.all { it.getPsiElement() == element && it.factory == factory }) + + val ranges = diagnostic.textRanges + + val presentationInfo: AnnotationPresentationInfo = when (factory.severity) { + Severity.ERROR -> { + when (factory) { + in Errors.UNRESOLVED_REFERENCE_DIAGNOSTICS -> { + val referenceExpression = element as KtReferenceExpression + val reference = referenceExpression.mainReference + if (reference is MultiRangeReference) { + AnnotationPresentationInfo( + ranges = reference.getRanges().map { it.shiftRight(referenceExpression.getTextOffset()) }, + highlightType = ProblemHighlightType.LIKE_UNKNOWN_SYMBOL) + } + else { + AnnotationPresentationInfo(ranges, highlightType = ProblemHighlightType.LIKE_UNKNOWN_SYMBOL) + } + } + + Errors.ILLEGAL_ESCAPE -> AnnotationPresentationInfo(ranges, textAttributes = JetHighlightingColors.INVALID_STRING_ESCAPE) + + Errors.REDECLARATION -> AnnotationPresentationInfo( + ranges = listOf(diagnostic.getTextRanges().first()), nonDefaultMessage = "") + + else -> { + AnnotationPresentationInfo( + ranges, + highlightType = if (factory == Errors.INVISIBLE_REFERENCE) + ProblemHighlightType.LIKE_UNKNOWN_SYMBOL + else + null) + } + } + } + Severity.WARNING -> { + if (factory == Errors.UNUSED_PARAMETER && shouldSuppressUnusedParameter(element as KtParameter)) { + return + } + + AnnotationPresentationInfo( + ranges, + textAttributes = if (factory == Errors.DEPRECATION) CodeInsightColors.DEPRECATED_ATTRIBUTES else null, + highlightType = if (factory in Errors.UNUSED_ELEMENT_DIAGNOSTICS) + ProblemHighlightType.LIKE_UNUSED_SYMBOL + else + null + ) + } + Severity.INFO -> return // Do nothing } - private fun checkType(type: Type, field: Field) { - when (type) { - is Class<*> -> { - if (DeclarationDescriptor::class.java.isAssignableFrom(type)) { - LOG.error("QuickFix class ${field.declaringClass.name} contains field ${field.name} that holds DeclarationDescriptor") + setUpAnnotations(diagnostics, presentationInfo) + } + + private fun setUpAnnotations(diagnostics: List, data: AnnotationPresentationInfo) { + val fixesMap = createQuickFixes(diagnostics) + for (range in data.ranges) { + for (diagnostic in diagnostics) { + val annotation = data.create(diagnostic, range, holder) + val fixes = fixesMap[diagnostic] + + fixes.forEach { annotation.registerFix(it) } + + if (diagnostic.getSeverity() == Severity.WARNING) { + annotation.setProblemGroup(KotlinSuppressableWarningProblemGroup(diagnostic.getFactory())) + + if (fixes.isEmpty()) { + // if there are no quick fixes we need to register an EmptyIntentionAction to enable 'suppress' actions + annotation.registerFix(EmptyIntentionAction(diagnostic.getFactory().getName())) } } - - is GenericArrayType -> checkType(type.genericComponentType, field) - - is ParameterizedType -> { - if (Collection::class.java.isAssignableFrom(type.rawType as Class<*>)) { - type.actualTypeArguments.forEach { checkType(it, field) } - } - } - - is WildcardType -> type.upperBounds.forEach { checkType(it, field) } } } } } + +private class AnnotationPresentationInfo( + val ranges: List, + val nonDefaultMessage: String? = null, + val highlightType: ProblemHighlightType? = null, + val textAttributes: TextAttributesKey? = null) { + + public fun create(diagnostic: Diagnostic, range: TextRange, holder: AnnotationHolder): Annotation { + val defaultMessage = nonDefaultMessage?: getDefaultMessage(diagnostic) + + val annotation = when (diagnostic.severity) { + Severity.ERROR -> holder.createErrorAnnotation(range, defaultMessage) + Severity.WARNING -> holder.createWarningAnnotation(range, defaultMessage) + else -> throw IllegalArgumentException("Only ERROR and WARNING diagnostics are supported") + } + + annotation.tooltip = getMessage(diagnostic) + + if (highlightType != null) { + annotation.highlightType = highlightType + } + + if (textAttributes != null) { + annotation.textAttributes = textAttributes + } + + return annotation + } + + private fun getMessage(diagnostic: Diagnostic): String { + var message = IdeErrorMessages.render(diagnostic) + if (KotlinInternalMode.enabled || ApplicationManager.getApplication().isUnitTestMode()) { + val factoryName = diagnostic.getFactory().getName() + if (message.startsWith("")) { + message = "[$factoryName] ${message.substring("".length)}" + } + else { + message = "[$factoryName] $message" + } + } + if (!message.startsWith("")) { + message = "${XmlStringUtil.escapeString(message)}" + } + return message + } + + private fun getDefaultMessage(diagnostic: Diagnostic): String { + val message = DefaultErrorMessages.render(diagnostic) + if (KotlinInternalMode.enabled || ApplicationManager.getApplication().isUnitTestMode()) { + return "[${diagnostic.getFactory().getName()}] $message" + } + return message + } +} diff --git a/idea/idea-analysis/src/org/jetbrains/kotlin/idea/quickfix/JetIntentionActionsFactory.kt b/idea/idea-analysis/src/org/jetbrains/kotlin/idea/quickfix/JetIntentionActionsFactory.kt index aca6e0000b4..b9735184d43 100644 --- a/idea/idea-analysis/src/org/jetbrains/kotlin/idea/quickfix/JetIntentionActionsFactory.kt +++ b/idea/idea-analysis/src/org/jetbrains/kotlin/idea/quickfix/JetIntentionActionsFactory.kt @@ -19,21 +19,23 @@ package org.jetbrains.kotlin.idea.quickfix import com.intellij.codeInsight.intention.IntentionAction import org.jetbrains.kotlin.diagnostics.Diagnostic import org.jetbrains.kotlin.psi.KtCodeFragment -import java.util.Collections +import org.jetbrains.kotlin.utils.singletonOrEmptyList -// TODO: Replace with trait when all subclasses are translated to Kotlin public abstract class JetIntentionActionsFactory { protected open fun isApplicableForCodeFragment(): Boolean = false protected abstract fun doCreateActions(diagnostic: Diagnostic): List - public open fun canFixSeveralSameProblems(): Boolean = false - protected open fun doCreateActions(@Suppress("UNUSED_PARAMETER") sameTypeDiagnostics: List): List = - throw NotImplementedError() + protected open fun doCreateActionsForAllProblems( + sameTypeDiagnostics: Collection): List = emptyList() - public fun createActions(diagnostic: Diagnostic) = createActions(listOf(diagnostic)) + public fun createActions(diagnostic: Diagnostic): List = + createActions(diagnostic.singletonOrEmptyList(), false) - public fun createActions(sameTypeDiagnostics: List): List { + public fun createActionsForAllProblems(sameTypeDiagnostics: Collection): List = + createActions(sameTypeDiagnostics, true) + + private fun createActions(sameTypeDiagnostics: Collection, createForAll: Boolean): List { if (sameTypeDiagnostics.isEmpty()) return emptyList() val first = sameTypeDiagnostics.first() @@ -41,14 +43,12 @@ public abstract class JetIntentionActionsFactory { return emptyList() } - if (sameTypeDiagnostics.size > 1) { + if (sameTypeDiagnostics.size > 1 && createForAll) { assert(sameTypeDiagnostics.all { it.psiElement == first.psiElement && it.factory == first.factory }) { "It's expected to be the list of diagnostics of same type and for same element" } - if (canFixSeveralSameProblems()) { - return doCreateActions(sameTypeDiagnostics) - } + return doCreateActionsForAllProblems(sameTypeDiagnostics) } return sameTypeDiagnostics.flatMapTo(arrayListOf()) { doCreateActions(it) } diff --git a/idea/src/org/jetbrains/kotlin/idea/inspections/KotlinCleanupInspection.kt b/idea/src/org/jetbrains/kotlin/idea/inspections/KotlinCleanupInspection.kt index 619f5997383..0143b8507b6 100644 --- a/idea/src/org/jetbrains/kotlin/idea/inspections/KotlinCleanupInspection.kt +++ b/idea/src/org/jetbrains/kotlin/idea/inspections/KotlinCleanupInspection.kt @@ -106,7 +106,7 @@ public class KotlinCleanupInspection(): LocalInspectionTool(), CleanupLocalInspe } private fun Diagnostic.toCleanupFixes(): Collection { - return JetPsiChecker.createQuickfixes(this).filterIsInstance() + return JetPsiChecker.createQuickFixes(this).filterIsInstance() } private class Wrapper(val intention: IntentionAction, file: KtFile) : IntentionWrapper(intention, file) {