slachiewicz opened a new issue, #1072: URL: https://github.com/apache/maven-doxia/issues/1072
### New feature, improvement proposal `doxia-sink-api` declares `Sink` methods that take a `SinkEventAttributes`, but the module ships no implementation of that interface and no factory for one. It contains six files in total: `Sink`, `SinkFactory`, `SinkEventAttributes`, `Locator`, `EmptyLocator` and `package-info`. The only implementation, `SinkEventAttributeSet`, 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. In practice that is most report renderers in the Maven ecosystem. In a checkout of the Maven repositories, 14 files import `org.apache.maven.doxia.sink.impl`: 10 use `SinkEventAttributeSet`, 2 use its nested `SinkEventAttributeSet.Semantics`, 1 uses `SinkAdapter`. Among them maven-project-info-reports-plugin, maven-changes-plugin, maven-pmd-plugin, maven-plugin-report-plugin, maven-surefire-report-plugin and maven-reporting-impl. This was raised for maven-reporting-impl in [MSHARED-1364](https://issues.apache.org/jira/browse/MSHARED-1364) / apache/maven-reporting-impl#184, where the conclusion was that the class should be copied into `doxia-sink-api` and the `doxia-core` one deprecated. Michael Osipov asked for a Doxia ticket to be filed for it; as far as I can tell none was, so here it is. Worth noting how the situation arose: the class used to be in package `org.apache.maven.doxia.sink` (in `doxia-core`) and was moved into `.impl` by [DOXIA-506](https://issues.apache.org/jira/browse/DOXIA-506) (commit cc0cf036), together with the genuinely internal sink implementations. It is not an internal class that consumers reached into, it is a class consumers have always used that was relabelled as internal. ### Proposal Move `SinkEventAttributeSet`, including its nested `Semantics`, to `doxia-sink-api` in package `org.apache.maven.doxia.sink`. This costs the API module nothing in dependencies. The class is 563 lines and imports only `javax.swing.text.AttributeSet`, JDK collections, and `SinkEventAttributes` itself, and `SinkEventAttributes` already lives in `doxia-sink-api` and already extends `javax.swing.text.MutableAttributeSet`. Within Doxia the change touches 18 main and 16 test source files, all of them import edits. ### Binary compatibility Leaving `org.apache.maven.doxia.sink.impl.SinkEventAttributeSet` behind as a deprecated subclass of the new one preserves the static constants, since a `getstatic` resolves through superclasses. It does not preserve the nested class: already compiled code references `org/apache/maven/doxia/sink/impl/SinkEventAttributeSet$Semantics`, which would no longer exist. The deprecated stub therefore needs its own nested `Semantics` subclass, otherwise consumers of `Semantics` break with a `NoClassDefFoundError` rather than a compile error. Four files in the checkout above use `Semantics`. `SinkAdapter` is the other class outside Doxia consumes from that package. Not in scope here, but it is the same question if someone wants to take it further. ### Note on a related claim MSHARED-1364 states that these constants lost their effect when [DOXIA-685](https://issues.apache.org/jira/browse/DOXIA-685) replaced `BOXED` with `SOURCE`. That is no longer accurate for `SOURCE`: `Xhtml5BaseSink.verbatim(SinkEventAttributes)` in 2.1.0 still evaluates `DECORATION` and emits `<pre><code>` for it, and maven-reporting-impl has an integration test asserting that output. This proposal is about API structure, not about a broken attribute. -- 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]
