Index: trunk/src/org/openstreetmap/josm/data/validation/tests/MapCSSTagChecker.java =================================================================== --- trunk/src/org/openstreetmap/josm/data/validation/tests/MapCSSTagChecker.java (revision 15063) +++ trunk/src/org/openstreetmap/josm/data/validation/tests/MapCSSTagChecker.java (revision 15064) @@ -13,5 +13,4 @@ import java.text.MessageFormat; import java.util.ArrayList; -import java.util.Arrays; import java.util.Collection; import java.util.Collections; @@ -40,4 +39,5 @@ import org.openstreetmap.josm.data.osm.DataSet; import org.openstreetmap.josm.data.osm.INode; +import org.openstreetmap.josm.data.osm.IPrimitive; import org.openstreetmap.josm.data.osm.IRelation; import org.openstreetmap.josm.data.osm.IWay; @@ -70,4 +70,6 @@ import org.openstreetmap.josm.gui.mappaint.mapcss.Selector; import org.openstreetmap.josm.gui.mappaint.mapcss.Selector.AbstractSelector; +import org.openstreetmap.josm.gui.mappaint.mapcss.Selector.ChildOrParentSelector; +import org.openstreetmap.josm.gui.mappaint.mapcss.Selector.ChildOrParentSelectorType; import org.openstreetmap.josm.gui.mappaint.mapcss.Selector.GeneralSelector; import org.openstreetmap.josm.gui.mappaint.mapcss.Selector.OptimizedGeneralSelector; @@ -734,10 +736,11 @@ * @return an instance of {@link TestError}, or returns null if the primitive does not give rise to an error. */ - TestError getErrorForPrimitive(OsmPrimitive p) { + List getErrorsForPrimitive(OsmPrimitive p) { final Environment env = new Environment(p); - return getErrorForPrimitive(p, whichSelectorMatchesEnvironment(env), env, null); - } - - TestError getErrorForPrimitive(OsmPrimitive p, Selector matchingSelector, Environment env, Test tester) { + return getErrorsForPrimitive(p, whichSelectorMatchesEnvironment(env), env, null); + } + + private List getErrorsForPrimitive(OsmPrimitive p, Selector matchingSelector, Environment env, Test tester) { + List res = new ArrayList<>(); if (matchingSelector != null && !errors.isEmpty()) { final Command fix = fixPrimitive(p); @@ -745,21 +748,28 @@ final String description1 = group == null ? description : group; final String description2 = group == null ? null : description; - final List primitives; + TestError.Builder errorBuilder = TestError.builder(tester, getSeverity(), 3000) + .messageWithManuallyTranslatedDescription(description1, description2, matchingSelector.toString()); + if (fix != null) { + errorBuilder = errorBuilder.fix(() -> fix); + } if (env.child instanceof OsmPrimitive) { - primitives = Arrays.asList(p, (OsmPrimitive) env.child); + res.add(errorBuilder.primitives(p, (OsmPrimitive) env.child).build()); + } else if (env.children != null) { + for (IPrimitive c : env.children) { + if (c instanceof OsmPrimitive) { + errorBuilder = TestError.builder(tester, getSeverity(), 3000) + .messageWithManuallyTranslatedDescription(description1, description2, + matchingSelector.toString()); + if (fix != null) { + errorBuilder = errorBuilder.fix(() -> fix); + } + res.add(errorBuilder.primitives(p, (OsmPrimitive) c).build()); + } + } } else { - primitives = Collections.singletonList(p); - } - final TestError.Builder error = TestError.builder(tester, getSeverity(), 3000) - .messageWithManuallyTranslatedDescription(description1, description2, matchingSelector.toString()) - .primitives(primitives); - if (fix != null) { - return error.fix(() -> fix).build(); - } else { - return error.build(); - } - } else { - return null; - } + res.add(errorBuilder.primitives(p).build()); + } + } + return res; } @@ -855,4 +865,23 @@ MapCSSRule r = candidates.next(); env.clearSelectorMatchingInformation(); + if (partialSelection && r.selector instanceof Selector.ChildOrParentSelector) { + ChildOrParentSelector sel = (Selector.ChildOrParentSelector) r.selector; + if (sel.type == ChildOrParentSelectorType.ELEMENT_OF && p.getDataSet() != null) { + List toCheck = new ArrayList<>(); + toCheck.addAll(p.getDataSet().searchWays(p.getBBox())); + toCheck.addAll(p.getDataSet().searchRelations(p.getBBox())); + toCheck.removeIf(OsmPrimitive::isSelected); + if (!toCheck.isEmpty()) { + Set> checksCol = Collections.singleton(Collections.singleton(indexData.getCheck(r))); + for (OsmPrimitive p2 : toCheck) { + for (TestError e : getErrorsForPrimitive(p2, includeOtherSeverity, checksCol)) { + if (e.getPrimitives().contains(p)) { + addIfNotSimilar(e, res); + } + } + } + } + } + } if (r.selector.matches(env)) { // as side effect env.parent will be set (if s is a child selector) TagCheck check = indexData.getCheck(r); @@ -864,14 +893,38 @@ r.declaration.execute(env); if (!check.errors.isEmpty()) { - final TestError error = check.getErrorForPrimitive(p, r.selector, env, new MapCSSTagCheckerAndRule(check.rule)); - if (error != null) { - res.add(error); + for (TestError e: check.getErrorsForPrimitive(p, r.selector, env, new MapCSSTagCheckerAndRule(check.rule))) { + addIfNotSimilar(e, res); } } - } } } return res; + } + + /** + * See #12627 + * Add error to given list if list doesn't already contain a similar error. + * Similar means same code and description and same combination of primitives and same combination of highlighted objects, + * but maybe with different orders. + * @param toAdd the error to add + * @param errors the list of errors + */ + private static void addIfNotSimilar(TestError toAdd, List errors) { + boolean isDup = false; + if (toAdd.getPrimitives().size() >= 2) { + for (TestError e : errors) { + if (e.getCode() == toAdd.getCode() && e.getMessage().equals(toAdd.getMessage()) + && e.getPrimitives().size() == toAdd.getPrimitives().size() + && e.getPrimitives().containsAll(toAdd.getPrimitives()) + && e.getHighlighted().size() == toAdd.getHighlighted().size() + && e.getHighlighted().containsAll(toAdd.getHighlighted())) { + isDup = true; + break; + } + } + } + if (!isDup) + errors.add(toAdd); } @@ -891,8 +944,5 @@ check.rule.declaration.execute(env); if (!ignoreError && !check.errors.isEmpty()) { - final TestError error = check.getErrorForPrimitive(p, selector, env, new MapCSSTagCheckerAndRule(check.rule)); - if (error != null) { - r.add(error); - } + r.addAll(check.getErrorsForPrimitive(p, selector, env, new MapCSSTagCheckerAndRule(check.rule))); } } @@ -909,5 +959,7 @@ @Override public void check(OsmPrimitive p) { - errors.addAll(getErrorsForPrimitive(p, ValidatorPrefHelper.PREF_OTHER.get())); + for (TestError e : getErrorsForPrimitive(p, ValidatorPrefHelper.PREF_OTHER.get())) { + addIfNotSimilar(e, errors); + } } Index: trunk/src/org/openstreetmap/josm/gui/mappaint/Environment.java =================================================================== --- trunk/src/org/openstreetmap/josm/gui/mappaint/Environment.java (revision 15063) +++ trunk/src/org/openstreetmap/josm/gui/mappaint/Environment.java (revision 15064) @@ -1,4 +1,7 @@ // License: GPL. For details, see LICENSE file. package org.openstreetmap.josm.gui.mappaint; + +import java.util.LinkedHashSet; +import java.util.Set; import org.openstreetmap.josm.data.osm.IPrimitive; @@ -59,4 +62,9 @@ */ public Integer count; + + /** + * Set of matched children filled by ContainsFinder and CrossingFinder, null if nothing matched + */ + public Set children; /** @@ -109,4 +117,5 @@ this.count = other.count; this.context = other.getContext(); + this.children = other.children == null ? null : new LinkedHashSet<>(other.children); } @@ -274,4 +283,5 @@ index = null; count = null; + children = null; } Index: trunk/src/org/openstreetmap/josm/gui/mappaint/mapcss/Selector.java =================================================================== --- trunk/src/org/openstreetmap/josm/gui/mappaint/mapcss/Selector.java (revision 15063) +++ trunk/src/org/openstreetmap/josm/gui/mappaint/mapcss/Selector.java (revision 15064) @@ -7,4 +7,5 @@ import java.util.Collection; import java.util.Collections; +import java.util.LinkedHashSet; import java.util.List; import java.util.NoSuchElementException; @@ -258,4 +259,11 @@ return !e.osm.equals(p) && p.isUsable(); } + + protected void addToChildren(Environment e, IPrimitive p) { + if (e.children == null) { + e.children = new LinkedHashSet<>(); + } + e.children.add(p); + } } @@ -299,9 +307,9 @@ @Override public void visit(IWay w) { - if (e.child == null && Objects.equals(layer, OsmUtils.getLayer(w)) + if (Objects.equals(layer, OsmUtils.getLayer(w)) && left.matches(new Environment(w).withParent(e.osm)) && e.osm instanceof IWay && Geometry.PolygonIntersection.CROSSING.equals( Geometry.polygonIntersection(w.getNodes(), ((IWay) e.osm).getNodes()))) { - e.child = w; + addToChildren(e, w); } } @@ -316,9 +324,9 @@ @Override public void visit(INode n) { - if (e.child == null && left.matches(new Environment(n).withParent(e.osm)) + if (left.matches(new Environment(n).withParent(e.osm)) && ((e.osm instanceof IWay && Geometry.nodeInsidePolygon(n, ((IWay) e.osm).getNodes())) || (e.osm instanceof Relation && ( (Relation) e.osm).isMultipolygon() && Geometry.isNodeInsideMultiPolygon(n, (Relation) e.osm, null)))) { - e.child = n; + addToChildren(e, n); } } @@ -326,5 +334,5 @@ @Override public void visit(IWay w) { - if (e.child == null && left.matches(new Environment(w).withParent(e.osm)) + if (left.matches(new Environment(w).withParent(e.osm)) && ((e.osm instanceof IWay && Geometry.PolygonIntersection.FIRST_INSIDE_SECOND.equals( Geometry.polygonIntersection(w.getNodes(), ((IWay) e.osm).getNodes()))) @@ -332,5 +340,5 @@ (Relation) e.osm).isMultipolygon() && Geometry.isPolygonInsideMultiPolygon(w.getNodes(), (Relation) e.osm, null)))) { - e.child = w; + addToChildren(e, w); } } @@ -388,14 +396,14 @@ } - return e.child != null; + return e.children != null; } else if (ChildOrParentSelectorType.CROSSING == type && e.osm instanceof IWay) { e.parent = e.osm; - final CrossingFinder crossingFinder = new CrossingFinder(e); if (right instanceof OptimizedGeneralSelector && ((OptimizedGeneralSelector) right).matchesBase(OsmPrimitiveType.WAY)) { + final CrossingFinder crossingFinder = new CrossingFinder(e); crossingFinder.visit(e.osm.getDataSet().searchWays(e.osm.getBBox())); } - return e.child != null; + return e.children != null; } else if (ChildOrParentSelectorType.SIBLING == type) { if (e.osm instanceof INode) {