Changeset 18757 in josm for trunk/src/org/openstreetmap/josm/data

Timestamp:

2023-06-14T18:01:00+02:00 (2 years ago)

Author:

taylor.smock

Message:

Fix #17669, #22096: Allow placeholders in more locations in MapCSS

Location:

trunk/src/org/openstreetmap/josm/data/validation/tests

Files:

• MapCSSTagChecker.java (modified) (1 diff)
• MapCSSTagCheckerAsserts.java (modified) (1 diff)
• MapCSSTagCheckerFixCommand.java (modified) (1 diff)
• MapCSSTagCheckerRule.java (modified) (7 diffs)

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

@@ -232 +232 @@
                 final Selector selector = check.whichSelectorMatchesEnvironment(env);
                 if (selector != null) {
-                    check.rule.declaration.execute(env);
+                    final Environment envWithSelector = env.withSelector(selector);
+                    check.rule.declaration.execute(envWithSelector);
                     if (!ignoreError && !check.errors.isEmpty()) {
-                        r.addAll(check.getErrorsForPrimitive(p, selector, env, new MapCSSTagCheckerAndRule(check.rule)));
+                        r.addAll(check.getErrorsForPrimitive(p, selector, envWithSelector, new MapCSSTagCheckerAndRule(check.rule)));
                     }
                 }

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

@@ -75 +75 @@
                 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));
+                    assertionConsumer.accept(MessageFormat.format("Autofix does not work for test ''{0}'' (i.e., {1}). Failing test: {2}",
+                            check.getMessage(p), check.rule.selectors, i.getKey()));
                 }
             }

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

@@ -49 +49 @@
         final String s;
         if (obj instanceof Expression) {
-            s = (String) ((Expression) obj).evaluate(new Environment(p));
+            s = (String) ((Expression) obj).evaluate(new Environment(p).withSelector(matchingSelector));
         } else if (obj instanceof String) {
             s = (String) obj;

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

@@ -18 +18 @@
 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;
 
@@ -27 +25 @@
 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;
@@ -35 +32 @@
 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.Condition.TagCondition;
 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.PlaceholderExpression;
 import org.openstreetmap.josm.gui.mappaint.mapcss.Selector;
 import org.openstreetmap.josm.gui.mappaint.mapcss.parsergen.MapCSSParser;
@@ -215 +213 @@
 
     Selector whichSelectorMatchesPrimitive(OsmPrimitive primitive) {
-        return whichSelectorMatchesEnvironment(new Environment(primitive));
+        return whichSelectorMatchesEnvironment(new Environment(primitive, new MultiCascade(), Environment.DEFAULT_LAYER, null));
     }
 
@@ -226 +224 @@
 
     /**
-     * Determines the {@code index}-th key/value/tag (depending on {@code type}) of the
-     * {@link org.openstreetmap.josm.gui.mappaint.mapcss.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 TagCondition
-                    ? ((TagCondition) 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 <code>{i.key}</code>, <code>{i.value}</code>, <code>{i.tag}</code> in {@code s} by the corresponding
      * key/value/tag of the {@code index}-th {@link Condition} of {@code matchingSelector}.
@@ -266 +233 @@
      */
     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();
+        return PlaceholderExpression.insertArguments(matchingSelector, s, p);
     }
 
@@ -329 +278 @@
             return String.valueOf(
                     val instanceof Expression
-                            ? ((Expression) val).evaluate(new Environment(p))
+                            ? ((Expression) val).evaluate(new Environment(p).withSelector(p == null ? null : whichSelectorMatchesPrimitive(p)))
                             : val
             );