[
https://issues.apache.org/jira/browse/GROOVY-12207?page=com.atlassian.jira.plugin.system.issuetabpanels:comment-tabpanel&focusedCommentId=18099907#comment-18099907
]
ASF GitHub Bot commented on GROOVY-12207:
-----------------------------------------
Copilot commented on code in PR #2745:
URL: https://github.com/apache/groovy/pull/2745#discussion_r3670595002
##########
src/main/java/org/codehaus/groovy/control/ClassNodeResolver.java:
##########
@@ -160,6 +174,76 @@ public ClassNode getFromClassCache(final String name) {
return cached;
}
+ /**
+ * Resolves a package name to a {@link PackageNode} carrying the
annotations found on the
+ * package's compiled {@code package-info.class}, if any (GROOVY-12207).
This makes
+ * package-level annotations of precompiled dependencies (e.g. JSpecify's
{@code @NullMarked})
+ * visible to type checkers and AST transforms.
+ * <p>
+ * The {@code package-info.class} is located on the compilation unit's
class path and decompiled
+ * on demand using the same ASM infrastructure as ordinary classes;
results are cached per
+ * resolver, including a negative cache for packages that have no
(annotation-bearing)
+ * package-info. Returns {@code null} if the package has no such metadata.
+ *
+ * @param packageName the fully qualified package name (no trailing dot),
e.g. {@code "foo.bar"}
+ * @param compilationUnit the current {@link CompilationUnit}
+ * @return a {@link PackageNode} with the package's annotations, or {@code
null} if none
+ */
+ public synchronized PackageNode resolvePackage(final String packageName,
final CompilationUnit compilationUnit) {
+ if (packageName == null || packageName.isEmpty() || compilationUnit ==
null) {
+ return null;
+ }
+ PackageNode cached = cachedPackages.get(packageName);
+ if (cached != null) {
+ return cached == NO_PACKAGE ? null : cached;
+ }
+ PackageNode result = findPackageInfo(packageName, compilationUnit);
+ cachedPackages.put(packageName, result == null ? NO_PACKAGE : result);
+ return result;
+ }
+
+ /**
+ * Loads and decompiles {@code <packageName>/package-info.class} from the
compilation unit's
+ * class loader, copying its (class-level) annotations onto a fresh {@link
PackageNode}. The
+ * annotations of a {@code package-info} type are stored as ordinary class
annotations in the
+ * bytecode, so the existing decompiler pipeline reads them without
special handling.
+ *
+ * @return a populated {@link PackageNode}, or {@code null} if there is no
package-info, it has
+ * no annotations, or it could not be read
+ */
+ private PackageNode findPackageInfo(final String packageName, final
CompilationUnit compilationUnit) {
+ GroovyClassLoader loader = compilationUnit.getClassLoader();
+ if (loader == null) {
+ return null;
+ }
+ String fileName = packageName.replace('.', '/') +
"/package-info.class";
+ URL resource = loader.getResource(fileName);
+ if (resource == null) {
+ return null;
+ }
+ try {
+ DecompiledClassNode packageInfo = new DecompiledClassNode(
+ AsmDecompiler.parseClass(resource), new
AsmReferenceResolver(this, compilationUnit));
+ if (!packageInfo.getName().equals(packageName + ".package-info")) {
+ // this may happen under Windows/macOS because getResource is
case-insensitive there!
+ return null;
+ }
+ List<AnnotationNode> annotations = packageInfo.getAnnotations();
+ if (annotations == null || annotations.isEmpty()) {
+ return null;
+ }
+ PackageNode packageNode = new PackageNode(packageName + ".");
Review Comment:
`resolvePackage` documents `packageName` as having no trailing dot, but
`findPackageInfo` constructs `PackageNode` with `packageName + "."`. This makes
`PackageNode.getName()` inconsistent with the rest of the AST (and would render
as `package foo.bar.` via `PackageNode#getText`). Create the `PackageNode` with
the plain package name instead.
##########
src/main/java/org/codehaus/groovy/ast/decompiled/DecompiledClassNode.java:
##########
@@ -164,6 +165,23 @@ public long getCompilationTimeStamp() {
* @throws org.codehaus.groovy.GroovyBugError if the class cannot be
resolved at runtime
* @see AsmReferenceResolver#resolveJvmClass(String)
*/
+ /**
+ * Returns the {@link PackageNode} for this decompiled class, carrying any
annotations declared
+ * on the package's compiled {@code package-info.class} (GROOVY-12207).
Unlike a source-based
+ * {@link ClassNode}, a decompiled class has no owning module, so the
package (and its
+ * annotations) is resolved on demand from the class path via the
reference resolver's
Review Comment:
There are now two consecutive Javadoc blocks before `getPackage()`. The
first block (about resolving the runtime JVM `Class`) is no longer attached to
`getTypeClass()` and can trigger dangling-Javadoc warnings and misleading
generated docs. Removing that stray Javadoc block (or relocating it directly
above `getTypeClass()`) will keep Javadoc output clean.
> Consult package-info.class of dependencies during resolution
> ------------------------------------------------------------
>
> Key: GROOVY-12207
> URL: https://issues.apache.org/jira/browse/GROOVY-12207
> Project: Groovy
> Issue Type: Improvement
> Reporter: Paul King
> Priority: Major
>
> Groovy's resolver does not consult {{package-info.class}} for the packages of
> precompiled dependencies, so package-level annotations of libraries are
> invisible to the compiler and to type-checking extensions.
> JSpecify's {{@NullMarked}} is commonly applied at package level (and may also
> be applied at module level), meaning an entire library's nullness defaults
> are typically expressed in files Groovy never reads. Without this,
> per-element type annotations (see the companion type-annotation ingestion
> issue) tell only half the story: a {{@NullMarked}} package with unannotated
> method signatures means "everything here is non-null", and Groovy would
> wrongly treat it as unspecified.
> Proposed scope:
> # On-demand loading of {{package-info.class}} for the package of a resolved
> class, with caching (including negative caching for packages without one),
> reusing the existing decompiled-class infrastructure.
> # Expose the package annotations via a suitable API (e.g. on {{PackageNode}}
> or a resolver service) so checkers and AST transforms can query them for
> dependency packages the way they already can for source packages.
> # Extend {{NullChecker}} to know about this new annotation information.
> # Consider {{module-info}} annotations ({{@NullMarked}} at module level) as a
> stretch goal.
> Behaviour-neutral on its own: this only makes existing metadata reachable.
> Consumers include the incubating {{groovy.typecheckers.NullChecker}} (which
> already recognizes {{@NullMarked}}/{{@NullUnmarked}} by name but can
> currently only see them on classes in the compilation unit).
--
This message was sent by Atlassian Jira
(v8.20.10#820010)
