Index: /trunk/src/org/openstreetmap/josm/data/osm/OsmPrimitive.java =================================================================== --- /trunk/src/org/openstreetmap/josm/data/osm/OsmPrimitive.java (revision 17349) +++ /trunk/src/org/openstreetmap/josm/data/osm/OsmPrimitive.java (revision 17350) @@ -1118,11 +1118,14 @@ */ public final boolean hasAreaTags() { - return hasKey("landuse", "amenity", "building", "building:part") + return hasKey("building", "landuse", "amenity", "shop", "building:part", "boundary", "historic", "place") || hasTag("area", OsmUtils.TRUE_VALUE) || hasTag("waterway", "riverbank") + || hasTag("highway", "rest_area", "services", "platform") + || hasTag("railway", "platform") || hasTagDifferent("leisure", "picnic_table", "slipway", "firepit") || hasTag("natural", "water", "wood", "scrub", "wetland", "grassland", "heath", "rock", "bare_rock", "sand", "beach", "scree", "bay", "glacier", "shingle", "fell", "reef", "stone", - "mud", "landslide", "sinkhole", "crevasse", "desert"); + "mud", "landslide", "sinkhole", "crevasse", "desert") + || hasTag("aeroway", "aerodrome"); } Index: /trunk/src/org/openstreetmap/josm/data/validation/TestError.java =================================================================== --- /trunk/src/org/openstreetmap/josm/data/validation/TestError.java (revision 17349) +++ /trunk/src/org/openstreetmap/josm/data/validation/TestError.java (revision 17350) @@ -106,5 +106,5 @@ * Sets the error message. * - * @param message The the message of this error group + * @param message The message of this error group * @param marktrDescription The {@linkplain I18n#marktr prepared for i18n} description of this error * @param args The description arguments to be applied in {@link I18n#tr(String, Object...)} Index: /trunk/src/org/openstreetmap/josm/data/validation/tests/OverlappingWays.java =================================================================== --- /trunk/src/org/openstreetmap/josm/data/validation/tests/OverlappingWays.java (revision 17349) +++ /trunk/src/org/openstreetmap/josm/data/validation/tests/OverlappingWays.java (revision 17350) @@ -4,4 +4,5 @@ import static org.openstreetmap.josm.data.validation.tests.CrossingWays.HIGHWAY; import static org.openstreetmap.josm.data.validation.tests.CrossingWays.RAILWAY; +import static org.openstreetmap.josm.data.validation.tests.CrossingWays.WATERWAY; import static org.openstreetmap.josm.tools.I18n.tr; @@ -12,4 +13,5 @@ import java.util.HashMap; import java.util.HashSet; +import java.util.LinkedHashSet; import java.util.List; import java.util.Map; @@ -17,4 +19,5 @@ import java.util.TreeSet; import java.util.function.Predicate; +import java.util.stream.Collectors; import org.openstreetmap.josm.data.osm.Node; @@ -25,8 +28,10 @@ import org.openstreetmap.josm.data.osm.WaySegment; import org.openstreetmap.josm.data.preferences.ListProperty; +import org.openstreetmap.josm.data.preferences.sources.ValidatorPrefHelper; import org.openstreetmap.josm.data.validation.Severity; import org.openstreetmap.josm.data.validation.Test; import org.openstreetmap.josm.data.validation.TestError; import org.openstreetmap.josm.gui.progress.ProgressMonitor; +import org.openstreetmap.josm.spi.preferences.Config; import org.openstreetmap.josm.tools.MultiMap; import org.openstreetmap.josm.tools.Pair; @@ -43,11 +48,20 @@ private MultiMap, WaySegment> nodePairs; + private boolean onlyKnwonLinear; + private boolean includeOther; + private boolean ignoreLayer; + protected static final int OVERLAPPING_HIGHWAY = 101; protected static final int OVERLAPPING_RAILWAY = 102; protected static final int OVERLAPPING_WAY = 103; + protected static final int OVERLAPPING_WATERWAY = 104; protected static final int OVERLAPPING_HIGHWAY_AREA = 111; protected static final int OVERLAPPING_RAILWAY_AREA = 112; protected static final int OVERLAPPING_WAY_AREA = 113; + protected static final int OVERLAPPING_WATERWAY_AREA = 114; protected static final int DUPLICATE_WAY_SEGMENT = 121; + protected static final int OVERLAPPING_HIGHWAY_LINEAR_WAY = 131; + protected static final int OVERLAPPING_RAILWAY_LINEAR_WAY = 132; + protected static final int OVERLAPPING_WATERWAY_LINEAR_WAY = 133; protected static final ListProperty IGNORED_KEYS = new ListProperty( @@ -69,9 +83,12 @@ super.startTest(monitor); nodePairs = new MultiMap<>(1000); + includeOther = isBeforeUpload ? ValidatorPrefHelper.PREF_OTHER_UPLOAD.get() : ValidatorPrefHelper.PREF_OTHER.get(); + onlyKnwonLinear = Config.getPref().getBoolean("overlapping-ways.only-known-linear", true); + ignoreLayer = Config.getPref().getBoolean("overlapping-ways.ignore-layer", false); } private static boolean parentMultipolygonConcernsArea(OsmPrimitive p) { return p.referrers(Relation.class) - .anyMatch(Relation::concernsArea); + .anyMatch(Relation::isMultipolygon); } @@ -80,89 +97,136 @@ Map, Set> seenWays = new HashMap<>(500); - Collection preliminaryErrors = new ArrayList<>(); for (Set duplicated : nodePairs.values()) { - int ways = duplicated.size(); - - if (ways > 1) { - List prims = new ArrayList<>(); - List currentWays = new ArrayList<>(); - Collection highlight; - int highway = 0; - int railway = 0; - int area = 0; - + if (duplicated.size() <= 1) + continue; + if (ignoreLayer) { + analyseOverlaps(duplicated, seenWays); + } else { + // group by layer tag value (which is very likely null) + Map> grouped = new HashMap<>(); for (WaySegment ws : duplicated) { - if (ws.way.hasKey(HIGHWAY)) { - highway++; - } else if (ws.way.hasKey(RAILWAY)) { - railway++; - } - Boolean ar = OsmUtils.getOsmBoolean(ws.way.get("area")); - if (ar != null && ar) { - area++; - } - if (ws.way.concernsArea() || parentMultipolygonConcernsArea(ws.way)) { - area++; - ways--; - } - - prims.add(ws.way); - currentWays.add(ws.way); + // order in set is important + grouped.computeIfAbsent(OsmUtils.getLayer(ws.way), k -> new LinkedHashSet<>()).add(ws); } - // These ways not seen before - // If two or more of the overlapping ways are highways or railways mark a separate error - if ((highlight = seenWays.get(currentWays)) == null) { - String errortype; - int type; - - if (area > 0) { - if (ways == 0 || duplicated.size() == area) { - continue; // We previously issued an annoying "Areas share segment" warning - } else if (highway == ways) { - errortype = tr("Highways share segment with area"); - type = OVERLAPPING_HIGHWAY_AREA; - } else if (railway == ways) { - errortype = tr("Railways share segment with area"); - type = OVERLAPPING_RAILWAY_AREA; - } else { - errortype = tr("Ways share segment with area"); - type = OVERLAPPING_WAY_AREA; - } - } else if (highway == ways) { - errortype = tr("Overlapping highways"); - type = OVERLAPPING_HIGHWAY; - } else if (railway == ways) { - errortype = tr("Overlapping railways"); - type = OVERLAPPING_RAILWAY; - } else { - errortype = tr("Overlapping ways"); - type = OVERLAPPING_WAY; - } - - Severity severity = type < OVERLAPPING_HIGHWAY_AREA ? Severity.WARNING : Severity.OTHER; - preliminaryErrors.add(TestError.builder(this, severity, type) - .message(errortype) - .primitives(prims) - .highlightWaySegments(duplicated) - .build()); - seenWays.put(currentWays, duplicated); - } else { /* way seen, mark highlight layer only */ - highlight.addAll(duplicated); + grouped.values().forEach(group -> analyseOverlaps(group, seenWays)); + } + } + nodePairs = null; + + super.endTest(); + } + + private void analyseOverlaps(Set duplicated, Map, Set> seenWays) { + int ways = duplicated.size(); + if (ways <= 1) + return; + + List currentWays = duplicated.stream().map(ws -> ws.way).collect(Collectors.toList()); + Collection highlight; + if ((highlight = seenWays.get(currentWays)) != null) { + /* this combination of ways was seen before, just add highlighted segment */ + highlight.addAll(duplicated); + } else { + int countHighway = 0; + int countRailway = 0; + int countWaterway = 0; + int countOther = 0; + int numAreas = 0; + for (WaySegment ws : duplicated) { + boolean isArea = ws.way.concernsArea(); + if (ws.way.hasKey(HIGHWAY)) { + if (!isArea) { + countHighway++; + } + } else if (ws.way.hasKey(RAILWAY)) { + if (!isArea) { + countRailway++; + } + } else if (ws.way.hasKey(WATERWAY)) { + if (!isArea) { + countWaterway++; + } + } else { + if (ws.way.getInterestingTags().isEmpty() && parentMultipolygonConcernsArea(ws.way)) + isArea = true; + if (!isArea && isOtherLinear(ws.way)) { + countOther++; + } } - } - } - - // see ticket #9598 - only report if at least 3 segments are shared, except for overlapping ways, i.e warnings (see #9820) - for (TestError error : preliminaryErrors) { - if (error.getSeverity() == Severity.WARNING || error.getHighlighted().size() / error.getPrimitives().size() >= 3) { - boolean ignore = error.getPrimitives().stream().anyMatch(IGNORED); - if (!ignore) { - errors.add(error); + if (isArea) { + numAreas++; } } - } - - super.endTest(); - nodePairs = null; + if (numAreas == ways) { + // no linear object, we don't care when areas share segments + return; + } + + + // If two or more of the overlapping ways are highways or railways mark a separate error + String errortype; + int type; + int allKnownLinear = countHighway + countRailway + countWaterway + countOther; + final Severity severity; + if (countHighway > 1) { + errortype = tr("Overlapping highways"); + type = OVERLAPPING_HIGHWAY; + severity = Severity.ERROR; + } else if (countRailway > 1) { + errortype = tr("Overlapping railways"); + type = OVERLAPPING_RAILWAY; + severity = Severity.ERROR; + } else if (countWaterway > 1) { + errortype = tr("Overlapping waterways"); + type = OVERLAPPING_WATERWAY; + severity = Severity.ERROR; + } else if (countHighway > 0 && countHighway < allKnownLinear) { + errortype = tr("Highway shares segment with linear way"); + type = OVERLAPPING_HIGHWAY_LINEAR_WAY; + severity = Severity.WARNING; + } else if (countRailway > 0 && countRailway < allKnownLinear) { + errortype = tr("Railway shares segment with linear way"); + type = OVERLAPPING_HIGHWAY_LINEAR_WAY; + severity = Severity.WARNING; + } else if (countWaterway > 0 && countWaterway < allKnownLinear) { + errortype = tr("Waterway shares segment with linear way"); + type = OVERLAPPING_WATERWAY_LINEAR_WAY; + severity = Severity.WARNING; + } else if (!includeOther || onlyKnwonLinear) { + return; + } else if (countHighway > 0) { + errortype = tr("Highway shares segment with other way"); + type = OVERLAPPING_HIGHWAY_AREA; + severity = Severity.OTHER; + } else if (countRailway > 0) { + errortype = tr("Railway shares segment with other way"); + type = OVERLAPPING_RAILWAY_AREA; + severity = Severity.OTHER; + } else if (countWaterway > 0) { + errortype = tr("Waterway shares segment with other way"); + type = OVERLAPPING_WATERWAY_AREA; + severity = Severity.OTHER; + } else { + errortype = tr("Ways share segment"); + type = OVERLAPPING_WAY; + severity = Severity.OTHER; + } + + List prims = new ArrayList<>(currentWays); + errors.add(TestError.builder(this, severity, type) + .message(errortype) + .primitives(prims) + .highlightWaySegments(duplicated) + .build()); + seenWays.put(currentWays, duplicated); + } + } + + private static boolean isOtherLinear(Way way) { + // it is assumed that area=* was evaluated before and is false + return (way.hasKey("barrier", "addr:interpolation", "route", "ford") + || way.hasTag("natural", "tree_row", "cliff", "ridge") + || way.hasTag("power", "line", "minor_line", "cable", "portal") + || way.hasTag("man_made", "pipeline")); } @@ -203,4 +267,10 @@ } + if (IGNORED.test(w)) + return; + + if (onlyKnwonLinear && (w.concernsArea() || w.getInterestingTags().isEmpty())) + return; + Node lastN = null; int i = -2;