Index: trunk/data/validator/unnecessary.mapcss =================================================================== --- trunk/data/validator/unnecessary.mapcss (revision 6628) +++ trunk/data/validator/unnecessary.mapcss (revision 6629) @@ -16,2 +16,7 @@ assertNoMatch: "way emergency=designated"; } + +/* see ticket #7639 -- Warn when a node has the same tags as its parent way. */ +way >:sameTags node:tagged { + throwWarning: tr("Nodes duplicating parent way tags"); +} Index: trunk/src/org/openstreetmap/josm/data/osm/AbstractPrimitive.java =================================================================== --- trunk/src/org/openstreetmap/josm/data/osm/AbstractPrimitive.java (revision 6628) +++ trunk/src/org/openstreetmap/josm/data/osm/AbstractPrimitive.java (revision 6629) @@ -671,18 +671,4 @@ /** - * Replies true if other isn't null and has the same tags (key/value-pairs) as this. - * - * @param other the other object primitive - * @return true if other isn't null and has the same tags (key/value-pairs) as this. - */ - public boolean hasSameTags(OsmPrimitive other) { - // We cannot directly use Arrays.equals(keys, other.keys) as keys is not ordered by key - // but we can at least check if both arrays are null or of the same size before creating - // and comparing the key maps (costly operation, see #7159) - return (keys == null && other.keys == null) - || (keys != null && other.keys != null && keys.length == other.keys.length && (keys.length == 0 || getKeys().equals(other.getKeys()))); - } - - /** * What to do, when the tags have changed by one of the tag-changing methods. */ Index: trunk/src/org/openstreetmap/josm/data/osm/OsmPrimitive.java =================================================================== --- trunk/src/org/openstreetmap/josm/data/osm/OsmPrimitive.java (revision 6628) +++ trunk/src/org/openstreetmap/josm/data/osm/OsmPrimitive.java (revision 6629) @@ -10,4 +10,5 @@ import java.util.Collections; import java.util.Date; +import java.util.HashMap; import java.util.HashSet; import java.util.LinkedHashSet; @@ -726,4 +727,20 @@ } + /** + * Returns {@link #getKeys()} for which {@code key} does not fulfill {@link #isUninterestingKey}. + */ + public Map getInterestingTags() { + Map result = new HashMap(); + String[] keys = this.keys; + if (keys != null) { + for (int i = 0; i < keys.length; i += 2) { + if (!isUninterestingKey(keys[i])) { + result.put(keys[i], keys[i + 1]); + } + } + } + return result; + } + private static volatile Match directionKeys = null; private static volatile Match reversedDirectionKeys = null; @@ -1115,4 +1132,19 @@ /** + * Replies true if other isn't null and has the same interesting tags (key/value-pairs) as this. + * + * @param other the other object primitive + * @return true if other isn't null and has the same interesting tags (key/value-pairs) as this. + */ + public boolean hasSameInterestingTags(OsmPrimitive other) { + // We cannot directly use Arrays.equals(keys, other.keys) as keys is not ordered by key + // but we can at least check if both arrays are null or of the same size before creating + // and comparing the key maps (costly operation, see #7159) + return (keys == null && other.keys == null) + || (keys != null && other.keys != null && keys.length == other.keys.length + && (keys.length == 0 || getInterestingTags().equals(other.getInterestingTags()))); + } + + /** * Replies true if this primitive and other are equal with respect to their * semantic attributes. @@ -1133,5 +1165,5 @@ // can't do an equals check on the internal keys array because it is not ordered // - return hasSameTags(other); + return hasSameInterestingTags(other); } Index: trunk/src/org/openstreetmap/josm/data/validation/OsmValidator.java =================================================================== --- trunk/src/org/openstreetmap/josm/data/validation/OsmValidator.java (revision 6628) +++ trunk/src/org/openstreetmap/josm/data/validation/OsmValidator.java (revision 6629) @@ -36,5 +36,4 @@ import org.openstreetmap.josm.data.validation.tests.MultipolygonTest; import org.openstreetmap.josm.data.validation.tests.NameMismatch; -import org.openstreetmap.josm.data.validation.tests.NodesDuplicatingWayTags; import org.openstreetmap.josm.data.validation.tests.OpeningHourTest; import org.openstreetmap.josm.data.validation.tests.OverlappingWays; @@ -110,5 +109,4 @@ DuplicateRelation.class, // ID 1901 .. 1999 WayConnectedToArea.class, // ID 2301 .. 2399 - NodesDuplicatingWayTags.class, // ID 2401 .. 2499 PowerLines.class, // ID 2501 .. 2599 Addresses.class, // ID 2601 .. 2699 Index: trunk/src/org/openstreetmap/josm/data/validation/tests/NodesDuplicatingWayTags.java =================================================================== --- trunk/src/org/openstreetmap/josm/data/validation/tests/NodesDuplicatingWayTags.java (revision 6628) +++ (revision ) @@ -1,61 +1,0 @@ -// License: GPL. For details, see LICENSE file. -package org.openstreetmap.josm.data.validation.tests; - -import static org.openstreetmap.josm.tools.I18n.tr; - -import java.util.HashSet; -import java.util.Set; - -import org.openstreetmap.josm.data.osm.Node; -import org.openstreetmap.josm.data.osm.OsmPrimitive; -import org.openstreetmap.josm.data.osm.Way; -import org.openstreetmap.josm.data.validation.Severity; -import org.openstreetmap.josm.data.validation.Test; -import org.openstreetmap.josm.data.validation.TestError; - -/** - * Warn when a node has the same tags as its parent way. The check is rather - * conservative: it warns only when the tags are identical and important (i.e., - * no warning for a way and a node that only have a "source=PGS" tag). - *

- * See JOSM ticket #7639 for the original request. - * - * @author Mrwojo - */ -public class NodesDuplicatingWayTags extends Test { - - protected static final int NODE_DUPING_PARENT_WAY_TAGS = 2401; - - /** - * Constructs a new {@code NodesDuplicatingWayTags} test. - */ - public NodesDuplicatingWayTags() { - super(tr("Nodes duplicating way tags"), - tr("Checks for nodes that have the same tags as their parent way.")); - } - - @Override - public void visit(Way way) { - // isTagged represents interesting tags (not "source", "created_by", ...) - if (!way.isUsable() || !way.isTagged()) - return; - - // Use a set so you don't report the same node of an area/loop more than once. - Set dupedWayTags = new HashSet(); - - // Check for nodes in the way that have tags identical to the way's tags. - for (Node node : way.getNodes()) { - if (way.hasSameTags(node)) { - dupedWayTags.add(node); - } - } - - if (!dupedWayTags.isEmpty()) { - // Add the way for the warning. - dupedWayTags.add(way); - - errors.add(new TestError(this, Severity.WARNING, tr("Nodes duplicating parent way tags"), - NODE_DUPING_PARENT_WAY_TAGS, dupedWayTags)); - } - } -} Index: trunk/src/org/openstreetmap/josm/gui/mappaint/mapcss/Condition.java =================================================================== --- trunk/src/org/openstreetmap/josm/gui/mappaint/mapcss/Condition.java (revision 6628) +++ trunk/src/org/openstreetmap/josm/gui/mappaint/mapcss/Condition.java (revision 6629) @@ -9,5 +9,4 @@ import org.openstreetmap.josm.data.osm.Node; -import org.openstreetmap.josm.data.osm.OsmUtils; import org.openstreetmap.josm.data.osm.Relation; import org.openstreetmap.josm.data.osm.Tag; @@ -15,4 +14,5 @@ import org.openstreetmap.josm.gui.mappaint.Cascade; import org.openstreetmap.josm.gui.mappaint.Environment; +import org.openstreetmap.josm.tools.CheckParameterUtil; import org.openstreetmap.josm.tools.Predicates; import org.openstreetmap.josm.tools.Utils; @@ -58,5 +58,5 @@ public static Condition createPseudoClassCondition(String id, boolean not, Context context) { - return new PseudoClassCondition(id, not); + return new PseudoClassCondition(id, not, context); } @@ -305,7 +305,8 @@ public final boolean not; - public PseudoClassCondition(String id, boolean not) { + public PseudoClassCondition(String id, boolean not, Context context) { this.id = id; this.not = not; + CheckParameterUtil.ensureThat(!"sameTags".equals(id) || Context.LINK.equals(context), "sameTags only supported in LINK context"); } @@ -322,12 +323,15 @@ return true; return false; - } else if (equal(id, "modified")) + } else if (equal(id, "modified")) { return e.osm.isModified() || e.osm.isNewOrUndeleted(); - else if (equal(id, "new")) + } else if (equal(id, "new")) { return e.osm.isNew(); - else if (equal(id, "connection") && (e.osm instanceof Node)) + } else if (equal(id, "connection") && (e.osm instanceof Node)) { return ((Node) e.osm).isConnectionNode(); - else if (equal(id, "tagged")) + } else if (equal(id, "tagged")) { return e.osm.isTagged(); + } else if ("sameTags".equals(id)) { + return e.osm.hasSameInterestingTags(Utils.firstNonNull(e.child, e.parent)); + } return true; }