Index: unk/src/org/openstreetmap/josm/data/validation/FixableTestError.java =================================================================== --- /trunk/src/org/openstreetmap/josm/data/validation/FixableTestError.java (revision 11128) +++ (revision ) @@ -1,122 +1,0 @@ -// License: GPL. For details, see LICENSE file. -package org.openstreetmap.josm.data.validation; - -import java.util.Collection; - -import org.openstreetmap.josm.command.Command; -import org.openstreetmap.josm.data.osm.OsmPrimitive; - -/** - * Validation error easily fixable right at its detection. The fix can be given when constructing the error. - * @since 6377 - */ -public class FixableTestError extends TestError { - protected final Command fix; - - /** - * Constructs a new {@code FixableTestError} for a single primitive. - * @param tester The tester - * @param severity The severity of this error - * @param message The error message - * @param code The test error reference code - * @param primitive The affected primitive - * @param fix The command used to fix the error - */ - public FixableTestError(Test tester, Severity severity, String message, int code, OsmPrimitive primitive, Command fix) { - super(tester, severity, message, code, primitive); - this.fix = fix; - } - - /** - * Constructs a new {@code FixableTestError} for multiple primitives. - * @param tester The tester - * @param severity The severity of this error - * @param message The error message - * @param code The test error reference code - * @param primitives The affected primitives - * @param fix The command used to fix the error - */ - public FixableTestError(Test tester, Severity severity, String message, int code, Collection primitives, - Command fix) { - super(tester, severity, message, code, primitives); - this.fix = fix; - } - - /** - * Constructs a new {@code FixableTestError} for multiple primitives. - * @param tester The tester - * @param severity The severity of this error - * @param message The error message - * @param code The test error reference code - * @param primitives The affected primitives - * @param highlighted OSM primitives to highlight - * @param fix The command used to fix the error - */ - public FixableTestError(Test tester, Severity severity, String message, int code, Collection primitives, - Collection highlighted, Command fix) { - super(tester, severity, message, code, primitives, highlighted); - this.fix = fix; - } - - /** - * Constructs a new {@code FixableTestError} for a single primitive. - * @param tester The tester - * @param severity The severity of this error - * @param message The error message - * @param description The translated description - * @param descriptionEn The English description - * @param code The test error reference code - * @param primitive The affected primitive - * @param fix The command used to fix the error - */ - public FixableTestError(Test tester, Severity severity, String message, String description, String descriptionEn, int code, - OsmPrimitive primitive, Command fix) { - super(tester, severity, message, description, descriptionEn, code, primitive); - this.fix = fix; - } - - /** - * Constructs a new {@code FixableTestError} for multiple primitives. - * @param tester The tester - * @param severity The severity of this error - * @param message The error message - * @param description The translated description - * @param descriptionEn The English description - * @param code The test error reference code - * @param primitives The affected primitives - * @param fix The command used to fix the error - */ - public FixableTestError(Test tester, Severity severity, String message, String description, String descriptionEn, int code, - Collection primitives, Command fix) { - super(tester, severity, message, description, descriptionEn, code, primitives); - this.fix = fix; - } - - /** - * Constructs a new {@code FixableTestError} for multiple primitives. - * @param tester The tester - * @param severity The severity of this error - * @param message The error message - * @param description The translated description - * @param descriptionEn The English description - * @param code The test error reference code - * @param primitives The affected primitives - * @param highlighted OSM primitives to highlight - * @param fix The command used to fix the error - */ - public FixableTestError(Test tester, Severity severity, String message, String description, String descriptionEn, int code, - Collection primitives, Collection highlighted, Command fix) { - super(tester, severity, message, description, descriptionEn, code, primitives, highlighted); - this.fix = fix; - } - - @Override - public Command getFix() { - return fix; - } - - @Override - public final boolean isFixable() { - return true; - } -} Index: /trunk/src/org/openstreetmap/josm/data/validation/TestError.java =================================================================== --- /trunk/src/org/openstreetmap/josm/data/validation/TestError.java (revision 11128) +++ /trunk/src/org/openstreetmap/josm/data/validation/TestError.java (revision 11129) @@ -2,9 +2,13 @@ package org.openstreetmap.josm.data.validation; +import java.text.MessageFormat; import java.util.ArrayList; +import java.util.Arrays; import java.util.Collection; import java.util.Collections; import java.util.List; +import java.util.Locale; import java.util.TreeSet; +import java.util.function.Supplier; import org.openstreetmap.josm.Main; @@ -26,4 +30,6 @@ import org.openstreetmap.josm.data.validation.util.MultipleNameVisitor; import org.openstreetmap.josm.tools.AlphanumComparator; +import org.openstreetmap.josm.tools.CheckParameterUtil; +import org.openstreetmap.josm.tools.I18n; /** @@ -39,16 +45,205 @@ private String message; /** Deeper error description */ - private String description; - private String descriptionEn; + private final String description; + private final String descriptionEn; /** The affected primitives */ private Collection primitives; /** The primitives or way segments to be highlighted */ - private Collection highlighted; + private final Collection highlighted; /** The tester that raised this error */ private Test tester; /** Internal code used by testers to classify errors */ - private int code; + private final int code; /** If this error is selected */ private boolean selected; + /** Supplying a command to fix the error */ + private final Supplier fixingCommand; + + /** + * A builder for a {@code TestError}. + * @since 11129 + */ + public static final class Builder { + private final Test tester; + private final Severity severity; + private final int code; + private String message; + private String description; + private String descriptionEn; + private Collection primitives; + private Collection highlighted; + private Supplier fixingCommand; + + private Builder(Test tester, Severity severity, int code) { + this.tester = tester; + this.severity = severity; + this.code = code; + } + + /** + * Sets the error message. + * + * @param message The error message + * @return {@code this} + */ + public Builder message(String message) { + this.message = message; + return this; + } + + /** + * Sets the error message. + * + * @param message The the message of this error group + * @param description The translated description of this error + * @param descriptionEn The English description (for ignoring errors) + * @return {@code this} + */ + public Builder messageWithManuallyTranslatedDescription(String message, String description, String descriptionEn) { + this.message = message; + this.description = description; + this.descriptionEn = descriptionEn; + return this; + } + + /** + * Sets the error message. + * + * @param message The 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...)} + * @return {@code this} + */ + public Builder message(String message, String marktrDescription, Object... args) { + this.message = message; + this.description = I18n.tr(marktrDescription, args); + this.descriptionEn = new MessageFormat(marktrDescription, Locale.ENGLISH).format(args); + return this; + } + + /** + * Sets the primitives affected by this error. + * + * @param primitives the primitives affected by this error + * @return {@code this} + */ + public Builder primitives(OsmPrimitive... primitives) { + return primitives(Arrays.asList(primitives)); + } + + /** + * Sets the primitives affected by this error. + * + * @param primitives the primitives affected by this error + * @return {@code this} + */ + public Builder primitives(Collection primitives) { + CheckParameterUtil.ensureThat(this.primitives != null, "primitives already set"); + this.primitives = primitives; + return this; + } + + /** + * Sets the primitives to highlight when selecting this error. + * + * @param highlighted the primitives to highlight + * @return {@code this} + * @see ValidatorVisitor#visit(OsmPrimitive) + */ + public Builder highlight(OsmPrimitive... highlighted) { + return highlight(Arrays.asList(highlighted)); + } + + /** + * Sets the primitives to highlight when selecting this error. + * + * @param highlighted the primitives to highlight + * @return {@code this} + * @see ValidatorVisitor#visit(OsmPrimitive) + */ + public Builder highlight(Collection highlighted) { + CheckParameterUtil.ensureThat(this.highlighted != null, "highlighted already set"); + this.highlighted = highlighted; + return this; + } + + /** + * Sets the way segments to highlight when selecting this error. + * + * @param highlighted the way segments to highlight + * @return {@code this} + * @see ValidatorVisitor#visit(WaySegment) + */ + public Builder highlightWaySegments(Collection highlighted) { + CheckParameterUtil.ensureThat(this.highlighted != null, "highlighted already set"); + this.highlighted = highlighted; + return this; + } + + /** + * Sets the node pairs to highlight when selecting this error. + * + * @param highlighted the node pairs to highlight + * @return {@code this} + * @see ValidatorVisitor#visit(List) + */ + public Builder highlightNodePairs(Collection> highlighted) { + CheckParameterUtil.ensureThat(this.highlighted != null, "highlighted already set"); + this.highlighted = highlighted; + return this; + } + + /** + * Sets a supplier to obtain a command to fix the error. + * + * @param fixingCommand the fix supplier + * @return {@code this} + */ + public Builder fix(Supplier fixingCommand) { + CheckParameterUtil.ensureThat(this.fixingCommand != null, "fixingCommand already set"); + this.fixingCommand = fixingCommand; + return this; + } + + /** + * Returns a new test error with the specified values + * + * @return a new test error with the specified values + * @throws IllegalArgumentException when {@link #message} or {@link #primitives} is null/empty. + */ + public TestError build() { + CheckParameterUtil.ensureParameterNotNull(message, "message not set"); + CheckParameterUtil.ensureParameterNotNull(primitives, "primitives not set"); + CheckParameterUtil.ensureThat(!primitives.isEmpty(), "primitives is empty"); + if (this.highlighted == null) { + this.highlighted = Collections.emptySet(); + } + return new TestError(this); + } + } + + /** + * Starts building a new {@code TestError} + * @param tester The tester + * @param severity The severity of this error + * @param code The test error reference code + * @return a new test builder + * @since 11129 + */ + public static Builder builder(Test tester, Severity severity, int code) { + return new Builder(tester, severity, code); + } + + private TestError(Builder builder) { + this.tester = builder.tester; + this.severity = builder.severity; + this.message = builder.message; + this.description = builder.description; + this.descriptionEn = builder.descriptionEn; + this.primitives = builder.primitives; + this.highlighted = builder.highlighted; + this.code = builder.code; + this.fixingCommand = builder.fixingCommand; + } /** @@ -62,5 +257,7 @@ * @param primitives The affected primitives * @param highlighted OSM primitives to highlight - */ + * @deprecated Use {@link #builder} instead. Will be removed in 2016-12. + */ + @Deprecated public TestError(Test tester, Severity severity, String message, String description, String descriptionEn, int code, Collection primitives, Collection highlighted) { @@ -73,4 +270,5 @@ this.highlighted = highlighted; this.code = code; + this.fixingCommand = null; } @@ -83,5 +281,7 @@ * @param primitives The affected primitives * @param highlighted OSM primitives to highlight - */ + * @deprecated Use {@link #builder} instead. Will be removed in 2016-12. + */ + @Deprecated public TestError(Test tester, Severity severity, String message, int code, Collection primitives, Collection highlighted) { @@ -98,5 +298,7 @@ * @param code The test error reference code * @param primitives The affected primitives - */ + * @deprecated Use {@link #builder} instead. Will be removed in 2016-12. + */ + @Deprecated public TestError(Test tester, Severity severity, String message, String description, String descriptionEn, int code, Collection primitives) { @@ -111,5 +313,7 @@ * @param code The test error reference code * @param primitives The affected primitives - */ + * @deprecated Use {@link #builder} instead. Will be removed in 2016-12. + */ + @Deprecated public TestError(Test tester, Severity severity, String message, int code, Collection primitives) { this(tester, severity, message, null, null, code, primitives, primitives); @@ -123,5 +327,7 @@ * @param code The test error reference code * @param primitive The affected primitive - */ + * @deprecated Use {@link #builder} instead. Will be removed in 2016-12. + */ + @Deprecated public TestError(Test tester, Severity severity, String message, int code, OsmPrimitive primitive) { this(tester, severity, message, null, null, code, Collections.singletonList(primitive), Collections @@ -138,5 +344,7 @@ * @param code The test error reference code * @param primitive The affected primitive - */ + * @deprecated Use {@link #builder} instead. Will be removed in 2016-12. + */ + @Deprecated public TestError(Test tester, Severity severity, String message, String description, String descriptionEn, int code, OsmPrimitive primitive) { @@ -163,5 +371,7 @@ * Sets the error message * @param message The error message - */ + * @deprecated Use {@link #builder} instead. Will be removed in 2016-12. + */ + @Deprecated public void setMessage(String message) { this.message = message; @@ -192,6 +402,8 @@ /** * Sets the list of primitives affected by this error - * @param primitives the list of primitives affected by this error - */ + * @param primitives the list of primitives affected by this error* + * @deprecated Use {@link #builder} instead. Will be removed in 2016-12. + */ + @Deprecated public void setPrimitives(List primitives) { this.primitives = primitives; @@ -209,5 +421,7 @@ * Sets the severity of this error * @param severity the severity of this error - */ + * @deprecated Use {@link #builder} instead. Will be removed in 2016-12. + */ + @Deprecated public void setSeverity(Severity severity) { this.severity = severity; @@ -272,5 +486,7 @@ * Set the tester that raised the error. * @param tester te tester - */ + * @deprecated Use {@link #builder} instead. Will be removed in 2016-12. + */ + @Deprecated public void setTester(Test tester) { this.tester = tester; @@ -291,5 +507,5 @@ */ public boolean isFixable() { - return tester != null && tester.isFixable(this); + return fixingCommand != null || ((tester != null) && tester.isFixable(this)); } @@ -300,4 +516,11 @@ */ public Command getFix() { + // obtain fix from the error + final Command fix = fixingCommand != null ? fixingCommand.get() : null; + if (fix != null) { + return fix; + } + + // obtain fix from the tester if (tester == null || !tester.isFixable(this) || primitives.isEmpty()) return null; Index: /trunk/src/org/openstreetmap/josm/data/validation/tests/Addresses.java =================================================================== --- /trunk/src/org/openstreetmap/josm/data/validation/tests/Addresses.java (revision 11128) +++ /trunk/src/org/openstreetmap/josm/data/validation/tests/Addresses.java (revision 11129) @@ -7,5 +7,4 @@ import java.util.ArrayList; import java.util.Collection; -import java.util.Collections; import java.util.HashMap; import java.util.HashSet; @@ -51,25 +50,4 @@ // CHECKSTYLE.ON: SingleSpaceSeparator - protected static class AddressError extends TestError { - - public AddressError(Addresses tester, int code, OsmPrimitive p, String message) { - this(tester, code, Collections.singleton(p), message); - } - - public AddressError(Addresses tester, int code, Collection collection, String message) { - this(tester, code, collection, message, null, null); - } - - public AddressError(Addresses tester, int code, Collection collection, String message, - String description, String englishDescription) { - this(tester, code, Severity.WARNING, collection, message, description, englishDescription); - } - - public AddressError(Addresses tester, int code, Severity severity, Collection collection, String message, - String description, String englishDescription) { - super(tester, severity, message, description, englishDescription, code, collection); - } - } - /** * Constructor @@ -98,6 +76,8 @@ List errorList = new ArrayList<>(list); errorList.add(0, p); - errors.add(new AddressError(this, MULTIPLE_STREET_RELATIONS, level, errorList, - tr("Multiple associatedStreet relations"), null, null)); + errors.add(TestError.builder(this, level, MULTIPLE_STREET_RELATIONS) + .message(tr("Multiple associatedStreet relations")) + .primitives(errorList) + .build()); } return list; @@ -119,5 +99,8 @@ } // No street found - errors.add(new AddressError(this, HOUSE_NUMBER_WITHOUT_STREET, p, tr("House number without street"))); + errors.add(TestError.builder(this, Severity.WARNING, HOUSE_NUMBER_WITHOUT_STREET) + .message(tr("House number without street")) + .primitives(p) + .build()); } } @@ -179,16 +162,19 @@ } // Report duplicate house numbers - String englishDescription = marktr("House number ''{0}'' duplicated"); for (Entry> entry : map.entrySet()) { List list = entry.getValue(); if (list.size() > 1) { - errors.add(new AddressError(this, DUPLICATE_HOUSE_NUMBER, list, - tr("Duplicate house numbers"), tr(englishDescription, entry.getKey()), englishDescription)); + errors.add(TestError.builder(this, Severity.WARNING, DUPLICATE_HOUSE_NUMBER) + .message(tr("Duplicate house numbers"), marktr("House number ''{0}'' duplicated"), entry.getKey()) + .primitives(list) + .build()); } } // Report wrong street names if (!wrongStreetNames.isEmpty()) { - errors.add(new AddressError(this, MULTIPLE_STREET_NAMES, wrongStreetNames, - tr("Multiple street names in relation"))); + errors.add(TestError.builder(this, Severity.WARNING, MULTIPLE_STREET_NAMES) + .message(tr("Multiple street names in relation")) + .primitives(wrongStreetNames) + .build()); } // Report addresses too far away @@ -245,6 +231,8 @@ List errorList = new ArrayList<>(street); errorList.add(0, house); - errors.add(new AddressError(this, HOUSE_NUMBER_TOO_FAR, errorList, - tr("House number too far from street"))); + errors.add(TestError.builder(this, Severity.WARNING, HOUSE_NUMBER_TOO_FAR) + .message(tr("House number too far from street")) + .primitives(errorList) + .build()); } } Index: /trunk/src/org/openstreetmap/josm/data/validation/tests/ApiCapabilitiesTest.java =================================================================== --- /trunk/src/org/openstreetmap/josm/data/validation/tests/ApiCapabilitiesTest.java (revision 11128) +++ /trunk/src/org/openstreetmap/josm/data/validation/tests/ApiCapabilitiesTest.java (revision 11129) @@ -50,5 +50,8 @@ message = tr("Way contains more than {0} nodes. It should be split or simplified", maxNodes); } - errors.add(new TestError(this, Severity.ERROR, message, MAX_WAY_NODES_ERROR, w)); + errors.add(TestError.builder(this, Severity.ERROR, MAX_WAY_NODES_ERROR) + .message(message) + .primitives(w) + .build()); } } Index: /trunk/src/org/openstreetmap/josm/data/validation/tests/BarriersEntrances.java =================================================================== --- /trunk/src/org/openstreetmap/josm/data/validation/tests/BarriersEntrances.java (revision 11128) +++ /trunk/src/org/openstreetmap/josm/data/validation/tests/BarriersEntrances.java (revision 11129) @@ -33,5 +33,9 @@ } } - errors.add(new TestError(this, Severity.WARNING, tr("Barrier entrance not set on a barrier"), BARRIER_ENTRANCE_WITHOUT_BARRIER, n)); + errors.add(TestError + .builder(this, Severity.WARNING, BARRIER_ENTRANCE_WITHOUT_BARRIER) + .message(tr("Barrier entrance not set on a barrier")) + .primitives(n) + .build()); } } Index: /trunk/src/org/openstreetmap/josm/data/validation/tests/Coastlines.java =================================================================== --- /trunk/src/org/openstreetmap/josm/data/validation/tests/Coastlines.java (revision 11128) +++ /trunk/src/org/openstreetmap/josm/data/validation/tests/Coastlines.java (revision 11129) @@ -147,6 +147,8 @@ // simple closed ways are reported by WronglyOrderedWays if (visited.size() > 1 && nodes.get(0) == nodes.get(nodes.size()-1) && Geometry.isClockwise(nodes)) { - errors.add(new TestError(this, Severity.WARNING, tr("Reversed coastline: land not on left side"), - WRONG_ORDER_COASTLINE, visited)); + errors.add(TestError.builder(this, Severity.WARNING, WRONG_ORDER_COASTLINE) + .message(tr("Reversed coastline: land not on left side")) + .primitives(visited) + .build()); } } @@ -226,7 +228,14 @@ } if (errCode != REVERSED_COASTLINE) - errors.add(new TestError(this, Severity.ERROR, msg, errCode, primitives, Collections.singletonList(n))); + errors.add(TestError.builder(this, Severity.ERROR, errCode) + .message(msg) + .primitives(primitives) + .highlight(n) + .build()); else - errors.add(new TestError(this, Severity.ERROR, msg, errCode, primitives)); + errors.add(TestError.builder(this, Severity.ERROR, errCode) + .message(msg) + .primitives(primitives) + .build()); } Index: /trunk/src/org/openstreetmap/josm/data/validation/tests/ConditionalKeys.java =================================================================== --- /trunk/src/org/openstreetmap/josm/data/validation/tests/ConditionalKeys.java (revision 11128) +++ /trunk/src/org/openstreetmap/josm/data/validation/tests/ConditionalKeys.java (revision 11129) @@ -167,5 +167,8 @@ Pattern.compile(":conditional(:.*)?$").asPredicate())) { if (!isKeyValid(key)) { - errors.add(new TestError(this, Severity.WARNING, tr("Wrong syntax in {0} key", key), 3201, p)); + errors.add(TestError.builder(this, Severity.WARNING, 3201) + .message(tr("Wrong syntax in {0} key", key)) + .primitives(p) + .build()); continue; } @@ -173,5 +176,8 @@ final String error = validateValue(key, value); if (error != null) { - errors.add(new TestError(this, Severity.WARNING, tr("Error in {0} value: {1}", key, error), 3202, p)); + errors.add(TestError.builder(this, Severity.WARNING, 3202) + .message(tr("Error in {0} value: {1}", key, error)) + .primitives(p) + .build()); } } Index: /trunk/src/org/openstreetmap/josm/data/validation/tests/CrossingWays.java =================================================================== --- /trunk/src/org/openstreetmap/josm/data/validation/tests/CrossingWays.java (revision 11128) +++ /trunk/src/org/openstreetmap/josm/data/validation/tests/CrossingWays.java (revision 11129) @@ -247,9 +247,9 @@ final String message = createMessage(es1.way, es2.way); - errors.add(new TestError(this, Severity.WARNING, - message, - CROSSING_WAYS, - prims, - highlight)); + errors.add(TestError.builder(this, Severity.WARNING, CROSSING_WAYS) + .message(message) + .primitives(prims) + .highlightWaySegments(highlight) + .build()); seenWays.put(prims, highlight); } else { Index: /trunk/src/org/openstreetmap/josm/data/validation/tests/DuplicateNode.java =================================================================== --- /trunk/src/org/openstreetmap/josm/data/validation/tests/DuplicateNode.java (revision 11128) +++ /trunk/src/org/openstreetmap/josm/data/validation/tests/DuplicateNode.java (revision 11129) @@ -32,5 +32,4 @@ import org.openstreetmap.josm.data.validation.TestError; import org.openstreetmap.josm.gui.progress.ProgressMonitor; -import org.openstreetmap.josm.tools.CheckParameterUtil; import org.openstreetmap.josm.tools.MultiMap; @@ -84,11 +83,4 @@ LatLon coorK = getLatLon(k); return coorK == null ? 0 : coorK.hashCode(); - } - } - - private static class DuplicateNodeTestError extends TestError { - DuplicateNodeTestError(Test parentTest, Severity severity, String msg, int code, Set primitives) { - super(parentTest, severity, tr("Duplicated nodes"), tr(msg), msg, code, primitives); - CheckParameterUtil.ensureThat(!primitives.isEmpty(), "Empty primitives: " + msg); } } @@ -214,83 +206,53 @@ if (nbType > 1) { - errors.add(new DuplicateNodeTestError( - parentTest, - Severity.WARNING, - marktr("Mixed type duplicated nodes"), - DUPLICATE_NODE_MIXED, - primitives - )); + errors.add(TestError.builder(parentTest, Severity.WARNING, DUPLICATE_NODE_MIXED) + .message(marktr("Mixed type duplicated nodes")) + .primitives(primitives) + .build()); } else if (typeMap.get("highway")) { - errors.add(new DuplicateNodeTestError( - parentTest, - Severity.ERROR, - marktr("Highway duplicated nodes"), - DUPLICATE_NODE_HIGHWAY, - primitives - )); + errors.add(TestError.builder(parentTest, Severity.ERROR, DUPLICATE_NODE_HIGHWAY) + .message(marktr("Highway duplicated nodes")) + .primitives(primitives) + .build()); } else if (typeMap.get("railway")) { - errors.add(new DuplicateNodeTestError( - parentTest, - Severity.ERROR, - marktr("Railway duplicated nodes"), - DUPLICATE_NODE_RAILWAY, - primitives - )); + errors.add(TestError.builder(parentTest, Severity.ERROR, DUPLICATE_NODE_RAILWAY) + .message(marktr("Railway duplicated nodes")) + .primitives(primitives) + .build()); } else if (typeMap.get("waterway")) { - errors.add(new DuplicateNodeTestError( - parentTest, - Severity.ERROR, - marktr("Waterway duplicated nodes"), - DUPLICATE_NODE_WATERWAY, - primitives - )); + errors.add(TestError.builder(parentTest, Severity.ERROR, DUPLICATE_NODE_WATERWAY) + .message(marktr("Waterway duplicated nodes")) + .primitives(primitives) + .build()); } else if (typeMap.get("boundary")) { - errors.add(new DuplicateNodeTestError( - parentTest, - Severity.ERROR, - marktr("Boundary duplicated nodes"), - DUPLICATE_NODE_BOUNDARY, - primitives - )); + errors.add(TestError.builder(parentTest, Severity.ERROR, DUPLICATE_NODE_BOUNDARY) + .message(marktr("Boundary duplicated nodes")) + .primitives(primitives) + .build()); } else if (typeMap.get("power")) { - errors.add(new DuplicateNodeTestError( - parentTest, - Severity.ERROR, - marktr("Power duplicated nodes"), - DUPLICATE_NODE_POWER, - primitives - )); + errors.add(TestError.builder(parentTest, Severity.ERROR, DUPLICATE_NODE_POWER) + .message(marktr("Power duplicated nodes")) + .primitives(primitives) + .build()); } else if (typeMap.get("natural")) { - errors.add(new DuplicateNodeTestError( - parentTest, - Severity.ERROR, - marktr("Natural duplicated nodes"), - DUPLICATE_NODE_NATURAL, - primitives - )); + errors.add(TestError.builder(parentTest, Severity.ERROR, DUPLICATE_NODE_NATURAL) + .message(marktr("Natural duplicated nodes")) + .primitives(primitives) + .build()); } else if (typeMap.get("building")) { - errors.add(new DuplicateNodeTestError( - parentTest, - Severity.ERROR, - marktr("Building duplicated nodes"), - DUPLICATE_NODE_BUILDING, - primitives - )); + errors.add(TestError.builder(parentTest, Severity.ERROR, DUPLICATE_NODE_BUILDING) + .message(marktr("Building duplicated nodes")) + .primitives(primitives) + .build()); } else if (typeMap.get("landuse")) { - errors.add(new DuplicateNodeTestError( - parentTest, - Severity.ERROR, - marktr("Landuse duplicated nodes"), - DUPLICATE_NODE_LANDUSE, - primitives - )); + errors.add(TestError.builder(parentTest, Severity.ERROR, DUPLICATE_NODE_LANDUSE) + .message(marktr("Landuse duplicated nodes")) + .primitives(primitives) + .build()); } else { - errors.add(new DuplicateNodeTestError( - parentTest, - Severity.WARNING, - marktr("Other duplicated nodes"), - DUPLICATE_NODE_OTHER, - primitives - )); + errors.add(TestError.builder(parentTest, Severity.WARNING, DUPLICATE_NODE_OTHER) + .message(marktr("Other duplicated nodes")) + .primitives(primitives) + .build()); } it.remove(); @@ -305,11 +267,8 @@ } if (duplicates.size() > 1) { - errors.add(new TestError( - parentTest, - Severity.WARNING, - tr("Nodes at same position"), - DUPLICATE_NODE, - duplicates - )); + errors.add(TestError.builder(parentTest, Severity.WARNING, DUPLICATE_NODE) + .message(tr("Nodes at same position")) + .primitives(duplicates) + .build()); } } Index: /trunk/src/org/openstreetmap/josm/data/validation/tests/DuplicateRelation.java =================================================================== --- /trunk/src/org/openstreetmap/josm/data/validation/tests/DuplicateRelation.java (revision 11128) +++ /trunk/src/org/openstreetmap/josm/data/validation/tests/DuplicateRelation.java (revision 11129) @@ -203,5 +203,8 @@ for (Set duplicated : relations.values()) { if (duplicated.size() > 1) { - TestError testError = new TestError(this, Severity.ERROR, tr("Duplicated relations"), DUPLICATE_RELATION, duplicated); + TestError testError = TestError.builder(this, Severity.ERROR, DUPLICATE_RELATION) + .message(tr("Duplicated relations")) + .primitives(duplicated) + .build(); errors.add(testError); } @@ -210,5 +213,8 @@ for (Set duplicated : relationsNoKeys.values()) { if (duplicated.size() > 1) { - TestError testError = new TestError(this, Severity.WARNING, tr("Relations with same members"), SAME_RELATION, duplicated); + TestError testError = TestError.builder(this, Severity.WARNING, SAME_RELATION) + .message(tr("Relations with same members")) + .primitives(duplicated) + .build(); errors.add(testError); } Index: /trunk/src/org/openstreetmap/josm/data/validation/tests/DuplicateWay.java =================================================================== --- /trunk/src/org/openstreetmap/josm/data/validation/tests/DuplicateWay.java (revision 11128) +++ /trunk/src/org/openstreetmap/josm/data/validation/tests/DuplicateWay.java (revision 11129) @@ -123,5 +123,8 @@ for (Set duplicated : ways.values()) { if (duplicated.size() > 1) { - TestError testError = new TestError(this, Severity.ERROR, tr("Duplicated ways"), DUPLICATE_WAY, duplicated); + TestError testError = TestError.builder(this, Severity.ERROR, DUPLICATE_WAY) + .message(tr("Duplicated ways")) + .primitives(duplicated) + .build(); errors.add(testError); } @@ -150,5 +153,8 @@ continue; } - TestError testError = new TestError(this, Severity.WARNING, tr("Ways with same position"), SAME_WAY, sameway); + TestError testError = TestError.builder(this, Severity.WARNING, SAME_WAY) + .message(tr("Ways with same position")) + .primitives(sameway) + .build(); errors.add(testError); } Index: /trunk/src/org/openstreetmap/josm/data/validation/tests/DuplicatedWayNodes.java =================================================================== --- /trunk/src/org/openstreetmap/josm/data/validation/tests/DuplicatedWayNodes.java (revision 11128) +++ /trunk/src/org/openstreetmap/josm/data/validation/tests/DuplicatedWayNodes.java (revision 11129) @@ -4,5 +4,4 @@ import static org.openstreetmap.josm.tools.I18n.tr; -import java.util.Arrays; import java.util.Collections; import java.util.Iterator; @@ -43,6 +42,9 @@ } if (lastN == n) { - errors.add(new TestError(this, Severity.ERROR, tr("Duplicated way nodes"), DUPLICATE_WAY_NODE, - Arrays.asList(w), Arrays.asList(n))); + errors.add(TestError.builder(this, Severity.ERROR, DUPLICATE_WAY_NODE) + .message(tr("Duplicated way nodes")) + .primitives(w) + .highlight(n) + .build()); break; } Index: /trunk/src/org/openstreetmap/josm/data/validation/tests/Highways.java =================================================================== --- /trunk/src/org/openstreetmap/josm/data/validation/tests/Highways.java (revision 11128) +++ /trunk/src/org/openstreetmap/josm/data/validation/tests/Highways.java (revision 11129) @@ -8,5 +8,4 @@ import java.util.HashMap; import java.util.HashSet; -import java.util.Iterator; import java.util.List; import java.util.Locale; @@ -15,10 +14,8 @@ import org.openstreetmap.josm.command.ChangePropertyCommand; -import org.openstreetmap.josm.command.Command; import org.openstreetmap.josm.data.osm.Node; import org.openstreetmap.josm.data.osm.OsmPrimitive; import org.openstreetmap.josm.data.osm.OsmUtils; import org.openstreetmap.josm.data.osm.Way; -import org.openstreetmap.josm.data.validation.FixableTestError; import org.openstreetmap.josm.data.validation.Severity; import org.openstreetmap.josm.data.validation.Test; @@ -76,16 +73,4 @@ } - protected static class WrongRoundaboutHighway extends TestError { - - public final String correctValue; - - public WrongRoundaboutHighway(Highways tester, Way w, String key) { - super(tester, Severity.WARNING, - tr("Incorrect roundabout (highway: {0} instead of {1})", w.get("highway"), key), - WRONG_ROUNDABOUT_HIGHWAY, w); - this.correctValue = key; - } - } - @Override public void visit(Node n) { @@ -147,5 +132,9 @@ // Error when the highway tags do not match if (!w.get("highway").equals(s)) { - errors.add(new WrongRoundaboutHighway(this, w, s)); + errors.add(TestError.builder(this, Severity.WARNING, WRONG_ROUNDABOUT_HIGHWAY) + .message(tr("Incorrect roundabout (highway: {0} instead of {1})", w.get("highway"), s)) + .primitives(w) + .fix(() -> new ChangePropertyCommand(w, "highway", s)) + .build()); } break; @@ -180,6 +169,8 @@ private void testHighwayLink(final Way way) { if (!isHighwayLinkOkay(way)) { - errors.add(new TestError(this, Severity.WARNING, - tr("Highway link is not linked to adequate highway/link"), SOURCE_WRONG_LINK, way)); + errors.add(TestError.builder(this, Severity.WARNING, SOURCE_WRONG_LINK) + .message(tr("Highway link is not linked to adequate highway/link")) + .primitives(way) + .build()); } } @@ -213,6 +204,8 @@ } if ((leftByPedestrians || leftByCyclists) && leftByCars) { - errors.add(new TestError(this, Severity.OTHER, tr("Missing pedestrian crossing information"), - MISSING_PEDESTRIAN_CROSSING, n)); + errors.add(TestError.builder(this, Severity.OTHER, MISSING_PEDESTRIAN_CROSSING) + .message(tr("Missing pedestrian crossing information")) + .primitives(n) + .build()); return; } @@ -249,11 +242,11 @@ String country = value.substring(0, index); if (!ISO_COUNTRIES.contains(country)) { + final TestError.Builder error = TestError.builder(this, Severity.WARNING, SOURCE_MAXSPEED_UNKNOWN_COUNTRY_CODE) + .message(tr("Unknown country code: {0}", country)) + .primitives(p); if ("UK".equals(country)) { - errors.add(new FixableTestError(this, Severity.WARNING, - tr("Unknown country code: {0}", country), SOURCE_MAXSPEED_UNKNOWN_COUNTRY_CODE, p, - new ChangePropertyCommand(p, SOURCE_MAXSPEED, value.replace("UK:", "GB:")))); + errors.add(error.fix(() -> new ChangePropertyCommand(p, SOURCE_MAXSPEED, value.replace("UK:", "GB:"))).build()); } else { - errors.add(new TestError(this, Severity.WARNING, - tr("Unknown country code: {0}", country), SOURCE_MAXSPEED_UNKNOWN_COUNTRY_CODE, p)); + errors.add(error.build()); } } @@ -261,6 +254,8 @@ String context = value.substring(index+1); if (!KNOWN_SOURCE_MAXSPEED_CONTEXTS.contains(context)) { - errors.add(new TestError(this, Severity.WARNING, - tr("Unknown source:maxspeed context: {0}", context), SOURCE_MAXSPEED_UNKNOWN_CONTEXT, p)); + errors.add(TestError.builder(this, Severity.WARNING, SOURCE_MAXSPEED_UNKNOWN_CONTEXT) + .message(tr("Unknown source:maxspeed context: {0}", context)) + .primitives(p) + .build()); } // TODO: Check coherence of context against maxspeed @@ -268,21 +263,3 @@ } } - - @Override - public boolean isFixable(TestError testError) { - return testError instanceof WrongRoundaboutHighway; - } - - @Override - public Command fixError(TestError testError) { - if (testError instanceof WrongRoundaboutHighway) { - // primitives list can be empty if all primitives have been purged - Iterator it = testError.getPrimitives().iterator(); - if (it.hasNext()) { - return new ChangePropertyCommand(it.next(), - "highway", ((WrongRoundaboutHighway) testError).correctValue); - } - } - return null; - } } Index: /trunk/src/org/openstreetmap/josm/data/validation/tests/InternetTags.java =================================================================== --- /trunk/src/org/openstreetmap/josm/data/validation/tests/InternetTags.java (revision 11128) +++ /trunk/src/org/openstreetmap/josm/data/validation/tests/InternetTags.java (revision 11129) @@ -2,4 +2,5 @@ package org.openstreetmap.josm.data.validation.tests; +import static org.openstreetmap.josm.tools.I18n.marktr; import static org.openstreetmap.josm.tools.I18n.tr; @@ -102,7 +103,8 @@ return doValidateTag(p, k, proto+value, validator, code); } - String msg = tr("''{0}'': {1}", k, errMsg); - // todo obtain English message for ignore functionality - error = new TestError(this, Severity.WARNING, validator.getValidatorName(), msg, msg, code, p); + error = TestError.builder(this, Severity.WARNING, code) + .message(validator.getValidatorName(), marktr("''{0}'': {1}"), k, errMsg) + .primitives(p) + .build(); } return error; Index: /trunk/src/org/openstreetmap/josm/data/validation/tests/Lanes.java =================================================================== --- /trunk/src/org/openstreetmap/josm/data/validation/tests/Lanes.java (revision 11128) +++ /trunk/src/org/openstreetmap/josm/data/validation/tests/Lanes.java (revision 11129) @@ -49,10 +49,16 @@ if (lanesCount.size() > 1) { // if not all numbers are the same - errors.add(new TestError(this, Severity.WARNING, message, 3100, p)); + errors.add(TestError.builder(this, Severity.WARNING, 3100) + .message(message) + .primitives(p) + .build()); } else if (lanesCount.size() == 1 && p.hasKey(lanesKey)) { // ensure that lanes <= *:lanes try { if (Integer.parseInt(p.get(lanesKey)) > lanesCount.iterator().next()) { - errors.add(new TestError(this, Severity.WARNING, tr("Number of {0} greater than {1}", lanesKey, "*:" + lanesKey), 3100, p)); + errors.add(TestError.builder(this, Severity.WARNING, 3100) + .message(tr("Number of {0} greater than {1}", lanesKey, "*:" + lanesKey)) + .primitives(p) + .build()); } } catch (NumberFormatException ignore) { @@ -69,6 +75,8 @@ try { if (Integer.parseInt(lanes) < Integer.parseInt(forward) + Integer.parseInt(backward)) { - errors.add(new TestError(this, Severity.WARNING, - tr("Number of {0} greater than {1}", tr("{0}+{1}", "lanes:forward", "lanes:backward"), "lanes"), 3101, p)); + errors.add(TestError.builder(this, Severity.WARNING, 3101) + .message(tr("Number of {0} greater than {1}", tr("{0}+{1}", "lanes:forward", "lanes:backward"), "lanes")) + .primitives(p) + .build()); } } catch (NumberFormatException ignore) { Index: /trunk/src/org/openstreetmap/josm/data/validation/tests/LongSegment.java =================================================================== --- /trunk/src/org/openstreetmap/josm/data/validation/tests/LongSegment.java (revision 11128) +++ /trunk/src/org/openstreetmap/josm/data/validation/tests/LongSegment.java (revision 11129) @@ -2,4 +2,5 @@ package org.openstreetmap.josm.data.validation.tests; +import static org.openstreetmap.josm.tools.I18n.marktr; import static org.openstreetmap.josm.tools.I18n.tr; @@ -40,8 +41,8 @@ if (length > maxlength) { length /= 1000.0; - errors.add(new TestError(this, Severity.WARNING, tr("Long segments"), - tr("Very long segment of {0} kilometers", length.intValue()), - String.format("Very long segment of %d kilometers", length.intValue()), - LONG_SEGMENT, w)); + errors.add(TestError.builder(this, Severity.WARNING, LONG_SEGMENT) + .message(tr("Long segments"), marktr("Very long segment of {0} kilometers"), length.intValue()) + .primitives(w) + .build()); } } Index: /trunk/src/org/openstreetmap/josm/data/validation/tests/MapCSSTagChecker.java =================================================================== --- /trunk/src/org/openstreetmap/josm/data/validation/tests/MapCSSTagChecker.java (revision 11128) +++ /trunk/src/org/openstreetmap/josm/data/validation/tests/MapCSSTagChecker.java (revision 11129) @@ -39,5 +39,4 @@ import org.openstreetmap.josm.data.osm.OsmUtils; import org.openstreetmap.josm.data.osm.Tag; -import org.openstreetmap.josm.data.validation.FixableTestError; import org.openstreetmap.josm.data.validation.Severity; import org.openstreetmap.josm.data.validation.Test; @@ -567,8 +566,8 @@ TestError getErrorForPrimitive(OsmPrimitive p) { final Environment env = new Environment(p); - return getErrorForPrimitive(p, whichSelectorMatchesEnvironment(env), env); - } - - TestError getErrorForPrimitive(OsmPrimitive p, Selector matchingSelector, Environment env) { + return getErrorForPrimitive(p, whichSelectorMatchesEnvironment(env), env, null); + } + + TestError getErrorForPrimitive(OsmPrimitive p, Selector matchingSelector, Environment env, Test tester) { if (matchingSelector != null && !errors.isEmpty()) { final Command fix = fixPrimitive(p); @@ -582,9 +581,11 @@ primitives = Collections.singletonList(p); } + final TestError.Builder error = TestError.builder(tester, getSeverity(), 3000) + .messageWithManuallyTranslatedDescription(description1, description2, matchingSelector.toString()) + .primitives(primitives); if (fix != null) { - return new FixableTestError(null, getSeverity(), description1, description2, matchingSelector.toString(), 3000, - primitives, fix); + return error.fix(() -> fix).build(); } else { - return new TestError(null, getSeverity(), description1, description2, matchingSelector.toString(), 3000, primitives); + return error.build(); } } else { @@ -685,7 +686,6 @@ if (selector != null) { check.rule.declaration.execute(env); - final TestError error = check.getErrorForPrimitive(p, selector, env); + final TestError error = check.getErrorForPrimitive(p, selector, env, new MapCSSTagCheckerAndRule(check.rule)); if (error != null) { - error.setTester(new MapCSSTagCheckerAndRule(check.rule)); r.add(error); } Index: /trunk/src/org/openstreetmap/josm/data/validation/tests/MultipolygonTest.java =================================================================== --- /trunk/src/org/openstreetmap/josm/data/validation/tests/MultipolygonTest.java (revision 11128) +++ /trunk/src/org/openstreetmap/josm/data/validation/tests/MultipolygonTest.java (revision 11129) @@ -2,9 +2,9 @@ package org.openstreetmap.josm.data.validation.tests; +import static org.openstreetmap.josm.tools.I18n.marktr; import static org.openstreetmap.josm.tools.I18n.tr; import static org.openstreetmap.josm.tools.I18n.trn; import java.awt.geom.GeneralPath; -import java.text.MessageFormat; import java.util.ArrayList; import java.util.Arrays; @@ -12,5 +12,4 @@ import java.util.Collections; import java.util.HashSet; -import java.util.LinkedList; import java.util.List; import java.util.Set; @@ -148,6 +147,9 @@ } } - errors.add(new TestError(this, Severity.WARNING, tr("Area style way is not closed"), NOT_CLOSED, - Collections.singletonList(w), Arrays.asList(nodes.get(0), nodes.get(nodes.size() - 1)))); + errors.add(TestError.builder(this, Severity.WARNING, NOT_CLOSED) + .message(tr("Area style way is not closed")) + .primitives(w) + .highlight(Arrays.asList(nodes.get(0), nodes.get(nodes.size() - 1))) + .build()); } } @@ -186,5 +188,8 @@ } if (!hasOuterWay) { - addError(r, new TestError(this, Severity.WARNING, tr("No outer way for multipolygon"), MISSING_OUTER_WAY, r)); + errors.add(TestError.builder(this, Severity.WARNING, MISSING_OUTER_WAY) + .message(tr("No outer way for multipolygon")) + .primitives(r) + .build()); } } @@ -204,13 +209,11 @@ final String roleInNewMP = memberInNewMP.iterator().next().getRole(); if (!member.getRole().equals(roleInNewMP)) { - List l = new ArrayList<>(); - l.add(r); - l.add(member.getMember()); - addError(r, new TestError(this, Severity.WARNING, RelationChecker.ROLE_VERIF_PROBLEM_MSG, - tr("Role for ''{0}'' should be ''{1}''", - member.getMember().getDisplayName(DefaultNameFormatter.getInstance()), roleInNewMP), - MessageFormat.format("Role for ''{0}'' should be ''{1}''", - member.getMember().getDisplayName(DefaultNameFormatter.getInstance()), roleInNewMP), - WRONG_MEMBER_ROLE, l, Collections.singleton(member.getMember()))); + errors.add(TestError.builder(this, Severity.WARNING, WRONG_MEMBER_ROLE) + .message(RelationChecker.ROLE_VERIF_PROBLEM_MSG, + marktr("Role for ''{0}'' should be ''{1}''"), + member.getMember().getDisplayName(DefaultNameFormatter.getInstance()), roleInNewMP) + .primitives(addRelationIfNeeded(r, member.getMember())) + .highlight(member.getMember()) + .build()); } } @@ -242,12 +245,16 @@ } if (area == null) { - addError(r, new TestError(this, Severity.OTHER, tr("No area style for multipolygon"), NO_STYLE, r)); + errors.add(TestError.builder(this, Severity.OTHER, NO_STYLE) + .message(tr("No area style for multipolygon")) + .primitives(r) + .build()); } else { /* old style multipolygon - solve: copy tags from outer way to multipolygon */ - addError(r, new TestError(this, Severity.WARNING, - trn("Multipolygon relation should be tagged with area tags and not the outer way", + errors.add(TestError.builder(this, Severity.WARNING, NO_STYLE_POLYGON) + .message(trn("Multipolygon relation should be tagged with area tags and not the outer way", "Multipolygon relation should be tagged with area tags and not the outer ways", - polygon.getOuterWays().size()), - NO_STYLE_POLYGON, r)); + polygon.getOuterWays().size())) + .primitives(r) + .build()); } } @@ -258,10 +265,9 @@ if (areaInner != null && area.equals(areaInner)) { - List l = new ArrayList<>(); - l.add(r); - l.add(wInner); - addError(r, new TestError(this, Severity.OTHER, - tr("With the currently used mappaint style the style for inner way equals the multipolygon style"), - INNER_STYLE_MISMATCH, l, Collections.singletonList(wInner))); + errors.add(TestError.builder(this, Severity.OTHER, INNER_STYLE_MISMATCH) + .message(tr("With the currently used mappaint style the style for inner way equals the multipolygon style")) + .primitives(addRelationIfNeeded(r, wInner)) + .highlight(wInner) + .build()); } } @@ -269,14 +275,18 @@ AreaElement areaOuter = ElemStyles.getAreaElemStyle(wOuter, false); if (areaOuter != null) { - List l = new ArrayList<>(); - l.add(r); - l.add(wOuter); if (!area.equals(areaOuter)) { - addError(r, new TestError(this, Severity.OTHER, !areaStyle ? tr("Style for outer way mismatches") - : tr("With the currently used mappaint style(s) the style for outer way mismatches the area style"), - OUTER_STYLE_MISMATCH, l, Collections.singletonList(wOuter))); + String message = !areaStyle ? tr("Style for outer way mismatches") + : tr("With the currently used mappaint style(s) the style for outer way mismatches the area style"); + errors.add(TestError.builder(this, Severity.OTHER, OUTER_STYLE_MISMATCH) + .message(message) + .primitives(addRelationIfNeeded(r, wOuter)) + .highlight(wOuter) + .build()); } else if (areaStyle) { /* style on outer way of multipolygon, but equal to polygon */ - addError(r, new TestError(this, Severity.WARNING, tr("Area style on outer way"), OUTER_STYLE, - l, Collections.singletonList(wOuter))); + errors.add(TestError.builder(this, Severity.WARNING, OUTER_STYLE) + .message(tr("Area style on outer way")) + .primitives(addRelationIfNeeded(r, wOuter)) + .highlight(wOuter) + .build()); } } @@ -298,8 +308,9 @@ List openNodes = polygon.getOpenEnds(); if (!openNodes.isEmpty()) { - List primitives = new LinkedList<>(); - primitives.add(r); - primitives.addAll(openNodes); - addError(r, new TestError(this, Severity.WARNING, tr("Multipolygon is not closed"), NON_CLOSED_WAY, primitives, openNodes)); + errors.add(TestError.builder(this, Severity.WARNING, NON_CLOSED_WAY) + .message(tr("Multipolygon is not closed")) + .primitives(addRelationIfNeeded(r, openNodes)) + .highlight(openNodes) + .build()); } @@ -328,6 +339,9 @@ } if (outside) { - addError(r, new TestError(this, Severity.WARNING, tr("Multipolygon inner way is outside"), - INNER_WAY_OUTSIDE, Collections.singletonList(r), Arrays.asList(pdInner.getNodes()))); + errors.add(TestError.builder(this, Severity.WARNING, INNER_WAY_OUTSIDE) + .message(tr("Multipolygon inner way is outside")) + .primitives(r) + .highlightNodePairs(Collections.singletonList(pdInner.getNodes())) + .build()); } } @@ -339,6 +353,9 @@ PolyData pdOther = polygons.get(idx); if (pdOther != null) { - addError(r, new TestError(this, Severity.WARNING, tr("Intersection between multipolygon ways"), - CROSSING_WAYS, Collections.singletonList(r), Arrays.asList(pd.getNodes(), pdOther.getNodes()))); + errors.add(TestError.builder(this, Severity.WARNING, CROSSING_WAYS) + .message(tr("Intersection between multipolygon ways")) + .primitives(r) + .highlightNodePairs(Arrays.asList(pd.getNodes(), pdOther.getNodes())) + .build()); } } @@ -357,23 +374,31 @@ if (rm.isWay()) { if (!(rm.hasRole("inner", "outer") || !rm.hasRole())) { - addError(r, new TestError(this, Severity.WARNING, tr("No useful role for multipolygon member"), - WRONG_MEMBER_ROLE, rm.getMember())); + errors.add(TestError.builder(this, Severity.WARNING, WRONG_MEMBER_ROLE) + .message(tr("No useful role for multipolygon member")) + .primitives(addRelationIfNeeded(r, rm.getMember())) + .build()); } } else { if (!rm.hasRole("admin_centre", "label", "subarea", "land_area")) { - addError(r, new TestError(this, Severity.WARNING, tr("Non-Way in multipolygon"), WRONG_MEMBER_TYPE, rm.getMember())); - } - } - } - } - - private static void addRelationIfNeeded(TestError error, Relation r) { + errors.add(TestError.builder(this, Severity.WARNING, WRONG_MEMBER_TYPE) + .message(tr("Non-Way in multipolygon")) + .primitives(addRelationIfNeeded(r, rm.getMember())) + .build()); + } + } + } + } + + private static Collection addRelationIfNeeded(Relation r, OsmPrimitive primitive) { + return addRelationIfNeeded(r, Collections.singleton(primitive)); + } + + private static Collection addRelationIfNeeded(Relation r, Collection primitives) { // Fix #8212 : if the error references only incomplete primitives, // add multipolygon in order to let user select something and fix the error - Collection primitives = error.getPrimitives(); if (!primitives.contains(r)) { for (OsmPrimitive p : primitives) { if (!p.isIncomplete()) { - return; + return null; } } @@ -382,11 +407,9 @@ List newPrimitives = new ArrayList(primitives); newPrimitives.add(0, r); - error.setPrimitives(newPrimitives); - } - } - - private void addError(Relation r, TestError error) { - addRelationIfNeeded(error, r); - errors.add(error); - } + return newPrimitives; + } else { + return primitives; + } + } + } Index: /trunk/src/org/openstreetmap/josm/data/validation/tests/NameMismatch.java =================================================================== --- /trunk/src/org/openstreetmap/josm/data/validation/tests/NameMismatch.java (revision 11128) +++ /trunk/src/org/openstreetmap/josm/data/validation/tests/NameMismatch.java (revision 11129) @@ -2,4 +2,5 @@ package org.openstreetmap.josm.data.validation.tests; +import static org.openstreetmap.josm.tools.I18n.marktr; import static org.openstreetmap.josm.tools.I18n.tr; @@ -51,8 +52,8 @@ */ private void missingTranslation(OsmPrimitive p, String name) { - errors.add(new TestError(this, Severity.OTHER, - tr("Missing name:* translation"), - tr("Missing name:*={0}. Add tag with correct language key.", name), - String.format("Missing name:*=%s. Add tag with correct language key.", name), NAME_TRANSLATION_MISSING, p)); + errors.add(TestError.builder(this, Severity.OTHER, NAME_TRANSLATION_MISSING) + .message(tr("Missing name:* translation"), marktr("Missing name:*={0}. Add tag with correct language key."), name) + .primitives(p) + .build()); } @@ -80,7 +81,8 @@ if (name == null) { - errors.add(new TestError(this, Severity.OTHER, - tr("A name is missing, even though name:* exists."), - NAME_MISSING, p)); + errors.add(TestError.builder(this, Severity.OTHER, NAME_MISSING) + .message(tr("A name is missing, even though name:* exists.")) + .primitives(p) + .build()); return; } Index: /trunk/src/org/openstreetmap/josm/data/validation/tests/OpeningHourTest.java =================================================================== --- /trunk/src/org/openstreetmap/josm/data/validation/tests/OpeningHourTest.java (revision 11128) +++ /trunk/src/org/openstreetmap/josm/data/validation/tests/OpeningHourTest.java (revision 11129) @@ -18,5 +18,4 @@ import org.openstreetmap.josm.command.ChangePropertyCommand; import org.openstreetmap.josm.data.osm.OsmPrimitive; -import org.openstreetmap.josm.data.validation.FixableTestError; import org.openstreetmap.josm.data.validation.Severity; import org.openstreetmap.josm.data.validation.Test; @@ -132,10 +131,11 @@ */ public TestError getTestError(final OsmPrimitive p, final String key) { - final String messageEn = message; // todo obtain English message for ignore functionality + final TestError.Builder error = TestError.builder(OpeningHourTest.this, severity, 2901) + .message(tr("Opening hours syntax"), message) // todo obtain English message for ignore functionality + .primitives(p); if (prettifiedValue == null || prettifiedValue.equals(p.get(key))) { - return new TestError(OpeningHourTest.this, severity, tr("Opening hours syntax"), message, messageEn, 2901, p); + return error.build(); } else { - return new FixableTestError(OpeningHourTest.this, severity, tr("Opening hours syntax"), message, messageEn, 2901, p, - new ChangePropertyCommand(p, key, prettifiedValue)); + return error.fix(() -> new ChangePropertyCommand(p, key, prettifiedValue)).build(); } } Index: /trunk/src/org/openstreetmap/josm/data/validation/tests/OverlappingWays.java =================================================================== --- /trunk/src/org/openstreetmap/josm/data/validation/tests/OverlappingWays.java (revision 11128) +++ /trunk/src/org/openstreetmap/josm/data/validation/tests/OverlappingWays.java (revision 11129) @@ -140,7 +140,10 @@ } - preliminaryErrors.add(new TestError(this, - type < OVERLAPPING_HIGHWAY_AREA ? Severity.WARNING : Severity.OTHER, - errortype, type, prims, duplicated)); + 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 */ @@ -202,6 +205,9 @@ final Set duplicateWaySegment = checkDuplicateWaySegment(w); if (duplicateWaySegment != null) { - errors.add(new TestError(this, Severity.ERROR, tr("Way contains segment twice"), - DUPLICATE_WAY_SEGMENT, Collections.singleton(w), duplicateWaySegment)); + errors.add(TestError.builder(this, Severity.ERROR, DUPLICATE_WAY_SEGMENT) + .message(tr("Way contains segment twice")) + .primitives(w) + .highlightWaySegments(duplicateWaySegment) + .build()); return; } Index: /trunk/src/org/openstreetmap/josm/data/validation/tests/PowerLines.java =================================================================== --- /trunk/src/org/openstreetmap/josm/data/validation/tests/PowerLines.java (revision 11128) +++ /trunk/src/org/openstreetmap/josm/data/validation/tests/PowerLines.java (revision 11129) @@ -7,12 +7,8 @@ import java.util.Arrays; import java.util.Collection; -import java.util.HashMap; -import java.util.Iterator; import java.util.List; -import java.util.Map; import org.openstreetmap.josm.Main; import org.openstreetmap.josm.command.ChangePropertyCommand; -import org.openstreetmap.josm.command.Command; import org.openstreetmap.josm.data.osm.Node; import org.openstreetmap.josm.data.osm.OsmPrimitive; @@ -48,7 +44,5 @@ "portal", "terminal", "insulator"); - private final Map towerPoleTagMap = new HashMap<>(); - - private final List potentialErrors = new ArrayList<>(); + private final List potentialErrors = new ArrayList<>(); private final List powerStations = new ArrayList<>(); @@ -66,5 +60,6 @@ if (isPowerLine(w) && !w.hasTag("location", "underground")) { String fixValue = null; - boolean erroneous = false; + TestError.Builder error = null; + Node errorNode = null; boolean canFix = false; for (Node n : w.getNodes()) { @@ -72,6 +67,8 @@ if (!isPowerAllowed(n) && IN_DOWNLOADED_AREA.test(n)) { if (!w.isFirstLastNode(n) || !isPowerStation(n)) { - potentialErrors.add(new PowerLineError(this, n, w)); - erroneous = true; + error = TestError.builder(this, Severity.WARNING, POWER_LINES) + .message(tr("Missing power tower/pole within power line")) + .primitives(n); + errorNode = n; } } @@ -85,6 +82,9 @@ } } - if (erroneous && canFix) { - towerPoleTagMap.put(w, fixValue); + if (error != null && canFix) { + final ChangePropertyCommand fix = new ChangePropertyCommand(errorNode, "power", fixValue); + potentialErrors.add(error.fix(() -> fix).build()); + } else if (error != null) { + potentialErrors.add(error.build()); } } else if (w.isClosed() && isPowerStation(w)) { @@ -104,5 +104,4 @@ public void startTest(ProgressMonitor progressMonitor) { super.startTest(progressMonitor); - towerPoleTagMap.clear(); powerStations.clear(); potentialErrors.clear(); @@ -111,9 +110,10 @@ @Override public void endTest() { - for (PowerLineError e : potentialErrors) { - Node n = e.getNode(); - if (n != null && !isInPowerStation(n)) { - errors.add(e); - } + for (TestError e : potentialErrors) { + e.getPrimitives().stream() + .map(Node.class::cast) + .filter(n -> !isInPowerStation(n)) + .findAny() + .ifPresent(ignore -> errors.add(e)); } potentialErrors.clear(); @@ -143,22 +143,4 @@ } - @Override - public Command fixError(TestError testError) { - if (testError instanceof PowerLineError && isFixable(testError)) { - // primitives list can be empty if all primitives have been purged - Iterator it = testError.getPrimitives().iterator(); - if (it.hasNext()) { - return new ChangePropertyCommand(it.next(), - "power", towerPoleTagMap.get(((PowerLineError) testError).line)); - } - } - return null; - } - - @Override - public boolean isFixable(TestError testError) { - return testError instanceof PowerLineError && towerPoleTagMap.containsKey(((PowerLineError) testError).line); - } - /** * Determines if the specified way denotes a power line. @@ -218,19 +200,3 @@ return v != null && values != null && values.contains(v); } - - protected static class PowerLineError extends TestError { - private final Way line; - - public PowerLineError(PowerLines tester, Node n, Way line) { - super(tester, Severity.WARNING, - tr("Missing power tower/pole within power line"), POWER_LINES, n); - this.line = line; - } - - public final Node getNode() { - // primitives list can be empty if all primitives have been purged - Iterator it = getPrimitives().iterator(); - return it.hasNext() ? (Node) it.next() : null; - } - } } Index: /trunk/src/org/openstreetmap/josm/data/validation/tests/PublicTransportRouteTest.java =================================================================== --- /trunk/src/org/openstreetmap/josm/data/validation/tests/PublicTransportRouteTest.java (revision 11128) +++ /trunk/src/org/openstreetmap/josm/data/validation/tests/PublicTransportRouteTest.java (revision 11129) @@ -5,5 +5,4 @@ import java.util.ArrayList; -import java.util.Arrays; import java.util.HashSet; import java.util.List; @@ -48,5 +47,8 @@ for (RelationMember member : r.getMembers()) { if (member.hasRole("forward", "backward")) { - errors.add(new TestError(this, Severity.WARNING, tr("Route relation contains a ''{0}'' role", "forward/backward"), 3601, r)); + errors.add(TestError.builder(this, Severity.WARNING, 3601) + .message(tr("Route relation contains a ''{0}'' role", "forward/backward")) + .primitives(r) + .build()); return; } else if (member.hasRole("") && OsmPrimitiveType.WAY.equals(member.getType())) { @@ -67,5 +69,8 @@ || WayConnectionType.Direction.NONE.equals(link.direction); if (hasError) { - errors.add(new TestError(this, Severity.WARNING, tr("Route relation contains a gap"), 3602, r)); + errors.add(TestError.builder(this, Severity.WARNING, 3602) + .message(tr("Route relation contains a gap")) + .primitives(r) + .build()); return; } @@ -76,6 +81,8 @@ && OsmPrimitiveType.NODE.equals(member.getType()) && !routeNodes.contains(member.getNode())) { - errors.add(new TestError(this, Severity.WARNING, - tr("Stop position not part of route"), 3603, Arrays.asList(member.getMember(), r))); + errors.add(TestError.builder(this, Severity.WARNING, 3603) + .message(tr("Stop position not part of route")) + .primitives(member.getMember(), r) + .build()); } } Index: /trunk/src/org/openstreetmap/josm/data/validation/tests/RelationChecker.java =================================================================== --- /trunk/src/org/openstreetmap/josm/data/validation/tests/RelationChecker.java (revision 11128) +++ /trunk/src/org/openstreetmap/josm/data/validation/tests/RelationChecker.java (revision 11129) @@ -5,5 +5,4 @@ import static org.openstreetmap.josm.tools.I18n.tr; -import java.text.MessageFormat; import java.util.Collection; import java.util.EnumSet; @@ -116,14 +115,21 @@ if (allroles.isEmpty() && n.hasTag("type", "route") && n.hasTag("route", "train", "subway", "monorail", "tram", "bus", "trolleybus", "aerialway", "ferry")) { - errors.add(new TestError(this, Severity.WARNING, - tr("Route scheme is unspecified. Add {0} ({1}=public_transport; {2}=legacy)", "public_transport:version", "2", "1"), - RELATION_UNKNOWN, n)); + errors.add(TestError.builder(this, Severity.WARNING, RELATION_UNKNOWN) + .message(tr("Route scheme is unspecified. Add {0} ({1}=public_transport; {2}=legacy)", "public_transport:version", "2", "1")) + .primitives(n) + .build()); } else if (allroles.isEmpty()) { - errors.add(new TestError(this, Severity.WARNING, tr("Relation type is unknown"), RELATION_UNKNOWN, n)); + errors.add(TestError.builder(this, Severity.WARNING, RELATION_UNKNOWN) + .message(tr("Relation type is unknown")) + .primitives(n) + .build()); } Map map = buildRoleInfoMap(n); if (map.isEmpty()) { - errors.add(new TestError(this, Severity.ERROR, tr("Relation is empty"), RELATION_EMPTY, n)); + errors.add(TestError.builder(this, Severity.ERROR, RELATION_EMPTY) + .message(tr("Relation is empty")) + .primitives(n) + .build()); } else if (!allroles.isEmpty()) { checkRoles(n, allroles, map); @@ -230,8 +236,10 @@ // different present, for which memberExpression will match // but stash the error in case no better reason will be found later - String s = marktr("Role member does not match expression {0} in template {1}"); - possibleMatchError = new TestError(this, Severity.WARNING, ROLE_VERIF_PROBLEM_MSG, - tr(s, r.memberExpression, rolePreset.name), s, WRONG_TYPE, - member.getMember().isUsable() ? member.getMember() : n); + possibleMatchError = TestError.builder(this, Severity.WARNING, WRONG_TYPE) + .message(ROLE_VERIF_PROBLEM_MSG, + marktr("Role member does not match expression {0} in template {1}"), + r.memberExpression, rolePreset.name) + .primitives(member.getMember().isUsable() ? member.getMember() : n) + .build(); } } @@ -252,5 +260,4 @@ // no errors found till now. So member at least failed at matching the type // it could also fail at memberExpression, but we can't guess at which - String s = marktr("Role member type {0} does not match accepted list of {1} in template {2}"); // prepare Set of all accepted types in template @@ -263,7 +270,10 @@ String typesStr = types.stream().map(x -> tr(x.getName())).collect(Collectors.joining("/")); - errors.add(new TestError(this, Severity.WARNING, ROLE_VERIF_PROBLEM_MSG, - tr(s, member.getType(), typesStr, rolePreset.name), s, WRONG_TYPE, - member.getMember().isUsable() ? member.getMember() : n)); + errors.add(TestError.builder(this, Severity.WARNING, WRONG_TYPE) + .message(ROLE_VERIF_PROBLEM_MSG, + marktr("Role member type {0} does not match accepted list of {1} in template {2}"), + member.getType(), typesStr, rolePreset.name) + .primitives(member.getMember().isUsable() ? member.getMember() : n) + .build()); } return false; @@ -301,12 +311,14 @@ if (!key.isEmpty()) { - String s = marktr("Role {0} unknown in templates {1}"); - - errors.add(new TestError(this, Severity.WARNING, ROLE_VERIF_PROBLEM_MSG, - tr(s, key, templates), MessageFormat.format(s, key), ROLE_UNKNOWN, n)); + + errors.add(TestError.builder(this, Severity.WARNING, ROLE_UNKNOWN) + .message(ROLE_VERIF_PROBLEM_MSG, marktr("Role {0} unknown in templates {1}"), key, templates) + .primitives(n) + .build()); } else { - String s = marktr("Empty role type found when expecting one of {0}"); - errors.add(new TestError(this, Severity.WARNING, ROLE_VERIF_PROBLEM_MSG, - tr(s, templates), s, ROLE_EMPTY, n)); + errors.add(TestError.builder(this, Severity.WARNING, ROLE_EMPTY) + .message(ROLE_VERIF_PROBLEM_MSG, marktr("Empty role type found when expecting one of {0}"), templates) + .primitives(n) + .build()); } } @@ -319,15 +331,18 @@ if (count != vc) { if (count == 0) { - String s = marktr("Role {0} missing"); - errors.add(new TestError(this, Severity.WARNING, ROLE_VERIF_PROBLEM_MSG, - tr(s, keyname), MessageFormat.format(s, keyname), ROLE_MISSING, n)); + errors.add(TestError.builder(this, Severity.WARNING, ROLE_MISSING) + .message(ROLE_VERIF_PROBLEM_MSG, marktr("Role {0} missing"), keyname) + .primitives(n) + .build()); } else if (vc > count) { - String s = marktr("Number of {0} roles too low ({1})"); - errors.add(new TestError(this, Severity.WARNING, ROLE_VERIF_PROBLEM_MSG, - tr(s, keyname, count), MessageFormat.format(s, keyname, count), LOW_COUNT, n)); + errors.add(TestError.builder(this, Severity.WARNING, LOW_COUNT) + .message(ROLE_VERIF_PROBLEM_MSG, marktr("Number of {0} roles too low ({1})"), keyname, count) + .primitives(n) + .build()); } else { - String s = marktr("Number of {0} roles too high ({1})"); - errors.add(new TestError(this, Severity.WARNING, ROLE_VERIF_PROBLEM_MSG, - tr(s, keyname, count), MessageFormat.format(s, keyname, count), HIGH_COUNT, n)); + errors.add(TestError.builder(this, Severity.WARNING, HIGH_COUNT) + .message(ROLE_VERIF_PROBLEM_MSG, marktr("Number of {0} roles too high ({1})"), keyname, count) + .primitives(n) + .build()); } } Index: /trunk/src/org/openstreetmap/josm/data/validation/tests/SelfIntersectingWay.java =================================================================== --- /trunk/src/org/openstreetmap/josm/data/validation/tests/SelfIntersectingWay.java (revision 11128) +++ /trunk/src/org/openstreetmap/josm/data/validation/tests/SelfIntersectingWay.java (revision 11129) @@ -4,5 +4,4 @@ import static org.openstreetmap.josm.tools.I18n.tr; -import java.util.Arrays; import java.util.HashSet; import java.util.Set; @@ -37,7 +36,9 @@ Node n = w.getNode(i); if (nodes.contains(n)) { - errors.add(new TestError(this, - Severity.WARNING, tr("Self-intersecting ways"), SELF_INTERSECT, - Arrays.asList(w), Arrays.asList(n))); + errors.add(TestError.builder(this, Severity.WARNING, SELF_INTERSECT) + .message(tr("Self-intersecting ways")) + .primitives(w) + .highlight(n) + .build()); break; } else { Index: /trunk/src/org/openstreetmap/josm/data/validation/tests/SimilarNamedWays.java =================================================================== --- /trunk/src/org/openstreetmap/josm/data/validation/tests/SimilarNamedWays.java (revision 11128) +++ /trunk/src/org/openstreetmap/josm/data/validation/tests/SimilarNamedWays.java (revision 11129) @@ -97,5 +97,8 @@ primitives.add(w); primitives.add(w2); - errors.add(new TestError(this, Severity.WARNING, tr("Similarly named ways"), SIMILAR_NAMED, primitives)); + errors.add(TestError.builder(this, Severity.WARNING, SIMILAR_NAMED) + .message(tr("Similarly named ways")) + .primitives(primitives) + .build()); errorWays.put(w, w2); } Index: /trunk/src/org/openstreetmap/josm/data/validation/tests/TagChecker.java =================================================================== --- /trunk/src/org/openstreetmap/josm/data/validation/tests/TagChecker.java (revision 11128) +++ /trunk/src/org/openstreetmap/josm/data/validation/tests/TagChecker.java (revision 11129) @@ -9,5 +9,4 @@ import java.io.BufferedReader; import java.io.IOException; -import java.text.MessageFormat; import java.util.ArrayList; import java.util.Arrays; @@ -37,5 +36,4 @@ import org.openstreetmap.josm.data.osm.OsmUtils; import org.openstreetmap.josm.data.osm.Tag; -import org.openstreetmap.josm.data.validation.FixableTestError; import org.openstreetmap.josm.data.validation.Severity; import org.openstreetmap.josm.data.validation.Test.TagTest; @@ -392,6 +390,8 @@ for (CheckerData d : checkerData) { if (d.match(p, keys)) { - errors.add(new TestError(this, d.getSeverity(), tr("Suspicious tag/value combinations"), - d.getDescription(), d.getDescriptionOrig(), d.getCode(), p)); + errors.add(TestError.builder(this, d.getSeverity(), d.getCode()) + .message(tr("Suspicious tag/value combinations"), d.getDescription()) + .primitives(p) + .build()); withErrors.put(p, "TC"); } @@ -404,46 +404,64 @@ String value = prop.getValue(); if (checkValues && (containsLow(value)) && !withErrors.contains(p, "ICV")) { - errors.add(new TestError(this, Severity.WARNING, tr("Tag value contains character with code less than 0x20"), - tr(s, key), MessageFormat.format(s, key), LOW_CHAR_VALUE, p)); + errors.add(TestError.builder(this, Severity.WARNING, LOW_CHAR_VALUE) + .message(tr("Tag value contains character with code less than 0x20"), s, key) + .primitives(p) + .build()); withErrors.put(p, "ICV"); } if (checkKeys && (containsLow(key)) && !withErrors.contains(p, "ICK")) { - errors.add(new TestError(this, Severity.WARNING, tr("Tag key contains character with code less than 0x20"), - tr(s, key), MessageFormat.format(s, key), LOW_CHAR_KEY, p)); + errors.add(TestError.builder(this, Severity.WARNING, LOW_CHAR_KEY) + .message(tr("Tag key contains character with code less than 0x20"), s, key) + .primitives(p) + .build()); withErrors.put(p, "ICK"); } if (checkValues && (value != null && value.length() > 255) && !withErrors.contains(p, "LV")) { - errors.add(new TestError(this, Severity.ERROR, tr("Tag value longer than allowed"), - tr(s, key), MessageFormat.format(s, key), LONG_VALUE, p)); + errors.add(TestError.builder(this, Severity.ERROR, LONG_VALUE) + .message(tr("Tag value longer than allowed"), s, key) + .primitives(p) + .build()); withErrors.put(p, "LV"); } if (checkKeys && (key != null && key.length() > 255) && !withErrors.contains(p, "LK")) { - errors.add(new TestError(this, Severity.ERROR, tr("Tag key longer than allowed"), - tr(s, key), MessageFormat.format(s, key), LONG_KEY, p)); + errors.add(TestError.builder(this, Severity.ERROR, LONG_KEY) + .message(tr("Tag key longer than allowed"), s, key) + .primitives(p) + .build()); withErrors.put(p, "LK"); } if (checkValues && (value == null || value.trim().isEmpty()) && !withErrors.contains(p, "EV")) { - errors.add(new TestError(this, Severity.WARNING, tr("Tags with empty values"), - tr(s, key), MessageFormat.format(s, key), EMPTY_VALUES, p)); + errors.add(TestError.builder(this, Severity.WARNING, EMPTY_VALUES) + .message(tr("Tags with empty values"), s, key) + .primitives(p) + .build()); withErrors.put(p, "EV"); } if (checkKeys && key != null && key.indexOf(' ') >= 0 && !withErrors.contains(p, "IPK")) { - errors.add(new TestError(this, Severity.WARNING, tr("Invalid white space in property key"), - tr(s, key), MessageFormat.format(s, key), INVALID_KEY_SPACE, p)); + errors.add(TestError.builder(this, Severity.WARNING, INVALID_KEY_SPACE) + .message(tr("Invalid white space in property key"), s, key) + .primitives(p) + .build()); withErrors.put(p, "IPK"); } if (checkValues && value != null && (value.startsWith(" ") || value.endsWith(" ")) && !withErrors.contains(p, "SPACE")) { - errors.add(new TestError(this, Severity.WARNING, tr("Property values start or end with white space"), - tr(s, key), MessageFormat.format(s, key), INVALID_SPACE, p)); + errors.add(TestError.builder(this, Severity.WARNING, INVALID_SPACE) + .message(tr("Property values start or end with white space"), s, key) + .primitives(p) + .build()); withErrors.put(p, "SPACE"); } if (checkValues && value != null && value.contains(" ") && !withErrors.contains(p, "SPACE")) { - errors.add(new TestError(this, Severity.WARNING, tr("Property values contain multiple white spaces"), - tr(s, key), MessageFormat.format(s, key), MULTIPLE_SPACES, p)); + errors.add(TestError.builder(this, Severity.WARNING, MULTIPLE_SPACES) + .message(tr("Property values contain multiple white spaces"), s, key) + .primitives(p) + .build()); withErrors.put(p, "SPACE"); } if (checkValues && value != null && !value.equals(Entities.unescape(value)) && !withErrors.contains(p, "HTML")) { - errors.add(new TestError(this, Severity.OTHER, tr("Property values contain HTML entity"), - tr(s, key), MessageFormat.format(s, key), INVALID_HTML, p)); + errors.add(TestError.builder(this, Severity.OTHER, INVALID_HTML) + .message(tr("Property values contain HTML entity"), s, key) + .primitives(p) + .build()); withErrors.put(p, "HTML"); } @@ -455,22 +473,18 @@ if (fixedKey != null && !"".equals(fixedKey) && !fixedKey.equals(key)) { // misspelled preset key - String i = marktr("Key ''{0}'' looks like ''{1}''."); - final TestError error; + final TestError.Builder error = TestError.builder(this, Severity.WARNING, MISSPELLED_KEY) + .message(tr("Misspelled property key"), marktr("Key ''{0}'' looks like ''{1}''."), key, fixedKey) + .primitives(p); if (p.hasKey(fixedKey)) { - error = new TestError(this, Severity.WARNING, tr("Misspelled property key"), - tr(i, key, fixedKey), - MessageFormat.format(i, key, fixedKey), MISSPELLED_KEY, p); + errors.add(error.build()); } else { - error = new FixableTestError(this, Severity.WARNING, tr("Misspelled property key"), - tr(i, key, fixedKey), - MessageFormat.format(i, key, fixedKey), MISSPELLED_KEY, p, - new ChangePropertyKeyCommand(p, key, fixedKey)); + errors.add(error.fix(() -> new ChangePropertyKeyCommand(p, key, fixedKey)).build()); } - errors.add(error); withErrors.put(p, "WPK"); } else { - String i = marktr("Key ''{0}'' not in presets."); - errors.add(new TestError(this, Severity.OTHER, tr("Presets do not contain property key"), - tr(i, key), MessageFormat.format(i, key), INVALID_VALUE, p)); + errors.add(TestError.builder(this, Severity.OTHER, INVALID_VALUE) + .message(tr("Presets do not contain property key"), marktr("Key ''{0}'' not in presets."), key) + .primitives(p) + .build()); withErrors.put(p, "UPK"); } @@ -480,16 +494,20 @@ Map possibleValues = getPossibleValues(presetsValueData.get(key)); if (possibleValues.containsKey(fixedValue)) { - fixedValue = possibleValues.get(fixedValue); + final String newKey = possibleValues.get(fixedValue); // misspelled preset value - String i = marktr("Value ''{0}'' for key ''{1}'' looks like ''{2}''."); - errors.add(new FixableTestError(this, Severity.WARNING, tr("Misspelled property value"), - tr(i, prop.getValue(), key, fixedValue), MessageFormat.format(i, prop.getValue(), fixedValue), - MISSPELLED_VALUE, p, new ChangePropertyCommand(p, key, fixedValue))); + errors.add(TestError.builder(this, Severity.WARNING, MISSPELLED_VALUE) + .message(tr("Misspelled property value"), + marktr("Value ''{0}'' for key ''{1}'' looks like ''{2}''."), prop.getValue(), key, fixedValue) + .primitives(p) + .fix(() -> new ChangePropertyCommand(p, key, newKey)) + .build()); withErrors.put(p, "WPV"); } else { // unknown preset value - String i = marktr("Value ''{0}'' for key ''{1}'' not in presets."); - errors.add(new TestError(this, Severity.OTHER, tr("Presets do not contain property value"), - tr(i, prop.getValue(), key), MessageFormat.format(i, prop.getValue(), key), INVALID_VALUE, p)); + errors.add(TestError.builder(this, Severity.OTHER, INVALID_VALUE) + .message(tr("Presets do not contain property value"), + marktr("Value ''{0}'' for key ''{1}'' not in presets."), prop.getValue(), key) + .primitives(p) + .build()); withErrors.put(p, "UPV"); } @@ -502,6 +520,8 @@ || key.contains("todo") || key.toLowerCase(Locale.ENGLISH).contains("fixme")) && !withErrors.contains(p, "FIXME")) { - errors.add(new TestError(this, Severity.OTHER, - tr("FIXMES"), FIXME, p)); + errors.add(TestError.builder(this, Severity.OTHER, FIXME) + .message(tr("FIXMES")) + .primitives(p) + .build()); withErrors.put(p, "FIXME"); } @@ -646,28 +666,24 @@ List commands = new ArrayList<>(50); - if (testError instanceof FixableTestError) { - commands.add(testError.getFix()); - } else { - Collection primitives = testError.getPrimitives(); - for (OsmPrimitive p : primitives) { - Map tags = p.getKeys(); - if (tags == null || tags.isEmpty()) { - continue; - } - - for (Entry prop: tags.entrySet()) { - String key = prop.getKey(); - String value = prop.getValue(); - if (value == null || value.trim().isEmpty()) { - commands.add(new ChangePropertyCommand(p, key, null)); - } else if (value.startsWith(" ") || value.endsWith(" ") || value.contains(" ")) { - commands.add(new ChangePropertyCommand(p, key, Tag.removeWhiteSpaces(value))); - } else if (key.startsWith(" ") || key.endsWith(" ") || key.contains(" ")) { - commands.add(new ChangePropertyKeyCommand(p, key, Tag.removeWhiteSpaces(key))); - } else { - String evalue = Entities.unescape(value); - if (!evalue.equals(value)) { - commands.add(new ChangePropertyCommand(p, key, evalue)); - } + Collection primitives = testError.getPrimitives(); + for (OsmPrimitive p : primitives) { + Map tags = p.getKeys(); + if (tags == null || tags.isEmpty()) { + continue; + } + + for (Entry prop: tags.entrySet()) { + String key = prop.getKey(); + String value = prop.getValue(); + if (value == null || value.trim().isEmpty()) { + commands.add(new ChangePropertyCommand(p, key, null)); + } else if (value.startsWith(" ") || value.endsWith(" ") || value.contains(" ")) { + commands.add(new ChangePropertyCommand(p, key, Tag.removeWhiteSpaces(value))); + } else if (key.startsWith(" ") || key.endsWith(" ") || key.contains(" ")) { + commands.add(new ChangePropertyKeyCommand(p, key, Tag.removeWhiteSpaces(key))); + } else { + String evalue = Entities.unescape(value); + if (!evalue.equals(value)) { + commands.add(new ChangePropertyCommand(p, key, evalue)); } } @@ -840,8 +856,4 @@ public String getDescription() { - return tr(description); - } - - public String getDescriptionOrig() { return description; } Index: /trunk/src/org/openstreetmap/josm/data/validation/tests/TurnrestrictionTest.java =================================================================== --- /trunk/src/org/openstreetmap/josm/data/validation/tests/TurnrestrictionTest.java (revision 11128) +++ /trunk/src/org/openstreetmap/josm/data/validation/tests/TurnrestrictionTest.java (revision 11129) @@ -5,6 +5,4 @@ import java.util.ArrayList; -import java.util.Arrays; -import java.util.Collections; import java.util.List; @@ -99,6 +97,9 @@ break; default: - errors.add(new TestError(this, Severity.WARNING, tr("Unknown role"), UNKNOWN_ROLE, - l, Collections.singletonList(m))); + errors.add(TestError.builder(this, Severity.WARNING, UNKNOWN_ROLE) + .message(tr("Unknown role")) + .primitives(l) + .highlight(m.getMember()) + .build()); } } else if (m.isNode()) { @@ -115,39 +116,69 @@ } } else { - errors.add(new TestError(this, Severity.WARNING, tr("Unknown role"), UNKNOWN_ROLE, - l, Collections.singletonList(m))); + errors.add(TestError.builder(this, Severity.WARNING, UNKNOWN_ROLE) + .message(tr("Unknown role")) + .primitives(l) + .highlight(m.getMember()) + .build()); } } else { - errors.add(new TestError(this, Severity.WARNING, tr("Unknown member type"), UNKNOWN_TYPE, - l, Collections.singletonList(m))); + errors.add(TestError.builder(this, Severity.WARNING, UNKNOWN_TYPE) + .message(tr("Unknown member type")) + .primitives(l) + .highlight(m.getMember()) + .build()); } } if (morefrom) { - errors.add(new TestError(this, Severity.ERROR, tr("More than one \"from\" way found"), MORE_FROM, r)); + errors.add(TestError.builder(this, Severity.ERROR, MORE_FROM) + .message(tr("More than one \"from\" way found")) + .primitives(r) + .build()); } if (moreto) { - errors.add(new TestError(this, Severity.ERROR, tr("More than one \"to\" way found"), MORE_TO, r)); + errors.add(TestError.builder(this, Severity.ERROR, MORE_TO) + .message(tr("More than one \"to\" way found")) + .primitives(r) + .build()); } if (morevia) { - errors.add(new TestError(this, Severity.ERROR, tr("More than one \"via\" node found"), MORE_VIA, r)); + errors.add(TestError.builder(this, Severity.ERROR, MORE_VIA) + .message(tr("More than one \"via\" node found")) + .primitives(r) + .build()); } if (mixvia) { - errors.add(new TestError(this, Severity.ERROR, tr("Cannot mix node and way for role \"via\""), MIX_VIA, r)); + errors.add(TestError.builder(this, Severity.ERROR, MIX_VIA) + .message(tr("Cannot mix node and way for role \"via\"")) + .primitives(r) + .build()); } if (fromWay == null) { - errors.add(new TestError(this, Severity.ERROR, tr("No \"from\" way found"), NO_FROM, r)); + errors.add(TestError.builder(this, Severity.ERROR, NO_FROM) + .message(tr("No \"from\" way found")) + .primitives(r) + .build()); return; } if (toWay == null) { - errors.add(new TestError(this, Severity.ERROR, tr("No \"to\" way found"), NO_TO, r)); + errors.add(TestError.builder(this, Severity.ERROR, NO_TO) + .message(tr("No \"to\" way found")) + .primitives(r) + .build()); return; } if (fromWay.equals(toWay)) { - errors.add(new TestError(this, r.hasTag("restriction", "no_u_turn") ? Severity.OTHER : Severity.WARNING, - tr("\"from\" way equals \"to\" way"), FROM_EQUALS_TO, r)); + Severity severity = r.hasTag("restriction", "no_u_turn") ? Severity.OTHER : Severity.WARNING; + errors.add(TestError.builder(this, severity, FROM_EQUALS_TO) + .message(tr("\"from\" way equals \"to\" way")) + .primitives(r) + .build()); } if (via.isEmpty()) { - errors.add(new TestError(this, Severity.ERROR, tr("No \"via\" node or way found"), NO_VIA, r)); + errors.add(TestError.builder(this, Severity.ERROR, NO_VIA) + .message(tr("No \"via\" node or way found")) + .primitives(r) + .build()); return; } @@ -160,5 +191,8 @@ tr("The \"from\" way does not start or end at a \"via\" node."), FROM_VIA_NODE); if (toWay.isOneway() != 0 && viaNode.equals(toWay.lastNode(true))) { - errors.add(new TestError(this, Severity.WARNING, tr("Superfluous turnrestriction as \"to\" way is oneway"), SUPERFLUOUS, r)); + errors.add(TestError.builder(this, Severity.WARNING, SUPERFLUOUS) + .message(tr("Superfluous turnrestriction as \"to\" way is oneway")) + .primitives(r) + .build()); return; } @@ -178,5 +212,8 @@ } if (toWay.isOneway() != 0 && ((Way) via.get(via.size() - 1)).isFirstLastNode(toWay.lastNode(true))) { - errors.add(new TestError(this, Severity.WARNING, tr("Superfluous turnrestriction as \"to\" way is oneway"), SUPERFLUOUS, r)); + errors.add(TestError.builder(this, Severity.WARNING, SUPERFLUOUS) + .message(tr("Superfluous turnrestriction as \"to\" way is oneway")) + .primitives(r) + .build()); return; } @@ -206,5 +243,8 @@ } if (!c) { - errors.add(new TestError(this, Severity.ERROR, msg, code, Arrays.asList(previous, current))); + errors.add(TestError.builder(this, Severity.ERROR, code) + .message(msg) + .primitives(previous, current) + .build()); } } Index: /trunk/src/org/openstreetmap/josm/data/validation/tests/UnclosedWays.java =================================================================== --- /trunk/src/org/openstreetmap/josm/data/validation/tests/UnclosedWays.java (revision 11128) +++ /trunk/src/org/openstreetmap/josm/data/validation/tests/UnclosedWays.java (revision 11129) @@ -5,5 +5,4 @@ import static org.openstreetmap.josm.tools.I18n.tr; -import java.text.MessageFormat; import java.util.Arrays; import java.util.Collections; @@ -96,13 +95,9 @@ String value = w.get(key); if (isValueErroneous(value)) { - // CHECKSTYLE.OFF: SingleSpaceSeparator - String type = engMessage.contains("{0}") ? tr(engMessage, tr(value)) : tr(engMessage); - String etype = engMessage.contains("{0}") ? MessageFormat.format(engMessage, value) : engMessage; - // CHECKSTYLE.ON: SingleSpaceSeparator - return new TestError(test, Severity.WARNING, tr("Unclosed way"), - type, etype, code, Arrays.asList(w), - // The important parts of an unclosed way are the first and - // the last node which should be connected, therefore we highlight them - Arrays.asList(w.firstNode(), w.lastNode())); + return TestError.builder(test, Severity.WARNING, code) + .message(tr("Unclosed way"), engMessage, engMessage.contains("{0}") ? new Object[]{value} : new Object[]{}) + .primitives(w) + .highlight(Arrays.asList(w.firstNode(), w.lastNode())) + .build(); } return null; Index: /trunk/src/org/openstreetmap/josm/data/validation/tests/UnconnectedWays.java =================================================================== --- /trunk/src/org/openstreetmap/josm/data/validation/tests/UnconnectedWays.java (revision 11128) +++ /trunk/src/org/openstreetmap/josm/data/validation/tests/UnconnectedWays.java (revision 11129) @@ -8,5 +8,4 @@ import java.awt.geom.Point2D; import java.util.ArrayList; -import java.util.Arrays; import java.util.Collection; import java.util.Collections; @@ -264,7 +263,9 @@ protected final void addErrors(Severity severity, Map errorMap, String message) { for (Map.Entry error : errorMap.entrySet()) { - errors.add(new TestError(this, severity, message, UNCONNECTED_WAYS, - Arrays.asList(error.getKey(), error.getValue()), - Arrays.asList(error.getKey()))); + errors.add(TestError.builder(this, severity, UNCONNECTED_WAYS) + .message(message) + .primitives(error.getKey(), error.getValue()) + .highlight(error.getKey()) + .build()); } } Index: /trunk/src/org/openstreetmap/josm/data/validation/tests/UntaggedNode.java =================================================================== --- /trunk/src/org/openstreetmap/josm/data/validation/tests/UntaggedNode.java (revision 11128) +++ /trunk/src/org/openstreetmap/josm/data/validation/tests/UntaggedNode.java (revision 11129) @@ -44,6 +44,8 @@ if (!n.hasKeys() && IN_DOWNLOADED_AREA.test(n)) { - String msg = marktr("No tags"); - errors.add(new TestError(this, Severity.WARNING, ERROR_MESSAGE, tr(msg), msg, UNTAGGED_NODE_BLANK, n)); + errors.add(TestError.builder(this, Severity.WARNING, UNTAGGED_NODE_BLANK) + .message(ERROR_MESSAGE, marktr("No tags")) + .primitives(n) + .build()); return; } @@ -56,6 +58,8 @@ if (key.toLowerCase(Locale.ENGLISH).contains("fixme") || value.toLowerCase(Locale.ENGLISH).contains("fixme")) { /* translation note: don't translate quoted words */ - String msg = marktr("Has tag containing ''fixme'' or ''FIXME''"); - errors.add(new TestError(this, Severity.WARNING, ERROR_MESSAGE, tr(msg), msg, UNTAGGED_NODE_FIXME, (OsmPrimitive) n)); + errors.add(TestError.builder(this, Severity.WARNING, UNTAGGED_NODE_FIXME) + .message(ERROR_MESSAGE, marktr("Has tag containing ''fixme'' or ''FIXME''")) + .primitives((OsmPrimitive) n) + .build()); return; } @@ -81,9 +85,15 @@ } if (msg != null) { - errors.add(new TestError(this, Severity.WARNING, ERROR_MESSAGE, tr(msg), msg, code, (OsmPrimitive) n)); + errors.add(TestError.builder(this, Severity.WARNING, code) + .message(ERROR_MESSAGE, msg) + .primitives((OsmPrimitive) n) + .build()); return; } // Does not happen, but just to be sure. Maybe definition of uninteresting tags changes in future. - errors.add(new TestError(this, Severity.WARNING, ERROR_MESSAGE, tr("Other"), "Other", UNTAGGED_NODE_OTHER, (OsmPrimitive) n)); + errors.add(TestError.builder(this, Severity.WARNING, UNTAGGED_NODE_OTHER) + .message(ERROR_MESSAGE, marktr("Other")) + .primitives((OsmPrimitive) n) + .build()); } Index: /trunk/src/org/openstreetmap/josm/data/validation/tests/UntaggedWay.java =================================================================== --- /trunk/src/org/openstreetmap/josm/data/validation/tests/UntaggedWay.java (revision 11128) +++ /trunk/src/org/openstreetmap/josm/data/validation/tests/UntaggedWay.java (revision 11129) @@ -98,7 +98,13 @@ if (!hasName && !isJunction) { - errors.add(new TestError(this, Severity.WARNING, tr("Unnamed ways"), UNNAMED_WAY, w)); + errors.add(TestError.builder(this, Severity.WARNING, UNNAMED_WAY) + .message(tr("Unnamed ways")) + .primitives(w) + .build()); } else if (isJunction) { - errors.add(new TestError(this, Severity.OTHER, tr("Unnamed junction"), UNNAMED_JUNCTION, w)); + errors.add(TestError.builder(this, Severity.OTHER, UNNAMED_JUNCTION) + .message(tr("Unnamed junction")) + .primitives(w) + .build()); } } @@ -107,14 +113,26 @@ if (!w.isTagged() && !waysUsedInRelations.contains(w)) { if (w.hasKeys()) { - errors.add(new TestError(this, Severity.WARNING, tr("Untagged ways (commented)"), COMMENTED_WAY, w)); + errors.add(TestError.builder(this, Severity.WARNING, COMMENTED_WAY) + .message(tr("Untagged ways (commented)")) + .primitives(w) + .build()); } else { - errors.add(new TestError(this, Severity.WARNING, tr("Untagged ways"), UNTAGGED_WAY, w)); + errors.add(TestError.builder(this, Severity.WARNING, UNTAGGED_WAY) + .message(tr("Untagged ways")) + .primitives(w) + .build()); } } if (w.getNodesCount() == 0) { - errors.add(new TestError(this, Severity.ERROR, tr("Empty ways"), EMPTY_WAY, w)); + errors.add(TestError.builder(this, Severity.ERROR, EMPTY_WAY) + .message(tr("Empty ways")) + .primitives(w) + .build()); } else if (w.getNodesCount() == 1) { - errors.add(new TestError(this, Severity.ERROR, tr("One node ways"), ONE_NODE_WAY, w)); + errors.add(TestError.builder(this, Severity.ERROR, ONE_NODE_WAY) + .message(tr("One node ways")) + .primitives(w) + .build()); } } Index: /trunk/src/org/openstreetmap/josm/data/validation/tests/WayConnectedToArea.java =================================================================== --- /trunk/src/org/openstreetmap/josm/data/validation/tests/WayConnectedToArea.java (revision 11128) +++ /trunk/src/org/openstreetmap/josm/data/validation/tests/WayConnectedToArea.java (revision 11129) @@ -4,5 +4,4 @@ import static org.openstreetmap.josm.tools.I18n.tr; -import java.util.Arrays; import java.util.List; @@ -92,8 +91,9 @@ return; } - errors.add(new TestError(this, Severity.WARNING, - tr("Way terminates on Area"), 2301, - Arrays.asList(w, p), - Arrays.asList(wayNode))); + errors.add(TestError.builder(this, Severity.WARNING, 2301) + .message(tr("Way terminates on Area")) + .primitives(w, p) + .highlight(wayNode) + .build()); } } Index: /trunk/src/org/openstreetmap/josm/data/validation/tests/WronglyOrderedWays.java =================================================================== --- /trunk/src/org/openstreetmap/josm/data/validation/tests/WronglyOrderedWays.java (revision 11128) +++ /trunk/src/org/openstreetmap/josm/data/validation/tests/WronglyOrderedWays.java (revision 11129) @@ -3,6 +3,4 @@ import static org.openstreetmap.josm.tools.I18n.tr; - -import java.util.Collections; import org.openstreetmap.josm.data.osm.Way; @@ -51,5 +49,8 @@ private void reportError(Way w, String msg, int type) { - errors.add(new TestError(this, Severity.WARNING, msg, type, Collections.singletonList(w))); + errors.add(TestError.builder(this, Severity.WARNING, type) + .message(msg) + .primitives(w) + .build()); } } Index: /trunk/test/unit/org/openstreetmap/josm/gui/dialogs/validator/ValidatorTreePanelTest.java =================================================================== --- /trunk/test/unit/org/openstreetmap/josm/gui/dialogs/validator/ValidatorTreePanelTest.java (revision 11128) +++ /trunk/test/unit/org/openstreetmap/josm/gui/dialogs/validator/ValidatorTreePanelTest.java (revision 11129) @@ -42,6 +42,12 @@ ValidatorTreePanel vtp = new ValidatorTreePanel(new ArrayList<>(Arrays.asList( - new TestError(null, Severity.ERROR, "err", 0, new Node(1)), - new TestError(null, Severity.WARNING, "warn", 0, new Node(2))))); + TestError.builder(null, Severity.ERROR, 0) + .message("err") + .primitives(new Node(1)) + .build(), + TestError.builder(null, Severity.WARNING, 0) + .message("warn") + .primitives(new Node(2)) + .build()))); assertNotNull(vtp); assertEquals(2, vtp.getErrors().size()); @@ -49,5 +55,8 @@ vtp.setVisible(false); Node n = new Node(10); - vtp.setErrors(Arrays.asList(new TestError(null, Severity.ERROR, "", 0, n))); + vtp.setErrors(Arrays.asList(TestError.builder(null, Severity.ERROR, 0) + .message("") + .primitives(n) + .build())); assertEquals(1, vtp.getErrors().size()); vtp.selectRelatedErrors(Collections.singleton(n));