8378908: [lworld] Serializations warnings for value classes that have private serialization methods

src/java.base/share/classes/java/time/Duration.java

@@ -1598,6 +1598,7 @@ private Object writeReplace() {
      * @throws InvalidObjectException always
      */
     @java.io.Serial
+    @SuppressWarnings("serial")
     private void readObject(ObjectInputStream s) throws InvalidObjectException {
         throw new InvalidObjectException("Deserialization via serialization delegate");
     }

src/java.base/share/classes/java/time/Instant.java

@@ -1438,6 +1438,7 @@ private Object writeReplace() {
      * @throws InvalidObjectException always
      */
     @java.io.Serial
+    @SuppressWarnings("serial")
     private void readObject(ObjectInputStream s) throws InvalidObjectException {
         throw new InvalidObjectException("Deserialization via serialization delegate");
     }

src/java.base/share/classes/java/time/LocalDate.java

@@ -2203,6 +2203,7 @@ private Object writeReplace() {
      * @throws InvalidObjectException always
      */
     @java.io.Serial
+    @SuppressWarnings("serial")
     private void readObject(ObjectInputStream s) throws InvalidObjectException {
         throw new InvalidObjectException("Deserialization via serialization delegate");
     }

src/java.base/share/classes/java/time/LocalDateTime.java

@@ -2006,6 +2006,7 @@ private Object writeReplace() {
      * @throws InvalidObjectException always
      */
     @java.io.Serial
+    @SuppressWarnings("serial")
     private void readObject(ObjectInputStream s) throws InvalidObjectException {
         throw new InvalidObjectException("Deserialization via serialization delegate");
     }

src/java.base/share/classes/java/time/LocalTime.java

@@ -1688,6 +1688,7 @@ private Object writeReplace() {
      * @throws InvalidObjectException always
      */
     @java.io.Serial
+    @SuppressWarnings("serial")
     private void readObject(ObjectInputStream s) throws InvalidObjectException {
         throw new InvalidObjectException("Deserialization via serialization delegate");
     }

src/java.base/share/classes/java/time/MonthDay.java

@@ -807,6 +807,7 @@ private Object writeReplace() {
      * @throws InvalidObjectException always
      */
     @java.io.Serial
+    @SuppressWarnings("serial")
     private void readObject(ObjectInputStream s) throws InvalidObjectException {
         throw new InvalidObjectException("Deserialization via serialization delegate");
     }

src/java.base/share/classes/java/time/OffsetDateTime.java

@@ -1964,6 +1964,7 @@ private Object writeReplace() {
      * @throws InvalidObjectException always
      */
     @java.io.Serial
+    @SuppressWarnings("serial")
     private void readObject(ObjectInputStream s) throws InvalidObjectException {
         throw new InvalidObjectException("Deserialization via serialization delegate");
     }

src/java.base/share/classes/java/time/OffsetTime.java

@@ -1439,6 +1439,7 @@ private Object writeReplace() {
      * @throws InvalidObjectException always
      */
     @java.io.Serial
+    @SuppressWarnings("serial")
     private void readObject(ObjectInputStream s) throws InvalidObjectException {
         throw new InvalidObjectException("Deserialization via serialization delegate");
     }

src/java.base/share/classes/java/time/Period.java

@@ -1071,6 +1071,7 @@ private Object writeReplace() {
      * @throws java.io.InvalidObjectException always
      */
     @java.io.Serial
+    @SuppressWarnings("serial")
     private void readObject(ObjectInputStream s) throws InvalidObjectException {
         throw new InvalidObjectException("Deserialization via serialization delegate");
     }

src/java.base/share/classes/java/time/Year.java

@@ -1124,6 +1124,7 @@ private Object writeReplace() {
      * @throws InvalidObjectException always
      */
     @java.io.Serial
+    @SuppressWarnings("serial")
     private void readObject(ObjectInputStream s) throws InvalidObjectException {
         throw new InvalidObjectException("Deserialization via serialization delegate");
     }

src/java.base/share/classes/java/time/YearMonth.java

@@ -1261,6 +1261,7 @@ private Object writeReplace() {
      * @throws InvalidObjectException always
      */
     @java.io.Serial
+    @SuppressWarnings("serial")
     private void readObject(ObjectInputStream s) throws InvalidObjectException {
         throw new InvalidObjectException("Deserialization via serialization delegate");
     }

src/java.base/share/classes/java/time/ZonedDateTime.java

@@ -2269,6 +2269,7 @@ private Object writeReplace() {
      * @throws InvalidObjectException always
      */
     @java.io.Serial
+    @SuppressWarnings("serial")
     private void readObject(ObjectInputStream s) throws InvalidObjectException {
         throw new InvalidObjectException("Deserialization via serialization delegate");
     }

src/java.base/share/classes/java/time/chrono/HijrahDate.java

@@ -664,6 +664,7 @@ public int hashCode() {
      * @throws InvalidObjectException always
      */
     @java.io.Serial
+    @SuppressWarnings("serial")
     private void readObject(ObjectInputStream s) throws InvalidObjectException {
         throw new InvalidObjectException("Deserialization via serialization delegate");
     }

src/java.base/share/classes/java/time/chrono/JapaneseDate.java

@@ -725,6 +725,7 @@ public int hashCode() {
      * @throws InvalidObjectException always
      */
     @java.io.Serial
+    @SuppressWarnings("serial")
     private void readObject(ObjectInputStream s) throws InvalidObjectException {
         throw new InvalidObjectException("Deserialization via serialization delegate");
     }

src/java.base/share/classes/java/time/chrono/MinguoDate.java

@@ -486,6 +486,7 @@ public int hashCode() {
      * @throws InvalidObjectException always
      */
     @java.io.Serial
+    @SuppressWarnings("serial")
     private void readObject(ObjectInputStream s) throws InvalidObjectException {
         throw new InvalidObjectException("Deserialization via serialization delegate");
     }

src/java.base/share/classes/java/time/chrono/ThaiBuddhistDate.java

@@ -488,6 +488,7 @@ public int hashCode() {
      * @throws InvalidObjectException always
      */
     @java.io.Serial
+    @SuppressWarnings("serial")
     private void readObject(ObjectInputStream s) throws InvalidObjectException {
         throw new InvalidObjectException("Deserialization via serialization delegate");
     }

src/jdk.compiler/share/classes/com/sun/tools/javac/comp/Check.java

@@ -5054,7 +5054,8 @@ public Void visitTypeAsClass(TypeElement e,
 
             // Check declarations of serialization-related methods and
             // fields
-            final boolean[] hasWriteReplace = {false};
+            final Map<String, Symbol> declaredSerialMethodNames = new HashMap<>();
+            final boolean[] isSerialMethodCorrect = new boolean[] { false };
             for(Symbol el : c.getEnclosedElements()) {
                 runUnderLint(el, p, (enclosed, tree) -> {
                     String name = null;
@@ -5128,19 +5129,22 @@ public Void visitTypeAsClass(TypeElement e,
                         name = method.getSimpleName().toString();
                         if (serialMethodNames.contains(name)) {
                             switch (name) {
-                            case "writeObject"      -> checkWriteObject(tree, e, method);
-                            case "writeReplace"     -> {hasWriteReplace[0] = true; hasAppropriateWriteReplace(tree, method, true);}
-                            case "readObject"       -> checkReadObject(tree,e, method);
-                            case "readObjectNoData" -> checkReadObjectNoData(tree, e, method);
-                            case "readResolve"      -> checkReadResolve(tree, e, method);
+                            case "writeObject"      -> isSerialMethodCorrect[0] = hasAppropriateWriteObject(tree, e, method);
+                            case "writeReplace"     -> isSerialMethodCorrect[0] = hasAppropriateWriteReplace(tree, method, true);
+                            case "readObject"       -> isSerialMethodCorrect[0] = hasAppropriateReadObject(tree, e, method);
+                            case "readObjectNoData" -> isSerialMethodCorrect[0] = hasAppropriateReadObjectNoData(tree, e, method);
+                            case "readResolve"      -> isSerialMethodCorrect[0] = hasAppropriateReadResolve(tree, e, method);
                             default ->  throw new AssertionError();
                             }
+                            if (isSerialMethodCorrect[0]) {
+                                declaredSerialMethodNames.put(name, el);
+                            }
                         }
                     }
                     }
                 });
             }
-            if (!hasWriteReplace[0] &&
+            if (declaredSerialMethodNames.get("writeReplace") == null &&
                     (c.isValueClass() || hasAbstractValueSuperClass(c, Set.of(syms.numberType.tsym))) &&
                     !c.isAbstract() && !c.isRecord() &&
                     types.unboxedType(c.type) == Type.noType) {
@@ -5160,6 +5164,14 @@ public Void visitTypeAsClass(TypeElement e,
                                     LintWarnings.SerializableValueClassWithoutWriteReplace2);
                 }
             }
+            if (c.isValueClass()) {
+                for (Map.Entry<String, Symbol> entry : declaredSerialMethodNames.entrySet()) {
+                    String key = entry.getKey();
+                    if (key.equals("writeObject") || key.equals("readObject") || key.equals("readObjectNoData")) {
+                        log.warning(TreeInfo.diagnosticPositionFor(entry.getValue(), p), LintWarnings.IneffectualSerialMethodValueClass(key));
+                    }
+                }
+            }
             return null;
         }
 
@@ -5299,17 +5311,17 @@ private void checkSerialPersistentFields(JCClassDecl tree, Element e, VarSymbol
             }
         }
 
-        private void checkWriteObject(JCClassDecl tree, Element e, MethodSymbol method) {
+        private boolean hasAppropriateWriteObject(JCClassDecl tree, Element e, MethodSymbol method) {
             // The "synchronized" modifier is seen in the wild on
             // readObject and writeObject methods and is generally
             // innocuous.
 
             // private void writeObject(ObjectOutputStream stream) throws IOException
-            checkPrivateNonStaticMethod(tree, method);
-            isExpectedReturnType(tree, method, syms.voidType, true);
-            checkOneArg(tree, e, method, syms.objectOutputStreamType);
-            hasExpectedExceptions(tree, method, true, syms.ioExceptionType);
-            checkExternalizable(tree, e, method);
+            return isPrivateNonStaticMethod(tree, method) &  // no short-circuit we need to evaluate all
+                    isExpectedReturnType(tree, method, syms.voidType, true) &
+                    hasExpectedArg(tree, method, syms.objectOutputStreamType) &
+                    hasExpectedExceptions(tree, method, true, syms.ioExceptionType) &
+                    checkExternalizable(tree, e, method);
         }
 
         private boolean hasAppropriateWriteReplace(JCClassDecl tree, MethodSymbol method, boolean warn) {
@@ -5318,45 +5330,45 @@ private boolean hasAppropriateWriteReplace(JCClassDecl tree, MethodSymbol method
 
             // Excluding abstract, could have a more complicated
             // rule based on abstract-ness of the class
-            return isConcreteInstanceMethod(tree, method, warn) &&
-                    isExpectedReturnType(tree, method, syms.objectType, warn) &&
-                    hasNoArgs(tree, method, warn) &&
+            return isConcreteInstanceMethod(tree, method, warn) &  // no short-circuit we need to evaluate all
+                    isExpectedReturnType(tree, method, syms.objectType, warn) &
+                    hasNoArgs(tree, method, warn) &
                     hasExpectedExceptions(tree, method, warn, syms.objectStreamExceptionType);
         }
 
-        private void checkReadObject(JCClassDecl tree, Element e, MethodSymbol method) {
+        private boolean hasAppropriateReadObject(JCClassDecl tree, Element e, MethodSymbol method) {
             // The "synchronized" modifier is seen in the wild on
             // readObject and writeObject methods and is generally
             // innocuous.
 
             // private void readObject(ObjectInputStream stream)
             //   throws IOException, ClassNotFoundException
-            checkPrivateNonStaticMethod(tree, method);
-            isExpectedReturnType(tree, method, syms.voidType, true);
-            checkOneArg(tree, e, method, syms.objectInputStreamType);
-            hasExpectedExceptions(tree, method, true, syms.ioExceptionType, syms.classNotFoundExceptionType);
-            checkExternalizable(tree, e, method);
+            return isPrivateNonStaticMethod(tree, method) & // no short-circuit we need to evaluate all
+                    isExpectedReturnType(tree, method, syms.voidType, true) &
+                    hasExpectedArg(tree, method, syms.objectInputStreamType) &
+                    hasExpectedExceptions(tree, method, true, syms.ioExceptionType, syms.classNotFoundExceptionType) &
+                    checkExternalizable(tree, e, method);
         }
 
-        private void checkReadObjectNoData(JCClassDecl tree, Element e, MethodSymbol method) {
+        private boolean hasAppropriateReadObjectNoData(JCClassDecl tree, Element e, MethodSymbol method) {
             // private void readObjectNoData() throws ObjectStreamException
-            checkPrivateNonStaticMethod(tree, method);
-            isExpectedReturnType(tree, method, syms.voidType, true);
-            hasNoArgs(tree, method, true);
-            hasExpectedExceptions(tree, method, true, syms.objectStreamExceptionType);
-            checkExternalizable(tree, e, method);
+            return isPrivateNonStaticMethod(tree, method) &
+                    isExpectedReturnType(tree, method, syms.voidType, true) &
+                    hasNoArgs(tree, method, true) &
+                    hasExpectedExceptions(tree, method, true, syms.objectStreamExceptionType) &
+                    checkExternalizable(tree, e, method);
         }
 
-        private void checkReadResolve(JCClassDecl tree, Element e, MethodSymbol method) {
+        private boolean hasAppropriateReadResolve(JCClassDecl tree, Element e, MethodSymbol method) {
             // ANY-ACCESS-MODIFIER Object readResolve()
             // throws ObjectStreamException
 
             // Excluding abstract, could have a more complicated
             // rule based on abstract-ness of the class
-            isConcreteInstanceMethod(tree, method, true);
-            isExpectedReturnType(tree, method, syms.objectType, true);
-            hasNoArgs(tree, method, true);
-            hasExpectedExceptions(tree, method, true, syms.objectStreamExceptionType);
+            return isConcreteInstanceMethod(tree, method, true) & // no short-circuit we need to evaluate all
+                    isExpectedReturnType(tree, method, syms.objectType, true) &
+                    hasNoArgs(tree, method, true) &
+                    hasExpectedExceptions(tree, method, true, syms.objectStreamExceptionType);
         }
 
         private void checkWriteExternalRecord(JCClassDecl tree, Element e, MethodSymbol method, boolean isExtern) {
@@ -5378,19 +5390,23 @@ private void checkExternMethodRecord(JCClassDecl tree, Element e, MethodSymbol m
             }
         }
 
-        void checkPrivateNonStaticMethod(JCClassDecl tree, MethodSymbol method) {
+        boolean isPrivateNonStaticMethod(JCClassDecl tree, MethodSymbol method) {
             var flags = method.flags();
+            boolean result = true;
             if ((flags & PRIVATE) == 0) {
                 log.warning(
                         TreeInfo.diagnosticPositionFor(method, tree),
                             LintWarnings.SerialMethodNotPrivate(method.getSimpleName()));
+                result = false;
             }
 
             if ((flags & STATIC) != 0) {
                 log.warning(
                         TreeInfo.diagnosticPositionFor(method, tree),
                             LintWarnings.SerialMethodStatic(method.getSimpleName()));
+                result = false;
             }
+            return result;
         }
 
         /**
@@ -5607,7 +5623,7 @@ public Void visitTypeAsRecord(TypeElement e,
                         var method = (MethodSymbol)enclosed;
                         switch(name) {
                         case "writeReplace" -> hasAppropriateWriteReplace(tree, method, true);
-                        case "readResolve"  -> checkReadResolve(tree, e, method);
+                        case "readResolve"  -> hasAppropriateReadResolve(tree, e, method);
 
                         case "writeExternal" -> checkWriteExternalRecord(tree, e, method, isExtern);
                         case "readExternal"  -> checkReadExternalRecord(tree, e, method, isExtern);
@@ -5660,19 +5676,17 @@ private boolean isExpectedReturnType(JCClassDecl tree,
             return true;
         }
 
-        private void checkOneArg(JCClassDecl tree,
-                                 Element enclosing,
-                                 MethodSymbol method,
-                                 Type expectedType) {
-            String name = method.getSimpleName().toString();
+        private boolean hasExpectedArg(JCClassDecl tree,
+                                    MethodSymbol method,
+                                    Type expectedType) {
 
             var parameters= method.getParameters();
 
             if (parameters.size() != 1) {
                 log.warning(
                         TreeInfo.diagnosticPositionFor(method, tree),
                             LintWarnings.SerialMethodOneArg(method.getSimpleName(), parameters.size()));
-                return;
+                return false;
             }
 
             Type parameterType = parameters.get(0).asType();
@@ -5682,7 +5696,9 @@ private void checkOneArg(JCClassDecl tree,
                             LintWarnings.SerialMethodParameterType(method.getSimpleName(),
                                                                expectedType,
                                                                parameterType));
+                return false;
             }
+            return true;
         }
 
         private boolean hasExactlyOneArgWithType(JCClassDecl tree,
@@ -5708,14 +5724,15 @@ boolean hasNoArgs(JCClassDecl tree, MethodSymbol method, boolean warn) {
             return true;
         }
 
-        private void checkExternalizable(JCClassDecl tree, Element enclosing, MethodSymbol method) {
+        private boolean checkExternalizable(JCClassDecl tree, Element enclosing, MethodSymbol method) {
             // If the enclosing class is externalizable, warn for the method
             if (isExternalizable((Type)enclosing.asType())) {
                 log.warning(
                         TreeInfo.diagnosticPositionFor(method, tree),
                             LintWarnings.IneffectualSerialMethodExternalizable(method.getSimpleName()));
+                return false;
             }
-            return;
+            return true;
         }
 
         private boolean hasExpectedExceptions(JCClassDecl tree,

src/jdk.compiler/share/classes/com/sun/tools/javac/resources/compiler.properties

@@ -2176,6 +2176,11 @@ compiler.warn.serializable.value.class.without.write.replace.1=\
 compiler.warn.serializable.value.class.without.write.replace.2=\
     serializable class does not declare, or inherits, a writeReplace method
 
+# 0: string
+# lint: serial
+compiler.warn.ineffectual.serial.method.value.class=\
+    serialization-related method ''{0}'' is not effective in a value class
+
 # 0: symbol, 1: symbol, 2: symbol, 3: symbol
 # lint: overloads
 compiler.warn.potentially.ambiguous.overload=\