Inline Property supported for properties with setter

#KT-2638 Fixed
This commit is contained in:
Valentin Kipyatkov
2017-05-23 15:26:04 +03:00
parent e6bfa55534
commit ab1b985bac
16 changed files with 181 additions and 66 deletions
@@ -24,10 +24,7 @@ import org.jetbrains.kotlin.resolve.calls.callUtil.getResolvedCall
import org.jetbrains.kotlin.resolve.calls.model.VariableAsFunctionResolvedCall
import org.jetbrains.kotlin.resolve.lazy.BodyResolveMode
class CallableUsageReplacementStrategy(
private val replacement: CodeToInline
) : UsageReplacementStrategy {
class CallableUsageReplacementStrategy(private val replacement: CodeToInline) : UsageReplacementStrategy {
override fun createReplacer(usage: KtSimpleNameExpression): (() -> KtElement?)? {
val bindingContext = usage.analyze(BodyResolveMode.PARTIAL)
val resolvedCall = usage.getResolvedCall(bindingContext) ?: return null
@@ -21,6 +21,8 @@ import com.intellij.psi.PsiElement
import com.intellij.refactoring.rename.RenameProcessor
import org.jetbrains.kotlin.builtins.KotlinBuiltIns
import org.jetbrains.kotlin.descriptors.CallableDescriptor
import org.jetbrains.kotlin.descriptors.PropertyDescriptor
import org.jetbrains.kotlin.descriptors.PropertySetterDescriptor
import org.jetbrains.kotlin.descriptors.ValueParameterDescriptor
import org.jetbrains.kotlin.idea.caches.resolve.analyze
import org.jetbrains.kotlin.idea.caches.resolve.getResolutionFacade
@@ -63,10 +65,15 @@ class CodeInliner<TCallElement : KtElement>(
val descriptor = resolvedCall.resultingDescriptor
val file = nameExpression.containingKtFile
val elementToBeReplaced = when (callElement) {
is KtExpression -> callElement.getQualifiedExpressionForSelectorOrThis()
else -> callElement
}
val qualifiedElement = if (callElement is KtExpression) callElement.getQualifiedExpressionForSelectorOrThis() else callElement
val assignment = (qualifiedElement as? KtExpression)
?.getAssignmentByLHS()
?.takeIf { it.operationToken == KtTokens.EQ }
val elementToBeReplaced = assignment ?: qualifiedElement
val callableForParameters = if (assignment != null && descriptor is PropertyDescriptor)
descriptor.setter ?: descriptor
else
descriptor
val commentSaver = CommentSaver(elementToBeReplaced, saveLineBreaks = true)
@@ -103,11 +110,10 @@ class CodeInliner<TCallElement : KtElement>(
}
}
val introduceValuesForParameters = processValueParameterUsages()
val introduceValuesForParameters = processValueParameterUsages(callableForParameters)
processTypeParameterUsages()
val lexicalScope = callElement.parent.getResolutionScope(bindingContext)
if (elementToBeReplaced is KtSafeQualifiedExpression) {
@@ -164,13 +170,12 @@ class CodeInliner<TCallElement : KtElement>(
}
}
private fun processValueParameterUsages(): Collection<IntroduceValueForParameter> {
private fun processValueParameterUsages(descriptor: CallableDescriptor): Collection<IntroduceValueForParameter> {
val introduceValuesForParameters = ArrayList<IntroduceValueForParameter>()
// process parameters in reverse order because default values can use previous parameters
val parameters = resolvedCall.resultingDescriptor.valueParameters
for (parameter in parameters.asReversed()) {
val argument = argumentForParameter(parameter) ?: continue
for (parameter in descriptor.valueParameters.asReversed()) {
val argument = argumentForParameter(parameter, descriptor) ?: continue
argument.expression.put(PARAMETER_VALUE_KEY, parameter)
@@ -329,7 +334,15 @@ class CodeInliner<TCallElement : KtElement>(
val isNamed: Boolean = false,
val isDefaultValue: Boolean = false)
private fun argumentForParameter(parameter: ValueParameterDescriptor): Argument? {
private fun argumentForParameter(parameter: ValueParameterDescriptor, callableDescriptor: CallableDescriptor): Argument? {
if (callableDescriptor is PropertySetterDescriptor) {
val valueAssigned = (callElement as? KtExpression)
?.getQualifiedExpressionForSelectorOrThis()
?.getAssignmentByLHS()
?.right ?: return null
return Argument(valueAssigned, bindingContext.getType(valueAssigned))
}
val resolvedArgument = resolvedCall.valueArguments[parameter]!!
when (resolvedArgument) {
is ExpressionValueArgument -> {
@@ -343,19 +356,18 @@ class CodeInliner<TCallElement : KtElement>(
}
is DefaultValueArgument -> {
val defaultValue = OptionalParametersHelper.defaultParameterValue(parameter, project) ?: return null
val (expression, parameterUsages) = defaultValue
val (defaultValue, parameterUsages) = OptionalParametersHelper.defaultParameterValue(parameter, project) ?: return null
for ((param, usages) in parameterUsages) {
usages.forEach { it.put(CodeToInline.PARAMETER_USAGE_KEY, param.name) }
}
val expressionCopy = expression.copied()
val defaultValueCopy = defaultValue.copied()
// clean up user data in original
expression.forEachDescendantOfType<KtExpression> { it.clear(CodeToInline.PARAMETER_USAGE_KEY) }
defaultValue.forEachDescendantOfType<KtExpression> { it.clear(CodeToInline.PARAMETER_USAGE_KEY) }
return Argument(expressionCopy, null/*TODO*/, isDefaultValue = true)
return Argument(defaultValueCopy, null/*TODO*/, isDefaultValue = true)
}
is VarargValueArgument -> {
@@ -0,0 +1,36 @@
/*
* 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.idea.codeInliner
import org.jetbrains.kotlin.idea.references.ReferenceAccess
import org.jetbrains.kotlin.idea.references.readWriteAccess
import org.jetbrains.kotlin.psi.KtElement
import org.jetbrains.kotlin.psi.KtSimpleNameExpression
class PropertyUsageReplacementStrategy(readReplacement: CodeToInline?, writeReplacement: CodeToInline?) : UsageReplacementStrategy {
private val readReplacementStrategy = readReplacement?.let { CallableUsageReplacementStrategy(it) }
private val writeReplacementStrategy = writeReplacement?.let { CallableUsageReplacementStrategy(it) }
override fun createReplacer(usage: KtSimpleNameExpression): (() -> KtElement?)? {
val access = usage.readWriteAccess(useResolveForReadWrite = true)
return when (access) {
ReferenceAccess.READ -> readReplacementStrategy?.createReplacer(usage)
ReferenceAccess.WRITE -> writeReplacementStrategy?.createReplacer(usage)
ReferenceAccess.READ_WRITE -> null
}
}
}
@@ -28,12 +28,12 @@ import org.jetbrains.kotlin.diagnostics.Diagnostic
import org.jetbrains.kotlin.diagnostics.DiagnosticFactory
import org.jetbrains.kotlin.diagnostics.Errors
import org.jetbrains.kotlin.idea.caches.resolve.getResolutionFacade
import org.jetbrains.kotlin.idea.core.OptionalParametersHelper
import org.jetbrains.kotlin.idea.quickfix.KotlinQuickFixAction
import org.jetbrains.kotlin.idea.references.mainReference
import org.jetbrains.kotlin.idea.codeInliner.CallableUsageReplacementStrategy
import org.jetbrains.kotlin.idea.codeInliner.ClassUsageReplacementStrategy
import org.jetbrains.kotlin.idea.codeInliner.UsageReplacementStrategy
import org.jetbrains.kotlin.idea.core.OptionalParametersHelper
import org.jetbrains.kotlin.idea.quickfix.KotlinQuickFixAction
import org.jetbrains.kotlin.idea.references.mainReference
import org.jetbrains.kotlin.psi.KtConstructorCalleeExpression
import org.jetbrains.kotlin.psi.KtFile
import org.jetbrains.kotlin.psi.KtSimpleNameExpression
@@ -26,7 +26,7 @@ import com.intellij.refactoring.util.CommonRefactoringUtil
import com.intellij.usageView.UsageInfo
import com.intellij.usageView.UsageViewBundle
import com.intellij.usageView.UsageViewDescriptor
import org.jetbrains.kotlin.idea.codeInliner.CallableUsageReplacementStrategy
import org.jetbrains.kotlin.idea.codeInliner.UsageReplacementStrategy
import org.jetbrains.kotlin.idea.codeInliner.replaceUsages
import org.jetbrains.kotlin.idea.references.KtSimpleNameReference
import org.jetbrains.kotlin.idea.stubindex.KotlinSourceFilterScope
@@ -35,7 +35,7 @@ import org.jetbrains.kotlin.psi.*
class KotlinInlineCallableProcessor(
project: Project,
private val replacementStrategy: CallableUsageReplacementStrategy,
private val replacementStrategy: UsageReplacementStrategy,
private val declaration: KtCallableDeclaration,
private val reference: KtSimpleNameReference?,
private val inlineThisOnly: Boolean,
@@ -21,7 +21,8 @@ import com.intellij.openapi.project.Project
import com.intellij.refactoring.HelpID
import com.intellij.refactoring.JavaRefactoringSettings
import com.intellij.refactoring.inline.InlineOptionsDialog
import org.jetbrains.kotlin.idea.codeInliner.CallableUsageReplacementStrategy
import org.jetbrains.kotlin.idea.codeInliner.PropertyUsageReplacementStrategy
import org.jetbrains.kotlin.idea.codeInliner.UsageReplacementStrategy
import org.jetbrains.kotlin.idea.quickfix.createFromUsage.callableBuilder.getReturnTypeReference
import org.jetbrains.kotlin.idea.references.KtSimpleNameReference
import org.jetbrains.kotlin.psi.KtConstructor
@@ -32,7 +33,7 @@ class KotlinInlineFunctionDialog(
project: Project,
private val function: KtNamedFunction,
private val reference: KtSimpleNameReference?,
private val replacementStrategy: CallableUsageReplacementStrategy,
private val replacementStrategy: UsageReplacementStrategy,
private val allowInlineThisOnly: Boolean
) : InlineOptionsDialog(project, true, function) {
@@ -32,6 +32,7 @@ import org.jetbrains.kotlin.idea.KotlinLanguage
import org.jetbrains.kotlin.idea.caches.resolve.analyzeFully
import org.jetbrains.kotlin.idea.caches.resolve.resolveToDescriptor
import org.jetbrains.kotlin.idea.codeInliner.CallableUsageReplacementStrategy
import org.jetbrains.kotlin.idea.codeInliner.PropertyUsageReplacementStrategy
import org.jetbrains.kotlin.idea.references.KtSimpleNameReference
import org.jetbrains.kotlin.psi.KtExpression
import org.jetbrains.kotlin.psi.KtNamedFunction
@@ -19,7 +19,7 @@ package org.jetbrains.kotlin.idea.refactoring.inline
import com.intellij.openapi.util.text.StringUtil
import com.intellij.refactoring.JavaRefactoringSettings
import com.intellij.refactoring.inline.InlineOptionsDialog
import org.jetbrains.kotlin.idea.codeInliner.CallableUsageReplacementStrategy
import org.jetbrains.kotlin.idea.codeInliner.UsageReplacementStrategy
import org.jetbrains.kotlin.idea.references.KtSimpleNameReference
import org.jetbrains.kotlin.psi.KtBinaryExpression
import org.jetbrains.kotlin.psi.KtProperty
@@ -27,7 +27,7 @@ import org.jetbrains.kotlin.psi.KtProperty
class KotlinInlineValDialog(
private val property: KtProperty,
private val reference: KtSimpleNameReference?,
private val replacementStrategy: CallableUsageReplacementStrategy,
private val replacementStrategy: UsageReplacementStrategy,
private val assignmentToDelete: KtBinaryExpression?
) : InlineOptionsDialog(property.project, true, property) {
@@ -35,9 +35,9 @@ import org.jetbrains.kotlin.idea.KotlinLanguage
import org.jetbrains.kotlin.idea.analysis.analyzeInContext
import org.jetbrains.kotlin.idea.caches.resolve.getResolutionFacade
import org.jetbrains.kotlin.idea.caches.resolve.resolveToDescriptor
import org.jetbrains.kotlin.idea.codeInliner.CallableUsageReplacementStrategy
import org.jetbrains.kotlin.idea.codeInliner.CodeToInline
import org.jetbrains.kotlin.idea.codeInliner.CodeToInlineBuilder
import org.jetbrains.kotlin.idea.codeInliner.PropertyUsageReplacementStrategy
import org.jetbrains.kotlin.idea.core.copied
import org.jetbrains.kotlin.idea.refactoring.checkConflictsInteractively
import org.jetbrains.kotlin.idea.references.KtSimpleNameReference
@@ -53,7 +53,6 @@ import org.jetbrains.kotlin.psi.psiUtil.getAssignmentByLHS
import org.jetbrains.kotlin.psi.psiUtil.getQualifiedExpressionForSelectorOrThis
import org.jetbrains.kotlin.resolve.BindingContext
import org.jetbrains.kotlin.types.TypeUtils
import org.jetbrains.kotlin.utils.addIfNotNull
class KotlinInlineValHandler : InlineActionHandler() {
@@ -75,17 +74,11 @@ class KotlinInlineValHandler : InlineActionHandler() {
val getter = declaration.getter?.takeIf { it.hasBody() }
val setter = declaration.setter?.takeIf { it.hasBody() }
if (getter != null || setter != null) {
if (declaration.initializer != null) {
return showErrorHint(project, editor, "Cannot inline property with accessor(s) and backing field")
}
if (setter != null) {
return showErrorHint(project, editor, "Inline property not supported for properties with setter")
}
if ((getter != null || setter != null) && declaration.initializer != null) {
return showErrorHint(project, editor, "Cannot inline property with accessor(s) and backing field")
}
val (referenceExpressions, foreignUsages) = findUsages(declaration)
val (referenceExpressions, conflicts) = findUsages(declaration)
if (referenceExpressions.isEmpty()) {
val kind = if (declaration.isLocal) "Variable" else "Property"
@@ -93,49 +86,61 @@ class KotlinInlineValHandler : InlineActionHandler() {
}
val referencesInOriginalFile = referenceExpressions.filter { it.containingFile == file }
val isHighlighting = referencesInOriginalFile.isNotEmpty()
val hasHighlightings = referencesInOriginalFile.isNotEmpty()
highlightElements(project, editor, referencesInOriginalFile)
val codeToInline: CodeToInline
val assignment: KtBinaryExpression?
if (getter == null) {
val readReplacement: CodeToInline?
val writeReplacement: CodeToInline?
val assignmentToDelete: KtBinaryExpression?
if (getter == null && setter == null) {
val initialization = extractInitialization(declaration, referenceExpressions, project, editor) ?: return
codeToInline = buildCodeToInline(declaration, initialization.value)
assignment = initialization.assignment
readReplacement = buildCodeToInline(declaration, initialization.value)
writeReplacement = null
assignmentToDelete = initialization.assignment
}
else {
val descriptor = declaration.resolveToDescriptor() as PropertyDescriptor
codeToInline = buildCodeToInline(getter, descriptor.type, editor) ?: return
assignment = null
readReplacement = getter?.let { buildCodeToInline(getter, descriptor.type, editor) ?: return }
writeReplacement = setter?.let { buildCodeToInline(setter, descriptor.type, editor) ?: return }
assignmentToDelete = null
}
if (foreignUsages.isNotEmpty()) {
val conflicts = MultiMap<PsiElement, String>().apply {
putValue(null, "Property '$name' has non-Kotlin usages. They won't be processed by the Inline refactoring.")
foreignUsages.forEach { putValue(it, it.text) }
if (!conflicts.isEmpty) {
val conflictsCopy = conflicts.copy()
conflictsCopy.putValue(null, "The following usages are not supported by the Inline refactoring. They won't be processed.")
project.checkConflictsInteractively(conflictsCopy) {
performRefactoring(declaration, readReplacement, writeReplacement, assignmentToDelete, editor, hasHighlightings)
}
project.checkConflictsInteractively(conflicts) { performRefactoring(declaration, codeToInline, editor, assignment, isHighlighting) }
}
else {
performRefactoring(declaration, codeToInline, editor, assignment, isHighlighting)
performRefactoring(declaration, readReplacement, writeReplacement, assignmentToDelete, editor, hasHighlightings)
}
}
private data class Usages(val referenceExpressions: Collection<KtExpression>, val foreignUsages: Collection<PsiElement>)
private data class Usages(val referenceExpressions: Collection<KtExpression>, val conflicts: MultiMap<PsiElement, String>)
private fun findUsages(declaration: KtProperty): Usages {
val references = ReferencesSearch.search(declaration)
val referenceExpressions = mutableListOf<KtExpression>()
val foreignUsages = mutableListOf<PsiElement>()
val conflictUsages = MultiMap.create<PsiElement, String>()
for (ref in references) {
val refElement = ref.element ?: continue
if (refElement !is KtElement) {
foreignUsages.add(refElement)
conflictUsages.putValue(refElement, "Non-Kotlin usage: ${refElement.text}")
continue
}
referenceExpressions.addIfNotNull((refElement as? KtExpression)?.getQualifiedExpressionForSelectorOrThis())
val expression = (refElement as? KtExpression)?.getQualifiedExpressionForSelectorOrThis()
//TODO: what if null?
if (expression != null) {
if (expression.readWriteAccess(useResolveForReadWrite = true) == ReferenceAccess.READ_WRITE) {
conflictUsages.putValue(expression, "Unsupported usage: ${expression.parent.text}")
}
referenceExpressions.add(expression)
}
}
return Usages(referenceExpressions, foreignUsages)
return Usages(referenceExpressions, conflictUsages)
}
private data class Initialization(val value: KtExpression, val assignment: KtBinaryExpression?)
@@ -189,20 +194,21 @@ class KotlinInlineValHandler : InlineActionHandler() {
private fun performRefactoring(
declaration: KtProperty,
replacement: CodeToInline,
readReplacement: CodeToInline?,
writeReplacement: CodeToInline?,
assignmentToDelete: KtBinaryExpression?,
editor: Editor?,
assignment: KtBinaryExpression?,
isHighlighting: Boolean
hasHighlightings: Boolean
) {
val replacementStrategy = CallableUsageReplacementStrategy(replacement)
val replacementStrategy = PropertyUsageReplacementStrategy(readReplacement, writeReplacement)
val reference = editor?.let { TargetElementUtil.findReference(it, it.caretModel.offset) } as? KtSimpleNameReference
val dialog = KotlinInlineValDialog(declaration, reference, replacementStrategy, assignment)
val dialog = KotlinInlineValDialog(declaration, reference, replacementStrategy, assignmentToDelete)
if (!ApplicationManager.getApplication().isUnitTestMode) {
dialog.show()
if (!dialog.isOK && isHighlighting) {
if (!dialog.isOK && hasHighlightings) {
val statusBar = WindowManager.getInstance().getStatusBar(declaration.project)
statusBar?.info = RefactoringBundle.message("press.escape.to.remove.the.highlighting")
}
@@ -0,0 +1,8 @@
class A {
var property = 10
private set
fun f() {
println(<caret>property)
}
}
@@ -0,0 +1,6 @@
class A {
fun f() {
println(10)
}
}
@@ -0,0 +1,10 @@
var Int.<caret>property: Int
get() = this
set(value) {
println("Set value of $value for $this")
}
fun foo() {
println(1.property)
2.property = 10
}
@@ -0,0 +1,4 @@
fun foo() {
println(1)
println("Set value of ${10} for ${2}")
}
@@ -0,0 +1,10 @@
// ERROR: The following usages are not supported by the Inline refactoring. They won't be processed.\nUnsupported usage: property++
var <caret>property: Int
get() = 1
set(value) {
}
fun foo() {
property++
}
@@ -22,6 +22,7 @@ import com.intellij.codeInsight.TargetElementUtil.REFERENCED_ELEMENT_ACCEPTED
import com.intellij.lang.refactoring.InlineActionHandler
import com.intellij.openapi.extensions.Extensions
import com.intellij.openapi.util.io.FileUtil
import com.intellij.refactoring.BaseRefactoringProcessor
import com.intellij.refactoring.util.CommonRefactoringUtil
import com.intellij.testFramework.UsefulTestCase
import com.intellij.testFramework.fixtures.JavaCodeInsightTestFixture
@@ -69,9 +70,14 @@ abstract class AbstractInlineTest : KotlinLightCodeInsightFixtureTestCase() {
TestCase.assertEquals("Expected errors", 1, expectedErrors.size)
TestCase.assertEquals("Error message", expectedErrors[0].replace("\\n", "\n"), e.message)
}
catch (e: BaseRefactoringProcessor.ConflictsInTestsException) {
TestCase.assertFalse("Conflicts: ${e.message}", afterFileExists)
TestCase.assertEquals("Expected errors", 1, expectedErrors.size)
TestCase.assertEquals("Error message", expectedErrors[0].replace("\\n", "\n"), e.message)
}
}
else {
TestCase.assertFalse(afterFileExists)
TestCase.assertFalse("No refactoring handler available", afterFileExists)
}
}
@@ -1030,6 +1030,12 @@ public class InlineTestGenerated extends AbstractInlineTest {
doTest(fileName);
}
@TestMetadata("PrivateSet.kt")
public void testPrivateSet() throws Exception {
String fileName = KotlinTestUtils.navigationMetadata("idea/testData/refactoring/inline/inlineVariableOrProperty/property/PrivateSet.kt");
doTest(fileName);
}
@TestMetadata("QualifiedUsage.kt")
public void testQualifiedUsage() throws Exception {
String fileName = KotlinTestUtils.navigationMetadata("idea/testData/refactoring/inline/inlineVariableOrProperty/property/QualifiedUsage.kt");
@@ -1068,6 +1074,18 @@ public class InlineTestGenerated extends AbstractInlineTest {
doTest(fileName);
}
@TestMetadata("GetterAndSetter.kt")
public void testGetterAndSetter() throws Exception {
String fileName = KotlinTestUtils.navigationMetadata("idea/testData/refactoring/inline/inlineVariableOrProperty/property/accessors/GetterAndSetter.kt");
doTest(fileName);
}
@TestMetadata("PlusPlus.kt")
public void testPlusPlus() throws Exception {
String fileName = KotlinTestUtils.navigationMetadata("idea/testData/refactoring/inline/inlineVariableOrProperty/property/accessors/PlusPlus.kt");
doTest(fileName);
}
@TestMetadata("WithInitializer.kt")
public void testWithInitializer() throws Exception {
String fileName = KotlinTestUtils.navigationMetadata("idea/testData/refactoring/inline/inlineVariableOrProperty/property/accessors/WithInitializer.kt");