xpp3-reader: set method called before list is populated

[kwin]

With the goal xpp3-reader the set method of the bean is called before the collection has been populated. For example the following pattern is generated:

java.util.List<MojoDescriptor> mojos = new java.util.ArrayList<MojoDescriptor>();
pluginDescriptor.setMojos( mojos );
while ( parser.nextTag() == XmlPullParser.START_TAG )
{
    if ( "mojo".equals( parser.getName() ) )
    {
        mojos.add( parseMojoDescriptor( parser, strict ) );
    }
    else
    {
        checkUnknownElement( parser, strict );
    }
}

when using the following model:

<field xdoc.separator="blank">
  <name>mojos</name>
  <version>1.0.0+</version>
  <association>
    <type>MojoDescriptor</type>
    <multiplicity>*</multiplicity>
  </association>
  <description>Description of each Mojo provided by the plugin.</description>
</field>

The code fails if the called setter method does not just add the reference to the collection but copies/processes the given argument.
Therefore the setter method should be called only after population (i.e. after the while loop).
The code is being generated in

modello/modello-plugins/modello-plugin-xpp3/src/main/java/org/codehaus/modello/plugin/xpp3/Xpp3ReaderGenerator.java

Line 1639 in b405413

private void writeNewSetLocation( String key, String objectName, String trackerVariable, JSourceCode sc )

.

[gnodet]

Fwiw, this would not affect maven 4.x where the problem has already been fixed:
https://maven.apache.org/ref/4.0.0-alpha-2/maven-model/xref/org/apache/maven/model/v4/MavenXpp3Reader.html#L254

That's because the v3 readers delegates to the v4 ones which have:
https://github.com/apache/maven/blob/2a9f39336cec1d8e52d30cc48503d51ed8672536/maven-model/src/main/mdo/reader.vm#L285-L299

[kwin]

Fixing this in Modello allows us to get rid of more custom code in reader.vm for Maven 4, right?

[gnodet]

Fixing this in Modello allows us to get rid of more custom code in reader.vm for Maven 4, right?

No, for maven 4, modello would require brand new generators for readers, writers, model, merger and transformer. The current generators are highly inflexible, and I abandoned that idea during the v4 dev. Now that things actually work, it may be possible to retrofit to custom generators, but I think they are too tied to one way of writing things, but that's a huge work and I'm not sure it's worth it. The main downside I see (in addition to the time spent to write those generators) is outside maven, whereas is looks very specific to me.

The main changes that make the current generators completely incompatible is that the object model is now fully immutable and uses a builder api instead of plain getters/setters. So all java code generators have to be rewritten. Also, the types are too limited and this would need to be opened to plug in custom types such as java.nio.Path or org.apache.maven.api.xml.Dom.

In addition, I found that using velocity templates are much easier to work with because you can roughly read the java code (even if interleaved with velocity pragmas), whereas the current generators are not very readable (to say the least), because you need to build an object model which represents the java code instead of just generating the java code.
Just compare the current generator (2663 lines):
https://github.com/codehaus-plexus/modello/blob/4c008938a7483b0c0cdd3f9d784211d5263a1df7/modello-plugins/modello-plugin-java/src/main/java/org/codehaus/modello/plugin/java/JavaModelloGenerator.java
to the new one (319 lines)
https://github.com/apache/maven/blob/master/maven-model/src/main/mdo/model-v3.vm