slachiewicz opened a new pull request, #1073: URL: https://github.com/apache/maven-doxia/pull/1073
Draft implementation of #1072, to make the discussion concrete. Happy to drop it if you would rather solve this differently. `doxia-sink-api` declares `Sink` methods that take a `SinkEventAttributes`, but ships no implementation of that interface and no factory for one. The only implementation lives in `doxia-core` under `org.apache.maven.doxia.sink.impl`, so every caller that wants to pass attributes to a sink has to depend on `doxia-core` and import from a package whose name says it is internal. That is what [MSHARED-1364](https://issues.apache.org/jira/browse/MSHARED-1364) / apache/maven-reporting-impl#184 is about, and it applies to most report renderers in the ecosystem, not just that one. The class was in package `org.apache.maven.doxia.sink` until [DOXIA-506](https://issues.apache.org/jira/browse/DOXIA-506) moved it in with the genuinely internal sink implementations. ### What this does Moves `SinkEventAttributeSet` and its nested `Semantics` to `doxia-sink-api`, in package `org.apache.maven.doxia.sink`, next to the interface it implements. Its unit test moves with it, which is why the module gains a `junit-jupiter-api` test dependency; it still has no compile dependencies. The move costs the API module nothing. The class needs only `javax.swing.text.AttributeSet`, JDK collections and `SinkEventAttributes`, and `SinkEventAttributes` already extends `javax.swing.text.MutableAttributeSet`. ### Binary compatibility `japicmp` passes on every module with no exclusions. Three things were needed to get there, and the last one is the part worth your attention. 1. `org.apache.maven.doxia.sink.impl.SinkEventAttributeSet` stays as a deprecated subclass of the new class. The static constants resolve through it, since a `getstatic` resolves through superclasses. 2. It carries its own deprecated nested `Semantics` subclass and an override of `unmodifiable()`. Neither comes for free: a nested class is not inherited under its old binary name, so existing code referencing `…/sink/impl/SinkEventAttributeSet$Semantics` would fail with `NoClassDefFoundError`, and `unmodifiable()` declares its own type as the return type, so the inherited one has the wrong descriptor. 3. The protected parser hooks in `doxia-core` that take or return this type keep taking the deprecated one, so nothing on the extension API changes: - `AbstractXmlParser.getAttributesFromParser(XmlPullParser)` - `AbstractXmlParser.handleUnknown(String, SinkEventAttributeSet, Sink, int)` - `Xhtml5BaseParser.baseStartTag(String, SinkEventAttributeSet, Sink)` - `Xhtml5BaseParser.baseEndTag(String, SinkEventAttributeSet, Sink)` - `Xhtml5BaseParser.consecutiveSections(int, Sink, SinkEventAttributeSet)` Retyping those would be a silent break rather than a loud one: a subclass compiled against the old signature would stop overriding the method and simply never be called again. So I left them alone. They arguably want to take the `SinkEventAttributes` interface rather than a concrete set at all, but that is a separate change needing a version bump, and it is not what the issue is about. ### Follow-up Once this is released, `AbstractMavenReportRenderer` in maven-reporting-impl is a one-line import change and MSHARED-1364 is done. ### Verification `mvn verify` is green: full test suite, rat, and japicmp across all modules. -- 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]
