diff options
author | Treehugger Robot <android-test-infra-autosubmit@system.gserviceaccount.com> | 2024-05-07 13:42:25 +0000 |
---|---|---|
committer | Gerrit Code Review <noreply-gerritcodereview@google.com> | 2024-05-07 13:42:25 +0000 |
commit | 4925954a79915de8c5b60b621c4cbbafcfe3b506 (patch) | |
tree | bf95daa6d827ded7f2f9d1a76d5eb11ada573af3 | |
parent | f9ce18b5c8b7e75ac3ab3564bff67384ef4c80e9 (diff) | |
parent | d2c707613e650b7c0f9ec02d6c469cecb408973c (diff) | |
download | build-4925954a79915de8c5b60b621c4cbbafcfe3b506.tar.gz |
Merge changes Ia6dfcfa8,I8d93c230,I4db7ff47,I003535c7,I8c0619fa into main
* changes:
check-flagged-apis: consider interfaces when looking up symbol
check-flagged-apis: skip self-referential interfaces
check-flagged-apis: record interfaces when parsing classes
check-flagged-apis: add more details to Symbol class
check-flagged-apis: api-versions.xml: correctly parse nested class ctor
-rw-r--r-- | tools/check-flagged-apis/src/com/android/checkflaggedapis/CheckFlaggedApisTest.kt | 112 | ||||
-rw-r--r-- | tools/check-flagged-apis/src/com/android/checkflaggedapis/Main.kt | 120 |
2 files changed, 178 insertions, 54 deletions
diff --git a/tools/check-flagged-apis/src/com/android/checkflaggedapis/CheckFlaggedApisTest.kt b/tools/check-flagged-apis/src/com/android/checkflaggedapis/CheckFlaggedApisTest.kt index 2f76b2a5a7..3cd6a24256 100644 --- a/tools/check-flagged-apis/src/com/android/checkflaggedapis/CheckFlaggedApisTest.kt +++ b/tools/check-flagged-apis/src/com/android/checkflaggedapis/CheckFlaggedApisTest.kt @@ -95,20 +95,20 @@ class CheckFlaggedApisTest { fun testParseApiSignature() { val expected = setOf( - Pair(Symbol("android/Clazz"), Flag("android.flag.foo")), - Pair(Symbol("android/Clazz/Clazz()"), Flag("android.flag.foo")), - Pair(Symbol("android/Clazz/FOO"), Flag("android.flag.foo")), - Pair(Symbol("android/Clazz/getErrorCode()"), Flag("android.flag.foo")), + Pair(Symbol.createClass("android/Clazz", setOf()), Flag("android.flag.foo")), + Pair(Symbol.createMethod("android/Clazz", "Clazz()"), Flag("android.flag.foo")), + Pair(Symbol.createField("android/Clazz", "FOO"), Flag("android.flag.foo")), + Pair(Symbol.createMethod("android/Clazz", "getErrorCode()"), Flag("android.flag.foo")), Pair( - Symbol("android/Clazz/setData(I[[ILandroid/util/Utility;)"), + Symbol.createMethod("android/Clazz", "setData(I[[ILandroid/util/Utility;)"), Flag("android.flag.foo")), Pair( - Symbol("android/Clazz/setVariableData(I[Landroid/util/Atom;)"), + Symbol.createMethod("android/Clazz", "setVariableData(I[Landroid/util/Atom;)"), Flag("android.flag.foo")), Pair( - Symbol("android/Clazz/innerClassArg(Landroid/Clazz/Builder;)"), + Symbol.createMethod("android/Clazz", "innerClassArg(Landroid/Clazz/Builder;)"), Flag("android.flag.foo")), - Pair(Symbol("android/Clazz/Builder"), Flag("android.flag.bar")), + Pair(Symbol.createClass("android/Clazz/Builder", setOf()), Flag("android.flag.bar")), ) val actual = parseApiSignature("in-memory", API_SIGNATURE.byteInputStream()) assertEquals(expected, actual) @@ -126,20 +126,41 @@ class CheckFlaggedApisTest { fun testParseApiVersions() { val expected: Set<Symbol> = setOf( - Symbol("android/Clazz"), - Symbol("android/Clazz/Clazz()"), - Symbol("android/Clazz/FOO"), - Symbol("android/Clazz/getErrorCode()"), - Symbol("android/Clazz/setData(I[[ILandroid/util/Utility;)"), - Symbol("android/Clazz/setVariableData(I[Landroid/util/Atom;)"), - Symbol("android/Clazz/innerClassArg(Landroid/Clazz/Builder;)"), - Symbol("android/Clazz/Builder"), + Symbol.createClass("android/Clazz", setOf()), + Symbol.createMethod("android/Clazz", "Clazz()"), + Symbol.createField("android/Clazz", "FOO"), + Symbol.createMethod("android/Clazz", "getErrorCode()"), + Symbol.createMethod("android/Clazz", "setData(I[[ILandroid/util/Utility;)"), + Symbol.createMethod("android/Clazz", "setVariableData(I[Landroid/util/Atom;)"), + Symbol.createMethod("android/Clazz", "innerClassArg(Landroid/Clazz/Builder;)"), + Symbol.createClass("android/Clazz/Builder", setOf()), ) val actual = parseApiVersions(API_VERSIONS.byteInputStream()) assertEquals(expected, actual) } @Test + fun testParseApiVersionsNestedClasses() { + val apiVersions = + """ + <?xml version="1.0" encoding="utf-8"?> + <api version="3"> + <class name="android/Clazz${'$'}Foo${'$'}Bar" since="1"> + <method name="<init>()V"/> + </class> + </api> + """ + .trim() + val expected: Set<Symbol> = + setOf( + Symbol.createClass("android/Clazz/Foo/Bar", setOf()), + Symbol.createMethod("android/Clazz/Foo/Bar", "Bar()"), + ) + val actual = parseApiVersions(apiVersions.byteInputStream()) + assertEquals(expected, actual) + } + + @Test fun testFindErrorsNoErrors() { val expected = setOf<ApiError>() val actual = @@ -151,26 +172,69 @@ class CheckFlaggedApisTest { } @Test + fun testFindErrorsVerifyImplements() { + val apiSignature = + """ + // Signature format: 2.0 + package android { + @FlaggedApi("android.flag.foo") public final class Clazz implements android.Interface { + method @FlaggedApi("android.flag.foo") public boolean foo(); + method @FlaggedApi("android.flag.foo") public boolean bar(); + } + public interface Interface { + method public boolean bar(); + } + } + """ + .trim() + + val apiVersions = + """ + <?xml version="1.0" encoding="utf-8"?> + <api version="3"> + <class name="android/Clazz" since="1"> + <implements name="android/Interface"/> + <method name="foo()Z"/> + </class> + <class name="android/Interface" since="1"> + <method name="bar()Z"/> + </class> + </api> + """ + .trim() + + val expected = setOf<ApiError>() + val actual = + findErrors( + parseApiSignature("in-memory", apiSignature.byteInputStream()), + parseFlagValues(generateFlagsProto(ENABLED, ENABLED)), + parseApiVersions(apiVersions.byteInputStream())) + assertEquals(expected, actual) + } + + @Test fun testFindErrorsDisabledFlaggedApiIsPresent() { val expected = setOf<ApiError>( - DisabledFlaggedApiIsPresentError(Symbol("android/Clazz"), Flag("android.flag.foo")), DisabledFlaggedApiIsPresentError( - Symbol("android/Clazz/Clazz()"), Flag("android.flag.foo")), - DisabledFlaggedApiIsPresentError(Symbol("android/Clazz/FOO"), Flag("android.flag.foo")), + Symbol.createClass("android/Clazz", setOf()), Flag("android.flag.foo")), + DisabledFlaggedApiIsPresentError( + Symbol.createMethod("android/Clazz", "Clazz()"), Flag("android.flag.foo")), + DisabledFlaggedApiIsPresentError( + Symbol.createField("android/Clazz", "FOO"), Flag("android.flag.foo")), DisabledFlaggedApiIsPresentError( - Symbol("android/Clazz/getErrorCode()"), Flag("android.flag.foo")), + Symbol.createMethod("android/Clazz", "getErrorCode()"), Flag("android.flag.foo")), DisabledFlaggedApiIsPresentError( - Symbol("android/Clazz/setData(I[[ILandroid/util/Utility;)"), + Symbol.createMethod("android/Clazz", "setData(I[[ILandroid/util/Utility;)"), Flag("android.flag.foo")), DisabledFlaggedApiIsPresentError( - Symbol("android/Clazz/setVariableData(I[Landroid/util/Atom;)"), + Symbol.createMethod("android/Clazz", "setVariableData(I[Landroid/util/Atom;)"), Flag("android.flag.foo")), DisabledFlaggedApiIsPresentError( - Symbol("android/Clazz/innerClassArg(Landroid/Clazz/Builder;)"), + Symbol.createMethod("android/Clazz", "innerClassArg(Landroid/Clazz/Builder;)"), Flag("android.flag.foo")), DisabledFlaggedApiIsPresentError( - Symbol("android/Clazz/Builder"), Flag("android.flag.bar")), + Symbol.createClass("android/Clazz/Builder", setOf()), Flag("android.flag.bar")), ) val actual = findErrors( diff --git a/tools/check-flagged-apis/src/com/android/checkflaggedapis/Main.kt b/tools/check-flagged-apis/src/com/android/checkflaggedapis/Main.kt index e8b1b65fce..867ee94a9a 100644 --- a/tools/check-flagged-apis/src/com/android/checkflaggedapis/Main.kt +++ b/tools/check-flagged-apis/src/com/android/checkflaggedapis/Main.kt @@ -54,29 +54,41 @@ import org.w3c.dom.Node * * 1. https://docs.oracle.com/javase/specs/jvms/se7/html/jvms-4.html#jvms-4.3.2 */ -@JvmInline -internal value class Symbol(val name: String) { +internal sealed class Symbol { companion object { private val FORBIDDEN_CHARS = listOf('#', '$', '.') - /** Create a new Symbol from a String that may include delimiters other than dot. */ - fun create(name: String): Symbol { - var sanitizedName = name + fun createClass(clazz: String, interfaces: Set<String>): Symbol { + return ClassSymbol(toInternalFormat(clazz), interfaces.map { toInternalFormat(it) }.toSet()) + } + + fun createField(clazz: String, field: String): Symbol { + require(!field.contains("(") && !field.contains(")")) + return MemberSymbol(toInternalFormat(clazz), toInternalFormat(field)) + } + + fun createMethod(clazz: String, method: String): Symbol { + return MemberSymbol(toInternalFormat(clazz), toInternalFormat(method)) + } + + protected fun toInternalFormat(name: String): String { + var internalName = name for (ch in FORBIDDEN_CHARS) { - sanitizedName = sanitizedName.replace(ch, '/') + internalName = internalName.replace(ch, '/') } - return Symbol(sanitizedName) + return internalName } } - init { - require(!name.isEmpty()) { "empty string" } - for (ch in FORBIDDEN_CHARS) { - require(!name.contains(ch)) { "$name: contains $ch" } - } - } + abstract fun toPrettyString(): String +} - override fun toString(): String = name.toString() +internal data class ClassSymbol(val clazz: String, val interfaces: Set<String>) : Symbol() { + override fun toPrettyString(): String = "$clazz" +} + +internal data class MemberSymbol(val clazz: String, val member: String) : Symbol() { + override fun toPrettyString(): String = "$clazz/$member" } /** @@ -102,7 +114,7 @@ internal data class EnabledFlaggedApiNotPresentError( override val flag: Flag ) : ApiError() { override fun toString(): String { - return "error: enabled @FlaggedApi not present in built artifact: symbol=$symbol flag=$flag" + return "error: enabled @FlaggedApi not present in built artifact: symbol=${symbol.toPrettyString()} flag=$flag" } } @@ -111,14 +123,14 @@ internal data class DisabledFlaggedApiIsPresentError( override val flag: Flag ) : ApiError() { override fun toString(): String { - return "error: disabled @FlaggedApi is present in built artifact: symbol=$symbol flag=$flag" + return "error: disabled @FlaggedApi is present in built artifact: symbol=${symbol.toPrettyString()} flag=$flag" } } internal data class UnknownFlagError(override val symbol: Symbol, override val flag: Flag) : ApiError() { override fun toString(): String { - return "error: unknown flag: symbol=$symbol flag=$flag" + return "error: unknown flag: symbol=${symbol.toPrettyString()} flag=$flag" } } @@ -183,29 +195,34 @@ internal fun parseApiSignature(path: String, input: InputStream): Set<Pair<Symbo object : BaseItemVisitor() { override fun visitClass(cls: ClassItem) { getFlagOrNull(cls)?.let { flag -> - val symbol = Symbol.create(cls.baselineElementId()) + val symbol = + Symbol.createClass( + cls.baselineElementId(), + cls.allInterfaces() + .map { it.baselineElementId() } + .filter { it != cls.baselineElementId() } + .toSet()) output.add(Pair(symbol, flag)) } } override fun visitField(field: FieldItem) { getFlagOrNull(field)?.let { flag -> - val symbol = Symbol.create(field.baselineElementId()) + val symbol = + Symbol.createField(field.containingClass().baselineElementId(), field.name()) output.add(Pair(symbol, flag)) } } override fun visitMethod(method: MethodItem) { getFlagOrNull(method)?.let { flag -> - val name = buildString { - append(method.containingClass().qualifiedName()) - append(".") + val methodName = buildString { append(method.name()) append("(") method.parameters().joinTo(this, separator = "") { it.type().internalName() } append(")") } - val symbol = Symbol.create(name) + val symbol = Symbol.createMethod(method.containingClass().qualifiedName(), methodName) output.add(Pair(symbol, flag)) } } @@ -246,7 +263,19 @@ internal fun parseApiVersions(input: InputStream): Set<Symbol> { requireNotNull(cls.getAttribute("name")) { "Bad XML: <class> element without name attribute" } - output.add(Symbol.create(className.replace("/", "."))) + val interfaces = mutableSetOf<String>() + val children = cls.getChildNodes() + for (j in 0.rangeUntil(children.getLength())) { + val child = children.item(j) + if (child.getNodeName() == "implements") { + val interfaceName = + requireNotNull(child.getAttribute("name")) { + "Bad XML: <implements> element without name attribute" + } + interfaces.add(interfaceName) + } + } + output.add(Symbol.createClass(className, interfaces)) } val fields = document.getElementsByTagName("field") @@ -261,7 +290,7 @@ internal fun parseApiVersions(input: InputStream): Set<Symbol> { requireNotNull(field.getParentNode()?.getAttribute("name")) { "Bad XML: top level <field> element" } - output.add(Symbol.create("${className.replace("/", ".")}.$fieldName")) + output.add(Symbol.createField(className, fieldName)) } val methods = document.getElementsByTagName("method") @@ -279,12 +308,13 @@ internal fun parseApiVersions(input: InputStream): Set<Symbol> { var (methodName, methodArgs, _) = methodSignatureParts val packageAndClassName = requireNotNull(method.getParentNode()?.getAttribute("name")) { - "Bad XML: top level <method> element, or <class> element missing name attribute" - } + "Bad XML: top level <method> element, or <class> element missing name attribute" + } + .replace("$", "/") if (methodName == "<init>") { methodName = packageAndClassName.split("/").last() } - output.add(Symbol.create("${packageAndClassName.replace("/", ".")}.$methodName($methodArgs)")) + output.add(Symbol.createMethod(packageAndClassName, "$methodName($methodArgs)")) } return output @@ -303,15 +333,45 @@ internal fun findErrors( flags: Map<Flag, Boolean>, symbolsInOutput: Set<Symbol> ): Set<ApiError> { + fun Set<Symbol>.containsSymbol(symbol: Symbol): Boolean { + // trivial case: the symbol is explicitly listed in api-versions.xml + if (contains(symbol)) { + return true + } + + // non-trivial case: the symbol could be part of the surrounding class' + // super class or interfaces + val (className, memberName) = + when (symbol) { + is ClassSymbol -> return false + is MemberSymbol -> { + Pair(symbol.clazz, symbol.member) + } + } + val clazz = find { it is ClassSymbol && it.clazz == className } as? ClassSymbol? + if (clazz == null) { + return false + } + + for (interfaceName in clazz.interfaces) { + // createMethod is the same as createField, except it allows parenthesis + val interfaceSymbol = Symbol.createMethod(interfaceName, memberName) + if (contains(interfaceSymbol)) { + return true + } + } + + return false + } val errors = mutableSetOf<ApiError>() for ((symbol, flag) in flaggedSymbolsInSource) { try { if (flags.getValue(flag)) { - if (!symbolsInOutput.contains(symbol)) { + if (!symbolsInOutput.containsSymbol(symbol)) { errors.add(EnabledFlaggedApiNotPresentError(symbol, flag)) } } else { - if (symbolsInOutput.contains(symbol)) { + if (symbolsInOutput.containsSymbol(symbol)) { errors.add(DisabledFlaggedApiIsPresentError(symbol, flag)) } } |