gnodet-bot commented on code in PR #27997:
URL: https://github.com/apache/camel/pull/27997#discussion_r4301949581
##########
tooling/maven/camel-package-maven-plugin/src/main/java/org/apache/camel/maven/packaging/PrepareCamelJBangCommandsMojo.java:
##########
@@ -534,22 +518,109 @@ private void
parseOptionsFromClassHierarchy(JavaClassSource clazz, List<OptionIn
// Also parse options from @CommandLine.Mixin fields
for (FieldSource<JavaClassSource> field : clazz.getFields()) {
if (hasMixinAnnotation(field)) {
- String mixinTypeName = field.getType().getName();
- File mixinFile = findClassFile(baseDir, mixinTypeName);
- if (mixinFile != null && mixinFile.exists()) {
- try {
- JavaType<?> parsedMixin = Roaster.parse(mixinFile);
- if (parsedMixin instanceof JavaClassSource mixinClazz)
{
- parseOptionsFromClassHierarchy(mixinClazz,
options, mixinFile.getParentFile());
- } else {
- getLog().debug("Skipping options from non-class
mixin type: " + mixinTypeName);
- }
- } catch (Exception e) {
- getLog().debug("Could not parse mixin class: " +
mixinTypeName + " - " + e.getMessage());
+ parseOptionsFromType(clazz, field.getType().getName(),
options, baseDir, "mixin");
+ }
+ }
+ }
+
+ /**
+ * Parses the options of a parent class or a mixin type of a class: from
its source when it is one of the command
+ * classes, or else from its class on the classpath of the project, such
as the class of a library that a plugin
+ * command extends. The source is found by the simple name of the type, so
for a type that the class imports by name
+ * it is only taken when it is of that type, as a class of a library may
have the simple name of a command of
+ * camel-jbang-core. (The package that Roaster resolves for a type that is
not imported by name is not certain: with
+ * an import on demand of a class, it takes a class of the same package
for a nested class of the imported one.)
+ */
+ private void parseOptionsFromType(JavaClassSource clazz, String type,
List<OptionInfo> options, File baseDir, String kind) {
+ String simpleName = type.substring(type.lastIndexOf('.') + 1);
+ String imported = importedByName(clazz, simpleName);
+ File file = findClassFile(baseDir, simpleName);
+ if (file != null && file.exists()) {
+ try {
+ JavaType<?> parsed = Roaster.parse(file);
+ if (imported == null ||
imported.equals(parsed.getQualifiedName())) {
+ if (parsed instanceof JavaClassSource source) {
+ parseOptionsFromClassHierarchy(source, options,
file.getParentFile());
+ } else {
+ getLog().debug("Skipping options from non-class " +
kind + " type: " + simpleName);
}
+ return;
}
+ } catch (Exception e) {
+ getLog().debug("Could not parse " + kind + " class: " +
simpleName + " - " + e.getMessage());
+ return;
Review Comment:
💡 **Suggestion:** When `Roaster.parse(file)` throws and `imported != null`,
the unconditional `return` skips the classpath fallback at the bottom of the
method. If the source file exists but is corrupt or unparseable, and the class
is imported (so the import points to a valid library class), that library
class's options are silently lost.
Consider only returning early when there's no classpath fallback to try:
```suggestion
if (imported == null) { return; }
```
This way, a parse failure on a local source file gracefully degrades to the
reflection-based classpath path when an import is available.
--
This is an automated message from the Apache Git Service.
To respond to the message, please log on to GitHub and use the
URL above to go to the specific comment.
To unsubscribe, e-mail: [email protected]
For queries about this service, please contact Infrastructure at:
[email protected]
