Changeset 14484 in josm for trunk

Timestamp:

2018-12-02T02:32:13+01:00 (5 years ago)

Author:

Don-vip

Message:

fix #17058 - Cannot add assertMatch/assertNoMatch declarations with inside() selector

Location:

trunk

Files:

• src/org/openstreetmap/josm/data/osm/OsmUtils.java (modified) (3 diffs)
• src/org/openstreetmap/josm/data/validation/tests/MapCSSTagChecker.java (modified) (8 diffs)
• src/org/openstreetmap/josm/gui/mappaint/mapcss/ConditionFactory.java (modified) (1 diff)
• src/org/openstreetmap/josm/gui/mappaint/mapcss/ExpressionFactory.java (modified) (1 diff)
• src/org/openstreetmap/josm/gui/mappaint/mapcss/LiteralExpression.java (modified) (1 diff)
• src/org/openstreetmap/josm/tools/DefaultGeoProperty.java (modified) (1 diff)
• src/org/openstreetmap/josm/tools/GeoPropertyIndex.java (modified) (1 diff)
• src/org/openstreetmap/josm/tools/Territories.java (modified) (1 diff)
• test/unit/org/openstreetmap/josm/data/validation/tests/MapCSSTagCheckerTest.java (modified) (2 diffs)

trunk/src/org/openstreetmap/josm/data/osm/OsmUtils.java

@@ -113 +113 @@
 
     /**
-     * Creates a new OSM primitive according to the given assertion. Originally written for unit tests,
+     * Creates a new OSM primitive around (0,0) according to the given assertion. Originally written for unit tests,
      * this can also be used in another places like validation of local MapCSS validator rules.
      * @param assertion The assertion describing OSM primitive (ex: "way name=Foo railway=rail")
@@ -121 +121 @@
      */
     public static OsmPrimitive createPrimitive(String assertion) {
+        return createPrimitive(assertion, LatLon.ZERO);
+    }
+
+    /**
+     * Creates a new OSM primitive according to the given assertion. Originally written for unit tests,
+     * this can also be used in another places like validation of local MapCSS validator rules.
+     * @param assertion The assertion describing OSM primitive (ex: "way name=Foo railway=rail")
+     * @param around the coordinate at which the primitive will be located
+     * @return a new OSM primitive according to the given assertion
+     * @throws IllegalArgumentException if assertion is null or if the primitive type cannot be deduced from it
+     * @since 14484
+     */
+    public static OsmPrimitive createPrimitive(String assertion, LatLon around) {
         CheckParameterUtil.ensureParameterNotNull(assertion, "assertion");
         final String[] x = assertion.split("\\s+", 2);
         final OsmPrimitive p = "n".equals(x[0]) || "node".equals(x[0])
-                ? new Node(LatLon.ZERO)
+                ? new Node(around)
                 : "w".equals(x[0]) || "way".equals(x[0]) || /*for MapCSS related usage*/ "area".equals(x[0])
-                ? new Way()
+                ? newWay(around)
                 : "r".equals(x[0]) || "relation".equals(x[0])
-                ? new Relation()
+                ? newRelation(around)
                 : null;
         if (p == null) {
@@ -139 +152 @@
         }
         return p;
+    }
+
+    private static Node newNode(LatLon around) {
+        return new Node(around);
+    }
+
+    private static Way newWay(LatLon around) {
+        Way w = new Way();
+        w.addNode(newNode(new LatLon(around.lat()+0.1, around.lon())));
+        w.addNode(newNode(new LatLon(around.lat()-0.1, around.lon())));
+        return w;
+    }
+
+    private static Relation newRelation(LatLon around) {
+        Relation r = new Relation();
+        r.addMember(new RelationMember(null, newNode(around)));
+        return r;
     }
 

trunk/src/org/openstreetmap/josm/data/validation/tests/MapCSSTagChecker.java

@@ -4 +4 @@
 import static org.openstreetmap.josm.tools.I18n.tr;
 
+import java.awt.Rectangle;
 import java.io.BufferedReader;
 import java.io.IOException;
@@ -9 +10 @@
 import java.io.Reader;
 import java.io.StringReader;
+import java.lang.reflect.Method;
 import java.text.MessageFormat;
 import java.util.ArrayList;
@@ -35 +37 @@
 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.INode;
@@ -53 +56 @@
 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.Functions;
+import org.openstreetmap.josm.gui.mappaint.mapcss.ExpressionFactory.ParameterFunction;
 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;
@@ -73 +80 @@
 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.JosmRuntimeException;
 import org.openstreetmap.josm.tools.Logging;
 import org.openstreetmap.josm.tools.MultiMap;
+import org.openstreetmap.josm.tools.Territories;
 import org.openstreetmap.josm.tools.Utils;
 
@@ -958 +969 @@
     }
 
+    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<String> 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)
+                .findFirst();
+    }
+
+    private static LatLon getLocation(TagCheck check, Method insideMethod) {
+        Optional<String> inside = getFirstInsideCountry(check, insideMethod);
+        if (inside.isPresent()) {
+            GeoPropertyIndex<Boolean> index = Territories.getGeoPropertyIndex(inside.get());
+            if (index != null) {
+                GeoProperty<Boolean> prop = index.getGeoProperty();
+                if (prop instanceof DefaultGeoProperty) {
+                    Rectangle bounds = ((DefaultGeoProperty) prop).getArea().getBounds();
+                    return new LatLon(bounds.getCenterY(), bounds.getCenterX());
+                }
+            }
+        }
+        return LatLon.ZERO;
+    }
+
     /**
      * Checks that rule assertions are met for the given set of TagChecks.
@@ -966 +1018 @@
     public Set<String> checkAsserts(final Collection<TagCheck> schecks) {
         Set<String> assertionErrors = new LinkedHashSet<>();
+        final Method insideMethod = getFunctionMethod("inside");
         final DataSet ds = new DataSet();
         for (final TagCheck check : schecks) {
@@ -971 +1024 @@
             for (final Map.Entry<String, Boolean> i : check.assertions.entrySet()) {
                 Logging.debug("- Assertion: {0}", i);
-                final OsmPrimitive p = OsmUtils.createPrimitive(i.getKey());
+                final OsmPrimitive p = OsmUtils.createPrimitive(i.getKey(), getLocation(check, insideMethod));
                 // Build minimal ordered list of checks to run to test the assertion
                 List<Set<TagCheck>> checksToRun = new ArrayList<>();

trunk/src/org/openstreetmap/josm/gui/mappaint/mapcss/ConditionFactory.java

@@ -872 +872 @@
         }
 
+        /**
+         * Returns the expression.
+         * @return the expression
+         * @since 14484
+         */
+        public final Expression getExpression() {
+            return e;
+        }
+
         @Override
         public boolean applies(Environment env) {

trunk/src/org/openstreetmap/josm/gui/mappaint/mapcss/ExpressionFactory.java

@@ -1306 +1306 @@
         }
 
+        /**
+         * Returns the method.
+         * @return the method
+         * @since 14484
+         */
+        public final Method getMethod() {
+            return m;
+        }
+
+        /**
+         * Returns the arguments.
+         * @return the arguments
+         * @since 14484
+         */
+        public final List<Expression> getArgs() {
+            return args;
+        }
+
         @Override
         public Object evaluate(Environment env) {

trunk/src/org/openstreetmap/josm/gui/mappaint/mapcss/LiteralExpression.java

@@ -23 +23 @@
     }
 
+    /**
+     * Returns the literal.
+     * @return the literal
+     * @since 14484
+     */
+    public final Object getLiteral() {
+        return literal;
+    }
+
     @Override
     public Object evaluate(Environment env) {

trunk/src/org/openstreetmap/josm/tools/DefaultGeoProperty.java

@@ -60 +60 @@
     }
 
+    /**
+     * Returns the area.
+     * @return the area
+     * @since 14484
+     */
+    public final Area getArea() {
+        return area;
+    }
 }

trunk/src/org/openstreetmap/josm/tools/GeoPropertyIndex.java

@@ -48 +48 @@
     public T get(LatLon ll) {
         return lastLevelUsed.get(ll);
+    }
+
+    /**
+     * Returns the geo property.
+     * @return the geo property
+     * @since 14484
+     */
+    public final GeoProperty<T> getGeoProperty() {
+        return geoProp;
     }
 

trunk/src/org/openstreetmap/josm/tools/Territories.java

@@ -51 +51 @@
 
     /**
-     * Determine, if a point is inside a territory with the given the ISO3166-1
+     * Returns the {@link GeoPropertyIndex} for the given ISO3166-1 or ISO3166-2 code.
+     * @param code the ISO3166-1 or ISO3166-2 code
+     * @return the {@link GeoPropertyIndex} for the given {@code code}
+     * @since 14484
+     */
+    public static GeoPropertyIndex<Boolean> getGeoPropertyIndex(String code) {
+        return iso3166Cache.get(code);
+    }
+
+    /**
+     * Determine, if a point is inside a territory with the given ISO3166-1
      * or ISO3166-2 code.
      *

trunk/test/unit/org/openstreetmap/josm/data/validation/tests/MapCSSTagCheckerTest.java

@@ -52 +52 @@
     @Rule
     @SuppressFBWarnings(value = "URF_UNREAD_PUBLIC_OR_PROTECTED_FIELD")
-    public JOSMTestRules test = new JOSMTestRules().projection();
+    public JOSMTestRules test = new JOSMTestRules().projection().territories();
 
     static MapCSSTagChecker buildTagChecker(String css) throws ParseException {
@@ -191 +191 @@
 
     /**
+     * Checks that assertions work for country-specific checks.
+     * @throws ParseException if a parsing error occurs
+     */
+    @Test
+    public void testAssertInsideCountry() throws ParseException {
+        final MapCSSTagChecker test = buildTagChecker(
+                "node[amenity=parking][inside(\"BR\")] {\n" +
+                "  throwWarning: \"foo\";\n" +
+                "  assertMatch: \"node amenity=parking\";\n" +
+                "  assertNoMatch: \"node amenity=restaurant\";\n" +
+                "}");
+        Set<String> errors = test.checkAsserts(test.checks.get("test"));
+        assertTrue(errors.toString(), errors.isEmpty());
+    }
+
+    /**
      * Non-regression test for <a href="https://josm.openstreetmap.de/ticket/13762">Bug #13762</a>.
      * @throws ParseException if a parsing error occurs