From ca6c11124848583c79e124b1b32085631437ab4c Mon Sep 17 00:00:00 2001 From: Alexey Belkov Date: Mon, 27 Nov 2023 17:16:41 +0400 Subject: [PATCH] [java-analysis] "Infer Nullity" cleanup * Formatting * Nullability annotations * Small refactoring to improve readability * No semantic changes GitOrigin-RevId: ac02f7c7561ef85da500153823f538ce786e1f89 --- .../inferNullity/AnnotateTask.java | 3 +- .../InferNullityAnnotationsAction.java | 41 +++-- .../inferNullity/NullityInferrer.java | 141 +++++++++--------- 3 files changed, 98 insertions(+), 87 deletions(-) diff --git a/java/java-impl/src/com/intellij/codeInspection/inferNullity/AnnotateTask.java b/java/java-impl/src/com/intellij/codeInspection/inferNullity/AnnotateTask.java index 1a3810b3af96..c3e495702581 100644 --- a/java/java-impl/src/com/intellij/codeInspection/inferNullity/AnnotateTask.java +++ b/java/java-impl/src/com/intellij/codeInspection/inferNullity/AnnotateTask.java @@ -7,6 +7,7 @@ import com.intellij.openapi.project.Project; import com.intellij.usageView.UsageInfo; import com.intellij.util.SequentialModalProgressTask; import com.intellij.util.SequentialTask; +import org.jetbrains.annotations.NotNull; class AnnotateTask implements SequentialTask { private final Project myProject; @@ -17,7 +18,7 @@ class AnnotateTask implements SequentialTask { private final int myTotal; private final NullableNotNullManager myNotNullManager; - AnnotateTask(Project project, SequentialModalProgressTask progressTask, UsageInfo[] infos) { + AnnotateTask(Project project, SequentialModalProgressTask progressTask, UsageInfo @NotNull [] infos) { myProject = project; myInfos = infos; myNotNullManager = NullableNotNullManager.getInstance(myProject); diff --git a/java/java-impl/src/com/intellij/codeInspection/inferNullity/InferNullityAnnotationsAction.java b/java/java-impl/src/com/intellij/codeInspection/inferNullity/InferNullityAnnotationsAction.java index bcff728d55bd..264813a8108a 100644 --- a/java/java-impl/src/com/intellij/codeInspection/inferNullity/InferNullityAnnotationsAction.java +++ b/java/java-impl/src/com/intellij/codeInspection/inferNullity/InferNullityAnnotationsAction.java @@ -85,7 +85,7 @@ public class InferNullityAnnotationsAction extends BaseAnalysisAction { final Set modulesWithLL = new HashSet<>(); final JavaPsiFacade javaPsiFacade = JavaPsiFacade.getInstance(project); final String defaultNullable = NullableNotNullManager.getInstance(project).getDefaultNullable(); - final int[] fileCount = new int[] {0}; + final int[] fileCount = new int[]{0}; if (!progressManager.runProcessWithProgressSynchronously(() -> scope.accept(new PsiElementVisitor() { final private Set processed = new HashSet<>(); @@ -120,7 +120,7 @@ public class InferNullityAnnotationsAction extends BaseAnalysisAction { } if (!modulesWithoutAnnotations.isEmpty()) { addAnnotationsDependency(project, modulesWithoutAnnotations, defaultNullable, - JavaBundle.message("action.title.infer.nullity.annotations")) + JavaBundle.message("action.title.infer.nullity.annotations")) .onSuccess(__ -> { restartAnalysis(project, scope); }); @@ -144,7 +144,7 @@ public class InferNullityAnnotationsAction extends BaseAnalysisAction { public static Promise addAnnotationsDependency(@NotNull final Project project, @NotNull final Set modulesWithoutAnnotations, - @NotNull String annoFQN, final @NlsContexts.DialogTitle String title) { + @NotNull String annoFQN, final @NlsContexts.DialogTitle @NotNull String title) { final Library annotationsLib = LibraryUtil.findLibraryByClass(annoFQN, project); if (annotationsLib != null) { String message = JavaBundle.message("dialog.message.modules.dont.refer.to.existing.annotations.library", @@ -173,8 +173,8 @@ public class InferNullityAnnotationsAction extends BaseAnalysisAction { } @NotNull - private static OkCancelDialogBuilder createDependencyDialog(@NlsContexts.DialogTitle String title, - @NlsContexts.DialogMessage String message, + private static OkCancelDialogBuilder createDependencyDialog(@NlsContexts.DialogTitle @NotNull String title, + @NlsContexts.DialogMessage @NotNull String message, @NotNull Project project) { return MessageDialogBuilder.okCancel(title, message) .icon(Messages.getErrorIcon()) @@ -228,7 +228,8 @@ public class InferNullityAnnotationsAction extends BaseAnalysisAction { .message("action.description.infer.nullity.annotations"), true, project)) { return null; } - } else { + } + else { searchForUsages.run(); } @@ -239,7 +240,7 @@ public class InferNullityAnnotationsAction extends BaseAnalysisAction { return myUi.get().getCheckBox().isSelected(); } - private static Runnable applyRunnable(final Project project, final Computable computable) { + private static @NotNull Runnable applyRunnable(final @NotNull Project project, final @NotNull Computable computable) { return () -> { final LocalHistoryAction action = LocalHistory.getInstance().startAction( JavaBundle.message("action.description.infer.nullity.annotations")); @@ -270,7 +271,7 @@ public class InferNullityAnnotationsAction extends BaseAnalysisAction { }; CommandProcessor.getInstance() .executeCommand(project, command, JavaBundle.message("action.title.infer.nullity.annotations"), null); - NOTIFICATION_GROUP.createNotification(JavaBundle.message("notification.content.added.annotations", command.myCount), + NOTIFICATION_GROUP.createNotification(JavaBundle.message("notification.content.added.annotations", command.myCount), NotificationType.INFORMATION) .notify(project); } @@ -285,15 +286,18 @@ public class InferNullityAnnotationsAction extends BaseAnalysisAction { }; } - protected void restartAnalysis(final Project project, final AnalysisScope scope) { + protected void restartAnalysis(final @NotNull Project project, final @NotNull AnalysisScope scope) { AppUIExecutor.onUiThread().inSmartMode(project).execute(() -> analyze(project, scope)); } - private void showUsageView(@NotNull Project project, final UsageInfo[] usageInfos, @NotNull AnalysisScope scope) { + private void showUsageView(@NotNull Project project, final UsageInfo @NotNull [] usageInfos, @NotNull AnalysisScope scope) { final UsageTarget[] targets = UsageTarget.EMPTY_ARRAY; final Ref convertUsagesRef = new Ref<>(); - if (!ProgressManager.getInstance().runProcessWithProgressSynchronously(() -> ApplicationManager.getApplication().runReadAction(() -> convertUsagesRef.set(UsageInfo2UsageAdapter.convert(usageInfos))), - JavaBundle.message("progress.title.preprocess.usages"), true, project)) return; + if (!ProgressManager.getInstance().runProcessWithProgressSynchronously( + () -> ApplicationManager.getApplication().runReadAction(() -> convertUsagesRef.set(UsageInfo2UsageAdapter.convert(usageInfos))), + JavaBundle.message("progress.title.preprocess.usages"), true, project)) { + return; + } if (convertUsagesRef.isNull()) return; final Usage[] usages = convertUsagesRef.get(); @@ -304,14 +308,18 @@ public class InferNullityAnnotationsAction extends BaseAnalysisAction { presentation.setShowCancelButton(true); presentation.setUsagesString(RefactoringBundle.message("usageView.usagesText")); - final UsageView usageView = UsageViewManager.getInstance(project).showUsages(targets, usages, presentation, rerunFactory(project, scope)); + final UsageView usageView = + UsageViewManager.getInstance(project).showUsages(targets, usages, presentation, rerunFactory(project, scope)); final Runnable refactoringRunnable = applyRunnable(project, () -> { final Set infos = UsageViewUtil.getNotExcludedUsageInfos(usageView); return infos.toArray(UsageInfo.EMPTY_ARRAY); }); - String canNotMakeString = "Cannot perform operation.\nThere were changes in code after usages have been found.\nPlease perform operation search again."; + String canNotMakeString = """ + Cannot perform operation. + There were changes in code after usages have been found. + Please perform operation search again."""; usageView.addPerformOperationAction(refactoringRunnable, JavaBundle.message("action.title.infer.nullity.annotations"), canNotMakeString, JavaBundle.message("action.title.infer.nullity.annotations"), false); @@ -322,7 +330,8 @@ public class InferNullityAnnotationsAction extends BaseAnalysisAction { return () -> new UsageInfoSearcherAdapter() { @Override protected UsageInfo @NotNull [] findUsages() { - return ObjectUtils.notNull(InferNullityAnnotationsAction.this.findUsages(project, scope, scope.getFileCount()), UsageInfo.EMPTY_ARRAY); + return ObjectUtils.notNull(InferNullityAnnotationsAction.this.findUsages(project, scope, scope.getFileCount()), + UsageInfo.EMPTY_ARRAY); } @Override @@ -333,7 +342,7 @@ public class InferNullityAnnotationsAction extends BaseAnalysisAction { } @Override - protected JComponent getAdditionalActionSettings(@NotNull Project project, BaseAnalysisActionDialog dialog) { + protected @Nullable JComponent getAdditionalActionSettings(@NotNull Project project, BaseAnalysisActionDialog dialog) { InferNullityAdditionalUi ui = myUi.get(); ui.getCheckBox().setSelected(PropertiesComponent.getInstance().getBoolean(ANNOTATE_LOCAL_VARIABLES)); return ui.getPanel(); diff --git a/java/java-impl/src/com/intellij/codeInspection/inferNullity/NullityInferrer.java b/java/java-impl/src/com/intellij/codeInspection/inferNullity/NullityInferrer.java index 6424fd03dcb0..d0b267ff1744 100644 --- a/java/java-impl/src/com/intellij/codeInspection/inferNullity/NullityInferrer.java +++ b/java/java-impl/src/com/intellij/codeInspection/inferNullity/NullityInferrer.java @@ -42,7 +42,7 @@ public class NullityInferrer { private final boolean myAnnotateLocalVariables; private final SmartPointerManager myPointerManager; - public NullityInferrer(boolean annotateLocalVariables, Project project) { + public NullityInferrer(boolean annotateLocalVariables, @NotNull Project project) { myAnnotateLocalVariables = annotateLocalVariables; myPointerManager = SmartPointerManager.getInstance(project); } @@ -76,9 +76,13 @@ public class NullityInferrer { return false; } } - else if (!variable.hasModifierProperty(PsiModifier.FINAL) || variable instanceof PsiParameter && ((PsiParameter)variable).getDeclarationScope() instanceof PsiCatchSection) { + else if (!variable.hasModifierProperty(PsiModifier.FINAL)) { return false; } + else if (variable instanceof PsiParameter parameter && parameter.getDeclarationScope() instanceof PsiCatchSection) { + return false; + } + final Query references = ReferencesSearch.search(variable); for (final PsiReference reference : references) { final PsiElement element = reference.getElement(); @@ -89,11 +93,13 @@ public class NullityInferrer { if (!(parent instanceof PsiAssignmentExpression assignment)) { continue; } + if (assignment.getLExpression().equals(element) && !expressionIsNeverNull(assignment.getRExpression())) { return false; } } + return true; } @@ -102,6 +108,7 @@ public class NullityInferrer { if (initializer != null && expressionIsSometimesNull(initializer)) { return true; } + final Query references = ReferencesSearch.search(variable); for (final PsiReference reference : references) { final PsiElement element = reference.getElement(); @@ -112,10 +119,12 @@ public class NullityInferrer { if (!(parent instanceof PsiAssignmentExpression assignment)) { continue; } + if (assignment.getLExpression().equals(element) && expressionIsSometimesNull(assignment.getRExpression())) { return true; } } + return false; } @@ -132,7 +141,7 @@ public class NullityInferrer { } @TestOnly - public void apply(final Project project) { + public void apply(final @NotNull Project project) { final NullableNotNullManager manager = NullableNotNullManager.getInstance(project); for (SmartPsiElementPointer pointer : myNullableSet) { annotateNullable(project, manager, pointer.getElement()); @@ -153,28 +162,28 @@ public class NullityInferrer { JavaBundle.message("dialog.title.infer.nullity.results"))); } - private static boolean annotateNotNull(Project project, - NullableNotNullManager manager, - final PsiModifierListOwner element) { + private static boolean annotateNotNull(@NotNull Project project, + @NotNull NullableNotNullManager manager, + final @Nullable PsiModifierListOwner element) { if (element == null || - element instanceof PsiField && ((PsiField)element).hasInitializer() && element.hasModifierProperty(PsiModifier.FINAL)) { + element instanceof PsiField field && field.hasInitializer() && field.hasModifierProperty(PsiModifier.FINAL)) { return false; } invoke(project, element, manager.getDefaultNotNull(), manager.getDefaultNullable()); return true; } - private static boolean annotateNullable(Project project, - NullableNotNullManager manager, - final PsiModifierListOwner element) { + private static boolean annotateNullable(@NotNull Project project, + @NotNull NullableNotNullManager manager, + final @Nullable PsiModifierListOwner element) { if (element == null) return false; invoke(project, element, manager.getDefaultNullable(), manager.getDefaultNotNull()); return true; } - private static void invoke(final Project project, - final PsiModifierListOwner element, - final String fqn, final String toRemove) { + private static void invoke(final @NotNull Project project, + final @NotNull PsiModifierListOwner element, + final @NotNull String fqn, final @NotNull String toRemove) { new AddAnnotationFix(fqn, element, toRemove).invoke(project, null, element.getContainingFile()); } @@ -182,7 +191,7 @@ public class NullityInferrer { return myNotNullSet.size() + myNullableSet.size(); } - public static boolean apply(Project project, NullableNotNullManager manager, UsageInfo info) { + public static boolean apply(@NotNull Project project, @NotNull NullableNotNullManager manager, UsageInfo info) { if (info instanceof NullableUsageInfo) { return annotateNullable(project, manager, (PsiModifierListOwner)info.getElement()); } @@ -192,33 +201,41 @@ public class NullityInferrer { return false; } - private boolean shouldIgnore(PsiModifierListOwner element) { - if (!myAnnotateLocalVariables){ - if (element instanceof PsiLocalVariable) return true; - if (element instanceof PsiParameter && ((PsiParameter)element).getDeclarationScope() instanceof PsiForeachStatement) return true; - } + private boolean shouldIgnore(@NotNull PsiModifierListOwner element) { + if (myAnnotateLocalVariables) return false; + if (element instanceof PsiLocalVariable) return true; + if (element instanceof PsiParameter parameter && parameter.getDeclarationScope() instanceof PsiForeachStatement) return true; return false; } - private void registerNullableAnnotation(@NotNull PsiModifierListOwner method) { - registerAnnotation(method, true); + private void registerNullableAnnotation(@NotNull PsiModifierListOwner declaration) { + registerAnnotation(declaration, true); } - private void registerNotNullAnnotation(@NotNull PsiModifierListOwner method) { - registerAnnotation(method, false); + private void registerNotNullAnnotation(@NotNull PsiModifierListOwner declaration) { + registerAnnotation(declaration, false); } - private void registerAnnotation(@NotNull PsiModifierListOwner method, boolean isNullable) { - final SmartPsiElementPointer methodPointer = myPointerManager.createSmartPsiElementPointer(method); + private void registerAnnotation(@NotNull PsiModifierListOwner declaration, boolean isNullable) { + final SmartPsiElementPointer declarationPointer = myPointerManager.createSmartPsiElementPointer(declaration); if (isNullable) { - myNullableSet.add(methodPointer); + myNullableSet.add(declarationPointer); } else { - myNotNullSet.add(methodPointer); + myNotNullSet.add(declarationPointer); } numAnnotationsAdded++; } + private void registerAnnotationByNullAssignmentStatus(PsiVariable variable) { + if (variableNeverAssignedNull(variable)) { + registerNotNullAnnotation(variable); + } + if (variableSometimesAssignedNull(variable)) { + registerNullableAnnotation(variable); + } + } + private static final class NullableUsageInfo extends UsageInfo { private NullableUsageInfo(@NotNull PsiElement element) { super(element); @@ -231,12 +248,12 @@ public class NullityInferrer { } } - void collect(List usages) { + void collect(@NotNull List usages) { collect(usages, true); collect(usages, false); } - private void collect(List usages, boolean nullable) { + private void collect(@NotNull List usages, boolean nullable) { final List> set = nullable ? myNullableSet : myNotNullSet; for (SmartPsiElementPointer elementPointer : set) { ReadAction.run(() -> { @@ -274,7 +291,7 @@ public class NullityInferrer { return myNotNullSet.contains(pointer) || myNullableSet.contains(pointer); } - private class NullityInferrerVisitor extends JavaRecursiveElementWalkingVisitor{ + private class NullityInferrerVisitor extends JavaRecursiveElementWalkingVisitor { @Override public void visitMethod(@NotNull PsiMethod method) { @@ -282,6 +299,7 @@ public class NullityInferrer { if (method.isConstructor() || method.getReturnType() instanceof PsiPrimitiveType) { return; } + final Collection overridingMethods = OverridingMethodsSearch.search(method).findAll(); for (final PsiMethod overridingMethod : overridingMethods) { if (isNullable(overridingMethod)) { @@ -289,47 +307,39 @@ public class NullityInferrer { return; } } + final NullableNotNullManager manager = NullableNotNullManager.getInstance(method.getProject()); if (!manager.isNotNull(method, false) && manager.isNotNull(method, true)) { registerNotNullAnnotation(method); return; } + if (hasNullability(method)) { return; } Nullability nullability = DfaUtil.inferMethodNullability(method); - if (nullability == Nullability.NULLABLE) { - registerNullableAnnotation(method); - return; - } + switch (nullability) { + case NULLABLE -> registerNullableAnnotation(method); - if (nullability == Nullability.NOT_NULL) { - for (final PsiMethod overridingMethod : overridingMethods) { - if (!isNotNull(overridingMethod)) { - return; + case NOT_NULL -> { + for (final PsiMethod overridingMethod : overridingMethods) { + if (!isNotNull(overridingMethod)) { + return; + } } + registerNotNullAnnotation(method); } - //and check that all the submethods are not nullable - registerNotNullAnnotation(method); } } - @Override public void visitLocalVariable(@NotNull PsiLocalVariable variable) { super.visitLocalVariable(variable); - if (variable.getType() instanceof PsiPrimitiveType || - isNotNull(variable) || isNullable(variable)) { + if (variable.getType() instanceof PsiPrimitiveType || hasNullability(variable)) { return; } - - if (variableNeverAssignedNull(variable)) { - registerNotNullAnnotation(variable); - } - if (variableSometimesAssignedNull(variable)) { - registerNullableAnnotation(variable); - } + registerAnnotationByNullAssignmentStatus(variable); } @Override @@ -338,6 +348,7 @@ public class NullityInferrer { if (parameter.getType() instanceof PsiPrimitiveType || hasNullability(parameter)) { return; } + final PsiElement grandParent = parameter.getDeclarationScope(); if (grandParent instanceof PsiMethod method) { if (method.getBody() != null) { @@ -345,6 +356,7 @@ public class NullityInferrer { registerNotNullAnnotation(parameter); return; } + for (PsiReferenceExpression expr : VariableAccessUtils.getVariableReferences(parameter, method)) { final PsiElement parent = PsiTreeUtil.skipParentsOfType(expr, PsiParenthesizedExpression.class, PsiTypeCastExpression.class); if (processParameter(parameter, expr, parent)) return; @@ -372,17 +384,13 @@ public class NullityInferrer { } } else { - if (variableNeverAssignedNull(parameter)) { - registerNotNullAnnotation(parameter); - } - if (variableSometimesAssignedNull(parameter)) { - registerNullableAnnotation(parameter); - } + registerAnnotationByNullAssignmentStatus(parameter); } } - private boolean processParameter(PsiParameter parameter, PsiReferenceExpression expr, PsiElement parent) { + private boolean processParameter(@NotNull PsiParameter parameter, @NotNull PsiReferenceExpression expr, PsiElement parent) { if (PsiUtil.isAccessedForWriting(expr)) return true; + if (parent instanceof PsiBinaryExpression binOp) { PsiExpression opposite = null; final PsiExpression lOperand = binOp.getLOperand(); @@ -393,6 +401,7 @@ public class NullityInferrer { else if (rOperand == expr) { opposite = lOperand; } + if (opposite != null && opposite.getType() == PsiTypes.nullType()) { if (DfaPsiUtil.isAssertionEffectively(binOp, binOp.getOperationTokenType() == JavaTokenType.NE)) { registerNotNullAnnotation(parameter); @@ -429,7 +438,8 @@ public class NullityInferrer { registerNotNullAnnotation(parameter); return true; } - } else if (parent instanceof PsiForeachStatement forEach) { + } + else if (parent instanceof PsiForeachStatement forEach) { if (forEach.getIteratedValue() == expr) { registerNotNullAnnotation(parameter); return true; @@ -461,26 +471,17 @@ public class NullityInferrer { } } } + return false; } @Override public void visitField(@NotNull PsiField field) { super.visitField(field); - if (field instanceof PsiEnumConstant) { + if (field instanceof PsiEnumConstant || field.getType() instanceof PsiPrimitiveType || hasNullability(field)) { return; } - if (field.getType() instanceof PsiPrimitiveType || - isNotNull(field) || isNullable(field)) { - return; - } - - if (variableNeverAssignedNull(field)) { - registerNotNullAnnotation(field); - } - if (variableSometimesAssignedNull(field)) { - registerNullableAnnotation(field); - } + registerAnnotationByNullAssignmentStatus(field); } } }