Skip to content

Commit

Permalink
Do not require option converters to be public.
Browse files Browse the repository at this point in the history
Change how we reflectively instantiate the option converters so that Bazel no longer crashes when we try to use a non-public class. Technically, this is overriding the visibility of the class and the constructor, however this is merely a side effect of the fact that annotations do not allow us to use an actual object.

It is not obvious why these classes have to be public and a mistake here results with Bazel failing to start.

PiperOrigin-RevId: 482281892
Change-Id: I064301ce88f83fd55a305385309cfb357e5e054c
  • Loading branch information
alexjski authored and copybara-github committed Oct 19, 2022
1 parent dfccbf9 commit 58edc17
Show file tree
Hide file tree
Showing 2 changed files with 6 additions and 3 deletions.
6 changes: 4 additions & 2 deletions src/main/java/com/google/devtools/common/options/Option.java
Original file line number Diff line number Diff line change
Expand Up @@ -123,9 +123,11 @@
* an object or a simple type. The default is to use the builtin converters ({@link
* Converters#DEFAULT_CONVERTERS}). Custom converters must implement the {@link Converter}
* interface.
*
* <p>This class will be instantiated reflectively using a nullary constructor. Provided class
* does not have to be visible in the {@linkplain com.google.devtools.common.options options}
* package, e.g. private classes are allowed.
*/
@SuppressWarnings({"unchecked", "rawtypes"})
// Can't figure out how to coerce Converter.class into Class<? extends Converter<?>>
Class<? extends Converter> converter() default Converter.class;

/**
Expand Down
Original file line number Diff line number Diff line change
Expand Up @@ -238,7 +238,8 @@ public Converter<?> getConverter() {
} else {
try {
// Instantiate the given Converter class.
Constructor<?> constructor = converterClass.getConstructor();
Constructor<?> constructor = converterClass.getDeclaredConstructor();
constructor.setAccessible(true);
converter = (Converter<?>) constructor.newInstance();
} catch (SecurityException | IllegalArgumentException | ReflectiveOperationException e) {
// This indicates an error in the Converter, and should be discovered the first time it is
Expand Down

0 comments on commit 58edc17

Please sign in to comment.