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
2024-05-07 13:42:25 +00:00
parent f9ce18b5c8 d2c707613e
commit 4925954a79
2 changed files with 178 additions and 54 deletions
--- 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,19 +126,40 @@ 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="&lt;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>()
@@ -150,27 +171,70 @@ class CheckFlaggedApisTest {
    assertEquals(expected, actual)
  }

+  @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("android/Clazz/getErrorCode()"), Flag("android.flag.foo")),
+                Symbol.createMethod("android/Clazz", "Clazz()"), Flag("android.flag.foo")),
            DisabledFlaggedApiIsPresentError(
-                Symbol("android/Clazz/setData(I[[ILandroid/util/Utility;)"),
+                Symbol.createField("android/Clazz", "FOO"), Flag("android.flag.foo")),
+            DisabledFlaggedApiIsPresentError(
+                Symbol.createMethod("android/Clazz", "getErrorCode()"), Flag("android.flag.foo")),
+            DisabledFlaggedApiIsPresentError(
+                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(
--- 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
-      for (ch in FORBIDDEN_CHARS) {
-        sanitizedName = sanitizedName.replace(ch, '/')
+    fun createClass(clazz: String, interfaces: Set<String>): Symbol {
+      return ClassSymbol(toInternalFormat(clazz), interfaces.map { toInternalFormat(it) }.toSet())
    }
-      return Symbol(sanitizedName)
+
+    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) {
+        internalName = internalName.replace(ch, '/')
+      }
+      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")
@@ -281,10 +310,11 @@ internal fun parseApiVersions(input: InputStream): Set<Symbol> {
        requireNotNull(method.getParentNode()?.getAttribute("name")) {
              "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))
        }
      }