From c72622c6d7811873da7ffb09daf64197a0d9aa32 Mon Sep 17 00:00:00 2001 From: Toshiaki Kameyama Date: Wed, 27 Nov 2019 22:08:54 +0900 Subject: [PATCH] KT-32551 New J2K: Non-canonical modifiers order inspection is not applied during convertion of inner super class (#2806) New J2K: add SortModifiers inspection to post-processing #KT-32551 Fixed --- .../inspections/SortModifiersInspection.kt | 102 ++++++++---------- .../sortModifiers/annotation.kt | 1 + .../sortModifiers/annotation.kt.after | 1 + .../sortModifiers/annotationGroup.kt | 1 + .../sortModifiers/annotationGroup.kt.after | 1 + .../inspectionsLocal/sortModifiers/simple.kt | 2 + .../sortModifiers/simple.kt.after | 2 + .../nj2k/postProcessing/J2kPostProcessor.kt | 3 +- .../newJ2k/postProcessing/SortModifiers.java | 4 + .../newJ2k/postProcessing/SortModifiers.kt | 4 + ...otlinConverterSingleFileTestGenerated.java | 5 + 11 files changed, 69 insertions(+), 57 deletions(-) create mode 100644 nj2k/testData/newJ2k/postProcessing/SortModifiers.java create mode 100644 nj2k/testData/newJ2k/postProcessing/SortModifiers.kt diff --git a/idea/src/org/jetbrains/kotlin/idea/inspections/SortModifiersInspection.kt b/idea/src/org/jetbrains/kotlin/idea/inspections/SortModifiersInspection.kt index 506b1c1abf6..9a22f94d097 100644 --- a/idea/src/org/jetbrains/kotlin/idea/inspections/SortModifiersInspection.kt +++ b/idea/src/org/jetbrains/kotlin/idea/inspections/SortModifiersInspection.kt @@ -5,75 +5,48 @@ package org.jetbrains.kotlin.idea.inspections -import com.intellij.codeInspection.* +import com.intellij.codeInspection.CleanupLocalInspectionTool +import com.intellij.openapi.editor.Editor import com.intellij.openapi.project.Project import com.intellij.openapi.util.TextRange -import com.intellij.psi.PsiElementVisitor import org.jetbrains.kotlin.lexer.KtModifierKeywordToken import org.jetbrains.kotlin.lexer.KtTokens -import org.jetbrains.kotlin.psi.* +import org.jetbrains.kotlin.psi.KtAnnotation +import org.jetbrains.kotlin.psi.KtAnnotationEntry +import org.jetbrains.kotlin.psi.KtModifierList +import org.jetbrains.kotlin.psi.KtModifierListOwner import org.jetbrains.kotlin.psi.addRemoveModifier.sortModifiers import org.jetbrains.kotlin.psi.psiUtil.allChildren import org.jetbrains.kotlin.psi.psiUtil.endOffset import org.jetbrains.kotlin.psi.psiUtil.startOffset -class SortModifiersInspection : AbstractKotlinInspection(), CleanupLocalInspectionTool { - - override fun buildVisitor(holder: ProblemsHolder, isOnTheFly: Boolean, session: LocalInspectionToolSession): PsiElementVisitor { - return object : KtVisitorVoid() { - override fun visitModifierList(list: KtModifierList) { - super.visitModifierList(list) - - val modifierElements = list.allChildren.toList() - var modifiersBeforeAnnotations = false - var seenModifiers = false - for (modifierElement in modifierElements) { - if (modifierElement.node.elementType is KtModifierKeywordToken) { - seenModifiers = true - } else if (seenModifiers && (modifierElement is KtAnnotationEntry || modifierElement is KtAnnotation)) { - modifiersBeforeAnnotations = true - } - } - - val modifiers = modifierElements.asSequence().mapNotNull { it.node.elementType as? KtModifierKeywordToken }.toList() - if (modifiers.isEmpty()) return - - val startElement = modifierElements.firstOrNull { it.node.elementType is KtModifierKeywordToken } ?: return - val endElement = modifierElements.lastOrNull { it.node.elementType is KtModifierKeywordToken } ?: return - val rangeInElement = TextRange(startElement.startOffset, endElement.endOffset).shiftLeft(list.startOffset) - - val sortedModifiers = sortModifiers(modifiers) - if (modifiers == sortedModifiers && !modifiersBeforeAnnotations) return - - val message = if (modifiersBeforeAnnotations) - "Modifiers should follow annotations" - else - "Non-canonical modifiers order" - - val descriptor = holder.manager.createProblemDescriptor( - list, - rangeInElement, - message, - ProblemHighlightType.GENERIC_ERROR_OR_WARNING, - isOnTheFly, - SortModifiersFix(sortedModifiers) - ) - holder.registerProblem(descriptor) - } - } +class SortModifiersInspection : AbstractApplicabilityBasedInspection( + KtModifierList::class.java +), CleanupLocalInspectionTool { + override fun isApplicable(element: KtModifierList): Boolean { + val modifiers = element.modifierKeywordTokens() + if (modifiers.isEmpty()) return false + val sortedModifiers = sortModifiers(modifiers) + if (modifiers == sortedModifiers && !element.modifiersBeforeAnnotations()) return false + return true } -} -private class SortModifiersFix(private val modifiers: List) : LocalQuickFix { - override fun getName() = "Sort modifiers" + override fun inspectionHighlightRangeInElement(element: KtModifierList): TextRange? { + val modifierElements = element.allChildren.toList() + val startElement = modifierElements.firstOrNull { it.node.elementType is KtModifierKeywordToken } ?: return null + val endElement = modifierElements.lastOrNull { it.node.elementType is KtModifierKeywordToken } ?: return null + return TextRange(startElement.startOffset, endElement.endOffset).shiftLeft(element.startOffset) + } - override fun getFamilyName() = name + override fun inspectionText(element: KtModifierList) = + if (element.modifiersBeforeAnnotations()) "Modifiers should follow annotations" else "Non-canonical modifiers order" - override fun applyFix(project: Project, descriptor: ProblemDescriptor) { - val list = descriptor.psiElement as? KtModifierList ?: return - val owner = list.parent as? KtModifierListOwner ?: return + override val defaultFixText = "Sort modifiers" - val existingModifiers = modifiers.filter { owner.hasModifier(it) } + override fun applyTo(element: KtModifierList, project: Project, editor: Editor?) { + val owner = element.parent as? KtModifierListOwner ?: return + val sortedModifiers = sortModifiers(element.modifierKeywordTokens()) + val existingModifiers = sortedModifiers.filter { owner.hasModifier(it) } existingModifiers.forEach { owner.removeModifier(it) } // We add visibility / modality modifiers after all others, // because they can be redundant or not depending on others (e.g. override) @@ -82,5 +55,22 @@ private class SortModifiersFix(private val modifiers: List { + return allChildren.mapNotNull { it.node.elementType as? KtModifierKeywordToken }.toList() + } + + private fun KtModifierList.modifiersBeforeAnnotations(): Boolean { + val modifierElements = this.allChildren.toList() + var modifiersBeforeAnnotations = false + var seenModifiers = false + for (modifierElement in modifierElements) { + if (modifierElement.node.elementType is KtModifierKeywordToken) { + seenModifiers = true + } else if (seenModifiers && (modifierElement is KtAnnotationEntry || modifierElement is KtAnnotation)) { + modifiersBeforeAnnotations = true + } + } + return modifiersBeforeAnnotations + } +} diff --git a/idea/testData/inspectionsLocal/sortModifiers/annotation.kt b/idea/testData/inspectionsLocal/sortModifiers/annotation.kt index 1e3b1adac7e..4944dd07044 100644 --- a/idea/testData/inspectionsLocal/sortModifiers/annotation.kt +++ b/idea/testData/inspectionsLocal/sortModifiers/annotation.kt @@ -1,3 +1,4 @@ +// PROBLEM: Modifiers should follow annotations annotation class Test public @Test /* this is a test */ class MyTest diff --git a/idea/testData/inspectionsLocal/sortModifiers/annotation.kt.after b/idea/testData/inspectionsLocal/sortModifiers/annotation.kt.after index 78aaed93693..d78907159cc 100644 --- a/idea/testData/inspectionsLocal/sortModifiers/annotation.kt.after +++ b/idea/testData/inspectionsLocal/sortModifiers/annotation.kt.after @@ -1,3 +1,4 @@ +// PROBLEM: Modifiers should follow annotations annotation class Test @Test diff --git a/idea/testData/inspectionsLocal/sortModifiers/annotationGroup.kt b/idea/testData/inspectionsLocal/sortModifiers/annotationGroup.kt index f80a6931cd6..ebd2c48fbe4 100644 --- a/idea/testData/inspectionsLocal/sortModifiers/annotationGroup.kt +++ b/idea/testData/inspectionsLocal/sortModifiers/annotationGroup.kt @@ -1,3 +1,4 @@ +// PROBLEM: Modifiers should follow annotations annotation class Inject annotation class VisibleForTesting diff --git a/idea/testData/inspectionsLocal/sortModifiers/annotationGroup.kt.after b/idea/testData/inspectionsLocal/sortModifiers/annotationGroup.kt.after index 410a211af90..a12c8648adb 100644 --- a/idea/testData/inspectionsLocal/sortModifiers/annotationGroup.kt.after +++ b/idea/testData/inspectionsLocal/sortModifiers/annotationGroup.kt.after @@ -1,3 +1,4 @@ +// PROBLEM: Modifiers should follow annotations annotation class Inject annotation class VisibleForTesting diff --git a/idea/testData/inspectionsLocal/sortModifiers/simple.kt b/idea/testData/inspectionsLocal/sortModifiers/simple.kt index 92bd0c9ffe6..483b134f18f 100644 --- a/idea/testData/inspectionsLocal/sortModifiers/simple.kt +++ b/idea/testData/inspectionsLocal/sortModifiers/simple.kt @@ -1 +1,3 @@ +// PROBLEM: Non-canonical modifiers order +// FIX: Sort modifiers abstract public class Test diff --git a/idea/testData/inspectionsLocal/sortModifiers/simple.kt.after b/idea/testData/inspectionsLocal/sortModifiers/simple.kt.after index f50992fcb8e..2e458389a18 100644 --- a/idea/testData/inspectionsLocal/sortModifiers/simple.kt.after +++ b/idea/testData/inspectionsLocal/sortModifiers/simple.kt.after @@ -1 +1,3 @@ +// PROBLEM: Non-canonical modifiers order +// FIX: Sort modifiers public abstract class Test diff --git a/nj2k/nj2k-services/src/org/jetbrains/kotlin/nj2k/postProcessing/J2kPostProcessor.kt b/nj2k/nj2k-services/src/org/jetbrains/kotlin/nj2k/postProcessing/J2kPostProcessor.kt index f678e67a33c..b0c5debc7ca 100644 --- a/nj2k/nj2k-services/src/org/jetbrains/kotlin/nj2k/postProcessing/J2kPostProcessor.kt +++ b/nj2k/nj2k-services/src/org/jetbrains/kotlin/nj2k/postProcessing/J2kPostProcessor.kt @@ -220,7 +220,8 @@ private val inspectionLikePostProcessingGroup = MayBeConstantInspectionBasedProcessing(), RemoveForExpressionLoopParameterTypeProcessing(), intentionBasedProcessing(ReplaceMapGetOrDefaultIntention()), - inspectionBasedProcessing(ReplaceGuardClauseWithFunctionCallInspection()) + inspectionBasedProcessing(ReplaceGuardClauseWithFunctionCallInspection()), + inspectionBasedProcessing(SortModifiersInspection()) ) diff --git a/nj2k/testData/newJ2k/postProcessing/SortModifiers.java b/nj2k/testData/newJ2k/postProcessing/SortModifiers.java new file mode 100644 index 00000000000..510603c5b59 --- /dev/null +++ b/nj2k/testData/newJ2k/postProcessing/SortModifiers.java @@ -0,0 +1,4 @@ +public class TestInnerClasses { + public class Base {} + public class Derived extends Base {} +} \ No newline at end of file diff --git a/nj2k/testData/newJ2k/postProcessing/SortModifiers.kt b/nj2k/testData/newJ2k/postProcessing/SortModifiers.kt new file mode 100644 index 00000000000..c4841d5f056 --- /dev/null +++ b/nj2k/testData/newJ2k/postProcessing/SortModifiers.kt @@ -0,0 +1,4 @@ +class TestInnerClasses { + open inner class Base + inner class Derived : Base() +} \ No newline at end of file diff --git a/nj2k/tests/org/jetbrains/kotlin/nj2k/NewJavaToKotlinConverterSingleFileTestGenerated.java b/nj2k/tests/org/jetbrains/kotlin/nj2k/NewJavaToKotlinConverterSingleFileTestGenerated.java index 8ea52259f75..592d318f4b6 100644 --- a/nj2k/tests/org/jetbrains/kotlin/nj2k/NewJavaToKotlinConverterSingleFileTestGenerated.java +++ b/nj2k/tests/org/jetbrains/kotlin/nj2k/NewJavaToKotlinConverterSingleFileTestGenerated.java @@ -4226,6 +4226,11 @@ public class NewJavaToKotlinConverterSingleFileTestGenerated extends AbstractNew runTest("nj2k/testData/newJ2k/postProcessing/RedunduntTypeCastAndProhibitedInline.java"); } + @TestMetadata("SortModifiers.java") + public void testSortModifiers() throws Exception { + runTest("nj2k/testData/newJ2k/postProcessing/SortModifiers.java"); + } + @TestMetadata("SyntheticExtensionPropertyAccess.java") public void testSyntheticExtensionPropertyAccess() throws Exception { runTest("nj2k/testData/newJ2k/postProcessing/SyntheticExtensionPropertyAccess.java");