code review

This commit is contained in:
Dmitry Jemerov
2015-09-25 17:04:02 +02:00
parent 4e7236529c
commit 9de74921ca
10 changed files with 113 additions and 78 deletions
@@ -18,18 +18,17 @@ package org.jetbrains.kotlin.psi
import com.intellij.openapi.util.TextRange
import com.intellij.psi.PsiElement
import com.intellij.psi.PsiWhiteSpace
import com.intellij.psi.SmartPointerManager
import com.intellij.psi.SmartPsiElementPointer
import com.intellij.psi.codeStyle.CodeStyleManager
import com.intellij.psi.impl.source.codeStyle.CodeEditUtil
import org.jetbrains.kotlin.lexer.JetTokens
import org.jetbrains.kotlin.name.Name
import org.jetbrains.kotlin.psi.psiUtil.*
import org.jetbrains.kotlin.psi.psiUtil.PsiChildRange
import org.jetbrains.kotlin.psi.psiUtil.endOffset
import org.jetbrains.kotlin.psi.psiUtil.parentsWithSelf
import org.jetbrains.kotlin.psi.psiUtil.startOffset
import org.jetbrains.kotlin.renderer.render
import java.util.ArrayList
import java.util.HashMap
import java.util.LinkedHashMap
import java.util.*
public fun JetPsiFactory.createExpressionByPattern(pattern: String, vararg args: Any): JetExpression
= createByPattern(pattern, *args) { createExpression(it) }
@@ -296,6 +295,10 @@ public fun JetPsiFactory.buildExpression(build: BuilderByPattern<JetExpression>.
return buildByPattern({ pattern, args -> this.createExpressionByPattern(pattern, *args) }, build)
}
public fun JetPsiFactory.buildDeclaration(build: BuilderByPattern<JetDeclaration>.() -> Unit): JetDeclaration {
return buildByPattern({ pattern, args -> this.createDeclarationByPattern(pattern, *args) }, build)
}
public fun <TElement> buildByPattern(factory: (String, Array<out Any>) -> TElement, build: BuilderByPattern<TElement>.() -> Unit): TElement {
val builder = BuilderByPattern<TElement>()
builder.build()
@@ -310,10 +310,3 @@ public fun SearchScope.contains(element: PsiElement): Boolean = PsiSearchScopeUt
public fun <E : PsiElement> E.createSmartPointer(): SmartPsiElementPointer<E> =
SmartPointerManager.getInstance(getProject()).createSmartPsiElementPointer(this)
fun PsiElement.resolveAllReferences(): Sequence<PsiElement?> {
return PsiReferenceService.getService().getReferences(this, PsiReferenceService.Hints.NO_HINTS)
.asSequence()
.map { it.resolve() }
}
@@ -19,12 +19,13 @@ package org.jetbrains.kotlin.idea.intentions
import com.intellij.codeInsight.intention.IntentionAction
import com.intellij.openapi.editor.Editor
import com.intellij.openapi.project.Project
import com.intellij.psi.PsiWhiteSpace
import com.intellij.psi.search.LocalSearchScope
import com.intellij.psi.search.searches.ReferencesSearch
import org.jetbrains.kotlin.descriptors.PropertyDescriptor
import org.jetbrains.kotlin.descriptors.impl.SyntheticFieldDescriptor
import org.jetbrains.kotlin.diagnostics.Diagnostic
import org.jetbrains.kotlin.idea.caches.resolve.analyze
import org.jetbrains.kotlin.idea.caches.resolve.getResolutionFacade
import org.jetbrains.kotlin.idea.quickfix.JetIntentionAction
import org.jetbrains.kotlin.idea.quickfix.JetSingleIntentionActionFactory
import org.jetbrains.kotlin.lexer.JetTokens
@@ -35,11 +36,7 @@ import org.jetbrains.kotlin.resolve.BindingContext
class IntroduceBackingPropertyIntention(): JetSelfTargetingIntention<JetProperty>(javaClass(), "Introduce backing property") {
override fun isApplicableTo(element: JetProperty, caretOffset: Int): Boolean {
if (!canIntroduceBackingProperty(element)) return false
var elementAtCaret = element.containingFile.findElementAt(caretOffset)
if (elementAtCaret is PsiWhiteSpace) {
elementAtCaret = element.containingFile.findElementAt(caretOffset - 1)
}
return elementAtCaret == element.nameIdentifier || elementAtCaret == element.valOrVarKeyword
return element.nameIdentifier?.textRange?.containsOffset(caretOffset) == true
}
override fun applyTo(element: JetProperty, editor: Editor) {
@@ -47,43 +44,46 @@ class IntroduceBackingPropertyIntention(): JetSelfTargetingIntention<JetProperty
}
companion object {
public fun canIntroduceBackingProperty(element: JetProperty): Boolean {
val name = element.name ?: return false
if (name.startsWith('_')) return false
fun canIntroduceBackingProperty(property: JetProperty): Boolean {
val name = property.name ?: return false
if (element.hasDelegate() || element.receiverTypeReference != null) return false
val bindingContext = property.getResolutionFacade().analyzeFullyAndGetResult(listOf(property)).bindingContext
val descriptor = bindingContext.get(BindingContext.DECLARATION_TO_DESCRIPTOR, property) as? PropertyDescriptor ?: return false
if (bindingContext.get(BindingContext.BACKING_FIELD_REQUIRED, descriptor) == false) return false
val containingClass = element.getStrictParentOfType<JetClassOrObject>() ?: return false
val containingClass = property.getStrictParentOfType<JetClassOrObject>() ?: return false
return containingClass.declarations.none { it is JetProperty && it.name == "_" + name }
}
fun introduceBackingProperty(element: JetProperty) {
createBackingProperty(element)
fun introduceBackingProperty(property: JetProperty) {
createBackingProperty(property)
val type = SpecifyTypeExplicitlyIntention.getTypeForDeclaration(element)
SpecifyTypeExplicitlyIntention.addTypeAnnotation(null, element, type)
if (property.typeReference == null) {
val type = SpecifyTypeExplicitlyIntention.getTypeForDeclaration(property)
SpecifyTypeExplicitlyIntention.addTypeAnnotation(null, property, type)
}
val getter = element.getter
val getter = property.getter
if (getter == null) {
createGetter(element)
createGetter(property)
}
else {
replaceFieldReferences(getter, element.name!!)
replaceFieldReferences(getter, property.name!!)
}
if (element.isVar) {
val setter = element.setter
if (property.isVar) {
val setter = property.setter
if (setter == null) {
createSetter(element)
createSetter(property)
}
else {
replaceFieldReferences(setter, element.name!!)
replaceFieldReferences(setter, property.name!!)
}
}
element.setInitializer(null)
property.setInitializer(null)
replaceBackingFieldReferences(element)
replaceBackingFieldReferences(property)
}
private fun createGetter(element: JetProperty) {
@@ -98,41 +98,41 @@ class IntroduceBackingPropertyIntention(): JetSelfTargetingIntention<JetProperty
element.add(newSetter)
}
private fun createBackingProperty(element: JetProperty) {
val backingPropertyText = StringBuilder {
append("private ")
append(element.valOrVarKeyword.text)
append(" _")
append(element.name)
val typeRef = element.typeReference
if (typeRef != null) {
append(": ")
append(typeRef.text)
private fun createBackingProperty(property: JetProperty) {
val backingProperty = JetPsiFactory(property).buildDeclaration {
appendFixedText("private ")
appendFixedText(property.valOrVarKeyword.text)
appendFixedText(" _${property.name}")
if (property.typeReference != null) {
appendFixedText(": ")
appendTypeReference(property.typeReference)
}
val initializer = element.initializer
if (initializer != null) {
append(" = ")
append(initializer.text)
if (property.initializer != null) {
appendFixedText(" = ")
appendExpression(property.initializer)
}
}.toString()
}
val backingProp = JetPsiFactory(element).createProperty(backingPropertyText)
element.parent.addBefore(backingProp, element)
property.parent.addBefore(backingProperty, property)
}
private fun replaceFieldReferences(element: JetElement, propertyName: String) {
val bindingContext = element.analyze()
element.acceptChildren(object : JetTreeVisitorVoid() {
override fun visitSimpleNameExpression(expression: JetSimpleNameExpression) {
val bindingContext = expression.analyze()
val target = bindingContext.get(BindingContext.REFERENCE_TARGET, expression)
if (target is SyntheticFieldDescriptor) {
expression.replace(JetPsiFactory(element).createSimpleName("_$propertyName"))
}
}
override fun visitPropertyAccessor(accessor: JetPropertyAccessor) {
// don't go into accessors of properties in local classes because 'field' will mean something different in them
}
})
}
// TODO: drop this when we get rid of backing field syntax
private fun replaceBackingFieldReferences(prop: JetProperty) {
val containingClass = prop.getStrictParentOfType<JetClassOrObject>()!!
ReferencesSearch.search(prop, LocalSearchScope(containingClass)).forEach {
@@ -18,14 +18,14 @@ package org.jetbrains.kotlin.idea.intentions
import com.intellij.openapi.editor.Editor
import com.intellij.psi.PsiElement
import com.intellij.psi.TokenType
import com.intellij.psi.PsiReferenceService
import com.intellij.psi.util.PsiTreeUtil
import org.jetbrains.kotlin.lexer.JetTokens
import org.jetbrains.kotlin.psi.*
import org.jetbrains.kotlin.psi.psiUtil.anyDescendantOfType
import org.jetbrains.kotlin.psi.psiUtil.contentRange
import org.jetbrains.kotlin.psi.psiUtil.getNonStrictParentOfType
import org.jetbrains.kotlin.psi.psiUtil.getStrictParentOfType
import org.jetbrains.kotlin.psi.psiUtil.resolveAllReferences
import org.jetbrains.kotlin.utils.addToStdlib.firstIsInstanceOrNull
public class MoveAssignmentToInitializerIntention :
@@ -35,47 +35,41 @@ public class MoveAssignmentToInitializerIntention :
if (element.operationToken != JetTokens.EQ) {
return false
}
val rhs = element.right ?: return false
val rightExpression = element.right ?: return false
val block = PsiTreeUtil.getParentOfType(element,
JetClassInitializer::class.java,
JetSecondaryConstructor::class.java) ?: return false
val initializer = PsiTreeUtil.getParentOfType(element,
JetClassInitializer::class.java,
JetSecondaryConstructor::class.java) ?: return false
val target = findTargetProperty(element)
return target != null && target.initializer == null && target.receiverTypeReference == null &&
target.getNonStrictParentOfType<JetClassOrObject>() == element.getNonStrictParentOfType<JetClassOrObject>() &&
hasNoLocalDependencies(rhs, block)
hasNoLocalDependencies(rightExpression, initializer)
}
override fun applyTo(element: JetBinaryExpression, editor: Editor) {
val prop = findTargetProperty(element) ?: return
val property = findTargetProperty(element) ?: return
val initializer = element.right ?: return
prop.setInitializer(initializer)
property.setInitializer(initializer)
val initializerBlock = element.getStrictParentOfType<JetClassInitializer>()
element.delete()
if (initializerBlock != null && (initializerBlock.body as? JetBlockExpression)?.isEmpty() == true) {
initializerBlock.delete()
}
property.initializer?.navigate(true)
}
private fun findTargetProperty(expr: JetBinaryExpression): JetProperty? {
val lhs = expr.left as? JetSimpleNameExpression ?: return null
return lhs.resolveAllReferences().firstIsInstanceOrNull<JetProperty>()
val leftExpression = expr.left as? JetSimpleNameExpression ?: return null
return leftExpression.resolveAllReferences().firstIsInstanceOrNull<JetProperty>()
}
fun JetBlockExpression.isEmpty(): Boolean {
var node = node?.firstChildNode
while (node != null) {
if (node.elementType != TokenType.WHITE_SPACE &&
node.elementType != JetTokens.LBRACE &&
node.elementType != JetTokens.RBRACE) {
return false
}
node = node.treeNext
}
return true
// a block that only contains comments is not empty
return contentRange().isEmpty
}
private fun hasNoLocalDependencies(element: JetElement, localContext: PsiElement): Boolean {
@@ -84,3 +78,11 @@ public class MoveAssignmentToInitializerIntention :
}
}
}
private fun PsiElement.resolveAllReferences(): Sequence<PsiElement?> {
return PsiReferenceService.getService().getReferences(this, PsiReferenceService.Hints.NO_HINTS)
.asSequence()
.map { it.resolve() }
}
@@ -0,0 +1,5 @@
// IS_APPLICABLE: false
abstract class Foo {
abstract val <caret>x: Int
}
@@ -0,0 +1,6 @@
// IS_APPLICABLE: false
class Foo {
val <caret>x: Int
get() = 1
}
@@ -0,0 +1,3 @@
class Foo {
val <caret>x: Any = ""
}
@@ -0,0 +1,5 @@
class Foo {
private val _x: Any = ""
val x: Any
get() = _x
}
@@ -1,5 +1,5 @@
class A {
var a: Int = 1
var a: Int =<caret> 1
var b: Int
init {
@@ -5187,12 +5187,30 @@ public class IntentionTestGenerated extends AbstractIntentionTest {
doTest(fileName);
}
@TestMetadata("inapplicableAbstractProperty.kt")
public void testInapplicableAbstractProperty() throws Exception {
String fileName = JetTestUtils.navigationMetadata("idea/testData/intentions/introduceBackingProperty/inapplicableAbstractProperty.kt");
doTest(fileName);
}
@TestMetadata("inapplicableNoBackingField.kt")
public void testInapplicableNoBackingField() throws Exception {
String fileName = JetTestUtils.navigationMetadata("idea/testData/intentions/introduceBackingProperty/inapplicableNoBackingField.kt");
doTest(fileName);
}
@TestMetadata("simpleVal.kt")
public void testSimpleVal() throws Exception {
String fileName = JetTestUtils.navigationMetadata("idea/testData/intentions/introduceBackingProperty/simpleVal.kt");
doTest(fileName);
}
@TestMetadata("simpleValWithType.kt")
public void testSimpleValWithType() throws Exception {
String fileName = JetTestUtils.navigationMetadata("idea/testData/intentions/introduceBackingProperty/simpleValWithType.kt");
doTest(fileName);
}
@TestMetadata("simpleVar.kt")
public void testSimpleVar() throws Exception {
String fileName = JetTestUtils.navigationMetadata("idea/testData/intentions/introduceBackingProperty/simpleVar.kt");