RFR: 8272234: Pass originating elements from Filer to JavaFileManager [v6]

[Jonathan Gibbons]

On Tue, 23 Nov 2021 13:40:46 GMT, Jan Lahoda <jlahoda at openjdk.org> wrote:

>> This is a first prototype of a patch that propagates originating elements from `Filer` (`createSourceFile`/`createClassFile`/`createResource`) to the corresponding methods in `JavaFileManager`. As file managers generally don't know about `Element`s, the `Element`s are first converted to their corresponding `FileObject`s (if any). As the currently existing methods only take one `FileObject` as a sibling of the newly created file, a new set of methods is proposed that take multiple originating files.
>> 
>> Any feedback on this prototype would be welcome.
>
> Jan Lahoda has updated the pull request incrementally with one additional commit since the last revision:
> 
>   Improving javadoc as suggested in the PR.

src/java.compiler/share/classes/javax/tools/JavaFileManager.java line 349:

> 347:      *
> 348:      * <p>The provided {@code originatingFiles} represent files that
> 349:      * were in, an unspecified way, used to create the content of

misplaced comma.  should be `were, in an unspecified way,`

src/java.compiler/share/classes/javax/tools/JavaFileManager.java line 365:

> 363:      * @param originatingFiles the files which are contributing to this newly created file;
> 364:      *                         {@code null} is equivalent to empty {@code originatingFiles},
> 365:      *                         meaning no known originating files

(minor/white-space) inconsistent indentation of `@param` tags

src/jdk.compiler/share/classes/com/sun/tools/javac/processing/JavacFiler.java line 531:

> 529:         }
> 530:         JavaFileObject[] originatingFiles = Arrays.asList(originatingElements)
> 531:                 .stream()

Why not start with `Stream.of(originatingElements)` ?

test/langtools/tools/javac/processing/filer/TestOriginatingElements.java line 24:

> 22:  */
> 23: 
> 24: /**

Should be `/*` not `/**`. This is not a documentation comment for javadoc.

test/langtools/tools/javac/processing/filer/TestOriginatingElements.java line 185:

> 183:                 List<String> options = List.of("-sourcepath", src.toString(),
> 184:                                                "-processor", "TestOriginatingElements$P",
> 185:                                                "-processorpath", System.getProperty("test.classes"),

This may be enough. but note there is also `test.class.path` .... one is just the output directory, the other is the full path.

test/langtools/tools/javac/processing/filer/TestOriginatingElements.java line 229:

> 227:                 PackageElement pack = processingEnv.getElementUtils().getPackageElement("p");
> 228:                 PackageElement lib1Pack = processingEnv.getElementUtils().getPackageElement("lib1");
> 229:                 PackageElement lib2Pack = processingEnv.getElementUtils().getPackageElement("lib2");

strongly suggest using a local variable for `processingEnv.getElementUtils()`

-------------

PR: https://git.openjdk.java.net/jdk/pull/5076