highsource / highsource/jaxb2-basics

simplify plugin does not set replacement property parent

Open
#106 0 comments 0 reactions 0 assignees View on GitHub
Dominant language
Java
Stars
119
Forks
49
PR merge metrics
No merged PRs in 30d

Description

**Problem:**

When the simplify plugin replaces multiple choice properties, it does not set the parent of the replacement property which leads to failures in other plugins that may be executed after it. For instance any call to `CPropertyInfo.displayName()` on a replaced property will fail with a NPE.

This is caused by adding the replaced property to the `CClassInfo.properties` but not setting the class as the parent of the property.

[SimplifyPlugin.java#L167](https://github.com/highsource/jaxb2-basics/blob/master/basic/src/main/java/org/jvnet/jaxb2_commons/plugin/simplify/SimplifyPlugin.java#L167)
[SimplifyPlugin.java#L192](https://github.com/highsource/jaxb2-basics/blob/master/basic/src/main/java/org/jvnet/jaxb2_commons/plugin/simplify/SimplifyPlugin.java#L192)
[SimplifyPlugin.java#L195](https://github.com/highsource/jaxb2-basics/blob/master/basic/src/main/java/org/jvnet/jaxb2_commons/plugin/simplify/SimplifyPlugin.java#L195)
[SimplifyPlugin.java#L243](https://github.com/highsource/jaxb2-basics/blob/master/basic/src/main/java/org/jvnet/jaxb2_commons/plugin/simplify/SimplifyPlugin.java#L243)

**Possible solution:**

The method `CPropertyInfo.setParent` is package scoped. The only way to set the parent is to first add the replacement property to the class using the `CClassInfo.addProperty()` method, then remove it from the collection and then add it back at the correct index position.

```java
CPropertyInfo newProperty = createElementPropertyInfo(oldProperty, elementDef, ... );
// attach/detach to set parent
classInfo.addProperty(newProperty);
classInfo.getProperties().remove(newProperty);
// then add to right place and remove old one
classInfo.getProperties().add(index, newProperty);
classInfo.getProperties().remove(oldProperty);
```

This will require modification of the class properties outside the visitation loop or else the code that modifies the properties list will fail with `ConcurrentModificationException`. We would need to collect enough information in the visitor to be able to apply the modifications after finalising the visitation.

```java

SimplifyVisitor visitor = new SimplifyVisitor();

for (CPropertyInfo property : classInfo.getProperties()) {
property.accept(visitor);
}

for(Modification mod : visitor.getModifications()) {
mod.apply(classInfo);
}
```

A bit hacky but works reliably.

Contributor guide

No contributing guide indexed for this repository

Assessment

This issue has not been assessed yet.

Get new issues in your inbox

A short digest of beginner-friendly GitHub issues.