8310835: Address gaps in -Xlint:serial checks

Reviewed-by: rriggs, jjg
This commit is contained in:
Joe Darcy 2023-07-20 01:10:46 +00:00
parent 5d57b5c2f0
commit 61ab27087e
10 changed files with 313 additions and 16 deletions

View File

@ -235,7 +235,10 @@ public class Symtab {
public final Type objectOutputStreamType;
public final Type ioExceptionType;
public final Type objectStreamExceptionType;
// For externalization lint checking
public final Type externalizableType;
public final Type objectInputType;
public final Type objectOutputType;
// For string templates
public final Type stringTemplateType;
@ -622,6 +625,8 @@ public class Symtab {
ioExceptionType = enterClass("java.io.IOException");
objectStreamExceptionType = enterClass("java.io.ObjectStreamException");
externalizableType = enterClass("java.io.Externalizable");
objectInputType = enterClass("java.io.ObjectInput");
objectOutputType = enterClass("java.io.ObjectOutput");
synthesizeEmptyInterfaceIfMissing(autoCloseableType);
synthesizeEmptyInterfaceIfMissing(cloneableType);
synthesizeEmptyInterfaceIfMissing(serializableType);

View File

@ -4768,8 +4768,7 @@ public class Check {
boolean isExternalizable(Type t) {
try {
syms.externalizableType.complete();
}
catch (CompletionFailure e) {
} catch (CompletionFailure e) {
return false;
}
return types.isSubtype(t, syms.externalizableType);
@ -5140,6 +5139,25 @@ public class Check {
checkExceptions(tree, e, method, syms.objectStreamExceptionType);
}
private void checkWriteExternalRecord(JCClassDecl tree, Element e, MethodSymbol method, boolean isExtern) {
//public void writeExternal(ObjectOutput) throws IOException
checkExternMethodRecord(tree, e, method, syms.objectOutputType, isExtern);
}
private void checkReadExternalRecord(JCClassDecl tree, Element e, MethodSymbol method, boolean isExtern) {
// public void readExternal(ObjectInput) throws IOException
checkExternMethodRecord(tree, e, method, syms.objectInputType, isExtern);
}
private void checkExternMethodRecord(JCClassDecl tree, Element e, MethodSymbol method, Type argType,
boolean isExtern) {
if (isExtern && isExternMethod(tree, e, method, argType)) {
log.warning(LintCategory.SERIAL,
TreeInfo.diagnosticPositionFor(method, tree),
Warnings.IneffectualExternalizableMethodRecord(method.getSimpleName().toString()));
}
}
void checkPrivateNonStaticMethod(JCClassDecl tree, MethodSymbol method) {
var flags = method.flags();
if ((flags & PRIVATE) == 0) {
@ -5166,6 +5184,7 @@ public class Check {
@Override
public Void visitTypeAsEnum(TypeElement e,
JCClassDecl p) {
boolean isExtern = isExternalizable((Type)e.asType());
for(Element el : e.getEnclosedElements()) {
runUnderLint(el, p, (enclosed, tree) -> {
String name = enclosed.getSimpleName().toString();
@ -5186,13 +5205,47 @@ public class Check {
TreeInfo.diagnosticPositionFor(method, tree),
Warnings.IneffectualSerialMethodEnum(name));
}
if (isExtern) {
switch(name) {
case "writeExternal" -> checkWriteExternalEnum(tree, e, method);
case "readExternal" -> checkReadExternalEnum(tree, e, method);
}
}
}
}
});
}});
}
return null;
}
private void checkWriteExternalEnum(JCClassDecl tree, Element e, MethodSymbol method) {
//public void writeExternal(ObjectOutput) throws IOException
checkExternMethodEnum(tree, e, method, syms.objectOutputType);
}
private void checkReadExternalEnum(JCClassDecl tree, Element e, MethodSymbol method) {
// public void readExternal(ObjectInput) throws IOException
checkExternMethodEnum(tree, e, method, syms.objectInputType);
}
private void checkExternMethodEnum(JCClassDecl tree, Element e, MethodSymbol method, Type argType) {
if (isExternMethod(tree, e, method, argType)) {
log.warning(LintCategory.SERIAL,
TreeInfo.diagnosticPositionFor(method, tree),
Warnings.IneffectualExternMethodEnum(method.getSimpleName().toString()));
}
}
private boolean isExternMethod(JCClassDecl tree, Element e, MethodSymbol method, Type argType) {
long flags = method.flags();
Type rtype = method.getReturnType();
// Not necessary to check throws clause in this context
return (flags & PUBLIC) != 0 && (flags & STATIC) == 0 &&
types.isSameType(syms.voidType, rtype) &&
hasExactlyOneArgWithType(tree, e, method, argType);
}
/**
* Most serialization-related fields and methods on interfaces
* are ineffectual or problematic.
@ -5238,8 +5291,7 @@ public class Check {
}
}
}
}
}}
});
}
@ -5298,6 +5350,7 @@ public class Check {
@Override
public Void visitTypeAsRecord(TypeElement e,
JCClassDecl p) {
boolean isExtern = isExternalizable((Type)e.asType());
for(Element el : e.getEnclosedElements()) {
runUnderLint(el, p, (enclosed, tree) -> {
String name = enclosed.getSimpleName().toString();
@ -5316,9 +5369,7 @@ public class Check {
// svuid value is not checked to match for
// records.
checkSerialVersionUID(tree, e, field);
}
}
}}
}
case METHOD -> {
@ -5327,18 +5378,17 @@ public class Check {
case "writeReplace" -> checkWriteReplace(tree, e, method);
case "readResolve" -> checkReadResolve(tree, e, method);
case "writeExternal" -> checkWriteExternalRecord(tree, e, method, isExtern);
case "readExternal" -> checkReadExternalRecord(tree, e, method, isExtern);
default -> {
if (serialMethodNames.contains(name)) {
log.warning(LintCategory.SERIAL,
TreeInfo.diagnosticPositionFor(method, tree),
Warnings.IneffectualSerialMethodRecord(name));
}
}
}
}
}
});
}}
}}});
}
return null;
}
@ -5396,6 +5446,16 @@ public class Check {
}
}
private boolean hasExactlyOneArgWithType(JCClassDecl tree,
Element enclosing,
MethodSymbol method,
Type expectedType) {
var parameters = method.getParameters();
return (parameters.size() == 1) &&
types.isSameType(parameters.get(0).asType(), expectedType);
}
private void checkNoArgs(JCClassDecl tree, Element enclosing, MethodSymbol method) {
var parameters = method.getParameters();
if (!parameters.isEmpty()) {

View File

@ -2015,6 +2015,10 @@ compiler.warn.ineffectual.serial.field.enum=\
compiler.warn.ineffectual.serial.method.enum=\
serialization-related method {0} is not effective in an enum class
# 0: string
compiler.warn.ineffectual.extern.method.enum=\
externalization-related method {0} is not effective in an enum class
compiler.warn.ineffectual.serial.field.record=\
serialPersistentFields is not effective in a record class
@ -2022,6 +2026,10 @@ compiler.warn.ineffectual.serial.field.record=\
compiler.warn.ineffectual.serial.method.record=\
serialization-related method {0} is not effective in a record class
# 0: string
compiler.warn.ineffectual.externalizable.method.record=\
externalization-related method {0} is not effective in a record class
# 0: name
compiler.warn.ineffectual.serial.method.externalizable=\
serialization-related method {0} is not effective in an Externalizable class

View File

@ -0,0 +1,42 @@
/*
* Copyright (c) 2021, 2023, Oracle and/or its affiliates. All rights reserved.
* DO NOT ALTER OR REMOVE COPYRIGHT NOTICES OR THIS FILE HEADER.
*
* This code is free software; you can redistribute it and/or modify it
* under the terms of the GNU General Public License version 2 only, as
* published by the Free Software Foundation.
*
* This code is distributed in the hope that it will be useful, but WITHOUT
* ANY WARRANTY; without even the implied warranty of MERCHANTABILITY or
* FITNESS FOR A PARTICULAR PURPOSE. See the GNU General Public License
* version 2 for more details (a copy is included in the LICENSE file that
* accompanied this code).
*
* You should have received a copy of the GNU General Public License version
* 2 along with this work; if not, write to the Free Software Foundation,
* Inc., 51 Franklin St, Fifth Floor, Boston, MA 02110-1301 USA.
*
* Please contact Oracle, 500 Oracle Parkway, Redwood Shores, CA 94065 USA
* or visit www.oracle.com if you need additional information or have any
* questions.
*/
// key: compiler.warn.ineffectual.extern.method.enum
// options: -Xlint:serial
import java.io.*;
enum IneffectualExternEnum implements Externalizable {
INSTANCE;
@Override
public void writeExternal(ObjectOutput oo) {
;
}
@Override
public void readExternal(ObjectInput oi) {
;
}
}

View File

@ -0,0 +1,40 @@
/*
* Copyright (c) 2021, 2023, Oracle and/or its affiliates. All rights reserved.
* DO NOT ALTER OR REMOVE COPYRIGHT NOTICES OR THIS FILE HEADER.
*
* This code is free software; you can redistribute it and/or modify it
* under the terms of the GNU General Public License version 2 only, as
* published by the Free Software Foundation.
*
* This code is distributed in the hope that it will be useful, but WITHOUT
* ANY WARRANTY; without even the implied warranty of MERCHANTABILITY or
* FITNESS FOR A PARTICULAR PURPOSE. See the GNU General Public License
* version 2 for more details (a copy is included in the LICENSE file that
* accompanied this code).
*
* You should have received a copy of the GNU General Public License version
* 2 along with this work; if not, write to the Free Software Foundation,
* Inc., 51 Franklin St, Fifth Floor, Boston, MA 02110-1301 USA.
*
* Please contact Oracle, 500 Oracle Parkway, Redwood Shores, CA 94065 USA
* or visit www.oracle.com if you need additional information or have any
* questions.
*/
// key: compiler.warn.ineffectual.externalizable.method.record
// options: -Xlint:serial
import java.io.*;
record IneffectualExternRecord(int foo) implements Externalizable {
@Override
public void writeExternal(ObjectOutput oo) {
;
}
@Override
public void readExternal(ObjectInput oi) {
;
}
}

View File

@ -0,0 +1,59 @@
/*
* @test /nodynamiccopyright/
* @bug 8310835
* @compile/ref=EnumExtern.out -XDrawDiagnostics -Xlint:serial EnumExtern.java
* @compile/ref=empty.out -XDrawDiagnostics EnumExtern.java
*/
import java.io.*;
enum EnumExtern implements Externalizable {
INSTANCE;
// Verify a warning is generated in an enum class for each of the
// distinguished serial fields and methods as well as extern methods.
private static final long serialVersionUID = 42;
private static final ObjectStreamField[] serialPersistentFields = {};
private void writeObject(ObjectOutputStream stream) throws IOException {
stream.defaultWriteObject();
}
private Object writeReplace() throws ObjectStreamException {
return null;
}
private void readObject(ObjectInputStream stream)
throws IOException, ClassNotFoundException {
stream.defaultReadObject();
}
private void readObjectNoData() throws ObjectStreamException {
return;
}
private Object readResolve() throws ObjectStreamException {
return null;
}
// ineffective Externalizable methods
@Override
public void writeExternal(ObjectOutput oo) {
;
}
@Override
public void readExternal(ObjectInput oi) {
;
}
// _Not_ Externalizable methods; shouldn't generate a warning
public void writeExternal() {
;
}
public void readExternal() {
;
}
}

View File

@ -0,0 +1,10 @@
EnumExtern.java:16:31: compiler.warn.ineffectual.serial.field.enum: serialVersionUID
EnumExtern.java:17:46: compiler.warn.ineffectual.serial.field.enum: serialPersistentFields
EnumExtern.java:19:18: compiler.warn.ineffectual.serial.method.enum: writeObject
EnumExtern.java:23:20: compiler.warn.ineffectual.serial.method.enum: writeReplace
EnumExtern.java:27:18: compiler.warn.ineffectual.serial.method.enum: readObject
EnumExtern.java:32:18: compiler.warn.ineffectual.serial.method.enum: readObjectNoData
EnumExtern.java:36:20: compiler.warn.ineffectual.serial.method.enum: readResolve
EnumExtern.java:42:17: compiler.warn.ineffectual.extern.method.enum: writeExternal
EnumExtern.java:47:17: compiler.warn.ineffectual.extern.method.enum: readExternal
9 warnings

View File

@ -2,7 +2,7 @@
* @test /nodynamiccopyright/
* @bug 8202056
* @compile/ref=EnumSerial.out -XDrawDiagnostics -Xlint:serial EnumSerial.java
* @compile/ref=empty.out -XDrawDiagnostics EnumSerial.java
* @compile/ref=empty.out -XDrawDiagnostics EnumSerial.java
*/
import java.io.*;

View File

@ -0,0 +1,66 @@
/*
* @test /nodynamiccopyright/
* @bug 8310835
* @compile/ref=RecordExtern.out -XDrawDiagnostics -Xlint:serial RecordExtern.java
*/
import java.io.*;
record RecordExtern(int foo) implements Externalizable {
// Verify a warning is generated in a record class for each of the
// ineffectual extern methods.
// ineffective Externalizable methods
@Override
public void writeExternal(ObjectOutput oo) {
;
}
@Override
public void readExternal(ObjectInput oi) {
;
}
// *Not* Externalizable methods; shouldn't generate a warning
public void writeExternal() {
;
}
public void readExternal() {
;
}
// Check warnings for serialization methods and fields too
// partially effective
private static final long serialVersionUID = 42;
// ineffectual
private static final ObjectStreamField[] serialPersistentFields = {};
// ineffectual
private void writeObject(ObjectOutputStream stream) throws IOException {
stream.defaultWriteObject();
}
// (possibly) effective
private Object writeReplace() throws ObjectStreamException {
return null;
}
// ineffectual
private void readObject(ObjectInputStream stream)
throws IOException, ClassNotFoundException {
stream.defaultReadObject();
}
// ineffectual
private void readObjectNoData() throws ObjectStreamException {
return;
}
// (possibly) effective
private Object readResolve() throws ObjectStreamException {
return null;
}
}

View File

@ -0,0 +1,7 @@
RecordExtern.java:15:17: compiler.warn.ineffectual.externalizable.method.record: writeExternal
RecordExtern.java:20:17: compiler.warn.ineffectual.externalizable.method.record: readExternal
RecordExtern.java:39:46: compiler.warn.ineffectual.serial.field.record
RecordExtern.java:42:18: compiler.warn.ineffectual.serial.method.record: writeObject
RecordExtern.java:52:18: compiler.warn.ineffectual.serial.method.record: readObject
RecordExtern.java:58:18: compiler.warn.ineffectual.serial.method.record: readObjectNoData
6 warnings