Copilot commented on code in PR #1073: URL: https://github.com/apache/maven-doxia/pull/1073#discussion_r3740896959
########## 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: equals(Object) and hashCode() are inconsistent: hashCode() incorporates resolveParent, but equals() delegates to isEqual(...) and ignores resolveParent. This violates the equals/hashCode contract and can break use of SinkEventAttributeSet as a key in hash-based collections once a resolveParent is set. ########## 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: The deprecated compatibility subclass does not override clone(). It inherits org.apache.maven.doxia.sink.SinkEventAttributeSet.clone(), which creates a new instance of the *new* class, so calling clone() on the legacy org.apache.maven.doxia.sink.impl.SinkEventAttributeSet will return the wrong runtime type. Existing binaries that cast the clone() result back to the legacy type would start failing with ClassCastException. -- 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]
