Move externalizable check to inspection

(IDEABKL-3776 "Externalizable class should have public no-args constructor" warning false negative)
2026-09-27 10:03:11 +07:00 · 2012-05-16 21:21:58 +02:00
parent ceac1c66bb
commit 9dfc44532d
11 changed files with 226 additions and 64 deletions
@@ -1,5 +1,5 @@
 /*
- * Copyright 2000-2011 JetBrains s.r.o.
+ * Copyright 2000-2012 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.
@@ -756,36 +756,6 @@ public class HighlightClassUtil {
    return true;
  }

-
-  static HighlightInfo checkExternalizableHasPublicNoArgsConstructor(PsiClass aClass, PsiElement context) {
-    if (!isExternalizable(aClass) || aClass.hasModifierProperty(PsiModifier.ABSTRACT)) {
-      return null;
-    }
-    PsiMethod[] constructors = aClass.getConstructors();
-    boolean hasPublicNoArgsConstructor = constructors.length == 0;
-    for (PsiMethod constructor : constructors) {
-      if (constructor.getParameterList().getParametersCount() == 0 && constructor.hasModifierProperty(PsiModifier.PUBLIC)) {
-        hasPublicNoArgsConstructor = true;
-        break;
-      }
-    }
-    if (!hasPublicNoArgsConstructor) {
-      HighlightInfo highlightInfo = HighlightInfo.createHighlightInfo(HighlightInfoType.WARNING,
-                                                                      context,
-                                                                      JavaErrorMessages.message("externalizable.class.should.have.public.constructor"));
-      QuickFixAction.registerQuickFixAction(highlightInfo, QUICK_FIX_FACTORY.createAddDefaultConstructorFix(aClass));
-      return highlightInfo;
-    }
-    return null;
-  }
-
-  private static boolean isExternalizable(PsiClass aClass) {
-    PsiManager manager = aClass.getManager();
-    PsiClass externalizableClass =
-      JavaPsiFacade.getInstance(manager.getProject()).findClass("java.io.Externalizable", aClass.getResolveScope());
-    return externalizableClass != null && aClass.isInheritor(externalizableClass, true);
-  }
-
  public static boolean hasEnclosingInstanceInScope(PsiClass aClass, PsiElement scope, final boolean isSuperClassAccepted) {
    PsiManager manager = aClass.getManager();
    PsiElement place = scope;
@@ -407,7 +407,6 @@ public class HighlightVisitorImpl extends JavaElementVisitor implements Highligh
      }

      myHolder.add(HighlightClassUtil.checkClassAlreadyImported(aClass, identifier));
-      myHolder.add(HighlightClassUtil.checkExternalizableHasPublicNoArgsConstructor(aClass, identifier));
      if (!(parent instanceof PsiAnonymousClass) && aClass.getNameIdentifier() == identifier) {
        myHolder.add(HighlightNamesUtil.highlightClassName(aClass, identifier, colorsScheme));
      }
@@ -107,7 +107,6 @@ qualified.new.of.static.class=Qualified new of static class
 invalid.qualified.new=Invalid qualified new
 class.name.expected=Class name expected
 no.enclosing.instance.in.scope=No enclosing instance of type ''{0}'' is in scope
-externalizable.class.should.have.public.constructor=Externalizable class should have public no-args constructor
 is.not.an.enclosing.class=''{0}'' is not an enclosing class
 cannot.be.referenced.from.static.context=''{0}'' cannot be referenced from a static context
 no.default.constructor.available=There is no default constructor available in ''{0}''
@@ -48,33 +48,6 @@ class b  {
 }

 ////////////////////////////
-abstract class e implements Externalizable {
-}
-class eImpl extends e {
-    public void readExternal(ObjectInput in) throws IOException, ClassNotFoundException {
-    }
-
-    public void writeExternal(ObjectOutput out) throws IOException {
-    }
-}
-class <warning descr="Externalizable class should have public no-args constructor">eImpl1</warning> extends e {
-    private eImpl1() {}
-    public void readExternal(ObjectInput in) throws IOException, ClassNotFoundException {
-    }
-
-    public void writeExternal(ObjectOutput out) throws IOException {
-    }
-}
-class <warning descr="Externalizable class should have public no-args constructor">eImpl2</warning> extends e {
-    public eImpl2(int i) {
-      System.out.print(i);
-    }
-    public void readExternal(ObjectInput in) throws IOException, ClassNotFoundException {
-    }
-
-    public void writeExternal(ObjectOutput out) throws IOException {
-    }
-}

 abstract class abstractNoSerializable {
  protected Object readResolve() throws ObjectStreamException {
@@ -2025,6 +2025,10 @@
                     key="comparator.not.serializable.display.name" groupBundle="messages.InspectionsBundle"
                     groupKey="group.names.serialization.issues" enabledByDefault="false" level="WARNING"
                     implementationClass="com.siyeh.ig.serialization.ComparatorNotSerializableInspection"/>
+    <localInspection language="JAVA" shortName="ExternalizableWithoutPublicNoArgConstructor" bundle="com.siyeh.InspectionGadgetsBundle"
+                     key="externalizable.without.public.no.arg.constructor.display.name" groupBundle="messages.InspectionsBundle"
+                     groupKey="group.names.serialization.issues" enabledByDefault="true" level="WARNING"
+                     implementationClass="com.siyeh.ig.serialization.ExternalizableWithoutPublicNoArgConstructorInspection"/>
    <localInspection language="JAVA" suppressId="ExternalizableClassWithSerializationMethods" shortName="ExternalizableWithSerializationMethods"
                     bundle="com.siyeh.InspectionGadgetsBundle" key="externalizable.with.serialization.methods.display.name"
                     groupBundle="messages.InspectionsBundle" groupKey="group.names.serialization.issues" enabledByDefault="false"
@@ -2187,7 +2191,7 @@
    <localInspection language="JAVA" shortName="SimplifiableEqualsExpression" bundle="com.siyeh.InspectionGadgetsBundle"
                     key="simplifiable.equals.expression.display.name" groupBundle="messages.InspectionsBundle"
                     groupKey="group.names.code.style.issues" enabledByDefault="false" level="WARNING"
-                     implementationClass="com.siyeh.ig.style.SimplifiableEqualsExpressionInspection"/>
+                     implementationClass="com.siyeh.ig.controlflow.SimplifiableEqualsExpressionInspection"/>
    <localInspection language="JAVA" shortName="StringBufferReplaceableByString" bundle="com.siyeh.InspectionGadgetsBundle"
                     key="string.buffer.replaceable.by.string.display.name" groupBundle="messages.InspectionsBundle"
                     groupKey="group.names.performance.issues" enabledByDefault="true" level="WARNING"
@@ -1114,7 +1114,7 @@ public.constructor.in.non.public.class.problem.descriptor=Constructor is declare
 static.inheritance.replace.quickfix=Replace inheritance with qualified references in {0}
 utility.class.with.public.constructor.make.quickfix=Make {0, choice, 1#constructor|2#constructors} private
 utility.class.without.private.constructor.create.quickfix=Generate empty private constructor
-utility.class.without.private.constructor.make.quickfix=Make constructor private
+utility.class.without.private.constructor.make.quickfix=Make constructor 'private'
 annotation.naming.convention.problem.descriptor.short=Annotation name <code>#ref</code> is too short #loc
 annotation.naming.convention.problem.descriptor.long=Annotation name <code>#ref</code> is too long #loc
 annotation.naming.convention.problem.descriptor.regex.mismatch=Annotation name <code>#ref</code> doesn''t match regex ''{0}'' #loc
@@ -1208,7 +1208,7 @@ unsecure.random.number.generation.problem.descriptor3=For security purposes, use
 serializable.has.serialization.methods.problem.descriptor=Serializable class <code>#ref</code> does not define 'readObject()' or 'writeObject()' #loc
 serializable.has.serialization.methods.problem.descriptor1=Serializable class <code>#ref</code> does not define 'writeObject()' #loc
 serializable.has.serialization.methods.problem.descriptor2=Serializable class <code>#ref</code> does not define 'readObject()' #loc
-serializable.with.unconstructable.ancestor.problem.descriptor=<code>#ref</code> has an non-serializable ancestor ''{0}'' without a no-arg constructor #loc
+serializable.with.unconstructable.ancestor.problem.descriptor=<code>#ref</code> has a non-serializable ancestor ''{0}'' without no-arg constructor #loc
 transient.field.in.non.serializable.class.problem.descriptor=Field ''{0}'' is marked <code>#ref</code>, in non-Serializable class #loc
 transient.field.in.non.serializable.class.remove.quickfix=Remove 'transient'
 condition.signal.replace.quickfix=Replace with 'signalAll()'
@@ -1957,4 +1957,7 @@ string.builder.replaceable.by.string.quickfix=Replace 'StringBuilder' with 'Stri
 string.buffer.replaceable.by.string.quickfix=Replace 'StringBuffer' with 'String'
 add.0.to.ignore.if.annotated.by.list.quickfix=Add ''{0}'' to ''Ignore if annotated by'' list
 non.final.field.in.enum.display.name=Non-'final' field in enum
-non.final.field.in.enum.problem.descriptor=non-''final'' field <code>#ref</code> in enum ''{0}''
+non.final.field.in.enum.problem.descriptor=non-''final'' field <code>#ref</code> in enum ''{0}''
+externalizable.without.public.no.arg.constructor.display.name=Externalizable class without public no-arg constructor
+externalizable.without.public.no.arg.constructor.problem.descriptor=Externalizable class <code>#ref</code> has no public no-arg constructor
+make.constructor.public=Make constructor 'public'
@@ -0,0 +1,134 @@
+/*
+ * Copyright 2000-2012 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.siyeh.ig.serialization;
+
+import com.intellij.codeInsight.daemon.impl.quickfix.AddDefaultConstructorFix;
+import com.intellij.codeInspection.ProblemDescriptor;
+import com.intellij.openapi.project.Project;
+import com.intellij.psi.*;
+import com.intellij.util.IncorrectOperationException;
+import com.siyeh.InspectionGadgetsBundle;
+import com.siyeh.ig.BaseInspection;
+import com.siyeh.ig.BaseInspectionVisitor;
+import com.siyeh.ig.DelegatingFix;
+import com.siyeh.ig.InspectionGadgetsFix;
+import org.jetbrains.annotations.Nls;
+import org.jetbrains.annotations.NotNull;
+import org.jetbrains.annotations.Nullable;
+
+/**
+ * @author Bas Leijdekkers
+ */
+public class ExternalizableWithoutPublicNoArgConstructorInspection extends BaseInspection {
+
+  @Nls
+  @NotNull
+  @Override
+  public String getDisplayName() {
+    return InspectionGadgetsBundle.message("externalizable.without.public.no.arg.constructor.display.name");
+  }
+
+  @NotNull
+  @Override
+  protected String buildErrorString(Object... infos) {
+    return InspectionGadgetsBundle.message("externalizable.without.public.no.arg.constructor.problem.descriptor");
+  }
+
+  @Override
+  protected InspectionGadgetsFix buildFix(Object... infos) {
+    final PsiMethod constructor = (PsiMethod)infos[1];
+    if (constructor == null) {
+      final PsiClass aClass = (PsiClass)infos[0];
+      return new DelegatingFix(new AddDefaultConstructorFix(aClass, PsiModifier.PUBLIC));
+    }
+    else {
+      return new MakeConstructorPublicFix();
+    }
+  }
+
+  @Nullable
+  private static PsiMethod getNoArgConstructor(PsiClass aClass) {
+    final PsiMethod[] constructors = aClass.getConstructors();
+    for (PsiMethod constructor : constructors) {
+      final PsiParameterList parameterList = constructor.getParameterList();
+      if (parameterList.getParametersCount() == 0) {
+        return constructor;
+      }
+    }
+    return null;
+  }
+
+  private static class MakeConstructorPublicFix extends InspectionGadgetsFix {
+
+    @NotNull
+    public String getName() {
+      return InspectionGadgetsBundle.message("make.constructor.public");
+    }
+
+    @Override
+    public void doFix(Project project, ProblemDescriptor descriptor)
+      throws IncorrectOperationException {
+      final PsiElement classNameIdentifier = descriptor.getPsiElement();
+      final PsiClass aClass = (PsiClass)classNameIdentifier.getParent();
+      if (aClass == null) {
+        return;
+      }
+      final PsiMethod constructor = getNoArgConstructor(aClass);
+      if (constructor == null) {
+        return;
+      }
+      constructor.getModifierList().setModifierProperty(PsiModifier.PUBLIC, true);
+    }
+  }
+
+  @Override
+  public BaseInspectionVisitor buildVisitor() {
+    return new ExternalizableWithoutPublicNoArgConstructorVisitor();
+  }
+
+  private static class ExternalizableWithoutPublicNoArgConstructorVisitor extends BaseInspectionVisitor {
+
+    @Override
+    public void visitClass(@NotNull PsiClass aClass) {
+      if (aClass.isInterface() || aClass.isEnum() || aClass.isAnnotationType() || aClass instanceof PsiTypeParameter) {
+        return;
+      }
+      if (aClass.hasModifierProperty(PsiModifier.ABSTRACT)) {
+        return;
+      }
+      if (!isExternalizable(aClass)) {
+        return;
+      }
+      final PsiMethod constructor = getNoArgConstructor(aClass);
+      if (constructor == null) {
+        if (aClass.hasModifierProperty(PsiModifier.PUBLIC)) {
+          return;
+        }
+      } else {
+        if (constructor.hasModifierProperty(PsiModifier.PUBLIC)) {
+          return;
+        }
+      }
+      registerClassError(aClass, aClass, constructor);
+    }
+
+    private static boolean isExternalizable(PsiClass aClass) {
+      final JavaPsiFacade psiFacade = JavaPsiFacade.getInstance(aClass.getProject());
+      final PsiClass externalizableClass = psiFacade.findClass("java.io.Externalizable", aClass.getResolveScope());
+      return externalizableClass != null && aClass.isInheritor(externalizableClass, true);
+    }
+  }
+}
@@ -0,0 +1,9 @@
+<html>
+<body>
+This inspection reports a <b>Externalizable</b> classes without a public no-argument constructor.
+When an Externalizable object is reconstructed, an instance is created using the public no-arg constructor before the readExternal
+method called. If a public no-arg constructor is not present a <b>java.io.InvalidClassException</b> will be thrown at runtime.
+<p>
+<small>New in 12, Powered by InspectionGadgets</small>
+</body>
+</html>
@@ -0,0 +1,34 @@
+package com.siyeh.igtest.serialization.externalizable_without_public_no_arg_constructor;
+
+import java.io.*;
+
+abstract class e implements Externalizable {
+
+  protected e() {}
+}
+class eImpl extends e {
+  public void readExternal(ObjectInput in) throws IOException, ClassNotFoundException {
+  }
+
+  public void writeExternal(ObjectOutput out) throws IOException {
+  }
+}
+class eImpl1 extends e {
+  private eImpl1() {}
+  public void readExternal(ObjectInput in) throws IOException, ClassNotFoundException {
+  }
+
+  public void writeExternal(ObjectOutput out) throws IOException {
+  }
+}
+class eImpl2 extends e {
+  public eImpl2(int i) {
+    System.out.print(i);
+  }
+  public void readExternal(ObjectInput in) throws IOException, ClassNotFoundException {
+  }
+
+  public void writeExternal(ObjectOutput out) throws IOException {
+  }
+}
+
@@ -0,0 +1,26 @@
+<?xml version="1.0" encoding="UTF-8"?>
+<problems>
+  <problem>
+    <file>ExternalizableWithoutPublicNoArgConstructor.java</file>
+    <line>9</line>
+    <module>test_5975879954938100914</module>
+    <problem_class severity="WARNING" attribute_key="WARNING_ATTRIBUTES">Externalizable class without public no-arg constructor</problem_class>
+    <description>Externalizable class &lt;code&gt;eImpl&lt;/code&gt; has no public no-arg constructor</description>
+  </problem>
+
+  <problem>
+    <file>ExternalizableWithoutPublicNoArgConstructor.java</file>
+    <line>16</line>
+    <entry_point TYPE="class" FQNAME="com.siyeh.igtest.serialization.externalizable_without_public_no_arg_constructor.eImpl1" />
+    <problem_class severity="WARNING" attribute_key="WARNING_ATTRIBUTES">Externalizable class without public no-arg constructor</problem_class>
+    <description>Externalizable class &lt;code&gt;eImpl1&lt;/code&gt; has no public no-arg constructor</description>
+  </problem>
+  
+  <problem>
+    <file>ExternalizableWithoutPublicNoArgConstructor.java</file>
+    <line>24</line>
+    <entry_point TYPE="class" FQNAME="com.siyeh.igtest.serialization.externalizable_without_public_no_arg_constructor.eImpl2" />
+    <problem_class severity="WARNING" attribute_key="WARNING_ATTRIBUTES">Externalizable class without public no-arg constructor</problem_class>
+    <description>Externalizable class &lt;code&gt;eImpl2&lt;/code&gt; has no public no-arg constructor</description>
+  </problem>
+</problems>
@@ -0,0 +1,11 @@
+package com.siyeh.ig.serialization;
+
+import com.siyeh.ig.IGInspectionTestCase;
+
+public class ExternalizableWithoutPublicNoArgConstructorInspectionTest extends IGInspectionTestCase {
+
+  public void test() throws Exception {
+    doTest("com/siyeh/igtest/serialization/externalizable_without_public_no_arg_constructor",
+           new ExternalizableWithoutPublicNoArgConstructorInspection());
+  }
+}