slachiewicz commented on code in PR #1073:
URL: https://github.com/apache/maven-doxia/pull/1073#discussion_r3740911142
##########
doxia-core/src/main/java/org/apache/maven/doxia/sink/impl/SinkEventAttributeSet.java:
##########
@@ -157,407 +65,29 @@ public SinkEventAttributeSet(String... attributes) {
* @param attributes the specified AttributeSet.
*/
public SinkEventAttributeSet(AttributeSet attributes) {
- attribs = new LinkedHashMap<>(attributes.getAttributeCount());
-
- Enumeration<?> names = attributes.getAttributeNames();
-
- while (names.hasMoreElements()) {
- Object name = names.nextElement();
-
- attribs.put(name.toString(), attributes.getAttribute(name));
- }
+ super(attributes);
}
/**
- * Replace this AttributeSet by an unmodifiable view of itself.
- * Any subsequent attempt to add, remove or modify the underlying mapping
- * will result in an UnsupportedOperationException.
+ * {@inheritDoc}
*
- * @return an unmodifiable view of this AttributeSet.
- * @since 1.1.1
+ * Overridden only to keep returning this type, so that code compiled
against the old signature keeps
+ * resolving the method.
*/
+ @Override
public SinkEventAttributeSet unmodifiable() {
- this.attribs = Collections.unmodifiableMap(attribs);
+ super.unmodifiable();
return this;
}
Review Comment:
Good catch, this was a real regression and fixed in 0efdcd4e.
`clone()` built its result with an explicit `new SinkEventAttributeSet(...)`
rather than `super.clone()`, so the deprecated subclass would indeed have
cloned to the parent type. japicmp cannot see it because no signature changes.
Fixed at the root rather than by overriding in the stub: `clone()` now uses
`super.clone()` and replaces the shared fields afterwards. Behaviour of the
class itself is unchanged, and any subclass now clones correctly. Added
`SinkEventAttributeSetCompatibilityTest` in doxia-core covering the three
things the deprecated class has to keep doing that japicmp cannot express:
reaching the constants, keeping the return type of `unmodifiable()`, and
cloning to itself. It fails on the previous implementation with exactly the
type mismatch you describe.
##########
doxia-sink-api/src/main/java/org/apache/maven/doxia/sink/SinkEventAttributeSet.java:
##########
@@ -0,0 +1,561 @@
+/*
+ * Licensed to the Apache Software Foundation (ASF) under one
+ * or more contributor license agreements. See the NOTICE file
+ * distributed with this work for additional information
+ * regarding copyright ownership. The ASF licenses this file
+ * to you under the Apache License, Version 2.0 (the
+ * "License"); you may not use this file except in compliance
+ * with the License. You may obtain a copy of the License at
+ *
+ * http://www.apache.org/licenses/LICENSE-2.0
+ *
+ * Unless required by applicable law or agreed to in writing,
+ * software distributed under the License is distributed on an
+ * "AS IS" BASIS, WITHOUT WARRANTIES OR CONDITIONS OF ANY
+ * KIND, either express or implied. See the License for the
+ * specific language governing permissions and limitations
+ * under the License.
+ */
+package org.apache.maven.doxia.sink;
+
+import javax.swing.text.AttributeSet;
+
+import java.util.Collections;
+import java.util.Enumeration;
+import java.util.LinkedHashMap;
+import java.util.Map;
+import java.util.Map.Entry;
+import java.util.Set;
+
+/**
+ * Implementation of MutableAttributeSet using a LinkedHashMap.
+ *
+ * @author ltheussl
+ * @since 1.1
+ */
+public class SinkEventAttributeSet implements SinkEventAttributes, Cloneable {
+ /**
+ * An unmodifiable attribute set containing only an underline attribute.
+ */
+ public static final SinkEventAttributes UNDERLINE;
+
+ /**
+ * An unmodifiable attribute set containing only an overline attribute.
+ */
+ public static final SinkEventAttributes OVERLINE;
+
+ /**
+ * An unmodifiable attribute set containing only a linethrough attribute.
+ */
+ public static final SinkEventAttributes LINETHROUGH;
+
+ /**
+ * An unmodifiable attribute set containing only a source attribute.
+ */
+ public static final SinkEventAttributes SOURCE;
+
+ /**
+ * An unmodifiable attribute set containing only a bold attribute.
+ */
+ public static final SinkEventAttributes BOLD;
+
+ /**
+ * An unmodifiable attribute set containing only an italic attribute.
+ */
+ public static final SinkEventAttributes ITALIC;
+
+ /**
+ * An unmodifiable attribute set containing only a monospaced attribute.
+ */
+ public static final SinkEventAttributes MONOSPACED;
+
+ /**
+ * An unmodifiable attribute set containing only a left attribute.
+ */
+ public static final SinkEventAttributes LEFT;
+
+ /**
+ * An unmodifiable attribute set containing only a right attribute.
+ */
+ public static final SinkEventAttributes RIGHT;
+
+ /**
+ * An unmodifiable attribute set containing only a center attribute.
+ */
+ public static final SinkEventAttributes CENTER;
+
+ /**
+ * An unmodifiable attribute set containing only a justify attribute.
+ */
+ public static final SinkEventAttributes JUSTIFY;
+
+ static {
+ UNDERLINE = new SinkEventAttributeSet(DECORATION,
"underline").unmodifiable();
+ OVERLINE = new SinkEventAttributeSet(DECORATION,
"overline").unmodifiable();
+ LINETHROUGH = new SinkEventAttributeSet(DECORATION,
"line-through").unmodifiable();
+ SOURCE = new SinkEventAttributeSet(DECORATION,
"source").unmodifiable();
+
+ BOLD = new SinkEventAttributeSet(STYLE, "bold").unmodifiable();
+ ITALIC = new SinkEventAttributeSet(STYLE, "italic").unmodifiable();
+ MONOSPACED = new SinkEventAttributeSet(STYLE,
"monospaced").unmodifiable();
+
+ LEFT = new SinkEventAttributeSet(ALIGN, "left").unmodifiable();
+ RIGHT = new SinkEventAttributeSet(ALIGN, "right").unmodifiable();
+ CENTER = new SinkEventAttributeSet(ALIGN, "center").unmodifiable();
+ JUSTIFY = new SinkEventAttributeSet(ALIGN, "justify").unmodifiable();
+ }
+
+ private Map<String, Object> attribs;
+
+ private AttributeSet resolveParent;
+
+ /**
+ * Constructs a new, empty SinkEventAttributeSet with default size 5.
+ */
+ public SinkEventAttributeSet() {
+ this(5);
+ }
+
+ /**
+ * Constructs a new, empty SinkEventAttributeSet with the specified
initial size.
+ *
+ * @param size the initial number of attribs.
+ */
+ public SinkEventAttributeSet(int size) {
+ attribs = new LinkedHashMap<>(size);
+ }
+
+ /**
+ * Constructs a new SinkEventAttributeSet with the attribute name-value
+ * mappings as given by the specified String array.
+ * Each even index of the array is an attribute name, and the following
odd index is the corresponding attribute value.
+ * This constructor only supports String attribute values.
+ *
+ * @param attributes the specified String array. If the length of this
array
+ * is not an even number, an IllegalArgumentException is thrown.
+ */
+ public SinkEventAttributeSet(String... attributes) {
+ int n = attributes.length;
+
+ if ((n % 2) != 0) {
+ throw new IllegalArgumentException("Missing attribute!");
+ }
+
+ attribs = new LinkedHashMap<>(n / 2);
+
+ for (int i = 0; i < n; i += 2) {
+ attribs.put(attributes[i], attributes[i + 1]);
+ }
+ }
+
+ /**
+ * Constructs a new SinkEventAttributeSet with the same attribute
name-value
+ * mappings as in the specified AttributeSet.
+ *
+ * @param attributes the specified AttributeSet.
+ */
+ public SinkEventAttributeSet(AttributeSet attributes) {
+ attribs = new LinkedHashMap<>(attributes.getAttributeCount());
+
+ Enumeration<?> names = attributes.getAttributeNames();
+
+ while (names.hasMoreElements()) {
+ Object name = names.nextElement();
+
+ attribs.put(name.toString(), attributes.getAttribute(name));
+ }
+ }
+
+ /**
+ * Replace this AttributeSet by an unmodifiable view of itself.
+ * Any subsequent attempt to add, remove or modify the underlying mapping
+ * will result in an UnsupportedOperationException.
+ *
+ * @return an unmodifiable view of this AttributeSet.
+ * @since 1.1.1
+ */
+ public SinkEventAttributeSet unmodifiable() {
+ this.attribs = Collections.unmodifiableMap(attribs);
+
+ return this;
+ }
+
+ /**
+ * Checks whether the set of attribs is empty.
+ *
+ * @return true if the set is empty.
+ */
+ public boolean isEmpty() {
+ return attribs.isEmpty();
+ }
+
+ /**
+ * {@inheritDoc}
+ *
+ * @return a int.
+ */
+ public int getAttributeCount() {
+ return attribs.size();
+ }
+
+ public boolean isDefined(Object attrName) {
+ return attribs.containsKey(attrName);
+ }
+
+ public boolean isEqual(AttributeSet attr) {
+ return ((getAttributeCount() == attr.getAttributeCount()) &&
containsAttributes(attr));
+ }
+
+ /**
+ * {@inheritDoc}
+ *
+ * @return a {@link javax.swing.text.AttributeSet} object.
+ */
+ public AttributeSet copyAttributes() {
+ return ((AttributeSet) clone());
+ }
+
+ /**
+ * {@inheritDoc}
+ *
+ * @return a {@link java.util.Enumeration} object.
+ */
+ public Enumeration<String> getAttributeNames() {
+ return Collections.enumeration(attribs.keySet());
+ }
+
+ public Object getAttribute(Object key) {
+ Object value = attribs.get(key);
+
+ if (value == null) {
+ AttributeSet parent = getResolveParent();
+
+ if (parent != null) {
+ value = parent.getAttribute(key);
+ }
+ }
+
+ return value;
+ }
+
+ public boolean containsAttribute(Object name, Object value) {
+ return value.equals(getAttribute(name));
+ }
+
+ public boolean containsAttributes(AttributeSet attributes) {
+ boolean result = true;
+
+ Enumeration<?> names = attributes.getAttributeNames();
+
+ while (result && names.hasMoreElements()) {
+ Object name = names.nextElement();
+ result = attributes.getAttribute(name).equals(getAttribute(name));
+ }
+
+ return result;
+ }
+
+ /**
+ * {@inheritDoc}
+ *
+ * Adds an attribute with the given name and value.
+ */
+ public void addAttribute(Object name, Object value) {
+ attribs.put(name.toString(), value);
+ }
+
+ public void addAttributes(AttributeSet attributes) {
+ if (attributes == null || attributes.getAttributeCount() == 0) {
+ return;
+ }
+
+ Enumeration<?> names = attributes.getAttributeNames();
+
+ while (names.hasMoreElements()) {
+ Object name = names.nextElement();
+
+ addAttribute(name, attributes.getAttribute(name));
+ }
+ }
+
+ public void removeAttribute(Object name) {
+ attribs.remove(name);
+ }
+
+ public void removeAttributes(Enumeration<?> names) {
+ while (names.hasMoreElements()) {
+ removeAttribute(names.nextElement());
+ }
+ }
+
+ /**
+ * {@inheritDoc}
+ *
+ * @param attributes a {@link javax.swing.text.AttributeSet} object.
+ */
+ public void removeAttributes(AttributeSet attributes) {
+ if (attributes == null) {
+ return;
+ } else if (attributes == this) {
+ attribs.clear();
+ } else {
+ Enumeration<?> names = attributes.getAttributeNames();
+
+ while (names.hasMoreElements()) {
+ Object name = names.nextElement();
+ Object value = attributes.getAttribute(name);
+
+ if (value.equals(getAttribute(name))) {
+ removeAttribute(name);
+ }
+ }
+ }
+ }
+
+ /**
+ * {@inheritDoc}
+ *
+ * @return a {@link javax.swing.text.AttributeSet} object.
+ */
+ public AttributeSet getResolveParent() {
+ return this.resolveParent;
+ }
+
+ public void setResolveParent(AttributeSet parent) {
+ this.resolveParent = parent;
+ }
+
+ @Override
+ public Set<Entry<String, Object>> entrySet() {
+ return attribs.entrySet();
+ }
+
+ @Override
+ public Object clone() {
+ SinkEventAttributeSet attr = new SinkEventAttributeSet(attribs.size());
+ attr.attribs = new LinkedHashMap<>(attribs);
+
+ if (resolveParent != null) {
+ attr.resolveParent = resolveParent.copyAttributes();
+ }
+
+ return attr;
+ }
+
+ @Override
+ public int hashCode() {
+ final int parentHash = (resolveParent == null ? 0 :
resolveParent.hashCode());
+
+ return attribs.hashCode() + parentHash;
+ }
+
+ @Override
+ public boolean equals(Object obj) {
+ if (this == obj) {
+ return true;
+ }
+
+ if (obj instanceof SinkEventAttributeSet) {
+ return isEqual((SinkEventAttributeSet) obj);
+ }
+
+ return false;
+ }
Review Comment:
Correct as an observation, but out of scope here: this is pre-existing code
that the PR only moves, byte for byte. Changing `equals` or `hashCode`
semantics inside a move commit would hide a behaviour change in a diff that
otherwise has none.
For what it is worth, the impact is currently theoretical: `resolveParent`
is never set anywhere in Doxia or in the report plugins that use this class,
only in the class's own unit test. Happy to raise it separately if a maintainer
wants it addressed.
--
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]
