From b263e82d099969b1575cd895ccb662942f359126 Mon Sep 17 00:00:00 2001 From: Pavel Dolgov Date: Wed, 19 Apr 2017 18:38:05 +0300 Subject: [PATCH] Java: Inspection that checks if a field or method accessed via reflection does exist and is visible (IDEA-168837) --- .../JavaReflectionInvocationInspection.java | 10 +- .../JavaReflectionMemberAccessInspection.java | 318 ++++++++++++++++++ .../impl/JavaReflectionReferenceUtil.java | 13 +- .../ConstructorParamCount.java | 4 + .../MethodParamCount.java | 4 + .../ConstructorExists.java | 42 +++ .../Constructors.java | 42 +++ .../FieldExists.java | 67 ++++ .../javaReflectionMemberAccess/Fields.java | 67 ++++ .../MethodExists.java | 98 ++++++ .../javaReflectionMemberAccess/Methods.java | 98 ++++++ .../JavaReflectionMemberAccessTest.kt | 55 +++ .../src/messages/InspectionsBundle.properties | 16 +- .../JavaReflectionMemberAccess.html | 14 + resources/src/META-INF/IdeaPlugin.xml | 4 + 15 files changed, 844 insertions(+), 8 deletions(-) create mode 100644 java/java-impl/src/com/intellij/codeInspection/reflectiveAccess/JavaReflectionMemberAccessInspection.java create mode 100644 java/java-tests/testData/inspection/javaReflectionMemberAccess/ConstructorExists.java create mode 100644 java/java-tests/testData/inspection/javaReflectionMemberAccess/Constructors.java create mode 100644 java/java-tests/testData/inspection/javaReflectionMemberAccess/FieldExists.java create mode 100644 java/java-tests/testData/inspection/javaReflectionMemberAccess/Fields.java create mode 100644 java/java-tests/testData/inspection/javaReflectionMemberAccess/MethodExists.java create mode 100644 java/java-tests/testData/inspection/javaReflectionMemberAccess/Methods.java create mode 100644 java/java-tests/testSrc/com/intellij/codeInspection/JavaReflectionMemberAccessTest.kt create mode 100644 resources-en/src/inspectionDescriptions/JavaReflectionMemberAccess.html diff --git a/java/java-impl/src/com/intellij/codeInspection/reflectiveAccess/JavaReflectionInvocationInspection.java b/java/java-impl/src/com/intellij/codeInspection/reflectiveAccess/JavaReflectionInvocationInspection.java index 7e4dd3af7055..23fa89cdc37b 100644 --- a/java/java-impl/src/com/intellij/codeInspection/reflectiveAccess/JavaReflectionInvocationInspection.java +++ b/java/java-impl/src/com/intellij/codeInspection/reflectiveAccess/JavaReflectionInvocationInspection.java @@ -78,10 +78,9 @@ public class JavaReflectionInvocationInspection extends BaseJavaBatchLocalInspec final List requiredTypes = getRequiredMethodArguments(methodCall.getMethodExpression().getQualifierExpression(), argumentOffset, methodPredicate); if (requiredTypes != null) { - final Arguments actualArguments = getActualMethodArguments(methodCall, argumentOffset); + final PsiExpressionList argumentList = methodCall.getArgumentList(); + final Arguments actualArguments = getActualMethodArguments(argumentList.getExpressions(), argumentOffset); if (actualArguments != null) { - - final PsiExpressionList argumentList = methodCall.getArgumentList(); if (requiredTypes.size() != actualArguments.expressions.length) { if (actualArguments.varargAsArray) { final PsiExpression[] expressions = argumentList.getExpressions(); @@ -149,8 +148,7 @@ public class JavaReflectionInvocationInspection extends BaseJavaBatchLocalInspec } @Nullable - private static Arguments getActualMethodArguments(PsiMethodCallExpression methodCall, int argumentOffset) { - final PsiExpression[] arguments = methodCall.getArgumentList().getExpressions(); + static Arguments getActualMethodArguments(PsiExpression[] arguments, int argumentOffset) { if (arguments.length == argumentOffset + 1) { final PsiExpression[] expressions = getVarargAsArray(arguments[argumentOffset]); if (expressions != null) { @@ -212,7 +210,7 @@ public class JavaReflectionInvocationInspection extends BaseJavaBatchLocalInspec return null; } - private static class Arguments { + static class Arguments { final PsiExpression[] expressions; final boolean varargAsArray; diff --git a/java/java-impl/src/com/intellij/codeInspection/reflectiveAccess/JavaReflectionMemberAccessInspection.java b/java/java-impl/src/com/intellij/codeInspection/reflectiveAccess/JavaReflectionMemberAccessInspection.java new file mode 100644 index 000000000000..9621ffcca80d --- /dev/null +++ b/java/java-impl/src/com/intellij/codeInspection/reflectiveAccess/JavaReflectionMemberAccessInspection.java @@ -0,0 +1,318 @@ +/* + * Copyright 2000-2017 JetBrains s.r.o. + * + * Licensed under the Apache License, Version 2.0 (the "License"); + * you may not use this file except in compliance with the License. + * You may obtain a copy of the License at + * + * http://www.apache.org/licenses/LICENSE-2.0 + * + * Unless required by applicable law or agreed to in writing, software + * distributed under the License is distributed on an "AS IS" BASIS, + * WITHOUT WARRANTIES OR CONDITIONS OF ANY KIND, either express or implied. + * See the License for the specific language governing permissions and + * limitations under the License. + */ +package com.intellij.codeInspection.reflectiveAccess; + +import com.intellij.codeInspection.BaseJavaBatchLocalInspectionTool; +import com.intellij.codeInspection.InspectionsBundle; +import com.intellij.codeInspection.ProblemsHolder; +import com.intellij.codeInspection.ui.ListTable; +import com.intellij.codeInspection.ui.ListWrappingTableModel; +import com.intellij.openapi.util.InvalidDataException; +import com.intellij.openapi.util.WriteExternalException; +import com.intellij.psi.*; +import com.intellij.psi.impl.source.resolve.reference.impl.JavaReflectionReferenceUtil; +import com.intellij.util.containers.ContainerUtil; +import com.intellij.util.ui.CheckBox; +import com.siyeh.ig.ui.UiUtils; +import org.jdom.Element; +import org.jetbrains.annotations.Contract; +import org.jetbrains.annotations.NotNull; +import org.jetbrains.annotations.Nullable; + +import javax.swing.*; +import java.awt.*; +import java.util.ArrayList; +import java.util.Collections; +import java.util.List; +import java.util.Set; +import java.util.stream.Collectors; + +import static com.intellij.psi.CommonClassNames.*; +import static com.intellij.psi.impl.source.resolve.reference.impl.JavaReflectionReferenceUtil.*; + +/** + * @author Pavel.Dolgov + */ +public class JavaReflectionMemberAccessInspection extends BaseJavaBatchLocalInspectionTool { + + private static final Set MEMBER_METHOD_NAMES = Collections.unmodifiableSet( + ContainerUtil.set(GET_FIELD, GET_DECLARED_FIELD, + GET_METHOD, GET_DECLARED_METHOD, + GET_CONSTRUCTOR, GET_DECLARED_CONSTRUCTOR)); + + private final List ignoredClassNames = new ArrayList<>(); + + public boolean checkMemberExistsInNonFinalClasses = true; + public String ignoredClassNamesString = JAVA_LANG_OBJECT + "," + JAVA_LANG_THROWABLE; + + public JavaReflectionMemberAccessInspection() { + parseSettings(); + } + + @Nullable + @Override + public JComponent createOptionsPanel() { + final JComponent panel = new JPanel(new GridBagLayout()); + + final ListTable table = new ListTable( + new ListWrappingTableModel(ignoredClassNames, InspectionsBundle.message( + "inspection.reflection.member.access.check.exists.exclude"))); + final JPanel tablePanel = UiUtils.createAddRemoveTreeClassChooserPanel(table, InspectionsBundle.message( + "inspection.reflection.member.access.check.exists.exclude.chooser")); + + final GridBagConstraints constraints = new GridBagConstraints(); + constraints.gridx = 0; + constraints.gridy = 0; + constraints.weightx = 1.0; + constraints.fill = GridBagConstraints.HORIZONTAL; + final CheckBox checkBox = new CheckBox(InspectionsBundle.message("inspection.reflection.member.access.check.exists"), + this, "checkMemberExistsInNonFinalClasses"); + panel.add(checkBox, constraints); + + constraints.weighty = 1.0; + constraints.gridy = 1; + constraints.fill = GridBagConstraints.BOTH; + panel.add(tablePanel, constraints); + + return panel; + } + + @Override + public void readSettings(@NotNull Element element) throws InvalidDataException { + super.readSettings(element); + parseSettings(); + } + + @Override + public void writeSettings(@NotNull Element element) throws WriteExternalException { + collectSettings(); + super.writeSettings(element); + } + + private void parseSettings() { + ignoredClassNames.clear(); + ContainerUtil.addAll(ignoredClassNames, ignoredClassNamesString.split(",")); + } + + private void collectSettings() { + ignoredClassNamesString = ignoredClassNames.stream().collect(Collectors.joining(",")); + } + + @NotNull + @Override + public PsiElementVisitor buildVisitor(@NotNull ProblemsHolder holder, boolean isOnTheFly) { + return new JavaElementVisitor() { + @Override + public void visitMethodCallExpression(PsiMethodCallExpression expression) { + super.visitMethodCallExpression(expression); + + final String referenceName = expression.getMethodExpression().getReferenceName(); + if (referenceName != null && MEMBER_METHOD_NAMES.contains(referenceName)) { + final PsiMethod method = expression.resolveMethod(); + if (method != null) { + final PsiClass containingClass = method.getContainingClass(); + if (containingClass != null && JAVA_LANG_CLASS.equals(containingClass.getQualifiedName())) { + switch (referenceName) { + + case GET_FIELD: + checkField(expression, false, holder); + break; + case GET_DECLARED_FIELD: + checkField(expression, true, holder); + break; + + case GET_METHOD: + checkMethod(expression, false, holder); + break; + case GET_DECLARED_METHOD: + checkMethod(expression, true, holder); + break; + + case GET_CONSTRUCTOR: + checkConstructor(expression, false, holder); + break; + case GET_DECLARED_CONSTRUCTOR: { + checkConstructor(expression, true, holder); + break; + } + } + } + } + } + } + }; + } + + private void checkField(@NotNull PsiMethodCallExpression callExpression, boolean isDeclared, @NotNull ProblemsHolder holder) { + final PsiExpression[] arguments = callExpression.getArgumentList().getExpressions(); + if (arguments.length != 0) { + final PsiExpression nameExpression = arguments[0]; + final String fieldName = getMemberName(nameExpression); + if (fieldName != null) { + final PsiClass psiClass = getPsiClass(callExpression); + if (psiClass != null) { + final PsiField field = psiClass.findFieldByName(fieldName, true); + if (field == null) { + if (reportUnresolvedMembersOf(psiClass)) { + holder.registerProblem(nameExpression, InspectionsBundle.message( + "inspection.reflection.member.access.cannot.resolve.field", fieldName)); + } + return; + } + if (isDeclared && field.getContainingClass() != psiClass) { + holder.registerProblem(nameExpression, InspectionsBundle.message( + "inspection.reflection.member.access.field.not.in.class", fieldName, psiClass.getQualifiedName())); + return; + } + if (!isDeclared && !field.hasModifierProperty(PsiModifier.PUBLIC)) { + holder.registerProblem(nameExpression, InspectionsBundle.message( + "inspection.reflection.member.access.field.not.public", fieldName)); + } + } + } + } + } + + private void checkMethod(@NotNull PsiMethodCallExpression callExpression, boolean isDeclared, @NotNull ProblemsHolder holder) { + final PsiExpression[] arguments = callExpression.getArgumentList().getExpressions(); + if (arguments.length != 0) { + final PsiExpression nameExpression = arguments[0]; + final String methodName = getMemberName(nameExpression); + if (methodName != null) { + final PsiClass psiClass = getPsiClass(callExpression); + if (psiClass != null) { + final PsiMethod[] methods = psiClass.findMethodsByName(methodName, true); + if (methods.length == 0) { + if (reportUnresolvedMembersOf(psiClass)) { + holder.registerProblem(nameExpression, InspectionsBundle.message( + "inspection.reflection.member.access.cannot.resolve.method", methodName)); + } + return; + } + final PsiMethod matchingMethod = matchMethod(methods, arguments, 1); + if (matchingMethod == null) { + if (reportUnresolvedMembersOf(psiClass)) { + holder.registerProblem(nameExpression, InspectionsBundle.message( + "inspection.reflection.member.access.cannot.resolve.method.arguments", methodName)); + } + return; + } + if (isDeclared && matchingMethod.getContainingClass() != psiClass) { + holder.registerProblem(nameExpression, InspectionsBundle.message( + "inspection.reflection.member.access.method.not.in.class", methodName, psiClass.getQualifiedName())); + return; + } + if (!isDeclared && !matchingMethod.hasModifierProperty(PsiModifier.PUBLIC)) { + holder.registerProblem(nameExpression, InspectionsBundle.message( + "inspection.reflection.member.access.method.not.public", methodName)); + } + } + } + } + } + + private void checkConstructor(@NotNull PsiMethodCallExpression callExpression, + boolean isDeclared, + @NotNull ProblemsHolder holder) { + final PsiClass psiClass = getPsiClass(callExpression); + if (psiClass != null) { + final PsiMethod[] methods = psiClass.getConstructors(); + final PsiExpression[] arguments = callExpression.getArgumentList().getExpressions(); + final PsiModifierListOwner constructorOrClass; + if (methods.length != 0) { + constructorOrClass = matchMethod(methods, arguments, 0); + } + else { + // implicit constructor + constructorOrClass = arguments.length == 0 ? psiClass : null; + } + if (constructorOrClass == null) { + if (reportUnresolvedMembersOf(psiClass)) { + holder.registerProblem(callExpression.getArgumentList(), InspectionsBundle.message( + "inspection.reflection.member.access.cannot.resolve.constructor.arguments")); + } + return; + } + if (!isDeclared && !constructorOrClass.hasModifierProperty(PsiModifier.PUBLIC)) { + holder.registerProblem(callExpression.getArgumentList(), InspectionsBundle.message( + "inspection.reflection.member.access.constructor.not.public")); + } + } + } + + private boolean reportUnresolvedMembersOf(@NotNull PsiClass psiClass) { + return (checkMemberExistsInNonFinalClasses || psiClass.hasModifierProperty(PsiModifier.FINAL)) && + !ignoredClassNames.contains(psiClass.getQualifiedName()); + } + + @Contract("null->null") + @Nullable + private static String getMemberName(@Nullable PsiExpression memberNameArgument) { + return computeConstantExpression(memberNameArgument, String.class); + } + + @Nullable + private static PsiClass getPsiClass(@NotNull PsiMethodCallExpression callExpression) { + return getReflectiveClass(callExpression.getMethodExpression().getQualifierExpression()); + } + + @Nullable + private static PsiMethod matchMethod(PsiMethod[] methods, PsiExpression[] arguments, int argumentOffset) { + final JavaReflectionInvocationInspection.Arguments methodArguments = + JavaReflectionInvocationInspection.getActualMethodArguments(arguments, argumentOffset); + if (methodArguments == null) { + return null; + } + final List argumentTypes = + ContainerUtil.map(methodArguments.expressions, JavaReflectionReferenceUtil::getReflectiveType); + + int mismatchCount = Integer.MAX_VALUE; + PsiMethod bestGuess = null; + for (PsiMethod method : methods) { + final int match = matchMethodArguments(method, argumentTypes); + if (match == 0) { + return method; + } + if (match < 0) { + continue; + } + if (mismatchCount > match) { + mismatchCount = match; + bestGuess = method; + } + } + return bestGuess; + } + + private static int matchMethodArguments(PsiMethod method, List argumentTypes) { + final PsiParameter[] parameters = method.getParameterList().getParameters(); + if (parameters.length != argumentTypes.size()) { + return -1; + } + int mismatchCount = 0; + for (int i = 0; i < parameters.length; i++) { + final ReflectiveType argumentType = argumentTypes.get(i); + if (argumentType == null) { + mismatchCount++; + continue; + } + if (!argumentType.isEqualTo(parameters[i].getType())) { + return -1; + } + } + return mismatchCount; + } +} diff --git a/java/java-impl/src/com/intellij/psi/impl/source/resolve/reference/impl/JavaReflectionReferenceUtil.java b/java/java-impl/src/com/intellij/psi/impl/source/resolve/reference/impl/JavaReflectionReferenceUtil.java index 7bd2c2bc9442..4abdd440a6ca 100644 --- a/java/java-impl/src/com/intellij/psi/impl/source/resolve/reference/impl/JavaReflectionReferenceUtil.java +++ b/java/java-impl/src/com/intellij/psi/impl/source/resolve/reference/impl/JavaReflectionReferenceUtil.java @@ -143,7 +143,7 @@ public class JavaReflectionReferenceUtil { if (typeArgument instanceof PsiCapturedWildcardType) { typeArgument = ((PsiCapturedWildcardType)typeArgument).getUpperBound(); } - final PsiClass argumentClass = PsiTypesUtil.getPsiClass(typeArgument); + final PsiClass argumentClass = unwrapTypeParameter(PsiTypesUtil.getPsiClass(typeArgument)); if (argumentClass != null && !isJavaLangObject(argumentClass)) { return ReflectiveType.create(argumentClass); } @@ -161,6 +161,17 @@ public class JavaReflectionReferenceUtil { return null; } + @Contract("null -> null") + @Nullable + private static PsiClass unwrapTypeParameter(@Nullable PsiClass psiClass) { + int preventEndlessLoop = 5; + while (psiClass instanceof PsiTypeParameter && --preventEndlessLoop > 0) { + final PsiClassType[] extendsList = psiClass.getExtendsListTypes(); + psiClass = extendsList.length != 0 ? extendsList[0].resolve() : null; + } + return psiClass; + } + @Contract("null,_->null") @Nullable public static T computeConstantExpression(@Nullable PsiExpression expression, @NotNull Class expectedType) { diff --git a/java/java-tests/testData/inspection/javaReflectionInvocation/ConstructorParamCount.java b/java/java-tests/testData/inspection/javaReflectionInvocation/ConstructorParamCount.java index 1c1c76f41f9d..e387d1eb237a 100644 --- a/java/java-tests/testData/inspection/javaReflectionInvocation/ConstructorParamCount.java +++ b/java/java-tests/testData/inspection/javaReflectionInvocation/ConstructorParamCount.java @@ -66,7 +66,11 @@ class ConstructorParamCount { Constructor m = cls.getConstructor(new Class[0]); m.newInstance(new Object[0]); + m.newInstance(new Object[]{}); m.newInstance(new Object[] {"abc"}); + + m = cls.getConstructor(new Class[]{}); + m.newInstance(new Object[]{}); } class Test { diff --git a/java/java-tests/testData/inspection/javaReflectionInvocation/MethodParamCount.java b/java/java-tests/testData/inspection/javaReflectionInvocation/MethodParamCount.java index 38562b4a48a3..5ce1b6a8dc2f 100644 --- a/java/java-tests/testData/inspection/javaReflectionInvocation/MethodParamCount.java +++ b/java/java-tests/testData/inspection/javaReflectionInvocation/MethodParamCount.java @@ -67,7 +67,11 @@ class MethodParamCount { Method m = cls.getMethod("m0", new Class[0]); m.invoke(obj, new Object[0]); + m.invoke(obj, new Object[]{}); m.invoke(obj, new Object[] {"abc"}); + + m = cls.getMethod("m0", new Class[]{}); + m.invoke(obj, new Object[]{}); } static class Test { diff --git a/java/java-tests/testData/inspection/javaReflectionMemberAccess/ConstructorExists.java b/java/java-tests/testData/inspection/javaReflectionMemberAccess/ConstructorExists.java new file mode 100644 index 000000000000..8cfa2866caee --- /dev/null +++ b/java/java-tests/testData/inspection/javaReflectionMemberAccess/ConstructorExists.java @@ -0,0 +1,42 @@ +class ConstructorExists { + void foo(Object o, Class c) throws Exception { + Class ca = A.class; + + ca.getConstructor(int.class); + ca.getConstructor(float.class); + ca.getConstructor(boolean.class); + ca.getConstructor(String.class); + ca.getConstructor(Exception.class); + + ca.getConstructor(int.class, boolean.class, String.class); + ca.getConstructor(int.class, c, String.class); + ca.getConstructor(int.class, o.getClass(), String.class); + + ca.getDeclaredConstructor(int.class); + ca.getDeclaredConstructor(float.class); + ca.getDeclaredConstructor(boolean.class); + ca.getDeclaredConstructor(String.class); + ca.getDeclaredConstructor(Exception.class); + + ca.getDeclaredConstructor(int.class, boolean.class, String.class); + ca.getDeclaredConstructor(int.class, c, String.class); + ca.getDeclaredConstructor(int.class, o.getClass(), String.class); + + C.class.getConstructor(int.class); + C.class.getConstructor(boolean.class); + } + + static class A { + public A(int n) {} + protected A(float f) {} + A(boolean b) {} + private A(String s) {} + + public A(int n, boolean b, String s) {} + public A(int n, boolean b, float f) {} + } + + static final class C extends A { + public C(int n) { super(n); } + } +} \ No newline at end of file diff --git a/java/java-tests/testData/inspection/javaReflectionMemberAccess/Constructors.java b/java/java-tests/testData/inspection/javaReflectionMemberAccess/Constructors.java new file mode 100644 index 000000000000..aba6c2d19443 --- /dev/null +++ b/java/java-tests/testData/inspection/javaReflectionMemberAccess/Constructors.java @@ -0,0 +1,42 @@ +class Constructors { + void foo(Object o, Class c) throws Exception { + Class ca = A.class; + + ca.getConstructor(int.class); + ca.getConstructor(float.class); + ca.getConstructor(boolean.class); + ca.getConstructor(String.class); + ca.getConstructor(Exception.class); + + ca.getConstructor(int.class, boolean.class, String.class); + ca.getConstructor(int.class, c, String.class); + ca.getConstructor(int.class, o.getClass(), String.class); + + ca.getDeclaredConstructor(int.class); + ca.getDeclaredConstructor(float.class); + ca.getDeclaredConstructor(boolean.class); + ca.getDeclaredConstructor(String.class); + ca.getDeclaredConstructor(Exception.class); + + ca.getDeclaredConstructor(int.class, boolean.class, String.class); + ca.getDeclaredConstructor(int.class, c, String.class); + ca.getDeclaredConstructor(int.class, o.getClass(), String.class); + + C.class.getConstructor(int.class); + C.class.getConstructor(boolean.class); + } + + static class A { + public A(int n) {} + protected A(float f) {} + A(boolean b) {} + private A (String s) {} + + public A(int n, boolean b, String s) {} + public A(int n, boolean b, float f) {} + } + + static final class C extends A { + public C(int n) { super(n); } + } +} \ No newline at end of file diff --git a/java/java-tests/testData/inspection/javaReflectionMemberAccess/FieldExists.java b/java/java-tests/testData/inspection/javaReflectionMemberAccess/FieldExists.java new file mode 100644 index 000000000000..876c25095a2c --- /dev/null +++ b/java/java-tests/testData/inspection/javaReflectionMemberAccess/FieldExists.java @@ -0,0 +1,67 @@ +class Fields { + void foo() throws Exception { + Class ca = A.class; + Class cb = B.class; + + ca.getField("a1"); + ca.getField("a2"); + ca.getField("a3"); + ca.getField("a4"); + + ca.getField("b1"); + ca.getField("b2"); + ca.getField("b3"); + ca.getField("b4"); + + cb.getField("a1"); + cb.getField("a2"); + cb.getField("a3"); + cb.getField("a4"); + + cb.getField("b1"); + cb.getField("b2"); + cb.getField("b3"); + cb.getField("b4"); + + ca.getDeclaredField("a1"); + ca.getDeclaredField("a2"); + ca.getDeclaredField("a3"); + ca.getDeclaredField("a4"); + + ca.getDeclaredField("b1"); + ca.getDeclaredField("b2"); + ca.getDeclaredField("b3"); + ca.getDeclaredField("b4"); + + cb.getDeclaredField("a1"); + cb.getDeclaredField("a2"); + cb.getDeclaredField("a3"); + cb.getDeclaredField("a4"); + + cb.getDeclaredField("b1"); + cb.getDeclaredField("b2"); + cb.getDeclaredField("b3"); + cb.getDeclaredField("b4"); + + C.class.getField("c"); + C.class.getField("d"); + } + + static class A { + public int a1; + protected int a2; + int a3; + private int a4; + } + + static class B extends A { + public int b1; + protected int b2; + int b3; + private int b4; + } + + static final class C extends A { + public int c; + } +} \ No newline at end of file diff --git a/java/java-tests/testData/inspection/javaReflectionMemberAccess/Fields.java b/java/java-tests/testData/inspection/javaReflectionMemberAccess/Fields.java new file mode 100644 index 000000000000..05b257ed1874 --- /dev/null +++ b/java/java-tests/testData/inspection/javaReflectionMemberAccess/Fields.java @@ -0,0 +1,67 @@ +class Fields { + void foo() throws Exception { + Class ca = A.class; + Class cb = B.class; + + ca.getField("a1"); + ca.getField("a2"); + ca.getField("a3"); + ca.getField("a4"); + + ca.getField("b1"); + ca.getField("b2"); + ca.getField("b3"); + ca.getField("b4"); + + cb.getField("a1"); + cb.getField("a2"); + cb.getField("a3"); + cb.getField("a4"); + + cb.getField("b1"); + cb.getField("b2"); + cb.getField("b3"); + cb.getField("b4"); + + ca.getDeclaredField("a1"); + ca.getDeclaredField("a2"); + ca.getDeclaredField("a3"); + ca.getDeclaredField("a4"); + + ca.getDeclaredField("b1"); + ca.getDeclaredField("b2"); + ca.getDeclaredField("b3"); + ca.getDeclaredField("b4"); + + cb.getDeclaredField("a1"); + cb.getDeclaredField("a2"); + cb.getDeclaredField("a3"); + cb.getDeclaredField("a4"); + + cb.getDeclaredField("b1"); + cb.getDeclaredField("b2"); + cb.getDeclaredField("b3"); + cb.getDeclaredField("b4"); + + C.class.getField("c"); + C.class.getField("d"); + } + + static class A { + public int a1; + protected int a2; + int a3; + private int a4; + } + + static class B extends A { + public int b1; + protected int b2; + int b3; + private int b4; + } + + static final class C extends A { + public int c; + } +} \ No newline at end of file diff --git a/java/java-tests/testData/inspection/javaReflectionMemberAccess/MethodExists.java b/java/java-tests/testData/inspection/javaReflectionMemberAccess/MethodExists.java new file mode 100644 index 000000000000..b2562329dae1 --- /dev/null +++ b/java/java-tests/testData/inspection/javaReflectionMemberAccess/MethodExists.java @@ -0,0 +1,98 @@ +class MethodExists { + void foo() throws Exception { + Class ca = A.class; + Class cb = B.class; + + ca.getMethod("a1", int.class); + ca.getMethod("a2", int.class); + ca.getMethod("a3", int.class); + ca.getMethod("a4", int.class); + + ca.getMethod("b1", int.class); + ca.getMethod("b2", int.class); + ca.getMethod("b3", int.class); + ca.getMethod("b4", int.class); + + cb.getMethod("a1", int.class); + cb.getMethod("a2", int.class); + cb.getMethod("a3", int.class); + cb.getMethod("a4", int.class); + + cb.getMethod("b1", int.class); + cb.getMethod("b2", int.class); + cb.getMethod("b3", int.class); + cb.getMethod("b4", int.class); + + + ca.getDeclaredMethod("a1", int.class); + ca.getDeclaredMethod("a2", int.class); + ca.getDeclaredMethod("a3", int.class); + ca.getDeclaredMethod("a4", int.class); + + ca.getDeclaredMethod("b1", int.class); + ca.getDeclaredMethod("b2", int.class); + ca.getDeclaredMethod("b3", int.class); + ca.getDeclaredMethod("b4", int.class); + + cb.getDeclaredMethod("a1", int.class); + cb.getDeclaredMethod("a2", int.class); + cb.getDeclaredMethod("a3", int.class); + cb.getDeclaredMethod("a4", int.class); + + cb.getDeclaredMethod("b1", int.class); + cb.getDeclaredMethod("b2", int.class); + cb.getDeclaredMethod("b3", int.class); + cb.getDeclaredMethod("b4", int.class); + + + ca.getMethod("a5", String.class); + ca.getMethod("a5", String[].class); + ca.getMethod("a5", String.class, String.class); + ca.getMethod("a5", int.class); + + ca.getDeclaredMethod("a5", String.class); + ca.getDeclaredMethod("a5", String[].class); + ca.getDeclaredMethod("a5", String.class, String.class); + ca.getDeclaredMethod("a5", int.class); + + cb.getMethod("a5", String.class); + cb.getMethod("a5", String[].class); + cb.getMethod("a5", String.class, String.class); + cb.getMethod("a5", int.class); + + cb.getDeclaredMethod("a5", String.class); + cb.getDeclaredMethod("a5", String[].class); + cb.getDeclaredMethod("a5", String.class, String.class); + cb.getDeclaredMethod("a5", int.class); + + C.class.getMethod("c", int.class); + C.class.getMethod("c", boolean.class); + C.class.getMethod("d", int.class); + } + + static class A { + public int a1(int n) {return n;} + protected int a2(int n) {return n;} + int a3(int n) {return n;} + private int a4(int n) {return n;} + + public String a5(String s) {return s;} + public String a5(String s, String t) {return s;} + protected String a5(String[] s) {return s[0];} + } + + static class B extends A { + public int b1(int n) {return n;} + protected int b2(int n) {return n;} + int b3(int n) {return n;} + private int b4(int n) {return n;} + + public String b5(String s) {return s;} + public String b5(String s, String t) {return s;} + protected String b5(String[] s) {return s[0];} + } + + static final class C extends A { + public int c(int n) {return n;} + } +} \ No newline at end of file diff --git a/java/java-tests/testData/inspection/javaReflectionMemberAccess/Methods.java b/java/java-tests/testData/inspection/javaReflectionMemberAccess/Methods.java new file mode 100644 index 000000000000..ee3386c3e59f --- /dev/null +++ b/java/java-tests/testData/inspection/javaReflectionMemberAccess/Methods.java @@ -0,0 +1,98 @@ +class Methods { + void foo() throws Exception { + Class ca = A.class; + Class cb = B.class; + + ca.getMethod("a1", int.class); + ca.getMethod("a2", int.class); + ca.getMethod("a3", int.class); + ca.getMethod("a4", int.class); + + ca.getMethod("b1", int.class); + ca.getMethod("b2", int.class); + ca.getMethod("b3", int.class); + ca.getMethod("b4", int.class); + + cb.getMethod("a1", int.class); + cb.getMethod("a2", int.class); + cb.getMethod("a3", int.class); + cb.getMethod("a4", int.class); + + cb.getMethod("b1", int.class); + cb.getMethod("b2", int.class); + cb.getMethod("b3", int.class); + cb.getMethod("b4", int.class); + + + ca.getDeclaredMethod("a1", int.class); + ca.getDeclaredMethod("a2", int.class); + ca.getDeclaredMethod("a3", int.class); + ca.getDeclaredMethod("a4", int.class); + + ca.getDeclaredMethod("b1", int.class); + ca.getDeclaredMethod("b2", int.class); + ca.getDeclaredMethod("b3", int.class); + ca.getDeclaredMethod("b4", int.class); + + cb.getDeclaredMethod("a1", int.class); + cb.getDeclaredMethod("a2", int.class); + cb.getDeclaredMethod("a3", int.class); + cb.getDeclaredMethod("a4", int.class); + + cb.getDeclaredMethod("b1", int.class); + cb.getDeclaredMethod("b2", int.class); + cb.getDeclaredMethod("b3", int.class); + cb.getDeclaredMethod("b4", int.class); + + + ca.getMethod("a5", String.class); + ca.getMethod("a5", String[].class); + ca.getMethod("a5", String.class, String.class); + ca.getMethod("a5", int.class); + + ca.getDeclaredMethod("a5", String.class); + ca.getDeclaredMethod("a5", String[].class); + ca.getDeclaredMethod("a5", String.class, String.class); + ca.getDeclaredMethod("a5", int.class); + + cb.getMethod("a5", String.class); + cb.getMethod("a5", String[].class); + cb.getMethod("a5", String.class, String.class); + cb.getMethod("a5", int.class); + + cb.getDeclaredMethod("a5", String.class); + cb.getDeclaredMethod("a5", String[].class); + cb.getDeclaredMethod("a5", String.class, String.class); + cb.getDeclaredMethod("a5", int.class); + + C.class.getMethod("c", int.class); + C.class.getMethod("c", boolean.class); + C.class.getMethod("d", int.class); + } + + static class A { + public int a1(int n) {return n;} + protected int a2(int n) {return n;} + int a3(int n) {return n;} + private int a4(int n) {return n;} + + public String a5(String s) {return s;} + public String a5(String s, String t) {return s;} + protected String a5(String[] s) {return s[0];} + } + + static class B extends A { + public int b1(int n) {return n;} + protected int b2(int n) {return n;} + int b3(int n) {return n;} + private int b4(int n) {return n;} + + public String b5(String s) {return s;} + public String b5(String s, String t) {return s;} + protected String b5(String[] s) {return s[0];} + } + + static final class C extends A { + public int c(int n) {return n;} + } +} \ No newline at end of file diff --git a/java/java-tests/testSrc/com/intellij/codeInspection/JavaReflectionMemberAccessTest.kt b/java/java-tests/testSrc/com/intellij/codeInspection/JavaReflectionMemberAccessTest.kt new file mode 100644 index 000000000000..d17104d02b42 --- /dev/null +++ b/java/java-tests/testSrc/com/intellij/codeInspection/JavaReflectionMemberAccessTest.kt @@ -0,0 +1,55 @@ +/* + * Copyright 2000-2017 JetBrains s.r.o. + * + * Licensed under the Apache License, Version 2.0 (the "License"); + * you may not use this file except in compliance with the License. + * You may obtain a copy of the License at + * + * http://www.apache.org/licenses/LICENSE-2.0 + * + * Unless required by applicable law or agreed to in writing, software + * distributed under the License is distributed on an "AS IS" BASIS, + * WITHOUT WARRANTIES OR CONDITIONS OF ANY KIND, either express or implied. + * See the License for the specific language governing permissions and + * limitations under the License. + */ +package com.intellij.codeInspection + +import com.intellij.JavaTestUtil +import com.intellij.codeInspection.reflectiveAccess.JavaReflectionMemberAccessInspection +import com.intellij.openapi.roots.LanguageLevelProjectExtension +import com.intellij.pom.java.LanguageLevel +import com.intellij.testFramework.LightProjectDescriptor +import com.intellij.testFramework.fixtures.LightCodeInsightFixtureTestCase + +/** + * @author Pavel.Dolgov + */ +class JavaReflectionMemberAccessTest : LightCodeInsightFixtureTestCase() { + + private val inspection = JavaReflectionMemberAccessInspection() + + override fun setUp() { + super.setUp() + LanguageLevelProjectExtension.getInstance(project).languageLevel = LanguageLevel.JDK_1_5 + myFixture.enableInspections(inspection) + } + + override fun getProjectDescriptor(): LightProjectDescriptor = LightCodeInsightFixtureTestCase.JAVA_1_5 + + override fun getBasePath() = JavaTestUtil.getRelativeJavaTestDataPath() + "/inspection/javaReflectionMemberAccess" + + fun testFields() = doTest() + fun testMethods() = doTest() + fun testConstructors() = doTest() + + fun testFieldExists() = doTest(true) + fun testMethodExists() = doTest(true) + fun testConstructorExists() = doTest(true) + + + private fun doTest(checkExists: Boolean = false) { + inspection.checkMemberExistsInNonFinalClasses = checkExists + myFixture.testHighlighting("${getTestName(false)}.java") + } +} \ No newline at end of file diff --git a/platform/platform-resources-en/src/messages/InspectionsBundle.properties b/platform/platform-resources-en/src/messages/InspectionsBundle.properties index 5ecc2e4e783d..a32002429a56 100644 --- a/platform/platform-resources-en/src/messages/InspectionsBundle.properties +++ b/platform/platform-resources-en/src/messages/InspectionsBundle.properties @@ -831,4 +831,18 @@ inspection.implicit.subclass.make.class.extendable=Make class extendable inspection.implicit.subclass.extendable=Make ''{0}'' overridable inspection.implicit.subclass.display.name=Final class is eligible for implicit subclassing -inspection.reflection.visibility.name=Reflective access across modules issues \ No newline at end of file +inspection.reflection.visibility.name=Reflective access across modules issues + +inspection.reflection.member.access.name=Reflective access to nonexistent/not visible class member +inspection.reflection.member.access.cannot.resolve.field=Cannot resolve field ''{0}'' +inspection.reflection.member.access.field.not.in.class=Field ''{0}'' is not declared in class ''{1}'' +inspection.reflection.member.access.field.not.public=Field ''{0}'' is not public +inspection.reflection.member.access.cannot.resolve.method=Cannot resolve method ''{0}'' +inspection.reflection.member.access.cannot.resolve.method.arguments=Cannot resolve method ''{0}'' with specified argument types +inspection.reflection.member.access.method.not.in.class=Method ''{0}'' is not declared in class ''{1}'' +inspection.reflection.member.access.method.not.public=Method ''{0}'' is not public +inspection.reflection.member.access.cannot.resolve.constructor.arguments=Cannot resolve constructor with specified argument types +inspection.reflection.member.access.constructor.not.public=Constructor is not public +inspection.reflection.member.access.check.exists=Check that field/method exists in non-final classes +inspection.reflection.member.access.check.exists.exclude=Exclude classes +inspection.reflection.member.access.check.exists.exclude.chooser=Class to exclude \ No newline at end of file diff --git a/resources-en/src/inspectionDescriptions/JavaReflectionMemberAccess.html b/resources-en/src/inspectionDescriptions/JavaReflectionMemberAccess.html new file mode 100644 index 000000000000..ec89baceaa49 --- /dev/null +++ b/resources-en/src/inspectionDescriptions/JavaReflectionMemberAccess.html @@ -0,0 +1,14 @@ + + +This inspection detects reflective access to fields and methods which don't exist or aren't visible. + +

+ With a 'final' class it's clear if there's a field or method with the specified name in the class. +

+

+ With non-final classes it's possible that a subclass has a field or method with that name, so there could be false positives. + You may use the inspection settings to get rid of such false positives, everywhere or with specific classes. +

+

New in 2017.2

+ + \ No newline at end of file diff --git a/resources/src/META-INF/IdeaPlugin.xml b/resources/src/META-INF/IdeaPlugin.xml index d84da084bbbe..2f073ac6faef 100644 --- a/resources/src/META-INF/IdeaPlugin.xml +++ b/resources/src/META-INF/IdeaPlugin.xml @@ -942,6 +942,10 @@ groupPath="Java" groupBundle="messages.InspectionsBundle" groupKey="group.names.reflective.access.issues" bundle="messages.InspectionsBundle" key="inspection.reflection.invocation.name" implementationClass="com.intellij.codeInspection.reflectiveAccess.JavaReflectionInvocationInspection"/> +