Merge "analyze_bcpf: Explain why a package is split/single"

This commit is contained in:
Paul Duffin 2022-04-05 11:41:17 +00:00 committed by Gerrit Code Review
commit 3f19ffb711
2 changed files with 164 additions and 24 deletions

View file

@ -318,6 +318,28 @@ class HiddenApiPropertyChange:
return True
@dataclasses.dataclass()
class PackagePropertyReason:
"""Provides the reasons why a package was added to a specific property.
A split package is one that contains classes from the bootclasspath_fragment
and other bootclasspath modules. So, for a split package this contains the
corresponding lists of classes.
A single package is one that contains classes sub-packages from the
For a split package this contains a list of classes in that package that are
provided by the bootclasspath_fragment and a list of classes
"""
# The list of classes/sub-packages that is provided by the
# bootclasspath_fragment.
bcpf: typing.List[str]
# The list of classes/sub-packages that is provided by other modules on the
# bootclasspath.
other: typing.List[str]
@dataclasses.dataclass()
class Result:
"""Encapsulates the result of the analysis."""
@ -325,6 +347,20 @@ class Result:
# The diffs in the flags.
diffs: typing.Optional[FlagDiffs] = None
# A map from package name to the reason why it belongs in the
# split_packages property.
split_packages: typing.Dict[str, PackagePropertyReason] = dataclasses.field(
default_factory=dict)
# A map from package name to the reason why it belongs in the
# single_packages property.
single_packages: typing.Dict[str,
PackagePropertyReason] = dataclasses.field(
default_factory=dict)
# The list of packages to add to the package_prefixes property.
package_prefixes: typing.List[str] = dataclasses.field(default_factory=list)
# The bootclasspath_fragment hidden API properties changes.
property_changes: typing.List[HiddenApiPropertyChange] = dataclasses.field(
default_factory=list)
@ -1053,13 +1089,16 @@ merge these properties into it.
""").strip("\n")
def analyze_hiddenapi_package_properties(self, result):
split_packages, single_packages, package_prefixes = \
self.compute_hiddenapi_package_properties()
self.compute_hiddenapi_package_properties(result)
def indent_lines(lines):
return "\n".join([f" {cls}" for cls in lines])
# TODO(b/202154151): Find those classes in split packages that are not
# part of an API, i.e. are an internal implementation class, and so
# can, and should, be safely moved out of the split packages.
split_packages = result.split_packages.keys()
result.property_changes.append(
HiddenApiPropertyChange(
property_name="split_packages",
@ -1090,6 +1129,19 @@ merge these properties into it.
details.
""")
for package in split_packages:
reason = result.split_packages[package]
self.report(f"""
Package {package} is split because while this bootclasspath_fragment
provides the following classes:
{indent_lines(reason.bcpf)}
Other module(s) on the bootclasspath provides the following classes in
that package:
{indent_lines(reason.other)}
""")
single_packages = result.single_packages.keys()
if single_packages:
result.property_changes.append(
HiddenApiPropertyChange(
@ -1105,6 +1157,30 @@ merge these properties into it.
action=PropertyChangeAction.REPLACE,
))
self.report_dedent(f"""
bootclasspath_fragment {self.bcpf} contains classes from
packages that has sub-packages which contain classes provided by
other bootclasspath modules. Those packages are called single
packages. Single packages should be avoided where possible but
are often unavoidable when modularizing existing code.
Because some sub-packages contains classes from other
bootclasspath modules it is not possible to use the package as a
package prefix as that treats the package and all its
sub-packages as being provided by this module.
""")
for package in single_packages:
reason = result.single_packages[package]
self.report(f"""
Package {package} is not a package prefix because while this
bootclasspath_fragment provides the following sub-packages:
{indent_lines(reason.bcpf)}
Other module(s) on the bootclasspath provide the following sub-packages:
{indent_lines(reason.other)}
""")
package_prefixes = result.package_prefixes
if package_prefixes:
result.property_changes.append(
HiddenApiPropertyChange(
@ -1154,7 +1230,7 @@ merge these properties into it.
by another module will end with .../*.
""")
def compute_hiddenapi_package_properties(self):
def compute_hiddenapi_package_properties(self, result):
trie = signature_trie()
# Populate the trie with the classes that are provided by the
# bootclasspath_fragment tagging them to make it clear where they
@ -1163,6 +1239,7 @@ merge these properties into it.
for class_name in sorted_classes:
trie.add(class_name + _FAKE_MEMBER, ClassProvider.BCPF)
# Now the same for monolithic classes.
monolithic_classes = set()
abs_flags_file = os.path.join(self.top_dir, _FLAGS_FILE)
with open(abs_flags_file, "r", encoding="utf8") as f:
@ -1177,15 +1254,54 @@ merge these properties into it.
only_if_matches=True)
monolithic_classes.add(class_name)
split_packages = []
single_packages = []
package_prefixes = []
self.recurse_hiddenapi_packages_trie(trie, split_packages,
single_packages, package_prefixes)
return split_packages, single_packages, package_prefixes
self.recurse_hiddenapi_packages_trie(trie, result)
def recurse_hiddenapi_packages_trie(self, node, split_packages,
single_packages, package_prefixes):
@staticmethod
def selector_to_java_reference(node):
return node.selector.replace("/", ".")
@staticmethod
def determine_reason_for_single_package(node):
bcpf_packages = []
other_packages = []
def recurse(n):
if n.type != "package":
return
providers = n.get_matching_rows("*")
package_ref = BcpfAnalyzer.selector_to_java_reference(n)
if ClassProvider.BCPF in providers:
bcpf_packages.append(package_ref)
else:
other_packages.append(package_ref)
children = n.child_nodes()
if children:
for child in children:
recurse(child)
recurse(node)
return PackagePropertyReason(bcpf=bcpf_packages, other=other_packages)
@staticmethod
def determine_reason_for_split_package(node):
bcpf_classes = []
other_classes = []
for child in node.child_nodes():
if child.type != "class":
continue
providers = child.values(lambda _: True)
class_ref = BcpfAnalyzer.selector_to_java_reference(child)
if ClassProvider.BCPF in providers:
bcpf_classes.append(class_ref)
else:
other_classes.append(class_ref)
return PackagePropertyReason(bcpf=bcpf_classes, other=other_classes)
def recurse_hiddenapi_packages_trie(self, node, result):
nodes = node.child_nodes()
if nodes:
for child in nodes:
@ -1193,7 +1309,7 @@ merge these properties into it.
if child.type != "package":
continue
package = child.selector.replace("/", ".")
package = self.selector_to_java_reference(child)
providers = set(child.get_matching_rows("**"))
if not providers:
@ -1204,7 +1320,7 @@ merge these properties into it.
# The package and all its sub packages only contain
# classes provided by the bootclasspath_fragment.
logging.debug("Package '%s.**' is not split", package)
package_prefixes.append(package)
result.package_prefixes.append(package)
# There is no point traversing into the sub packages.
continue
elif providers == {ClassProvider.OTHER}:
@ -1229,8 +1345,17 @@ merge these properties into it.
elif providers == {ClassProvider.BCPF}:
# The package only contains classes provided by the
# bootclasspath_fragment.
logging.debug("Package '%s.*' is not split", package)
single_packages.append(package)
logging.debug(
"Package '%s.*' is not split but does have "
"sub-packages from other modules", package)
# Partition the sub-packages into those that are provided by
# this bootclasspath_fragment and those provided by other
# modules. They can be used to explain the reason for the
# single package to developers.
reason = self.determine_reason_for_single_package(child)
result.single_packages[package] = reason
elif providers == {ClassProvider.OTHER}:
# The package contains no classes provided by the
# bootclasspath_fragment. Child nodes make contain such
@ -1241,11 +1366,15 @@ merge these properties into it.
# The package contains classes provided by both the
# bootclasspath_fragment and some other source.
logging.debug("Package '%s.*' is split", package)
split_packages.append(package)
self.recurse_hiddenapi_packages_trie(child, split_packages,
single_packages,
package_prefixes)
# Partition the classes in this split package into those
# that come from this bootclasspath_fragment and those that
# come from other modules. That can be used to explain the
# reason for the split package to developers.
reason = self.determine_reason_for_split_package(child)
result.split_packages[package] = reason
self.recurse_hiddenapi_packages_trie(child, result)
def newline_stripping_iter(iterator):

View file

@ -377,6 +377,7 @@ La/b/E;->m()V
La/b/c/D;->m()V
La/b/c/E;->m()V
La/b/c/d/E;->m()V
La/b/c/d/e/F;->m()V
Lb/c/D;->m()V
Lb/c/E;->m()V
Lb/c/d/E;->m()V
@ -385,11 +386,21 @@ Lb/c/d/E;->m()V
analyzer = self.create_analyzer_for_test(fs)
analyzer.load_all_flags()
split_packages, single_packages, package_prefixes = \
analyzer.compute_hiddenapi_package_properties()
self.assertEqual(["a.b"], split_packages)
self.assertEqual(["a.b.c"], single_packages)
self.assertEqual(["b"], package_prefixes)
result = ab.Result()
analyzer.compute_hiddenapi_package_properties(result)
self.assertEqual(["a.b"], list(result.split_packages.keys()))
reason = result.split_packages["a.b"]
self.assertEqual(["a.b.C"], reason.bcpf)
self.assertEqual(["a.b.D", "a.b.E"], reason.other)
self.assertEqual(["a.b.c"], list(result.single_packages.keys()))
reason = result.single_packages["a.b.c"]
self.assertEqual(["a.b.c"], reason.bcpf)
self.assertEqual(["a.b.c.d", "a.b.c.d.e"], reason.other)
self.assertEqual(["b"], result.package_prefixes)
class TestHiddenApiPropertyChange(unittest.TestCase):