Skip to content

Commit

Permalink
8202056: Expand serial warning to check for bad overloads of serial-r…
Browse files Browse the repository at this point in the history 
…elated methods and ineffectual fields

8160675: Issue lint warning for non-serializable non-transient instance fields in serializable type

Reviewed-by: erikj, sspitsyn, jlahoda, vromero, rriggs, smarks
  • Loading branch information
@jddarcy
jddarcy committed Oct 21, 2021
1 parent 4e9dd4b commit 6a466fe
Show file tree
Hide file tree
Showing 46 changed files with 1,928 additions and 70 deletions.
15 changes: 15 additions & 0 deletions src/jdk.compiler/share/classes/com/sun/tools/javac/code/Symtab.java
View file
Original file line number Diff line number Diff line change
Expand Up @@ -224,6 +224,14 @@ public static Symtab instance(Context context) {
public final Type valueBasedType;
public final Type valueBasedInternalType;

// For serialization lint checking
public final Type objectStreamFieldType;
public final Type objectInputStreamType;
public final Type objectOutputStreamType;
public final Type ioExceptionType;
public final Type objectStreamExceptionType;
public final Type externalizableType;

/** The symbol representing the length field of an array.
*/
public final VarSymbol lengthVar;
Expand Down Expand Up @@ -590,6 +598,13 @@ public <R, P> R accept(ElementVisitor<R, P> v, P p) {
switchBootstrapsType = enterClass("java.lang.runtime.SwitchBootstraps");
valueBasedType = enterClass("jdk.internal.ValueBased");
valueBasedInternalType = enterSyntheticAnnotation("jdk.internal.ValueBased+Annotation");
// For serialization lint checking
objectStreamFieldType = enterClass("java.io.ObjectStreamField");
objectInputStreamType = enterClass("java.io.ObjectInputStream");
objectOutputStreamType = enterClass("java.io.ObjectOutputStream");
ioExceptionType = enterClass("java.io.IOException");
objectStreamExceptionType = enterClass("java.io.ObjectStreamException");
externalizableType = enterClass("java.io.Externalizable");

synthesizeEmptyInterfaceIfMissing(autoCloseableType);
synthesizeEmptyInterfaceIfMissing(cloneableType);
Expand Down
69 changes: 8 additions & 61 deletions src/jdk.compiler/share/classes/com/sun/tools/javac/comp/Attr.java
View file
Original file line number Diff line number Diff line change
Expand Up @@ -2955,7 +2955,7 @@ private void visitAnonymousClassDefinition(JCNewClass tree, JCExpression clazz,
}

if (resultInfo.checkContext.deferredAttrContext().mode == DeferredAttr.AttrMode.CHECK &&
isSerializable(clazztype)) {
rs.isSerializable(clazztype)) {
localEnv.info.isSerializable = true;
}

Expand Down Expand Up @@ -3080,7 +3080,7 @@ public void visitLambda(final JCLambda that) {
boolean needsRecovery =
resultInfo.checkContext.deferredAttrContext().mode == DeferredAttr.AttrMode.CHECK;
try {
if (needsRecovery && isSerializable(pt())) {
if (needsRecovery && rs.isSerializable(pt())) {
localEnv.info.isSerializable = true;
localEnv.info.isSerializableLambda = true;
}
Expand Down Expand Up @@ -3581,7 +3581,7 @@ public void visitReference(final JCMemberReference that) {

boolean isTargetSerializable =
resultInfo.checkContext.deferredAttrContext().mode == DeferredAttr.AttrMode.CHECK &&
isSerializable(pt());
rs.isSerializable(pt());
TargetInfo targetInfo = getTargetInfo(that, resultInfo, null);
Type currentTarget = targetInfo.target;
Type desc = targetInfo.descriptor;
Expand Down Expand Up @@ -5366,7 +5366,7 @@ void attribClass(ClassSymbol c) throws CompletionFailure {
log.error(env.tree.pos(), Errors.EnumTypesNotExtensible);
}

if (isSerializable(c.type)) {
if (rs.isSerializable(c.type)) {
env.info.isSerializable = true;
}

Expand Down Expand Up @@ -5501,12 +5501,12 @@ private void attribClassBody(Env<AttrContext> env, ClassSymbol c) {
// Check for cycles among annotation elements.
chk.checkNonCyclicElements(tree);

// Check for proper use of serialVersionUID
// Check for proper use of serialVersionUID and other
// serialization-related fields and methods
if (env.info.lint.isEnabled(LintCategory.SERIAL)
&& isSerializable(c.type)
&& (c.flags() & (Flags.ENUM | Flags.INTERFACE)) == 0
&& rs.isSerializable(c.type)
&& !c.isAnonymous()) {
checkSerialVersionUID(tree, c, env);
chk.checkSerialStructure(tree, c);
}
if (allowTypeAnnos) {
// Correctly organize the positions of the type annotations
Expand All @@ -5527,59 +5527,6 @@ private DiagnosticPosition getDiagnosticPosition(JCClassDecl tree, Type t) {
return null;
}

/** check if a type is a subtype of Serializable, if that is available. */
boolean isSerializable(Type t) {
try {
syms.serializableType.complete();
}
catch (CompletionFailure e) {
return false;
}
return types.isSubtype(t, syms.serializableType);
}

/** Check that an appropriate serialVersionUID member is defined. */
private void checkSerialVersionUID(JCClassDecl tree, ClassSymbol c, Env<AttrContext> env) {

// check for presence of serialVersionUID
VarSymbol svuid = null;
for (Symbol sym : c.members().getSymbolsByName(names.serialVersionUID)) {
if (sym.kind == VAR) {
svuid = (VarSymbol)sym;
break;
}
}

if (svuid == null) {
if (!c.isRecord())
log.warning(LintCategory.SERIAL, tree.pos(), Warnings.MissingSVUID(c));
return;
}

// Check if @SuppressWarnings("serial") is an annotation of serialVersionUID.
// See JDK-8231622 for more information.
Lint lint = env.info.lint.augment(svuid);
if (lint.isSuppressed(LintCategory.SERIAL)) {
return;
}

// check that it is static final
if ((svuid.flags() & (STATIC | FINAL)) !=
(STATIC | FINAL))
log.warning(LintCategory.SERIAL,
TreeInfo.diagnosticPositionFor(svuid, tree), Warnings.ImproperSVUID(c));

// check that it is long
else if (!svuid.type.hasTag(LONG))
log.warning(LintCategory.SERIAL,
TreeInfo.diagnosticPositionFor(svuid, tree), Warnings.LongSVUID(c));

// check constant
else if (svuid.getConstValue() == null)
log.warning(LintCategory.SERIAL,
TreeInfo.diagnosticPositionFor(svuid, tree), Warnings.ConstantSVUID(c));
}

private Type capture(Type type) {
return types.capture(type);
}
Expand Down
Loading

1 comment on commit 6a466fe

@openjdk-notifier
Copy link

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Review

Issues

Please sign in to comment.