Index: trunk/src/org/openstreetmap/josm/data/validation/tests/MapCSSTagChecker.java =================================================================== --- trunk/src/org/openstreetmap/josm/data/validation/tests/MapCSSTagChecker.java (revision 15978) +++ trunk/src/org/openstreetmap/josm/data/validation/tests/MapCSSTagChecker.java (revision 15979) @@ -10,14 +10,10 @@ import java.io.Reader; import java.io.StringReader; -import java.lang.reflect.Method; -import java.text.MessageFormat; import java.util.ArrayList; import java.util.Collection; -import java.util.Collections; import java.util.HashMap; import java.util.HashSet; import java.util.Iterator; import java.util.LinkedHashMap; -import java.util.LinkedHashSet; import java.util.LinkedList; import java.util.List; @@ -36,10 +32,6 @@ import org.openstreetmap.josm.command.DeleteCommand; import org.openstreetmap.josm.command.SequenceCommand; -import org.openstreetmap.josm.data.coor.LatLon; -import org.openstreetmap.josm.data.osm.DataSet; import org.openstreetmap.josm.data.osm.IPrimitive; import org.openstreetmap.josm.data.osm.OsmPrimitive; -import org.openstreetmap.josm.data.osm.OsmUtils; -import org.openstreetmap.josm.data.osm.Relation; import org.openstreetmap.josm.data.osm.Tag; import org.openstreetmap.josm.data.osm.Way; @@ -55,11 +47,6 @@ import org.openstreetmap.josm.gui.mappaint.MultiCascade; import org.openstreetmap.josm.gui.mappaint.mapcss.Condition; -import org.openstreetmap.josm.gui.mappaint.mapcss.ConditionFactory.ClassCondition; -import org.openstreetmap.josm.gui.mappaint.mapcss.ConditionFactory.ExpressionCondition; import org.openstreetmap.josm.gui.mappaint.mapcss.Expression; -import org.openstreetmap.josm.gui.mappaint.mapcss.ExpressionFactory.ParameterFunction; -import org.openstreetmap.josm.gui.mappaint.mapcss.Functions; import org.openstreetmap.josm.gui.mappaint.mapcss.Instruction; -import org.openstreetmap.josm.gui.mappaint.mapcss.LiteralExpression; import org.openstreetmap.josm.gui.mappaint.mapcss.MapCSSRule; import org.openstreetmap.josm.gui.mappaint.mapcss.MapCSSRule.Declaration; @@ -67,6 +54,4 @@ import org.openstreetmap.josm.gui.mappaint.mapcss.MapCSSStyleSource.MapCSSRuleIndex; import org.openstreetmap.josm.gui.mappaint.mapcss.Selector; -import org.openstreetmap.josm.gui.mappaint.mapcss.Selector.AbstractSelector; -import org.openstreetmap.josm.gui.mappaint.mapcss.Selector.GeneralSelector; import org.openstreetmap.josm.gui.mappaint.mapcss.Selector.OptimizedGeneralSelector; import org.openstreetmap.josm.gui.mappaint.mapcss.parsergen.MapCSSParser; @@ -80,11 +65,7 @@ import org.openstreetmap.josm.spi.preferences.Config; import org.openstreetmap.josm.tools.CheckParameterUtil; -import org.openstreetmap.josm.tools.DefaultGeoProperty; -import org.openstreetmap.josm.tools.GeoProperty; -import org.openstreetmap.josm.tools.GeoPropertyIndex; import org.openstreetmap.josm.tools.I18n; import org.openstreetmap.josm.tools.Logging; import org.openstreetmap.josm.tools.MultiMap; -import org.openstreetmap.josm.tools.Territories; import org.openstreetmap.josm.tools.Utils; @@ -96,5 +77,5 @@ private MapCSSTagCheckerIndex indexData; private final Set tested = new HashSet<>(); - final Map mpAreaCache = new HashMap<>(); + private static final Map mpAreaCache = new HashMap<>(); /** @@ -668,47 +649,4 @@ } - /** - * Returns the set of tagchecks on which this check depends on. - * @param schecks the collection of tagcheks to search in - * @return the set of tagchecks on which this check depends on - * @since 7881 - */ - public Set getTagCheckDependencies(Collection schecks) { - Set result = new HashSet<>(); - Set classes = getClassesIds(); - if (schecks != null && !classes.isEmpty()) { - for (TagCheck tc : schecks) { - if (this.equals(tc)) { - continue; - } - for (String id : tc.setClassExpressions) { - if (classes.contains(id)) { - result.add(tc); - break; - } - } - } - } - return result; - } - - /** - * Returns the list of ids of all MapCSS classes referenced in the rule selectors. - * @return the list of ids of all MapCSS classes referenced in the rule selectors - * @since 7881 - */ - public Set getClassesIds() { - Set result = new HashSet<>(); - for (Selector s : rule.selectors) { - if (s instanceof AbstractSelector) { - for (Condition c : ((AbstractSelector) s).getConditions()) { - if (c instanceof ClassCondition) { - result.add(((ClassCondition) c).id); - } - } - } - } - return result; - } } @@ -814,5 +752,5 @@ } - private Collection getErrorsForPrimitive(OsmPrimitive p, boolean includeOtherSeverity, + static Collection getErrorsForPrimitive(OsmPrimitive p, boolean includeOtherSeverity, Collection> checksCol) { // this variant is only used by the assertion tests @@ -877,5 +815,5 @@ // Check assertions, useful for development of local files if (Config.getPref().getBoolean("validator.check_assert_local_rules", false) && Utils.isLocalUrl(url)) { - for (String msg : checkAsserts(result.parseChecks)) { + for (String msg : MapCSSTagCheckerAsserts.checkAsserts(result.parseChecks)) { Logging.warn(msg); } @@ -912,99 +850,4 @@ } } - } - - private static Method getFunctionMethod(String method) { - try { - return Functions.class.getDeclaredMethod(method, Environment.class, String.class); - } catch (NoSuchMethodException | SecurityException e) { - Logging.error(e); - return null; - } - } - - private static Optional getFirstInsideCountry(TagCheck check, Method insideMethod) { - return check.rule.selectors.stream() - .filter(s -> s instanceof GeneralSelector) - .flatMap(s -> ((GeneralSelector) s).getConditions().stream()) - .filter(c -> c instanceof ExpressionCondition) - .map(c -> ((ExpressionCondition) c).getExpression()) - .filter(c -> c instanceof ParameterFunction) - .map(c -> (ParameterFunction) c) - .filter(c -> c.getMethod().equals(insideMethod)) - .flatMap(c -> c.getArgs().stream()) - .filter(e -> e instanceof LiteralExpression) - .map(e -> ((LiteralExpression) e).getLiteral()) - .filter(l -> l instanceof String) - .map(l -> ((String) l).split(",")[0]) - .findFirst(); - } - - private static LatLon getLocation(TagCheck check, Method insideMethod) { - Optional inside = getFirstInsideCountry(check, insideMethod); - if (inside.isPresent()) { - GeoPropertyIndex index = Territories.getGeoPropertyIndex(inside.get()); - if (index != null) { - GeoProperty prop = index.getGeoProperty(); - if (prop instanceof DefaultGeoProperty) { - return ((DefaultGeoProperty) prop).getRandomLatLon(); - } - } - } - return LatLon.ZERO; - } - - /** - * 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 - */ - public Set checkAsserts(final Collection schecks) { - Set assertionErrors = new LinkedHashSet<>(); - final Method insideMethod = getFunctionMethod("inside"); - final DataSet ds = new DataSet(); - for (final 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 = check.getTagCheckDependencies(schecks); - 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 = getErrorsForPrimitive(p, true, checksToRun); - Logging.debug("- Errors: {0}", pErrors); - final boolean isError = pErrors.stream().anyMatch(e -> e.getTester() instanceof MapCSSTagCheckerAndRule - && ((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() && !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); - } - } - return assertionErrors; - } - - private static void addPrimitive(DataSet ds, OsmPrimitive p) { - if (p instanceof Way) { - ((Way) p).getNodes().forEach(n -> addPrimitive(ds, n)); - } else if (p instanceof Relation) { - ((Relation) p).getMembers().forEach(m -> addPrimitive(ds, m.getMember())); - } - ds.addPrimitive(p); } Index: trunk/src/org/openstreetmap/josm/data/validation/tests/MapCSSTagCheckerAsserts.java =================================================================== --- trunk/src/org/openstreetmap/josm/data/validation/tests/MapCSSTagCheckerAsserts.java (revision 15979) +++ trunk/src/org/openstreetmap/josm/data/validation/tests/MapCSSTagCheckerAsserts.java (revision 15979) @@ -0,0 +1,164 @@ +// License: GPL. For details, see LICENSE file. +package org.openstreetmap.josm.data.validation.tests; + +import java.lang.reflect.Method; +import java.text.MessageFormat; +import java.util.ArrayList; +import java.util.Collection; +import java.util.Collections; +import java.util.HashSet; +import java.util.LinkedHashSet; +import java.util.List; +import java.util.Map; +import java.util.Optional; +import java.util.Set; +import java.util.stream.Collectors; + +import org.openstreetmap.josm.command.Command; +import org.openstreetmap.josm.data.coor.LatLon; +import org.openstreetmap.josm.data.osm.DataSet; +import org.openstreetmap.josm.data.osm.OsmPrimitive; +import org.openstreetmap.josm.data.osm.OsmUtils; +import org.openstreetmap.josm.data.osm.Relation; +import org.openstreetmap.josm.data.osm.Way; +import org.openstreetmap.josm.data.validation.TestError; +import org.openstreetmap.josm.gui.mappaint.Environment; +import org.openstreetmap.josm.gui.mappaint.mapcss.ConditionFactory; +import org.openstreetmap.josm.gui.mappaint.mapcss.ExpressionFactory; +import org.openstreetmap.josm.gui.mappaint.mapcss.Functions; +import org.openstreetmap.josm.gui.mappaint.mapcss.LiteralExpression; +import org.openstreetmap.josm.gui.mappaint.mapcss.Selector; +import org.openstreetmap.josm.tools.DefaultGeoProperty; +import org.openstreetmap.josm.tools.GeoProperty; +import org.openstreetmap.josm.tools.GeoPropertyIndex; +import org.openstreetmap.josm.tools.Logging; +import org.openstreetmap.josm.tools.Territories; + +/** + * Utility class for checking rule {@linkplain MapCSSTagChecker.TagCheck#assertions assertions} of {@link MapCSSTagChecker.TagCheck}. + */ +class MapCSSTagCheckerAsserts { + private MapCSSTagCheckerAsserts() { + // private constructor + } + + /** + * 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 + */ + public static Set checkAsserts(final Collection schecks) { + Set assertionErrors = new LinkedHashSet<>(); + 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); + } + 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); + } + } + return assertionErrors; + } + + private static Method getFunctionMethod(String method) { + try { + return Functions.class.getDeclaredMethod(method, Environment.class, String.class); + } catch (NoSuchMethodException | SecurityException e) { + Logging.error(e); + return null; + } + } + + private static void addPrimitive(DataSet ds, OsmPrimitive p) { + if (p instanceof Way) { + ((Way) p).getNodes().forEach(n -> addPrimitive(ds, n)); + } else if (p instanceof Relation) { + ((Relation) p).getMembers().forEach(m -> addPrimitive(ds, m.getMember())); + } + ds.addPrimitive(p); + } + + private static LatLon getLocation(MapCSSTagChecker.TagCheck check, Method insideMethod) { + Optional inside = getFirstInsideCountry(check, insideMethod); + if (inside.isPresent()) { + GeoPropertyIndex index = Territories.getGeoPropertyIndex(inside.get()); + if (index != null) { + GeoProperty prop = index.getGeoProperty(); + if (prop instanceof DefaultGeoProperty) { + return ((DefaultGeoProperty) prop).getRandomLatLon(); + } + } + } + return LatLon.ZERO; + } + + private static Optional getFirstInsideCountry(MapCSSTagChecker.TagCheck check, Method insideMethod) { + return check.rule.selectors.stream() + .filter(s -> s instanceof Selector.GeneralSelector) + .flatMap(s -> ((Selector.GeneralSelector) s).getConditions().stream()) + .filter(c -> c instanceof ConditionFactory.ExpressionCondition) + .map(c -> ((ConditionFactory.ExpressionCondition) c).getExpression()) + .filter(c -> c instanceof ExpressionFactory.ParameterFunction) + .map(c -> (ExpressionFactory.ParameterFunction) c) + .filter(c -> c.getMethod().equals(insideMethod)) + .flatMap(c -> c.getArgs().stream()) + .filter(e -> e instanceof LiteralExpression) + .map(e -> ((LiteralExpression) e).getLiteral()) + .filter(l -> l instanceof String) + .map(l -> ((String) l).split(",")[0]) + .findFirst(); + } + + /** + * Returns the set of tagchecks on which this check depends on. + * @param check the tagcheck + * @param schecks the collection of tagcheks to search in + * @return the set of tagchecks on which this check depends on + * @since 7881 + */ + private static Set getTagCheckDependencies(MapCSSTagChecker.TagCheck check, + Collection schecks) { + Set result = new HashSet<>(); + Set classes = check.rule.selectors.stream() + .filter(s -> s instanceof Selector.AbstractSelector) + .flatMap(s -> ((Selector.AbstractSelector) s).getConditions().stream()) + .filter(c -> c instanceof ConditionFactory.ClassCondition) + .map(c -> ((ConditionFactory.ClassCondition) c).id) + .collect(Collectors.toSet()); + if (schecks != null && !classes.isEmpty()) { + return schecks.stream() + .filter(tc -> !check.equals(tc)) + .filter(tc -> tc.setClassExpressions.stream().anyMatch(classes::contains)) + .collect(Collectors.toSet()); + } + return result; + } +}