`DefaultSettingsXmlFactory.write()` Always Emits Location Comments

[juulhobert]

Affected version

4.0-rc-5

Bug description

DefaultSettingsXmlFactory.write() always emits InputLocation comments (e.g.
<!-- /path/to/settings.xml:163 -->) because it creates a bare new SettingsStaxWriter()
whose addLocationInformation field defaults to true, and never calls
setAddLocationInformation(false). It also completely ignores the
inputLocationFormatter from the XmlWriterRequest. This is inconsistent with
DefaultModelXmlFactory.write(), which correctly defaults location tracking to false
and only enables it when an explicit inputLocationFormatter is provided.

Affected Goal

help:effective-settings (Theoretical, there is no Maven Help plugin version which is Maven4 only. See reproducer for a practical example)

Reproduction

Reproducer.zip

A standalone reproducer plugin is available in Reproducer/ that calls
SettingsXmlFactory.write() directly without an inputLocationFormatter:

cd Reproducer
./mvnw install
./mvnw org.apache.maven.its:settings-location-reproducer:1.0-SNAPSHOT:dump-settings

The output contains inline comments with file paths and line numbers after each element:

<mirror>
  <mirrorOf>external:http:*</mirrorOf>
  <!-- /path/to/settings.xml:163 -->
  <name>Pseudo repository</name>
  <!-- /path/to/settings.xml:164 -->
</mirror>

Expected Behaviour

The serialized XML should be clean — no location comments — matching the output
produced by Maven 3's SettingsXpp3Writer and by Maven 4's DefaultModelXmlFactory
(which respects the XmlWriterRequest contract):

<mirror>
  <mirrorOf>external:http:*</mirrorOf>
  <name>Pseudo repository</name>
</mirror>

Location comments should only appear when the caller explicitly provides an
inputLocationFormatter via XmlWriterRequest.builder().inputLocationFormatter(…).

Observed Behaviour

Every element that carries InputLocation metadata gets a trailing
<!-- /path/to/settings.xml:NNN --> comment. There is no way for the caller to
suppress them through the XmlWriterRequest API because DefaultSettingsXmlFactory
ignores all request properties except content, writer, and outputStream.

Suspected Cause

File: impl/maven-impl/src/main/java/org/apache/maven/impl/DefaultSettingsXmlFactory.java
Method: write(XmlWriterRequest<Settings>) (lines 71–90)

The current code:

public void write(XmlWriterRequest<Settings> request) throws XmlWriterException {
    nonNull(request, "request");
    Settings content = nonNull(request.getContent(), "content");
    OutputStream outputStream = request.getOutputStream();
    Writer writer = request.getWriter();
    if (writer == null && outputStream == null) {
        throw new IllegalArgumentException("writer or outputStream must be non null");
    }
    try {
        if (writer != null) {
            new SettingsStaxWriter().write(writer, content);
        } else {
            new SettingsStaxWriter().write(outputStream, content);
        }
    } catch (Exception e) {
        throw new XmlWriterException(
                "Unable to write settings: " + getMessage(e), getLocation(e), e);
    }
}

Problems:

1. new SettingsStaxWriter() — the constructor sets addLocationInformation = true
   (verified in bytecode: iconst_1 → putfield addLocationInformation).
2. setAddLocationInformation(false) is never called.
3. request.getInputLocationFormatter() is never read.

Compare with DefaultModelXmlFactory.write() (lines 154–177) which does it correctly:

MavenStaxWriter xmlWriter = new MavenStaxWriter();
xmlWriter.setAddLocationInformation(false);           // ← defaults OFF

Function<Object, String> formatter = request.getInputLocationFormatter();
if (formatter != null) {
    xmlWriter.setAddLocationInformation(true);         // ← only ON with formatter
    Function<InputLocation, String> adapter = formatter::apply;
    xmlWriter.setStringFormatter(adapter);
}

Workaround

None known. All plugin-side workarounds are either brittle or incorrect:

• Stripping all XML comments post-serialization would destroy legitimate user
  comments.
• Regex-filtering location-pattern comments is fragile — user comments could match.
• Rebuilding the entire Settings model tree with newBuilder(obj, false) to strip
  InputLocation data is excessively complex, untested for deep nesting, and couples
  the plugin to internal model implementation details.