apicheck: Allow safe addition and removal of final qualifier.
Split the CHANGED_FINAL error into ADDED_FINAL,
ADDED_FINAL_UNINSTANTIABLE and REMOVED_FINAL so that the
API check scripts can more precisely target policy around
changes to the final qualifier.
ADDED_FINAL_UNINSTANTIABLE covers the case of a class
that has become final but that was previously not
instantiable. This change is safe since applications could
not have declared subclasses of their own.
Change-Id: I36a143bdbcffd04379788a55ff996b47398ba4fe
diff --git a/src/com/google/doclava/ClassInfo.java b/src/com/google/doclava/ClassInfo.java
index 927b240..f3993eb 100644
--- a/src/com/google/doclava/ClassInfo.java
+++ b/src/com/google/doclava/ClassInfo.java
@@ -1694,10 +1694,25 @@
+ " changed abstract qualifier");
}
- if (mIsFinal != cl.mIsFinal) {
+ if (!mIsFinal && cl.mIsFinal) {
+ /*
+ * It is safe to make a class final if it did not previously have any public
+ * constructors because it was impossible for an application to create a subclass.
+ */
+ if (mApiCheckConstructors.isEmpty()) {
+ consistent = false;
+ Errors.error(Errors.ADDED_FINAL_UNINSTANTIABLE, cl.position(),
+ "Class " + cl.qualifiedName() + " added final qualifier but "
+ + "was previously uninstantiable and therefore could not be subclassed");
+ } else {
+ consistent = false;
+ Errors.error(Errors.ADDED_FINAL, cl.position(), "Class " + cl.qualifiedName()
+ + " added final qualifier");
+ }
+ } else if (mIsFinal && !cl.mIsFinal) {
consistent = false;
- Errors.error(Errors.CHANGED_FINAL, cl.position(), "Class " + cl.qualifiedName()
- + " changed final qualifier");
+ Errors.error(Errors.REMOVED_FINAL, cl.position(), "Class " + cl.qualifiedName()
+ + " removed final qualifier");
}
if (mIsStatic != cl.mIsStatic) {
diff --git a/src/com/google/doclava/Errors.java b/src/com/google/doclava/Errors.java
index 2fc0f0c..e890aa4 100644
--- a/src/com/google/doclava/Errors.java
+++ b/src/com/google/doclava/Errors.java
@@ -134,7 +134,7 @@
public static Error REMOVED_FIELD = new Error(10, WARNING);
public static Error REMOVED_INTERFACE = new Error(11, WARNING);
public static Error CHANGED_STATIC = new Error(12, WARNING);
- public static Error CHANGED_FINAL = new Error(13, WARNING);
+ public static Error ADDED_FINAL = new Error(13, WARNING);
public static Error CHANGED_TRANSIENT = new Error(14, WARNING);
public static Error CHANGED_VOLATILE = new Error(15, WARNING);
public static Error CHANGED_TYPE = new Error(16, WARNING);
@@ -147,6 +147,8 @@
public static Error CHANGED_CLASS = new Error(23, WARNING);
public static Error CHANGED_DEPRECATED = new Error(24, WARNING);
public static Error CHANGED_SYNCHRONIZED = new Error(25, ERROR);
+ public static Error ADDED_FINAL_UNINSTANTIABLE = new Error(26, WARNING);
+ public static Error REMOVED_FINAL = new Error(27, WARNING);
// Errors in javadoc generation
public static final Error UNRESOLVED_LINK = new Error(101, WARNING);
@@ -175,10 +177,11 @@
UNAVAILABLE_SYMBOL, HIDDEN_SUPERCLASS, DEPRECATED, DEPRECATION_MISMATCH, MISSING_COMMENT,
IO_ERROR, NO_SINCE_DATA, NO_FEDERATION_DATA, PARSE_ERROR, ADDED_PACKAGE, ADDED_CLASS,
ADDED_METHOD, ADDED_FIELD, ADDED_INTERFACE, REMOVED_PACKAGE, REMOVED_CLASS,
- REMOVED_METHOD, REMOVED_FIELD, REMOVED_INTERFACE, CHANGED_STATIC, CHANGED_FINAL,
+ REMOVED_METHOD, REMOVED_FIELD, REMOVED_INTERFACE, CHANGED_STATIC, ADDED_FINAL,
CHANGED_TRANSIENT, CHANGED_VOLATILE, CHANGED_TYPE, CHANGED_VALUE, CHANGED_SUPERCLASS,
CHANGED_SCOPE, CHANGED_ABSTRACT, CHANGED_THROWS, CHANGED_NATIVE, CHANGED_CLASS,
- CHANGED_DEPRECATED, CHANGED_SYNCHRONIZED, BROKEN_SINCE_FILE, INVALID_CONTENT_TYPE};
+ CHANGED_DEPRECATED, CHANGED_SYNCHRONIZED, ADDED_FINAL_UNINSTANTIABLE, REMOVED_FINAL,
+ BROKEN_SINCE_FILE, INVALID_CONTENT_TYPE};
public static boolean setErrorLevel(int code, int level) {
for (Error e : ERRORS) {
diff --git a/src/com/google/doclava/FieldInfo.java b/src/com/google/doclava/FieldInfo.java
index 0200b95..09391a6 100644
--- a/src/com/google/doclava/FieldInfo.java
+++ b/src/com/google/doclava/FieldInfo.java
@@ -438,9 +438,13 @@
consistent = false;
}
- if (mIsFinal != fInfo.mIsFinal) {
- Errors.error(Errors.CHANGED_FINAL, fInfo.position(), "Field " + fInfo.qualifiedName()
- + " has changed 'final' qualifier");
+ if (!mIsFinal && fInfo.mIsFinal) {
+ Errors.error(Errors.ADDED_FINAL, fInfo.position(), "Field " + fInfo.qualifiedName()
+ + " has added 'final' qualifier");
+ consistent = false;
+ } else if (mIsFinal && !fInfo.mIsFinal) {
+ Errors.error(Errors.REMOVED_FINAL, fInfo.position(), "Field " + fInfo.qualifiedName()
+ + " has removed 'final' qualifier");
consistent = false;
}
diff --git a/src/com/google/doclava/MethodInfo.java b/src/com/google/doclava/MethodInfo.java
index db5e0cf..b6a5cc3 100644
--- a/src/com/google/doclava/MethodInfo.java
+++ b/src/com/google/doclava/MethodInfo.java
@@ -624,6 +624,10 @@
return mIsVarargs;
}
+ public boolean isFinalOrBelongsToFinalClass() {
+ return mIsFinal || (containingClass() != null && containingClass().isFinal());
+ }
+
@Override
public String toString() {
return this.name();
@@ -729,17 +733,19 @@
+ " has changed 'native' qualifier");
}
- if (mIsFinal != mInfo.mIsFinal) {
- // Compiler-generated methods vary in their 'final' qual between versions of
+ if (!mIsStatic) {
+ // Compiler-generated methods vary in their 'final' qualifier between versions of
// the compiler, so this check needs to be quite narrow. A change in 'final'
// status of a method is only relevant if (a) the method is not declared 'static'
- // and (b) the method's class is not itself 'final'.
- if (!mIsStatic) {
- if ((containingClass() == null) || (!containingClass().isFinal())) {
- consistent = false;
- Errors.error(Errors.CHANGED_FINAL, mInfo.position(), "Method " + mInfo.qualifiedName()
- + " has changed 'final' qualifier");
- }
+ // and (b) the method is not already inferred to be 'final' by virtue of its class.
+ if (!isFinalOrBelongsToFinalClass() && mInfo.isFinalOrBelongsToFinalClass()) {
+ consistent = false;
+ Errors.error(Errors.ADDED_FINAL, mInfo.position(), "Method " + mInfo.qualifiedName()
+ + " has added 'final' qualifier");
+ } else if (isFinalOrBelongsToFinalClass() && !mInfo.isFinalOrBelongsToFinalClass()) {
+ consistent = false;
+ Errors.error(Errors.REMOVED_FINAL, mInfo.position(), "Method " + mInfo.qualifiedName()
+ + " has removed 'final' qualifier");
}
}
