From 9238a3ab76c46e2bc476ff0398658f6d6f5a28a7 Mon Sep 17 00:00:00 2001 From: =?UTF-8?q?M=C3=A5rten=20Kongstad?= Date: Tue, 16 Apr 2024 13:19:50 +0200 Subject: [PATCH] check-flagged-apis: create list of @FlaggedApi errors Teach check-flagged-apis to cross-check the data from its three input sources. This allows the tool to detect - @FlaggedApi references to non-existent flags - @FlaggedApi APIs present in the build artifacts even though the flag is disabled - @FlaggedApi APIs not present in the build artifacts even though the flag is enabled By passing in different sources, the tool can detect these errors for any of the API surfaces (public, @SystemApi(MODULE_LIBRARIES), etc). Note: the tool assumes that a disabled flag means that the @FlaggedApi should not be present in the build output. This is currently true, but won't be once metalava starts reverting @FlaggedApis to their previous SDK snapshot. Bug: 334870672 Test: atest --host check-flagged-apis-test Test: check-flagged-apis --api-signature out/target/product/mainline_x86/obj/ETC/frameworks-base-api-current.txt_intermediates/frameworks-base-api-current.txt --flag-values out/soong/.intermediates/all_aconfig_declarations.pb --api-versions out/dist/data/api-versions.xml Change-Id: I790234865f831af7d45895def14d1d6740365622 --- .../checkflaggedapis/CheckFlaggedApisTest.kt | 58 ++++++++++----- .../src/com/android/checkflaggedapis/Main.kt | 71 +++++++++++++++++-- 2 files changed, 108 insertions(+), 21 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 9e6a6e334a..d2b75d48ec 100644 --- a/tools/check-flagged-apis/src/com/android/checkflaggedapis/CheckFlaggedApisTest.kt +++ b/tools/check-flagged-apis/src/com/android/checkflaggedapis/CheckFlaggedApisTest.kt @@ -20,6 +20,7 @@ import com.android.tradefed.testtype.DeviceJUnit4ClassRunner import com.android.tradefed.testtype.junit4.BaseHostJUnit4Test import java.io.ByteArrayInputStream import java.io.ByteArrayOutputStream +import java.io.InputStream import org.junit.Assert.assertEquals import org.junit.Test import org.junit.runner.RunWith @@ -36,22 +37,6 @@ private val API_SIGNATURE = """ .trim() -private val PARSED_FLAGS = - { - val parsed_flag = - Aconfig.parsed_flag - .newBuilder() - .setPackage("android.flag") - .setName("foo") - .setState(Aconfig.flag_state.ENABLED) - .setPermission(Aconfig.flag_permission.READ_ONLY) - .build() - val parsed_flags = Aconfig.parsed_flags.newBuilder().addParsedFlag(parsed_flag).build() - val binaryProto = ByteArrayOutputStream() - parsed_flags.writeTo(binaryProto) - ByteArrayInputStream(binaryProto.toByteArray()) - }() - private val API_VERSIONS = """ @@ -64,6 +49,21 @@ private val API_VERSIONS = """ .trim() +private fun generateFlagsProto(fooState: Aconfig.flag_state): InputStream { + val parsed_flag = + Aconfig.parsed_flag + .newBuilder() + .setPackage("android.flag") + .setName("foo") + .setState(fooState) + .setPermission(Aconfig.flag_permission.READ_ONLY) + .build() + val parsed_flags = Aconfig.parsed_flags.newBuilder().addParsedFlag(parsed_flag).build() + val binaryProto = ByteArrayOutputStream() + parsed_flags.writeTo(binaryProto) + return ByteArrayInputStream(binaryProto.toByteArray()) +} + @RunWith(DeviceJUnit4ClassRunner::class) class CheckFlaggedApisTest : BaseHostJUnit4Test() { @Test @@ -76,7 +76,7 @@ class CheckFlaggedApisTest : BaseHostJUnit4Test() { @Test fun testParseFlagValues() { val expected: Map = mapOf(Flag("android.flag.foo") to true) - val actual = parseFlagValues(PARSED_FLAGS) + val actual = parseFlagValues(generateFlagsProto(Aconfig.flag_state.ENABLED)) assertEquals(expected, actual) } @@ -86,4 +86,28 @@ class CheckFlaggedApisTest : BaseHostJUnit4Test() { val actual = parseApiVersions(API_VERSIONS.byteInputStream()) assertEquals(expected, actual) } + + @Test + fun testFindErrorsNoErrors() { + val expected = setOf() + val actual = + findErrors( + parseApiSignature("in-memory", API_SIGNATURE.byteInputStream()), + parseFlagValues(generateFlagsProto(Aconfig.flag_state.ENABLED)), + parseApiVersions(API_VERSIONS.byteInputStream())) + assertEquals(expected, actual) + } + + @Test + fun testFindErrorsDisabledFlaggedApiIsPresent() { + val expected = + setOf( + DisabledFlaggedApiIsPresentError(Symbol("android.Clazz.FOO"), Flag("android.flag.foo"))) + val actual = + findErrors( + parseApiSignature("in-memory", API_SIGNATURE.byteInputStream()), + parseFlagValues(generateFlagsProto(Aconfig.flag_state.DISABLED)), + parseApiVersions(API_VERSIONS.byteInputStream())) + assertEquals(expected, actual) + } } 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 e7eff176be..84564ba34c 100644 --- a/tools/check-flagged-apis/src/com/android/checkflaggedapis/Main.kt +++ b/tools/check-flagged-apis/src/com/android/checkflaggedapis/Main.kt @@ -81,6 +81,36 @@ internal value class Flag(val name: String) { override fun toString(): String = name.toString() } +internal sealed class ApiError { + abstract val symbol: Symbol + abstract val flag: Flag +} + +internal data class EnabledFlaggedApiNotPresentError( + override val symbol: Symbol, + override val flag: Flag +) : ApiError() { + override fun toString(): String { + return "error: enabled @FlaggedApi not present in built artifact: symbol=$symbol flag=$flag" + } +} + +internal data class DisabledFlaggedApiIsPresentError( + override val symbol: Symbol, + override val flag: Flag +) : ApiError() { + override fun toString(): String { + return "error: disabled @FlaggedApi is present in built artifact: symbol=$symbol 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" + } +} + class CheckCommand : CliktCommand( help = @@ -122,16 +152,17 @@ The tool will exit with a non-zero exit code if any flagged APIs are found to be .required() override fun run() { - @Suppress("UNUSED_VARIABLE") val flaggedSymbols = apiSignaturePath.toFile().inputStream().use { parseApiSignature(apiSignaturePath.toString(), it) } - @Suppress("UNUSED_VARIABLE") val flags = flagValuesPath.toFile().inputStream().use { parseFlagValues(it) } - @Suppress("UNUSED_VARIABLE") val exportedSymbols = apiVersionsPath.toFile().inputStream().use { parseApiVersions(it) } - throw ProgramResult(0) + val errors = findErrors(flaggedSymbols, flags, exportedSymbols) + for (e in errors) { + println(e) + } + throw ProgramResult(errors.size) } } @@ -185,4 +216,36 @@ internal fun parseApiVersions(input: InputStream): Set { return output } +/** + * Find errors in the given data. + * + * @param flaggedSymbolsInSource the set of symbols that are flagged in the source code + * @param flags the set of flags and their values + * @param symbolsInOutput the set of symbols that are present in the output + * @return the set of errors found + */ +internal fun findErrors( + flaggedSymbolsInSource: Set>, + flags: Map, + symbolsInOutput: Set +): Set { + val errors = mutableSetOf() + for ((symbol, flag) in flaggedSymbolsInSource) { + try { + if (flags.getValue(flag)) { + if (!symbolsInOutput.contains(symbol)) { + errors.add(EnabledFlaggedApiNotPresentError(symbol, flag)) + } + } else { + if (symbolsInOutput.contains(symbol)) { + errors.add(DisabledFlaggedApiIsPresentError(symbol, flag)) + } + } + } catch (e: NoSuchElementException) { + errors.add(UnknownFlagError(symbol, flag)) + } + } + return errors +} + fun main(args: Array) = CheckCommand().main(args)