From 0402e66f9cdcbefe7ef680b3b5d3db7ad6cd8338 Mon Sep 17 00:00:00 2001
From: Tor Norbye <tnorbye@google.com>
Date: Thu, 10 Jan 2019 10:10:42 -0800
Subject: [PATCH] Add support for merging baselines
From --help:
--update-baseline [file] Rewrite the existing baseline file with the
current set of warnings. 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.
--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.
Test: Unit test added
Change-Id: If53a3df2d2a4b973de63babda981666c78d47133
---
README.md | 10 +++
.../com/android/tools/metalava/Baseline.kt | 55 ++++++++-------
.../com/android/tools/metalava/Options.kt | 22 ++++--
.../android/tools/metalava/BaselineTest.kt | 68 +++++++++++++++++++
.../com/android/tools/metalava/DriverTest.kt | 18 +++--
.../com/android/tools/metalava/OptionsTest.kt | 7 ++
.../com/android/tools/metalava/StubsTest.kt | 2 +-
.../tools/metalava/model/psi/JavadocTest.kt | 2 +-
8 files changed, 144 insertions(+), 40 deletions(-)
diff --git a/README.md b/README.md
index ed88779..2034bcc 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/Baseline.kt b/src/main/java/com/android/tools/metalava/Baseline.kt
index 6a86591..335cee4 100644
--- a/src/main/java/com/android/tools/metalava/Baseline.kt
+++ b/src/main/java/com/android/tools/metalava/Baseline.kt
@@ -41,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
@@ -54,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()
}
@@ -180,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/Options.kt b/src/main/java/com/android/tools/metalava/Options.kt
index 497bf5f..b83105d 100644
--- a/src/main/java/com/android/tools/metalava/Options.kt
+++ b/src/main/java/com/android/tools/metalava/Options.kt
@@ -138,6 +138,7 @@ 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"
@@ -550,6 +551,7 @@ class Options(
var currentJar: File? = null
var updateBaselineFile: File? = null
var baselineFile: File? = null
+ var mergeBaseline = false
var index = 0
while (index < args.size) {
@@ -720,8 +722,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("-")) {
@@ -1294,18 +1297,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 (System.getenv("ANDROID_BUILD_TOP") != null)
"// 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()
@@ -1828,6 +1832,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/test/java/com/android/tools/metalava/BaselineTest.kt b/src/test/java/com/android/tools/metalava/BaselineTest.kt
index c836c69..955019e 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/DriverTest.kt b/src/test/java/com/android/tools/metalava/DriverTest.kt
index 50c861a..a30654c 100644
--- a/src/test/java/com/android/tools/metalava/DriverTest.kt
+++ b/src/test/java/com/android/tools/metalava/DriverTest.kt
@@ -191,8 +191,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) */
@@ -337,11 +335,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 {
@@ -817,10 +819,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 +1032,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()) {
diff --git a/src/test/java/com/android/tools/metalava/OptionsTest.kt b/src/test/java/com/android/tools/metalava/OptionsTest.kt
index e006c9a..7c61c9e 100644
--- a/src/test/java/com/android/tools/metalava/OptionsTest.kt
+++ b/src/test/java/com/android/tools/metalava/OptionsTest.kt
@@ -234,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/StubsTest.kt b/src/test/java/com/android/tools/metalava/StubsTest.kt
index ee9f0d1..c5439aa 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,
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 ea7d702..b7de34f 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,
--
GitLab