From ef3ba5f0b1b73b47de19b15ec93b13066282a526 Mon Sep 17 00:00:00 2001 From: Mikhail Glukhikh Date: Tue, 17 Apr 2018 20:11:23 +0300 Subject: [PATCH] Test local inspections on the fly via highlighting Fixes problem with INFORMATION highlight type in batch mode (IDEA 181). It's no more required to report INFORMATIONs in unit tests in batch. --- .../inspections/AbstractKotlinInspection.kt | 3 +- .../inspectionData/expected.xml | 16 -- .../inspections/nullableBooleanElvis/test.kt | 4 +- .../inspectionData/expected.xml | 16 -- .../replaceCallWithComparison/test.kt | 4 +- .../nullableBooleanElvis/notInIf.kt | 2 + .../nullableBooleanElvis/notInIf.kt.after | 2 + .../nullableBooleanElvis/notInIfWithTrue.kt | 2 + .../notInIfWithTrue.kt.after | 2 + .../AbstractLocalInspectionTest.kt | 177 +++++++++++------- .../AbstractMultiFileLocalInspectionTest.kt | 6 +- 11 files changed, 122 insertions(+), 112 deletions(-) diff --git a/idea/idea-analysis/src/org/jetbrains/kotlin/idea/inspections/AbstractKotlinInspection.kt b/idea/idea-analysis/src/org/jetbrains/kotlin/idea/inspections/AbstractKotlinInspection.kt index 1b6cf0da7d4..40d14fc5cce 100644 --- a/idea/idea-analysis/src/org/jetbrains/kotlin/idea/inspections/AbstractKotlinInspection.kt +++ b/idea/idea-analysis/src/org/jetbrains/kotlin/idea/inspections/AbstractKotlinInspection.kt @@ -66,8 +66,7 @@ abstract class AbstractKotlinInspection: LocalInspectionTool(), CustomSuppressab range: TextRange?, vararg fixes: LocalQuickFix ) { - if (!ApplicationManager.getApplication().isUnitTestMode && - !isOnTheFly && highlightType == ProblemHighlightType.INFORMATION) return + if (!isOnTheFly && highlightType == ProblemHighlightType.INFORMATION) return val problemDescriptor = manager.createProblemDescriptor(element, range, description, highlightType, isOnTheFly, *fixes) registerProblem(problemDescriptor) } diff --git a/idea/testData/inspections/nullableBooleanElvis/inspectionData/expected.xml b/idea/testData/inspections/nullableBooleanElvis/inspectionData/expected.xml index a02ffb07bb0..f8700010bec 100644 --- a/idea/testData/inspections/nullableBooleanElvis/inspectionData/expected.xml +++ b/idea/testData/inspections/nullableBooleanElvis/inspectionData/expected.xml @@ -31,21 +31,5 @@ Equality check can be used instead of elvis for nullable boolean check Equality check should be used instead of elvis for nullable boolean check - - test.kt - 13 - light_idea_test_case - - Equality check can be used instead of elvis for nullable boolean check - Equality check can be used instead of elvis for nullable boolean check - - - test.kt - 14 - light_idea_test_case - - Equality check can be used instead of elvis for nullable boolean check - Equality check can be used instead of elvis for nullable boolean check - \ No newline at end of file diff --git a/idea/testData/inspections/nullableBooleanElvis/test.kt b/idea/testData/inspections/nullableBooleanElvis/test.kt index 2d976b1dc7c..b0be6c92e06 100644 --- a/idea/testData/inspections/nullableBooleanElvis/test.kt +++ b/idea/testData/inspections/nullableBooleanElvis/test.kt @@ -10,6 +10,6 @@ fun foo() { if (a ?: false || !(b ?: true)) { } - val x = a ?: false - val y = !(b ?: true) + val x = a ?: false // INFORMATION -- not reported in batch + val y = !(b ?: true) // INFORMATION -- not reported in batch } \ No newline at end of file diff --git a/idea/testData/inspections/replaceCallWithComparison/inspectionData/expected.xml b/idea/testData/inspections/replaceCallWithComparison/inspectionData/expected.xml index c2625acdfdf..2f227a34861 100644 --- a/idea/testData/inspections/replaceCallWithComparison/inspectionData/expected.xml +++ b/idea/testData/inspections/replaceCallWithComparison/inspectionData/expected.xml @@ -31,20 +31,4 @@ Can be replaced with binary operator Call replaceable with binary operator - - test.kt - 8 - light_idea_test_case - - Can be replaced with binary operator - Call replaceable with binary operator - - - test.kt - 9 - light_idea_test_case - - Can be replaced with binary operator - Call replaceable with binary operator - \ No newline at end of file diff --git a/idea/testData/inspections/replaceCallWithComparison/test.kt b/idea/testData/inspections/replaceCallWithComparison/test.kt index bde63e5fed4..0d87cfdac81 100644 --- a/idea/testData/inspections/replaceCallWithComparison/test.kt +++ b/idea/testData/inspections/replaceCallWithComparison/test.kt @@ -5,6 +5,6 @@ fun foo() { 1.compareTo(1) == 0 // NO 2.compareTo(1) > 0 // YES 0 >= 1.compareTo(2) // YES - 2.plus(2) // YES (information) - 2.times(2) // YES (information) + 2.plus(2) // YES (information) -- not reported in batch + 2.times(2) // YES (information) -- not reported in batch } \ No newline at end of file diff --git a/idea/testData/inspectionsLocal/nullableBooleanElvis/notInIf.kt b/idea/testData/inspectionsLocal/nullableBooleanElvis/notInIf.kt index 759ad185a07..9232bf12ca4 100644 --- a/idea/testData/inspectionsLocal/nullableBooleanElvis/notInIf.kt +++ b/idea/testData/inspectionsLocal/nullableBooleanElvis/notInIf.kt @@ -1,3 +1,5 @@ +// HIGHLIGHT: INFORMATION + fun foo() { var a: Boolean? = null val x = a ?: false diff --git a/idea/testData/inspectionsLocal/nullableBooleanElvis/notInIf.kt.after b/idea/testData/inspectionsLocal/nullableBooleanElvis/notInIf.kt.after index 902dddf9085..ef5406bab81 100644 --- a/idea/testData/inspectionsLocal/nullableBooleanElvis/notInIf.kt.after +++ b/idea/testData/inspectionsLocal/nullableBooleanElvis/notInIf.kt.after @@ -1,3 +1,5 @@ +// HIGHLIGHT: INFORMATION + fun foo() { var a: Boolean? = null val x = a == true diff --git a/idea/testData/inspectionsLocal/nullableBooleanElvis/notInIfWithTrue.kt b/idea/testData/inspectionsLocal/nullableBooleanElvis/notInIfWithTrue.kt index 05daebfe2a3..c3fc5af2821 100644 --- a/idea/testData/inspectionsLocal/nullableBooleanElvis/notInIfWithTrue.kt +++ b/idea/testData/inspectionsLocal/nullableBooleanElvis/notInIfWithTrue.kt @@ -1,3 +1,5 @@ +// HIGHLIGHT: INFORMATION + fun foo() { var b: Boolean? = null val x = !(b ?: true) diff --git a/idea/testData/inspectionsLocal/nullableBooleanElvis/notInIfWithTrue.kt.after b/idea/testData/inspectionsLocal/nullableBooleanElvis/notInIfWithTrue.kt.after index baf5d809161..d679c34edb0 100644 --- a/idea/testData/inspectionsLocal/nullableBooleanElvis/notInIfWithTrue.kt.after +++ b/idea/testData/inspectionsLocal/nullableBooleanElvis/notInIfWithTrue.kt.after @@ -1,3 +1,5 @@ +// HIGHLIGHT: INFORMATION + fun foo() { var b: Boolean? = null val x = b == false diff --git a/idea/tests/org/jetbrains/kotlin/idea/inspections/AbstractLocalInspectionTest.kt b/idea/tests/org/jetbrains/kotlin/idea/inspections/AbstractLocalInspectionTest.kt index 75a1a71c342..1aca0e0c712 100644 --- a/idea/tests/org/jetbrains/kotlin/idea/inspections/AbstractLocalInspectionTest.kt +++ b/idea/tests/org/jetbrains/kotlin/idea/inspections/AbstractLocalInspectionTest.kt @@ -17,11 +17,15 @@ package org.jetbrains.kotlin.idea.inspections import com.google.common.collect.Lists -import com.intellij.codeInspection.ProblemDescriptor +import com.intellij.codeHighlighting.HighlightDisplayLevel +import com.intellij.codeHighlighting.Pass +import com.intellij.codeInsight.daemon.impl.HighlightInfoType +import com.intellij.codeInsight.intention.EmptyIntentionAction import com.intellij.codeInspection.ProblemHighlightType import com.intellij.openapi.util.SystemInfo import com.intellij.openapi.util.io.FileUtil -import com.intellij.openapi.vfs.VirtualFile +import com.intellij.profile.codeInspection.ProjectInspectionProfileManager +import com.intellij.testFramework.fixtures.impl.CodeInsightTestFixtureImpl import junit.framework.ComparisonFailure import junit.framework.TestCase import org.jetbrains.kotlin.idea.test.DirectiveBasedActionUtils @@ -29,7 +33,6 @@ import org.jetbrains.kotlin.idea.test.KotlinLightCodeInsightFixtureTestCase import org.jetbrains.kotlin.idea.test.configureCompilerOptions import org.jetbrains.kotlin.idea.util.application.executeWriteCommand import org.jetbrains.kotlin.psi.KtFile -import org.jetbrains.kotlin.psi.psiUtil.startOffset import org.jetbrains.kotlin.test.InTextDirectivesUtils import org.jetbrains.kotlin.test.KotlinTestUtils import org.junit.Assert @@ -64,12 +67,16 @@ abstract class AbstractLocalInspectionTest : KotlinLightCodeInsightFixtureTestCa } if (candidateFiles.isEmpty()) { - throw AssertionError(".inspection file is not found for " + testDataFile + - "\nAdd it to base directory of test data. It should contain fully-qualified name of inspection class.") + throw AssertionError( + ".inspection file is not found for " + testDataFile + + "\nAdd it to base directory of test data. It should contain fully-qualified name of inspection class." + ) } if (candidateFiles.size > 1) { - throw AssertionError("Several .inspection files are available for " + testDataFile + - "\nPlease remove some of them\n" + candidateFiles) + throw AssertionError( + "Several .inspection files are available for " + testDataFile + + "\nPlease remove some of them\n" + candidateFiles + ) } val className = FileUtil.loadFile(candidateFiles[0]).trim { it <= ' ' } @@ -106,9 +113,9 @@ abstract class AbstractLocalInspectionTest : KotlinLightCodeInsightFixtureTestCa break } - val psiFile = myFixture.configureByFiles(*(listOf(mainFile.name) + extraFileNames).toTypedArray()).first() + myFixture.configureByFiles(*(listOf(mainFile.name) + extraFileNames).toTypedArray()).first() - doTestFor(mainFile.name, psiFile.virtualFile!!, inspection, fileText) + doTestFor(mainFile.name, inspection, fileText) if (file is KtFile && !InTextDirectivesUtils.isDirectiveDefined(fileText, "// SKIP_ERRORS_AFTER")) { DirectiveBasedActionUtils.checkForUnexpectedErrors(file as KtFile) @@ -116,90 +123,118 @@ abstract class AbstractLocalInspectionTest : KotlinLightCodeInsightFixtureTestCa } protected fun runInspectionWithFixesAndCheck( - file: VirtualFile, - inspection: AbstractKotlinInspection, - problemExpectedString: String?, - highlightExpectedString: String?, - localFixTextString: String? + inspection: AbstractKotlinInspection, + expectedProblemString: String?, + expectedHighlightString: String?, + localFixTextString: String? ): Boolean { - val problemExpected = problemExpectedString == null || problemExpectedString != "none" - val presentation = runInspection(inspection, project, listOf(file)) - val problemDescriptors = presentation.problemDescriptors - .filterIsInstance() - .filter { - val caretOffset = myFixture.caretOffset - caretOffset in it.textRangeInElement?.shiftRight(it.psiElement.startOffset) ?: it.psiElement.textRange - } + val problemExpected = expectedProblemString == null || expectedProblemString != "none" + myFixture.enableInspections(inspection::class.java) + + // Set default level to WARNING to make possible to test DO_NOT_SHOW + val inspectionProfileManager = ProjectInspectionProfileManager.getInstance(project) + val inspectionProfile = inspectionProfileManager.currentProfile + val state = inspectionProfile.getToolDefaultState(inspection.shortName, project) + state.level = HighlightDisplayLevel.WARNING + + val caretOffset = myFixture.caretOffset + val highlightInfos = CodeInsightTestFixtureImpl.instantiateAndRun( + file, editor, intArrayOf( + Pass.LINE_MARKERS, + Pass.EXTERNAL_TOOLS, + Pass.POPUP_HINTS, + Pass.UPDATE_ALL, + Pass.UPDATE_FOLDING, + Pass.WOLF + ), false + ).filter { it.description != null && caretOffset in it.startOffset..it.endOffset } + Assert.assertTrue( - if (!problemExpected) - "No problems should be detected at caret\n" + - "Detected problems: ${problemDescriptors.joinToString { it.descriptionTemplate }}" - else - "Expected at least one problem at caret", - problemExpected == problemDescriptors.isNotEmpty()) - if (!problemExpected) return false - problemDescriptors - .filter { it.highlightType != ProblemHighlightType.INFORMATION } - .forEach { - Assert.assertTrue("Problem description should not contain 'can': ${it.descriptionTemplate}", - " can " !in it.descriptionTemplate) - } - if (problemExpectedString != null) { - Assert.assertTrue("Expected the following problem at caret: $problemExpectedString\n" + - "Active problems: ${problemDescriptors.joinToString { it.descriptionTemplate }}", - problemDescriptors.any { it.descriptionTemplate == problemExpectedString }) + if (!problemExpected) + "No problems should be detected at caret\n" + + "Detected problems: ${highlightInfos.joinToString { it.description }}" + else + "Expected at least one problem at caret", + problemExpected == highlightInfos.isNotEmpty() + ) + if (!problemExpected || highlightInfos.isEmpty()) return false + highlightInfos + .filter { it.type != HighlightInfoType.INFORMATION } + .forEach { + val description = it.description + Assert.assertTrue( + "Problem description should not contain 'can': $description", + " can " !in description + ) + } + + if (expectedProblemString != null) { + Assert.assertTrue( + "Expected the following problem at caret: $expectedProblemString\n" + + "Active problems: ${highlightInfos.joinToString { it.description }}", + highlightInfos.any { it.description == expectedProblemString } + ) } - if (highlightExpectedString != null) { - Assert.assertTrue("Expected the following problem highlight type\n" + - "Actual types: ${problemDescriptors.joinToString { it.highlightType.toString() } }", - problemDescriptors.all { it.highlightType.toString() == highlightExpectedString }) + val expectedHighlightType = when (expectedHighlightString) { + null -> null + ProblemHighlightType.GENERIC_ERROR_OR_WARNING.name -> HighlightDisplayLevel.WARNING.name + else -> expectedHighlightString + } + if (expectedHighlightType != null) { + Assert.assertTrue( + "Expected the following problem highlight type: $expectedHighlightType\n" + + "Actual type: ${highlightInfos.joinToString { it.type.toString() }}", + highlightInfos.all { expectedHighlightType in it.type.toString() } + ) } - val allLocalFixActions = problemDescriptors.flatMap { problem -> - val fixes = problem.fixes - fixes?.toList() ?: emptyList() - } - val localFixActions = allLocalFixActions.filter { fix -> localFixTextString == null || fix.name == localFixTextString } - val availableDescription = allLocalFixActions.joinToString { it.name } + val allLocalFixActions = highlightInfos.flatMap { it.quickFixActionMarkers }.map { it.first.action } - val fixDescription = localFixTextString?.let { "with specified text '$localFixTextString'"} ?: "" - TestCase.assertTrue("No fix action $fixDescription\n" + - "Available actions: $availableDescription", - localFixActions.isNotEmpty()) + val localFixActions = allLocalFixActions.filter { fix -> localFixTextString == null || fix.text == localFixTextString } + val availableDescription = allLocalFixActions.joinToString { it.text } - val localFixAction = localFixActions.singleOrNull() - TestCase.assertTrue("More than one fix action $fixDescription\n" + - "Available actions: $availableDescription", - localFixAction != null) + val fixDescription = localFixTextString?.let { "with specified text '$localFixTextString'" } ?: "" + TestCase.assertTrue( + "No fix action $fixDescription\n" + + "Available actions: $availableDescription", + localFixActions.isNotEmpty() + ) - val problemDescriptor = problemDescriptors.find { localFixAction in it.fixes?.toList() ?: emptyList() }!! + val localFixAction = localFixActions.singleOrNull { it !is EmptyIntentionAction } + TestCase.assertTrue( + "More than one fix action $fixDescription\n" + + "Available actions: $availableDescription", + localFixAction != null + ) - project.executeWriteCommand(localFixAction!!.name, null) { - localFixAction.applyFix(project, problemDescriptor) + project.executeWriteCommand(localFixAction!!.text, null) { + localFixAction.invoke(project, editor, file) } return true } - private fun doTestFor(mainFilePath: String, file: VirtualFile, inspection: AbstractKotlinInspection, fileText: String) { - val problemExpectedString = InTextDirectivesUtils.findStringWithPrefixes( - fileText, "// $expectedProblemDirectiveName: ") - val highlightExpectedString = InTextDirectivesUtils.findStringWithPrefixes( - fileText, "// $expectedProblemHighlightType: ") + private fun doTestFor(mainFilePath: String, inspection: AbstractKotlinInspection, fileText: String) { + val expectedProblemString = InTextDirectivesUtils.findStringWithPrefixes( + fileText, "// $expectedProblemDirectiveName: " + ) + val expectedHighlightString = InTextDirectivesUtils.findStringWithPrefixes( + fileText, "// $expectedProblemHighlightType: " + ) val localFixTextString = InTextDirectivesUtils.findStringWithPrefixes( - fileText, "// $fixTextDirectiveName: ") + fileText, "// $fixTextDirectiveName: " + ) - if (!runInspectionWithFixesAndCheck(file, inspection, problemExpectedString, highlightExpectedString, localFixTextString)) { + if (!runInspectionWithFixesAndCheck(inspection, expectedProblemString, expectedHighlightString, localFixTextString)) { return } val canonicalPathToExpectedFile = mainFilePath + afterFileNameSuffix try { myFixture.checkResultByFile(canonicalPathToExpectedFile) - } - catch (e: ComparisonFailure) { + } catch (e: ComparisonFailure) { KotlinTestUtils.assertEqualsToFile( - File(testDataPath, canonicalPathToExpectedFile), - editor.document.text + File(testDataPath, canonicalPathToExpectedFile), + editor.document.text ) } } diff --git a/idea/tests/org/jetbrains/kotlin/idea/inspections/AbstractMultiFileLocalInspectionTest.kt b/idea/tests/org/jetbrains/kotlin/idea/inspections/AbstractMultiFileLocalInspectionTest.kt index 9f6c6426ee9..2a053c78d2e 100644 --- a/idea/tests/org/jetbrains/kotlin/idea/inspections/AbstractMultiFileLocalInspectionTest.kt +++ b/idea/tests/org/jetbrains/kotlin/idea/inspections/AbstractMultiFileLocalInspectionTest.kt @@ -58,10 +58,10 @@ abstract class AbstractMultiFileLocalInspectionTest : AbstractLocalInspectionTes val problemExpectedString = config["problem"]?.asString // null means "some problem", "none" means no problem val localFixTextString = config["fix"]?.asString // null means "some single fix" or "none" if no problem expected - doTest(path) test@ { _ -> - val mainFile = myFixture.configureFromTempProjectFile(mainFilePath) + doTest(path) test@{ _ -> + myFixture.configureFromTempProjectFile(mainFilePath) - runInspectionWithFixesAndCheck(mainFile.virtualFile, inspection, problemExpectedString, null, localFixTextString) + runInspectionWithFixesAndCheck(inspection, problemExpectedString, null, localFixTextString) } }