[lombok] IDEA-303185 Fix SneakyThrows, masking exceptions inside sibling constructors

GitOrigin-RevId: 9afd014cce3ea708d7cd88ce7a376a7d11bbab5b
This commit is contained in:
Michail Plushnikov
2023-12-11 19:51:25 +01:00
committed by intellij-monorepo-bot
parent 03da569fcd
commit c5257f0fea
7 changed files with 178 additions and 16 deletions

View File

@@ -3,6 +3,7 @@ package de.plushnikov.intellij.plugin.handler;
import com.intellij.codeInsight.CustomExceptionHandler;
import com.intellij.psi.*;
import com.intellij.psi.util.PsiTreeUtil;
import com.intellij.util.JavaPsiConstructorUtil;
import com.intellij.util.containers.ContainerUtil;
import de.plushnikov.intellij.plugin.LombokClassNames;
import de.plushnikov.intellij.plugin.util.PsiAnnotationSearchUtil;
@@ -11,6 +12,8 @@ import org.jetbrains.annotations.NotNull;
import org.jetbrains.annotations.Nullable;
import java.util.Collection;
import java.util.Collections;
import java.util.HashSet;
import java.util.List;
public class SneakyThrowsExceptionHandler extends CustomExceptionHandler {
@@ -19,25 +22,61 @@ public class SneakyThrowsExceptionHandler extends CustomExceptionHandler {
@Override
public boolean isHandled(@Nullable PsiElement element, @NotNull PsiClassType exceptionType, PsiElement topElement) {
final PsiCodeBlock containingCodeBlock = PsiTreeUtil.getParentOfType(element, PsiCodeBlock.class, false);
if (isCodeBlockWithExceptionInConstructorCall(containingCodeBlock, exceptionType)) {
// call to a sibling or super constructor is excluded from the @SneakyThrows treatment
return false;
}
PsiElement parent = PsiTreeUtil.getParentOfType(element, PsiLambdaExpression.class, PsiTryStatement.class, PsiMethod.class);
if (parent instanceof PsiLambdaExpression) {
// lambda it's another scope, @SneakyThrows annotation can't neglect exceptions in lambda only on method, constructor
return false;
} else if (parent instanceof PsiTryStatement && isHandledByTryCatch(exceptionType, (PsiTryStatement) parent)) {
}
if (parent instanceof PsiTryStatement && isHandledByTryCatch(exceptionType, (PsiTryStatement)parent)) {
// that exception MAY be already handled by regular try-catch statement
return false;
}
if (topElement instanceof PsiTryStatement && isHandledByTryCatch(exceptionType, (PsiTryStatement) topElement)) {
if (topElement instanceof PsiTryStatement && isHandledByTryCatch(exceptionType, (PsiTryStatement)topElement)) {
// that exception MAY be already handled by regular try-catch statement (don't forget about nested try-catch)
return false;
} else if (!(topElement instanceof PsiCodeBlock)) {
}
if (!(topElement instanceof PsiCodeBlock)) {
final PsiMethod psiMethod = PsiTreeUtil.getParentOfType(element, PsiMethod.class);
return psiMethod != null && isExceptionHandled(psiMethod, exceptionType);
}
return false;
}
private static boolean isCodeBlockWithExceptionInConstructorCall(@Nullable PsiCodeBlock codeBlock,
@NotNull PsiClassType exceptionType) {
final PsiMethod containingMethod = PsiTreeUtil.getParentOfType(codeBlock, PsiMethod.class);
if (null != containingMethod) {
final PsiMethodCallExpression thisOrSuperCallInConstructor =
JavaPsiConstructorUtil.findThisOrSuperCallInConstructor(containingMethod);
if (null != thisOrSuperCallInConstructor) {
ExceptionTypesCollector visitor = new ExceptionTypesCollector();
thisOrSuperCallInConstructor.accept(visitor);
return visitor.exceptionTypes.contains(exceptionType);
}
}
return false;
}
private static class ExceptionTypesCollector extends JavaRecursiveElementWalkingVisitor {
private final Collection<PsiClassType> exceptionTypes = new HashSet<>();
@Override
public void visitMethodCallExpression(@NotNull PsiMethodCallExpression expression) {
PsiMethod method = expression.resolveMethod();
if (method != null) {
Collections.addAll(exceptionTypes, method.getThrowsList().getReferencedTypes());
}
super.visitMethodCallExpression(expression);
}
}
private static boolean isHandledByTryCatch(@NotNull PsiClassType exceptionType, PsiTryStatement topElement) {
List<PsiType> caughtExceptions = ContainerUtil.map(topElement.getCatchBlockParameters(), PsiParameter::getType);
return isExceptionHandled(exceptionType, caughtExceptions);
@@ -49,11 +88,12 @@ public class SneakyThrowsExceptionHandler extends CustomExceptionHandler {
return false;
}
final Collection<PsiType> sneakedExceptionTypes = PsiAnnotationUtil.getAnnotationValues(psiAnnotation, PsiAnnotation.DEFAULT_REFERENCED_METHOD_NAME, PsiType.class);
final Collection<PsiType> sneakedExceptionTypes =
PsiAnnotationUtil.getAnnotationValues(psiAnnotation, PsiAnnotation.DEFAULT_REFERENCED_METHOD_NAME, PsiType.class);
//Default SneakyThrows handles all exceptions
return sneakedExceptionTypes.isEmpty()
|| sneakedExceptionTypes.iterator().next().equalsToText(JAVA_LANG_THROWABLE)
|| isExceptionHandled(exceptionClassType, sneakedExceptionTypes);
|| sneakedExceptionTypes.iterator().next().equalsToText(JAVA_LANG_THROWABLE)
|| isExceptionHandled(exceptionClassType, sneakedExceptionTypes);
}
private static boolean isExceptionHandled(@NotNull PsiClassType exceptionClassType, @NotNull Collection<PsiType> sneakedExceptionTypes) {
@@ -68,7 +108,7 @@ public class SneakyThrowsExceptionHandler extends CustomExceptionHandler {
if (null != unhandledExceptionClass) {
for (PsiType sneakedExceptionType : sneakedExceptionTypes) {
if (sneakedExceptionType instanceof PsiClassType) {
final PsiClass sneakedExceptionClass = ((PsiClassType) sneakedExceptionType).resolve();
final PsiClass sneakedExceptionClass = ((PsiClassType)sneakedExceptionType).resolve();
if (null != sneakedExceptionClass && unhandledExceptionClass.isInheritor(sneakedExceptionClass, true)) {
return true;

View File

@@ -1,9 +1,15 @@
package de.plushnikov.intellij.plugin.inspection;
import com.intellij.codeInsight.daemon.impl.quickfix.SafeDeleteFix;
import com.intellij.codeInspection.LocalQuickFix;
import com.intellij.codeInspection.ProblemHighlightType;
import com.intellij.codeInspection.ProblemsHolder;
import com.intellij.codeInspection.RemoveAnnotationQuickFix;
import com.intellij.psi.*;
import com.intellij.psi.util.PsiTreeUtil;
import com.intellij.refactoring.safeDelete.usageInfo.SafeDeleteAnnotation;
import com.intellij.refactoring.safeDelete.usageInfo.SafeDeleteOverrideAnnotation;
import com.intellij.util.JavaPsiConstructorUtil;
import de.plushnikov.intellij.plugin.LombokBundle;
import de.plushnikov.intellij.plugin.LombokClassNames;
import de.plushnikov.intellij.plugin.problem.LombokProblem;
@@ -73,6 +79,7 @@ public class LombokInspection extends LombokJavaInspectionBase {
private static void doAdditionalVerifications(@NotNull PsiAnnotation annotation, @NotNull Collection<LombokProblem> problems) {
verifyBuilderDefault(annotation, problems);
verifySneakyThrows(annotation, problems);
}
private static void verifyBuilderDefault(@NotNull PsiAnnotation annotation, @NotNull Collection<LombokProblem> problems) {
@@ -83,8 +90,27 @@ public class LombokInspection extends LombokJavaInspectionBase {
!parentOfAnnotation.hasAnnotation(LombokClassNames.SUPER_BUILDER)) {
final LombokProblemInstance problemInstance = new LombokProblemInstance(
LombokBundle.message("inspection.message.builder.default.requires.builder.annotation"), ProblemHighlightType.GENERIC_ERROR);
problemInstance.withLocalQuickFixes(() -> PsiQuickFixFactory.createAddAnnotationFix(LombokClassNames.BUILDER, parentOfAnnotation),
() -> PsiQuickFixFactory.createAddAnnotationFix(LombokClassNames.SUPER_BUILDER, parentOfAnnotation));
problemInstance.withLocalQuickFixes(
() -> PsiQuickFixFactory.createAddAnnotationFix(LombokClassNames.BUILDER, parentOfAnnotation),
() -> PsiQuickFixFactory.createAddAnnotationFix(LombokClassNames.SUPER_BUILDER, parentOfAnnotation));
problems.add(problemInstance);
}
}
}
}
private static void verifySneakyThrows(@NotNull PsiAnnotation annotation, @NotNull Collection<LombokProblem> problems) {
if (annotation.hasQualifiedName(LombokClassNames.SNEAKY_THROWS)) {
final PsiMethod parentOfAnnotation = PsiTreeUtil.getParentOfType(annotation, PsiMethod.class);
if (null != parentOfAnnotation && parentOfAnnotation.isConstructor()) {
final PsiMethodCallExpression thisOrSuperCallInConstructor =
JavaPsiConstructorUtil.findThisOrSuperCallInConstructor(parentOfAnnotation);
if (null != thisOrSuperCallInConstructor && parentOfAnnotation.getBody().getStatementCount() == 1) {
final LombokProblemInstance problemInstance = new LombokProblemInstance(
LombokBundle.message("inspection.message.sneakythrows.calls.to.sibling.super.constructors.excluded"),
ProblemHighlightType.GENERIC_ERROR_OR_WARNING);
problemInstance.withLocalQuickFixes(() -> new RemoveAnnotationQuickFix(annotation, parentOfAnnotation));
problems.add(problemInstance);
}
}
@@ -110,8 +136,10 @@ public class LombokInspection extends LombokJavaInspectionBase {
JavaResolveResult resolveResult = results.length == 1 ? results[0] : JavaResolveResult.EMPTY;
PsiElement resolved = resolveResult.getElement();
if (resolved instanceof LombokLightMethodBuilder && ((LombokLightMethodBuilder) resolved).getParameterList().getParameters().length != 0) {
holder.registerProblem(methodCall, LombokBundle.message("inspection.message.default.constructor.doesn.t.exist"), ProblemHighlightType.ERROR);
if (resolved instanceof LombokLightMethodBuilder &&
((LombokLightMethodBuilder)resolved).getParameterList().getParameters().length != 0) {
holder.registerProblem(methodCall, LombokBundle.message("inspection.message.default.constructor.doesn.t.exist"),
ProblemHighlightType.ERROR);
}
}
}

View File

@@ -112,6 +112,7 @@ inspection.message.builder.default.requires.builder.annotation=@Builder.Default
inspection.message.builder.default.requires.initializing.expression=@Builder.Default requires an initializing expression (' = something;').
inspection.message.builder.default.singular.cannot.be.mixed=@Builder.Default and @Singular cannot be mixed.
inspection.message.obtain.via.is.static.true.not.valid.unless.method.has.been.set=@ObtainVia(isStatic = true) is not valid unless 'method' has been set.
inspection.message.sneakythrows.calls.to.sibling.super.constructors.excluded=Calls to sibling / super constructors are always excluded from @SneakyThrows; @SneakyThrows has been ignored because there is no other code in this constructor.
inspection.message.lombok.builder.needs.proper.constructor.for.this.class=Lombok @Builder needs a proper constructor for this class
inspection.message.delegate.does.not.support.recursion.delegating=@Delegate does not support recursion (delegating to a type that itself has @Delegate members). Member ''{0}'' is @Delegate in type ''{1}''
inspection.message.delegate.legal.only.on.instance.fields=@Delegate is legal only on instance fields or no-argument instance methods.

View File

@@ -3,6 +3,7 @@ package de.plushnikov.intellij.plugin;
import com.intellij.codeInsight.ExceptionUtil;
import com.intellij.psi.*;
import com.intellij.testFramework.LightJavaCodeInsightTestCase;
import org.intellij.lang.annotations.Language;
import org.jetbrains.annotations.NonNls;
import org.jetbrains.annotations.NotNull;
import org.jetbrains.annotations.Nullable;
@@ -133,7 +134,7 @@ public class SneakyThrowsTest extends LightJavaCodeInsightTestCase {
private PsiMethodCallExpression createCall(@NonNls final String body) {
private PsiMethodCallExpression createCall(@NonNls @Language("JAVA") final String body) {
final PsiFile file = createTestFile(body);
PsiMethodCallExpression methodCall = findMethodCall(file);
assertNotNull(methodCall);
@@ -141,7 +142,7 @@ public class SneakyThrowsTest extends LightJavaCodeInsightTestCase {
}
@NotNull
private PsiFile createTestFile(@NonNls String body) {
private PsiFile createTestFile(@NonNls @Language("JAVA") String body) {
return createFile("test.java", "class Test { " + body +
"void throwsAnotherException() throws AnotherException {}" +
"void throwsMyException() throws MyException {}" +

View File

@@ -1,8 +1,8 @@
package de.plushnikov.intellij.plugin.highlights;
/**
* @author Lekanich
*/
import com.intellij.codeInspection.InspectionProfileEntry;
import de.plushnikov.intellij.plugin.inspection.LombokInspection;
public class SneakyThrowsHighlightTest extends AbstractLombokHighlightsTest {
@Override
@@ -10,6 +10,11 @@ public class SneakyThrowsHighlightTest extends AbstractLombokHighlightsTest {
return super.getBasePath() + "/sneakyThrows";
}
@Override
protected InspectionProfileEntry getInspection() {
return new LombokInspection();
}
public void testSneakThrowsDoesntCatchCaughtException() {
doTest();
}
@@ -17,4 +22,12 @@ public class SneakyThrowsHighlightTest extends AbstractLombokHighlightsTest {
public void testSneakThrowsDoesntCatchCaughtExceptionNested() {
doTest();
}
public void testSneakThrowsDoesntCatchExceptionFromSuperConstructor() {
doTest();
}
public void testSneakThrowsDoesntCatchExceptionFromThisConstructor() {
doTest();
}
}

View File

@@ -0,0 +1,40 @@
import lombok.SneakyThrows;
import java.io.FileInputStream;
import java.io.IOException;
public class SneakThrowsDoesntCatchExceptionFromSuperConstructor extends SomeSuperClass {
<warning descr="Calls to sibling / super constructors are always excluded from @SneakyThrows; @SneakyThrows has been ignored because there is no other code in this constructor.">@SneakyThrows</warning>
public SneakThrowsDoesntCatchExceptionFromSuperConstructor() {
<error descr="Unhandled exception: java.io.IOException">super</error>("somePath");
}
<warning descr="Calls to sibling / super constructors are always excluded from @SneakyThrows; @SneakyThrows has been ignored because there is no other code in this constructor.">@SneakyThrows</warning>
public SneakThrowsDoesntCatchExceptionFromSuperConstructor(int i, float f) {
super(<error descr="Unhandled exception: java.io.IOException">throwException</error>());
}
@SneakyThrows
public SneakThrowsDoesntCatchExceptionFromSuperConstructor(int i, float f, String s) {
super(<error descr="Unhandled exception: java.io.IOException">throwException</error>());
if (1 == 1) {
throw new Exception("12331323");
}
}
private static int throwException() throws IOException {
throw new IOException("Boom");
}
}
class SomeSuperClass {
private FileInputStream myStream;
public SomeSuperClass(int i) {
}
public SomeSuperClass(String path) throws IOException {
myStream = new FileInputStream(path);
}
}

View File

@@ -0,0 +1,39 @@
import lombok.SneakyThrows;
import java.io.FileInputStream;
import java.io.IOException;
public class SneakThrowsDoesntCatchExceptionFromThisConstructor {
private FileInputStream myStream;
<warning descr="Calls to sibling / super constructors are always excluded from @SneakyThrows; @SneakyThrows has been ignored because there is no other code in this constructor.">@SneakyThrows</warning>
public SneakThrowsDoesntCatchExceptionFromThisConstructor() {
<error descr="Unhandled exception: java.io.IOException">this</error>("somePath");
}
public SneakThrowsDoesntCatchExceptionFromThisConstructor(String path) throws IOException {
myStream = new FileInputStream(path);
}
<warning descr="Calls to sibling / super constructors are always excluded from @SneakyThrows; @SneakyThrows has been ignored because there is no other code in this constructor.">@SneakyThrows</warning>
public SneakThrowsDoesntCatchExceptionFromThisConstructor(int i, float f) {
this(<error descr="Unhandled exception: java.io.IOException">throwException</error>());
}
public SneakThrowsDoesntCatchExceptionFromThisConstructor(int i) {
}
@SneakyThrows
public SneakThrowsDoesntCatchExceptionFromThisConstructor(int i, float f, String s) {
this(<error descr="Unhandled exception: java.io.IOException">throwException</error>());
if (1 == 1) {
throw new Exception("12331323");
}
}
private static int throwException() throws IOException {
throw new IOException("Boom");
}
}