Improve messages in ReadOrWriteActionInServiceInitializationInspection

GitOrigin-RevId: 5d1dfb765163ebe5326da07dfc400131060dcd34
This commit is contained in:
Karol Lewandowski
2024-09-13 15:08:52 +02:00
committed by intellij-monorepo-bot
parent a3b5ae6cc2
commit ed7196bf47
4 changed files with 89 additions and 33 deletions

View File

@@ -684,8 +684,14 @@ inspections.listener.implementation.must.not.be.disposable.name=Listener impleme
inspections.listener.implementation.must.not.implement.disposable=Listener implementation must not implement 'Disposable' inspections.listener.implementation.must.not.implement.disposable=Listener implementation must not implement 'Disposable'
inspection.read.or.write.action.during.service.init.display.name=Read or Write Action run during service initialization inspection.read.or.write.action.during.service.init.display.name=Read or Write Action run during service initialization
inspection.read.or.write.action.during.service.init.message.read=Do not run read actions during service initialization inspection.read.or.write.action.during.service.init.message.read=Do not run read actions during service initialization{0}
inspection.read.or.write.action.during.service.init.message.write=Do not run write actions during service initialization inspection.read.or.write.action.during.service.init.message.write=Do not run write actions during service initialization{0}
inspection.read.or.write.action.during.service.init.message.context=\ ({0} is called in {1})
inspection.read.or.write.action.during.service.init.message.context.field=''{0}'' field initializer
inspection.read.or.write.action.during.service.init.message.context.method=''{0}'' method
inspection.read.or.write.action.during.service.init.message.context.constructor=''{0}'' constructor or init block
inspection.read.or.write.action.during.service.init.message.context.static.initializer=static initialization block
inspection.read.or.write.action.during.service.init.message.context.instance.initializer=instance initialization block
inspection.plugin.xml.registration.check.display.name=Plugin.xml registration check inspection.plugin.xml.registration.check.display.name=Plugin.xml registration check

View File

@@ -7,7 +7,8 @@ import com.intellij.openapi.components.PersistentStateComponent
import com.intellij.psi.PsiElementVisitor import com.intellij.psi.PsiElementVisitor
import com.intellij.uast.UastHintedVisitorAdapter import com.intellij.uast.UastHintedVisitorAdapter
import com.siyeh.ig.callMatcher.CallMatcher import com.siyeh.ig.callMatcher.CallMatcher
import org.jetbrains.idea.devkit.DevKitBundle import org.jetbrains.annotations.Nls
import org.jetbrains.idea.devkit.DevKitBundle.message
import org.jetbrains.uast.* import org.jetbrains.uast.*
import org.jetbrains.uast.visitor.AbstractUastNonRecursiveVisitor import org.jetbrains.uast.visitor.AbstractUastNonRecursiveVisitor
import org.jetbrains.uast.visitor.UastVisitor import org.jetbrains.uast.visitor.UastVisitor
@@ -44,8 +45,9 @@ internal class ReadOrWriteActionInServiceInitializationInspection : DevKitUastIn
object : AbstractUastNonRecursiveVisitor() { object : AbstractUastNonRecursiveVisitor() {
override fun visitCallExpression(node: UCallExpression): Boolean { override fun visitCallExpression(node: UCallExpression): Boolean {
val actionType = node.getReadOrWriteActionRunMethodCallType() val actionType = node.getReadOrWriteActionRunMethodCallType()
if (actionType.isReadOrWrite() && isCalledDuringServiceInitialization(node)) { val callContextHolder = CallContextHolder()
registerProblem(node, actionType, holder) if (actionType.isReadOrWrite() && isCalledDuringServiceInitialization(node, callContextHolder)) {
registerProblem(node, actionType, holder, callContextHolder)
} }
return true return true
} }
@@ -62,12 +64,12 @@ internal class ReadOrWriteActionInServiceInitializationInspection : DevKitUastIn
} }
} }
private fun isCalledDuringServiceInitialization(readOrWriteActionCall: UCallExpression): Boolean { private fun isCalledDuringServiceInitialization(readOrWriteActionCall: UCallExpression, callContextHolder: CallContextHolder): Boolean {
val uClass = readOrWriteActionCall.getContainingNonCompanionObjectClass() ?: return false val uClass = readOrWriteActionCall.getContainingNonCompanionObjectClass() ?: return false
return isService(uClass) && return isService(uClass) &&
(isCalledDuringInit(readOrWriteActionCall) || (isCalledDuringInit(readOrWriteActionCall) ||
isInMethodCalledDuringInit(uClass, readOrWriteActionCall) || isInMethodCalledDuringInit(uClass, readOrWriteActionCall, callContextHolder) ||
isCalledDuringPersistentStateComponentInit(uClass, readOrWriteActionCall)) isCalledDuringPersistentStateComponentInit(uClass, readOrWriteActionCall, callContextHolder))
} }
private fun UElement.getContainingNonCompanionObjectClass(): UClass? { private fun UElement.getContainingNonCompanionObjectClass(): UClass? {
@@ -100,13 +102,17 @@ internal class ReadOrWriteActionInServiceInitializationInspection : DevKitUastIn
return readOrWriteActionCall.getParentOfType<UField>() != null return readOrWriteActionCall.getParentOfType<UField>() != null
} }
private fun isInMethodCalledDuringInit(serviceClass: UClass, readOrWriteActionCall: UCallExpression): Boolean { private fun isInMethodCalledDuringInit(
serviceClass: UClass,
readOrWriteActionCall: UCallExpression,
callContextHolder: CallContextHolder,
): Boolean {
if (isCalledInAnonymousClassOrLambda(readOrWriteActionCall)) return false if (isCalledInAnonymousClassOrLambda(readOrWriteActionCall)) return false
val companionObject = serviceClass.innerClasses.firstOrNull { it.javaPsi.name == "Companion" } val companionObject = serviceClass.innerClasses.firstOrNull { it.javaPsi.name == "Companion" }
val initializationElements: List<UElement> = serviceClass.getInitializationElements() + val initializationElements: List<UElement> = serviceClass.getInitializationElements() +
(companionObject?.getInitializationElements() ?: emptyList()) (companionObject?.getInitializationElements() ?: emptyList())
val containingMethod = readOrWriteActionCall.getContainingUMethod() ?: return false val containingMethod = readOrWriteActionCall.getContainingUMethod() ?: return false
return containingMethod.isCalledInAnyOf(initializationElements) return containingMethod.isCalledInAnyOf(initializationElements, callContextHolder)
} }
private fun UClass.getInitializationElements(): List<UElement> { private fun UClass.getInitializationElements(): List<UElement> {
@@ -115,14 +121,15 @@ internal class ReadOrWriteActionInServiceInitializationInspection : DevKitUastIn
this.initializers.asList() this.initializers.asList()
} }
private fun UMethod.isCalledInAnyOf(elements: List<UElement>): Boolean { private fun UMethod.isCalledInAnyOf(potentialCallers: List<UElement>, callContextHolder: CallContextHolder): Boolean {
val checkedMethod = this val checkedMethod = this
var isCalledDuringInitialization = false var isCalledDuringInitialization = false
for (element in elements) { for (potentialCaller in potentialCallers) {
element.accept(object : UastVisitor { potentialCaller.accept(object : UastVisitor {
override fun visitCallExpression(node: UCallExpression): Boolean { override fun visitCallExpression(node: UCallExpression): Boolean {
if (checkedMethod == node.resolveToUElement() && !isCalledInAnonymousClassOrLambda(node)) { if (checkedMethod == node.resolveToUElement() && !isCalledInAnonymousClassOrLambda(node)) {
isCalledDuringInitialization = true isCalledDuringInitialization = true
callContextHolder.value = getContextText(checkedMethod, potentialCaller)
return SKIP_CHILDREN return SKIP_CHILDREN
} }
return VISIT_CHILDREN return VISIT_CHILDREN
@@ -137,10 +144,33 @@ internal class ReadOrWriteActionInServiceInitializationInspection : DevKitUastIn
return isCalledDuringInitialization return isCalledDuringInitialization
} }
private fun isCalledDuringPersistentStateComponentInit(serviceClass: UClass, readOrWriteActionCall: UCallExpression): Boolean { private fun getContextText(calledMethod: UMethod, caller: UElement): @Nls String? {
val callerName = when (caller) {
is UMethod ->
if (caller.isConstructor) message("inspection.read.or.write.action.during.service.init.message.context.constructor", caller.name)
else message("inspection.read.or.write.action.during.service.init.message.context.method", caller.name)
is UField ->
message("inspection.read.or.write.action.during.service.init.message.context.field", @Suppress("UElementAsPsi") caller.name)
is UClassInitializer ->
// there is no API in UAST to distinguish companion object context, but we can live with it
if (caller.isStatic) message("inspection.read.or.write.action.during.service.init.message.context.static.initializer")
else message("inspection.read.or.write.action.during.service.init.message.context.instance.initializer")
else -> return null
}
return message("inspection.read.or.write.action.during.service.init.message.context", "'${calledMethod.name}'", callerName)
}
private fun isCalledDuringPersistentStateComponentInit(
serviceClass: UClass,
readOrWriteActionCall: UCallExpression,
callContextHolder: CallContextHolder,
): Boolean {
return isPersistentStateComponent(serviceClass) && return isPersistentStateComponent(serviceClass) &&
(isCalledPersistentStateComponentInitMethods(readOrWriteActionCall) || (isCalledDuringPersistentStateComponentInitMethods(readOrWriteActionCall) ||
isInMethodCalledDuringPersistentStateComponentInit(serviceClass, readOrWriteActionCall)) isInMethodCalledDuringPersistentStateComponentInit(serviceClass, readOrWriteActionCall, callContextHolder))
} }
private fun isPersistentStateComponent(serviceClass: UClass): Boolean { private fun isPersistentStateComponent(serviceClass: UClass): Boolean {
@@ -148,26 +178,33 @@ internal class ReadOrWriteActionInServiceInitializationInspection : DevKitUastIn
return JvmInheritanceUtil.isInheritor(servicePsiClass, PersistentStateComponent::class.java.canonicalName) return JvmInheritanceUtil.isInheritor(servicePsiClass, PersistentStateComponent::class.java.canonicalName)
} }
private fun isCalledPersistentStateComponentInitMethods(readOrWriteActionCall: UCallExpression): Boolean { private fun isCalledDuringPersistentStateComponentInitMethods(readOrWriteActionCall: UCallExpression): Boolean {
if (isCalledInAnonymousClassOrLambda(readOrWriteActionCall)) return false if (isCalledInAnonymousClassOrLambda(readOrWriteActionCall)) return false
val containingMethod = readOrWriteActionCall.getContainingUMethod() ?: return false val containingMethod = readOrWriteActionCall.getContainingUMethod() ?: return false
return PERSISTENT_STATE_COMPONENT_INIT_METHOD_NAMES.contains(containingMethod.name) return PERSISTENT_STATE_COMPONENT_INIT_METHOD_NAMES.contains(containingMethod.name)
} }
private fun isInMethodCalledDuringPersistentStateComponentInit(serviceClass: UClass, readOrWriteActionCall: UCallExpression): Boolean { private fun isInMethodCalledDuringPersistentStateComponentInit(
serviceClass: UClass, readOrWriteActionCall: UCallExpression,
callContextHolder: CallContextHolder,
): Boolean {
if (isCalledInAnonymousClassOrLambda(readOrWriteActionCall)) return false if (isCalledInAnonymousClassOrLambda(readOrWriteActionCall)) return false
val lifecycleMethods: List<UMethod> = serviceClass.methods.filter { PERSISTENT_STATE_COMPONENT_INIT_METHOD_NAMES.contains(it.name) } val lifecycleMethods: List<UMethod> = serviceClass.methods.filter { PERSISTENT_STATE_COMPONENT_INIT_METHOD_NAMES.contains(it.name) }
val containingMethod = readOrWriteActionCall.getContainingUMethod() ?: return false val containingMethod = readOrWriteActionCall.getContainingUMethod() ?: return false
return containingMethod.isCalledInAnyOf(lifecycleMethods) return containingMethod.isCalledInAnyOf(lifecycleMethods, callContextHolder)
} }
private fun registerProblem(uCallExpression: UCallExpression, actionType: ActionType, holder: ProblemsHolder) { private fun registerProblem(
uCallExpression: UCallExpression,
actionType: ActionType,
holder: ProblemsHolder,
callContextHolder: CallContextHolder,
) {
val anchor = uCallExpression.methodIdentifier?.sourcePsi ?: return val anchor = uCallExpression.methodIdentifier?.sourcePsi ?: return
holder.registerProblem( val callContext = callContextHolder.value ?: ""
anchor, val message = if (actionType == ActionType.READ) message("inspection.read.or.write.action.during.service.init.message.read", callContext)
if (actionType == ActionType.READ) DevKitBundle.message("inspection.read.or.write.action.during.service.init.message.read") else message("inspection.read.or.write.action.during.service.init.message.write", callContext)
else DevKitBundle.message("inspection.read.or.write.action.during.service.init.message.write") holder.registerProblem(anchor, message)
)
} }
private enum class ActionType { private enum class ActionType {
@@ -176,4 +213,6 @@ internal class ReadOrWriteActionInServiceInitializationInspection : DevKitUastIn
fun isReadOrWrite() = this == READ || this == WRITE fun isReadOrWrite() = this == READ || this == WRITE
} }
private class CallContextHolder(@Nls var value: String? = null)
} }

View File

@@ -44,12 +44,12 @@ class ReadOrWriteActionInServiceInitializationInspectionTest : ReadOrWriteAction
String v3 = getV3(); String v3 = getV3();
private String getV3() { private String getV3() {
return ReadAction.<error descr="Do not run read actions during service initialization">compute</error>(() -> {return "";}); return ReadAction.<error descr="Do not run read actions during service initialization ('getV3' is called in 'v3' field initializer)">compute</error>(() -> {return "";});
} }
static final String v4 = getV4(); static final String v4 = getV4();
private static String getV4() { private static String getV4() {
return ReadAction.<error descr="Do not run read actions during service initialization">computeCancellable</error>(() -> {return "";}); return ReadAction.<error descr="Do not run read actions during service initialization ('getV4' is called in 'v4' field initializer)">computeCancellable</error>(() -> {return "";});
} }
static { static {
@@ -63,7 +63,7 @@ class ReadOrWriteActionInServiceInitializationInspectionTest : ReadOrWriteAction
} }
private static void writeActionMethodUsedInStaticInitBlock() { private static void writeActionMethodUsedInStaticInitBlock() {
WriteAction.<error descr="Do not run write actions during service initialization">run</error>(() -> {}); WriteAction.<error descr="Do not run write actions during service initialization ('writeActionMethodUsedInStaticInitBlock' is called in static initialization block)">run</error>(() -> {});
} }
TestService() { TestService() {
@@ -74,7 +74,7 @@ class ReadOrWriteActionInServiceInitializationInspectionTest : ReadOrWriteAction
private void readActionMethodUsedInConstructor() { private void readActionMethodUsedInConstructor() {
ReadAction.nonBlocking(() -> "") ReadAction.nonBlocking(() -> "")
.<error descr="Do not run read actions during service initialization">executeSynchronously</error>(); .<error descr="Do not run read actions during service initialization ('readActionMethodUsedInConstructor' is called in 'TestService' constructor or init block)">executeSynchronously</error>();
} }
public void notUsedInInit() { public void notUsedInInit() {
@@ -200,6 +200,7 @@ class ReadOrWriteActionInServiceInitializationInspectionTest : ReadOrWriteAction
@Override @Override
public void loadState(@NotNull State state) { public void loadState(@NotNull State state) {
String value = ReadAction.<error descr="Do not run read actions during service initialization">compute</error>(() -> {return "";}); String value = ReadAction.<error descr="Do not run read actions during service initialization">compute</error>(() -> {return "";});
readActionMethodUsedInLoadState();
myState = state; myState = state;
} }
@@ -210,6 +211,11 @@ class ReadOrWriteActionInServiceInitializationInspectionTest : ReadOrWriteAction
@Override public void noStateLoaded() { @Override public void noStateLoaded() {
WriteAction.<error descr="Do not run write actions during service initialization">run</error>(() -> {}); WriteAction.<error descr="Do not run write actions during service initialization">run</error>(() -> {});
} }
private void readActionMethodUsedInLoadState() {
ReadAction.nonBlocking(() -> "")
.<error descr="Do not run read actions during service initialization ('readActionMethodUsedInLoadState' is called in 'loadState' method)">executeSynchronously</error>();
}
@Override @Override
public @Nullable State getState() { public @Nullable State getState() {

View File

@@ -36,7 +36,7 @@ class KtReadOrWriteActionInServiceInitializationInspectionTest : ReadOrWriteActi
var v1: String = ReadAction.<error descr="Do not run read actions during service initialization">compute</error><String, RuntimeException> { "any" } var v1: String = ReadAction.<error descr="Do not run read actions during service initialization">compute</error><String, RuntimeException> { "any" }
var v2: String = getV2() var v2: String = getV2()
private fun getV2(): String { private fun getV2(): String {
return ReadAction.<error descr="Do not run read actions during service initialization">compute</error><String, RuntimeException> { "any" } return ReadAction.<error descr="Do not run read actions during service initialization ('getV2' is called in 'v2' field initializer)">compute</error><String, RuntimeException> { "any" }
} }
init { init {
@@ -46,7 +46,7 @@ class KtReadOrWriteActionInServiceInitializationInspectionTest : ReadOrWriteActi
} }
private fun readActionMethodUsedInInitBlock() { private fun readActionMethodUsedInInitBlock() {
ReadAction.nonBlocking<String> { "any" }.<error descr="Do not run read actions during service initialization">executeSynchronously</error>() ReadAction.nonBlocking<String> { "any" }.<error descr="Do not run read actions during service initialization ('readActionMethodUsedInInitBlock' is called in 'TestService' constructor or init block)">executeSynchronously</error>()
} }
fun notUsedInInit() { fun notUsedInInit() {
@@ -60,7 +60,7 @@ class KtReadOrWriteActionInServiceInitializationInspectionTest : ReadOrWriteActi
val v3: String = ReadAction.<error descr="Do not run read actions during service initialization">computeCancellable</error><String, RuntimeException> { "any" } val v3: String = ReadAction.<error descr="Do not run read actions during service initialization">computeCancellable</error><String, RuntimeException> { "any" }
val v4: String = getV4() val v4: String = getV4()
private fun getV4(): String { private fun getV4(): String {
return ReadAction.<error descr="Do not run read actions during service initialization">computeCancellable</error><String, RuntimeException> { "any" } return ReadAction.<error descr="Do not run read actions during service initialization ('getV4' is called in 'v4' field initializer)">computeCancellable</error><String, RuntimeException> { "any" }
} }
init { init {
@@ -74,7 +74,7 @@ class KtReadOrWriteActionInServiceInitializationInspectionTest : ReadOrWriteActi
} }
private fun writeActionMethodUsedInCompanionObjectInitBlock() { private fun writeActionMethodUsedInCompanionObjectInitBlock() {
WriteAction.<error descr="Do not run write actions during service initialization">run</error><RuntimeException> { WriteAction.<error descr="Do not run write actions during service initialization ('writeActionMethodUsedInCompanionObjectInitBlock' is called in 'Companion' constructor or init block)">run</error><RuntimeException> {
// do something // do something
} }
} }
@@ -181,9 +181,14 @@ class KtReadOrWriteActionInServiceInitializationInspectionTest : ReadOrWriteActi
@Suppress("UNUSED_VARIABLE") @Suppress("UNUSED_VARIABLE")
override fun loadState(state: State) { override fun loadState(state: State) {
val value = ReadAction.<error descr="Do not run read actions during service initialization">compute</error><String, RuntimeException> { "any" } val value = ReadAction.<error descr="Do not run read actions during service initialization">compute</error><String, RuntimeException> { "any" }
readActionMethodUsedInLoadState()
myState = state myState = state
} }
private fun readActionMethodUsedInLoadState() {
ReadAction.nonBlocking<String> { "any" }.<error descr="Do not run read actions during service initialization ('readActionMethodUsedInLoadState' is called in 'loadState' method)">executeSynchronously</error>()
}
override fun initializeComponent() { override fun initializeComponent() {
ReadAction.<error descr="Do not run read actions during service initialization">compute</error><String, RuntimeException> { "any" } ReadAction.<error descr="Do not run read actions during service initialization">compute</error><String, RuntimeException> { "any" }
} }