Index: trunk/src/org/openstreetmap/josm/actions/ValidateAction.java =================================================================== --- trunk/src/org/openstreetmap/josm/actions/ValidateAction.java (revision 18959) +++ trunk/src/org/openstreetmap/josm/actions/ValidateAction.java (revision 18960) @@ -13,5 +13,4 @@ import org.openstreetmap.josm.data.validation.Test; import org.openstreetmap.josm.data.validation.ValidationTask; -import org.openstreetmap.josm.data.validation.util.AggregatePrimitivesVisitor; import org.openstreetmap.josm.gui.MainApplication; import org.openstreetmap.josm.gui.MapFrame; @@ -71,6 +70,4 @@ lastSelection = null; } else { - AggregatePrimitivesVisitor v = new AggregatePrimitivesVisitor(); - selection = v.visit(selection); lastSelection = selection; } Index: trunk/src/org/openstreetmap/josm/actions/upload/ValidateUploadHook.java =================================================================== --- trunk/src/org/openstreetmap/josm/actions/upload/ValidateUploadHook.java (revision 18959) +++ trunk/src/org/openstreetmap/josm/actions/upload/ValidateUploadHook.java (revision 18960) @@ -7,4 +7,5 @@ import java.awt.GridBagLayout; import java.util.Collection; +import java.util.HashSet; import java.util.List; import java.util.concurrent.atomic.AtomicBoolean; @@ -18,5 +19,4 @@ import org.openstreetmap.josm.data.validation.TestError; import org.openstreetmap.josm.data.validation.ValidationTask; -import org.openstreetmap.josm.data.validation.util.AggregatePrimitivesVisitor; import org.openstreetmap.josm.gui.ExtendedDialog; import org.openstreetmap.josm.gui.MainApplication; @@ -46,7 +46,7 @@ public boolean checkUpload(APIDataSet apiDataSet) { AtomicBoolean returnCode = new AtomicBoolean(); - AggregatePrimitivesVisitor v = new AggregatePrimitivesVisitor(); - v.visit(apiDataSet.getPrimitivesToAdd()); - Collection visited = v.visit(apiDataSet.getPrimitivesToUpdate()); + Collection toCheck = new HashSet<>(); + toCheck.addAll(apiDataSet.getPrimitivesToAdd()); + toCheck.addAll(apiDataSet.getPrimitivesToUpdate()); OsmValidator.initializeTests(); new ValidationTask(errors -> { @@ -59,5 +59,5 @@ GuiHelper.runInEDTAndWait(() -> returnCode.set(displayErrorScreen(errors))); } - }, null, OsmValidator.getEnabledTests(true), visited, null, true).run(); + }, null, OsmValidator.getEnabledTests(true), toCheck, null, true).run(); return returnCode.get(); Index: trunk/src/org/openstreetmap/josm/data/preferences/sources/ValidatorPrefHelper.java =================================================================== --- trunk/src/org/openstreetmap/josm/data/preferences/sources/ValidatorPrefHelper.java (revision 18959) +++ trunk/src/org/openstreetmap/josm/data/preferences/sources/ValidatorPrefHelper.java (revision 18960) @@ -58,4 +58,18 @@ /** + * See #23397 + * The preferences key for the addition of parent objects for modified objects + */ + public static final BooleanProperty PREF_ADD_PARENTS = new BooleanProperty(PREFIX + ".partial.add.parents", true); + + /** + * See #23397 + * The preferences key for the deletion of results which do not belong to the selection + * or the parents of modified objects. + * + */ + public static final BooleanProperty PREF_REMOVE_IRRELEVANT = new BooleanProperty(PREFIX + ".partial.removeIrrelevant", true); + + /** * Constructs a new {@code PresetPrefHelper}. */ Index: trunk/src/org/openstreetmap/josm/data/validation/Test.java =================================================================== --- trunk/src/org/openstreetmap/josm/data/validation/Test.java (revision 18959) +++ trunk/src/org/openstreetmap/josm/data/validation/Test.java (revision 18960) @@ -7,6 +7,8 @@ import java.util.ArrayList; import java.util.Collection; +import java.util.HashSet; import java.util.List; import java.util.Optional; +import java.util.Set; import java.util.function.Predicate; import java.util.stream.Collectors; @@ -384,3 +386,21 @@ return "Java: " + this.getClass().getName(); } + + /** + * Filter the list of errors, remove all which do not concern the given list of primitives + * @param given the list of primitives + * @since 18960 + */ + public void removeIrrelevantErrors(Collection given) { + if (errors == null || errors.isEmpty()) + return; + // filter errors for those which are needed, don't show errors for objects which were not in the selection + final Set relevant; + if (given instanceof Set) { + relevant = (Set) given; + } else { + relevant = new HashSet<>(given); + } + errors.removeIf(e -> !e.isConcerned(relevant)); + } } Index: trunk/src/org/openstreetmap/josm/data/validation/TestError.java =================================================================== --- trunk/src/org/openstreetmap/josm/data/validation/TestError.java (revision 18959) +++ trunk/src/org/openstreetmap/josm/data/validation/TestError.java (revision 18960) @@ -13,4 +13,5 @@ import java.util.Locale; import java.util.Map; +import java.util.Set; import java.util.TreeSet; import java.util.function.Supplier; @@ -63,4 +64,6 @@ /** If this error is selected */ private boolean selected; + /** If all relevant primitives are known*/ + private boolean incompletePrimitives; /** Supplying a command to fix the error */ private final Supplier fixingCommand; @@ -81,4 +84,5 @@ private Collection highlighted; private Supplier fixingCommand; + private boolean incompletePrimitives; Builder(Test tester, Severity severity, int code) { @@ -215,4 +219,14 @@ CheckParameterUtil.ensureParameterNotNull(highlighted, "highlighted"); this.highlighted = Collections.singleton(highlighted); + return this; + } + + /** + * Sets a flag that the list of primitives may be incomplete. See #23397 + * + * @return {@code this} + */ + public Builder imcompletePrimitives() { + this.incompletePrimitives = true; return this; } @@ -277,4 +291,5 @@ this.uniqueCode = builder.uniqueCode; this.fixingCommand = builder.fixingCommand; + this.incompletePrimitives = builder.incompletePrimitives; } @@ -667,3 +682,19 @@ } + /** + * Check if any of the primitives in this error occurs in the given set of primitives. + * @param given the set of primitives + * @return true if any of the primitives in this error occurs in the given set of primitives, else false + * @since 18960 + */ + public boolean isConcerned(Set given) { + if (incompletePrimitives) + return true; + for (OsmPrimitive p : getPrimitives()) { + if (given.contains(p)) { + return true; + } + } + return false; + } } Index: trunk/src/org/openstreetmap/josm/data/validation/ValidationTask.java =================================================================== --- trunk/src/org/openstreetmap/josm/data/validation/ValidationTask.java (revision 18959) +++ trunk/src/org/openstreetmap/josm/data/validation/ValidationTask.java (revision 18960) @@ -7,5 +7,8 @@ import java.util.ArrayList; import java.util.Collection; +import java.util.Collections; +import java.util.HashSet; import java.util.List; +import java.util.Set; import java.util.function.BiConsumer; import java.util.function.Consumer; @@ -14,5 +17,7 @@ import org.openstreetmap.josm.data.osm.OsmPrimitive; +import org.openstreetmap.josm.data.osm.Way; import org.openstreetmap.josm.data.preferences.sources.ValidatorPrefHelper; +import org.openstreetmap.josm.data.validation.util.AggregatePrimitivesVisitor; import org.openstreetmap.josm.gui.MainApplication; import org.openstreetmap.josm.gui.MapFrame; @@ -31,5 +36,5 @@ private final Consumer> onFinish; private Collection tests; - private final Collection validatedPrimitives; + private final Collection initialPrimitives; private final Collection formerValidatedPrimitives; private final boolean beforeUpload; @@ -72,5 +77,5 @@ false /*don't ignore exceptions */); this.onFinish = onFinish; - this.validatedPrimitives = validatedPrimitives; + this.initialPrimitives = validatedPrimitives; this.formerValidatedPrimitives = formerValidatedPrimitives; this.tests = tests; @@ -78,4 +83,33 @@ } + /** + * Find objects parent objects of given objects which should be checked for geometry problems + * or mismatches between child tags and parent tags. + * @param primitives the given objects + * @return the collection of relevant parent objects + */ + private static Set getRelevantParents(Collection primitives) { + Set addedWays = new HashSet<>(); + Set addedRelations = new HashSet<>(); + for (OsmPrimitive p : primitives) { + for (OsmPrimitive parent : p.getReferrers()) { + if (parent.isDeleted()) + continue; + if (parent instanceof Way) + addedWays.add(parent); + else + addedRelations.add(parent); + } + } + + // allow to find invalid multipolygon relations caused by moved nodes + OsmPrimitive.getParentRelations(addedWays).stream().filter(r -> r.isMultipolygon() && !r.isDeleted()) + .forEach(addedRelations::add); + HashSet extendedSet = new HashSet<>(); + extendedSet.addAll(addedWays); + extendedSet.addAll(addedRelations); + return extendedSet; + + } protected ValidationTask(ProgressMonitor progressMonitor, Collection tests, @@ -123,6 +157,23 @@ if (Utils.isEmpty(tests)) return; + int testCounter = 0; + final boolean isPartial = this.beforeUpload || formerValidatedPrimitives != null; + Set filter = null; + Collection validatedPrimitives = initialPrimitives; + if (isPartial) { + Set other = Collections.emptySet(); + if (Boolean.TRUE.equals(ValidatorPrefHelper.PREF_ADD_PARENTS.get())) { + other = getRelevantParents(initialPrimitives); + } + HashSet extendedSet = new HashSet<>(); + AggregatePrimitivesVisitor v = new AggregatePrimitivesVisitor(); + extendedSet.addAll(v.visit(initialPrimitives)); + extendedSet.addAll(other); + validatedPrimitives = extendedSet; + filter = new HashSet<>(initialPrimitives); + filter.addAll(other); + } getProgressMonitor().setTicksCount(tests.size() * validatedPrimitives.size()); - int testCounter = 0; + for (Test test : tests) { if (canceled) @@ -132,8 +183,13 @@ test.setBeforeUpload(this.beforeUpload); // Pre-upload checks only run on a partial selection. - test.setPartialSelection(this.beforeUpload || formerValidatedPrimitives != null); + test.setPartialSelection(isPartial); test.startTest(getProgressMonitor().createSubTaskMonitor(validatedPrimitives.size(), false)); test.visit(validatedPrimitives); test.endTest(); + if (isPartial && Boolean.TRUE.equals(ValidatorPrefHelper.PREF_REMOVE_IRRELEVANT.get())) { + // #23397: remove errors for objects which were not in the initial list of primitives + test.removeIrrelevantErrors(filter); + } + errors.addAll(test.getErrors()); if (this.testConsumer != null) { Index: trunk/src/org/openstreetmap/josm/data/validation/tests/CrossingWays.java =================================================================== --- trunk/src/org/openstreetmap/josm/data/validation/tests/CrossingWays.java (revision 18959) +++ trunk/src/org/openstreetmap/josm/data/validation/tests/CrossingWays.java (revision 18960) @@ -7,4 +7,5 @@ import java.util.ArrayList; import java.util.Arrays; +import java.util.Collection; import java.util.HashMap; import java.util.HashSet; @@ -17,4 +18,6 @@ import org.openstreetmap.josm.data.coor.EastNorth; import org.openstreetmap.josm.data.coor.ILatLon; +import org.openstreetmap.josm.data.osm.DataSet; +import org.openstreetmap.josm.data.osm.OsmDataManager; import org.openstreetmap.josm.data.osm.OsmPrimitive; import org.openstreetmap.josm.data.osm.OsmUtils; @@ -82,4 +85,5 @@ /** The already detected ways in error */ private final Map, List> seenWays = new HashMap<>(50); + private final Set waysToTest = new HashSet<>(); protected final int code; @@ -306,7 +310,28 @@ @Override public void endTest() { - super.endTest(); + final Collection selection; + if (this instanceof SelfCrossing || !partialSelection) { + selection = waysToTest; + } else { + selection = new HashSet<>(); + DataSet ds = OsmDataManager.getInstance().getActiveDataSet(); + if (ds != null) { + for (Way w: waysToTest) { + selection.addAll(ds.searchWays(w.getBBox())); + } + } + } + for (Way w : selection) { + if (!w.isDeleted() && isPrimitiveUsable(w)) { + testWay(w); + } + } + // free storage cellSegments.clear(); seenWays.clear(); + waysToTest.clear(); + if (partialSelection) + removeIrrelevantErrors(waysToTest); + super.endTest(); } @@ -345,4 +370,8 @@ @Override public void visit(Way w) { + waysToTest.add(w); + } + + private void testWay(Way w) { boolean findSelfCrossingOnly = this instanceof SelfCrossing; if (findSelfCrossingOnly) { @@ -483,4 +512,5 @@ SelfCrossing test = new SelfCrossing(); test.visit(way); + test.endTest(); return !test.getErrors().isEmpty(); } Index: trunk/src/org/openstreetmap/josm/data/validation/tests/DuplicateWay.java =================================================================== --- trunk/src/org/openstreetmap/josm/data/validation/tests/DuplicateWay.java (revision 18959) +++ trunk/src/org/openstreetmap/josm/data/validation/tests/DuplicateWay.java (revision 18960) @@ -8,4 +8,5 @@ import java.util.Collections; import java.util.HashSet; +import java.util.LinkedHashSet; import java.util.LinkedList; import java.util.List; @@ -104,4 +105,5 @@ /** Set of known hashcodes for list of coordinates **/ private Set knownHashCodes; + private List waysToCheck; /** @@ -116,4 +118,5 @@ public void startTest(ProgressMonitor monitor) { super.startTest(monitor); + waysToCheck = new ArrayList<>(); ways = new MultiMap<>(1000); waysNoTags = new MultiMap<>(1000); @@ -123,5 +126,20 @@ @Override public void endTest() { - super.endTest(); + if (partialSelection && !waysToCheck.isEmpty()) { + // make sure that we have the error candidates even if not selected + Set extended = new LinkedHashSet<>(waysToCheck); + for (Way w : waysToCheck) { + // select a node, anyone can be used but a middle node is less likely to have many parent ways + final Node n = w.getNode(w.getNodesCount()/2); + // check the ways which might be in the same position + for (Way other : n.getParentWays()) { + if (other != w && !other.isDeleted() && other.isUsable() + && other.getNodesCount() == w.getNodesCount()) + extended.add(other); + } + } + extended.forEach(this::checkWay); + } + for (Set duplicated : ways.values()) { if (duplicated.size() > 1) { @@ -166,4 +184,6 @@ waysNoTags = null; knownHashCodes = null; + waysToCheck = null; + super.endTest(); } @@ -182,4 +202,11 @@ if (!w.isUsable()) return; + if (partialSelection) + waysToCheck.add(w); + else + checkWay(w); + } + + private void checkWay(Way w) { List wLat = getOrderedNodes(w); // If this way has not direction-dependant keys, make sure the list is ordered the same for all ways (fix #8015) Index: trunk/src/org/openstreetmap/josm/data/validation/tests/MapCSSTagCheckerRule.java =================================================================== --- trunk/src/org/openstreetmap/josm/data/validation/tests/MapCSSTagCheckerRule.java (revision 18959) +++ trunk/src/org/openstreetmap/josm/data/validation/tests/MapCSSTagCheckerRule.java (revision 18960) @@ -34,4 +34,5 @@ import org.openstreetmap.josm.gui.mappaint.MultiCascade; import org.openstreetmap.josm.gui.mappaint.mapcss.Condition; +import org.openstreetmap.josm.gui.mappaint.mapcss.ConditionFactory.ClassCondition; import org.openstreetmap.josm.gui.mappaint.mapcss.Expression; import org.openstreetmap.josm.gui.mappaint.mapcss.Instruction; @@ -378,4 +379,18 @@ } } + } else if (env.parent != null) { + boolean imcompletePrimitives = false; + if (matchingSelector instanceof Selector.ChildOrParentSelector) { + Selector right = ((Selector.ChildOrParentSelector) matchingSelector).right; + if (right.getConditions().stream().anyMatch(ClassCondition.class::isInstance)) { + // see #23397 + // TODO: find a way to collect all and only those parent objects which triggered this error + imcompletePrimitives = true; + } + } + if (imcompletePrimitives) + res.add(errorBuilder.primitives(p).highlight(p).imcompletePrimitives().build()); + else + res.add(errorBuilder.primitives(p, (OsmPrimitive) env.parent).highlight(p).build()); } else { res.add(errorBuilder.primitives(p).build()); Index: trunk/src/org/openstreetmap/josm/data/validation/util/AggregatePrimitivesVisitor.java =================================================================== --- trunk/src/org/openstreetmap/josm/data/validation/util/AggregatePrimitivesVisitor.java (revision 18959) +++ trunk/src/org/openstreetmap/josm/data/validation/util/AggregatePrimitivesVisitor.java (revision 18960) @@ -15,5 +15,4 @@ * A visitor that aggregates all primitives it visits. *

- * The primitives are sorted according to their type: first nodes, then ways. * * @author frsantos @@ -25,5 +24,5 @@ /** * Visits a collection of primitives - * @param data The collection of primitives + * @param data The collection of primitives in no specific order. * @return The aggregated primitives */