Fix extract refactoring for android extensions declarations (KT-11048)

Allow any target declarations in marking references. Otherwise conflicts
for references resolved to xml are not considered broken.

This also fix evaluate for extension fields.

 #KT-11048 Fixed
This commit is contained in:
Nikolay Krasko
2017-07-05 14:55:40 +03:00
parent a09b31b568
commit 72611d1337
11 changed files with 265 additions and 106 deletions
@@ -1325,6 +1325,10 @@ fun main(args: Array<String>) {
testClass<AbstractAndroidUsageHighlightingTest> {
model("android/usageHighlighting", recursive = false, extension = null)
}
testClass<AbstractAndroidExtractionTest> {
model("android/extraction", recursive = false, extension = null)
}
}
testGroup("idea/idea-android/tests", "idea/testData") {
@@ -22,8 +22,7 @@ import com.intellij.openapi.util.Key
import com.intellij.psi.PsiElement
import com.intellij.psi.PsiNameIdentifierOwner
import com.intellij.psi.util.PsiTreeUtil
import org.jetbrains.kotlin.descriptors.DeclarationDescriptor
import org.jetbrains.kotlin.descriptors.ValueParameterDescriptor
import org.jetbrains.kotlin.descriptors.*
import org.jetbrains.kotlin.idea.caches.resolve.analyze
import org.jetbrains.kotlin.idea.caches.resolve.getResolutionFacade
import org.jetbrains.kotlin.idea.codeInsight.DescriptorToSourceUtilsIde
@@ -66,7 +65,7 @@ data class ExtractionOptions(
data class ResolveResult(
val originalRefExpr: KtSimpleNameExpression,
val declaration: PsiNameIdentifierOwner,
val declaration: PsiElement,
val descriptor: DeclarationDescriptor,
val resolvedCall: ResolvedCall<*>?
)
@@ -128,14 +127,17 @@ data class ExtractionData(
return function == null || !function.isInsideOf(physicalElements)
}
private tailrec fun getDeclaration(descriptor: DeclarationDescriptor, context: BindingContext): PsiNameIdentifierOwner? {
(DescriptorToSourceUtilsIde.getAnyDeclaration(project, descriptor) as? PsiNameIdentifierOwner)?.let { return it }
private tailrec fun getDeclaration(descriptor: DeclarationDescriptor, context: BindingContext): PsiElement? {
val declaration = DescriptorToSourceUtilsIde.getAnyDeclaration(project, descriptor)
if (declaration is PsiNameIdentifierOwner) {
return declaration
}
return when {
isExtractableIt(descriptor, context) -> itFakeDeclaration
isSynthesizedInvoke(descriptor) -> synthesizedInvokeDeclaration
descriptor is SyntheticJavaPropertyDescriptor -> getDeclaration(descriptor.getMethod, context)
else -> null
else -> declaration
}
}
@@ -770,8 +770,9 @@ internal fun KtNamedDeclaration.getGeneratedBody() =
@JvmOverloads
fun ExtractableCodeDescriptor.validate(target: ExtractionTarget = ExtractionTarget.FUNCTION): ExtractableCodeDescriptorWithConflicts {
fun getDeclarationMessage(declaration: PsiNamedElement, messageKey: String, capitalize: Boolean = true): String {
val message = KotlinRefactoringBundle.message(messageKey, RefactoringUIUtil.getDescription(declaration, true))
fun getDeclarationMessage(declaration: PsiElement, messageKey: String, capitalize: Boolean = true): String {
val declarationStr = RefactoringUIUtil.getDescription(declaration, true)
val message = KotlinRefactoringBundle.message(messageKey, declarationStr)
return if (capitalize) message.capitalize() else message
}
@@ -94,7 +94,7 @@ private fun buildSignature(config: ExtractionGeneratorConfiguration, renderer: D
config.descriptor.parameters.forEach { parameter ->
param(parameter.name,
parameter.getParameterType(config.descriptor.extractionData.options.allowSpecialClassNames).typeAsString())
parameter.getParameterType(config.descriptor.extractionData.options.allowSpecialClassNames).typeAsString())
}
with(config.descriptor.returnType) {
@@ -17,6 +17,7 @@
package org.jetbrains.kotlin.idea.refactoring.introduce.extractionEngine
import com.intellij.psi.PsiElement
import com.intellij.psi.PsiNameIdentifierOwner
import com.intellij.util.containers.MultiMap
import org.jetbrains.kotlin.builtins.createFunctionType
import org.jetbrains.kotlin.cfg.pseudocode.Pseudocode
@@ -94,7 +95,7 @@ internal fun ExtractionData.inferParametersInfo(
}
else {
extensionReceiver
}) as? ReceiverValue
})
val twoReceivers = resolvedCall != null && resolvedCall.hasBothReceivers()
val dispatchReceiverDescriptor = (resolvedCall?.dispatchReceiver as? ImplicitReceiver)?.declarationDescriptor
@@ -136,7 +137,7 @@ internal fun ExtractionData.inferParametersInfo(
}
}
for (typeToCheck in info.typeParameters.flatMapTo(HashSet<KotlinType>()) { it.collectReferencedTypes(bindingContext) }) {
for (typeToCheck in info.typeParameters.flatMapTo(HashSet()) { it.collectReferencedTypes(bindingContext) }) {
typeToCheck.processTypeIfExtractable(info.typeParameters, info.nonDenotableTypes, options, targetScope)
}
@@ -179,7 +180,7 @@ private fun ExtractionData.extractReceiver(
is ConstructorDescriptor -> it.containingDeclaration
else -> null
} as? ClassifierDescriptor
}
}
if (referencedClassifierDescriptor != null) {
@@ -254,7 +255,10 @@ private fun ExtractionData.extractReceiver(
}
if (!extractThis) {
parameter.currentName = originalDeclaration.nameIdentifier?.text
parameter.currentName = when (originalDeclaration) {
is PsiNameIdentifierOwner -> originalDeclaration.nameIdentifier?.text
else -> null
}
}
parameter.refCount++
@@ -37,6 +37,7 @@ import com.intellij.refactoring.introduceParameter.Util
import com.intellij.refactoring.util.CommonRefactoringUtil
import com.intellij.refactoring.util.DocCommentPolicy
import com.intellij.refactoring.util.occurrences.ExpressionOccurrenceManager
import com.intellij.testFramework.fixtures.CodeInsightTestFixture
import com.intellij.testFramework.fixtures.JavaCodeInsightTestFixture
import com.intellij.testFramework.fixtures.LightCodeInsightFixtureTestCase
import org.jetbrains.kotlin.idea.KotlinFileType
@@ -60,6 +61,7 @@ import org.jetbrains.kotlin.idea.refactoring.memberInfo.extractClassMembers
import org.jetbrains.kotlin.idea.refactoring.selectElement
import org.jetbrains.kotlin.idea.test.ConfigLibraryUtil
import org.jetbrains.kotlin.idea.test.KotlinLightCodeInsightFixtureTestCase
import org.jetbrains.kotlin.idea.test.KotlinLightCodeInsightFixtureTestCaseBase
import org.jetbrains.kotlin.idea.test.PluginTestCaseBase
import org.jetbrains.kotlin.idea.util.IdeDescriptorRenderers
import org.jetbrains.kotlin.lexer.KtModifierKeywordToken
@@ -252,76 +254,7 @@ abstract class AbstractExtractionTest : KotlinLightCodeInsightFixtureTestCase()
}
protected fun doExtractFunctionTest(path: String) {
doTest(path) { file ->
file as KtFile
val explicitPreviousSibling = file.findElementByCommentPrefix("// SIBLING:")
val fileText = file.getText() ?: ""
val expectedNames = InTextDirectivesUtils.findListWithPrefixes(fileText, "// SUGGESTED_NAMES: ")
val expectedReturnTypes = InTextDirectivesUtils.findListWithPrefixes(fileText, "// SUGGESTED_RETURN_TYPES: ")
val expectedDescriptors =
InTextDirectivesUtils.findLinesWithPrefixesRemoved(fileText, "// PARAM_DESCRIPTOR: ").joinToString()
val expectedTypes =
InTextDirectivesUtils.findLinesWithPrefixesRemoved(fileText, "// PARAM_TYPES: ").map { "[$it]" }.joinToString()
val extractionOptions = InTextDirectivesUtils.findListWithPrefixes(fileText, "// OPTIONS: ").let {
if (it.isNotEmpty()) {
@Suppress("UNCHECKED_CAST")
val args = it.map { it.toBoolean() }.toTypedArray() as Array<Any?>
ExtractionOptions::class.java.constructors.first { it.parameterTypes.size == args.size }.newInstance(*args) as ExtractionOptions
} else ExtractionOptions.DEFAULT
}
val renderer = DescriptorRenderer.FQ_NAMES_IN_TYPES
val editor = fixture.editor
val handler = ExtractKotlinFunctionHandler(
helper = object : ExtractionEngineHelper(EXTRACT_FUNCTION) {
override fun adjustExtractionData(data: ExtractionData): ExtractionData {
return data.copy(options = extractionOptions)
}
override fun configureAndRun(
project: Project,
editor: Editor,
descriptorWithConflicts: ExtractableCodeDescriptorWithConflicts,
onFinish: (ExtractionResult) -> Unit
) {
val descriptor = descriptorWithConflicts.descriptor
val actualNames = descriptor.suggestedNames
val actualReturnTypes = descriptor.controlFlow.possibleReturnTypes.map {
IdeDescriptorRenderers.SOURCE_CODE.renderType(it)
}
val allParameters = listOfNotNull(descriptor.receiverParameter) + descriptor.parameters
val actualDescriptors = allParameters.map { renderer.render(it.originalDescriptor) }.joinToString()
val actualTypes = allParameters.map {
it.getParameterTypeCandidates(false).map { renderer.renderType(it) }.joinToString(", ", "[", "]")
}.joinToString()
if (actualNames.size != 1 || expectedNames.isNotEmpty()) {
assertEquals(expectedNames, actualNames, "Expected names mismatch.")
}
if (actualReturnTypes.size != 1 || expectedReturnTypes.isNotEmpty()) {
assertEquals(expectedReturnTypes, actualReturnTypes, "Expected return types mismatch.")
}
assertEquals("Expected descriptors mismatch.", expectedDescriptors, actualDescriptors)
assertEquals("Expected types mismatch.", expectedTypes, actualTypes)
val newDescriptor = if (descriptor.name == "") {
descriptor.copy(suggestedNames = Collections.singletonList("__dummyTestFun__"))
}
else {
descriptor
}
doRefactor(ExtractionGeneratorConfiguration(newDescriptor, ExtractionGeneratorOptions.DEFAULT), onFinish)
}
}
)
handler.selectElements(editor, file) { elements, previousSibling ->
handler.doInvoke(editor, file, elements, explicitPreviousSibling ?: previousSibling)
}
}
doTest(path) { file -> doExtractFunction(myFixture, file as KtFile) }
}
protected fun doIntroduceTypeParameterTest(path: String) {
@@ -397,8 +330,6 @@ abstract class AbstractExtractionTest : KotlinLightCodeInsightFixtureTestCase()
protected fun doTest(path: String, checkAdditionalAfterdata: Boolean = false, action: (PsiFile) -> Unit) {
val mainFile = File(path)
val afterFile = File("$path.after")
val conflictFile = File("$path.conflicts")
fixture.testDataPath = "${KotlinTestUtils.getHomeDirectory()}/${mainFile.parent}"
@@ -416,27 +347,7 @@ abstract class AbstractExtractionTest : KotlinLightCodeInsightFixtureTestCase()
}
try {
action(file)
assert(!conflictFile.exists()) { "Conflict file $conflictFile should not exist" }
KotlinTestUtils.assertEqualsToFile(afterFile, file.text!!)
if (checkAdditionalAfterdata) {
for ((extraPsiFile, extraFile) in extraFilesToPsi) {
KotlinTestUtils.assertEqualsToFile(File("${extraFile.path}.after"), extraPsiFile.text)
}
}
}
catch(e: ConflictsInTestsException) {
val message = e.messages.sorted().joinToString(" ").replace("\n", " ")
KotlinTestUtils.assertEqualsToFile(conflictFile, message)
}
catch(e: CommonRefactoringUtil.RefactoringErrorHintException) {
KotlinTestUtils.assertEqualsToFile(conflictFile, e.message!!)
}
catch(e: RuntimeException) { // RuntimeException is thrown by IDEA code in CodeInsightUtils.java
if (e::class.java != RuntimeException::class.java) throw e
KotlinTestUtils.assertEqualsToFile(conflictFile, e.message!!)
checkExtract(ExtractTestFiles(path, file, extraFilesToPsi), checkAdditionalAfterdata, action)
}
finally {
if (addKotlinRuntime) {
@@ -445,3 +356,110 @@ abstract class AbstractExtractionTest : KotlinLightCodeInsightFixtureTestCase()
}
}
}
class ExtractTestFiles(
val mainFile: PsiFile,
val afterFile: File,
val conflictFile: File,
val extraFilesToPsi: Map<PsiFile, File> = emptyMap()) {
constructor(path: String, mainFile: PsiFile, extraFilesToPsi: Map<PsiFile, File> = emptyMap()) :
this(mainFile, File("$path.after"), File("$path.conflicts"), extraFilesToPsi)
}
fun checkExtract(files: ExtractTestFiles, checkAdditionalAfterdata: Boolean = false, action: (PsiFile) -> Unit) {
val conflictFile = files.conflictFile
val afterFile = files.afterFile
try {
action(files.mainFile)
assert(!conflictFile.exists()) { "Conflict file $conflictFile should not exist" }
KotlinTestUtils.assertEqualsToFile(afterFile, files.mainFile.text!!)
if (checkAdditionalAfterdata) {
for ((extraPsiFile, extraFile) in files.extraFilesToPsi) {
KotlinTestUtils.assertEqualsToFile(File("${extraFile.path}.after"), extraPsiFile.text)
}
}
}
catch(e: ConflictsInTestsException) {
val message = e.messages.sorted().joinToString(" ").replace("\n", " ")
KotlinTestUtils.assertEqualsToFile(conflictFile, message)
}
catch(e: CommonRefactoringUtil.RefactoringErrorHintException) {
KotlinTestUtils.assertEqualsToFile(conflictFile, e.message!!)
}
catch(e: RuntimeException) { // RuntimeException is thrown by IDEA code in CodeInsightUtils.java
if (e::class.java != RuntimeException::class.java) throw e
KotlinTestUtils.assertEqualsToFile(conflictFile, e.message!!)
}
}
fun doExtractFunction(fixture: CodeInsightTestFixture, file: KtFile) {
val explicitPreviousSibling = file.findElementByCommentPrefix("// SIBLING:")
val fileText = file.getText() ?: ""
val expectedNames = InTextDirectivesUtils.findListWithPrefixes(fileText, "// SUGGESTED_NAMES: ")
val expectedReturnTypes = InTextDirectivesUtils.findListWithPrefixes(fileText, "// SUGGESTED_RETURN_TYPES: ")
val expectedDescriptors =
InTextDirectivesUtils.findLinesWithPrefixesRemoved(fileText, "// PARAM_DESCRIPTOR: ").joinToString()
val expectedTypes =
InTextDirectivesUtils.findLinesWithPrefixesRemoved(fileText, "// PARAM_TYPES: ").map { "[$it]" }.joinToString()
val extractionOptions = InTextDirectivesUtils.findListWithPrefixes(fileText, "// OPTIONS: ").let {
if (it.isNotEmpty()) {
@Suppress("UNCHECKED_CAST")
val args = it.map { it.toBoolean() }.toTypedArray() as Array<Any?>
ExtractionOptions::class.java.constructors.first { it.parameterTypes.size == args.size }.newInstance(*args) as ExtractionOptions
} else ExtractionOptions.DEFAULT
}
val renderer = DescriptorRenderer.FQ_NAMES_IN_TYPES
val editor = fixture.editor
val handler = ExtractKotlinFunctionHandler(
helper = object : ExtractionEngineHelper(EXTRACT_FUNCTION) {
override fun adjustExtractionData(data: ExtractionData): ExtractionData {
return data.copy(options = extractionOptions)
}
override fun configureAndRun(
project: Project,
editor: Editor,
descriptorWithConflicts: ExtractableCodeDescriptorWithConflicts,
onFinish: (ExtractionResult) -> Unit
) {
val descriptor = descriptorWithConflicts.descriptor
val actualNames = descriptor.suggestedNames
val actualReturnTypes = descriptor.controlFlow.possibleReturnTypes.map {
IdeDescriptorRenderers.SOURCE_CODE.renderType(it)
}
val allParameters = listOfNotNull(descriptor.receiverParameter) + descriptor.parameters
val actualDescriptors = allParameters.map { renderer.render(it.originalDescriptor) }.joinToString()
val actualTypes = allParameters.map {
it.getParameterTypeCandidates(false).map { renderer.renderType(it) }.joinToString(", ", "[", "]")
}.joinToString()
if (actualNames.size != 1 || expectedNames.isNotEmpty()) {
assertEquals(expectedNames, actualNames, "Expected names mismatch.")
}
if (actualReturnTypes.size != 1 || expectedReturnTypes.isNotEmpty()) {
assertEquals(expectedReturnTypes, actualReturnTypes, "Expected return types mismatch.")
}
KotlinLightCodeInsightFixtureTestCaseBase.assertEquals("Expected descriptors mismatch.", expectedDescriptors, actualDescriptors)
KotlinLightCodeInsightFixtureTestCaseBase.assertEquals("Expected types mismatch.", expectedTypes, actualTypes)
val newDescriptor = if (descriptor.name == "") {
descriptor.copy(suggestedNames = Collections.singletonList("__dummyTestFun__"))
}
else {
descriptor
}
doRefactor(ExtractionGeneratorConfiguration(newDescriptor, ExtractionGeneratorOptions.DEFAULT), onFinish)
}
}
)
handler.selectElements(editor, file) { elements, previousSibling ->
handler.doInvoke(editor, file, elements, explicitPreviousSibling ?: previousSibling)
}
}
@@ -0,0 +1,14 @@
<FrameLayout xmlns:android="http://schemas.android.com/apk/res/android"
xmlns:tools="http://schemas.android.com/tools"
android:layout_width="match_parent"
android:layout_height="match_parent"
tools:context=".ItemDetailActivity"
tools:ignore="MergeRootFrame" >
<org.my.cool.Button
android:id="@+id/MyButton"
android:layout_width="match_parent"
android:layout_height="wrap_content"
android:text="Sign in" />
</FrameLayout>
@@ -0,0 +1,17 @@
// SUGGESTED_NAMES: view, getButton
// SUGGESTED_RETURN_TYPES: android.view.View?, android.view.View
// PARAM_DESCRIPTOR: public final class MyActivity : android.app.Activity defined in test in file toTopLevelFun.kt
// PARAM_TYPES: test.MyActivity, android.app.Activity
package test
import android.app.Activity
import kotlinx.android.synthetic.main.layout.*
// SIBLING:
class MyActivity: Activity() {
fun test() {
val button = <selection>MyButton</selection>
}
}
@@ -0,0 +1,20 @@
// SUGGESTED_NAMES: view, getButton
// SUGGESTED_RETURN_TYPES: android.view.View?, android.view.View
// PARAM_DESCRIPTOR: public final class MyActivity : android.app.Activity defined in test in file toTopLevelFun.kt
// PARAM_TYPES: test.MyActivity, android.app.Activity
package test
import android.app.Activity
import android.view.View
import kotlinx.android.synthetic.main.layout.*
// SIBLING:
class MyActivity: Activity() {
fun test() {
val button = view()
}
}
private fun MyActivity.view(): View? = MyButton
@@ -0,0 +1,35 @@
/*
* Copyright 2010-2017 JetBrains s.r.o.
*
* Licensed under the Apache License, Version 2.0 (the "License");
* you may not use this file except in compliance with the License.
* You may obtain a copy of the License at
*
* http://www.apache.org/licenses/LICENSE-2.0
*
* Unless required by applicable law or agreed to in writing, software
* distributed under the License is distributed on an "AS IS" BASIS,
* WITHOUT WARRANTIES OR CONDITIONS OF ANY KIND, either express or implied.
* See the License for the specific language governing permissions and
* limitations under the License.
*/
package org.jetbrains.kotlin.android
import org.jetbrains.kotlin.idea.refactoring.introduce.ExtractTestFiles
import org.jetbrains.kotlin.idea.refactoring.introduce.checkExtract
import org.jetbrains.kotlin.idea.refactoring.introduce.doExtractFunction
import org.jetbrains.kotlin.psi.KtFile
abstract class AbstractAndroidExtractionTest: KotlinAndroidTestCase() {
fun doTest(path: String) {
copyResourceDirectoryForTest(path)
val testFilePath = path + getTestName(true) + ".kt"
val virtualFile = myFixture.copyFileToProject(testFilePath, "src/" + getTestName(true) + ".kt")
myFixture.configureFromExistingVirtualFile(virtualFile)
checkExtract(ExtractTestFiles(testFilePath, myFixture.file)) { file ->
doExtractFunction(myFixture, file as KtFile)
}
}
}
@@ -0,0 +1,44 @@
/*
* Copyright 2010-2017 JetBrains s.r.o.
*
* Licensed under the Apache License, Version 2.0 (the "License");
* you may not use this file except in compliance with the License.
* You may obtain a copy of the License at
*
* http://www.apache.org/licenses/LICENSE-2.0
*
* Unless required by applicable law or agreed to in writing, software
* distributed under the License is distributed on an "AS IS" BASIS,
* WITHOUT WARRANTIES OR CONDITIONS OF ANY KIND, either express or implied.
* See the License for the specific language governing permissions and
* limitations under the License.
*/
package org.jetbrains.kotlin.android;
import com.intellij.testFramework.TestDataPath;
import org.jetbrains.kotlin.test.JUnit3RunnerWithInners;
import org.jetbrains.kotlin.test.KotlinTestUtils;
import org.jetbrains.kotlin.test.TargetBackend;
import org.jetbrains.kotlin.test.TestMetadata;
import org.junit.runner.RunWith;
import java.io.File;
import java.util.regex.Pattern;
/** This class is generated by {@link org.jetbrains.kotlin.generators.tests.TestsPackage}. DO NOT MODIFY MANUALLY */
@SuppressWarnings("all")
@TestMetadata("plugins/android-extensions/android-extensions-idea/testData/android/extraction")
@TestDataPath("$PROJECT_ROOT")
@RunWith(JUnit3RunnerWithInners.class)
public class AndroidExtractionTestGenerated extends AbstractAndroidExtractionTest {
public void testAllFilesPresentInExtraction() throws Exception {
KotlinTestUtils.assertAllTestsPresentByMetadata(this.getClass(), new File("plugins/android-extensions/android-extensions-idea/testData/android/extraction"), Pattern.compile("^([^\\.]+)$"), TargetBackend.ANY, false);
}
@TestMetadata("toTopLevelFun")
public void testToTopLevelFun() throws Exception {
String fileName = KotlinTestUtils.navigationMetadata("plugins/android-extensions/android-extensions-idea/testData/android/extraction/toTopLevelFun/");
doTest(fileName);
}
}