Index: trunk/src/org/openstreetmap/josm/data/validation/FixableTestError.java =================================================================== --- trunk/src/org/openstreetmap/josm/data/validation/FixableTestError.java (revision 8433) +++ trunk/src/org/openstreetmap/josm/data/validation/FixableTestError.java (revision 8435) @@ -14,4 +14,13 @@ 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); @@ -19,26 +28,84 @@ } - public FixableTestError(Test tester, Severity severity, String message, int code, Collection primitives, Command 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; } - public FixableTestError(Test tester, Severity severity, String message, int code, Collection primitives, Collection highlighted, Command 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; } - public FixableTestError(Test tester, Severity severity, String message, String description, String description_en, int code, OsmPrimitive primitive, Command fix) { - super(tester, severity, message, description, description_en, code, primitive); + /** + * 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; } - public FixableTestError(Test tester, Severity severity, String message, String description, String description_en, int code, Collection primitives, Command fix) { - super(tester, severity, message, description, description_en, code, primitives); + /** + * 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; } - public FixableTestError(Test tester, Severity severity, String message, String description, String description_en, int code, Collection primitives, Collection highlighted, Command fix) { - super(tester, severity, message, description, description_en, code, primitives, highlighted); + /** + * 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; } Index: trunk/src/org/openstreetmap/josm/data/validation/TestError.java =================================================================== --- trunk/src/org/openstreetmap/josm/data/validation/TestError.java (revision 8433) +++ trunk/src/org/openstreetmap/josm/data/validation/TestError.java (revision 8435) @@ -57,8 +57,11 @@ * @param severity The severity of this error * @param message The error message + * @param description The translated description + * @param descriptionEn The English description * @param primitives The affected primitives * @param code The test error reference code - */ - public TestError(Test tester, Severity severity, String message, String description, String description_en, + * @param highlighted OSM primitives to highlight + */ + public TestError(Test tester, Severity severity, String message, String description, String descriptionEn, int code, Collection primitives, Collection highlighted) { this.tester = tester; @@ -66,5 +69,5 @@ this.message = message; this.description = description; - this.descriptionEn = description_en; + this.descriptionEn = descriptionEn; this.primitives = primitives; this.highlighted = highlighted; @@ -72,4 +75,13 @@ } + /** + * Constructs a new {@code TestError} without description. + * @param tester The tester + * @param severity The severity of this error + * @param message The error message + * @param primitives The affected primitives + * @param code The test error reference code + * @param highlighted OSM primitives to highlight + */ public TestError(Test tester, Severity severity, String message, int code, Collection primitives, Collection highlighted) { @@ -77,13 +89,39 @@ } - public TestError(Test tester, Severity severity, String message, String description, String description_en, + /** + * Constructs a new {@code TestError}. + * @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 primitives The affected primitives + * @param code The test error reference code + */ + public TestError(Test tester, Severity severity, String message, String description, String descriptionEn, int code, Collection primitives) { - this(tester, severity, message, description, description_en, code, primitives, primitives); - } - + this(tester, severity, message, description, descriptionEn, code, primitives, primitives); + } + + /** + * Constructs a new {@code TestError} without description. + * @param tester The tester + * @param severity The severity of this error + * @param message The error message + * @param primitives The affected primitives + * @param code The test error reference code + */ public TestError(Test tester, Severity severity, String message, int code, Collection primitives) { this(tester, severity, message, null, null, code, primitives, primitives); } + /** + * Constructs a new {@code TestError} without description, for a single primitive. + * @param tester The tester + * @param severity The severity of this error + * @param message The error message + * @param primitive The affected primitive + * @param code The test error reference code + */ public TestError(Test tester, Severity severity, String message, int code, OsmPrimitive primitive) { this(tester, severity, message, null, null, code, Collections.singletonList(primitive), Collections @@ -91,7 +129,17 @@ } - public TestError(Test tester, Severity severity, String message, String description, String description_en, + /** + * Constructs a new {@code TestError} 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 primitive The affected primitive + * @param code The test error reference code + */ + public TestError(Test tester, Severity severity, String message, String description, String descriptionEn, int code, OsmPrimitive primitive) { - this(tester, severity, message, description, description_en, code, Collections.singletonList(primitive)); + this(tester, severity, message, description, descriptionEn, code, Collections.singletonList(primitive)); } Index: trunk/src/org/openstreetmap/josm/data/validation/tests/TagChecker.java =================================================================== --- trunk/src/org/openstreetmap/josm/data/validation/tests/TagChecker.java (revision 8433) +++ trunk/src/org/openstreetmap/josm/data/validation/tests/TagChecker.java (revision 8435) @@ -39,6 +39,7 @@ 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; +import org.openstreetmap.josm.data.validation.Test.TagTest; import org.openstreetmap.josm.data.validation.TestError; import org.openstreetmap.josm.data.validation.util.Entities; @@ -56,4 +57,5 @@ import org.openstreetmap.josm.tools.GBC; import org.openstreetmap.josm.tools.MultiMap; +import org.openstreetmap.josm.tools.Utils; /** @@ -61,9 +63,8 @@ * * @author frsantos + * @since 3669 */ -public class TagChecker extends Test.TagTest { - - /** The default data file of tagchecker rules */ - //public static final String DATA_FILE = "resource://data/validator/tagchecker.cfg"; +public class TagChecker extends TagTest { + /** The config file of ignored tags */ public static final String IGNORE_FILE = "resource://data/validator/ignoretags.cfg"; @@ -125,4 +126,5 @@ protected static final int LOW_CHAR_VALUE = 1210; protected static final int LOW_CHAR_KEY = 1211; + protected static final int MISSPELLED_VALUE = 1212; /** 1250 and up is used by tagcheck */ @@ -324,6 +326,6 @@ 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(new TestError(this, d.getSeverity(), tr("Suspicious tag/value combinations"), + d.getDescription(), d.getDescriptionOrig(), d.getCode(), p)); withErrors.put(p, "TC"); } @@ -336,46 +338,46 @@ 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(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)); 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(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)); 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(new TestError(this, Severity.ERROR, tr("Tag value longer than allowed"), + tr(s, key), MessageFormat.format(s, key), LONG_VALUE, p)); 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(new TestError(this, Severity.ERROR, tr("Tag key longer than allowed"), + tr(s, key), MessageFormat.format(s, key), LONG_KEY, p)); 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(new TestError(this, Severity.WARNING, tr("Tags with empty values"), + tr(s, key), MessageFormat.format(s, key), EMPTY_VALUES, p)); withErrors.put(p, "EV"); } if (checkKeys && spellCheckKeyData.containsKey(key) && !withErrors.contains(p, "IPK")) { - errors.add( new TestError(this, Severity.WARNING, tr("Invalid property key"), - tr(s, key), MessageFormat.format(s, key), INVALID_KEY, p) ); + errors.add(new TestError(this, Severity.WARNING, tr("Invalid property key"), + tr(s, key), MessageFormat.format(s, key), INVALID_KEY, p)); withErrors.put(p, "IPK"); } 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(new TestError(this, Severity.WARNING, tr("Invalid white space in property key"), + tr(s, key), MessageFormat.format(s, key), INVALID_KEY_SPACE, p)); 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(new TestError(this, Severity.WARNING, tr("Property values start or end with white space"), + tr(s, key), MessageFormat.format(s, key), INVALID_SPACE, p)); 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(new TestError(this, Severity.OTHER, tr("Property values contain HTML entity"), + tr(s, key), MessageFormat.format(s, key), INVALID_HTML, p)); withErrors.put(p, "HTML"); } @@ -413,12 +415,24 @@ if (!keyInPresets) { 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(new TestError(this, Severity.OTHER, tr("Presets do not contain property key"), + tr(i, key), MessageFormat.format(i, key), INVALID_VALUE, p)); withErrors.put(p, "UPK"); } else if (!tagInPresets) { - 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) ); - withErrors.put(p, "UPV"); + // try to fix common typos and check again if value is still unknown + String fixedValue = prettifyValue(prop.getValue()); + if (values != null && values.contains(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))); + 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)); + withErrors.put(p, "UPV"); + } } } @@ -435,4 +449,11 @@ } } + } + + private static String prettifyValue(String value) { + // convert to lower case, replace ' ' or '-' with '_' + value = value.toLowerCase(Locale.ENGLISH).replace('-', '_').replace(' ', '_'); + // remove trailing or leading special chars + return Utils.strip(value, "-_;:,"); } @@ -554,28 +575,32 @@ List commands = new ArrayList<>(50); - 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(" ")) { - commands.add(new ChangePropertyCommand(p, key, Tag.removeWhiteSpaces(value))); - } else if (key.startsWith(" ") || key.endsWith(" ")) { - 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)); + 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(" ")) { + commands.add(new ChangePropertyCommand(p, key, Tag.removeWhiteSpaces(value))); + } else if (key.startsWith(" ") || key.endsWith(" ")) { + commands.add(new ChangePropertyKeyCommand(p, key, Tag.removeWhiteSpaces(key))); } else { - String replacementKey = spellCheckKeyData.get(key); - if (replacementKey != null) { - commands.add(new ChangePropertyKeyCommand(p, key, replacementKey)); + String evalue = entities.unescape(value); + if (!evalue.equals(value)) { + commands.add(new ChangePropertyCommand(p, key, evalue)); + } else { + String replacementKey = spellCheckKeyData.get(key); + if (replacementKey != null) { + commands.add(new ChangePropertyKeyCommand(p, key, replacementKey)); + } } } @@ -596,5 +621,6 @@ if (testError.getTester() instanceof TagChecker) { int code = testError.getCode(); - return code == INVALID_KEY || code == EMPTY_VALUES || code == INVALID_SPACE || code == INVALID_KEY_SPACE || code == INVALID_HTML; + return code == INVALID_KEY || code == EMPTY_VALUES || code == INVALID_SPACE || + code == INVALID_KEY_SPACE || code == INVALID_HTML || code == MISSPELLED_VALUE; } Index: trunk/src/org/openstreetmap/josm/tools/Utils.java =================================================================== --- trunk/src/org/openstreetmap/josm/tools/Utils.java (revision 8433) +++ trunk/src/org/openstreetmap/josm/tools/Utils.java (revision 8435) @@ -40,7 +40,9 @@ import java.util.Collection; import java.util.Collections; +import java.util.HashSet; import java.util.Iterator; import java.util.List; import java.util.Locale; +import java.util.Set; import java.util.concurrent.ExecutorService; import java.util.concurrent.Executors; @@ -899,24 +901,46 @@ * @since 5772 */ - public static String strip(String str) { + public static String strip(final String str) { + return strip(str, null); + } + + /** + * An alternative to {@link String#trim()} to effectively remove all leading and trailing white characters, including Unicode ones. + * @param str The string to strip + * @param skipChars additional characters to skip + * @return str, without leading and trailing characters, according to + * {@link Character#isWhitespace(char)}, {@link Character#isSpaceChar(char)} and skipChars. + * @since 8435 + */ + public static String strip(final String str, final String skipChars) { if (str == null || str.isEmpty()) { return str; } - int start = 0, end = str.length(); - boolean leadingWhite = true; - while (leadingWhite && start < end) { + // create set with chars to skip + Set skipSet = new HashSet<>(); + // '\u200B' (ZERO WIDTH SPACE character) needs to be handled manually because of change in Unicode 6.0 (Java 7, see #8918) + skipSet.add('\u200B'); + // same for '\uFEFF' (ZERO WIDTH NO-BREAK SPACE) + skipSet.add('\uFEFF'); + if (skipChars != null) { + for (char c : skipChars.toCharArray()) { + skipSet.add(c); + } + } + int start = 0; + int end = str.length(); + boolean leadingSkipChar = true; + while (leadingSkipChar && start < end) { char c = str.charAt(start); - // '\u200B' (ZERO WIDTH SPACE character) needs to be handled manually because of change in Unicode 6.0 (Java 7, see #8918) - // same for '\uFEFF' (ZERO WIDTH NO-BREAK SPACE) - leadingWhite = (Character.isWhitespace(c) || Character.isSpaceChar(c) || c == '\u200B' || c == '\uFEFF'); - if (leadingWhite) { + leadingSkipChar = Character.isWhitespace(c) || Character.isSpaceChar(c) || skipSet.contains(c); + if (leadingSkipChar) { start++; } } - boolean trailingWhite = true; - while (trailingWhite && end > start+1) { + boolean trailingSkipChar = true; + while (trailingSkipChar && end > start+1) { char c = str.charAt(end-1); - trailingWhite = (Character.isWhitespace(c) || Character.isSpaceChar(c) || c == '\u200B' || c == '\uFEFF'); - if (trailingWhite) { + trailingSkipChar = Character.isWhitespace(c) || Character.isSpaceChar(c) || skipSet.contains(c); + if (trailingSkipChar) { end--; }