Index: trunk/src/org/openstreetmap/josm/data/validation/tests/MapCSSTagChecker.java =================================================================== --- trunk/src/org/openstreetmap/josm/data/validation/tests/MapCSSTagChecker.java (revision 17618) +++ trunk/src/org/openstreetmap/josm/data/validation/tests/MapCSSTagChecker.java (revision 17619) @@ -10,5 +10,4 @@ import java.io.InputStream; import java.io.Reader; -import java.io.StringReader; import java.util.ArrayList; import java.util.Collection; @@ -20,11 +19,6 @@ import java.util.Map.Entry; import java.util.Objects; -import java.util.Optional; import java.util.Set; import java.util.function.Consumer; -import java.util.function.Predicate; -import java.util.regex.Matcher; -import java.util.regex.Pattern; -import java.util.stream.Collectors; import java.util.stream.Stream; @@ -32,11 +26,7 @@ import org.openstreetmap.josm.command.ChangePropertyKeyCommand; import org.openstreetmap.josm.command.Command; -import org.openstreetmap.josm.command.DeleteCommand; -import org.openstreetmap.josm.command.SequenceCommand; import org.openstreetmap.josm.data.osm.IPrimitive; import org.openstreetmap.josm.data.osm.OsmPrimitive; import org.openstreetmap.josm.data.osm.Tag; -import org.openstreetmap.josm.data.osm.Way; -import org.openstreetmap.josm.data.osm.WaySegment; import org.openstreetmap.josm.data.preferences.sources.SourceEntry; import org.openstreetmap.josm.data.preferences.sources.ValidatorPrefHelper; @@ -46,15 +36,9 @@ import org.openstreetmap.josm.data.validation.TestError; import org.openstreetmap.josm.gui.mappaint.Environment; -import org.openstreetmap.josm.gui.mappaint.Keyword; import org.openstreetmap.josm.gui.mappaint.MultiCascade; -import org.openstreetmap.josm.gui.mappaint.mapcss.Condition; import org.openstreetmap.josm.gui.mappaint.mapcss.Expression; -import org.openstreetmap.josm.gui.mappaint.mapcss.Instruction; import org.openstreetmap.josm.gui.mappaint.mapcss.MapCSSRule; import org.openstreetmap.josm.gui.mappaint.mapcss.MapCSSStyleIndex; -import org.openstreetmap.josm.gui.mappaint.mapcss.MapCSSStyleSource; import org.openstreetmap.josm.gui.mappaint.mapcss.Selector; -import org.openstreetmap.josm.gui.mappaint.mapcss.Selector.GeneralSelector; -import org.openstreetmap.josm.gui.mappaint.mapcss.parsergen.MapCSSParser; import org.openstreetmap.josm.gui.mappaint.mapcss.parsergen.ParseException; import org.openstreetmap.josm.gui.mappaint.mapcss.parsergen.TokenMgrError; @@ -62,5 +46,4 @@ import org.openstreetmap.josm.io.CachedFile; import org.openstreetmap.josm.io.FileWatcher; -import org.openstreetmap.josm.io.IllegalDataException; import org.openstreetmap.josm.io.UTFInputStreamReader; import org.openstreetmap.josm.spi.preferences.Config; @@ -103,5 +86,5 @@ /** * Creates the fixing {@link Command} for the given primitive. The {@code matchingSelector} is used to evaluate placeholders - * (cf. {@link MapCSSTagChecker.TagCheck#insertArguments(Selector, String, OsmPrimitive)}). + * (cf. {@link MapCSSTagCheckerRule#insertArguments(Selector, String, OsmPrimitive)}). * @param p OSM primitive * @param matchingSelector matching selector @@ -136,5 +119,5 @@ return null; } - return TagCheck.insertArguments(matchingSelector, s, p); + return MapCSSTagCheckerRule.insertArguments(matchingSelector, s, p); } @@ -192,6 +175,6 @@ public Command createCommand(OsmPrimitive p, Selector matchingSelector) { return new ChangePropertyKeyCommand(p, - TagCheck.insertArguments(matchingSelector, oldKey, p), - TagCheck.insertArguments(matchingSelector, newKey, p)); + MapCSSTagCheckerRule.insertArguments(matchingSelector, oldKey, p), + MapCSSTagCheckerRule.insertArguments(matchingSelector, newKey, p)); } @@ -204,5 +187,5 @@ } - final MultiMap checks = new MultiMap<>(); + final MultiMap checks = new MultiMap<>(); /** maps the source URL for a test to the title shown in the dialog where known */ @@ -210,10 +193,10 @@ /** - * Result of {@link TagCheck#readMapCSS} + * Result of {@link MapCSSTagCheckerRule#readMapCSS} * @since 8936 */ public static class ParseResult { /** Checks successfully parsed */ - public final List parseChecks; + public final List parseChecks; /** Errors that occurred during parsing */ public final Collection parseErrors; @@ -224,5 +207,5 @@ * @param parseErrors Errors that occurred during parsing */ - public ParseResult(List parseChecks, Collection parseErrors) { + public ParseResult(List parseChecks, Collection parseErrors) { this.parseChecks = parseChecks; this.parseErrors = parseErrors; @@ -230,375 +213,7 @@ } - /** - * Tag check. - */ - public static class TagCheck implements Predicate { - /** The selector of this {@code TagCheck} */ - protected final MapCSSRule rule; - /** Commands to apply in order to fix a matching primitive */ - protected final List fixCommands; - /** Tags (or arbitrary strings) of alternatives to be presented to the user */ - protected final List alternatives; - /** An {@link org.openstreetmap.josm.gui.mappaint.mapcss.Instruction.AssignmentInstruction}-{@link Severity} pair. - * Is evaluated on the matching primitive to give the error message. Map is checked to contain exactly one element. */ - protected final Map errors; - /** MapCSS Classes to set on matching primitives */ - protected final Collection setClassExpressions; - /** Denotes whether the object should be deleted for fixing it */ - protected boolean deletion; - /** A string used to group similar tests */ - protected String group; - - TagCheck(MapCSSRule rule) { - this.rule = rule; - this.fixCommands = new ArrayList<>(); - this.alternatives = new ArrayList<>(); - this.errors = new HashMap<>(); - this.setClassExpressions = new HashSet<>(); - } - - TagCheck(TagCheck check) { - this.rule = check.rule; - this.fixCommands = Utils.toUnmodifiableList(check.fixCommands); - this.alternatives = Utils.toUnmodifiableList(check.alternatives); - this.errors = Utils.toUnmodifiableMap(check.errors); - this.setClassExpressions = Utils.toUnmodifiableList(check.setClassExpressions); - this.deletion = check.deletion; - this.group = check.group; - } - - TagCheck toImmutable() { - return new TagCheck(this); - } - - private static final String POSSIBLE_THROWS = "throwError/throwWarning/throwOther"; - - static TagCheck ofMapCSSRule(final MapCSSRule rule, AssertionConsumer assertionConsumer) throws IllegalDataException { - final TagCheck check = new TagCheck(rule); - final Map assertions = new HashMap<>(); - for (Instruction i : rule.declaration.instructions) { - if (i instanceof Instruction.AssignmentInstruction) { - final Instruction.AssignmentInstruction ai = (Instruction.AssignmentInstruction) i; - if (ai.isSetInstruction) { - check.setClassExpressions.add(ai.key); - continue; - } - try { - final String val = ai.val instanceof Expression - ? Optional.ofNullable(((Expression) ai.val).evaluate(new Environment())) - .map(Object::toString).map(String::intern).orElse(null) - : ai.val instanceof String - ? (String) ai.val - : ai.val instanceof Keyword - ? ((Keyword) ai.val).val - : null; - if ("throwError".equals(ai.key)) { - check.errors.put(ai, Severity.ERROR); - } else if ("throwWarning".equals(ai.key)) { - check.errors.put(ai, Severity.WARNING); - } else if ("throwOther".equals(ai.key)) { - check.errors.put(ai, Severity.OTHER); - } else if (ai.key.startsWith("throw")) { - Logging.log(Logging.LEVEL_WARN, - "Unsupported " + ai.key + " instruction. Allowed instructions are " + POSSIBLE_THROWS + '.', null); - } else if ("fixAdd".equals(ai.key)) { - check.fixCommands.add(FixCommand.fixAdd(ai.val)); - } else if ("fixRemove".equals(ai.key)) { - CheckParameterUtil.ensureThat(!(ai.val instanceof String) || !(val != null && val.contains("=")), - "Unexpected '='. Please only specify the key to remove in: " + ai); - check.fixCommands.add(FixCommand.fixRemove(ai.val)); - } else if (val != null && "fixChangeKey".equals(ai.key)) { - CheckParameterUtil.ensureThat(val.contains("=>"), "Separate old from new key by '=>'!"); - final String[] x = val.split("=>", 2); - check.fixCommands.add(FixCommand.fixChangeKey(Utils.removeWhiteSpaces(x[0]), Utils.removeWhiteSpaces(x[1]))); - } else if (val != null && "fixDeleteObject".equals(ai.key)) { - CheckParameterUtil.ensureThat("this".equals(val), "fixDeleteObject must be followed by 'this'"); - check.deletion = true; - } else if (val != null && "suggestAlternative".equals(ai.key)) { - check.alternatives.add(val); - } else if (val != null && "assertMatch".equals(ai.key)) { - assertions.put(val, Boolean.TRUE); - } else if (val != null && "assertNoMatch".equals(ai.key)) { - assertions.put(val, Boolean.FALSE); - } else if (val != null && "group".equals(ai.key)) { - check.group = val; - } else if (ai.key.startsWith("-")) { - Logging.debug("Ignoring extension instruction: " + ai.key + ": " + ai.val); - } else { - throw new IllegalDataException("Cannot add instruction " + ai.key + ": " + ai.val + '!'); - } - } catch (IllegalArgumentException e) { - throw new IllegalDataException(e); - } - } - } - if (check.errors.isEmpty() && check.setClassExpressions.isEmpty()) { - throw new IllegalDataException( - "No "+POSSIBLE_THROWS+" given! You should specify a validation error message for " + rule.selectors); - } else if (check.errors.size() > 1) { - throw new IllegalDataException( - "More than one "+POSSIBLE_THROWS+" given! You should specify a single validation error message for " - + rule.selectors); - } - if (assertionConsumer != null) { - MapCSSTagCheckerAsserts.checkAsserts(check, assertions, assertionConsumer); - } - return check.toImmutable(); - } - - static ParseResult readMapCSS(Reader css) throws ParseException { - return readMapCSS(css, null); - } - - static ParseResult readMapCSS(Reader css, AssertionConsumer assertionConsumer) throws ParseException { - CheckParameterUtil.ensureParameterNotNull(css, "css"); - - final MapCSSStyleSource source = new MapCSSStyleSource(""); - final MapCSSParser preprocessor = new MapCSSParser(css, MapCSSParser.LexicalState.PREPROCESSOR); - try (StringReader mapcss = new StringReader(preprocessor.pp_root(source))) { - new MapCSSParser(mapcss, MapCSSParser.LexicalState.DEFAULT).sheet(source); - } - // Ignore "meta" rule(s) from external rules of JOSM wiki - source.removeMetaRules(); - List parseChecks = new ArrayList<>(); - for (MapCSSRule rule : source.rules) { - try { - parseChecks.add(TagCheck.ofMapCSSRule(rule, assertionConsumer)); - } catch (IllegalDataException e) { - Logging.error("Cannot add MapCSS rule: "+e.getMessage()); - source.logError(e); - } - } - return new ParseResult(parseChecks, source.getErrors()); - } - - @Override - public boolean test(OsmPrimitive primitive) { - // Tests whether the primitive contains a deprecated tag which is represented by this MapCSSTagChecker. - return whichSelectorMatchesPrimitive(primitive) != null; - } - - Selector whichSelectorMatchesPrimitive(OsmPrimitive primitive) { - return whichSelectorMatchesEnvironment(new Environment(primitive)); - } - - Selector whichSelectorMatchesEnvironment(Environment env) { - return rule.selectors.stream() - .filter(i -> i.matches(env.clearSelectorMatchingInformation())) - .findFirst() - .orElse(null); - } - - /** - * Determines the {@code index}-th key/value/tag (depending on {@code type}) of the - * {@link GeneralSelector}. - * @param matchingSelector matching selector - * @param index index - * @param type selector type ("key", "value" or "tag") - * @param p OSM primitive - * @return argument value, can be {@code null} - */ - static String determineArgument(GeneralSelector matchingSelector, int index, String type, OsmPrimitive p) { - try { - final Condition c = matchingSelector.getConditions().get(index); - final Tag tag = c instanceof Condition.ToTagConvertable - ? ((Condition.ToTagConvertable) c).asTag(p) - : null; - if (tag == null) { - return null; - } else if ("key".equals(type)) { - return tag.getKey(); - } else if ("value".equals(type)) { - return tag.getValue(); - } else if ("tag".equals(type)) { - return tag.toString(); - } - } catch (IndexOutOfBoundsException ignore) { - Logging.debug(ignore); - } - return null; - } - - /** - * Replaces occurrences of {i.key}, {i.value}, {i.tag} in {@code s} by the corresponding - * key/value/tag of the {@code index}-th {@link Condition} of {@code matchingSelector}. - * @param matchingSelector matching selector - * @param s any string - * @param p OSM primitive - * @return string with arguments inserted - */ - static String insertArguments(Selector matchingSelector, String s, OsmPrimitive p) { - if (s != null && matchingSelector instanceof Selector.ChildOrParentSelector) { - return insertArguments(((Selector.ChildOrParentSelector) matchingSelector).right, s, p); - } else if (s == null || !(matchingSelector instanceof GeneralSelector)) { - return s; - } - final Matcher m = Pattern.compile("\\{(\\d+)\\.(key|value|tag)\\}").matcher(s); - final StringBuffer sb = new StringBuffer(); - while (m.find()) { - final String argument = determineArgument((GeneralSelector) matchingSelector, - Integer.parseInt(m.group(1)), m.group(2), p); - try { - // Perform replacement with null-safe + regex-safe handling - m.appendReplacement(sb, String.valueOf(argument).replace("^(", "").replace(")$", "")); - } catch (IndexOutOfBoundsException | IllegalArgumentException e) { - Logging.log(Logging.LEVEL_ERROR, tr("Unable to replace argument {0} in {1}: {2}", argument, sb, e.getMessage()), e); - } - } - m.appendTail(sb); - return sb.toString(); - } - - /** - * Constructs a fix in terms of a {@link org.openstreetmap.josm.command.Command} for the {@link OsmPrimitive} - * if the error is fixable, or {@code null} otherwise. - * - * @param p the primitive to construct the fix for - * @return the fix or {@code null} - */ - Command fixPrimitive(OsmPrimitive p) { - if (p.getDataSet() == null || (fixCommands.isEmpty() && !deletion)) { - return null; - } - try { - final Selector matchingSelector = whichSelectorMatchesPrimitive(p); - Collection cmds = fixCommands.stream() - .map(fixCommand -> fixCommand.createCommand(p, matchingSelector)) - .filter(Objects::nonNull) - .collect(Collectors.toList()); - if (deletion && !p.isDeleted()) { - cmds.add(new DeleteCommand(p)); - } - return cmds.isEmpty() ? null - : new SequenceCommand(tr("Fix of {0}", getDescriptionForMatchingSelector(p, matchingSelector)), cmds); - } catch (IllegalArgumentException e) { - Logging.error(e); - return null; - } - } - - /** - * Constructs a (localized) message for this deprecation check. - * @param p OSM primitive - * - * @return a message - */ - String getMessage(OsmPrimitive p) { - if (errors.isEmpty()) { - // Return something to avoid NPEs - return rule.declaration.toString(); - } else { - final Object val = errors.keySet().iterator().next().val; - return String.valueOf( - val instanceof Expression - ? ((Expression) val).evaluate(new Environment(p)) - : val - ); - } - } - - /** - * Constructs a (localized) description for this deprecation check. - * @param p OSM primitive - * - * @return a description (possibly with alternative suggestions) - * @see #getDescriptionForMatchingSelector - */ - String getDescription(OsmPrimitive p) { - if (alternatives.isEmpty()) { - return getMessage(p); - } else { - /* I18N: {0} is the test error message and {1} is an alternative */ - return tr("{0}, use {1} instead", getMessage(p), String.join(tr(" or "), alternatives)); - } - } - - /** - * Constructs a (localized) description for this deprecation check - * where any placeholders are replaced by values of the matched selector. - * - * @param matchingSelector matching selector - * @param p OSM primitive - * @return a description (possibly with alternative suggestions) - */ - String getDescriptionForMatchingSelector(OsmPrimitive p, Selector matchingSelector) { - return insertArguments(matchingSelector, getDescription(p), p); - } - - Severity getSeverity() { - return errors.isEmpty() ? null : errors.values().iterator().next(); - } - - @Override - public String toString() { - return getDescription(null); - } - - /** - * Constructs a {@link TestError} for the given primitive, or returns null if the primitive does not give rise to an error. - * - * @param p the primitive to construct the error for - * @param matchingSelector the matching selector (e.g., obtained via {@link #whichSelectorMatchesPrimitive}) - * @param env the environment - * @param tester the tester - * @return an instance of {@link TestError}, or returns null if the primitive does not give rise to an error. - */ - protected List getErrorsForPrimitive(OsmPrimitive p, Selector matchingSelector, Environment env, Test tester) { - List res = new ArrayList<>(); - if (matchingSelector != null && !errors.isEmpty()) { - final Command fix = fixPrimitive(p); - final String description = getDescriptionForMatchingSelector(p, matchingSelector); - final String description1 = group == null ? description : group; - final String description2 = group == null ? null : description; - final String selector = matchingSelector.toString(); - TestError.Builder errorBuilder = TestError.builder(tester, getSeverity(), 3000) - .messageWithManuallyTranslatedDescription(description1, description2, selector); - if (fix != null) { - errorBuilder.fix(() -> fix); - } - if (env.child instanceof OsmPrimitive) { - res.add(errorBuilder.primitives(p, (OsmPrimitive) env.child).build()); - } else if (env.children != null) { - for (IPrimitive c : env.children) { - if (c instanceof OsmPrimitive) { - errorBuilder = TestError.builder(tester, getSeverity(), 3000) - .messageWithManuallyTranslatedDescription(description1, description2, selector); - if (fix != null) { - errorBuilder.fix(() -> fix); - } - // check if we have special information about highlighted objects */ - boolean hiliteFound = false; - if (env.intersections != null) { - Area is = env.intersections.get(c); - if (is != null) { - errorBuilder.highlight(is); - hiliteFound = true; - } - } - if (env.crossingWaysMap != null && !hiliteFound) { - Map, List> is = env.crossingWaysMap.get(c); - if (is != null) { - Set toHilite = new HashSet<>(); - for (List wsList : is.values()) { - toHilite.addAll(wsList); - } - errorBuilder.highlightWaySegments(toHilite); - } - } - res.add(errorBuilder.primitives(p, (OsmPrimitive) c).build()); - } - } - } else { - res.add(errorBuilder.primitives(p).build()); - } - } - return res; - } - - } - static class MapCSSTagCheckerAndRule extends MapCSSTagChecker { public final MapCSSRule rule; - private final TagCheck tagCheck; + private final MapCSSTagCheckerRule tagCheck; private final String source; @@ -609,5 +224,5 @@ } - MapCSSTagCheckerAndRule(TagCheck tagCheck, String source) { + MapCSSTagCheckerAndRule(MapCSSTagCheckerRule tagCheck, String source) { this.rule = tagCheck.rule; this.tagCheck = tagCheck; @@ -626,5 +241,6 @@ } - static MapCSSStyleIndex createMapCSSTagCheckerIndex(MultiMap checks, boolean includeOtherSeverity, boolean allTests) { + static MapCSSStyleIndex createMapCSSTagCheckerIndex( + MultiMap checks, boolean includeOtherSeverity, boolean allTests) { final MapCSSStyleIndex index = new MapCSSStyleIndex(); final Stream ruleStream = checks.values().stream() @@ -671,5 +287,5 @@ .findFirst() .orElse(null)); - TagCheck check = test == null ? null : test.tagCheck; + MapCSSTagCheckerRule check = test == null ? null : test.tagCheck; if (check != null) { r.declaration.execute(env); @@ -723,12 +339,12 @@ } - static Collection getErrorsForPrimitive(OsmPrimitive p, boolean includeOtherSeverity, - Collection> checksCol) { + static Collection getErrorsForPrimitive( + OsmPrimitive p, boolean includeOtherSeverity, Collection> checksCol) { // this variant is only used by the assertion tests final List r = new ArrayList<>(); final Environment env = new Environment(p, new MultiCascade(), Environment.DEFAULT_LAYER, null); env.mpAreaCache = mpAreaCache; - for (Set schecks : checksCol) { - for (TagCheck check : schecks) { + for (Set schecks : checksCol) { + for (MapCSSTagCheckerRule check : schecks) { boolean ignoreError = Severity.OTHER == check.getSeverity() && !includeOtherSeverity; // Do not run "information" level checks if not wanted, unless they also set a MapCSS class @@ -790,5 +406,5 @@ if (zip != null) I18n.addTexts(cache.getFile()); - result = TagCheck.readMapCSS(reader, assertionConsumer); + result = MapCSSTagCheckerRule.readMapCSS(reader, assertionConsumer); checks.remove(url); checks.putAll(url, result.parseChecks); @@ -890,5 +506,5 @@ Set surrounding = new HashSet<>(); - for (Entry> entry : checks.entrySet()) { + for (Entry> entry : checks.entrySet()) { if (isCanceled()) { break; @@ -905,7 +521,6 @@ * @param surrounding surrounding primitives, evtl. filled by this routine */ - private void visit(String url, Set checksForUrl, Collection selection, - Set surrounding) { - MultiMap currentCheck = new MultiMap<>(); + private void visit(String url, Set checksForUrl, Collection selection, Set surrounding) { + MultiMap currentCheck = new MultiMap<>(); currentCheck.putAll(url, checksForUrl); indexData = createMapCSSTagCheckerIndex(currentCheck, includeOtherSeverityChecks(), ALL_TESTS); @@ -945,6 +560,5 @@ } - private void testPartial(MultiMap currentCheck, Set tested, - Set surrounding) { + private void testPartial(MultiMap currentCheck, Set tested, Set surrounding) { // #14287: see https://josm.openstreetmap.de/ticket/14287#comment:15 @@ -989,5 +603,5 @@ Set surrounding = new HashSet<>(); - for (Entry> entry : checks.entrySet()) { + for (Entry> entry : checks.entrySet()) { if (isCanceled()) { break; Index: trunk/src/org/openstreetmap/josm/data/validation/tests/MapCSSTagCheckerAsserts.java =================================================================== --- trunk/src/org/openstreetmap/josm/data/validation/tests/MapCSSTagCheckerAsserts.java (revision 17618) +++ trunk/src/org/openstreetmap/josm/data/validation/tests/MapCSSTagCheckerAsserts.java (revision 17619) @@ -35,5 +35,5 @@ /** - * Utility class for checking rule assertions of {@link MapCSSTagChecker.TagCheck}. + * Utility class for checking rule assertions of {@link MapCSSTagCheckerRule}. */ final class MapCSSTagCheckerAsserts { @@ -43,5 +43,5 @@ } - private static final ArrayList previousChecks = new ArrayList<>(); + private static final ArrayList previousChecks = new ArrayList<>(); /** @@ -51,6 +51,6 @@ * @param assertionConsumer The handler for assertion error messages */ - static void checkAsserts(final MapCSSTagChecker.TagCheck check, final Map assertions, - final MapCSSTagChecker.AssertionConsumer assertionConsumer) { + static void checkAsserts(final MapCSSTagCheckerRule check, final Map assertions, + final MapCSSTagChecker.AssertionConsumer assertionConsumer) { final Method insideMethod = getFunctionMethod("inside"); final DataSet ds = new DataSet(); @@ -60,6 +60,6 @@ final OsmPrimitive p = OsmUtils.createPrimitive(i.getKey(), getLocation(check, insideMethod), true); // Build minimal ordered list of checks to run to test the assertion - List> checksToRun = new ArrayList<>(); - Set checkDependencies = getTagCheckDependencies(check, previousChecks); + List> checksToRun = new ArrayList<>(); + Set checkDependencies = getTagCheckDependencies(check, previousChecks); if (!checkDependencies.isEmpty()) { checksToRun.add(checkDependencies); @@ -112,5 +112,5 @@ } - private static LatLon getLocation(MapCSSTagChecker.TagCheck check, Method insideMethod) { + private static LatLon getLocation(MapCSSTagCheckerRule check, Method insideMethod) { Optional inside = getFirstInsideCountry(check, insideMethod); if (inside.isPresent()) { @@ -126,5 +126,5 @@ } - private static Optional getFirstInsideCountry(MapCSSTagChecker.TagCheck check, Method insideMethod) { + private static Optional getFirstInsideCountry(MapCSSTagCheckerRule check, Method insideMethod) { return check.rule.selectors.stream() .filter(s -> s instanceof Selector.GeneralSelector) @@ -150,7 +150,7 @@ * @since 7881 */ - private static Set getTagCheckDependencies(MapCSSTagChecker.TagCheck check, - Collection schecks) { - Set result = new HashSet<>(); + private static Set getTagCheckDependencies(MapCSSTagCheckerRule check, + Collection schecks) { + Set result = new HashSet<>(); Set classes = check.rule.selectors.stream() .filter(s -> s instanceof Selector.AbstractSelector) Index: trunk/src/org/openstreetmap/josm/data/validation/tests/MapCSSTagCheckerRule.java =================================================================== --- trunk/src/org/openstreetmap/josm/data/validation/tests/MapCSSTagCheckerRule.java (revision 17619) +++ trunk/src/org/openstreetmap/josm/data/validation/tests/MapCSSTagCheckerRule.java (revision 17619) @@ -0,0 +1,434 @@ +// License: GPL. For details, see LICENSE file. +package org.openstreetmap.josm.data.validation.tests; + +import org.openstreetmap.josm.command.Command; +import org.openstreetmap.josm.command.DeleteCommand; +import org.openstreetmap.josm.command.SequenceCommand; +import org.openstreetmap.josm.data.osm.IPrimitive; +import org.openstreetmap.josm.data.osm.OsmPrimitive; +import org.openstreetmap.josm.data.osm.Tag; +import org.openstreetmap.josm.data.osm.Way; +import org.openstreetmap.josm.data.osm.WaySegment; +import org.openstreetmap.josm.data.validation.Severity; +import org.openstreetmap.josm.data.validation.Test; +import org.openstreetmap.josm.data.validation.TestError; +import org.openstreetmap.josm.data.validation.tests.MapCSSTagChecker.AssertionConsumer; +import org.openstreetmap.josm.gui.mappaint.Environment; +import org.openstreetmap.josm.gui.mappaint.Keyword; +import org.openstreetmap.josm.gui.mappaint.mapcss.Condition; +import org.openstreetmap.josm.gui.mappaint.mapcss.Expression; +import org.openstreetmap.josm.gui.mappaint.mapcss.Instruction; +import org.openstreetmap.josm.gui.mappaint.mapcss.MapCSSRule; +import org.openstreetmap.josm.gui.mappaint.mapcss.MapCSSStyleSource; +import org.openstreetmap.josm.gui.mappaint.mapcss.Selector; +import org.openstreetmap.josm.gui.mappaint.mapcss.parsergen.MapCSSParser; +import org.openstreetmap.josm.gui.mappaint.mapcss.parsergen.ParseException; +import org.openstreetmap.josm.io.IllegalDataException; +import org.openstreetmap.josm.tools.CheckParameterUtil; +import org.openstreetmap.josm.tools.Logging; +import org.openstreetmap.josm.tools.Utils; + +import java.awt.geom.Area; +import java.io.Reader; +import java.io.StringReader; +import java.util.ArrayList; +import java.util.Collection; +import java.util.HashMap; +import java.util.HashSet; +import java.util.List; +import java.util.Map; +import java.util.Objects; +import java.util.Optional; +import java.util.Set; +import java.util.function.Predicate; +import java.util.regex.Matcher; +import java.util.regex.Pattern; +import java.util.stream.Collectors; + +import static org.openstreetmap.josm.tools.I18n.tr; + +/** + * Tag check. + */ +final class MapCSSTagCheckerRule implements Predicate { + /** + * The selector of this {@code TagCheck} + */ + protected final MapCSSRule rule; + /** + * Commands to apply in order to fix a matching primitive + */ + protected final List fixCommands; + /** + * Tags (or arbitrary strings) of alternatives to be presented to the user + */ + protected final List alternatives; + /** + * An {@link org.openstreetmap.josm.gui.mappaint.mapcss.Instruction.AssignmentInstruction}-{@link Severity} pair. + * Is evaluated on the matching primitive to give the error message. Map is checked to contain exactly one element. + */ + protected final Map errors; + /** + * MapCSS Classes to set on matching primitives + */ + protected final Collection setClassExpressions; + /** + * Denotes whether the object should be deleted for fixing it + */ + protected boolean deletion; + /** + * A string used to group similar tests + */ + protected String group; + + MapCSSTagCheckerRule(MapCSSRule rule) { + this.rule = rule; + this.fixCommands = new ArrayList<>(); + this.alternatives = new ArrayList<>(); + this.errors = new HashMap<>(); + this.setClassExpressions = new HashSet<>(); + } + + MapCSSTagCheckerRule(MapCSSTagCheckerRule check) { + this.rule = check.rule; + this.fixCommands = Utils.toUnmodifiableList(check.fixCommands); + this.alternatives = Utils.toUnmodifiableList(check.alternatives); + this.errors = Utils.toUnmodifiableMap(check.errors); + this.setClassExpressions = Utils.toUnmodifiableList(check.setClassExpressions); + this.deletion = check.deletion; + this.group = check.group; + } + + MapCSSTagCheckerRule toImmutable() { + return new MapCSSTagCheckerRule(this); + } + + private static final String POSSIBLE_THROWS = "throwError/throwWarning/throwOther"; + + static MapCSSTagCheckerRule ofMapCSSRule(final MapCSSRule rule, AssertionConsumer assertionConsumer) throws IllegalDataException { + final MapCSSTagCheckerRule check = new MapCSSTagCheckerRule(rule); + final Map assertions = new HashMap<>(); + for (Instruction i : rule.declaration.instructions) { + if (i instanceof Instruction.AssignmentInstruction) { + final Instruction.AssignmentInstruction ai = (Instruction.AssignmentInstruction) i; + if (ai.isSetInstruction) { + check.setClassExpressions.add(ai.key); + continue; + } + try { + final String val = ai.val instanceof Expression + ? Optional.ofNullable(((Expression) ai.val).evaluate(new Environment())) + .map(Object::toString).map(String::intern).orElse(null) + : ai.val instanceof String + ? (String) ai.val + : ai.val instanceof Keyword + ? ((Keyword) ai.val).val + : null; + if ("throwError".equals(ai.key)) { + check.errors.put(ai, Severity.ERROR); + } else if ("throwWarning".equals(ai.key)) { + check.errors.put(ai, Severity.WARNING); + } else if ("throwOther".equals(ai.key)) { + check.errors.put(ai, Severity.OTHER); + } else if (ai.key.startsWith("throw")) { + Logging.log(Logging.LEVEL_WARN, + "Unsupported " + ai.key + " instruction. Allowed instructions are " + POSSIBLE_THROWS + '.', null); + } else if ("fixAdd".equals(ai.key)) { + check.fixCommands.add(MapCSSTagChecker.FixCommand.fixAdd(ai.val)); + } else if ("fixRemove".equals(ai.key)) { + CheckParameterUtil.ensureThat(!(ai.val instanceof String) || !(val != null && val.contains("=")), + "Unexpected '='. Please only specify the key to remove in: " + ai); + check.fixCommands.add(MapCSSTagChecker.FixCommand.fixRemove(ai.val)); + } else if (val != null && "fixChangeKey".equals(ai.key)) { + CheckParameterUtil.ensureThat(val.contains("=>"), "Separate old from new key by '=>'!"); + final String[] x = val.split("=>", 2); + final String oldKey = Utils.removeWhiteSpaces(x[0]); + final String newKey = Utils.removeWhiteSpaces(x[1]); + check.fixCommands.add(MapCSSTagChecker.FixCommand.fixChangeKey(oldKey, newKey)); + } else if (val != null && "fixDeleteObject".equals(ai.key)) { + CheckParameterUtil.ensureThat("this".equals(val), "fixDeleteObject must be followed by 'this'"); + check.deletion = true; + } else if (val != null && "suggestAlternative".equals(ai.key)) { + check.alternatives.add(val); + } else if (val != null && "assertMatch".equals(ai.key)) { + assertions.put(val, Boolean.TRUE); + } else if (val != null && "assertNoMatch".equals(ai.key)) { + assertions.put(val, Boolean.FALSE); + } else if (val != null && "group".equals(ai.key)) { + check.group = val; + } else if (ai.key.startsWith("-")) { + Logging.debug("Ignoring extension instruction: " + ai.key + ": " + ai.val); + } else { + throw new IllegalDataException("Cannot add instruction " + ai.key + ": " + ai.val + '!'); + } + } catch (IllegalArgumentException e) { + throw new IllegalDataException(e); + } + } + } + if (check.errors.isEmpty() && check.setClassExpressions.isEmpty()) { + throw new IllegalDataException( + "No " + POSSIBLE_THROWS + " given! You should specify a validation error message for " + rule.selectors); + } else if (check.errors.size() > 1) { + throw new IllegalDataException( + "More than one " + POSSIBLE_THROWS + " given! You should specify a single validation error message for " + + rule.selectors); + } + if (assertionConsumer != null) { + MapCSSTagCheckerAsserts.checkAsserts(check, assertions, assertionConsumer); + } + return check.toImmutable(); + } + + static MapCSSTagChecker.ParseResult readMapCSS(Reader css) throws ParseException { + return readMapCSS(css, null); + } + + static MapCSSTagChecker.ParseResult readMapCSS(Reader css, AssertionConsumer assertionConsumer) throws ParseException { + CheckParameterUtil.ensureParameterNotNull(css, "css"); + + final MapCSSStyleSource source = new MapCSSStyleSource(""); + final MapCSSParser preprocessor = new MapCSSParser(css, MapCSSParser.LexicalState.PREPROCESSOR); + try (StringReader mapcss = new StringReader(preprocessor.pp_root(source))) { + new MapCSSParser(mapcss, MapCSSParser.LexicalState.DEFAULT).sheet(source); + } + // Ignore "meta" rule(s) from external rules of JOSM wiki + source.removeMetaRules(); + List parseChecks = new ArrayList<>(); + for (MapCSSRule rule : source.rules) { + try { + parseChecks.add(MapCSSTagCheckerRule.ofMapCSSRule(rule, assertionConsumer)); + } catch (IllegalDataException e) { + Logging.error("Cannot add MapCSS rule: " + e.getMessage()); + source.logError(e); + } + } + return new MapCSSTagChecker.ParseResult(parseChecks, source.getErrors()); + } + + @Override + public boolean test(OsmPrimitive primitive) { + // Tests whether the primitive contains a deprecated tag which is represented by this MapCSSTagChecker. + return whichSelectorMatchesPrimitive(primitive) != null; + } + + Selector whichSelectorMatchesPrimitive(OsmPrimitive primitive) { + return whichSelectorMatchesEnvironment(new Environment(primitive)); + } + + Selector whichSelectorMatchesEnvironment(Environment env) { + return rule.selectors.stream() + .filter(i -> i.matches(env.clearSelectorMatchingInformation())) + .findFirst() + .orElse(null); + } + + /** + * Determines the {@code index}-th key/value/tag (depending on {@code type}) of the + * {@link Selector.GeneralSelector}. + * + * @param matchingSelector matching selector + * @param index index + * @param type selector type ("key", "value" or "tag") + * @param p OSM primitive + * @return argument value, can be {@code null} + */ + static String determineArgument(Selector.GeneralSelector matchingSelector, int index, String type, OsmPrimitive p) { + try { + final Condition c = matchingSelector.getConditions().get(index); + final Tag tag = c instanceof Condition.ToTagConvertable + ? ((Condition.ToTagConvertable) c).asTag(p) + : null; + if (tag == null) { + return null; + } else if ("key".equals(type)) { + return tag.getKey(); + } else if ("value".equals(type)) { + return tag.getValue(); + } else if ("tag".equals(type)) { + return tag.toString(); + } + } catch (IndexOutOfBoundsException ignore) { + Logging.debug(ignore); + } + return null; + } + + /** + * Replaces occurrences of {i.key}, {i.value}, {i.tag} in {@code s} by the corresponding + * key/value/tag of the {@code index}-th {@link Condition} of {@code matchingSelector}. + * + * @param matchingSelector matching selector + * @param s any string + * @param p OSM primitive + * @return string with arguments inserted + */ + static String insertArguments(Selector matchingSelector, String s, OsmPrimitive p) { + if (s != null && matchingSelector instanceof Selector.ChildOrParentSelector) { + return insertArguments(((Selector.ChildOrParentSelector) matchingSelector).right, s, p); + } else if (s == null || !(matchingSelector instanceof Selector.GeneralSelector)) { + return s; + } + final Matcher m = Pattern.compile("\\{(\\d+)\\.(key|value|tag)\\}").matcher(s); + final StringBuffer sb = new StringBuffer(); + while (m.find()) { + final String argument = determineArgument((Selector.GeneralSelector) matchingSelector, + Integer.parseInt(m.group(1)), m.group(2), p); + try { + // Perform replacement with null-safe + regex-safe handling + m.appendReplacement(sb, String.valueOf(argument).replace("^(", "").replace(")$", "")); + } catch (IndexOutOfBoundsException | IllegalArgumentException e) { + Logging.log(Logging.LEVEL_ERROR, tr("Unable to replace argument {0} in {1}: {2}", argument, sb, e.getMessage()), e); + } + } + m.appendTail(sb); + return sb.toString(); + } + + /** + * Constructs a fix in terms of a {@link org.openstreetmap.josm.command.Command} for the {@link OsmPrimitive} + * if the error is fixable, or {@code null} otherwise. + * + * @param p the primitive to construct the fix for + * @return the fix or {@code null} + */ + Command fixPrimitive(OsmPrimitive p) { + if (p.getDataSet() == null || (fixCommands.isEmpty() && !deletion)) { + return null; + } + try { + final Selector matchingSelector = whichSelectorMatchesPrimitive(p); + Collection cmds = fixCommands.stream() + .map(fixCommand -> fixCommand.createCommand(p, matchingSelector)) + .filter(Objects::nonNull) + .collect(Collectors.toList()); + if (deletion && !p.isDeleted()) { + cmds.add(new DeleteCommand(p)); + } + return cmds.isEmpty() ? null + : new SequenceCommand(tr("Fix of {0}", getDescriptionForMatchingSelector(p, matchingSelector)), cmds); + } catch (IllegalArgumentException e) { + Logging.error(e); + return null; + } + } + + /** + * Constructs a (localized) message for this deprecation check. + * + * @param p OSM primitive + * @return a message + */ + String getMessage(OsmPrimitive p) { + if (errors.isEmpty()) { + // Return something to avoid NPEs + return rule.declaration.toString(); + } else { + final Object val = errors.keySet().iterator().next().val; + return String.valueOf( + val instanceof Expression + ? ((Expression) val).evaluate(new Environment(p)) + : val + ); + } + } + + /** + * Constructs a (localized) description for this deprecation check. + * + * @param p OSM primitive + * @return a description (possibly with alternative suggestions) + * @see #getDescriptionForMatchingSelector + */ + String getDescription(OsmPrimitive p) { + if (alternatives.isEmpty()) { + return getMessage(p); + } else { + /* I18N: {0} is the test error message and {1} is an alternative */ + return tr("{0}, use {1} instead", getMessage(p), String.join(tr(" or "), alternatives)); + } + } + + /** + * Constructs a (localized) description for this deprecation check + * where any placeholders are replaced by values of the matched selector. + * + * @param matchingSelector matching selector + * @param p OSM primitive + * @return a description (possibly with alternative suggestions) + */ + String getDescriptionForMatchingSelector(OsmPrimitive p, Selector matchingSelector) { + return insertArguments(matchingSelector, getDescription(p), p); + } + + Severity getSeverity() { + return errors.isEmpty() ? null : errors.values().iterator().next(); + } + + @Override + public String toString() { + return getDescription(null); + } + + /** + * Constructs a {@link TestError} for the given primitive, or returns null if the primitive does not give rise to an error. + * + * @param p the primitive to construct the error for + * @param matchingSelector the matching selector (e.g., obtained via {@link #whichSelectorMatchesPrimitive}) + * @param env the environment + * @param tester the tester + * @return an instance of {@link TestError}, or returns null if the primitive does not give rise to an error. + */ + protected List getErrorsForPrimitive(OsmPrimitive p, Selector matchingSelector, Environment env, Test tester) { + List res = new ArrayList<>(); + if (matchingSelector != null && !errors.isEmpty()) { + final Command fix = fixPrimitive(p); + final String description = getDescriptionForMatchingSelector(p, matchingSelector); + final String description1 = group == null ? description : group; + final String description2 = group == null ? null : description; + final String selector = matchingSelector.toString(); + TestError.Builder errorBuilder = TestError.builder(tester, getSeverity(), 3000) + .messageWithManuallyTranslatedDescription(description1, description2, selector); + if (fix != null) { + errorBuilder.fix(() -> fix); + } + if (env.child instanceof OsmPrimitive) { + res.add(errorBuilder.primitives(p, (OsmPrimitive) env.child).build()); + } else if (env.children != null) { + for (IPrimitive c : env.children) { + if (c instanceof OsmPrimitive) { + errorBuilder = TestError.builder(tester, getSeverity(), 3000) + .messageWithManuallyTranslatedDescription(description1, description2, selector); + if (fix != null) { + errorBuilder.fix(() -> fix); + } + // check if we have special information about highlighted objects */ + boolean hiliteFound = false; + if (env.intersections != null) { + Area is = env.intersections.get(c); + if (is != null) { + errorBuilder.highlight(is); + hiliteFound = true; + } + } + if (env.crossingWaysMap != null && !hiliteFound) { + Map, List> is = env.crossingWaysMap.get(c); + if (is != null) { + Set toHilite = new HashSet<>(); + for (List wsList : is.values()) { + toHilite.addAll(wsList); + } + errorBuilder.highlightWaySegments(toHilite); + } + } + res.add(errorBuilder.primitives(p, (OsmPrimitive) c).build()); + } + } + } else { + res.add(errorBuilder.primitives(p).build()); + } + } + return res; + } + +}