Skip to content

Commit a683c2b

Browse files
PVTalanovPVTalanov
committed
IntentionBasedInspection: Removing synchronizing on intention instances
Recreate intention instance in `buildVisitor` Lock caused contention for UpSource
1 parent 742d7db commit a683c2b

32 files changed

+235
-225
lines changed

‎idea/idea-analysis/src/org/jetbrains/kotlin/idea/inspections/IntentionBasedInspection.kt

Lines changed: 28 additions & 23 deletions
Original file line numberDiff line numberDiff line change
@@ -31,34 +31,34 @@ import com.intellij.psi.PsiElementVisitor
3131
import com.intellij.psi.PsiFile
3232
import com.intellij.util.SmartList
3333
import org.jetbrains.kotlin.idea.intentions.SelfTargetingRangeIntention
34+
import kotlin.reflect.KClass
3435

3536
abstract class IntentionBasedInspection<TElement : PsiElement>(
36-
val intentions: List<IntentionBasedInspection.IntentionData<TElement>>,
37-
protected open val problemText: String?,
38-
protected val elementType: Class<TElement>
37+
val intentionInfos: List<IntentionBasedInspection.IntentionData<TElement>>,
38+
protected open val problemText: String?
3939
) : AbstractKotlinInspection() {
4040

4141
constructor(
42-
intention: SelfTargetingRangeIntention<TElement>,
42+
intention: KClass<out SelfTargetingRangeIntention<TElement>>,
4343
problemText: String? = null
44-
) : this(listOf(IntentionData(intention)), problemText, intention.elementType)
44+
) : this(listOf(IntentionData(intention)), problemText)
4545

4646
constructor(
47-
intention: SelfTargetingRangeIntention<TElement>,
47+
intention: KClass<out SelfTargetingRangeIntention<TElement>>,
4848
additionalChecker: (TElement, IntentionBasedInspection<TElement>) -> Boolean,
4949
problemText: String? = null
50-
) : this(listOf(IntentionData(intention, additionalChecker)), problemText, intention.elementType)
50+
) : this(listOf(IntentionData(intention, additionalChecker)), problemText)
5151

5252
constructor(
53-
intention: SelfTargetingRangeIntention<TElement>,
53+
intention: KClass<out SelfTargetingRangeIntention<TElement>>,
5454
additionalChecker: (TElement) -> Boolean,
5555
problemText: String? = null
56-
) : this(listOf(IntentionData(intention, { element, inspection -> additionalChecker(element) } )), problemText, intention.elementType)
56+
) : this(listOf(IntentionData(intention, { element, inspection -> additionalChecker(element) } )), problemText)
5757

5858

5959

6060
data class IntentionData<TElement : PsiElement>(
61-
val intention: SelfTargetingRangeIntention<TElement>,
61+
val intention: KClass<out SelfTargetingRangeIntention<TElement>>,
6262
val additionalChecker: (TElement, IntentionBasedInspection<TElement>) -> Boolean = { element, inspection -> true }
6363
)
6464

@@ -67,6 +67,13 @@ abstract class IntentionBasedInspection<TElement : PsiElement>(
6767
open fun inspectionRange(element: TElement): TextRange? = null
6868

6969
override fun buildVisitor(holder: ProblemsHolder, isOnTheFly: Boolean, session: LocalInspectionToolSession): PsiElementVisitor {
70+
71+
val intentionsAndCheckers = intentionInfos.map {
72+
it.intention.constructors.single().call() to it.additionalChecker
73+
}
74+
val elementType = intentionsAndCheckers.map { it.first.elementType }.distinct().singleOrNull()
75+
?: error("$intentionInfos should have the same elementType")
76+
7077
return object : PsiElementVisitor() {
7178
override fun visitElement(element: PsiElement) {
7279
if (!elementType.isInstance(element) || element.textLength == 0) return
@@ -77,21 +84,19 @@ abstract class IntentionBasedInspection<TElement : PsiElement>(
7784
var problemRange: TextRange? = null
7885
var fixes: SmartList<LocalQuickFix>? = null
7986

80-
for ((intention, additionalChecker) in intentions) {
81-
synchronized(intention) {
82-
val range = intention.applicabilityRange(targetElement)?.let { range ->
83-
val elementRange = targetElement.textRange
84-
assert(range in elementRange) { "Wrong applicabilityRange() result for $intention - should be within element's range" }
85-
range.shiftRight(-elementRange.startOffset)
86-
}
87+
for ((intention, additionalChecker) in intentionsAndCheckers) {
88+
val range = intention.applicabilityRange(targetElement)?.let { range ->
89+
val elementRange = targetElement.textRange
90+
assert(range in elementRange) { "Wrong applicabilityRange() result for $intention - should be within element's range" }
91+
range.shiftRight(-elementRange.startOffset)
92+
}
8793

88-
if (range != null && additionalChecker(targetElement, this@IntentionBasedInspection)) {
89-
problemRange = problemRange?.union(range) ?: range
90-
if (fixes == null) {
91-
fixes = SmartList<LocalQuickFix>()
92-
}
93-
fixes!!.add(createQuickFix(intention, additionalChecker, targetElement))
94+
if (range != null && additionalChecker(targetElement, this@IntentionBasedInspection)) {
95+
problemRange = problemRange?.union(range) ?: range
96+
if (fixes == null) {
97+
fixes = SmartList<LocalQuickFix>()
9498
}
99+
fixes!!.add(createQuickFix(intention, additionalChecker, targetElement))
95100
}
96101
}
97102

‎idea/idea-analysis/src/org/jetbrains/kotlin/idea/intentions/SelfTargetingIntention.kt

Lines changed: 6 additions & 5 deletions
Original file line numberDiff line numberDiff line change
@@ -33,6 +33,7 @@ import org.jetbrains.kotlin.psi.KtElement
3333
import org.jetbrains.kotlin.psi.psiUtil.containsInside
3434
import org.jetbrains.kotlin.psi.psiUtil.parentsWithSelf
3535
import java.util.*
36+
import kotlin.reflect.KClass
3637

3738
abstract class SelfTargetingIntention<TElement : PsiElement>(
3839
val elementType: Class<TElement>,
@@ -88,14 +89,14 @@ abstract class SelfTargetingIntention<TElement : PsiElement>(
8889
}
8990

9091
protected fun isIntentionBaseInspectionEnabled(project: Project, target: TElement): Boolean {
91-
val inspection = findInspection(javaClass) ?: return false
92+
val inspection = findInspection(this.javaClass.kotlin) ?: return false
9293

9394
val key = HighlightDisplayKey.find(inspection.shortName)
9495
if (!InspectionProjectProfileManager.getInstance(project).getInspectionProfile(target).isToolEnabled(key)) {
9596
return false
9697
}
9798

98-
return inspection.intentions.single { it.intention.javaClass == javaClass }.additionalChecker(target, inspection)
99+
return inspection.intentionInfos.single { it.intention == this.javaClass.kotlin }.additionalChecker(target, inspection)
99100
}
100101

101102
final override fun invoke(project: Project, editor: Editor, file: PsiFile): Unit {
@@ -109,17 +110,17 @@ abstract class SelfTargetingIntention<TElement : PsiElement>(
109110
override fun toString(): String = getText()
110111

111112
companion object {
112-
private val intentionBasedInspections = HashMap<Class<out SelfTargetingIntention<*>>, IntentionBasedInspection<*>?>()
113+
private val intentionBasedInspections = HashMap<KClass<out SelfTargetingIntention<*>>, IntentionBasedInspection<*>?>()
113114

114-
fun <TElement : PsiElement> findInspection(intentionClass: Class<out SelfTargetingIntention<TElement>>): IntentionBasedInspection<TElement>? {
115+
fun <TElement : PsiElement> findInspection(intentionClass: KClass<out SelfTargetingIntention<TElement>>): IntentionBasedInspection<TElement>? {
115116
if (intentionBasedInspections.containsKey(intentionClass)) {
116117
@Suppress("UNCHECKED_CAST")
117118
return intentionBasedInspections[intentionClass] as IntentionBasedInspection<TElement>?
118119
}
119120

120121
for (extension in Extensions.getExtensions(LocalInspectionEP.LOCAL_INSPECTION)) {
121122
val inspection = extension.instance as? IntentionBasedInspection<*> ?: continue
122-
if (inspection.intentions.any { it.intention.javaClass == intentionClass }) {
123+
if (inspection.intentionInfos.any { it.intention == intentionClass }) {
123124
intentionBasedInspections[intentionClass] = inspection
124125
@Suppress("UNCHECKED_CAST")
125126
return inspection as IntentionBasedInspection<TElement>

‎idea/src/org/jetbrains/kotlin/idea/inspections/HasPlatformTypeInspection.kt

Lines changed: 3 additions & 4 deletions
Original file line numberDiff line numberDiff line change
@@ -33,14 +33,13 @@ import org.jetbrains.kotlin.types.isNullabilityFlexible
3333
import javax.swing.JComponent
3434

3535
class HasPlatformTypeInspection(
36-
val intention: SpecifyTypeExplicitlyIntention = SpecifyTypeExplicitlyIntention(),
3736
@JvmField var publicAPIOnly: Boolean = true,
3837
@JvmField var reportPlatformArguments: Boolean = false
3938
) : IntentionBasedInspection<KtCallableDeclaration>(
40-
intention,
39+
SpecifyTypeExplicitlyIntention::class,
4140
{ element, inspection ->
4241
with(inspection as HasPlatformTypeInspection) {
43-
intention.dangerousFlexibleTypeOrNull(element, this.publicAPIOnly, this.reportPlatformArguments) != null
42+
SpecifyTypeExplicitlyIntention.dangerousFlexibleTypeOrNull(element, this.publicAPIOnly, this.reportPlatformArguments) != null
4443
}
4544
}
4645
) {
@@ -50,7 +49,7 @@ class HasPlatformTypeInspection(
5049
override val problemText = "Declaration has platform type. Make the type explicit to prevent subtle bugs."
5150

5251
override fun additionalFixes(element: KtCallableDeclaration): List<LocalQuickFix>? {
53-
val type = intention.dangerousFlexibleTypeOrNull(element, publicAPIOnly, reportPlatformArguments) ?: return null
52+
val type = SpecifyTypeExplicitlyIntention.dangerousFlexibleTypeOrNull(element, publicAPIOnly, reportPlatformArguments) ?: return null
5453

5554
if (type.isNullabilityFlexible()) {
5655
val expression = element.node.findChildByType(KtTokens.EQ)?.psi?.getNextSiblingIgnoringWhitespaceAndComments()

‎idea/src/org/jetbrains/kotlin/idea/intentions/ConvertLambdaToReferenceIntention.kt

Lines changed: 34 additions & 34 deletions
Original file line numberDiff line numberDiff line change
@@ -32,11 +32,9 @@ import org.jetbrains.kotlin.types.isDynamic
3232
import org.jetbrains.kotlin.types.typeUtil.isTypeParameter
3333
import org.jetbrains.kotlin.types.typeUtil.isUnit
3434

35-
class ConvertLambdaToReferenceInspection(
36-
val intention: ConvertLambdaToReferenceIntention = ConvertLambdaToReferenceIntention()
37-
) : IntentionBasedInspection<KtLambdaExpression>(
38-
intention,
39-
{ it -> intention.shouldSuggestToConvert(it) }
35+
class ConvertLambdaToReferenceInspection() : IntentionBasedInspection<KtLambdaExpression>(
36+
ConvertLambdaToReferenceIntention::class,
37+
{ it -> ConvertLambdaToReferenceIntention.shouldSuggestToConvert(it) }
4038
)
4139

4240
class ConvertLambdaToReferenceIntention : SelfTargetingOffsetIndependentIntention<KtLambdaExpression>(
@@ -79,7 +77,7 @@ class ConvertLambdaToReferenceIntention : SelfTargetingOffsetIndependentIntentio
7977
if (calleeDescriptor.typeParameters.isNotEmpty()) return false
8078
// No references to Java synthetic properties
8179
if (calleeDescriptor is SyntheticJavaPropertyDescriptor) return false
82-
val descriptorHasReceiver = with (calleeDescriptor) {
80+
val descriptorHasReceiver = with(calleeDescriptor) {
8381
// No references to both member / extension
8482
if (dispatchReceiverParameter != null && extensionReceiverParameter != null) return false
8583
dispatchReceiverParameter != null || extensionReceiverParameter != null
@@ -144,34 +142,6 @@ class ConvertLambdaToReferenceIntention : SelfTargetingOffsetIndependentIntentio
144142
}
145143
}
146144

147-
internal fun shouldSuggestToConvert(element: KtLambdaExpression): Boolean {
148-
val body = element.bodyExpression ?: return false
149-
val referenceName = buildReferenceText(body.statements.singleOrNull() ?: return false) ?: return false
150-
return referenceName.length < element.text.length
151-
}
152-
153-
private fun KtCallExpression.getCallReferencedName() = (calleeExpression as? KtNameReferenceExpression)?.getReferencedName()
154-
155-
private fun buildReferenceText(expression: KtExpression): String? {
156-
return when (expression) {
157-
is KtCallExpression -> "::${expression.getCallReferencedName()}"
158-
is KtDotQualifiedExpression -> {
159-
val selector = expression.selectorExpression
160-
val selectorReferenceName = when (selector) {
161-
is KtCallExpression -> selector.getCallReferencedName() ?: return null
162-
is KtNameReferenceExpression -> selector.getReferencedName()
163-
else -> return null
164-
}
165-
val receiver = expression.receiverExpression as? KtNameReferenceExpression ?: return null
166-
val context = receiver.analyze()
167-
val receiverDescriptor = context[REFERENCE_TARGET, receiver] as? ParameterDescriptor ?: return null
168-
val receiverType = receiverDescriptor.type
169-
"$receiverType::$selectorReferenceName"
170-
}
171-
else -> null
172-
}
173-
}
174-
175145
override fun applyTo(element: KtLambdaExpression, editor: Editor?) {
176146
val body = element.bodyExpression ?: return
177147
val referenceName = buildReferenceText(body.statements.singleOrNull() ?: return) ?: return
@@ -221,4 +191,34 @@ class ConvertLambdaToReferenceIntention : SelfTargetingOffsetIndependentIntentio
221191
}
222192
}
223193
}
194+
195+
companion object {
196+
internal fun shouldSuggestToConvert(element: KtLambdaExpression): Boolean {
197+
val body = element.bodyExpression ?: return false
198+
val referenceName = buildReferenceText(body.statements.singleOrNull() ?: return false) ?: return false
199+
return referenceName.length < element.text.length
200+
}
201+
202+
private fun buildReferenceText(expression: KtExpression): String? {
203+
return when (expression) {
204+
is KtCallExpression -> "::${expression.getCallReferencedName()}"
205+
is KtDotQualifiedExpression -> {
206+
val selector = expression.selectorExpression
207+
val selectorReferenceName = when (selector) {
208+
is KtCallExpression -> selector.getCallReferencedName() ?: return null
209+
is KtNameReferenceExpression -> selector.getReferencedName()
210+
else -> return null
211+
}
212+
val receiver = expression.receiverExpression as? KtNameReferenceExpression ?: return null
213+
val context = receiver.analyze()
214+
val receiverDescriptor = context[REFERENCE_TARGET, receiver] as? ParameterDescriptor ?: return null
215+
val receiverType = receiverDescriptor.type
216+
"$receiverType::$selectorReferenceName"
217+
}
218+
else -> null
219+
}
220+
}
221+
222+
private fun KtCallExpression.getCallReferencedName() = (calleeExpression as? KtNameReferenceExpression)?.getReferencedName()
223+
}
224224
}

0 commit comments

Comments
 (0)