From e91500e82200c5c97bb2dccae35941d3931832ae Mon Sep 17 00:00:00 2001 From: Bas Leijdekkers Date: Tue, 16 May 2023 00:11:57 +0200 Subject: [PATCH] Java: add cast when necessary in more places (IJ-CR-107436, IDEA-248610) GitOrigin-RevId: 97431697ea4a59bb68a0d2309ad70d6bfe6e674f --- .../typeMigration/TypeMigrationLabeler.java | 2 +- .../rules/ThreadLocalConversionRule.java | 84 ++++++++++--------- .../threadLocal/afterTypeMismatch.java | 8 +- .../threadLocal/beforeTypeMismatch.java | 6 ++ .../threadLocal6/afterTypeMismatch.java | 15 ++++ .../threadLocal6/beforeTypeMismatch.java | 10 +++ .../directString/after/Test.items | 2 +- 7 files changed, 85 insertions(+), 42 deletions(-) create mode 100644 java/typeMigration/testData/intentions/threadLocal6/afterTypeMismatch.java create mode 100644 java/typeMigration/testData/intentions/threadLocal6/beforeTypeMismatch.java diff --git a/java/java-impl-refactorings/src/com/intellij/refactoring/typeMigration/TypeMigrationLabeler.java b/java/java-impl-refactorings/src/com/intellij/refactoring/typeMigration/TypeMigrationLabeler.java index 4f70bd5a5147..3cd3ee568c6a 100644 --- a/java/java-impl-refactorings/src/com/intellij/refactoring/typeMigration/TypeMigrationLabeler.java +++ b/java/java-impl-refactorings/src/com/intellij/refactoring/typeMigration/TypeMigrationLabeler.java @@ -1083,7 +1083,7 @@ public class TypeMigrationLabeler { return myRootsTree; } - TypeMigrationUsageInfo getCurrentRoot() { + public TypeMigrationUsageInfo getCurrentRoot() { return myCurrentRoot; } diff --git a/java/typeMigration/src/com/intellij/refactoring/typeMigration/rules/ThreadLocalConversionRule.java b/java/typeMigration/src/com/intellij/refactoring/typeMigration/rules/ThreadLocalConversionRule.java index 73693f735ffa..0d8aaaf5f66b 100644 --- a/java/typeMigration/src/com/intellij/refactoring/typeMigration/rules/ThreadLocalConversionRule.java +++ b/java/typeMigration/src/com/intellij/refactoring/typeMigration/rules/ThreadLocalConversionRule.java @@ -22,15 +22,14 @@ import java.util.List; public class ThreadLocalConversionRule extends TypeConversionRule { private static final Logger LOG = Logger.getInstance(ThreadLocalConversionRule.class); - @Override public TypeConversionDescriptorBase findConversion(PsiType from, PsiType to, PsiMember member, PsiExpression context, TypeMigrationLabeler labeler) { - if (to instanceof PsiClassType && isThreadLocalTypeMigration(from, (PsiClassType)to, context)) { - return findDirectConversion(context, to, from, labeler); + if (to instanceof PsiClassType toClassType && isThreadLocalTypeMigration(from, toClassType, context)) { + return findDirectConversion(context, toClassType, from, labeler); } return null; } @@ -64,13 +63,17 @@ public class ThreadLocalConversionRule extends TypeConversionRule { } @Nullable - private static TypeConversionDescriptor findDirectConversion(PsiElement context, PsiType to, PsiType from, TypeMigrationLabeler labeler) { - final PsiClass toTypeClass = PsiUtil.resolveClassInType(to); + private static TypeConversionDescriptor findDirectConversion(PsiElement context, + PsiClassType to, + PsiType from, + TypeMigrationLabeler labeler) { + final PsiClass toTypeClass = to.resolve(); LOG.assertTrue(toTypeClass != null); final PsiElement parent = context.getParent(); - if (parent instanceof PsiVariable && ((PsiVariable)parent).getInitializer() == context) { - return wrapWithNewExpression(to, from, (PsiExpression)context); + if (parent.equals(labeler.getCurrentRoot().getElement()) && ((PsiVariable)parent).getInitializer() == context) { + + return wrapWithNewExpression(from, to, (PsiExpression)context); } if (context instanceof PsiArrayAccessExpression) { return new TypeConversionDescriptor("$qualifier$[$val$]", "$qualifier$.get()[$val$]"); @@ -78,8 +81,10 @@ public class ThreadLocalConversionRule extends TypeConversionRule { if (parent instanceof PsiAssignmentExpression) { final IElementType operationSign = ((PsiAssignmentExpression)parent).getOperationTokenType(); if (operationSign == JavaTokenType.EQ) { - boolean rightInfected = ((PsiAssignmentExpression)parent).getLExpression() == context; - String replacement = rightInfected ? "$qualifier$ = $val$.get()" : "$qualifier$.set(" + toBoxed("$val$", from, context) + ")"; + final boolean rightInfected = ((PsiAssignmentExpression)parent).getLExpression() == context; + final String replacement = rightInfected + ? "$qualifier$ = $val$.get()" + : "$qualifier$.set(" + coerceType("$val$", from, to, context) + ")"; return new TypeConversionDescriptor("$qualifier$ = $val$", replacement, (PsiAssignmentExpression)parent); } } @@ -135,11 +140,11 @@ public class ThreadLocalConversionRule extends TypeConversionRule { if (lExpression instanceof PsiReferenceExpression) { final PsiElement element = ((PsiReferenceExpression)lExpression).resolve(); if (element instanceof PsiVariable && ((PsiVariable)element).hasModifierProperty(PsiModifier.FINAL)) { - return wrapWithNewExpression(to, from, ((PsiAssignmentExpression)context).getRExpression()); + return wrapWithNewExpression(from, to, ((PsiAssignmentExpression)context).getRExpression()); } } return new TypeConversionDescriptor("$qualifier$ = $val$", - "$qualifier$.set(" + toBoxed("$val$", from, context) + ")"); + "$qualifier$.set(" + coerceType("$val$", from, to, context) + ")"); } else { final PsiExpression rExpression = assignmentExpression.getRExpression(); @@ -155,39 +160,32 @@ public class ThreadLocalConversionRule extends TypeConversionRule { return null; } - private static TypeConversionDescriptor wrapWithNewExpression(PsiType to, PsiType from, PsiExpression initializer) { + private static TypeConversionDescriptor wrapWithNewExpression(PsiType from, PsiClassType to, PsiExpression initializer) { List toMakeFinal = TypeConversionRuleUtil.getVariablesToMakeFinal(initializer); if (toMakeFinal == null) return null; return new WrappingWithInnerClassOrLambdaDescriptor("$qualifier$", - createThreadLocalInitializerReplacement(to, from, initializer), + createThreadLocalInitializerReplacement(from, to, initializer), initializer, toMakeFinal); } - private static @NonNls String createThreadLocalInitializerReplacement(PsiType to, PsiType from, PsiElement context) { + private static @NonNls String createThreadLocalInitializerReplacement(PsiType from, PsiClassType to, PsiElement context) { if (PsiUtil.isLanguageLevel8OrHigher(context)) { - if (from instanceof PsiPrimitiveType) { - PsiType parameterType = ((PsiClassType)to).getParameters()[0]; - PsiPrimitiveType unboxed = PsiPrimitiveType.getUnboxedType(parameterType); - if (unboxed != null && !from.equals(unboxed)) { - return "java.lang.ThreadLocal.withInitial(() -> (" + unboxed.getCanonicalText() + ")$qualifier$)"; - } - } - return "java.lang.ThreadLocal.withInitial(() -> $qualifier$)"; + return "java.lang.ThreadLocal.withInitial(() -> " + coerceType("$qualifier$", from, to, context) + ")"; } - final String boxedTypeName = - from instanceof PsiPrimitiveType ? ((PsiPrimitiveType)from).getBoxedTypeName() : from.getCanonicalText(); - return (""" - new %s() { - @Override - protected %s initialValue() { - return %s; - } - }""").formatted(to.getCanonicalText(), - boxedTypeName, - from instanceof PsiPrimitiveType && !PsiUtil.isLanguageLevel5OrHigher(context) - ? "new " + ((PsiPrimitiveType)from).getBoxedTypeName() + "($qualifier$)" - : "$qualifier$"); + final StringBuilder result = new StringBuilder("new "); + result.append(to.getCanonicalText()).append("() {\n"); + if (PsiUtil.isLanguageLevel5OrHigher(context)) { + result.append(" @java.lang.Override\n"); + } + result.append(" protected ") + .append(PsiUtil.isLanguageLevel5OrHigher(context) ? to.getParameters()[0].getCanonicalText() : "java.lang.Object") + .append(" initialValue() {\n") + .append(" return ") + .append(coerceType("$qualifier$", from, to, context)).append(";\n") + .append(" }\n") + .append("}"); + return result.toString(); } private static @NonNls String toPrimitive(@NonNls String replaceByArg, PsiType from, PsiElement context) { @@ -199,8 +197,16 @@ public class ThreadLocalConversionRule extends TypeConversionRule { : "((" + from.getCanonicalText() + ")" + replaceByArg + ")"; } - private static @NonNls String toBoxed(@NonNls String replaceByArg, PsiType from, PsiElement context) { + private static @NonNls String coerceType(@NonNls String replaceByArg, PsiType from, PsiClassType to, PsiElement context) { if (PsiUtil.isLanguageLevel5OrHigher(context)) { + if (from instanceof PsiPrimitiveType) { + final PsiPrimitiveType unboxed = PsiPrimitiveType.getUnboxedType(to.getParameters()[0]); + if (unboxed != null && !from.equals(unboxed)) { + return context instanceof PsiLiteralExpression && PsiTypes.longType().equals(unboxed) + ? replaceByArg + "L" + : "(" + unboxed.getCanonicalText() + ")" + replaceByArg; + } + } return replaceByArg; } return from instanceof PsiPrimitiveType @@ -209,7 +215,7 @@ public class ThreadLocalConversionRule extends TypeConversionRule { } private static @NonNls String getBoxedWrapper(PsiType from, - PsiType to, + PsiClassType to, @NotNull @NonNls String arg, TypeMigrationLabeler labeler, PsiElement context, @@ -227,14 +233,14 @@ public class ThreadLocalConversionRule extends TypeConversionRule { final PsiType exprType = labeler.getTypeEvaluator().evaluateType( JavaPsiFacade.getElementFactory(threadLocalClass.getProject()).createExpressionFromText(tryType, context)); if (exprType != null && unboxedInitialType.isAssignableFrom(exprType)) { - return toBoxed(arg, from, context); + return coerceType(arg, from, to, context); } } return "new " + initial.getCanonicalText() + "((" + unboxedInitialType.getCanonicalText() + ")(" + arg + "))"; } } } - return toBoxed(arg, from, context); + return coerceType(arg, from, to, context); } private static final class WrappingWithInnerClassOrLambdaDescriptor extends ArrayInitializerAwareConversionDescriptor { diff --git a/java/typeMigration/testData/intentions/threadLocal/afterTypeMismatch.java b/java/typeMigration/testData/intentions/threadLocal/afterTypeMismatch.java index 6e306325163a..3464291febc2 100644 --- a/java/typeMigration/testData/intentions/threadLocal/afterTypeMismatch.java +++ b/java/typeMigration/testData/intentions/threadLocal/afterTypeMismatch.java @@ -1,4 +1,10 @@ // "Convert to 'ThreadLocal'" "true" class T { - private static final ThreadLocal l = ThreadLocal.withInitial(() -> (long) 1); // choose "Convert to ThreadLocal" intention + private static final ThreadLocal l = ThreadLocal.withInitial(() -> 1L); // choose "Convert to ThreadLocal" intention + + static { + int i = 1; + l.set((long) i); + long z = l.get(); + } } \ No newline at end of file diff --git a/java/typeMigration/testData/intentions/threadLocal/beforeTypeMismatch.java b/java/typeMigration/testData/intentions/threadLocal/beforeTypeMismatch.java index b9ac61bf54a6..b4c8ffe3d648 100644 --- a/java/typeMigration/testData/intentions/threadLocal/beforeTypeMismatch.java +++ b/java/typeMigration/testData/intentions/threadLocal/beforeTypeMismatch.java @@ -1,4 +1,10 @@ // "Convert to 'ThreadLocal'" "true" class T { private static final long l = 1; // choose "Convert to ThreadLocal" intention + + static { + int i = 1; + l = i; + long z = l; + } } \ No newline at end of file diff --git a/java/typeMigration/testData/intentions/threadLocal6/afterTypeMismatch.java b/java/typeMigration/testData/intentions/threadLocal6/afterTypeMismatch.java new file mode 100644 index 000000000000..69eb38d0299d --- /dev/null +++ b/java/typeMigration/testData/intentions/threadLocal6/afterTypeMismatch.java @@ -0,0 +1,15 @@ +// "Convert to 'ThreadLocal'" "true" +class T { + private static final ThreadLocal l = new ThreadLocal() { + @Override + protected Long initialValue() { + return 1L; + } + }; + + static { + int i = 1; + l.set((long) i); + long z = l.get(); + } +} \ No newline at end of file diff --git a/java/typeMigration/testData/intentions/threadLocal6/beforeTypeMismatch.java b/java/typeMigration/testData/intentions/threadLocal6/beforeTypeMismatch.java new file mode 100644 index 000000000000..5344d1aa7aa0 --- /dev/null +++ b/java/typeMigration/testData/intentions/threadLocal6/beforeTypeMismatch.java @@ -0,0 +1,10 @@ +// "Convert to 'ThreadLocal'" "true" +class T { + private static final long l = 1; + + static { + int i = 1; + l = i; + long z = l; + } +} \ No newline at end of file diff --git a/java/typeMigration/testData/refactoring/typeMigrationByThreadLocal/directString/after/Test.items b/java/typeMigration/testData/refactoring/typeMigrationByThreadLocal/directString/after/Test.items index af7e525fb346..6e9b767eca88 100644 --- a/java/typeMigration/testData/refactoring/typeMigrationByThreadLocal/directString/after/Test.items +++ b/java/typeMigration/testData/refactoring/typeMigrationByThreadLocal/directString/after/Test.items @@ -5,7 +5,7 @@ PsiReferenceExpression:myS : java.lang.ThreadLocal Conversions: "" -> new java.lang.ThreadLocal() { - @Override + @java.lang.Override protected java.lang.String initialValue() { return $qualifier$; }