Index: trunk/src/org/openstreetmap/josm/data/validation/tests/MapCSSTagChecker.java =================================================================== --- trunk/src/org/openstreetmap/josm/data/validation/tests/MapCSSTagChecker.java (revision 15980) +++ trunk/src/org/openstreetmap/josm/data/validation/tests/MapCSSTagChecker.java (revision 15981) @@ -22,4 +22,5 @@ import java.util.Optional; import java.util.Set; +import java.util.function.Consumer; import java.util.function.Predicate; import java.util.regex.Matcher; @@ -288,6 +289,4 @@ * Is evaluated on the matching primitive to give the error message. Map is checked to contain exactly one element. */ protected final Map errors = new HashMap<>(); - /** Unit tests */ - protected final Map assertions = new HashMap<>(); /** MapCSS Classes to set on matching primitives */ protected final Set setClassExpressions = new HashSet<>(); @@ -303,6 +302,7 @@ private static final String POSSIBLE_THROWS = "throwError/throwWarning/throwOther"; - static TagCheck ofMapCSSRule(final GroupedMapCSSRule rule) throws IllegalDataException { + static TagCheck ofMapCSSRule(final GroupedMapCSSRule 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) { @@ -346,7 +346,7 @@ check.alternatives.add(val); } else if (val != null && "assertMatch".equals(ai.key)) { - check.assertions.put(val, Boolean.TRUE); + assertions.put(val, Boolean.TRUE); } else if (val != null && "assertNoMatch".equals(ai.key)) { - check.assertions.put(val, Boolean.FALSE); + assertions.put(val, Boolean.FALSE); } else if (val != null && "group".equals(ai.key)) { check.group = val; @@ -369,12 +369,15 @@ + rule.selectors); } + if (assertionConsumer != null) { + MapCSSTagCheckerAsserts.checkAsserts(check, assertions, assertionConsumer); + } return check; } static ParseResult readMapCSS(Reader css) throws ParseException { - return readMapCSS(css, ""); - } - - static ParseResult readMapCSS(Reader css, String url) throws ParseException { + return readMapCSS(css, "", null); + } + + static ParseResult readMapCSS(Reader css, String url, AssertionConsumer assertionConsumer) throws ParseException { CheckParameterUtil.ensureParameterNotNull(css, "css"); @@ -401,5 +404,5 @@ try { parseChecks.add(TagCheck.ofMapCSSRule( - new GroupedMapCSSRule(map.getValue(), map.getKey(), url))); + new GroupedMapCSSRule(map.getValue(), map.getKey(), url), assertionConsumer)); } catch (IllegalDataException e) { Logging.error("Cannot add MapCss rule: "+e.getMessage()); @@ -781,4 +784,11 @@ /** + * A handler for assertion error messages (for not fulfilled "assertMatch", "assertNoMatch"). + */ + @FunctionalInterface + interface AssertionConsumer extends Consumer { + } + + /** * Adds a new MapCSS config file from the given URL. * @param url The unique URL of the MapCSS config file @@ -789,4 +799,10 @@ */ public synchronized ParseResult addMapCSS(String url) throws ParseException, IOException { + // Check assertions, useful for development of local files + final boolean checkAssertions = Config.getPref().getBoolean("validator.check_assert_local_rules", false) && Utils.isLocalUrl(url); + return addMapCSS(url, checkAssertions ? Logging::warn : null); + } + + synchronized ParseResult addMapCSS(String url, AssertionConsumer assertionConsumer) throws ParseException, IOException { CheckParameterUtil.ensureParameterNotNull(url, "url"); ParseResult result; @@ -797,14 +813,8 @@ if (zip != null) I18n.addTexts(cache.getFile()); - result = TagCheck.readMapCSS(reader, url); + result = TagCheck.readMapCSS(reader, url, assertionConsumer); checks.remove(url); checks.putAll(url, result.parseChecks); indexData = null; - // Check assertions, useful for development of local files - if (Config.getPref().getBoolean("validator.check_assert_local_rules", false) && Utils.isLocalUrl(url)) { - for (String msg : MapCSSTagCheckerAsserts.checkAsserts(result.parseChecks)) { - Logging.warn(msg); - } - } } return result; @@ -838,4 +848,5 @@ } } + MapCSSTagCheckerAsserts.clear(); } Index: trunk/src/org/openstreetmap/josm/data/validation/tests/MapCSSTagCheckerAsserts.java =================================================================== --- trunk/src/org/openstreetmap/josm/data/validation/tests/MapCSSTagCheckerAsserts.java (revision 15980) +++ trunk/src/org/openstreetmap/josm/data/validation/tests/MapCSSTagCheckerAsserts.java (revision 15981) @@ -8,5 +8,4 @@ import java.util.Collections; import java.util.HashSet; -import java.util.LinkedHashSet; import java.util.List; import java.util.Map; @@ -36,55 +35,60 @@ /** - * Utility class for checking rule {@linkplain MapCSSTagChecker.TagCheck#assertions assertions} of {@link MapCSSTagChecker.TagCheck}. + * Utility class for checking rule assertions of {@link MapCSSTagChecker.TagCheck}. */ -class MapCSSTagCheckerAsserts { +final class MapCSSTagCheckerAsserts { + private MapCSSTagCheckerAsserts() { // private constructor } + private static final ArrayList previousChecks = new ArrayList<>(); + /** * Checks that rule assertions are met for the given set of TagChecks. - * @param schecks The TagChecks for which assertions have to be checked - * @return A set of error messages, empty if all assertions are met - * @since 7356 + * @param check The TagCheck for which assertions have to be checked + * @param assertionConsumer The handler for assertion error messages */ - public static Set checkAsserts(final Collection schecks) { - Set assertionErrors = new LinkedHashSet<>(); + static void checkAsserts(final MapCSSTagChecker.TagCheck check, final Map assertions, + final MapCSSTagChecker.AssertionConsumer assertionConsumer) { final Method insideMethod = getFunctionMethod("inside"); final DataSet ds = new DataSet(); - for (final MapCSSTagChecker.TagCheck check : schecks) { - Logging.debug("Check: {0}", check); - for (final Map.Entry i : check.assertions.entrySet()) { - Logging.debug("- Assertion: {0}", i); - 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, schecks); - if (!checkDependencies.isEmpty()) { - checksToRun.add(checkDependencies); + Logging.debug("Check: {0}", check); + for (final Map.Entry i : assertions.entrySet()) { + Logging.debug("- Assertion: {0}", i); + 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); + if (!checkDependencies.isEmpty()) { + checksToRun.add(checkDependencies); + } + checksToRun.add(Collections.singleton(check)); + // Add primitive to dataset to avoid DataIntegrityProblemException when evaluating selectors + addPrimitive(ds, p); + final Collection pErrors = MapCSSTagChecker.getErrorsForPrimitive(p, true, checksToRun); + Logging.debug("- Errors: {0}", pErrors); + final boolean isError = pErrors.stream().anyMatch(e -> e.getTester() instanceof MapCSSTagChecker.MapCSSTagCheckerAndRule + && ((MapCSSTagChecker.MapCSSTagCheckerAndRule) e.getTester()).rule.equals(check.rule)); + if (isError != i.getValue()) { + assertionConsumer.accept(MessageFormat.format("Expecting test ''{0}'' (i.e., {1}) to {2} {3} (i.e., {4})", + check.getMessage(p), check.rule.selectors, i.getValue() ? "match" : "not match", i.getKey(), p.getKeys())); + } + if (isError) { + // Check that autofix works as expected + Command fix = check.fixPrimitive(p); + if (fix != null && fix.executeCommand() && !MapCSSTagChecker.getErrorsForPrimitive(p, true, checksToRun).isEmpty()) { + assertionConsumer.accept(MessageFormat.format("Autofix does not work for test ''{0}'' (i.e., {1})", + check.getMessage(p), check.rule.selectors)); } - checksToRun.add(Collections.singleton(check)); - // Add primitive to dataset to avoid DataIntegrityProblemException when evaluating selectors - addPrimitive(ds, p); - final Collection pErrors = MapCSSTagChecker.getErrorsForPrimitive(p, true, checksToRun); - Logging.debug("- Errors: {0}", pErrors); - final boolean isError = pErrors.stream().anyMatch(e -> e.getTester() instanceof MapCSSTagChecker.MapCSSTagCheckerAndRule - && ((MapCSSTagChecker.MapCSSTagCheckerAndRule) e.getTester()).rule.equals(check.rule)); - if (isError != i.getValue()) { - assertionErrors.add(MessageFormat.format("Expecting test ''{0}'' (i.e., {1}) to {2} {3} (i.e., {4})", - check.getMessage(p), check.rule.selectors, i.getValue() ? "match" : "not match", i.getKey(), p.getKeys())); - } - if (isError) { - // Check that autofix works as expected - Command fix = check.fixPrimitive(p); - if (fix != null && fix.executeCommand() && !MapCSSTagChecker.getErrorsForPrimitive(p, true, checksToRun).isEmpty()) { - assertionErrors.add(MessageFormat.format("Autofix does not work for test ''{0}'' (i.e., {1})", - check.getMessage(p), check.rule.selectors)); - } - } - ds.removePrimitive(p); } + ds.removePrimitive(p); } - return assertionErrors; + previousChecks.add(check); + } + + public static void clear() { + previousChecks.clear(); + previousChecks.trimToSize(); }