MDS/openide
1
0
Fork 0
mirror of https://gitflic.ru/project/openide/openide.git synced 2026-09-27 10:03:11 +07:00
Code Issues Packages Projects Releases Wiki Activity

IDEA-231906 IDEA should check violations of array/collection component nullability in overridden methods

Browse Source 
GitOrigin-RevId: e57066f48674a0b3491e1a57bbfa3f4cab4bd60d
This commit is contained in:
Peter Gromov
2020-03-06 19:01:54 +00:00
committed by intellij-monorepo-bot
parent f1fc795fdb
commit f3cb948613
4 changed files with 137 additions and 64 deletions
Download Patch File Download Diff File Expand all files Collapse all files
java/java-analysis-api/resources/messages/JavaAnalysisBundle.properties
+2
View File
@@ -155,6 +155,8 @@ annotate.overridden.methods.parameters=Annotate overridden method parameters as
anonymous.ref.loc.can.be.replaced.with.0=Anonymous #ref #loc can be replaced with {0}
anonymous.ref.loc.can.be.replaced.with.lambda=Anonymous #ref #loc can be replaced with lambda
assigning.a.collection.of.nullable.elements=Assigning a collection of nullable elements into a collection of non-null elements
nullable.stuff.error.overriding.nullable.with.notnull=Overriding a collection of nullable elements with a collection of non-null elements
nullable.stuff.error.overriding.notnull.with.nullable=Overriding a collection of not-null elements with a collection of nullable elements
comparision.between.object.and.primitive=Comparision between Object and primitive is illegal and is accepted in java 7 only
custom.exception.class.should.have.a.constructor=Custom exception class should have a constructor with a single message parameter of String type
delimiters.argument.contains.duplicated.characters=Delimiters argument contains duplicated characters
java/java-analysis-impl/src/com/intellij/codeInspection/nullable/NullableStuffInspectionBase.java
+100 -64
View File
@@ -278,7 +278,7 @@ public class NullableStuffInspectionBase extends AbstractBaseJavaLocalInspection
private void checkCollectionNullityOnAssignment(@NotNull PsiElement errorElement,
@Nullable PsiType expectedType,
@Nullable PsiType assignedType) {
if (isNullableNotNullCollectionConflict(expectedType, assignedType, new HashSet<>())) {
if (isNullableNotNullCollectionConflict(expectedType, assignedType, file, new HashSet<>())) {
holder.registerProblem(errorElement,
JavaAnalysisBundle
.message("assigning.a.collection.of.nullable.elements"),
@@ -287,35 +287,37 @@ public class NullableStuffInspectionBase extends AbstractBaseJavaLocalInspection
}
}
private boolean isNullableNotNullCollectionConflict(@Nullable PsiType expectedType,
@Nullable PsiType assignedType,
@NotNull Set<? super Couple<PsiType>> visited) {
if (!visited.add(Couple.of(expectedType, assignedType))) return false;
GlobalSearchScope scope = holder.getFile().getResolveScope();
if (isNullityConflict(JavaGenericsUtil.getCollectionItemType(expectedType, scope),
JavaGenericsUtil.getCollectionItemType(assignedType, scope))) {
return true;
}
for (int i = 0; i <= 1; i++) {
PsiType expectedArg = PsiUtil.substituteTypeParameter(expectedType, CommonClassNames.JAVA_UTIL_MAP, i, false);
PsiType assignedArg = PsiUtil.substituteTypeParameter(assignedType, CommonClassNames.JAVA_UTIL_MAP, i, false);
if (isNullityConflict(expectedArg, assignedArg) ||
expectedArg != null && assignedArg != null && isNullableNotNullCollectionConflict(expectedArg, assignedArg, visited)) {
return true;
}
}
return false;
}
private boolean isNullityConflict(PsiType expected, PsiType assigned) {
return DfaPsiUtil.getTypeNullability(expected) == Nullability.NOT_NULL && DfaPsiUtil.getTypeNullability(assigned) == Nullability.NULLABLE;
}
};
}
private static boolean isNullableNotNullCollectionConflict(@Nullable PsiType expectedType,
@Nullable PsiType assignedType,
@NotNull PsiFile place,
@NotNull Set<? super Couple<PsiType>> visited) {
if (!visited.add(Couple.of(expectedType, assignedType))) return false;
GlobalSearchScope scope = place.getResolveScope();
if (isNullityConflict(JavaGenericsUtil.getCollectionItemType(expectedType, scope),
JavaGenericsUtil.getCollectionItemType(assignedType, scope))) {
return true;
}
for (int i = 0; i <= 1; i++) {
PsiType expectedArg = PsiUtil.substituteTypeParameter(expectedType, CommonClassNames.JAVA_UTIL_MAP, i, false);
PsiType assignedArg = PsiUtil.substituteTypeParameter(assignedType, CommonClassNames.JAVA_UTIL_MAP, i, false);
if (isNullityConflict(expectedArg, assignedArg) ||
expectedArg != null && assignedArg != null && isNullableNotNullCollectionConflict(expectedArg, assignedArg, place, visited)) {
return true;
}
}
return false;
}
private static boolean isNullityConflict(PsiType expected, PsiType assigned) {
return DfaPsiUtil.getTypeNullability(expected) == Nullability.NOT_NULL && DfaPsiUtil.getTypeNullability(assigned) == Nullability.NULLABLE;
}
@Nullable
private String checkIndirectInheritance(PsiElement psiClass, PsiClass intf) {
for (PsiMethod intfMethod : intf.getAllMethods()) {
@@ -681,6 +683,15 @@ public class NullableStuffInspectionBase extends AbstractBaseJavaLocalInspection
createFixForNonAnnotatedOverridesNotNull(method, superMethod));
break;
}
PsiTypeElement returnTypeElement = method.getReturnTypeElement();
if (returnTypeElement != null &&
isNullableNotNullCollectionConflict(superMethod.getReturnType(), method.getReturnType(), holder.getFile(), new HashSet<>())) {
holder.registerProblem(returnTypeElement,
JavaAnalysisBundle.message("nullable.stuff.error.overriding.notnull.with.nullable"),
ProblemHighlightType.GENERIC_ERROR_OR_WARNING);
break;
}
}
}
@@ -724,49 +735,74 @@ public class NullableStuffInspectionBase extends AbstractBaseJavaLocalInspection
PsiParameter parameter = parameters[i];
if (parameter.getType() instanceof PsiPrimitiveType) continue;
List<PsiParameter> superParameters = new ArrayList<>();
for (PsiMethod superMethod : superMethods) {
PsiParameter[] _superParameters = superMethod.getParameterList().getParameters();
if (_superParameters.length == parameters.length) {
superParameters.add(_superParameters[i]);
}
}
List<PsiParameter> superParameters = getSuperParameters(superMethods, parameters, i);
PsiParameter nullableSuper = findNullableSuperForNotNullParameter(parameter, superParameters);
if (nullableSuper != null) {
PsiAnnotation annotation = AnnotationUtil.findAnnotation(parameter, nullableManager.getNotNulls(), true);
holder.registerProblem(annotation != null ? annotation : parameter.getNameIdentifier(),
JavaAnalysisBundle.message("inspection.nullable.problems.NotNull.parameter.overrides.Nullable",
getPresentableAnnoName(parameter),
getPresentableAnnoName(nullableSuper)),
ProblemHighlightType.GENERIC_ERROR_OR_WARNING);
}
PsiParameter notNullSuper = findNotNullSuperForNonAnnotatedParameter(nullableManager, parameter, superParameters);
if (notNullSuper != null) {
LocalQuickFix fix = AnnotationUtil.isAnnotatingApplicable(parameter, nullableManager.getDefaultNotNull())
? AddAnnotationPsiFix.createAddNotNullFix(parameter)
: createChangeDefaultNotNullFix(nullableManager, notNullSuper);
holder.registerProblem(parameter.getNameIdentifier(),
JavaAnalysisBundle.message("inspection.nullable.problems.parameter.overrides.NotNull", getPresentableAnnoName(notNullSuper)),
ProblemHighlightType.GENERIC_ERROR_OR_WARNING,
fix);
}
if (isNotNullParameterOverridingNonAnnotated(nullableManager, parameter, superParameters)) {
NullabilityAnnotationInfo info = nullableManager.findOwnNullabilityInfo(parameter);
assert info != null;
PsiAnnotation notNullAnnotation = info.getAnnotation();
boolean physical = PsiTreeUtil.isAncestor(parameter, notNullAnnotation, true);
final LocalQuickFix fix = physical ? new RemoveAnnotationQuickFix(notNullAnnotation, parameter) : null;
holder.registerProblem(physical ? notNullAnnotation : parameter.getNameIdentifier(),
JavaAnalysisBundle.message("inspection.nullable.problems.NotNull.parameter.overrides.not.annotated", getPresentableAnnoName(parameter)),
ProblemHighlightType.GENERIC_ERROR_OR_WARNING,
fix);
}
checkSuperParameterAnnotations(holder, nullableManager, parameter, superParameters);
checkNullLiteralArgumentOfNotNullParameterUsages(method, holder, nullableManager, isOnFly, i, parameter);
}
}
@NotNull
private static List<PsiParameter> getSuperParameters(List<? extends PsiMethod> superMethods, PsiParameter[] parameters, int i) {
List<PsiParameter> superParameters = new ArrayList<>();
for (PsiMethod superMethod : superMethods) {
PsiParameter[] _superParameters = superMethod.getParameterList().getParameters();
if (_superParameters.length == parameters.length) {
superParameters.add(_superParameters[i]);
}
}
return superParameters;
}
private void checkSuperParameterAnnotations(ProblemsHolder holder,
NullableNotNullManager nullableManager,
PsiParameter parameter,
List<PsiParameter> superParameters) {
PsiParameter nullableSuper = findNullableSuperForNotNullParameter(parameter, superParameters);
if (nullableSuper != null) {
PsiAnnotation annotation = AnnotationUtil.findAnnotation(parameter, nullableManager.getNotNulls(), true);
holder.registerProblem(annotation != null ? annotation : parameter.getNameIdentifier(),
JavaAnalysisBundle.message("inspection.nullable.problems.NotNull.parameter.overrides.Nullable",
getPresentableAnnoName(parameter),
getPresentableAnnoName(nullableSuper)),
ProblemHighlightType.GENERIC_ERROR_OR_WARNING);
}
PsiParameter notNullSuper = findNotNullSuperForNonAnnotatedParameter(nullableManager, parameter, superParameters);
if (notNullSuper != null) {
LocalQuickFix fix = AnnotationUtil.isAnnotatingApplicable(parameter, nullableManager.getDefaultNotNull())
? AddAnnotationPsiFix.createAddNotNullFix(parameter)
: createChangeDefaultNotNullFix(nullableManager, notNullSuper);
holder.registerProblem(parameter.getNameIdentifier(),
JavaAnalysisBundle.message("inspection.nullable.problems.parameter.overrides.NotNull", getPresentableAnnoName(notNullSuper)),
ProblemHighlightType.GENERIC_ERROR_OR_WARNING,
fix);
}
if (isNotNullParameterOverridingNonAnnotated(nullableManager, parameter, superParameters)) {
NullabilityAnnotationInfo info = nullableManager.findOwnNullabilityInfo(parameter);
assert info != null;
PsiAnnotation notNullAnnotation = info.getAnnotation();
boolean physical = PsiTreeUtil.isAncestor(parameter, notNullAnnotation, true);
final LocalQuickFix fix = physical ? new RemoveAnnotationQuickFix(notNullAnnotation, parameter) : null;
holder.registerProblem(physical ? notNullAnnotation : parameter.getNameIdentifier(),
JavaAnalysisBundle.message("inspection.nullable.problems.NotNull.parameter.overrides.not.annotated", getPresentableAnnoName(parameter)),
ProblemHighlightType.GENERIC_ERROR_OR_WARNING,
fix);
}
PsiTypeElement typeElement = parameter.getTypeElement();
if (typeElement != null) {
for (PsiParameter superParameter : superParameters) {
if (isNullableNotNullCollectionConflict(parameter.getType(), superParameter.getType(), holder.getFile(), new HashSet<>())) {
holder.registerProblem(typeElement,
JavaAnalysisBundle.message("nullable.stuff.error.overriding.nullable.with.notnull"),
ProblemHighlightType.GENERIC_ERROR_OR_WARNING);
break;
}
}
}
}
@Nullable
private PsiParameter findNotNullSuperForNonAnnotatedParameter(NullableNotNullManager nullableManager,
PsiParameter parameter,
java/java-tests/testData/inspection/nullableProblems/OverridingNotNullCollectionWithNullable.java
+30
View File
@@ -0,0 +1,30 @@
import typeUse.*;
import java.util.*;
abstract class Parent {
abstract @NotNull String @NotNull [] getStrings();
abstract @NotNull List<@NotNull String> getStringList();
abstract void foo(@Nullable String @NotNull [] p1,
@NotNull List<@Nullable String> p2);
}
class Child extends Parent {
@Override
@Nullable <warning descr="Overriding a collection of not-null elements with a collection of nullable elements">String @NotNull []</warning> getStrings() {
throw new UnsupportedOperationException();
}
@Override
@NotNull <warning descr="Overriding a collection of not-null elements with a collection of nullable elements">List<@Nullable String></warning> getStringList() {
throw new UnsupportedOperationException();
}
void foo(@NotNull <warning descr="Overriding a collection of nullable elements with a collection of non-null elements">String @NotNull []</warning> p1,
@NotNull <warning descr="Overriding a collection of nullable elements with a collection of non-null elements">List<@NotNull String></warning> p2) {
}
}
java/java-tests/testSrc/com/intellij/java/codeInspection/NullableStuffInspectionTest.java
+5
View File
@@ -254,6 +254,11 @@ public class NullableStuffInspectionTest extends LightJavaCodeInsightFixtureTest
doTest();
}
public void testOverridingNotNullCollectionWithNullable() {
DataFlowInspection8Test.setupTypeUseAnnotations("typeUse", myFixture);
doTest();
}
public void testNotNullCollectionItemWithNullableSuperType() {
DataFlowInspection8Test.setupTypeUseAnnotations("typeUse", myFixture);
doTest();