IDEA-357105 MagicConstantInspection must not ignore the current progress while doing heavy lifting

Otherwise, the whole IDE will be blocked for a long time

GitOrigin-RevId: 6fa8c7758708601c5cf86461035f5df60a1665a5
This commit is contained in:
Max Medvedev
2024-08-03 17:46:58 +02:00
committed by intellij-monorepo-bot
parent 323329f2d3
commit 625700c0d9
2 changed files with 27 additions and 13 deletions

View File

@@ -126,7 +126,7 @@ public final class MagicCompletionContributor extends CompletionContributor impl
if (resolved != null) {
result.add(Pair.create(resolved, operand.getType()));
// if something interesting assigned to this variable, e.g. magic method, suggest its magic too
MagicConstantInspection.processValuesFlownTo(operand, pos.getContainingFile(), pos.getManager(), expression -> {
MagicConstantInspection.processValuesFlownTo(operand, pos.getContainingFile(), pos.getManager(), true, expression -> {
PsiModifierListOwner assigned = resolveExpression(expression);
if (assigned != null) {
result.add(Pair.create(assigned, operand.getType()));

View File

@@ -21,6 +21,7 @@ import com.intellij.openapi.project.Project;
import com.intellij.openapi.projectRoots.Sdk;
import com.intellij.openapi.projectRoots.impl.JavaSdkImpl;
import com.intellij.openapi.roots.JdkUtils;
import com.intellij.openapi.util.Computable;
import com.intellij.openapi.util.Key;
import com.intellij.openapi.util.text.StringUtil;
import com.intellij.psi.*;
@@ -241,7 +242,7 @@ public final class MagicConstantInspection extends AbstractBaseJavaLocalInspecti
if (allowed == null) return;
PsiElement scope = PsiUtil.getTopLevelEnclosingCodeBlock(expression, null);
if (scope == null) scope = expression;
if (!isAllowed(expression, scope, allowed, expression.getManager(), null)) {
if (!isAllowed(expression, scope, allowed, expression.getManager(), null, holder.isOnTheFly())) {
registerProblem(expression, allowed, holder);
}
}
@@ -304,7 +305,8 @@ public final class MagicConstantInspection extends AbstractBaseJavaLocalInspecti
@NotNull ProblemsHolder holder) {
final PsiManager manager = PsiManager.getInstance(holder.getProject());
if (!argument.getTextRange().isEmpty() && !isAllowed(argument, parameter.getDeclarationScope(), allowedValues, manager, null)) {
if (!argument.getTextRange().isEmpty() &&
!isAllowed(argument, parameter.getDeclarationScope(), allowedValues, manager, null, holder.isOnTheFly())) {
registerProblem(argument, allowedValues, holder);
}
}
@@ -388,21 +390,25 @@ public final class MagicConstantInspection extends AbstractBaseJavaLocalInspecti
return null;
}
private static boolean isAllowed(final @NotNull PsiExpression argument, final @NotNull PsiElement scope,
private static boolean isAllowed(final @NotNull PsiExpression argument,
final @NotNull PsiElement scope,
final @NotNull AllowedValues allowedValues,
final @NotNull PsiManager manager,
@Nullable Set<PsiExpression> visited) {
if (isGoodExpression(argument, allowedValues, scope, manager, visited)) return true;
@Nullable Set<PsiExpression> visited,
boolean isOnTheFly) {
if (isGoodExpression(argument, allowedValues, scope, manager, visited, isOnTheFly)) return true;
return processValuesFlownTo(argument, scope, manager,
expression -> isGoodExpression(expression, allowedValues, scope, manager, visited));
isOnTheFly, expression -> isGoodExpression(expression, allowedValues, scope, manager, visited, isOnTheFly)
);
}
private static boolean isGoodExpression(@NotNull PsiExpression argument,
@NotNull AllowedValues allowedValues,
@NotNull PsiElement scope,
@NotNull PsiManager manager,
@Nullable Set<PsiExpression> visited) {
@Nullable Set<PsiExpression> visited,
boolean isOnTheFly) {
if (visited == null) visited = new HashSet<>();
if (!visited.add(argument)) return false;
if (argument instanceof PsiParenthesizedExpression ||
@@ -410,7 +416,7 @@ public final class MagicConstantInspection extends AbstractBaseJavaLocalInspecti
argument instanceof PsiSwitchExpression) {
List<PsiExpression> expressions = ExpressionUtils.nonStructuralChildren(argument).toList();
for (PsiExpression expression : expressions) {
if (!isAllowed(expression, scope, allowedValues, manager, visited)) {
if (!isAllowed(expression, scope, allowedValues, manager, visited, isOnTheFly)) {
return false;
}
}
@@ -432,7 +438,7 @@ public final class MagicConstantInspection extends AbstractBaseJavaLocalInspecti
if (JavaTokenType.OR.equals(tokenType) || JavaTokenType.XOR.equals(tokenType) ||
JavaTokenType.AND.equals(tokenType) || JavaTokenType.PLUS.equals(tokenType)) {
for (PsiExpression operand : polyadic.getOperands()) {
if (!isAllowed(operand, scope, allowedValues, manager, visited)) return false;
if (!isAllowed(operand, scope, allowedValues, manager, visited, isOnTheFly)) return false;
}
return true;
}
@@ -440,7 +446,7 @@ public final class MagicConstantInspection extends AbstractBaseJavaLocalInspecti
if (argument instanceof PsiPrefixExpression prefixExpression &&
JavaTokenType.TILDE.equals(prefixExpression.getOperationTokenType())) {
PsiExpression operand = prefixExpression.getOperand();
return operand == null || isAllowed(operand, scope, allowedValues, manager, visited);
return operand == null || isAllowed(operand, scope, allowedValues, manager, visited, isOnTheFly);
}
}
@@ -486,6 +492,7 @@ public final class MagicConstantInspection extends AbstractBaseJavaLocalInspecti
static boolean processValuesFlownTo(final @NotNull PsiExpression argument,
@NotNull PsiElement scope,
@NotNull PsiManager manager,
boolean isOnTheFly,
final @NotNull Processor<? super PsiExpression> processor) {
SliceAnalysisParams params = new SliceAnalysisParams();
params.dataFlowToThis = true;
@@ -494,8 +501,15 @@ public final class MagicConstantInspection extends AbstractBaseJavaLocalInspecti
SliceRootNode rootNode = new SliceRootNode(manager.getProject(), new DuplicateMap(),
LanguageSlicing.getProvider(argument).createRootUsage(argument, params));
Collection<? extends AbstractTreeNode<?>> children = ProgressManager.getInstance().runProcess(
() -> rootNode.getChildren().iterator().next().getChildren(), new ProgressIndicatorBase());
Computable<Collection<SliceNode>> computable = () -> rootNode.getChildren().iterator().next().getChildren();
Collection<? extends AbstractTreeNode<?>> children;
if (isOnTheFly) {
children = computable.compute();
}
else {
children = ProgressManager.getInstance().runProcess(
computable, new ProgressIndicatorBase());
}
for (AbstractTreeNode<?> child : children) {
SliceUsage usage = (SliceUsage)child.getValue();
PsiElement element = usage != null ? usage.getElement() : null;