Simplify constructor selection in stubsandroid10-gsi

Constructor selection was previously done on the unfiltered constructor
list. This made the code more complex and could result in constructors
that reference excluded types being present in the generated stubs. By
selecting from the already filtered constructors the code can be
greatly simplified and the resulting stubs will not reference excludeed
types.

It does change the behavior of some of the tests:
* CoreApiTest
  * 'Hidden with package javadoc and hiding default constructor explicitly'
    Removes the /**@hide*/ comment that was copied over from the
	original source. That is because the constructor is no longer copied
	over and instead was created.
* StubsTest
  * A number of tests were affected as instead of reusing a hidden
    constructor with parameters and throws from the source it creates a
	new no args constructor.

This should have no adverse effects because in the case when a
constructor that referenced excluded types was output it could never
actually have been used because it was marked as package private. So,
it is safe to replace it with a default package private constructor
as it did previously when it could not find a suitable constructor.

Bug: 141219826
Test: ./gradlew test + m checkbuild + manual inspection of affected classes
Change-Id: I62be87d47124d135a50a4c0b98f55c9d63dc1db5
(cherry picked from commit 09094fc5e566a380b7aa1a4c3948ac66cebc0aba)

5 files changed, 32 insertions, 99 deletions

diff --git a/src/main/java/com/android/tools/metalava/ApiAnalyzer.kt b/src/main/java/com/android/tools/metalava/ApiAnalyzer.kt
index f9ba1d0e..aed226ac 100644
--- a/src/main/java/com/android/tools/metalava/ApiAnalyzer.kt
+++ b/src/main/java/com/android/tools/metalava/ApiAnalyzer.kt
@@ -191,99 +191,50 @@ class ApiAnalyzer(
         }
 
         // Find default constructor, if one doesn't exist
-        if (cls.constructors().isNotEmpty()) {
-            val constructors = cls.constructors()
-            for (constructor in constructors) {
-                if (constructor.parameters().isEmpty() && constructor.isPublic && !constructor.hidden) {
-                    cls.stubConstructor = constructor
-                    return
-                }
-            }
+        val allConstructors = cls.constructors()
+        if (allConstructors.isNotEmpty()) {
 
+            // Try and use a publicly accessible constructor first.
+            val constructors = cls.filteredConstructors(filter).toList()
             if (!constructors.isEmpty()) {
-                // Try to pick the constructor, sorting first by "matches filter", then by
-                // uses available types, then by fewest throwables, then fewest parameters,
+                // Try to pick the constructor, select first by fewest throwables, then fewest parameters,
                 // then based on order in listFilter.test(cls)
-                val first = constructors.first()
-                val best =
-                    if (constructors.size > 1) {
-                        constructors.foldRight(first) { current, next -> pickBest(current, next, filter) }
-                    } else {
-                        first
-                    }
+                cls.stubConstructor = constructors.reduce {first, second -> pickBest(first, second)}
+                return
+            }
 
-                if (cls.filteredConstructors(filter).contains(best)) {
-                    cls.stubConstructor = best
-                    return
-                }
+            // No accessible constructors are available so one will have to be created, either a private constructor to
+            // prevent instances of the class from being created, or a package private constructor for use by subclasses
+            // in the package to use. Subclasses outside the package would need a protected or public constructor which
+            // would already be part of the API so should have dropped out above.
+            //
+            // The visibility levels on the constructors from the source can give a clue as to what is required. e.g.
+            // if all constructors are private then it is ok for the generated constructor to be private, otherwise it
+            // should be package private.
+            val allPrivate = allConstructors.asSequence()
+                .map { it.isPrivate }
+                .reduce {v1, v2 -> v1 and v2}
 
-                if (!referencesExcludedType(best, filter)) {
-                    cls.stubConstructor = best
-                    if (!best.isPrivate) {
-                        best.mutableModifiers().setVisibilityLevel(VisibilityLevel.PACKAGE_PRIVATE)
-                        best.hidden = false
-                    }
-                    best.docOnly = false
-                    return
-                }
-            }
+            val visibilityLevel = if (allPrivate) VisibilityLevel.PRIVATE else VisibilityLevel.PACKAGE_PRIVATE
 
             // No constructors, yet somebody extends this (or private constructor): we have to invent one, such that
             // subclasses can dispatch to it in the stub files etc
             cls.stubConstructor = cls.createDefaultConstructor().also {
-                it.mutableModifiers().setVisibilityLevel(VisibilityLevel.PACKAGE_PRIVATE)
+                it.mutableModifiers().setVisibilityLevel(visibilityLevel)
                 it.hidden = false
                 it.superConstructor = superClass?.stubConstructor
             }
         }
     }
 
-    private fun referencesExcludedType(constructor: ConstructorItem, filter: Predicate<Item>): Boolean {
-        // Checks parameter types and throws types
-        for (parameter in constructor.parameters()) {
-            val type = parameter.type()
-            if (type.referencesExcludedType(filter)) {
-                return true
-            }
-        }
-        for (cls in constructor.throwsTypes()) {
-            if (!filter.test(cls)) {
-                return true
-            }
-        }
-
-        return false
-    }
-
     // TODO: Annotation test: @ParameterName, if present, must be supplied on *all* the arguments!
     // Warn about @DefaultValue("null"); they probably meant @DefaultNull
     // Supplying default parameter in override is not allowed!
 
     private fun pickBest(
         current: ConstructorItem,
-        next: ConstructorItem,
-        filter: Predicate<Item>
+        next: ConstructorItem
     ): ConstructorItem {
-        val currentMatchesFilter = filter.test(current)
-        val nextMatchesFilter = filter.test(next)
-        if (currentMatchesFilter != nextMatchesFilter) {
-            return if (currentMatchesFilter) {
-                current
-            } else {
-                next
-            }
-        }
-
-        val currentUsesAvailableTypes = !referencesExcludedType(current, filter)
-        val nextUsesAvailableTypes = !referencesExcludedType(next, filter)
-        if (currentUsesAvailableTypes != nextUsesAvailableTypes) {
-            return if (currentUsesAvailableTypes) {
-                current
-            } else {
-                next
-            }
-        }
-
         val currentThrowsCount = current.throwsTypes().size
         val nextThrowsCount = next.throwsTypes().size
 
diff --git a/src/main/java/com/android/tools/metalava/StubWriter.kt b/src/main/java/com/android/tools/metalava/StubWriter.kt
index fa966901..878be519 100644
--- a/src/main/java/com/android/tools/metalava/StubWriter.kt
+++ b/src/main/java/com/android/tools/metalava/StubWriter.kt
@@ -496,10 +496,9 @@ class StubWriter(
     private fun generateMissingConstructors(cls: ClassItem) {
         val clsStubConstructor = cls.stubConstructor
         val constructors = cls.filteredConstructors(filterEmit)
+        // If the default stub constructor is not publicly visible then it won't be output during the normal visiting
+        // so visit it specially to ensure that it is output.
         if (clsStubConstructor != null && !constructors.contains(clsStubConstructor)) {
-            if (!clsStubConstructor.isPrivate) {
-                clsStubConstructor.mutableModifiers().setVisibilityLevel(VisibilityLevel.PACKAGE_PRIVATE)
-            }
             visitConstructor(clsStubConstructor)
             return
         }
diff --git a/src/main/java/com/android/tools/metalava/model/TypeItem.kt b/src/main/java/com/android/tools/metalava/model/TypeItem.kt
index ca9266c2..37d3df42 100644
--- a/src/main/java/com/android/tools/metalava/model/TypeItem.kt
+++ b/src/main/java/com/android/tools/metalava/model/TypeItem.kt
@@ -134,21 +134,6 @@ interface TypeItem {
         }
     }
 
-    /** Returns true if this type references a type not matched by the given predicate */
-    fun referencesExcludedType(filter: Predicate<Item>): Boolean {
-        if (primitive) {
-            return false
-        }
-
-        for (item in typeArgumentClasses()) {
-            if (!filter.test(item)) {
-                return true
-            }
-        }
-
-        return false
-    }
-
     fun defaultValueString(): String = defaultValue()?.toString() ?: "null"
 
     fun hasTypeArguments(): Boolean = toTypeString().contains("<")
diff --git a/src/test/java/com/android/tools/metalava/CoreApiTest.kt b/src/test/java/com/android/tools/metalava/CoreApiTest.kt
index 0748c145..ceb857ae 100644
--- a/src/test/java/com/android/tools/metalava/CoreApiTest.kt
+++ b/src/test/java/com/android/tools/metalava/CoreApiTest.kt
@@ -199,7 +199,6 @@ class CoreApiTest : DriverTest() {
                  */
                 @SuppressWarnings({"unchecked", "deprecation", "all"})
                 public class Exposed {
-                /** @hide */
                 Exposed() { throw new RuntimeException("Stub!"); }
                 public void exposed() { throw new RuntimeException("Stub!"); }
                 }
diff --git a/src/test/java/com/android/tools/metalava/StubsTest.kt b/src/test/java/com/android/tools/metalava/StubsTest.kt
index 158ad2a0..c2f70481 100644
--- a/src/test/java/com/android/tools/metalava/StubsTest.kt
+++ b/src/test/java/com/android/tools/metalava/StubsTest.kt
@@ -1544,11 +1544,11 @@ class StubsTest : DriverTest() {
                     }
                     @SuppressWarnings({"unchecked", "deprecation", "all"})
                     public class Child2 extends test.pkg.Constructors.Parent {
-                    Child2(java.lang.String s) { super(null, 0, 0, false, (short)0); throw new RuntimeException("Stub!"); }
+                    Child2() { super(null, 0, 0, false, (short)0); throw new RuntimeException("Stub!"); }
                     }
                     @SuppressWarnings({"unchecked", "deprecation", "all"})
                     public class Child3 extends test.pkg.Constructors.Child2 {
-                    private Child3(java.lang.String s) { super(null); throw new RuntimeException("Stub!"); }
+                    private Child3() { throw new RuntimeException("Stub!"); }
                     }
                     @SuppressWarnings({"unchecked", "deprecation", "all"})
                     public class Child4 extends test.pkg.Constructors.Parent {
@@ -1626,11 +1626,11 @@ class StubsTest : DriverTest() {
                     }
                     @SuppressWarnings({"unchecked", "deprecation", "all"})
                     public class Child2 extends test.pkg.Constructors.Parent {
-                    Child2(java.lang.String s) { super(null, 0, 0, false, (short)0); throw new RuntimeException("Stub!"); }
+                    Child2() { super(null, 0, 0, false, (short)0); throw new RuntimeException("Stub!"); }
                     }
                     @SuppressWarnings({"unchecked", "deprecation", "all"})
                     public class Child3 extends test.pkg.Constructors.Child2 {
-                    private Child3(java.lang.String s) { super(null); throw new RuntimeException("Stub!"); }
+                    private Child3() { throw new RuntimeException("Stub!"); }
                     }
                     @SuppressWarnings({"unchecked", "deprecation", "all"})
                     public class Child4 extends test.pkg.Constructors.Parent {
@@ -1972,29 +1972,28 @@ class StubsTest : DriverTest() {
                 package test.pkg;
                 @SuppressWarnings({"unchecked", "deprecation", "all"})
                 public class MyClass1 {
-                MyClass1(int myVar) { throw new RuntimeException("Stub!"); }
+                MyClass1() { throw new RuntimeException("Stub!"); }
                 }
                 """,
                 """
                 package test.pkg;
                 @SuppressWarnings({"unchecked", "deprecation", "all"})
                 public class MySubClass1 extends test.pkg.MyClass1 {
-                MySubClass1(int myVar) throws java.io.IOException { super(0); throw new RuntimeException("Stub!"); }
+                MySubClass1() { throw new RuntimeException("Stub!"); }
                 }
                 """,
                 """
                 package test.pkg;
                 @SuppressWarnings({"unchecked", "deprecation", "all"})
                 public class MyClass2 {
-                /** @hide */
-                MyClass2(int myVar) { throw new RuntimeException("Stub!"); }
+                MyClass2() { throw new RuntimeException("Stub!"); }
                 }
                 """,
                 """
                 package test.pkg;
                 @SuppressWarnings({"unchecked", "deprecation", "all"})
                 public class MySubClass2 extends test.pkg.MyClass2 {
-                public MySubClass2() { super(0); throw new RuntimeException("Stub!"); }
+                public MySubClass2() { throw new RuntimeException("Stub!"); }
                 }
                 """
             ),