Do package-infos need to be reset between annotation processing rounds?
joe darcy
joe.darcy at oracle.com
Wed Dec 13 08:16:32 UTC 2017
Hello,
Catching up on email amid JDK 10 RDP1 activities, the Filer
specification states:
> During each run of an annotation processing tool, a file with a given
> pathname may be created only once. If that file already exists before
> the first attempt to create it, the old contents will be deleted. Any
> subsequent attempt to create the same file during a run will throw a
> |FilerException|
> <http://download.java.net/java/jdk10/docs/api/javax/annotation/processing/FilerException.html>,
> as will attempting to create both a class file and source file for the
> same type name or same package name. The initial inputs
> <http://download.java.net/java/jdk10/docs/api/javax/annotation/processing/Processor.html>
> to the tool are considered to be created by the zeroth round;
> therefore, attempting to create a source or class file corresponding
> to one of those inputs will result in a |FilerException|
> <http://download.java.net/java/jdk10/docs/api/javax/annotation/processing/FilerException.html>.
>
>
> In general, processors must not knowingly attempt to overwrite
> existing files that were not generated by some processor. A |Filer|
> may reject attempts to open a file corresponding to an existing type,
> like |java.lang.Object|. Likewise, the invoker of the annotation
> processing tool must not knowingly configure the tool such that the
> discovered processors will attempt to overwrite existing files that
> were not generated.
>
http://download.java.net/java/jdk10/docs/api/javax/annotation/processing/Filer.html
For handling source files in particular:
> A source file can also be created to hold information about a package,
> including package annotations. To create a source file for a named
> package, have the |name| argument be the package's name followed by
> |".package-info"|; to create a source file for an unnamed package, use
> |"package-info"|.
To separate a few cases, the javac Filer implementation tracks state
about which files have been opened through the Filer on previous rounds
of processing and throws exceptions if an attempt is made to re-open a
file. (The check is a bit more involved; the Filer also checks the you
don't open a class file for a type you've already opened a source file
for, etc.) Package info files are handled the same way too. There is a
short regression test on that functionality
test/langtools/tools/javac/processing/filer/TestPackageInfo.java
but the exception on re-opening isn't explicitly tested there. Let me do
a quick check... the implementation has the expected behavior for
package-info files explicitly opened by annotation processors.. I'll
file a RFE to enhance the test to explicitly cover this case.
However, contrary to the documentation, the implementation does not
handle the case of a package-info file given on the initial inputs,
although types on the initial inputs are checked for properly. I believe
something like the patch below to JavacFiler *should* resolve that
discrepancy:
- ClassSymbol existing;
boolean alreadySeen =
aggregateGeneratedSourceNames.contains(Pair.of(mod, typename)) ||
aggregateGeneratedClassNames.contains(Pair.of(mod, typename)) ||
initialClassNames.contains(typename) ||
- ((existing =
elementUtils.getTypeElement(typename)) != null &&
- initialInputs.contains(existing.sourcefile));
+ containedInInitialInputs(typename);
if (alreadySeen) {
if (lint)
log.warning(Warnings.ProcTypeRecreate(typename));
@@ -731,6 +736,27 @@
}
}
+ private boolean containedInInitialInputs(String typename) {
+ // Name could be a type name or the name of a package-info file
+ JavaFileObject sourceFile = null;
+
+ ClassSymbol existingClass = elementUtils.getTypeElement(typename);
+ if (existingClass != null) {
+ sourceFile = existingClass.sourcefile;
+ } else if (typename.endsWith(".package-info")) {
+ String targetName = typename.substring(0, typename.length()
- ".package-info".length());
+ PackageSymbol existingPackage =
elementUtils.getPackageElement(targetName);
+ if (existingPackage != null)
+ sourceFile = existingPackage.sourcefile;
+ }
+ return (sourceFile == null) ? false :
initialInputs.contains(sourceFile);
+ }
+
*However,* the sourcefile field of the PackageSymbol doesn't seem to be
getting set to a non-null value via the lookup process. I didn't fully
run down which part of JavacElements or Symbol takes care of that.
In the broader sense of the intention of the Filer, in the simplest case
of input locations and output locations being disjoint, than the rules
quoted above should be enforced. However, people can and do run
configurations where the input and output locations overlap,
complicating implementing the proper intentions.
In general, package-info files should be handled analogously to source
files for types.
HTH,
-Joe
On 12/5/2017 11:48 PM, Jonathan Gibbons wrote:
>
> We'll need Joe "Mr Annotation Processing" Darcy to chime in here, but
> the Filer is supposed to detect clashes, and prevent
> overwriting/overriding symbols. That's definitely supposed to happen
> for normal classes/interfaces; I could believe that package-info has
> been overlooked, but I would expect it to follow the same guidelines.
>
> -- Jon
>
>
> On 12/5/17 6:24 PM, Liam Miller-Cushon wrote:
>> Is overriding package-infos different or worse than overriding other
>> symbols? I've seen a number of examples where the same source file
>> was compiled multiple times and the earlier results ended up on the
>> class path of the later compilations, so a processor-generated class
>> was both on the class path and generated during the compilation.
>> Making that an error would be a somewhat invasive breaking change. I
>> agree that it should be discouraged, but I'm not sure it's worth
>> making an error? (Unless there's something special about
>> package-infos that I'm missing.)
>>
>> On Tue, Dec 5, 2017 at 6:09 PM, Jonathan Gibbons
>> <jonathan.gibbons at oracle.com <mailto:jonathan.gibbons at oracle.com>> wrote:
>>
>> Generally, annotation processing is supposed to only create
>> information where there was no information previously, so if a
>> package had a package-info with annotations, it seems like it
>> should be an error to override it with another package-info, with
>> or without annotations.
>>
>> -- Jon
>>
>>
>> On 12/05/2017 05:39 PM, Liam Miller-Cushon wrote:
>>> Thanks! If an annotated package-info is loaded from the class
>>> path, and then a processor generates a package-info that
>>> contains no annotations, the reset is necessary. (The reset
>>> isn't necessary if the new package-info is annotated, since the
>>> old annotations get overwritten even if they weren't reset
>>> between rounds.)
>>>
>>> On Tue, Dec 5, 2017 at 4:46 PM, Jonathan Gibbons
>>> <jonathan.gibbons at oracle.com
>>> <mailto:jonathan.gibbons at oracle.com>> wrote:
>>>
>>> Liam,
>>>
>>> What about the case where an annotation processor generates
>>> the package-info.java file? Is that a case where it is
>>> important to reinit the packge symbol correctly, so that the
>>> newly generated code is read?
>>>
>>> -- Jon
>>>
>>>
>>> On 12/05/2017 03:39 PM, Liam Miller-Cushon wrote:
>>>
>>> I have a question about the logic in
>>> JavacProcessingEnvironment's treeCleaner that resets
>>> package-info state between annotation processing rounds [1].
>>>
>>> JDK-8193037 describes an issue where package-infos
>>> loaded from the classpath are reset by treeCleaner.
>>> Those symbols doesn't get reinitialized correctly, and
>>> package annotations are not visible during subsequent
>>> annotation processing rounds.
>>>
>>> I wondered if the logic was only meant to apply to
>>> package-infos being compiled from source in the current
>>> compilation (similar to how module-infos are handle by
>>> treeCleaner), but I'm having trouble understanding when
>>> that logic is necessary. Commenting out
>>> `node.packge.package_info.reset();` and
>>> `node.packge.reset();` in treeCleaner doesn't break any
>>> jtreg tests. Does anyone have examples where that code
>>> is needed? I'd like to add a regression test to ensure
>>> the fix for JDK-8193037 doesn't interfere with the
>>> original purpose of that code.
>>>
>>> Thanks,
>>>
>>> [1]
>>> http://hg.openjdk.java.net/jdk/jdk/file/a358ebcfacfb/src/jdk.compiler/share/classes/com/sun/tools/javac/processing/JavacProcessingEnvironment.java#l1518
>>> <http://hg.openjdk.java.net/jdk/jdk/file/a358ebcfacfb/src/jdk.compiler/share/classes/com/sun/tools/javac/processing/JavacProcessingEnvironment.java#l1518>
>>>
>>>
>>>
>>
>>
>
-------------- next part --------------
An HTML attachment was scrubbed...
URL: <http://mail.openjdk.java.net/pipermail/compiler-dev/attachments/20171213/776932e0/attachment.html>
More information about the compiler-dev
mailing list