tkobayas commented on code in PR #6780:
URL:
https://github.com/apache/incubator-kie-drools/pull/6780#discussion_r3503560324
##########
drools-model/drools-model-codegen/src/main/java/org/drools/model/codegen/execmodel/generator/declaredtype/DescrTypeDefinition.java:
##########
@@ -161,7 +165,45 @@ private static Optional<TypeDeclarationDescr>
getSuperType(TypeDeclarationDescr
@Override
public List<FieldDefinition> findInheritedDeclaredFields() {
- return findInheritedDeclaredFields(new ArrayList<>(),
getSuperType(typeDeclarationDescr, packageDescr));
+ List<FieldDefinition> fields = findInheritedDeclaredFields(new
ArrayList<>(), getSuperType(typeDeclarationDescr, packageDescr));
+ if (fields.isEmpty()) {
+ abstractClass.ifPresent(superClass ->
fields.addAll(inheritedFieldsFromSuperClass(superClass)));
+ }
+ return fields;
+ }
+
+ /**
+ * Collects the positional fields of a resolved Java superclass, walking
the class hierarchy.
+ * A field participates only when it carries {@link Position} (explicit
opt-in), ordered by its
+ * position value; this deterministically excludes non-positional members
(and keeps the
+ * generated {@code super(...)} call aligned with a positional constructor
on the superclass).
+ * When the superclass declares no {@link Position} at all, fall back to
all non-static instance
+ * fields in declaration order (top-most ancestor first).
+ */
+ private List<FieldDefinition> inheritedFieldsFromSuperClass(Class<?>
superClass) {
+ List<Class<?>> hierarchy = new ArrayList<>();
+ for (Class<?> c = superClass; c != null && c != Object.class; c =
c.getSuperclass()) {
+ hierarchy.add(0, c);
+ }
+
+ List<Field> instanceFields = new ArrayList<>();
+ for (Class<?> c : hierarchy) {
+ for (Field f : c.getDeclaredFields()) {
+ if (!Modifier.isStatic(f.getModifiers())) {
+ instanceFields.add(f);
+ }
+ }
+ }
+
+ List<Field> positioned = instanceFields.stream()
+ .filter(f -> f.getAnnotation(Position.class) != null)
+ .sorted(Comparator.comparingInt(f ->
f.getAnnotation(Position.class).value()))
+ .collect(Collectors.toList());
+
+ List<Field> chosen = positioned.isEmpty() ? instanceFields :
positioned;
+ return chosen.stream()
+ .map(f -> (FieldDefinition) new
DescrFieldDefinition(f.getName(), f.getType().getCanonicalName(), null))
+ .collect(Collectors.toList());
Review Comment:
One more point:
`testExtendPojoInheritedFieldsConstructor` needs
```
"declare PositionalParent end\n" +
```
to align with the usage explained in
https://github.com/apache/incubator-kie-docs/blob/main/drools-docs/src/modules/ROOT/pages/language-reference-traditional/_drl-declarations-extended-con.adoc
In reality, it works without `declare PositionalParent end` because
`ClassHierarchyManager.mergeFields` has a path to find an external class. But
it's better to stick with the documented way.
I found a subtle caveat case :
https://github.com/apache/incubator-kie-drools/issues/6790
--
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]
---------------------------------------------------------------------
To unsubscribe, e-mail: [email protected]
For additional commands, e-mail: [email protected]
