diff --git a/Android.bp b/Android.bp index 2061683a72a3b6a77b6c6b735a0e7ba0bb95e4e0..669e14a3cb8e78e8e157518c0a464581e4e573dc 100644 --- a/Android.bp +++ b/Android.bp @@ -25,6 +25,9 @@ java_binary_host { "metalava-gradle-plugin-deps", ], manifest: "manifest.txt", + dist: { + targets: ["sdk"], + }, } java_library { @@ -34,6 +37,13 @@ java_library { "stub-annotations/src/main/java/**/*.java", ], sdk_version: "core_current", + target: { + host: { + dist: { + targets: ["sdk"], + }, + }, + }, } genrule { @@ -46,7 +56,7 @@ genrule { "stub-annotations/src/main/java/**/*.java", ], cmd: "($(location metalava) --no-banner --copy-annotations tools/metalava/stub-annotations " + - "$(genDir)/private-stub-annotations) && ($(location soong_zip) -o $(out) -C $(genDir) -D $(genDir))", + "$(genDir)/private-stub-annotations) && ($(location soong_zip) -o $(out) -C $(genDir) -D $(genDir))", out: [ "private-stub-annotations.srcjar", ], diff --git a/Android.mk b/Android.mk deleted file mode 100644 index a03897c2923c7fd730a34b4a38e1829efa62a9ad..0000000000000000000000000000000000000000 --- a/Android.mk +++ /dev/null @@ -1,20 +0,0 @@ -# -# Copyright (C) 2018 The Android Open Source Project -# -# Licensed under the Apache License, Version 2.0 (the "License"); -# you may not use this file except in compliance with the License. -# You may obtain a copy of the License at -# -# http://www.apache.org/licenses/LICENSE-2.0 -# -# Unless required by applicable law or agreed to in writing, software -# distributed under the License is distributed on an "AS IS" BASIS, -# WITHOUT WARRANTIES OR CONDITIONS OF ANY KIND, either express or implied. -# See the License for the specific language governing permissions and -# limitations under the License. -# -LOCAL_PATH := $(call my-dir) - -$(call dist-for-goals,sdk,$(HOST_OUT_JAVA_LIBRARIES)/metalava.jar) - -$(call dist-for-goals,sdk,$(HOST_OUT_JAVA_LIBRARIES)/stub-annotations.jar) diff --git a/OWNERS b/OWNERS new file mode 100644 index 0000000000000000000000000000000000000000..923e1a6eaa824310ca0e9c5412ea8dec3b7d35cb --- /dev/null +++ b/OWNERS @@ -0,0 +1,3 @@ +# Default code reviewers picked from top 3 or more developers. +# Please update this list if you find better candidates. +tnorbye@google.com diff --git a/README.md b/README.md index ed88779e0a690a64cbb4b9952c7830009383f77e..2034bcca2351a9d10abac04ffc5f9f58a0e36225 100644 --- a/README.md +++ b/README.md @@ -180,6 +180,10 @@ example, you'll see something like this (unless running with --quiet) : * API Lint: Metalava can optionally (with --api-lint) run a series of additional checks on the public API in the codebase and flag issues that are discouraged or forbidden by the Android API Council; there are currently around 80 checks. + Some of these take advantage of looking at the source code which wasn't + possible with the signature-file based Python version; for example, it looks + inside method bodies to see if you're synchronizing on this or the current + class, which is forbidden. * Baselines: Metalava can report all of its issues into a "baseline" file, which records the current set of issues. From that point forward, when metalava @@ -288,6 +292,12 @@ Top referenced un-annotated members: android.jar files themselves to ensure that it computes the exact available SDK data for each API level.) +* Misc other features. For example, if you use the @VisibleForTesting annotation + from the support library, where you can express the intended visibility if the + method had not required visibility for testing, then metalava will treat that + method using the intended visibility instead when generating signature files + and stubs. + ## Architecture & Implementation Metalava is implemented on top of IntelliJ parsing APIs (PSI and UAST). However, diff --git a/src/main/java/com/android/tools/metalava/ApiAnalyzer.kt b/src/main/java/com/android/tools/metalava/ApiAnalyzer.kt index 01e173f8bd7d6e1976900add4179ac184054b94b..eed73212e281f3b20eabea505147a54e406f7beb 100644 --- a/src/main/java/com/android/tools/metalava/ApiAnalyzer.kt +++ b/src/main/java/com/android/tools/metalava/ApiAnalyzer.kt @@ -793,7 +793,8 @@ class ApiAnalyzer( if (checkHiddenShowAnnotations && item.hasShowAnnotation() && - !item.documentation.contains("@hide") + !item.documentation.contains("@hide") && + !item.modifiers.hasShowSingleAnnotation() ) { val annotationName = (item.modifiers.annotations().firstOrNull { options.showAnnotations.contains(it.qualifiedName()) @@ -1013,6 +1014,33 @@ class ApiAnalyzer( } } } + + val t = m.returnType() + if (t != null && !t.primitive && !m.deprecated && !cl.deprecated && t.asClass()?.deprecated == true) { + reporter.report( + Errors.REFERENCES_DEPRECATED, m, + "Returning deprecated type $t from ${cl.qualifiedName()}.${m.name()}(): this method should also be deprecated" + ) + } + } + + if (!cl.deprecated) { + val s = cl.superClass() + if (s?.deprecated == true) { + reporter.report( + Errors.EXTENDS_DEPRECATED, cl, + "Extending deprecated super class $s from ${cl.qualifiedName()}: this class should also be deprecated" + ) + } + + for (t in cl.interfaceTypes()) { + if (t.asClass()?.deprecated == true) { + reporter.report( + Errors.EXTENDS_DEPRECATED, cl, + "Implementing interface of deprecated type $t in ${cl.qualifiedName()}: this class should also be deprecated" + ) + } + } } } else if (cl.deprecated) { // not hidden, but deprecated @@ -1022,6 +1050,7 @@ class ApiAnalyzer( // Bring this class back cl.hidden = false cl.removed = false + cl.notStrippable = true } } } @@ -1050,6 +1079,7 @@ class ApiAnalyzer( false )}" ) + cl.notStrippable = true } if (!notStrippable.add(cl)) { diff --git a/src/main/java/com/android/tools/metalava/ApiLint.kt b/src/main/java/com/android/tools/metalava/ApiLint.kt index 465b17c84c84216dee4ed4015bd7c05d0e57c88e..abe0b2db9f773c7ce0288b2e60303d92ad446add 100644 --- a/src/main/java/com/android/tools/metalava/ApiLint.kt +++ b/src/main/java/com/android/tools/metalava/ApiLint.kt @@ -71,6 +71,7 @@ import com.android.tools.metalava.doclava1.Errors.EXCEPTION_NAME import com.android.tools.metalava.doclava1.Errors.EXECUTOR_REGISTRATION import com.android.tools.metalava.doclava1.Errors.EXTENDS_ERROR import com.android.tools.metalava.doclava1.Errors.Error +import com.android.tools.metalava.doclava1.Errors.FORBIDDEN_SUPER_CLASS import com.android.tools.metalava.doclava1.Errors.FRACTION_FLOAT import com.android.tools.metalava.doclava1.Errors.GENERIC_EXCEPTION import com.android.tools.metalava.doclava1.Errors.GETTER_SETTER_NAMES @@ -135,7 +136,19 @@ import com.android.tools.metalava.model.MethodItem import com.android.tools.metalava.model.PackageItem import com.android.tools.metalava.model.ParameterItem import com.android.tools.metalava.model.TypeItem +import com.android.tools.metalava.model.psi.PsiMethodItem import com.android.tools.metalava.model.visitors.ApiVisitor +import com.intellij.psi.JavaRecursiveElementVisitor +import com.intellij.psi.PsiClassObjectAccessExpression +import com.intellij.psi.PsiElement +import com.intellij.psi.PsiSynchronizedStatement +import com.intellij.psi.PsiThisExpression +import org.jetbrains.uast.UCallExpression +import org.jetbrains.uast.UClassLiteralExpression +import org.jetbrains.uast.UMethod +import org.jetbrains.uast.UQualifiedReferenceExpression +import org.jetbrains.uast.UThisExpression +import org.jetbrains.uast.visitor.AbstractUastVisitor import java.util.Locale import java.util.function.Predicate @@ -149,7 +162,7 @@ class ApiLint(private val codebase: Codebase, private val oldCodebase: Codebase? fieldComparator = FieldItem.comparator, ignoreShown = options.showUnannotated ) { - private fun report(id: Error, item: Item, message: String) { + private fun report(id: Error, item: Item, message: String, element: PsiElement? = null) { // Don't flag api warnings on deprecated APIs; these are obviously already known to // be problematic. if (item.deprecated) { @@ -162,7 +175,7 @@ class ApiLint(private val codebase: Codebase, private val oldCodebase: Codebase? return } - reporter.report(id, item, message) + reporter.report(id, item, message, element) } private fun check() { @@ -297,6 +310,7 @@ class ApiLint(private val codebase: Codebase, private val oldCodebase: Codebase? checkUserHandle(cls, methods) checkParams(cls) checkSingleton(cls, methods, constructors) + checkExtends(cls) // TODO: Not yet working // checkOverloadArgs(cls, methods) @@ -976,11 +990,52 @@ class ApiLint(private val codebase: Codebase, private val oldCodebase: Codebase? if "synchronized" in m.split: error(clazz, m, "M5", "Internal locks must not be exposed") */ + + fun reportError(method: MethodItem, psi: PsiElement? = null) { + val message = StringBuilder("Internal locks must not be exposed") + if (psi != null) { + message.append(" (synchronizing on this or class is still externally observable)") + } + message.append(": ") + message.append(method.describe()) + report(VISIBLY_SYNCHRONIZED, method, message.toString(), psi) + } + if (method.modifiers.isSynchronized()) { - report( - VISIBLY_SYNCHRONIZED, method, - "Internal locks must not be exposed: ${method.describe()}" - ) + reportError(method) + } else if (method is PsiMethodItem) { + val psiMethod = method.psiMethod + if (psiMethod is UMethod) { + psiMethod.accept(object : AbstractUastVisitor() { + override fun afterVisitCallExpression(node: UCallExpression) { + super.afterVisitCallExpression(node) + + if (node.methodName == "synchronized" && node.receiver == null) { + val arg = node.valueArguments.firstOrNull() + if (arg is UThisExpression || + arg is UClassLiteralExpression || + arg is UQualifiedReferenceExpression && arg.receiver is UClassLiteralExpression + ) { + reportError(method, arg.sourcePsi ?: node.sourcePsi ?: node.javaPsi) + } + } + } + }) + } else { + psiMethod.body?.accept(object : JavaRecursiveElementVisitor() { + override fun visitSynchronizedStatement(statement: PsiSynchronizedStatement) { + super.visitSynchronizedStatement(statement) + + val lock = statement.lockExpression + if (lock == null || lock is PsiThisExpression || + // locking on any class is visible + lock is PsiClassObjectAccessExpression + ) { + reportError(method, lock ?: statement) + } + } + }) + } } } @@ -1620,7 +1675,7 @@ class ApiLint(private val codebase: Codebase, private val oldCodebase: Codebase? else -> { report( RETHROW_REMOTE_EXCEPTION, method, - "Methods calling into system server should rethrow `RemoteException` as `RuntimeException`" + "Methods calling into system server should rethrow `RemoteException` as `RuntimeException` (but do not list it in the throws clause)" ) } } @@ -2703,6 +2758,13 @@ class ApiLint(private val codebase: Codebase, private val oldCodebase: Codebase? */ + fun flagKotlinOperator(method: MethodItem, message: String) { + report( + KOTLIN_OPERATOR, method, + "$message (this is usually desirable; just make sure it makes sense for this type of object)" + ) + } + for (method in methods) { if (method.modifiers.isStatic()) { continue @@ -2712,27 +2774,24 @@ class ApiLint(private val codebase: Codebase, private val oldCodebase: Codebase? // https://kotlinlang.org/docs/reference/operator-overloading.html#unary-prefix-operators "unaryPlus", "unaryMinus", "not" -> { if (method.parameters().isEmpty()) { - report( - KOTLIN_OPERATOR, method, - "Method can be invoked as a unary operator from Kotlin: `$name`" + flagKotlinOperator( + method, "Method can be invoked as a unary operator from Kotlin: `$name`" ) } } // https://kotlinlang.org/docs/reference/operator-overloading.html#increments-and-decrements "inc", "dec" -> { if (method.parameters().isEmpty() && method.returnType()?.toTypeString() != "void") { - report( - KOTLIN_OPERATOR, method, - "Method can be invoked as a pre/postfix inc/decrement operator from Kotlin: `$name`" + flagKotlinOperator( + method, "Method can be invoked as a pre/postfix inc/decrement operator from Kotlin: `$name`" ) } } // https://kotlinlang.org/docs/reference/operator-overloading.html#arithmetic "plus", "minus", "times", "div", "rem", "mod", "rangeTo" -> { if (method.parameters().size == 1) { - report( - KOTLIN_OPERATOR, method, - "Method can be invoked as a binary operator from Kotlin: `$name`" + flagKotlinOperator( + method, "Method can be invoked as a binary operator from Kotlin: `$name`" ) } val assignName = name + "Assign" @@ -2751,45 +2810,40 @@ class ApiLint(private val codebase: Codebase, private val oldCodebase: Codebase? // https://kotlinlang.org/docs/reference/operator-overloading.html#in "contains" -> { if (method.parameters().size == 1 && method.returnType()?.toTypeString() == "boolean") { - report( - KOTLIN_OPERATOR, method, - "Method can be invoked as a \"in\" operator from Kotlin: `$name`" + flagKotlinOperator( + method, "Method can be invoked as a \"in\" operator from Kotlin: `$name`" ) } } // https://kotlinlang.org/docs/reference/operator-overloading.html#indexed "get" -> { if (method.parameters().isNotEmpty()) { - report( - KOTLIN_OPERATOR, method, - "Method can be invoked with an indexing operator from Kotlin: `$name`" + flagKotlinOperator( + method, "Method can be invoked with an indexing operator from Kotlin: `$name`" ) } } // https://kotlinlang.org/docs/reference/operator-overloading.html#indexed "set" -> { if (method.parameters().size > 1) { - report( - KOTLIN_OPERATOR, method, - "Method can be invoked with an indexing operator from Kotlin: `$name`" + flagKotlinOperator( + method, "Method can be invoked with an indexing operator from Kotlin: `$name`" ) } } // https://kotlinlang.org/docs/reference/operator-overloading.html#invoke "invoke" -> { if (method.parameters().size > 1) { - report( - KOTLIN_OPERATOR, method, - "Method can be invoked with function call syntax from Kotlin: `$name`" + flagKotlinOperator( + method, "Method can be invoked with function call syntax from Kotlin: `$name`" ) } } // https://kotlinlang.org/docs/reference/operator-overloading.html#assignments "plusAssign", "minusAssign", "timesAssign", "divAssign", "remAssign", "modAssign" -> { if (method.parameters().size == 1 && method.returnType()?.toTypeString() == "void") { - report( - KOTLIN_OPERATOR, method, - "Method can be invoked as a compound assignment operator from Kotlin: `$name`" + flagKotlinOperator( + method, "Method can be invoked as a compound assignment operator from Kotlin: `$name`" ) } } @@ -3180,6 +3234,23 @@ class ApiLint(private val codebase: Codebase, private val oldCodebase: Codebase? } } + private fun checkExtends(cls: ClassItem) { + // Call cls.superClass().extends() instead of cls.extends() since extends returns true for self + val superCls = cls.superClass() ?: return + if (superCls.extends("android.os.AsyncTask")) { + report( + FORBIDDEN_SUPER_CLASS, cls, + "${cls.simpleName()} should not extend `AsyncTask`. AsyncTask is an implementation detail. Expose a listener or, in androidx, a `ListenableFuture` API instead" + ) + } + if (superCls.extends("android.app.Activity")) { + report( + FORBIDDEN_SUPER_CLASS, cls, + "${cls.simpleName()} should not extend `Activity`. Activity subclasses are impossible to compose. Expose a composable API instead." + ) + } + } + /** * Checks whether the given full package name is the same as the given root * package or a sub package (if we just did full.startsWith("java"), then @@ -3231,6 +3302,7 @@ class ApiLint(private val codebase: Codebase, private val oldCodebase: Codebase? } companion object { + private val badParameterClassNames = listOf( "Param", "Parameter", "Parameters", "Args", "Arg", "Argument", "Arguments", "Options", "Bundle" ) diff --git a/src/main/java/com/android/tools/metalava/ApiType.kt b/src/main/java/com/android/tools/metalava/ApiType.kt index f712f0e97e0ba6be398581e8379d78c74c413222..7ca3694ccf0231795eb5b58b581edbf55a52b2af 100644 --- a/src/main/java/com/android/tools/metalava/ApiType.kt +++ b/src/main/java/com/android/tools/metalava/ApiType.kt @@ -16,9 +16,11 @@ package com.android.tools.metalava +import com.android.SdkConstants.DOT_TXT import com.android.tools.metalava.doclava1.ApiPredicate import com.android.tools.metalava.doclava1.ElidingPredicate import com.android.tools.metalava.doclava1.FilterPredicate +import com.android.tools.metalava.model.Codebase import com.android.tools.metalava.model.Item import java.io.File import java.util.function.Predicate @@ -99,4 +101,24 @@ enum class ApiType(val flagName: String, val displayName: String = flagName) { abstract fun getReferenceFilter(): Predicate<Item> override fun toString(): String = displayName + + /** + * Gets the signature file for the given API type. Will create it if not already + * created. + */ + fun getSignatureFile(codebase: Codebase, defaultName: String): File { + val apiType = this + return apiType.getOptionFile() ?: run { + val tempFile = createTempFile(defaultName, DOT_TXT) + tempFile.deleteOnExit() + val apiEmit = apiType.getEmitFilter() + val apiReference = apiType.getReferenceFilter() + + createReportFile(codebase, tempFile, null) { printWriter -> + SignatureWriter(printWriter, apiEmit, apiReference, codebase.preFiltered) + } + + tempFile + } + } } diff --git a/src/main/java/com/android/tools/metalava/Baseline.kt b/src/main/java/com/android/tools/metalava/Baseline.kt index 926e0871154db748ee6ce01817609669d09e1403..335cee4abfa1c381ef11604c0ab5b20e9ed537f4 100644 --- a/src/main/java/com/android/tools/metalava/Baseline.kt +++ b/src/main/java/com/android/tools/metalava/Baseline.kt @@ -23,6 +23,7 @@ import com.android.tools.metalava.model.Item import com.android.tools.metalava.model.MethodItem import com.android.tools.metalava.model.PackageItem import com.android.tools.metalava.model.ParameterItem +import com.android.tools.metalava.model.configuration import com.intellij.openapi.vfs.VfsUtilCore import com.intellij.psi.PsiClass import com.intellij.psi.PsiElement @@ -40,6 +41,7 @@ const val DEFAULT_BASELINE_NAME = "baseline.txt" class Baseline( val file: File?, var updateFile: File?, + var merge: Boolean = false, private var headerComment: String = "", /** * Whether, when updating the baseline, we should fail the build if the main baseline does not @@ -53,7 +55,7 @@ class Baseline( private val map = HashMap<Errors.Error, MutableMap<String, String>>() init { - if (file?.isFile == true && !silentUpdate) { + if (file?.isFile == true && (!silentUpdate || merge)) { // We've set a baseline for a nonexistent file: read it read() } @@ -80,6 +82,9 @@ class Baseline( private fun mark(elementId: String, @Suppress("UNUSED_PARAMETER") message: String, error: Errors.Error): Boolean { val idMap: MutableMap<String, String>? = map[error] ?: run { if (updateFile != null) { + if (options.baselineErrorsOnly && configuration.getSeverity(error) != Severity.ERROR) { + return true + } val new = HashMap<String, String>() map[error] = new new @@ -176,33 +181,37 @@ class Baseline( private fun read() { val file = this.file ?: return - file.readLines(Charsets.UTF_8).forEach { line -> - if (!(line.startsWith("//") || line.startsWith("#") || line.isBlank() || line.startsWith(" "))) { - val idEnd = line.indexOf(':') - val elementEnd = line.indexOf(':', idEnd + 1) - if (idEnd == -1 || elementEnd == -1) { - println("Invalid metalava baseline format: $line") - } - val errorId = line.substring(0, idEnd).trim() - val elementId = line.substring(idEnd + 2, elementEnd).trim() + val lines = file.readLines(Charsets.UTF_8) + for (i in 0 until lines.size - 1) { + val line = lines[i] + if (line.startsWith("//") || + line.startsWith("#") || + line.isBlank() || + line.startsWith(" ")) { + continue + } + val idEnd = line.indexOf(':') + val elementEnd = line.indexOf(':', idEnd + 1) + if (idEnd == -1 || elementEnd == -1) { + println("Invalid metalava baseline format: $line") + } + val errorId = line.substring(0, idEnd).trim() + val elementId = line.substring(idEnd + 2, elementEnd).trim() - // For now we don't need the actual messages since we're only matching by - // issue id and API location, so don't bother reading. (These are listed - // on separate, indented, lines, so to read them we'd need to alternate - // line readers.) - val message = "" + // Unless merging, we don't need the actual messages since we're only matching by + // issue id and API location, so don't bother computing. + val message = if (merge) lines[i + 1].trim() else "" - val error = Errors.findErrorById(errorId) - if (error == null) { - println("Invalid metalava baseline file: unknown error id '$errorId'") - } else { - val newIdMap = map[error] ?: run { - val new = HashMap<String, String>() - map[error] = new - new - } - newIdMap[elementId] = message + val error = Errors.findErrorById(errorId) + if (error == null) { + println("Invalid metalava baseline file: unknown error id '$errorId'") + } else { + val newIdMap = map[error] ?: run { + val new = HashMap<String, String>() + map[error] = new + new } + newIdMap[elementId] = message } } } diff --git a/src/main/java/com/android/tools/metalava/CompatibilityCheck.kt b/src/main/java/com/android/tools/metalava/CompatibilityCheck.kt index 22d359fa78afcef8888bc98ffff86c8966061eb2..78aebe16ff64d43041b399eaee7c5cb14bc0392d 100644 --- a/src/main/java/com/android/tools/metalava/CompatibilityCheck.kt +++ b/src/main/java/com/android/tools/metalava/CompatibilityCheck.kt @@ -631,6 +631,13 @@ class CompatibilityCheck( } private fun handleAdded(error: Error, item: Item) { + if (item.originallyHidden) { + // This is an element which is hidden but is referenced from + // some public API. This is an error, but some existing code + // is doing this. This is not an API addition. + return + } + var message = "Added ${describe(item)}" // Clarify error message for removed API to make it less ambiguous diff --git a/src/main/java/com/android/tools/metalava/Diff.kt b/src/main/java/com/android/tools/metalava/Diff.kt new file mode 100644 index 0000000000000000000000000000000000000000..cd7f2e4bae7d3db74b2943cdd1d10bbc2f9f08b2 --- /dev/null +++ b/src/main/java/com/android/tools/metalava/Diff.kt @@ -0,0 +1,168 @@ +/* + * Copyright (C) 2019 The Android Open Source Project + * + * Licensed under the Apache License, Version 2.0 (the "License"); + * you may not use this file except in compliance with the License. + * You may obtain a copy of the License at + * + * http://www.apache.org/licenses/LICENSE-2.0 + * + * Unless required by applicable law or agreed to in writing, software + * distributed under the License is distributed on an "AS IS" BASIS, + * WITHOUT WARRANTIES OR CONDITIONS OF ANY KIND, either express or implied. + * See the License for the specific language governing permissions and + * limitations under the License. + */ + +package com.android.tools.metalava + +import com.android.ide.common.process.CachedProcessOutputHandler +import com.android.ide.common.process.DefaultProcessExecutor +import com.android.ide.common.process.ProcessInfoBuilder +import com.android.utils.LineCollector +import com.android.utils.StdLogger +import java.io.File + +// Copied from lint's test suite: TestUtils.diff in tools/base with +// some changes to allow running native diff. + +/** Returns the universal diff for the two given files, or null if not supported or working */ +fun getNativeDiff(before: File, after: File): String? { + if (options.noNativeDiff) { + return null + } + + // A lot faster for big files: + val native = File("/usr/bin/diff") + if (native.isFile) { + try { + val builder = ProcessInfoBuilder() + builder.setExecutable(native) + builder.addArgs("-u", before.path, after.path) + val processOutputHandler = CachedProcessOutputHandler() + DefaultProcessExecutor(StdLogger(StdLogger.Level.ERROR)) + .execute(builder.createProcess(), processOutputHandler) + .rethrowFailure() + + val output = processOutputHandler.processOutput + val lineCollector = LineCollector() + output.processStandardOutputLines(lineCollector) + + return lineCollector.result.joinToString(separator = "\n") + } catch (ignore: Throwable) { + } + } + + return null +} + +fun getDiff(before: String, after: String, windowSize: Int): String { + return getDiff( + before.split("\n".toRegex()).dropLastWhile { it.isEmpty() }.toTypedArray(), + after.split("\n".toRegex()).dropLastWhile { it.isEmpty() }.toTypedArray(), + windowSize + ) +} + +fun getDiff( + before: Array<String>, + after: Array<String>, + windowSize: Int +): String { + // Based on the LCS section in http://introcs.cs.princeton.edu/java/96optimization/ + val sb = StringBuilder() + val n = before.size + val m = after.size + + // Compute longest common subsequence of x[i..m] and y[j..n] bottom up + val lcs = Array(n + 1) { IntArray(m + 1) } + for (i in n - 1 downTo 0) { + for (j in m - 1 downTo 0) { + if (before[i] == after[j]) { + lcs[i][j] = lcs[i + 1][j + 1] + 1 + } else { + lcs[i][j] = Math.max(lcs[i + 1][j], lcs[i][j + 1]) + } + } + } + + var i = 0 + var j = 0 + while (i < n && j < m) { + if (before[i] == after[j]) { + i++ + j++ + } else { + sb.append("@@ -") + sb.append(Integer.toString(i + 1)) + sb.append(" +") + sb.append(Integer.toString(j + 1)) + sb.append('\n') + + if (windowSize > 0) { + for (context in Math.max(0, i - windowSize) until i) { + sb.append(" ") + sb.append(before[context]) + sb.append("\n") + } + } + + while (i < n && j < m && before[i] != after[j]) { + if (lcs[i + 1][j] >= lcs[i][j + 1]) { + sb.append('-') + if (!before[i].trim { it <= ' ' }.isEmpty()) { + sb.append(' ') + } + sb.append(before[i]) + sb.append('\n') + i++ + } else { + sb.append('+') + if (!after[j].trim { it <= ' ' }.isEmpty()) { + sb.append(' ') + } + sb.append(after[j]) + sb.append('\n') + j++ + } + } + + if (windowSize > 0) { + for (context in i until Math.min(n, i + windowSize)) { + sb.append(" ") + sb.append(before[context]) + sb.append("\n") + } + } + } + } + + if (i < n || j < m) { + assert(i == n || j == m) + sb.append("@@ -") + sb.append(Integer.toString(i + 1)) + sb.append(" +") + sb.append(Integer.toString(j + 1)) + sb.append('\n') + while (i < n) { + sb.append('-') + if (!before[i].trim { it <= ' ' }.isEmpty()) { + sb.append(' ') + } + sb.append(before[i]) + sb.append('\n') + i++ + } + while (j < m) { + sb.append('+') + if (!after[j].trim { it <= ' ' }.isEmpty()) { + sb.append(' ') + } + sb.append(after[j]) + sb.append('\n') + j++ + } + } + + return sb.toString() +} diff --git a/src/main/java/com/android/tools/metalava/DocAnalyzer.kt b/src/main/java/com/android/tools/metalava/DocAnalyzer.kt index 38b5ead3a7156e1310714561e1cd63c8d7521cf9..44eed3b410b3c0c72b61d468d2f5bfad8846b55b 100644 --- a/src/main/java/com/android/tools/metalava/DocAnalyzer.kt +++ b/src/main/java/com/android/tools/metalava/DocAnalyzer.kt @@ -624,7 +624,7 @@ class DocAnalyzer( } override fun getCacheDir(name: String?, create: Boolean): File? { - if (create && java.lang.Boolean.getBoolean(ENV_VAR_METALAVA_TESTS_RUNNING)) { + if (create && isUnderTest()) { // Pick unique directory during unit tests return Files.createTempDir() } diff --git a/src/main/java/com/android/tools/metalava/Driver.kt b/src/main/java/com/android/tools/metalava/Driver.kt index 6c57746f2710b7323abfdc7dd8eb1d3d27b65c0c..400c139b48cbaaccce9d7590f0b54c1546039bac 100644 --- a/src/main/java/com/android/tools/metalava/Driver.kt +++ b/src/main/java/com/android/tools/metalava/Driver.kt @@ -20,7 +20,6 @@ package com.android.tools.metalava import com.android.SdkConstants import com.android.SdkConstants.DOT_JAVA import com.android.SdkConstants.DOT_KT -import com.android.SdkConstants.DOT_TXT import com.android.ide.common.process.CachedProcessOutputHandler import com.android.ide.common.process.DefaultProcessExecutor import com.android.ide.common.process.ProcessInfoBuilder @@ -92,8 +91,7 @@ fun run( setExitCode: Boolean = false ): Boolean { - if (System.getenv(ENV_VAR_METALAVA_DUMP_ARGV) != null && - !java.lang.Boolean.getBoolean(ENV_VAR_METALAVA_TESTS_RUNNING) + if (System.getenv(ENV_VAR_METALAVA_DUMP_ARGV) != null && !isUnderTest() ) { stdout.println("---Running $PROGRAM_NAME----") stdout.println("pwd=${File("").absolutePath}") @@ -131,6 +129,7 @@ fun run( stdout.println("\"$arg\",") } stdout.println("----------------------------") + stdout.flush() } newArgs } @@ -145,6 +144,8 @@ fun run( } exitValue = true } catch (e: DriverException) { + stdout.flush() + stderr.flush() if (e.stderr.isNotBlank()) { stderr.println("\n${e.stderr}") } @@ -166,7 +167,7 @@ fun run( stdout.flush() stderr.flush() - if (setExitCode && reporter.hasErrors()) { + if (setExitCode) { exit(exitCode) } @@ -578,18 +579,7 @@ fun checkCompatibility( kotlinStyleNulls = options.inputKotlinStyleNulls ) } else if (options.showAnnotations.isNotEmpty() || apiType != ApiType.PUBLIC_API) { - val apiFile = apiType.getOptionFile() ?: run { - val tempFile = createTempFile("compat-check-signatures-$apiType", DOT_TXT) - tempFile.deleteOnExit() - val apiEmit = apiType.getEmitFilter() - val apiReference = apiType.getReferenceFilter() - - createReportFile(codebase, tempFile, null) { printWriter -> - SignatureWriter(printWriter, apiEmit, apiReference, codebase.preFiltered) - } - - tempFile - } + val apiFile = apiType.getSignatureFile(codebase, "compat-check-signatures-$apiType") // Fast path: if the signature files are identical, we're already good! if (apiFile.readText(UTF_8) == signatureFile.readText(UTF_8)) { @@ -615,6 +605,41 @@ fun checkCompatibility( // If configured, compares the new API with the previous API and reports // any incompatibilities. CompatibilityCheck.checkCompatibility(new, current, releaseType, apiType, base) + + // Make sure the text files are identical too? (only applies for *current.txt; + // last-released is expected to differ) + if (releaseType == ReleaseType.DEV && !options.allowCompatibleDifferences) { + val apiFile = if (new.location.isFile) + new.location + else + apiType.getSignatureFile(codebase, "compat-diff-signatures-$apiType") + + fun getCanonicalSignatures(file: File): String { + // Get rid of trailing newlines and Windows line endings + val text = file.readText(UTF_8) + return text.replace("\r\n", "\n").trim() + } + val currentTxt = getCanonicalSignatures(signatureFile) + val newTxt = getCanonicalSignatures(apiFile) + if (newTxt != currentTxt) { + val diff = getNativeDiff(signatureFile, apiFile) ?: getDiff(currentTxt, newTxt, 1) + val updateApi = if (isBuildingAndroid()) + "Run make update-api to update.\n" + else + "" + val message = + """ + Aborting: Your changes have resulted in differences in the signature file + for the ${apiType.displayName} API. + + The changes may be compatible, but the signature file needs to be updated. + $updateApi + Diffs: + """.trimIndent() + "\n" + diff + + throw DriverException(exitCode = -1, stderr = message) + } + } } fun createTempFile(namePrefix: String, nameSuffix: String): File { @@ -877,7 +902,7 @@ private fun createProjectEnvironment(): LintCoreProjectEnvironment { if (!assertionsEnabled() && System.getenv(ENV_VAR_METALAVA_DUMP_ARGV) == null && - !java.lang.Boolean.getBoolean(ENV_VAR_METALAVA_TESTS_RUNNING) + !isUnderTest() ) { DefaultLogger.disableStderrDumping(parentDisposable) } @@ -913,6 +938,11 @@ private fun createStubFiles(stubDir: File, codebase: Codebase, docStubs: Boolean // Generating stubs from a sig-file-based codebase is problematic assert(codebase.supportsDocumentation()) + // Temporary bug workaround for org.chromium.arc + if (options.sourcePath.firstOrNull()?.path?.endsWith("org.chromium.arc") == true) { + codebase.findClass("org.chromium.mojo.bindings.Callbacks")?.hidden = true + } + if (docStubs) { progress("\nGenerating documentation stub files: ") } else { @@ -1204,3 +1234,9 @@ fun findPackage(file: File): String? { fun findPackage(source: String): String? { return ClassName(source).packageName } + +/** Whether metalava is running unit tests */ +fun isUnderTest() = java.lang.Boolean.getBoolean(ENV_VAR_METALAVA_TESTS_RUNNING) + +/** Whether metalava is being invoked as part of an Android platform build */ +fun isBuildingAndroid() = System.getenv("ANDROID_BUILD_TOP") != null && !isUnderTest() diff --git a/src/main/java/com/android/tools/metalava/JDiffXmlWriter.kt b/src/main/java/com/android/tools/metalava/JDiffXmlWriter.kt index d4dbe0b4eb3a935c543a8ab6b8a3c7134aeb64b1..8087c3f06d58716079559ae762a6dc60afb5b59f 100644 --- a/src/main/java/com/android/tools/metalava/JDiffXmlWriter.kt +++ b/src/main/java/com/android/tools/metalava/JDiffXmlWriter.kt @@ -201,12 +201,10 @@ class JDiffXmlWriter( } } else null - val fullTypeName = escapeAttributeValue(field.type().toTypeString()) - writer.print("<field name=\"") writer.print(field.name()) writer.print("\"\n type=\"") - writer.print(fullTypeName) + writer.print(escapeAttributeValue(formatType(field.type()))) writer.print("\"\n transient=\"") writer.print(modifiers.isTransient()) writer.print("\"\n volatile=\"") @@ -295,9 +293,11 @@ class JDiffXmlWriter( return } escapeAttributeValue( - superClass.toTypeString( - erased = compatibility.omitTypeParametersInInterfaces, - context = superClass.asClass() + formatType( + superClass.toTypeString( + erased = compatibility.omitTypeParametersInInterfaces, + context = superClass.asClass() + ) ) ) } @@ -325,8 +325,7 @@ class JDiffXmlWriter( interfaces.sortedWith(TypeItem.comparator).forEach { item -> writer.print("<implements name=\"") val type = item.toTypeString(erased = compatibility.omitTypeParametersInInterfaces, context = cls) - val escapedType = escapeAttributeValue(type) - writer.print(escapedType) + writer.print(escapeAttributeValue(formatType(type))) writer.println("\">\n</implements>") } } @@ -343,13 +342,12 @@ class JDiffXmlWriter( } } - private fun formatType(type: TypeItem): String { - val typeString = type.toTypeString() - return if (compatibility.spaceAfterCommaInTypes) { - typeString.replace(",", ", ").replace(", ", ", ") - } else { - typeString - } + private fun formatType(type: TypeItem): String = formatType(type.toTypeString()) + + private fun formatType(typeString: String): String { + // In JDiff we always want to include spaces after commas; the API signature tests depend + // on this. + return typeString.replace(",", ", ").replace(", ", ", ") } private fun writeThrowsList(method: MethodItem) { diff --git a/src/main/java/com/android/tools/metalava/Options.kt b/src/main/java/com/android/tools/metalava/Options.kt index b99e667a66bda753fca05a687c75dcef1c97dcf6..33a26da263eca01836e683ffc9d7a7fb77963d92 100644 --- a/src/main/java/com/android/tools/metalava/Options.kt +++ b/src/main/java/com/android/tools/metalava/Options.kt @@ -86,6 +86,8 @@ const val ARG_CHECK_COMPATIBILITY_API_CURRENT = "--check-compatibility:api:curre const val ARG_CHECK_COMPATIBILITY_API_RELEASED = "--check-compatibility:api:released" const val ARG_CHECK_COMPATIBILITY_REMOVED_CURRENT = "--check-compatibility:removed:current" const val ARG_CHECK_COMPATIBILITY_REMOVED_RELEASED = "--check-compatibility:removed:released" +const val ARG_ALLOW_COMPATIBLE_DIFFERENCES = "--allow-compatible-differences" +const val ARG_NO_NATIVE_DIFF = "--no-native-diff" const val ARG_INPUT_KOTLIN_NULLS = "--input-kotlin-nulls" const val ARG_OUTPUT_KOTLIN_NULLS = "--output-kotlin-nulls" const val ARG_OUTPUT_DEFAULT_VALUES = "--output-default-values" @@ -138,6 +140,9 @@ const val ARG_DEX_API_MAPPING = "--dex-api-mapping" const val ARG_GENERATE_DOCUMENTATION = "--generate-documentation" const val ARG_BASELINE = "--baseline" const val ARG_UPDATE_BASELINE = "--update-baseline" +const val ARG_MERGE_BASELINE = "--merge-baseline" +const val ARG_STUB_PACKAGES = "--stub-packages" +const val ARG_STUB_IMPORT_PACKAGES = "--stub-import-packages" class Options( args: Array<String>, @@ -417,6 +422,19 @@ class Options( /** The list of compatibility checks to run */ val compatibilityChecks: List<CheckRequest> = mutableCompatibilityChecks + /** + * When checking signature files, whether compatible differences in signature + * files are allowed. This is normally not allowed (since it means the next + * engineer adding an incompatible change will suddenly see the cumulative + * differences show up in their diffs when checking in signature files), + * but is useful from the test suite etc. Controlled by + * [ARG_ALLOW_COMPATIBLE_DIFFERENCES]. + */ + var allowCompatibleDifferences = false + + /** If false, attempt to use the native diff utility on the system */ + var noNativeDiff = false + /** Existing external annotation files to merge in */ var mergeQualifierAnnotations: List<File> = mutableMergeQualifierAnnotations var mergeInclusionAnnotations: List<File> = mutableMergeInclusionAnnotations @@ -484,6 +502,9 @@ class Options( /** Whether all baseline files need to be updated */ var updateBaseline = false + /** Whether the baseline should only contain errors */ + var baselineErrorsOnly = false + /** * Whether to omit locations for warnings and errors. This is not a flag exposed to users * or listed in help; this is intended for the unit test suite, used for example for the @@ -545,6 +566,8 @@ class Options( var currentJar: File? = null var updateBaselineFile: File? = null var baselineFile: File? = null + var mergeBaseline = false + var delayedCheckApiFiles = false var index = 0 while (index < args.size) { @@ -570,8 +593,10 @@ class Options( ARG_COMPAT_OUTPUT -> compatOutput = true // For now we don't distinguish between bootclasspath and classpath - ARG_CLASS_PATH, "-classpath", "-bootclasspath" -> - mutableClassPath.addAll(stringToExistingDirsOrJars(getValue(args, ++index))) + ARG_CLASS_PATH, "-classpath", "-bootclasspath" -> { + val path = getValue(args, ++index) + mutableClassPath.addAll(stringToExistingDirsOrJars(path)) + } ARG_SOURCE_PATH, "--sources", "--sourcepath", "-sourcepath" -> { val path = getValue(args, ++index) @@ -684,17 +709,17 @@ class Options( ARG_HIDE_PACKAGE, "-hidePackage" -> mutableHidePackages.add(getValue(args, ++index)) - "--stub-packages", "-stubpackages" -> { + ARG_STUB_PACKAGES, "-stubpackages" -> { val packages = getValue(args, ++index) val filter = stubPackages ?: run { val newFilter = PackageFilter() stubPackages = newFilter newFilter } - filter.packagePrefixes += packages.split(File.pathSeparatorChar) + filter.addPackages(packages) } - "--stub-import-packages", "-stubimportpackages" -> { + ARG_STUB_IMPORT_PACKAGES, "-stubimportpackages" -> { val packages = getValue(args, ++index) for (pkg in packages.split(File.pathSeparatorChar)) { mutableStubImportPackages.add(pkg) @@ -713,8 +738,9 @@ class Options( baselineFile = stringToExistingFile(relative) } - ARG_UPDATE_BASELINE -> { + ARG_UPDATE_BASELINE, ARG_MERGE_BASELINE -> { updateBaseline = true + mergeBaseline = arg == ARG_MERGE_BASELINE if (index < args.size - 1) { val nextArg = args[index + 1] if (!nextArg.startsWith("-")) { @@ -816,28 +842,38 @@ class Options( mutableCompatibilityChecks.add(CheckRequest(file, ApiType.REMOVED, ReleaseType.RELEASED)) } + ARG_ALLOW_COMPATIBLE_DIFFERENCES -> allowCompatibleDifferences = true + ARG_NO_NATIVE_DIFF -> noNativeDiff = true + // Compat flag for the old API check command, invoked from build/make/core/definitions.mk: "--check-api-files" -> { - val stableApiFile = stringToExistingFile(getValue(args, ++index)) - val apiFileToBeTested = stringToExistingFile(getValue(args, ++index)) - val stableRemovedApiFile = stringToExistingFile(getValue(args, ++index)) - val removedApiFileToBeTested = stringToExistingFile(getValue(args, ++index)) - mutableCompatibilityChecks.add( - CheckRequest( - stableApiFile, - ApiType.PUBLIC_API, - ReleaseType.RELEASED, - apiFileToBeTested + if (index < args.size - 1 && args[index + 1].startsWith("-")) { + // Work around bug where --check-api-files is invoked with all + // the other metalava args before the 4 files; this will be + // fixed by https://android-review.googlesource.com/c/platform/build/+/874473 + delayedCheckApiFiles = true + } else { + val stableApiFile = stringToExistingFile(getValue(args, ++index)) + val apiFileToBeTested = stringToExistingFile(getValue(args, ++index)) + val stableRemovedApiFile = stringToExistingFile(getValue(args, ++index)) + val removedApiFileToBeTested = stringToExistingFile(getValue(args, ++index)) + mutableCompatibilityChecks.add( + CheckRequest( + stableApiFile, + ApiType.PUBLIC_API, + ReleaseType.RELEASED, + apiFileToBeTested + ) ) - ) - mutableCompatibilityChecks.add( - CheckRequest( - stableRemovedApiFile, - ApiType.REMOVED, - ReleaseType.RELEASED, - removedApiFileToBeTested + mutableCompatibilityChecks.add( + CheckRequest( + stableRemovedApiFile, + ApiType.REMOVED, + ReleaseType.RELEASED, + removedApiFileToBeTested + ) ) - ) + } } ARG_ANNOTATION_COVERAGE_STATS -> dumpAnnotationStatistics = true @@ -1232,8 +1268,32 @@ class Options( throw DriverException(stderr = "Invalid argument $arg\n\n$usage") } } else { - // All args that don't start with "-" are taken to be filenames - mutableSources.addAll(stringToExistingFiles(arg)) + if (delayedCheckApiFiles) { + delayedCheckApiFiles = false + val stableApiFile = stringToExistingFile(arg) + val apiFileToBeTested = stringToExistingFile(getValue(args, ++index)) + val stableRemovedApiFile = stringToExistingFile(getValue(args, ++index)) + val removedApiFileToBeTested = stringToExistingFile(getValue(args, ++index)) + mutableCompatibilityChecks.add( + CheckRequest( + stableApiFile, + ApiType.PUBLIC_API, + ReleaseType.RELEASED, + apiFileToBeTested + ) + ) + mutableCompatibilityChecks.add( + CheckRequest( + stableRemovedApiFile, + ApiType.REMOVED, + ReleaseType.RELEASED, + removedApiFileToBeTested + ) + ) + } else { + // All args that don't start with "-" are taken to be filenames + mutableSources.addAll(stringToExistingFiles(arg)) + } } } } @@ -1287,18 +1347,19 @@ class Options( } if (baselineFile == null) { - val defaultBaseline = getDefaultBaselineFile() - if (defaultBaseline != null && defaultBaseline.isFile) { - baseline = Baseline(defaultBaseline, updateBaselineFile) + val defaultBaselineFile = getDefaultBaselineFile() + if (defaultBaselineFile != null && defaultBaselineFile.isFile) { + baseline = Baseline(defaultBaselineFile, updateBaselineFile, mergeBaseline) } else if (updateBaselineFile != null) { - baseline = Baseline(null, updateBaselineFile) + baseline = Baseline(null, updateBaselineFile, mergeBaseline) } } else { - val headerComment = if (baselineFile.path.contains("frameworks/base/")) + // Add helpful doc in AOSP baseline files? + val headerComment = if (isBuildingAndroid()) "// See tools/metalava/API-LINT.md for how to update this file.\n\n" else "" - baseline = Baseline(baselineFile, updateBaselineFile, headerComment) + baseline = Baseline(baselineFile, updateBaselineFile, mergeBaseline, headerComment) } checkFlagConsistency() @@ -1324,16 +1385,22 @@ class Options( * etc. */ private fun getDefaultBaselineFile(): File? { - if (sourcePath.isNotEmpty() && !sourcePath[0].path.isBlank()) { + if (sourcePath.isNotEmpty() && sourcePath[0].path.isNotBlank()) { fun annotationToPrefix(qualifiedName: String): String { val name = qualifiedName.substring(qualifiedName.lastIndexOf('.') + 1) return name.toLowerCase(Locale.US).removeSuffix("api") + "-" } val sb = StringBuilder() - showAnnotations.forEach { sb.append(annotationToPrefix(it)).append('-') } - showSingleAnnotations.forEach { sb.append(annotationToPrefix(it)).append('-') } + showAnnotations.forEach { sb.append(annotationToPrefix(it)) } sb.append(DEFAULT_BASELINE_NAME) - return File(sourcePath[0], sb.toString()) + var base = sourcePath[0] + // Convention: in AOSP, signature files are often in sourcepath/api: let's place baseline + // files there too + val api = File(base, "api") + if (api.isDirectory) { + base = api + } + return File(base, sb.toString()) } else { return null } @@ -1726,6 +1793,11 @@ class Options( "as hidden", ARG_SHOW_UNANNOTATED, "Include un-annotated public APIs in the signature file as well", "$ARG_JAVA_SOURCE <level>", "Sets the source level for Java source files; default is 1.8.", + "$ARG_STUB_PACKAGES <path>", "List of packages (separated by ${File.pathSeparator} which will be " + + "used to filter out irrelevant code. If specified, only code in these packages will be " + + "included in signature files, stubs, etc. (This is not limited to just the stubs; the name " + + "is historical.) You can also use \".*\" at the end to match subpackages, so `foo.*` will " + + "match both `foo` and `foo.bar`.", "", "\nDocumentation:", ARG_PUBLIC, "Only include elements that are public", @@ -1810,6 +1882,10 @@ class Options( "If some warnings have been fixed, this will delete them from the baseline files. If a file " + "is provided, the updated baseline is written to the given file; otherwise the original source " + "baseline file is updated.", + "$ARG_MERGE_BASELINE [file]", "Like $ARG_UPDATE_BASELINE, but instead of always replacing entries " + + "in the baseline, it will merge the existing baseline with the new baseline. This is useful " + + "if $PROGRAM_NAME runs multiple times on the same source tree with different flags at different " + + "times, such as occasionally with $ARG_API_LINT.", "", "\nJDiff:", "$ARG_XML_API <file>", "Like $ARG_API, but emits the API in the JDiff XML format instead", diff --git a/src/main/java/com/android/tools/metalava/PackageFilter.kt b/src/main/java/com/android/tools/metalava/PackageFilter.kt index 089265dd6654b0958e38a1e9cba1a0a4cfcc6718..1e2ee35087caee96f9b5953e6cda4a8958a1ffb0 100644 --- a/src/main/java/com/android/tools/metalava/PackageFilter.kt +++ b/src/main/java/com/android/tools/metalava/PackageFilter.kt @@ -1,17 +1,46 @@ package com.android.tools.metalava import com.android.tools.metalava.model.PackageItem +import java.io.File +/** + * + * We see a number of different styles: + * - exact match (foo) + * - prefix match (foo*, probably not intentional) + * - subpackage match (foo.*) + * - package and subpackage match (foo:foo.*) + * + * Real examples: + * args: "-stubpackages com.android.test.power ", + * args: "-stubpackages android.car* ", + * args: "-stubpackages com.android.ahat:com.android.ahat.*", + * + * Note that doclava does *not* include subpackages by default: -stubpackage foo + * will match only foo, not foo.bar. Note also that "foo.*" will not match "foo", + * so doclava required you to supply both: "foo:foo.*". + * + * In metalava we've changed that: it's not likely that you want to + * match any subpackage of foo but not foo itself, so foo.* is taken + * to mean "foo" and "foo.*". + */ class PackageFilter( - val packagePrefixes: MutableList<String> = mutableListOf() + private val exactPackages: MutableSet<String> = mutableSetOf(), + private val packagePrefixes: MutableList<String> = mutableListOf() ) { + init { + // Wildcards should have been removed by option + assert(packagePrefixes.none { it.contains("*") }) + } fun matches(qualifiedName: String): Boolean { - for (prefix in packagePrefixes) { - if (qualifiedName.startsWith(prefix)) { - if (qualifiedName == prefix) { - return true - } else if (qualifiedName[prefix.length] == '.') { + if (exactPackages.contains(qualifiedName)) { + return true + } + + if (packagePrefixes.isNotEmpty()) { + for (prefix in packagePrefixes) { + if (qualifiedName.startsWith(prefix)) { return true } } @@ -20,6 +49,25 @@ class PackageFilter( return false } + fun addPackages(path: String) { + for (pkg in path.split(File.pathSeparatorChar)) { + val index = pkg.indexOf('*') + if (index != -1) { + if (index < pkg.length - 1) { + throw DriverException(stderr = "Wildcards in stub packages must be at the end of the package: $pkg)") + } + val prefix = pkg.removeSuffix("*") + if (prefix.endsWith(".")) { + // In doclava, "foo.*" does not match "foo", but we want to do that. + exactPackages.add(prefix.substring(0, prefix.length - 1)) + } + packagePrefixes += prefix + } else { + exactPackages.add(pkg) + } + } + } + fun matches(packageItem: PackageItem): Boolean { return matches(packageItem.qualifiedName()) } diff --git a/src/main/java/com/android/tools/metalava/Reporter.kt b/src/main/java/com/android/tools/metalava/Reporter.kt index 56009b7bf4c12b26e26092ef59c2d1534a503549..a7d0a1bbfb6416753a64cca0963286ed0fea08e3 100644 --- a/src/main/java/com/android/tools/metalava/Reporter.kt +++ b/src/main/java/com/android/tools/metalava/Reporter.kt @@ -112,7 +112,7 @@ open class Reporter(private val rootFolder: File? = null) { return report(severity, file?.path, message, id) } - fun report(id: Errors.Error, item: Item?, message: String): Boolean { + fun report(id: Errors.Error, item: Item?, message: String, psi: PsiElement? = null): Boolean { if (isSuppressed(id, item, message)) { return false } @@ -123,16 +123,33 @@ open class Reporter(private val rootFolder: File? = null) { return false } + // If we are only emitting some packages (--stub-packages), don't report + // issues from other packages + if (item != null) { + val packageFilter = options.stubPackages + if (packageFilter != null) { + val pkg = item.containingPackage(false) + if (pkg != null && !packageFilter.matches(pkg)) { + return false + } + } + } + val baseline = options.baseline if (item != null && baseline != null && baseline.mark(item, message, id)) { return false + } else if (psi != null && baseline != null && baseline.mark(psi, message, id)) { + return false } - return when (item) { - is PsiItem -> { + return when { + psi != null -> { + report(severity, psi, message, id) + } + item is PsiItem -> { report(severity, item.psi(), message, id) } - is TextItem -> report(severity, (item as? TextItem)?.position.toString(), message, id) + item is TextItem -> report(severity, (item as? TextItem)?.position.toString(), message, id) else -> report(severity, null as String?, message, id) } } diff --git a/src/main/java/com/android/tools/metalava/SignatureWriter.kt b/src/main/java/com/android/tools/metalava/SignatureWriter.kt index c38a1a125355217db8fc674d1ef6e3ac99511d35..9612e04e83e28d06cca7a405b11f76f172213493 100644 --- a/src/main/java/com/android/tools/metalava/SignatureWriter.kt +++ b/src/main/java/com/android/tools/metalava/SignatureWriter.kt @@ -48,6 +48,10 @@ class SignatureWriter( filterReference = filterReference, showUnannotated = options.showUnannotated ) { + override fun skip(item: Item): Boolean { + return super.skip(item) || item is ClassItem && item.notStrippable + } + init { if (options.includeSignatureFormatVersion) { writer.print(options.outputFormat.header()) diff --git a/src/main/java/com/android/tools/metalava/StubWriter.kt b/src/main/java/com/android/tools/metalava/StubWriter.kt index a5c500a6aa4468c274f5c47182e174da9991ecff..6f5060b20e870cfe2560c14d5970edf01056f294 100644 --- a/src/main/java/com/android/tools/metalava/StubWriter.kt +++ b/src/main/java/com/android/tools/metalava/StubWriter.kt @@ -54,7 +54,11 @@ class StubWriter( // Methods are by default sorted in source order in stubs, to encourage methods // that are near each other in the source to show up near each other in the documentation methodComparator = MethodItem.sourceOrderComparator, - filterEmit = FilterPredicate(ApiPredicate(ignoreShown = true, includeDocOnly = docStubs)), + filterEmit = FilterPredicate(ApiPredicate(ignoreShown = true, includeDocOnly = docStubs)) + // In stubs we have to include non-strippable things too. This is an error in the API, + // and we've removed all of it from the framework, but there are libraries which still + // have reference errors. + .or { it is ClassItem && it.notStrippable }, filterReference = ApiPredicate(ignoreShown = true, includeDocOnly = docStubs), includeEmptyOuterClasses = true ) { @@ -62,14 +66,6 @@ class StubWriter( private val sourceList = StringBuilder(20000) - override fun include(cls: ClassItem): Boolean { - val filter = options.stubPackages - if (filter != null && !filter.matches(cls.containingPackage())) { - return false - } - return super.include(cls) - } - /** Writes a source file list of the generated stubs */ fun writeSourceList(target: File, root: File?) { target.parentFile?.mkdirs() @@ -246,8 +242,9 @@ class StubWriter( generateTypeParameterList(typeList = cls.typeParameterList(), addSpace = false) generateSuperClassStatement(cls) - generateInterfaceList(cls) - + if (!cls.notStrippable) { + generateInterfaceList(cls) + } writer.print(" {\n") if (cls.isEnum()) { @@ -414,6 +411,9 @@ class StubWriter( } override fun visitConstructor(constructor: ConstructorItem) { + if (constructor.containingClass().notStrippable) { + return + } writeConstructor(constructor, constructor.superConstructor) } @@ -457,14 +457,17 @@ class StubWriter( writer.write(", ") } val type = parameter.type() - val typeString = type.toErasedTypeString(it) if (!type.primitive) { if (includeCasts) { - writer.write("(") - // Types with varargs can't appear as varargs when used as an argument - if (typeString.contains("...")) { - writer.write(typeString.replace("...", "[]")) + val typeString = type.toErasedTypeString(it).replace("...", "[]") + writer.write("(") + if (type.asTypeParameter(superConstructor) != null) { + // It's a type parameter: see if we should map the type back to the concrete + // type in this class + val map = constructor?.containingClass()?.mapTypeVariables(it.containingClass()) + val cast = map?.get(type.toTypeString(context = it)) ?: typeString + writer.write(cast) } else { writer.write(typeString) } @@ -472,6 +475,8 @@ class StubWriter( } writer.write("null") } else { + // Add cast for things like shorts and bytes + val typeString = type.toTypeString(context = it) if (typeString != "boolean" && typeString != "int" && typeString != "long") { writer.write("(") writer.write(typeString) @@ -498,6 +503,9 @@ class StubWriter( } override fun visitMethod(method: MethodItem) { + if (method.containingClass().notStrippable) { + return + } writeMethod(method.containingClass(), method, false) } @@ -562,6 +570,10 @@ class StubWriter( return } + if (field.containingClass().notStrippable) { + return + } + writer.println() appendDocumentation(field, writer) diff --git a/src/main/java/com/android/tools/metalava/doclava1/ApiFile.java b/src/main/java/com/android/tools/metalava/doclava1/ApiFile.java index f1d672ec2801a305881b5f6eea117a387f439933..00f3eb3126cb9c21107824d7d5c82ca1a67614a9 100644 --- a/src/main/java/com/android/tools/metalava/doclava1/ApiFile.java +++ b/src/main/java/com/android/tools/metalava/doclava1/ApiFile.java @@ -42,7 +42,9 @@ import org.jetbrains.annotations.Nullable; import java.io.File; import java.io.IOException; import java.util.ArrayList; +import java.util.HashMap; import java.util.List; +import java.util.Map; import static com.android.tools.metalava.ConstantsKt.ANDROIDX_NONNULL; import static com.android.tools.metalava.ConstantsKt.ANDROIDX_NULLABLE; @@ -418,7 +420,7 @@ public class ApiFile { name = token; method = new TextMethodItem(api, name, cl, modifiers, returnType, tokenizer.pos()); method.setDeprecated(modifiers.isDeprecated()); - if (cl.isInterface() && !modifiers.isDefault()) { + if (cl.isInterface() && !modifiers.isDefault() && !modifiers.isStatic()) { modifiers.setAbstract(true); } method.setTypeParameterList(typeParameterList); diff --git a/src/main/java/com/android/tools/metalava/doclava1/Errors.java b/src/main/java/com/android/tools/metalava/doclava1/Errors.java index cc0e5db9079ebcda4958a22c6fdee9e4399083aa..9587ae95bbad18c5cbb7d6c38201e6635812efff 100644 --- a/src/main/java/com/android/tools/metalava/doclava1/Errors.java +++ b/src/main/java/com/android/tools/metalava/doclava1/Errors.java @@ -248,14 +248,15 @@ public class Errors { // The plan is for this to be set as an error once (1) existing code is marked as @deprecated // and (2) the principle is adopted by the API council public static final Error REFERENCES_DEPRECATED = new Error(154, HIDDEN); - // Hidden until (1) API council agrees this should be an error, and (2) existing - // violations are annotated as @hide - public static final Error UNHIDDEN_SYSTEM_API = new Error(155, HIDDEN); + public static final Error UNHIDDEN_SYSTEM_API = new Error(155, ERROR); public static final Error SHOWING_MEMBER_IN_HIDDEN_CLASS = new Error(156, ERROR); public static final Error INVALID_NULLABILITY_ANNOTATION = new Error(157, ERROR); public static final Error REFERENCES_HIDDEN = new Error(158, ERROR); public static final Error IGNORING_SYMLINK = new Error(159, INFO); public static final Error INVALID_NULLABILITY_ANNOTATION_WARNING = new Error(160, WARNING); + // The plan is for this to be set as an error once (1) existing code is marked as @deprecated + // and (2) the principle is adopted by the API council + public static final Error EXTENDS_DEPRECATED = new Error(161, HIDDEN); // API lint public static final Error START_WITH_LOWER = new Error(300, ERROR, Category.API_LINT, "S1"); @@ -322,7 +323,7 @@ public class Errors { public static final Error FRACTION_FLOAT = new Error(362, ERROR, Category.API_LINT); public static final Error PERCENTAGE_INT = new Error(363, ERROR, Category.API_LINT); public static final Error NOT_CLOSEABLE = new Error(364, WARNING, Category.API_LINT); - public static final Error KOTLIN_OPERATOR = new Error(365, WARNING, Category.API_LINT); + public static final Error KOTLIN_OPERATOR = new Error(365, INFO, Category.API_LINT); public static final Error ARRAY_RETURN = new Error(366, WARNING, Category.API_LINT); public static final Error USER_HANDLE = new Error(367, WARNING, Category.API_LINT); public static final Error USER_HANDLE_NAME = new Error(368, WARNING, Category.API_LINT); @@ -341,6 +342,7 @@ public class Errors { public static final Error MISSING_JVMSTATIC = new Error(381, WARNING, Category.API_LINT); // Formerly 143 public static final Error DEFAULT_VALUE_CHANGE = new Error(382, ERROR, Category.API_LINT); // Formerly 144 public static final Error DOCUMENT_EXCEPTIONS = new Error(383, ERROR, Category.API_LINT); // Formerly 145 + public static final Error FORBIDDEN_SUPER_CLASS = new Error(384, ERROR, Category.API_LINT); static { // Attempt to initialize error names based on the field names diff --git a/src/main/java/com/android/tools/metalava/model/AnnotationItem.kt b/src/main/java/com/android/tools/metalava/model/AnnotationItem.kt index d398bff2a2a4e377d8bc45f580949f3025eda38f..0e6921d22c624dec1744526a7a18ec2100aefbce 100644 --- a/src/main/java/com/android/tools/metalava/model/AnnotationItem.kt +++ b/src/main/java/com/android/tools/metalava/model/AnnotationItem.kt @@ -28,8 +28,8 @@ import com.android.tools.metalava.ANDROIDX_ANNOTATION_PREFIX import com.android.tools.metalava.ANDROIDX_NONNULL import com.android.tools.metalava.ANDROIDX_NULLABLE import com.android.tools.metalava.ANDROID_SUPPORT_ANNOTATION_PREFIX +import com.android.tools.metalava.Compatibility import com.android.tools.metalava.JAVA_LANG_PREFIX -import com.android.tools.metalava.Options import com.android.tools.metalava.RECENTLY_NONNULL import com.android.tools.metalava.RECENTLY_NULLABLE import com.android.tools.metalava.doclava1.ApiPredicate @@ -456,7 +456,7 @@ interface AnnotationItem { /** * Given a "full" annotation name, shortens it by removing redundant package names. - * This is intended to be used by the [Options.omitCommonPackages] flag + * This is intended to be used by the [Compatibility.omitCommonPackages] flag * to reduce clutter in signature files. * * For example, this method will convert `@androidx.annotation.Nullable` to just diff --git a/src/main/java/com/android/tools/metalava/model/ClassItem.kt b/src/main/java/com/android/tools/metalava/model/ClassItem.kt index e06784caea513cae3abdaaed3a75a0304492c06b..7dc221b37a2f718ce5bf7df0c8ea7d37b95b5066 100644 --- a/src/main/java/com/android/tools/metalava/model/ClassItem.kt +++ b/src/main/java/com/android/tools/metalava/model/ClassItem.kt @@ -189,6 +189,12 @@ interface ClassItem : Item { /** The containing package */ fun containingPackage(): PackageItem + override fun containingPackage(strict: Boolean): PackageItem = containingPackage() + + override fun containingClass(strict: Boolean): ClassItem? { + return if (strict) containingClass() else this + } + /** Gets the type for this class */ fun toType(): TypeItem @@ -218,6 +224,9 @@ interface ClassItem : Item { var hasPrivateConstructor: Boolean + /** If true, this is an invisible element that was referenced by a public API. */ + var notStrippable: Boolean + /** * Maven artifact of this class, if any. (Not used for the Android SDK, but used in * for example support libraries. @@ -849,7 +858,7 @@ class VisitCandidate(private val cls: ClassItem, private val visitor: ApiVisitor return } - val emitThis = if (visitor.includeEmptyOuterClasses) emit else emitClass + val emitThis = cls.emit && if (visitor.includeEmptyOuterClasses) emit else emitClass if (emitThis) { if (!visitor.visitingPackage) { visitor.visitingPackage = true diff --git a/src/main/java/com/android/tools/metalava/model/Item.kt b/src/main/java/com/android/tools/metalava/model/Item.kt index d0f9722e6e3c09becdb6e08de92c610a736f8480..9fbc546a6505b93336eaa1fe4d6739f8c01dbbd0 100644 --- a/src/main/java/com/android/tools/metalava/model/Item.kt +++ b/src/main/java/com/android/tools/metalava/model/Item.kt @@ -196,6 +196,20 @@ interface Item { /** Produces a user visible description of this item, including a label such as "class" or "field" */ fun describe(capitalize: Boolean = false) = describe(this, capitalize) + /** + * Returns the package that contains this item. If [strict] is false, this will return self + * if called on a package, otherwise it will return the containing package (e.g. "foo" for "foo.bar"). + * The parameter is ignored on other item types. + */ + fun containingPackage(strict: Boolean = true): PackageItem? + + /** + * Returns the class that contains this item. If [strict] is false, this will return self + * if called on a class, otherwise it will return the outer class, if any. The parameter is + * ignored on other item types. + */ + fun containingClass(strict: Boolean = true): ClassItem? + companion object { fun describe(item: Item, capitalize: Boolean = false): String { return when (item) { diff --git a/src/main/java/com/android/tools/metalava/model/MemberItem.kt b/src/main/java/com/android/tools/metalava/model/MemberItem.kt index 63be2b6f9d3c1029647350f975f54692585ecd2e..962287e30ad77b972cac534526135c26fd626216 100644 --- a/src/main/java/com/android/tools/metalava/model/MemberItem.kt +++ b/src/main/java/com/android/tools/metalava/model/MemberItem.kt @@ -26,6 +26,8 @@ interface MemberItem : Item { /** The containing class */ fun containingClass(): ClassItem + override fun containingClass(strict: Boolean): ClassItem = containingClass() + override fun containingPackage(strict: Boolean): PackageItem = containingClass().containingPackage(false) override fun parent(): ClassItem? = containingClass() /** diff --git a/src/main/java/com/android/tools/metalava/model/PackageDocs.kt b/src/main/java/com/android/tools/metalava/model/PackageDocs.kt index 0c3c243055e2a1d7c454b98916265f20dfa2f1a8..107e9fde1c59cbc7ac076133bbb1b2298e087297 100644 --- a/src/main/java/com/android/tools/metalava/model/PackageDocs.kt +++ b/src/main/java/com/android/tools/metalava/model/PackageDocs.kt @@ -21,6 +21,5 @@ data class PackageDocs( val overviewDocs: MutableMap<String, String>, val hiddenPackages: MutableSet<String> ) { - fun getPackageDocumentation(pkg: PackageItem): String? = packageDocs[pkg.qualifiedName()] fun getOverviewDocumentation(pkg: PackageItem): String? = overviewDocs[pkg.qualifiedName()] } \ No newline at end of file diff --git a/src/main/java/com/android/tools/metalava/model/PackageItem.kt b/src/main/java/com/android/tools/metalava/model/PackageItem.kt index ad4190aae230e67698a7285c89b31690bd9584cd..970d2eb7416f4f71cedccc022e146dd047d38214 100644 --- a/src/main/java/com/android/tools/metalava/model/PackageItem.kt +++ b/src/main/java/com/android/tools/metalava/model/PackageItem.kt @@ -37,7 +37,10 @@ interface PackageItem : Item { override fun parent(): PackageItem? = if (qualifiedName().isEmpty()) null else containingPackage() - fun containingPackage(): PackageItem? { + override fun containingPackage(strict: Boolean): PackageItem? { + if (!strict) { + return this + } val name = qualifiedName() val lastDot = name.lastIndexOf('.') return if (lastDot != -1) { diff --git a/src/main/java/com/android/tools/metalava/model/ParameterItem.kt b/src/main/java/com/android/tools/metalava/model/ParameterItem.kt index 00210ecc2d085f276cba33eb6758ecc26a491339..3610d51685185d5383f2b21522b8e60e8da907d6 100644 --- a/src/main/java/com/android/tools/metalava/model/ParameterItem.kt +++ b/src/main/java/com/android/tools/metalava/model/ParameterItem.kt @@ -101,5 +101,8 @@ interface ParameterItem : Item { return modifiers.hasNullnessInfo() } + override fun containingClass(strict: Boolean): ClassItem? = containingMethod().containingClass(false) + override fun containingPackage(strict: Boolean): PackageItem? = containingMethod().containingPackage(false) + // TODO: modifier list } \ No newline at end of file diff --git a/src/main/java/com/android/tools/metalava/model/psi/Javadoc.kt b/src/main/java/com/android/tools/metalava/model/psi/Javadoc.kt index 77c6ea44bf21ecdc4aceed4a4ecc08d61a1ab32e..e98db1f00bc186e7270e174730fd89377e5f8a1c 100644 --- a/src/main/java/com/android/tools/metalava/model/psi/Javadoc.kt +++ b/src/main/java/com/android/tools/metalava/model/psi/Javadoc.kt @@ -761,6 +761,7 @@ fun handleTag( val referencedElement = referenceElement!!.resolve() if (referencedElement is PsiClass) { var className = PsiClassItem.computeFullClassName(referencedElement) + val fullName = className if (className.indexOf('.') != -1 && !referenceText.startsWith(className)) { val simpleName = referencedElement.name if (simpleName != null && referenceText.startsWith(simpleName)) { @@ -772,7 +773,15 @@ fun handleTag( sb.append(element.name) sb.append(' ') sb.append(referencedElement.qualifiedName) - val suffix = referenceText.substring(className.length) + var suffix = referenceText.substring(className.length) + if (suffix.startsWith("#") && suffix.startsWith(className, 1) && + (suffix.length == className.length + 1 || suffix[className.length + 1] == '(') + ) { + // It's a constructor reference. Unfortunately, javadoc doesn't + // handle this, so we'll need to work around by placing the full + // name as the method name + suffix = "#" + fullName + suffix.substring(className.length + 1) + } if (suffix.contains("(") && suffix.contains(")")) { expandArgumentList(element, suffix, sb) } else { diff --git a/src/main/java/com/android/tools/metalava/model/psi/PsiBasedCodebase.kt b/src/main/java/com/android/tools/metalava/model/psi/PsiBasedCodebase.kt index 1bc4bf6a79800865ae4d6f7d7224c855ff4c7246..349ba056fb0001dbbbab5ceb695f5e174e15af47 100644 --- a/src/main/java/com/android/tools/metalava/model/psi/PsiBasedCodebase.kt +++ b/src/main/java/com/android/tools/metalava/model/psi/PsiBasedCodebase.kt @@ -58,8 +58,8 @@ import java.util.ArrayList import java.util.HashMap import java.util.zip.ZipFile -const val PACKAGE_ESTIMATE = 400 -const val CLASS_ESTIMATE = 12000 +const val PACKAGE_ESTIMATE = 500 +const val CLASS_ESTIMATE = 15000 const val METHOD_ESTIMATE = 1000 open class PsiBasedCodebase(location: File, override var description: String = "Unknown") : DefaultCodebase(location) { diff --git a/src/main/java/com/android/tools/metalava/model/psi/PsiClassItem.kt b/src/main/java/com/android/tools/metalava/model/psi/PsiClassItem.kt index f3efc401295bb83d93596cb85c8a526a28873238..564e15cbe5afe41f8da6339ce06fae61df1624a4 100644 --- a/src/main/java/com/android/tools/metalava/model/psi/PsiClassItem.kt +++ b/src/main/java/com/android/tools/metalava/model/psi/PsiClassItem.kt @@ -79,6 +79,7 @@ open class PsiClassItem( } override var defaultConstructor: ConstructorItem? = null + override var notStrippable = false override var artifact: String? = null private var containingClass: PsiClassItem? = null diff --git a/src/main/java/com/android/tools/metalava/model/psi/PsiModifierItem.kt b/src/main/java/com/android/tools/metalava/model/psi/PsiModifierItem.kt index 015b4703927ed9b69d99967af6f93a52a5627713..3a112957cc7003ee2398ed9cc220c71c5335c18b 100644 --- a/src/main/java/com/android/tools/metalava/model/psi/PsiModifierItem.kt +++ b/src/main/java/com/android/tools/metalava/model/psi/PsiModifierItem.kt @@ -25,6 +25,7 @@ import com.intellij.psi.PsiDocCommentOwner import com.intellij.psi.PsiMethod import com.intellij.psi.PsiModifier import com.intellij.psi.PsiModifierListOwner +import com.intellij.psi.PsiReferenceExpression import org.jetbrains.kotlin.asJava.elements.KtLightModifierList import org.jetbrains.kotlin.lexer.KtTokens import org.jetbrains.kotlin.psi.KtNamedFunction @@ -143,7 +144,28 @@ class PsiModifierItem( PsiModifierItem(codebase, flags) } else { val annotations: MutableList<AnnotationItem> = - psiAnnotations.map { PsiAnnotationItem.create(codebase, it) }.toMutableList() + psiAnnotations.map { + val qualifiedName = it.qualifiedName + // TODO: com.android.internal.annotations.VisibleForTesting? + if (qualifiedName == "androidx.annotation.VisibleForTesting" || + qualifiedName == "android.support.annotation.VisibleForTesting") { + val otherwise = it.findAttributeValue("otherwise") + val ref = when { + otherwise is PsiReferenceExpression -> otherwise.referenceName ?: "" + otherwise != null -> otherwise.text + else -> "" + } + if (ref.endsWith("PROTECTED")) { + flags = (flags and PUBLIC.inv() and PRIVATE.inv() and INTERNAL.inv()) or PROTECTED + } else if (ref.endsWith("PACKAGE_PRIVATE")) { + flags = (flags and PUBLIC.inv() and PRIVATE.inv() and INTERNAL.inv() and PROTECTED.inv()) + } else if (ref.endsWith("PRIVATE") || ref.endsWith("NONE")) { + flags = (flags and PUBLIC.inv() and PROTECTED.inv() and INTERNAL.inv()) or PRIVATE + } + } + + PsiAnnotationItem.create(codebase, it, qualifiedName) + }.toMutableList() PsiModifierItem(codebase, flags, annotations) } } diff --git a/src/main/java/com/android/tools/metalava/model/psi/PsiPackageItem.kt b/src/main/java/com/android/tools/metalava/model/psi/PsiPackageItem.kt index 6060a07a02acd802c83a707ae4a977f8a3333524..821c105eb3a81b41e4020a7486ee9d185171d55f 100644 --- a/src/main/java/com/android/tools/metalava/model/psi/PsiPackageItem.kt +++ b/src/main/java/com/android/tools/metalava/model/psi/PsiPackageItem.kt @@ -40,7 +40,12 @@ class PsiPackageItem( lateinit var containingPackageField: PsiPackageItem - override fun containingPackage(): PackageItem? { + override fun containingClass(strict: Boolean): ClassItem? = null + + override fun containingPackage(strict: Boolean): PackageItem? { + if (!strict) { + return this + } return if (qualifiedName.isEmpty()) null else { if (!::containingPackageField.isInitialized) { var parentPackage = qualifiedName diff --git a/src/main/java/com/android/tools/metalava/model/text/TextClassItem.kt b/src/main/java/com/android/tools/metalava/model/text/TextClassItem.kt index f78e825140f09212389c4b69e6c03c9f553dd3ed..e890817d481ca604330a48d917cd4b40637c97f5 100644 --- a/src/main/java/com/android/tools/metalava/model/text/TextClassItem.kt +++ b/src/main/java/com/android/tools/metalava/model/text/TextClassItem.kt @@ -57,6 +57,8 @@ open class TextClassItem( override val isTypeParameter: Boolean = false + override var notStrippable = false + override var artifact: String? = null override fun equals(other: Any?): Boolean { @@ -252,6 +254,10 @@ open class TextClassItem( override fun qualifiedName(): String = qualifiedName override fun toString(): String = qualifiedName() + override fun mapTypeVariables(target: ClassItem): Map<String, String> { + return emptyMap() + } + companion object { fun createClassStub(codebase: TextCodebase, name: String): TextClassItem = createStub(codebase, name, isInterface = false) diff --git a/src/main/java/com/android/tools/metalava/model/text/TextPackageItem.kt b/src/main/java/com/android/tools/metalava/model/text/TextPackageItem.kt index 0c2d7d2aebb3cc213d59667ce26960b224d9f5d6..3af85943647c2245ec74f4335777fccce9b94b35 100644 --- a/src/main/java/com/android/tools/metalava/model/text/TextPackageItem.kt +++ b/src/main/java/com/android/tools/metalava/model/text/TextPackageItem.kt @@ -55,6 +55,8 @@ class TextPackageItem( override fun qualifiedName(): String = name + override fun containingClass(strict: Boolean): ClassItem? = null + override fun equals(other: Any?): Boolean { if (this === other) return true if (other !is PackageItem) return false diff --git a/src/main/java/com/android/tools/metalava/model/visitors/ApiVisitor.kt b/src/main/java/com/android/tools/metalava/model/visitors/ApiVisitor.kt index b7e34e93a14d0b4a7704e01a94aa2d5db63b9e2e..76b4a347ef5082b946c439000b473933952b3877 100644 --- a/src/main/java/com/android/tools/metalava/model/visitors/ApiVisitor.kt +++ b/src/main/java/com/android/tools/metalava/model/visitors/ApiVisitor.kt @@ -21,6 +21,7 @@ import com.android.tools.metalava.model.ClassItem import com.android.tools.metalava.model.FieldItem import com.android.tools.metalava.model.Item import com.android.tools.metalava.model.MethodItem +import com.android.tools.metalava.options import java.util.function.Predicate open class ApiVisitor( @@ -108,5 +109,12 @@ open class ApiVisitor( // this property keeps track of whether we've already visited the current package var visitingPackage = false - open fun include(cls: ClassItem): Boolean = cls.emit + open fun include(cls: ClassItem): Boolean { + val filter = options.stubPackages + if (filter != null && !filter.matches(cls.containingPackage())) { + return false + } + + return cls.emit || cls.codebase.preFiltered + } } diff --git a/src/main/resources/version.properties b/src/main/resources/version.properties index 7495ade0b2840f859dded608ee369f4eae91fc4f..233e17ea97e1c8f6cf1cc9d0fa73c97a81797f0f 100644 --- a/src/main/resources/version.properties +++ b/src/main/resources/version.properties @@ -2,4 +2,4 @@ # Version definition # This file is read by gradle build scripts, but also packaged with metalava # as a resource for the Version classes to read. -metalavaVersion=1.2.2 +metalavaVersion=1.2.5 diff --git a/src/test/java/com/android/tools/metalava/AnnotationsMergerTest.kt b/src/test/java/com/android/tools/metalava/AnnotationsMergerTest.kt index 2b06cbc4eeec84651088465a1495b4ac318d5ab2..cc7eb68c3c8845c98f32e019078176867b691b9d 100644 --- a/src/test/java/com/android/tools/metalava/AnnotationsMergerTest.kt +++ b/src/test/java/com/android/tools/metalava/AnnotationsMergerTest.kt @@ -213,6 +213,7 @@ class AnnotationsMergerTest : DriverTest() { @Test fun `Merge inclusion annotations from Java stub files`() { check( + warnings = "src/test/pkg/Example.java:6: error: @test.annotation.Show APIs must also be marked @hide: method test.pkg.Example.cShown() [UnhiddenSystemApi]", sourceFiles = *arrayOf( java( """ diff --git a/src/test/java/com/android/tools/metalava/ApiFileTest.kt b/src/test/java/com/android/tools/metalava/ApiFileTest.kt index c96c46de341a1959d47e2f577602724258795b0e..962f9c6147d277003c36d1497da9ec5ebd586d57 100644 --- a/src/test/java/com/android/tools/metalava/ApiFileTest.kt +++ b/src/test/java/com/android/tools/metalava/ApiFileTest.kt @@ -3116,4 +3116,219 @@ class ApiFileTest : DriverTest() { """ ) } + + @Test + fun `Skip incorrect inherit`() { + check( + // Simulate test-mock scenario for getIContentProvider + extraArguments = arrayOf("--stub-packages", "android.test.mock"), + compatibilityMode = false, + warnings = "src/android/test/mock/MockContentProvider.java:6: warning: Public class android.test.mock.MockContentProvider stripped of unavailable superclass android.content.ContentProvider [HiddenSuperclass]", + sourceFiles = *arrayOf( + java( + """ + package android.test.mock; + + import android.content.ContentProvider; + import android.content.IContentProvider; + + public abstract class MockContentProvider extends ContentProvider { + /** + * Returns IContentProvider which calls back same methods in this class. + * By overriding this class, we avoid the mechanism hidden behind ContentProvider + * (IPC, etc.) + * + * @hide + */ + @Override + public final IContentProvider getIContentProvider() { + return mIContentProvider; + } + } + """ + ), + java( + """ + package android.content; + + /** @hide */ + public abstract class ContentProvider { + protected boolean isTemporary() { + return false; + } + + // This is supposed to be @hide, but in turbine-combined/framework.jar included + // by java_sdk_library like test-mock, it's not; this is what the special + // flag is used to test + public IContentProvider getIContentProvider() { + return null; + } + } + """ + ), + java( + """ + package android.content; + import android.os.IInterface; + + /** + * The ipc interface to talk to a content provider. + * @hide + */ + public interface IContentProvider extends IInterface { + } + """ + ), + java( + """ + package android.content; + + // Not hidden. Here to make sure that we respect stub-packages + // and exclude it from everything, including signatures. + public class ClipData { + } + """ + ) + ), + api = """ + package android.test.mock { + public abstract class MockContentProvider { + ctor public MockContentProvider(); + } + } + """ + ) + } + + @Test + fun `Test Visible For Testing`() { + // Use the otherwise= visibility in signatures + // Regression test for issue 118763806 + check( + sourceFiles = *arrayOf( + java( + """ + package test.pkg; + import androidx.annotation.VisibleForTesting; + + @SuppressWarnings({"ClassNameDiffersFromFileName", "WeakerAccess"}) + public class ProductionCode { + private ProductionCode() { } + + @VisibleForTesting(otherwise = VisibleForTesting.PROTECTED) + public void shouldBeProtected() { + } + + @VisibleForTesting(otherwise = VisibleForTesting.PRIVATE) + protected void shouldBePrivate1() { + } + + @VisibleForTesting(otherwise = VisibleForTesting.PRIVATE) + public void shouldBePrivate2() { + } + + @VisibleForTesting(otherwise = VisibleForTesting.PACKAGE_PRIVATE) + public void shouldBePackagePrivate() { + } + + @VisibleForTesting(otherwise = VisibleForTesting.NONE) + public void shouldBeHidden() { + } + } + """ + ).indented(), + kotlin( + """ + package test.pkg + import androidx.annotation.VisibleForTesting + + open class ProductionCode2 private constructor() { + + @VisibleForTesting(otherwise = VisibleForTesting.PROTECTED) + fun shouldBeProtected() { + } + + @VisibleForTesting(otherwise = VisibleForTesting.PRIVATE) + protected fun shouldBePrivate1() { + } + + @VisibleForTesting(otherwise = VisibleForTesting.PRIVATE) + fun shouldBePrivate2() { + } + + @VisibleForTesting(otherwise = VisibleForTesting.PACKAGE_PRIVATE) + fun shouldBePackagePrivate() { + } + + @VisibleForTesting(otherwise = VisibleForTesting.NONE) + fun shouldBeHidden() { + } + } + """ + ).indented(), + visibleForTestingSource + ), + api = """ + package test.pkg { + public class ProductionCode { + method protected void shouldBeProtected(); + } + public class ProductionCode2 { + method protected final void shouldBeProtected(); + } + } + """, + extraArguments = arrayOf(ARG_HIDE_PACKAGE, "androidx.annotation") + ) + } + + @Test + fun `References Deprecated`() { + check( + extraArguments = arrayOf( + ARG_ERROR, "ReferencesDeprecated", + ARG_ERROR, "ExtendsDeprecated" + ), + warnings = """ + src/test/pkg/MyClass.java:3: error: Parameter of deprecated type test.pkg.DeprecatedClass in test.pkg.MyClass.method1(): this method should also be deprecated [ReferencesDeprecated] + src/test/pkg/MyClass.java:4: error: Return type of deprecated type test.pkg.DeprecatedInterface in test.pkg.MyClass.method2(): this method should also be deprecated [ReferencesDeprecated] + src/test/pkg/MyClass.java:4: error: Returning deprecated type test.pkg.DeprecatedInterface from test.pkg.MyClass.method2(): this method should also be deprecated [ReferencesDeprecated] + src/test/pkg/MyClass.java:2: error: Extending deprecated super class class test.pkg.DeprecatedClass from test.pkg.MyClass: this class should also be deprecated [ExtendsDeprecated] + src/test/pkg/MyClass.java:2: error: Implementing interface of deprecated type test.pkg.DeprecatedInterface in test.pkg.MyClass: this class should also be deprecated [ExtendsDeprecated] + """, + sourceFiles = *arrayOf( + java( + """ + package test.pkg; + /** @deprecated */ + @Deprecated + public class DeprecatedClass { + } + """ + ), + java( + """ + package test.pkg; + /** @deprecated */ + @Deprecated + public interface DeprecatedInterface { + } + """ + ), + java( + """ + package test.pkg; + public class MyClass extends DeprecatedClass implements DeprecatedInterface { + public void method1(DeprecatedClass p, int i) { } + public DeprecatedInterface method2(int i) { return null; } + + /** @deprecated */ + @Deprecated + public void method3(DeprecatedClass p, int i) { } + } + """ + ) + ) + ) + } } \ No newline at end of file diff --git a/src/test/java/com/android/tools/metalava/ApiFromTextTest.kt b/src/test/java/com/android/tools/metalava/ApiFromTextTest.kt index deb072b657356dd1b844f79cd7600e2167f35ed5..bbc1439e528c29ed8cc7d5cf4d216b48173fdfe9 100644 --- a/src/test/java/com/android/tools/metalava/ApiFromTextTest.kt +++ b/src/test/java/com/android/tools/metalava/ApiFromTextTest.kt @@ -43,7 +43,7 @@ class ApiFromTextTest : DriverTest() { } @Test - fun `Handle lambas as default values`() { + fun `Handle lambdas as default values`() { val source = """ // Signature format: 3.0 package androidx.collection { @@ -533,6 +533,10 @@ class ApiFromTextTest : DriverTest() { method public abstract boolean isCancelled(); method public abstract boolean isDone(); } + public class AuthenticatorException extends java.lang.Throwable { + } + public class OperationCanceledException extends java.lang.Throwable { + } } """, api = """ @@ -544,6 +548,10 @@ class ApiFromTextTest : DriverTest() { method public abstract boolean isCancelled(); method public abstract boolean isDone(); } + public class AuthenticatorException extends java.lang.Throwable { + } + public class OperationCanceledException extends java.lang.Throwable { + } } """ ) diff --git a/src/test/java/com/android/tools/metalava/ApiLintTest.kt b/src/test/java/com/android/tools/metalava/ApiLintTest.kt index 71514bbb8fe9482fc3a4d4ac0968895eef6ac760..dbbc41a8447596abc888d5f0a9212fabe0a362f9 100644 --- a/src/test/java/com/android/tools/metalava/ApiLintTest.kt +++ b/src/test/java/com/android/tools/metalava/ApiLintTest.kt @@ -22,7 +22,7 @@ class ApiLintTest : DriverTest() { @Test fun `Test names`() { - // Make sure we only flag isses in new API + // Make sure we only flag issues in new API check( apiLint = "", // enabled compatibilityMode = false, @@ -587,7 +587,14 @@ class ApiLintTest : DriverTest() { apiLint = "", // enabled compatibilityMode = false, warnings = """ - src/android/pkg/CheckSynchronization.java:5: error: Internal locks must not be exposed: method android.pkg.CheckSynchronization.method2(Runnable) [VisiblySynchronized] [Rule M5 in go/android-api-guidelines] + src/android/pkg/CheckSynchronization.java:10: error: Internal locks must not be exposed: method android.pkg.CheckSynchronization.errorMethod1(Runnable) [VisiblySynchronized] [Rule M5 in go/android-api-guidelines] + src/android/pkg/CheckSynchronization.java:12: error: Internal locks must not be exposed (synchronizing on this or class is still externally observable): method android.pkg.CheckSynchronization.errorMethod2() [VisiblySynchronized] [Rule M5 in go/android-api-guidelines] + src/android/pkg/CheckSynchronization.java:16: error: Internal locks must not be exposed (synchronizing on this or class is still externally observable): method android.pkg.CheckSynchronization.errorMethod2() [VisiblySynchronized] [Rule M5 in go/android-api-guidelines] + src/android/pkg/CheckSynchronization.java:21: error: Internal locks must not be exposed (synchronizing on this or class is still externally observable): method android.pkg.CheckSynchronization.errorMethod3() [VisiblySynchronized] [Rule M5 in go/android-api-guidelines] + src/android/pkg/CheckSynchronization2.kt:5: error: Internal locks must not be exposed (synchronizing on this or class is still externally observable): method android.pkg.CheckSynchronization2.errorMethod1() [VisiblySynchronized] [Rule M5 in go/android-api-guidelines] + src/android/pkg/CheckSynchronization2.kt:8: error: Internal locks must not be exposed (synchronizing on this or class is still externally observable): method android.pkg.CheckSynchronization2.errorMethod2() [VisiblySynchronized] [Rule M5 in go/android-api-guidelines] + src/android/pkg/CheckSynchronization2.kt:16: error: Internal locks must not be exposed (synchronizing on this or class is still externally observable): method android.pkg.CheckSynchronization2.errorMethod4() [VisiblySynchronized] [Rule M5 in go/android-api-guidelines] + src/android/pkg/CheckSynchronization2.kt:18: error: Internal locks must not be exposed (synchronizing on this or class is still externally observable): method android.pkg.CheckSynchronization2.errorMethod5() [VisiblySynchronized] [Rule M5 in go/android-api-guidelines] """, sourceFiles = *arrayOf( java( @@ -595,8 +602,55 @@ class ApiLintTest : DriverTest() { package android.pkg; public class CheckSynchronization { - public void method1(Runnable r) { } // OK - public synchronized void method2(Runnable r) { } // ERROR + public void okMethod1(Runnable r) { } + private static final Object LOCK = new Object(); + public void okMethod2() { + synchronized(LOCK) { + } + } + public synchronized void errorMethod1(Runnable r) { } // ERROR + public void errorMethod2() { + synchronized(this) { + } + } + public void errorMethod2() { + synchronized(CheckSynchronization.class) { + } + } + public void errorMethod3() { + if (true) { + synchronized(CheckSynchronization.class) { + } + } + } + } + """ + ), + kotlin( + """ + package android.pkg; + + class CheckSynchronization2 { + fun errorMethod1() { + synchronized(this) { println("hello") } + } + fun errorMethod2() { + synchronized(CheckSynchronization2::class.java) { println("hello") } + } + fun errorMethod3() { + @Suppress("ConstantConditionIf") + if (true) { + synchronized(CheckSynchronization2::class.java) { println("hello") } + } + } + fun errorMethod4() = synchronized(this) { println("hello") } + fun errorMethod5() { + synchronized(CheckSynchronization2::class) { println("hello") } + } + fun okMethod() { + val lock = Object() + synchronized(lock) { println("hello") } + } } """ ) @@ -639,9 +693,11 @@ class ApiLintTest : DriverTest() { src/android/pkg/MyClass1.java:3: error: Inconsistent class name; should be `<Foo>Activity`, was `MyClass1` [ContextNameSuffix] [Rule C4 in go/android-api-guidelines] src/android/pkg/MyClass1.java:6: warning: Methods implemented by developers should follow the on<Something> style, was `badlyNamedAbstractMethod` [OnNameExpected] src/android/pkg/MyClass1.java:7: warning: If implemented by developer, should follow the on<Something> style; otherwise consider marking final [OnNameExpected] + src/android/pkg/MyClass1.java:3: error: MyClass1 should not extend `Activity`. Activity subclasses are impossible to compose. Expose a composable API instead. [ForbiddenSuperClass] src/android/pkg/MyClass2.java:3: error: Inconsistent class name; should be `<Foo>Provider`, was `MyClass2` [ContextNameSuffix] [Rule C4 in go/android-api-guidelines] src/android/pkg/MyClass3.java:3: error: Inconsistent class name; should be `<Foo>Service`, was `MyClass3` [ContextNameSuffix] [Rule C4 in go/android-api-guidelines] src/android/pkg/MyClass4.java:3: error: Inconsistent class name; should be `<Foo>Receiver`, was `MyClass4` [ContextNameSuffix] [Rule C4 in go/android-api-guidelines] + src/android/pkg/MyOkActivity.java:3: error: MyOkActivity should not extend `Activity`. Activity subclasses are impossible to compose. Expose a composable API instead. [ForbiddenSuperClass] """, sourceFiles = *arrayOf( java( @@ -956,7 +1012,7 @@ class ApiLintTest : DriverTest() { src/android/pkg/MyClass.java:8: error: Methods must not throw generic exceptions (`java.lang.Error`) [GenericException] [Rule S1 in go/android-api-guidelines] src/android/pkg/MyClass.java:9: warning: Methods taking no arguments should throw `IllegalStateException` instead of `java.lang.IllegalArgumentException` [IllegalStateException] [Rule S1 in go/android-api-guidelines] src/android/pkg/MyClass.java:10: warning: Methods taking no arguments should throw `IllegalStateException` instead of `java.lang.NullPointerException` [IllegalStateException] [Rule S1 in go/android-api-guidelines] - src/android/pkg/MyClass.java:11: error: Methods calling into system server should rethrow `RemoteException` as `RuntimeException` [RethrowRemoteException] [Rule FW9 in go/android-api-guidelines] + src/android/pkg/MyClass.java:11: error: Methods calling into system server should rethrow `RemoteException` as `RuntimeException` (but do not list it in the throws clause) [RethrowRemoteException] [Rule FW9 in go/android-api-guidelines] """, sourceFiles = *arrayOf( java( @@ -1674,12 +1730,12 @@ class ApiLintTest : DriverTest() { apiLint = "", // enabled compatibilityMode = false, warnings = """ - src/android/pkg/KotlinOperatorTest.java:4: warning: Method can be invoked with an indexing operator from Kotlin: `get` [KotlinOperator] - src/android/pkg/KotlinOperatorTest.java:5: warning: Method can be invoked with an indexing operator from Kotlin: `set` [KotlinOperator] - src/android/pkg/KotlinOperatorTest.java:6: warning: Method can be invoked with function call syntax from Kotlin: `invoke` [KotlinOperator] - src/android/pkg/KotlinOperatorTest.java:7: warning: Method can be invoked as a binary operator from Kotlin: `plus` [KotlinOperator] + src/android/pkg/KotlinOperatorTest.java:4: info: Method can be invoked with an indexing operator from Kotlin: `get` (this is usually desirable; just make sure it makes sense for this type of object) [KotlinOperator] + src/android/pkg/KotlinOperatorTest.java:5: info: Method can be invoked with an indexing operator from Kotlin: `set` (this is usually desirable; just make sure it makes sense for this type of object) [KotlinOperator] + src/android/pkg/KotlinOperatorTest.java:6: info: Method can be invoked with function call syntax from Kotlin: `invoke` (this is usually desirable; just make sure it makes sense for this type of object) [KotlinOperator] + src/android/pkg/KotlinOperatorTest.java:7: info: Method can be invoked as a binary operator from Kotlin: `plus` (this is usually desirable; just make sure it makes sense for this type of object) [KotlinOperator] src/android/pkg/KotlinOperatorTest.java:7: error: Only one of `plus` and `plusAssign` methods should be present for Kotlin [UniqueKotlinOperator] - src/android/pkg/KotlinOperatorTest.java:8: warning: Method can be invoked as a compound assignment operator from Kotlin: `plusAssign` [KotlinOperator] + src/android/pkg/KotlinOperatorTest.java:8: info: Method can be invoked as a compound assignment operator from Kotlin: `plusAssign` (this is usually desirable; just make sure it makes sense for this type of object) [KotlinOperator] """, sourceFiles = *arrayOf( java( @@ -1976,4 +2032,43 @@ class ApiLintTest : DriverTest() { ) ) } + + @Test + fun `Check forbidden super-classes`() { + check( + apiLint = "", // enabled + compatibilityMode = false, + warnings = """ + src/android/pkg/FirstActivity.java:2: error: FirstActivity should not extend `Activity`. Activity subclasses are impossible to compose. Expose a composable API instead. [ForbiddenSuperClass] + src/android/pkg/IndirectActivity.java:2: error: IndirectActivity should not extend `Activity`. Activity subclasses are impossible to compose. Expose a composable API instead. [ForbiddenSuperClass] + src/android/pkg/MyTask.java:2: error: MyTask should not extend `AsyncTask`. AsyncTask is an implementation detail. Expose a listener or, in androidx, a `ListenableFuture` API instead [ForbiddenSuperClass] + """, + sourceFiles = *arrayOf( + java( + """ + package android.pkg; + public abstract class FirstActivity extends android.app.Activity { + private FirstActivity() { } + } + """ + ), + java( + """ + package android.pkg; + public abstract class IndirectActivity extends android.app.ListActivity { + private IndirectActivity() { } + } + """ + ), + java( + """ + package android.pkg; + public abstract class MyTask extends android.os.AsyncTask<String,String,String> { + private MyTask() { } + } + """ + ) + ) + ) + } } diff --git a/src/test/java/com/android/tools/metalava/BaselineTest.kt b/src/test/java/com/android/tools/metalava/BaselineTest.kt index c836c690532a07b4800b6fd96522191e48afeb0c..955019e3c41eaa9a8a0f01b1850e36912310ba8f 100644 --- a/src/test/java/com/android/tools/metalava/BaselineTest.kt +++ b/src/test/java/com/android/tools/metalava/BaselineTest.kt @@ -233,4 +233,72 @@ class BaselineTest : DriverTest() { checkDoclava1 = false ) } + + @Test + fun `Check merging`() { + // Checks merging existing baseline with new baseline: here we have 2 issues that are no longer + // in the code base, one issue that is in the code base before and after, and one new issue, and + // all 4 end up in the merged baseline. + check( + extraArguments = arrayOf( + ARG_HIDE, + "HiddenSuperclass", + ARG_HIDE, + "UnavailableSymbol", + ARG_HIDE, + "HiddenTypeParameter", + ARG_ERROR, + "ReferencesHidden" + ), + baseline = """ + // Baseline format: 1.0 + BothPackageInfoAndHtml: test/visible/package-info.java: + It is illegal to provide both a package-info.java file and a package.html file for the same package + IgnoringSymlink: test/pkg/sub1/sub2/sub3: + Ignoring symlink during package.html discovery directory traversal + ReferencesHidden: test.pkg.Foo#hidden2: + Class test.pkg.Hidden2 is hidden but was referenced (as field type) from public field test.pkg.Foo.hidden2 + """, + updateBaseline = false, + mergeBaseline = """ + // Baseline format: 1.0 + BothPackageInfoAndHtml: test/visible/package-info.java: + It is illegal to provide both a package-info.java file and a package.html file for the same package + IgnoringSymlink: test/pkg/sub1/sub2/sub3: + Ignoring symlink during package.html discovery directory traversal + ReferencesHidden: test.pkg.Foo#hidden1: + Class test.pkg.Hidden1 is not public but was referenced (as field type) from public field test.pkg.Foo.hidden1 + ReferencesHidden: test.pkg.Foo#hidden2: + Class test.pkg.Hidden2 is hidden but was referenced (as field type) from public field test.pkg.Foo.hidden2 + """, + sourceFiles = *arrayOf( + java( + """ + package test.pkg; + public class Foo extends Hidden2 { + public Hidden1 hidden1; + public Hidden2 hidden2; + } + """ + ), + java( + """ + package test.pkg; + // Implicitly not part of the API by being package private + class Hidden1 { + } + """ + ), + java( + """ + package test.pkg; + /** @hide */ + public class Hidden2 { + } + """ + ) + ), + checkDoclava1 = false + ) + } } \ No newline at end of file diff --git a/src/test/java/com/android/tools/metalava/CompatibilityCheckTest.kt b/src/test/java/com/android/tools/metalava/CompatibilityCheckTest.kt index d1aa93154befa3396cff152156d439a0836ad5a9..6c01e32849a29e733c05e0fc4d821d9d86937be1 100644 --- a/src/test/java/com/android/tools/metalava/CompatibilityCheckTest.kt +++ b/src/test/java/com/android/tools/metalava/CompatibilityCheckTest.kt @@ -2439,6 +2439,101 @@ CompatibilityCheckTest : DriverTest() { } } + @Test + fun `Ignore hidden references`() { + check( + warnings = """ + """, + compatibilityMode = false, + checkCompatibilityApi = """ + package test.pkg { + public class MyClass { + ctor public MyClass(); + method public void method1(test.pkg.Hidden); + } + } + """, + sourceFiles = *arrayOf( + java( + """ + package test.pkg; + + public class MyClass { + public void method1(Hidden hidden) { } + } + """ + ), + java( + """ + package test.pkg; + /** @hide */ + public class Hidden { + } + """ + ) + ), + extraArguments = arrayOf( + ARG_HIDE, "ReferencesHidden", + ARG_HIDE, "UnavailableSymbol", + ARG_HIDE, "HiddenTypeParameter" + ) + ) + } + + @Test + fun `Fail on compatible changes that affect signature file contents`() { + // Regression test for 122916999 + check( + extraArguments = arrayOf(ARG_NO_NATIVE_DIFF), + allowCompatibleDifferences = false, + expectedFail = """ + Aborting: Your changes have resulted in differences in the signature file + for the public API. + + The changes may be compatible, but the signature file needs to be updated. + + Diffs: + @@ -5 +5 + ctor public MyClass(); + - method public void method2(); + method public void method1(); + @@ -7 +6 + method public void method1(); + + method public void method2(); + method public void method3(); + """.trimIndent(), + compatibilityMode = false, + // Methods in order + checkCompatibilityApi = """ + package test.pkg { + + public class MyClass { + ctor public MyClass(); + method public void method2(); + method public void method1(); + method public void method3(); + method public void method4(); + } + + } + """, + sourceFiles = *arrayOf( + java( + """ + package test.pkg; + + public class MyClass { + public void method1() { } + public void method2() { } + public void method3() { } + public native void method4(); + } + """ + ) + ) + ) + } + // TODO: Check method signatures changing incompatibly (look especially out for adding new overloaded // methods and comparator getting confused!) // ..equals on the method items should actually be very useful! diff --git a/src/test/java/com/android/tools/metalava/DocAnalyzerTest.kt b/src/test/java/com/android/tools/metalava/DocAnalyzerTest.kt index 138ed7b43ec33130cc8c62efde2c396150508554..bc85a784ddef4862dbe33b8210f43aeb5de34a58 100644 --- a/src/test/java/com/android/tools/metalava/DocAnalyzerTest.kt +++ b/src/test/java/com/android/tools/metalava/DocAnalyzerTest.kt @@ -1472,6 +1472,13 @@ class DocAnalyzerTest : DriverTest() { println("Ignoring external doc test: JDK not found") return } + + val version = System.getProperty("java.version") + if (!version.startsWith("1.8.")) { + println("Javadoc invocation test does not work on Java 1.9 and later; bootclasspath not supported") + return + } + val javadoc = File(jdkPath, "bin/javadoc") if (!javadoc.isFile) { println("Ignoring external doc test: javadoc not found *or* not running on Linux/OSX") diff --git a/src/test/java/com/android/tools/metalava/DriverTest.kt b/src/test/java/com/android/tools/metalava/DriverTest.kt index 156abd58eb8cc05a482d7402de18b408e719e346..f862f3494177a1bf9ac964a1116617ac84b32c6a 100644 --- a/src/test/java/com/android/tools/metalava/DriverTest.kt +++ b/src/test/java/com/android/tools/metalava/DriverTest.kt @@ -57,7 +57,7 @@ import java.net.URL import kotlin.text.Charsets.UTF_8 const val CHECK_OLD_DOCLAVA_TOO = false -const val CHECK_JDIFF = true +const val CHECK_JDIFF = false const val CHECK_STUB_COMPILATION = false abstract class DriverTest { @@ -97,7 +97,9 @@ abstract class DriverTest { val writer = PrintWriter(sw) if (!com.android.tools.metalava.run(arrayOf(*args), writer, writer)) { val actualFail = cleanupString(sw.toString(), null) - if (expectedFail != actualFail.replace(".", "").trim()) { + if (cleanupString(expectedFail, null).replace(".", "").trim() != + actualFail.replace(".", "").trim() + ) { if (expectedFail == "Aborting: Found compatibility problems with --check-compatibility" && actualFail.startsWith("Aborting: Found compatibility problems checking the ") ) { @@ -106,6 +108,7 @@ abstract class DriverTest { // the signature was passed at the same time // ignore } else { + assertEquals(expectedFail, actualFail) fail(actualFail) } } @@ -191,8 +194,6 @@ abstract class DriverTest { ) protected fun check( - /** The source files to pass to the analyzer */ - vararg sourceFiles: TestFile, /** Any jars to add to the class path */ classpath: Array<TestFile>? = null, /** The API signature content (corresponds to --api) */ @@ -256,6 +257,7 @@ abstract class DriverTest { /** An optional API signature to check the last released removed API's compatibility with */ @Language("TEXT") checkCompatibilityRemovedApiReleased: String? = null, /** An optional API signature to compute nullness migration status from */ + allowCompatibleDifferences: Boolean = true, @Language("TEXT") migrateNullsApi: String? = null, /** An optional Proguard keep file to generate */ @Language("Proguard") proguard: String? = null, @@ -275,6 +277,8 @@ abstract class DriverTest { omitCommonPackages: Boolean = !compatibilityMode, /** Expected output (stdout and stderr combined). If null, don't check. */ expectedOutput: String? = null, + /** Expected fail message and state, if any */ + expectedFail: String? = null, /** List of extra jar files to record annotation coverage from */ coverageJars: Array<TestFile>? = null, /** Optional manifest to load and associate with the codebase */ @@ -337,11 +341,15 @@ abstract class DriverTest { baseline: String? = null, /** Whether to create the baseline if it does not exist. Requires [baseline] to be set. */ updateBaseline: Boolean = false, + /** Merge instead of replacing the baseline */ + mergeBaseline: String? = null, /** * If non null, enable API lint. If non-blank, a codebase where only new APIs not in the codebase * are linted. */ - @Language("TEXT") apiLint: String? = null + @Language("TEXT") apiLint: String? = null, + /** The source files to pass to the analyzer */ + vararg sourceFiles: TestFile ) { // Ensure different API clients don't interfere with each other try { @@ -377,8 +385,8 @@ abstract class DriverTest { } Errors.resetLevels() - /** Expected output if exiting with an error code */ - val expectedFail = if (checkCompatibilityApi != null || + @Suppress("NAME_SHADOWING") + val expectedFail = expectedFail ?: if (checkCompatibilityApi != null || checkCompatibilityApiReleased != null || checkCompatibilityRemovedApiCurrent != null || checkCompatibilityRemovedApiReleased != null @@ -565,7 +573,12 @@ abstract class DriverTest { } val checkCompatibilityArguments = if (checkCompatibilityApiFile != null) { - arrayOf(ARG_CHECK_COMPATIBILITY_API_CURRENT, checkCompatibilityApiFile.path) + val extra: Array<String> = if (allowCompatibleDifferences) { + arrayOf(ARG_ALLOW_COMPATIBLE_DIFFERENCES) + } else { + emptyArray() + } + arrayOf(ARG_CHECK_COMPATIBILITY_API_CURRENT, checkCompatibilityApiFile.path, *extra) } else { emptyArray() } @@ -577,7 +590,12 @@ abstract class DriverTest { } val checkCompatibilityRemovedCurrentArguments = if (checkCompatibilityRemovedApiCurrentFile != null) { - arrayOf(ARG_CHECK_COMPATIBILITY_REMOVED_CURRENT, checkCompatibilityRemovedApiCurrentFile.path) + val extra: Array<String> = if (allowCompatibleDifferences) { + arrayOf(ARG_ALLOW_COMPATIBLE_DIFFERENCES) + } else { + emptyArray() + } + arrayOf(ARG_CHECK_COMPATIBILITY_REMOVED_CURRENT, checkCompatibilityRemovedApiCurrentFile.path, *extra) } else { emptyArray() } @@ -817,10 +835,13 @@ abstract class DriverTest { val baselineArgs = if (baseline != null) { baselineFile = temporaryFolder.newFile("baseline.txt") baselineFile?.writeText(baseline.trimIndent()) - if (!updateBaseline) { + if (!(updateBaseline || mergeBaseline != null)) { arrayOf(ARG_BASELINE, baselineFile.path) } else { - arrayOf(ARG_BASELINE, baselineFile.path, ARG_UPDATE_BASELINE, baselineFile.path) + arrayOf(ARG_BASELINE, + baselineFile.path, + if (mergeBaseline != null) ARG_MERGE_BASELINE else ARG_UPDATE_BASELINE, + baselineFile.path) } } else { emptyArray() @@ -1027,7 +1048,8 @@ abstract class DriverTest { baselineFile.exists() ) val actualText = readFile(baselineFile, stripBlankLines, trim) - assertEquals(stripComments(baseline, stripLineComments = false).trimIndent(), actualText) + val sourceFile = mergeBaseline ?: baseline + assertEquals(stripComments(sourceFile, stripLineComments = false).trimIndent(), actualText) } if (convertFiles.isNotEmpty()) { @@ -1313,7 +1335,7 @@ abstract class DriverTest { } if (CHECK_JDIFF && apiXmlFile != null && convertToJDiff.isNotEmpty()) { - // Parse the XML file with jdiff too + // TODO: Parse the XML file with jdiff too } if (CHECK_OLD_DOCLAVA_TOO && checkDoclava1 && convertToJDiff.isNotEmpty()) { @@ -2353,3 +2375,20 @@ val restrictToSource: TestFile = java( } """ ).indented() + +val visibleForTestingSource: TestFile = java( + """ + package androidx.annotation; + import static java.lang.annotation.RetentionPolicy.CLASS; + import java.lang.annotation.Retention; + @Retention(CLASS) + @SuppressWarnings("WeakerAccess") + public @interface VisibleForTesting { + int otherwise() default PRIVATE; + int PRIVATE = 2; + int PACKAGE_PRIVATE = 3; + int PROTECTED = 4; + int NONE = 5; + } + """ +).indented() diff --git a/src/test/java/com/android/tools/metalava/JDiffXmlTest.kt b/src/test/java/com/android/tools/metalava/JDiffXmlTest.kt index efc9315915b6cf93d7a63a6b67bcad36287e18ff..1a190c2d9190c30edb14fc70cda1657d8d924851 100644 --- a/src/test/java/com/android/tools/metalava/JDiffXmlTest.kt +++ b/src/test/java/com/android/tools/metalava/JDiffXmlTest.kt @@ -711,7 +711,7 @@ class JDiffXmlTest : DriverTest() { <package name="test.pkg" > <interface name="AbstractList" - extends="test.pkg.List<A,B,C>" + extends="test.pkg.List<A, B, C>" abstract="true" static="false" final="false" @@ -720,7 +720,7 @@ class JDiffXmlTest : DriverTest() { > </interface> <interface name="ConcreteList" - extends="test.pkg.AbstractList<D,E,F>" + extends="test.pkg.AbstractList<D, E, F>" abstract="true" static="false" final="false" @@ -910,4 +910,221 @@ class JDiffXmlTest : DriverTest() { """ ) } + + @Test + fun `Test default methods from signature files`() { + // Ensure that we treat not just static but default methods in interfaces as non-abstract + check( + compatibilityMode = true, + checkDoclava1 = true, + format = FileFormat.V1, + signatureSource = """ + package test.pkg { + public abstract interface MethodHandleInfo { + method public static boolean refKindIsField(int); + } + } + """, + apiXml = + """ + <api> + <package name="test.pkg" + > + <interface name="MethodHandleInfo" + abstract="true" + static="false" + final="false" + deprecated="not deprecated" + visibility="public" + > + <method name="refKindIsField" + return="boolean" + abstract="false" + native="false" + synchronized="false" + static="true" + final="false" + deprecated="not deprecated" + visibility="public" + > + <parameter name="null" type="int"> + </parameter> + </method> + </interface> + </package> + </api> + """ + ) + } + + @Test + fun `Test partial signature files`() { + // Partial signature files, such as the system and test files which contain only the + // *diffs* relative to the base API, are tricky: They may for example list just an + // inner class. See 122926140 for a scenario where this happens. + check( + compatibilityMode = true, + checkDoclava1 = true, + format = FileFormat.V1, + signatureSource = """ + // Signature format: 2.0 + package android { + + public static final class Manifest.permission { + field public static final String ACCESS_AMBIENT_LIGHT_STATS = "android.permission.ACCESS_AMBIENT_LIGHT_STATS"; + field public static final String ACCESS_BROADCAST_RADIO = "android.permission.ACCESS_BROADCAST_RADIO"; + field public static final String ACCESS_CACHE_FILESYSTEM = "android.permission.ACCESS_CACHE_FILESYSTEM"; + field public static final String ACCESS_DRM_CERTIFICATES = "android.permission.ACCESS_DRM_CERTIFICATES"; + } + } + """, + apiXml = + """ + <api> + <package name="android" + > + <class name="Manifest.permission" + extends="java.lang.Object" + abstract="false" + static="true" + final="true" + deprecated="not deprecated" + visibility="public" + > + <field name="ACCESS_AMBIENT_LIGHT_STATS" + type="java.lang.String" + transient="false" + volatile="false" + value=""android.permission.ACCESS_AMBIENT_LIGHT_STATS"" + static="true" + final="true" + deprecated="not deprecated" + visibility="public" + > + </field> + <field name="ACCESS_BROADCAST_RADIO" + type="java.lang.String" + transient="false" + volatile="false" + value=""android.permission.ACCESS_BROADCAST_RADIO"" + static="true" + final="true" + deprecated="not deprecated" + visibility="public" + > + </field> + <field name="ACCESS_CACHE_FILESYSTEM" + type="java.lang.String" + transient="false" + volatile="false" + value=""android.permission.ACCESS_CACHE_FILESYSTEM"" + static="true" + final="true" + deprecated="not deprecated" + visibility="public" + > + </field> + <field name="ACCESS_DRM_CERTIFICATES" + type="java.lang.String" + transient="false" + volatile="false" + value=""android.permission.ACCESS_DRM_CERTIFICATES"" + static="true" + final="true" + deprecated="not deprecated" + visibility="public" + > + </field> + </class> + </package> + </api> + """ + ) + } + + @Test + fun `Spaces in type argument lists`() { + // JDiff expects spaces in type argument lists + // Regression test for 123140708 + check( + compatibilityMode = false, + checkDoclava1 = true, + format = FileFormat.V2, + signatureSource = """ + // Signature format: 2.0 + package org.apache.http.impl.conn.tsccm { + @Deprecated public class ConnPoolByRoute extends org.apache.http.impl.conn.tsccm.AbstractConnPool { + field @Deprecated protected final java.util.Map<org.apache.http.conn.routing.HttpRoute,org.apache.http.impl.conn.tsccm.RouteSpecificPool> routeToPool; + field @Deprecated protected java.util.Queue<org.apache.http.impl.conn.tsccm.WaitingThread> waitingThreads; + } + } + package test.pkg { + public abstract class MyClass extends HashMap<String,String> implements Map<String,String> { + field public Map<String,String> map; + } + } + """, + apiXml = + """ + <api> + <package name="org.apache.http.impl.conn.tsccm" + > + <class name="ConnPoolByRoute" + extends="org.apache.http.impl.conn.tsccm.AbstractConnPool" + abstract="false" + static="false" + final="false" + deprecated="deprecated" + visibility="public" + > + <field name="routeToPool" + type="java.util.Map<org.apache.http.conn.routing.HttpRoute, org.apache.http.impl.conn.tsccm.RouteSpecificPool>" + transient="false" + volatile="false" + static="false" + final="true" + deprecated="deprecated" + visibility="protected" + > + </field> + <field name="waitingThreads" + type="java.util.Queue<org.apache.http.impl.conn.tsccm.WaitingThread>" + transient="false" + volatile="false" + static="false" + final="false" + deprecated="deprecated" + visibility="protected" + > + </field> + </class> + </package> + <package name="test.pkg" + > + <class name="MyClass" + extends="java.lang.HashMap<String, String>" + abstract="true" + static="false" + final="false" + deprecated="not deprecated" + visibility="public" + > + <implements name="java.lang.Map<String, String>"> + </implements> + <field name="map" + type="java.lang.Map<String, String>" + transient="false" + volatile="false" + static="false" + final="false" + deprecated="not deprecated" + visibility="public" + > + </field> + </class> + </package> + </api> + """ + ) + } } \ No newline at end of file diff --git a/src/test/java/com/android/tools/metalava/OptionsTest.kt b/src/test/java/com/android/tools/metalava/OptionsTest.kt index 6a7deb56784395b0e87fdba29a8c87cd513a1176..7c61c9eac59ae290eb06e303ba54052f58d7753f 100644 --- a/src/test/java/com/android/tools/metalava/OptionsTest.kt +++ b/src/test/java/com/android/tools/metalava/OptionsTest.kt @@ -104,6 +104,14 @@ API sources: signature file as well --java-source <level> Sets the source level for Java source files; default is 1.8. +--stub-packages <path> List of packages (separated by : which will be + used to filter out irrelevant code. If + specified, only code in these packages will be + included in signature files, stubs, etc. (This + is not limited to just the stubs; the name is + historical.) You can also use ".*" at the end to + match subpackages, so `foo.*` will match both + `foo` and `foo.bar`. Documentation: --public Only include elements that are public @@ -226,6 +234,13 @@ Diffs and Checks: updated baseline is written to the given file; otherwise the original source baseline file is updated. +--merge-baseline [file] Like --update-baseline, but instead of always + replacing entries in the baseline, it will merge + the existing baseline with the new baseline. + This is useful if metalava runs multiple times + on the same source tree with different flags at + different times, such as occasionally with + --api-lint. JDiff: --api-xml <file> Like --api, but emits the API in the JDiff XML diff --git a/src/test/java/com/android/tools/metalava/PackageFilterTest.kt b/src/test/java/com/android/tools/metalava/PackageFilterTest.kt index e092d78a8b40c9aeae9ab1e512f6b173f48fa2b4..41150bd27f11e169b8b12b9b45688cec880a4149 100644 --- a/src/test/java/com/android/tools/metalava/PackageFilterTest.kt +++ b/src/test/java/com/android/tools/metalava/PackageFilterTest.kt @@ -21,12 +21,48 @@ import org.junit.Test class PackageFilterTest { @Test - fun test() { - val filter = PackageFilter(mutableListOf("foo.bar", "bar.baz")) + fun testExact() { + val filter = PackageFilter() + filter.addPackages("foo.bar:bar.baz") assertThat(filter.matches("foo.bar")).isTrue() assertThat(filter.matches("bar.baz")).isTrue() - assertThat(filter.matches("foo.bar.baz")).isTrue() + assertThat(filter.matches("foo.bar.baz")).isFalse() + assertThat(filter.matches("foo")).isFalse() assertThat(filter.matches("foo.barf")).isFalse() + } + + @Test + fun testWildcard() { + val filter = PackageFilter() + filter.addPackages("foo.bar*:bar.baz.*") + assertThat(filter.matches("foo.bar")).isTrue() + assertThat(filter.matches("foo.bars")).isTrue() + assertThat(filter.matches("foo.bar.baz")).isTrue() + assertThat(filter.matches("bar.baz")).isTrue() // different from doclava behavior + assertThat(filter.matches("bar.bazz")).isFalse() + assertThat(filter.matches("foo.bar.baz")).isTrue() + assertThat(filter.matches("foo")).isFalse() + } + + @Test + fun testBoth() { + val filter = PackageFilter() + filter.addPackages("foo.bar:foo.bar.*") + assertThat(filter.matches("foo.bar")).isTrue() + assertThat(filter.matches("foo.bar.sub")).isTrue() + assertThat(filter.matches("foo.bar.sub.sub")).isTrue() + assertThat(filter.matches("foo.bars")).isFalse() + assertThat(filter.matches("foo")).isFalse() + } + + @Test + fun testImplicit() { + val filter = PackageFilter() + filter.addPackages("foo.bar.*") + assertThat(filter.matches("foo.bar")).isTrue() + assertThat(filter.matches("foo.bar.sub")).isTrue() + assertThat(filter.matches("foo.bar.sub.sub")).isTrue() + assertThat(filter.matches("foo.bars")).isFalse() assertThat(filter.matches("foo")).isFalse() } } \ No newline at end of file diff --git a/src/test/java/com/android/tools/metalava/ShowAnnotationTest.kt b/src/test/java/com/android/tools/metalava/ShowAnnotationTest.kt index c2e6832ab1a77423b6e102ce0096756d0bac6d31..67556a974d3b93fbf2c3aeba3141b949509f76bc 100644 --- a/src/test/java/com/android/tools/metalava/ShowAnnotationTest.kt +++ b/src/test/java/com/android/tools/metalava/ShowAnnotationTest.kt @@ -47,7 +47,6 @@ class ShowAnnotationTest : DriverTest() { ), extraArguments = arrayOf( - ARG_ERROR, "UnhiddenSystemApi", ARG_HIDE_PACKAGE, "android.annotation", ARG_HIDE_PACKAGE, "android.support.annotation" ), @@ -106,7 +105,6 @@ class ShowAnnotationTest : DriverTest() { ), extraArguments = arrayOf( - ARG_ERROR, "UnhiddenSystemApi", ARG_HIDE_PACKAGE, "android.annotation", ARG_HIDE_PACKAGE, "android.support.annotation" ), @@ -255,4 +253,61 @@ class ShowAnnotationTest : DriverTest() { ) ) } + + @Test + fun `No UnhiddenSystemApi warning for --show-single-annotations`() { + check( + checkDoclava1 = true, + warnings = "", + sourceFiles = *arrayOf( + java( + """ + package test.pkg; + import android.annotation.SystemApi; + public class Foo { + public void method1() { } + + /** + * @hide Only for use by WebViewProvider implementations + */ + @SystemApi + public void method2() { } + + /** + * @hide Always hidden + */ + public void method3() { } + + @SystemApi + public void method4() { } + + } + """ + ), + java( + """ + package foo.bar; + public class Bar { + } + """ + ), + systemApiSource + ), + + extraArguments = arrayOf( + ARG_SHOW_SINGLE_ANNOTATION, "android.annotation.SystemApi", + ARG_HIDE_PACKAGE, "android.annotation", + ARG_HIDE_PACKAGE, "android.support.annotation" + ), + + api = """ + package test.pkg { + public class Foo { + method public void method2(); + method public void method4(); + } + } + """ + ) + } } \ No newline at end of file diff --git a/src/test/java/com/android/tools/metalava/StubsTest.kt b/src/test/java/com/android/tools/metalava/StubsTest.kt index e48c39935b30a4a23632d0976af086d24401d473..807bd9193c3fa65cbbfa4a2cf1c0294990fd9cac 100644 --- a/src/test/java/com/android/tools/metalava/StubsTest.kt +++ b/src/test/java/com/android/tools/metalava/StubsTest.kt @@ -43,7 +43,7 @@ class StubsTest : DriverTest() { vararg sourceFiles: TestFile ) { check( - *sourceFiles, + sourceFiles = *sourceFiles, showAnnotations = showAnnotations, stubs = arrayOf(source), compatibilityMode = compatibilityMode, @@ -3647,10 +3647,323 @@ class StubsTest : DriverTest() { ) } -// TODO: Add in some type variables in method signatures and constructors! -// TODO: Test what happens when a class extends a hidden extends a public in separate packages, -// and the hidden has a @hide constructor so the stub in the leaf class doesn't compile -- I should -// check for this and fail build. + @Test + fun `Include package private classes referenced from public API`() { + // Real world example: android.net.http.Connection in apache-http referenced from RequestHandle + check( + compatibilityMode = false, + warnings = """ + src/test/pkg/PublicApi.java:4: error: Class test.pkg.HiddenType is not public but was referenced (as return type) from public method test.pkg.PublicApi.getHiddenType() [ReferencesHidden] + src/test/pkg/PublicApi.java:5: error: Class test.pkg.HiddenType4 is hidden but was referenced (as return type) from public method test.pkg.PublicApi.getHiddenType4() [ReferencesHidden] + src/test/pkg/PublicApi.java:5: warning: Method test.pkg.PublicApi.getHiddenType4 returns unavailable type HiddenType4 [UnavailableSymbol] + src/test/pkg/PublicApi.java:4: warning: Method test.pkg.PublicApi.getHiddenType() references hidden type test.pkg.HiddenType. [HiddenTypeParameter] + src/test/pkg/PublicApi.java:5: warning: Method test.pkg.PublicApi.getHiddenType4() references hidden type test.pkg.HiddenType4. [HiddenTypeParameter] + """, + sourceFiles = *arrayOf( + java( + """ + package test.pkg; + + public class PublicApi { + public HiddenType getHiddenType() { return null; } + public HiddenType4 getHiddenType4() { return null; } + } + """ + ), + java( + """ + package test.pkg; + + public class PublicInterface { + } + """ + ), + java( + """ + package test.pkg; + + // Class exposed via public api above + final class HiddenType extends HiddenType2 implements HiddenType3, PublicInterface { + HiddenType(int i1, int i2) { } + public HiddenType2 getHiddenType2() { return null; } + public int field; + @Override public String toString() { return "hello"; } + } + """ + ), + java( + """ + package test.pkg; + + /** @hide */ + public class HiddenType4 { + void foo(); + } + """ + ), + java( + """ + package test.pkg; + + // Class not exposed; only referenced from HiddenType + class HiddenType2 { + HiddenType2(float f) { } + } + """ + ), + java( + """ + package test.pkg; + + // Class not exposed; only referenced from HiddenType + interface HiddenType3 { + } + """ + ) + ), + api = """ + package test.pkg { + public class PublicApi { + ctor public PublicApi(); + method public test.pkg.HiddenType getHiddenType(); + method public test.pkg.HiddenType4 getHiddenType4(); + } + public class PublicInterface { + ctor public PublicInterface(); + } + } + """, + stubs = arrayOf( + """ + package test.pkg; + @SuppressWarnings({"unchecked", "deprecation", "all"}) + public class PublicApi { + public PublicApi() { throw new RuntimeException("Stub!"); } + public test.pkg.HiddenType getHiddenType() { throw new RuntimeException("Stub!"); } + public test.pkg.HiddenType4 getHiddenType4() { throw new RuntimeException("Stub!"); } + } + """, + """ + package test.pkg; + @SuppressWarnings({"unchecked", "deprecation", "all"}) + public class PublicInterface { + public PublicInterface() { throw new RuntimeException("Stub!"); } + } + """, + """ + package test.pkg; + @SuppressWarnings({"unchecked", "deprecation", "all"}) + final class HiddenType { + } + """, + """ + package test.pkg; + /** @hide */ + @SuppressWarnings({"unchecked", "deprecation", "all"}) + public class HiddenType4 { + } + """ + ) + ) + } + + @Test + fun `Include hidden inner classes referenced from public API`() { + // Real world example: hidden android.car.vms.VmsOperationRecorder.Writer in android.car-system-stubs + // referenced from outer class constructor + check( + compatibilityMode = false, + warnings = """ + src/test/pkg/PublicApi.java:4: error: Class test.pkg.PublicApi.HiddenInner is hidden but was referenced (as parameter type) from public parameter inner in test.pkg.PublicApi(test.pkg.PublicApi.HiddenInner inner) [ReferencesHidden] + src/test/pkg/PublicApi.java:4: warning: Parameter inner references hidden type test.pkg.PublicApi.HiddenInner. [HiddenTypeParameter] + """, + sourceFiles = *arrayOf( + java( + """ + package test.pkg; + + public class PublicApi { + public PublicApi(HiddenInner inner) { } + /** @hide */ + public static class HiddenInner { + public void someHiddenMethod(); // should not be in stub + } + } + """ + ) + ), + api = """ + package test.pkg { + public class PublicApi { + ctor public PublicApi(test.pkg.PublicApi.HiddenInner); + } + } + """, + stubs = arrayOf( + """ + package test.pkg; + @SuppressWarnings({"unchecked", "deprecation", "all"}) + public class PublicApi { + public PublicApi(test.pkg.PublicApi.HiddenInner inner) { throw new RuntimeException("Stub!"); } + /** @hide */ + @SuppressWarnings({"unchecked", "deprecation", "all"}) + public static class HiddenInner { + } + } + """ + ) + ) + } + + @Test + fun `Use type argument in constructor cast`() { + check( + compatibilityMode = false, + sourceFiles = *arrayOf( + java( + """ + package test.pkg; + /** @deprecated */ + @Deprecated + public class BasicPoolEntryRef extends WeakRef<BasicPoolEntry> { + public BasicPoolEntryRef(BasicPoolEntry entry) { + super(entry); + } + } + """ + ), + java( + """ + package test.pkg; + + public class WeakRef<T> { + public WeakRef(T foo) { + } + // need to have more than one constructor to trigger casts in stubs + public WeakRef(T foo, int size) { + } + } + """ + ), + java( + """ + package test.pkg; + + public class BasicPoolEntry { + } + """ + ) + ), + api = """ + package test.pkg { + public class BasicPoolEntry { + ctor public BasicPoolEntry(); + } + @Deprecated public class BasicPoolEntryRef extends test.pkg.WeakRef<test.pkg.BasicPoolEntry> { + ctor @Deprecated public BasicPoolEntryRef(test.pkg.BasicPoolEntry); + } + public class WeakRef<T> { + ctor public WeakRef(T); + ctor public WeakRef(T, int); + } + } + """, + stubs = arrayOf( + """ + package test.pkg; + /** @deprecated */ + @SuppressWarnings({"unchecked", "deprecation", "all"}) + @Deprecated + public class BasicPoolEntryRef extends test.pkg.WeakRef<test.pkg.BasicPoolEntry> { + @Deprecated + public BasicPoolEntryRef(test.pkg.BasicPoolEntry entry) { super((test.pkg.BasicPoolEntry)null); throw new RuntimeException("Stub!"); } + } + """ + ) + ) + } + + @Test + fun `Regression test for 116777737`() { + // Regression test for 116777737: Stub generation broken for Bouncycastle + // """ + // It appears as though metalava does not handle the case where: + // 1) class Alpha extends Beta<Orange>. + // 2) class Beta<T> extends Charlie<T>. + // 3) class Beta is hidden. + // + // It should result in a stub where Alpha extends Charlie<Orange> but + // instead results in a stub where Alpha extends Charlie<T>, so the + // type substitution of Orange for T is lost. + // """ + check( + compatibilityMode = false, + warnings = "src/test/pkg/Alpha.java:2: warning: Public class test.pkg.Alpha stripped of unavailable superclass test.pkg.Beta [HiddenSuperclass]", + sourceFiles = *arrayOf( + java( + """ + package test.pkg; + public class Orange { + private Orange() { } + } + """ + ), + java( + """ + package test.pkg; + public class Alpha extends Beta<Orange> { + private Alpha() { } + } + """ + ), + java( + """ + package test.pkg; + /** @hide */ + public class Beta<T> extends Charlie<T> { + private Beta() { } + } + """ + ), + java( + """ + package test.pkg; + public class Charlie<T> { + private Charlie() { } + } + """ + ) + ), + api = """ + package test.pkg { + public class Alpha extends test.pkg.Charlie<test.pkg.Orange> { + } + public class Charlie<T> { + } + public class Orange { + } + } + """, + stubs = arrayOf( + """ + package test.pkg; + @SuppressWarnings({"unchecked", "deprecation", "all"}) + public class Orange { + Orange() { throw new RuntimeException("Stub!"); } + } + """, + """ + package test.pkg; + @SuppressWarnings({"unchecked", "deprecation", "all"}) + public class Alpha extends test.pkg.Charlie<test.pkg.Orange> { + Alpha() { throw new RuntimeException("Stub!"); } + } + """ + ) + ) + } -// TODO: Test -stubPackages + // TODO: Test what happens when a class extends a hidden extends a public in separate packages, + // and the hidden has a @hide constructor so the stub in the leaf class doesn't compile -- I should + // check for this and fail build. } \ No newline at end of file diff --git a/src/test/java/com/android/tools/metalava/model/psi/JavadocTest.kt b/src/test/java/com/android/tools/metalava/model/psi/JavadocTest.kt index 3a6f3e1bf4fa1ee6b5a749f8fd1b1d51cf5a3717..b7de34f6c635fd36d1297bf14071607cc326848a 100644 --- a/src/test/java/com/android/tools/metalava/model/psi/JavadocTest.kt +++ b/src/test/java/com/android/tools/metalava/model/psi/JavadocTest.kt @@ -37,7 +37,7 @@ class JavadocTest : DriverTest() { vararg sourceFiles: TestFile ) { check( - *sourceFiles, + sourceFiles = *sourceFiles, showAnnotations = showAnnotations, stubs = arrayOf(source), compatibilityMode = compatibilityMode, @@ -922,4 +922,92 @@ class JavadocTest : DriverTest() { """ ) } + + @Test + fun `Javadoc link to innerclass constructor`() { + // Regression test for + // 119190588: Javadoc link tag to constructor of static inner class not working + // See also https://bugs.openjdk.java.net/browse/JDK-8031625 + check( + sourceFiles = *arrayOf( + java( + """ + package android.view; + import android.graphics.Insets; + + + public final class WindowInsets { + /** + * Returns a copy of this WindowInsets with selected system window insets replaced + * with new values. + * + * @param left New left inset in pixels + * @param top New top inset in pixels + * @param right New right inset in pixels + * @param bottom New bottom inset in pixels + * @return A modified copy of this WindowInsets + * @deprecated use {@link Builder#Builder(WindowInsets)} with + * {@link Builder#setSystemWindowInsets(Insets)} instead. + */ + @Deprecated + public WindowInsets replaceSystemWindowInsets(int left, int top, int right, int bottom) { + + } + + public static class Builder { + public Builder() { + } + + public Builder(WindowInsets insets) { + } + + public Builder setSystemWindowInsets(Insets systemWindowInsets) { + return this; + } + } + } + """ + ) + ), + checkDoclava1 = false, + docStubs = true, + // You would *think* the right link to the constructor inner class would be + // {@link android.view.WindowInsets.Builder#Builder(android.view.WindowInsets) + // but that does not work; per https://bugs.openjdk.java.net/browse/JDK-8031625 + // we instead have to use + // {@link android.view.WindowInsets.Builder#android.view.WindowInsets.Builder(android.view.WindowInsets) + // to get javadoc to turn it into a valid link. (In the platform builds there's + // also doclava's LinkReference class which does some validation; it's unclear + // whether it works.) + stubs = arrayOf( + """ + package android.view; + @SuppressWarnings({"unchecked", "deprecation", "all"}) + public final class WindowInsets { + public WindowInsets() { throw new RuntimeException("Stub!"); } + /** + * Returns a copy of this WindowInsets with selected system window insets replaced + * with new values. + * + * @param left New left inset in pixels + * @param top New top inset in pixels + * @param right New right inset in pixels + * @param bottom New bottom inset in pixels + * @return A modified copy of this WindowInsets + * @deprecated use {@link android.view.WindowInsets.Builder#WindowInsets.Builder(android.view.WindowInsets) Builder#Builder(WindowInsets)} with + * {@link android.view.WindowInsets.Builder#setSystemWindowInsets(Insets) Builder#setSystemWindowInsets(Insets)} instead. + */ + @Deprecated + public android.view.WindowInsets replaceSystemWindowInsets(int left, int top, int right, int bottom) { throw new RuntimeException("Stub!"); } + @SuppressWarnings({"unchecked", "deprecation", "all"}) + public static class Builder { + public Builder() { throw new RuntimeException("Stub!"); } + public Builder(android.view.WindowInsets insets) { throw new RuntimeException("Stub!"); } + public android.view.WindowInsets.Builder setSystemWindowInsets(Insets systemWindowInsets) { throw new RuntimeException("Stub!"); } + } + } + """ + ) + ) + } } \ No newline at end of file