changeset 57011:a9021bd6ffbe records-and-sealed

reject serialization members if declared in a record
author vromero
date Tue, 20 Aug 2019 13:31:16 -0400
parents 4750d96927b1
children a363100f43a9 befc2bc52f1f
files src/jdk.compiler/share/classes/com/sun/tools/javac/code/Symtab.java src/jdk.compiler/share/classes/com/sun/tools/javac/comp/TypeEnter.java src/jdk.compiler/share/classes/com/sun/tools/javac/resources/compiler.properties src/jdk.compiler/share/classes/com/sun/tools/javac/util/Names.java test/langtools/tools/javac/records/RecordCompilationTests.java
diffstat 5 files changed, 89 insertions(+), 10 deletions(-) [+]
line wrap: on
line diff
--- a/src/jdk.compiler/share/classes/com/sun/tools/javac/code/Symtab.java	Mon Aug 19 16:00:24 2019 -0400
+++ b/src/jdk.compiler/share/classes/com/sun/tools/javac/code/Symtab.java	Tue Aug 20 13:31:16 2019 -0400
@@ -219,6 +219,9 @@
     public final Type patternHandleType;
     public final Type typeDescriptorType;
     public final Type recordType;
+    public final Type objectStreamFieldType;
+    public final Type objectOutputStreamType;
+    public final Type objectInputStreamType;
 
     /** The symbol representing the length field of an array.
      */
@@ -580,6 +583,9 @@
         patternHandleType = enterClass("java.lang.runtime.PatternHandle");
         typeDescriptorType = enterClass("java.lang.invoke.TypeDescriptor");
         recordType = enterClass("java.lang.Record");
+        objectStreamFieldType = enterClass("java.io.ObjectStreamField");
+        objectOutputStreamType = enterClass("java.io.ObjectOutputStream");
+        objectInputStreamType = enterClass("java.io.ObjectInputStream");
 
         synthesizeEmptyInterfaceIfMissing(autoCloseableType);
         synthesizeEmptyInterfaceIfMissing(cloneableType);
--- a/src/jdk.compiler/share/classes/com/sun/tools/javac/comp/TypeEnter.java	Mon Aug 19 16:00:24 2019 -0400
+++ b/src/jdk.compiler/share/classes/com/sun/tools/javac/comp/TypeEnter.java	Tue Aug 20 13:31:16 2019 -0400
@@ -28,9 +28,7 @@
 import java.util.HashSet;
 import java.util.Set;
 import java.util.function.BiConsumer;
-import java.util.stream.Collectors;
 
-import javax.lang.model.element.ElementKind;
 import javax.tools.JavaFileObject;
 
 import com.sun.tools.javac.code.*;
@@ -1059,6 +1057,9 @@
             List<JCTree> defsToEnter = isRecord ?
                     tree.defs.diff(List.convert(JCTree.class, TreeInfo.recordFields(tree))) : tree.defs;
             memberEnter.memberEnter(defsToEnter, env);
+            if (isRecord) {
+                checkForSerializationMembers(tree, env);
+            }
             List<JCTree> defsBeforeAddingNewMembers = tree.defs;
             if (isRecord) {
                 addRecordMembersIfNeeded(tree, env, defaultConstructorGenerated);
@@ -1192,6 +1193,54 @@
             memberEnter.memberEnter(valueOf, env);
         }
 
+        private void checkForSerializationMembers(JCClassDecl tree, Env<AttrContext> env) {
+            // non-static void writeObject(java.io.ObjectOutputStream) {}
+            MethodSymbol ms = lookupMethod(tree.sym, names.writeObject, List.of(syms.objectOutputStreamType));
+            if (ms != null) {
+                errorOnSerializationMember(tree, names.writeObject, ms, syms.voidType, false);
+            }
+            // non-static Object writeReplace() {}
+            ms = lookupMethod(tree.sym, names.writeReplace, List.nil());
+            if (ms != null) {
+                errorOnSerializationMember(tree, names.writeReplace, ms, syms.objectType, false);
+            }
+            // non-static Object readResolve() {}
+            ms = lookupMethod(tree.sym, names.readResolve, List.nil());
+            if (ms != null) {
+                errorOnSerializationMember(tree, names.readResolve, ms, syms.objectType, false);
+            }
+            // non-static void readObjectNoData() {}
+            ms = lookupMethod(tree.sym, names.readObjectNoData, List.nil());
+            if (ms != null) {
+                errorOnSerializationMember(tree, names.readObjectNoData, ms, syms.voidType, false);
+            }
+            // non-static void readObject(java.io.ObjectInputStream stream) {}
+            ms = lookupMethod(tree.sym, names.readObject, List.of(syms.objectInputStreamType));
+            if (ms != null) {
+                errorOnSerializationMember(tree, names.readObject, ms, syms.voidType, false);
+            }
+            Type objectStreamFieldArr = new ArrayType(syms.objectStreamFieldType, syms.arrayClass);
+            Symbol fieldSym = lookupField(tree.sym, names.serialPersistentFields, objectStreamFieldArr);
+            if (fieldSym != null) {
+                errorOnSerializationMember(tree, names.serialPersistentFields, fieldSym, objectStreamFieldArr, true);
+            }
+        }
+
+        private void errorOnSerializationMember(JCClassDecl tree,
+                                                Name name, Symbol sym, Type expectedType, boolean shouldBeStatic) {
+            Type typeOrReturnType = sym.kind == MTH ? sym.type.asMethodType().getReturnType() : sym.type;
+            if (sym.isStatic() == shouldBeStatic && (typeOrReturnType == expectedType || types.isSameType(typeOrReturnType, expectedType))) {
+                for (JCTree def : tree.defs) {
+                    Symbol sym2 = TreeInfo.symbolFor(def);
+                    if (sym2 == sym) {
+                        log.error(def, Errors.IllegalRecordMember(name));
+                        return;
+                    }
+                }
+                log.error(tree, Errors.IllegalRecordMember(name));
+            }
+        }
+
         /** Add the implicit members for a record
          *  to the symbol table.
          */
@@ -1311,6 +1360,15 @@
         return null;
     }
 
+    private Symbol lookupField(TypeSymbol tsym, Name name, Type type) {
+        for (Symbol s : tsym.members().getSymbolsByName(name, s -> s.kind == VAR)) {
+            if (types.isSameType(s.type, type)) {
+                return s;
+            }
+        }
+        return null;
+    }
+
 /* ***************************************************************************
  * tree building
  ****************************************************************************/
--- a/src/jdk.compiler/share/classes/com/sun/tools/javac/resources/compiler.properties	Mon Aug 19 16:00:24 2019 -0400
+++ b/src/jdk.compiler/share/classes/com/sun/tools/javac/resources/compiler.properties	Tue Aug 20 13:31:16 2019 -0400
@@ -3476,6 +3476,10 @@
 compiler.err.illegal.record.component.name=\
     Illegal record component name: {0}
 
+# 0: name
+compiler.err.illegal.record.member=\
+    Illegal record member: {0}
+
 compiler.err.invalid.supertype.record=\
     no class can explicitly extend java.lang.Record
 
--- a/src/jdk.compiler/share/classes/com/sun/tools/javac/util/Names.java	Mon Aug 19 16:00:24 2019 -0400
+++ b/src/jdk.compiler/share/classes/com/sun/tools/javac/util/Names.java	Tue Aug 20 13:31:16 2019 -0400
@@ -207,6 +207,12 @@
     public final Name non;
     public final Name ofLazyProjection;
 
+    // serialization members, used by records too
+    public final Name serialPersistentFields;
+    public final Name writeObject;
+    public final Name writeReplace;
+    public final Name readObjectNoData;
+
     // sealed types
     public final Name permits;
     public final Name sealed;
@@ -374,6 +380,11 @@
         non = fromString("non");
         ofLazyProjection = fromString("ofLazyProjection");
 
+        serialPersistentFields = fromString("serialPersistentFields");
+        writeObject = fromString("writeObject");
+        writeReplace = fromString("writeReplace");
+        readObjectNoData = fromString("readObjectNoData");
+
         // sealed types
         permits = fromString("permits");
         sealed = fromString("sealed");
--- a/test/langtools/tools/javac/records/RecordCompilationTests.java	Mon Aug 19 16:00:24 2019 -0400
+++ b/test/langtools/tools/javac/records/RecordCompilationTests.java	Tue Aug 20 13:31:16 2019 -0400
@@ -324,14 +324,14 @@
 
     public void testIllegalSerializationMembers() {
         // @@@ Should fail
-//        String template = "record R(int x) { # }";
-//        for (String s : List.of("private static final java.io.ObjectStreamField[] serialPersistentFields = {};",
-//                                "private void writeObject(java.io.ObjectOutputStream stream) { }",
-//                                "private Object writeReplace() { }",
-//                                "private Object readResolve() { }",
-//                                "private void readObject(java.io.ObjectInputStream stream) { }",
-//                                "private void readObjectNoData() { }"))
-//            assertFail("", template, s);
+        String template = "record R(int x) { # }";
+        for (String s : List.of("private static final java.io.ObjectStreamField[] serialPersistentFields = {};",
+                                "private void writeObject(java.io.ObjectOutputStream stream) { }",
+                                "private Object writeReplace() { }",
+                                "private Object readResolve() { }",
+                                "private void readObject(java.io.ObjectInputStream stream) { }",
+                                "private void readObjectNoData() { }"))
+            assertFail("compiler.err.illegal.record.member", template, s);
     }
 
     public void testLocalRecords() {