From 948c3ff3c2f26a7ec7e0368077558f16ce8a4b75 Mon Sep 17 00:00:00 2001 From: SilverYoCha Date: Thu, 24 Jan 2019 09:22:52 +0100 Subject: [PATCH] Making the inspection able to deal also with complex field initializations, like conditional expression for example. Fixing some IDE warnings and common SonarQube feedback. --- .gitignore | 3 +- META-INF/plugin.xml | 12 +-- .../idea/copyConstructor/ConstructorUtil.java | 4 +- .../GenerateCopyConstructorHandler.java | 7 +- .../IncompleteCopyConstructorInspection.java | 77 ++++++++++++++----- .../IncompleteCopyConstructor.html | 3 +- 6 files changed, 71 insertions(+), 35 deletions(-) diff --git a/.gitignore b/.gitignore index 562b66d..f70ae86 100644 --- a/.gitignore +++ b/.gitignore @@ -1,2 +1,3 @@ .idea/ -out/ \ No newline at end of file +out/ +*.jar diff --git a/META-INF/plugin.xml b/META-INF/plugin.xml index 77315d3..24245fd 100644 --- a/META-INF/plugin.xml +++ b/META-INF/plugin.xml @@ -1,7 +1,7 @@ - + de.u-mass.idea.copyConstructor GenerateCopyConstructor - 1.2 + 1.3 Johann Kovacs Also adds a number of inspections that generate warnings if a copy constructor might be faulty (e.g. not all fields copied or superclass constructor not invoked).

- ]]> + ]]> - + @@ -32,7 +34,7 @@ + description="Generates a copy constructor"> diff --git a/src/de/umass/idea/copyConstructor/ConstructorUtil.java b/src/de/umass/idea/copyConstructor/ConstructorUtil.java index 1a5d32a..30f57d9 100644 --- a/src/de/umass/idea/copyConstructor/ConstructorUtil.java +++ b/src/de/umass/idea/copyConstructor/ConstructorUtil.java @@ -40,7 +40,7 @@ public static PsiMethod findCopyConstructor(@Nullable PsiClass psiClass) { return null; } - public static boolean hasCopyConstructor(@Nullable PsiClass psiClass) { + static boolean hasCopyConstructor(@Nullable PsiClass psiClass) { return findCopyConstructor(psiClass) != null; } @@ -72,7 +72,7 @@ public static PsiMethod findConstructorCall(PsiMethod constructor) { * This excludes, for example, static fields. */ public static List getAllCopyableFields(PsiClass psiClass) { - List copyableFields = new ArrayList(); + List copyableFields = new ArrayList<>(); PsiField[] fields = psiClass.getFields(); for (PsiField field : fields) { if (isCopyableField(field)) { diff --git a/src/de/umass/idea/copyConstructor/GenerateCopyConstructorHandler.java b/src/de/umass/idea/copyConstructor/GenerateCopyConstructorHandler.java index c33a54e..7b405ce 100644 --- a/src/de/umass/idea/copyConstructor/GenerateCopyConstructorHandler.java +++ b/src/de/umass/idea/copyConstructor/GenerateCopyConstructorHandler.java @@ -45,12 +45,12 @@ protected ClassMember[] chooseMembers(ClassMember[] members, boolean allowEmptyS @Override protected List generateMemberPrototypes(PsiClass aClass, ClassMember[] members) throws IncorrectOperationException { PsiMethod copyConstructor = generateCopyConstructor(aClass, members); - return Collections.singletonList(new PsiGenerationInfo(copyConstructor)); + return Collections.singletonList(new PsiGenerationInfo<>(copyConstructor)); } @Override protected GenerationInfo[] generateMemberPrototypes(PsiClass aClass, ClassMember originalMember) throws IncorrectOperationException { - return null; + return new GenerationInfo[0]; } private PsiMethod generateCopyConstructor(PsiClass psiClass, ClassMember[] copyableFields) { @@ -72,8 +72,7 @@ private PsiMethod generateCopyConstructor(PsiClass psiClass, ClassMember[] copya code.append("}"); PsiElementFactory elementFactory = JavaPsiFacade.getElementFactory(psiClass.getProject()); - PsiMethod constructor = elementFactory.createMethodFromText(code.toString(), psiClass); - return constructor; + return elementFactory.createMethodFromText(code.toString(), psiClass); } private ClassMember[] toMembers(List allCopyableFields) { diff --git a/src/de/umass/idea/copyConstructor/inspection/IncompleteCopyConstructorInspection.java b/src/de/umass/idea/copyConstructor/inspection/IncompleteCopyConstructorInspection.java index bbc7389..aec0052 100644 --- a/src/de/umass/idea/copyConstructor/inspection/IncompleteCopyConstructorInspection.java +++ b/src/de/umass/idea/copyConstructor/inspection/IncompleteCopyConstructorInspection.java @@ -4,22 +4,7 @@ import com.intellij.codeInspection.LocalInspectionToolSession; import com.intellij.codeInspection.ProblemHighlightType; import com.intellij.codeInspection.ProblemsHolder; -import com.intellij.psi.JavaElementVisitor; -import com.intellij.psi.JavaRecursiveElementVisitor; -import com.intellij.psi.PsiAssignmentExpression; -import com.intellij.psi.PsiClass; -import com.intellij.psi.PsiElement; -import com.intellij.psi.PsiElementVisitor; -import com.intellij.psi.PsiExpression; -import com.intellij.psi.PsiField; -import com.intellij.psi.PsiIdentifier; -import com.intellij.psi.PsiMethod; -import com.intellij.psi.PsiParameter; -import com.intellij.psi.PsiReference; -import com.intellij.psi.PsiReferenceExpression; -import com.intellij.psi.PsiThisExpression; -import com.intellij.psi.PsiVariable; - +import com.intellij.psi.*; import de.umass.idea.copyConstructor.ConstructorUtil; import org.jetbrains.annotations.NotNull; import org.jetbrains.annotations.Nullable; @@ -61,7 +46,7 @@ public void visitMethod(PsiMethod method) { } private boolean constructorAssignsAllFields(final PsiMethod constructor, List allFields) { - final Set unassignedFields = new HashSet(allFields); + final Set unassignedFields = new HashSet<>(allFields); final PsiParameter copyParameter = constructor.getParameterList().getParameters()[0]; constructor.accept(new JavaRecursiveElementVisitor() { @Override @@ -71,7 +56,7 @@ public void visitAssignmentExpression(PsiAssignmentExpression expression) { PsiReference assignee = left.getReference(); if (assignee != null) { PsiElement leftReference = assignee.resolve(); - if (leftReference != null && leftReference instanceof PsiField) { + if (leftReference instanceof PsiField) { PsiField referencedField = (PsiField) leftReference; if (isReferenceToFieldInInstance(left, referencedField, null)) { if (isReferenceToFieldInInstance(right, referencedField, copyParameter)) { @@ -91,10 +76,60 @@ public void visitAssignmentExpression(PsiAssignmentExpression expression) { } private boolean isReferenceToFieldInInstance(@Nullable PsiExpression expression, @NotNull PsiField field, @Nullable PsiVariable instance) { - if (expression != null && expression instanceof PsiReferenceExpression) { + boolean isReference = false; + if (expression instanceof PsiReferenceExpression) { PsiReferenceExpression referenceExpression = (PsiReferenceExpression) expression; PsiExpression qualifierExpression = referenceExpression.getQualifierExpression(); - return referenceExpression.isReferenceTo(field) && qualifierReferencesVariable(qualifierExpression, instance); + isReference = referenceExpression.isReferenceTo(field) && qualifierReferencesVariable(qualifierExpression, instance); + if (!isReference && qualifierExpression instanceof PsiReferenceExpression) { + isReference = isReferenceToFieldInInstance(qualifierExpression, field, instance); + } + } else if (expression instanceof PsiCallExpression) { + PsiCallExpression callExpression = (PsiCallExpression) expression; + isReference = isCallExpressionReferencingToFieldInstance(callExpression, field, instance); + } else if (expression instanceof PsiPolyadicExpression) { + PsiPolyadicExpression polyadicExpression = (PsiPolyadicExpression) expression; + isReference = isPolyadicExpressionReferencingToFieldInstance(polyadicExpression, field, instance); + } else if (expression instanceof PsiConditionalExpression) { + PsiConditionalExpression conditionalExpression = (PsiConditionalExpression) expression; + isReference = isConditionalExpressionReferencingToFieldInstance(conditionalExpression, field, instance); + } else if (expression instanceof PsiParenthesizedExpression) { + PsiParenthesizedExpression parenthesizedExpression = (PsiParenthesizedExpression) expression; + isReference = isReferenceToFieldInInstance(parenthesizedExpression.getExpression(), field, instance); + } + return isReference; + } + + private boolean isConditionalExpressionReferencingToFieldInstance( + final PsiConditionalExpression conditionalExpression, @NotNull final PsiField field, + @Nullable final PsiVariable instance) { + return isReferenceToFieldInInstance(conditionalExpression.getCondition(), field, instance) + && (isReferenceToFieldInInstance(conditionalExpression.getThenExpression(), field, instance) + || isReferenceToFieldInInstance(conditionalExpression.getElseExpression(), field, instance)); + } + + private boolean isPolyadicExpressionReferencingToFieldInstance( + final PsiPolyadicExpression polyadicExpression, @NotNull final PsiField field, + @Nullable final PsiVariable instance) { + for(PsiExpression operandExpression : polyadicExpression.getOperands()) { + if (isReferenceToFieldInInstance(operandExpression, field, instance)) { + return true; + } + } + return false; + } + + private boolean isCallExpressionReferencingToFieldInstance(final PsiCallExpression callExpression, + @NotNull final PsiField field, @Nullable final PsiVariable instance) { + if (callExpression instanceof PsiMethodCallExpression) { + PsiMethodCallExpression methodExpression = (PsiMethodCallExpression) callExpression; + return isReferenceToFieldInInstance(methodExpression.getMethodExpression(), field, instance); + } else if (callExpression.getArgumentList() != null) { + for(PsiExpression argumentExpression : callExpression.getArgumentList().getExpressions()) { + if (isReferenceToFieldInInstance(argumentExpression, field, instance)) { + return true; + } + } } return false; } @@ -105,7 +140,7 @@ private boolean qualifierReferencesVariable(@Nullable PsiExpression qualifier, @ if (qualifier != null) { PsiReference reference = qualifier.getReference(); - return reference != null && reference.isReferenceTo(instance); + return reference != null && instance != null && reference.isReferenceTo(instance); } return false; diff --git a/src/inspectionDescriptions/IncompleteCopyConstructor.html b/src/inspectionDescriptions/IncompleteCopyConstructor.html index a2017d8..9ef02a6 100644 --- a/src/inspectionDescriptions/IncompleteCopyConstructor.html +++ b/src/inspectionDescriptions/IncompleteCopyConstructor.html @@ -1,7 +1,6 @@ -Reports a warinng if not all fields are assigned correctly in a copy constructor. +Reports a warning if not all fields are assigned correctly in a copy constructor. - \ No newline at end of file