Hi Mike
Hi, I seem to have found a bug in the style file reader - it does not appear
to work correctly if the first term is a not equals term. For example, with
the following two rules only the latter one correctly triggers:
highway!=bus_stop & name~'(?i)ferry terminal' {echotags 'Trigger 1'}
name~'(?i)ferry terminal' & highway!=bus_stop {echotags 'Trigger 2'}
Yes that is a bug, thanks for reporting it.
I think it is failing to match when there is no highway tag at all.
If you start with highway!=* as your first term in a rule, the style reader
throws it out as not valid with "Cannot start rule with tag!=*" and I think
this may be related.
The rules were more restrictive in the past, and when an error
message doesn't make sense it is usually down to a bug, since these
things should not happen any more. Although the code does require
that an equals or exists terms is first, it is also supposed to
re-arrange the expression so that is the case.
So your first expression should have been re-written to
name=* && name~'(?i)ferry terminal' & highway!=bus_stop
There is a second bug, in that having failed to re-arrange it
properly it should have then failed with an error. As it turns
out there was a test for this, but it was saying that it
should be allowed instead of being a failure.
The only intended restriction now is that you cannot write a rule that
would match an element without any tags at all.
The attached patch should fix both these issues.
..Steve
Index: test/uk/me/parabola/mkgmap/osmstyle/RuleFileReaderTest.java
IDEA additional info:
Subsystem: com.intellij.openapi.diff.impl.patch.CharsetEP
<+>UTF-8
===================================================================
--- test/uk/me/parabola/mkgmap/osmstyle/RuleFileReaderTest.java (revision 3976)
+++ test/uk/me/parabola/mkgmap/osmstyle/RuleFileReaderTest.java (revision )
@@ -357,15 +357,12 @@
assertEquals(2, type.getType());
}
- @Test
+ @Test(expected = SyntaxException.class)
public void testNEAtTop() {
- RuleSet rs = makeRuleSet("QUOTA != 'fred' [0x2]");
- Element el = new Way(1);
- el.addTag("QUOTA", "tom");
-
- GType type = getFirstType(rs, el);
- assertNotNull(type);
- assertEquals(2, type.getType());
+ // This is not allowed, in spite of the commit comment that introduced
+ // this test. It should also match an element with no tags and that
+ // is not allowed.
+ makeRuleSet("QUOTA != 'fred' [0x2]");
}
@Test
Index: src/uk/me/parabola/mkgmap/osmstyle/RuleFileReader.java
IDEA additional info:
Subsystem: com.intellij.openapi.diff.impl.patch.CharsetEP
<+>UTF-8
===================================================================
--- src/uk/me/parabola/mkgmap/osmstyle/RuleFileReader.java (revision 3976)
+++ src/uk/me/parabola/mkgmap/osmstyle/RuleFileReader.java (revision )
@@ -600,8 +600,10 @@
* EQUALS to the front followed by EXISTS. Without knowing tag
* frequency you can only guess at what the most selective operations
* are, so all we do is arrange EQUALS - EXISTS - everything else.
+ *
* Note that you must have an EQUALS or EXISTS first, so you can't
- * bring anything else earlier than them.
+ * bring anything else earlier than them. You cannot have NOT_EQUALS
+ * or NOT_EXISTS first, so they have to be moved after everything else.
*
* @return An integer, lower values mean the operation should be earlier
* in the expression than operations with higher values.
@@ -619,8 +621,14 @@
case OR:
return Math.max(selectivity(op.getFirst()), selectivity(op.getSecond()));
+ case NOT_EQUALS:
+ case NOT_EXISTS:
+ // Neither of these can be first, this will ensure that they never are
+ // when there is more than one term
+ return 200;
+
default:
- return 1000;
+ return 100;
}
}
@@ -629,7 +637,7 @@
// The lookup key for the exists operation is 'tag=*'
createAndSaveRule(op.getFirst().getKeyValue() + "=*", op, actions, gt);
} else {
- throw new SyntaxException(scanner, "Cannot start expression with: " + op);
+ throw new SyntaxException(scanner, "Expression cannot contain only not-equals and not-exists terms");
}
}
@@ -695,6 +703,9 @@
}
private AndOp combineWithExists(Op first, BinaryOp op) {
+ if (op.isType(NOT_EXISTS) || op.isType(NOT_EQUALS))
+ throw new SyntaxException("Expression cannot contain only not-equals and not-exists terms");
+
Op existsOp = new ExistsOp();
existsOp.setFirst(first);
Index: test/uk/me/parabola/mkgmap/osmstyle/RuleSetTest.java
IDEA additional info:
Subsystem: com.intellij.openapi.diff.impl.patch.CharsetEP
<+>UTF-8
===================================================================
--- test/uk/me/parabola/mkgmap/osmstyle/RuleSetTest.java (revision 3976)
+++ test/uk/me/parabola/mkgmap/osmstyle/RuleSetTest.java (revision )
@@ -21,6 +21,7 @@
import uk.me.parabola.mkgmap.reader.osm.Rule;
import uk.me.parabola.mkgmap.reader.osm.TypeResult;
import uk.me.parabola.mkgmap.reader.osm.Way;
+import uk.me.parabola.mkgmap.scan.SyntaxException;
import org.junit.Test;
@@ -77,7 +78,6 @@
/**
* An action variable is set on a rule that starts with an exists clause.
* We then attempt to match on value that it is
- * @throws Exception
*/
@Test
public void testActionVarSetOnExistsRule1() throws Exception {
@@ -338,8 +338,49 @@
assertEquals("second element", 0x6, list.get(1).getType());
}
+ @Test
+ public void testNotEqualPlusRegex() {
+ // Test first term not-equal, second a regex
+ RuleSet rs = makeRuleSet("highway!=bus_stop & name~'(?i)ferry terminal' [0x6]");
+
+ Way el = new Way(1);
+ el.addTag("name", "Ferry terminal");
+
+ GType type = getFirstType(rs, el);
+ assertNotNull("Expect rule to match", type);
+ assertEquals("Expect rule to match", 6, type.getType());
+ }
+
+ @Test
+ public void testNotExistsPlusRegex() {
+ // Test first term not-equal, second a regex
+ RuleSet rs = makeRuleSet("highway!=* & name~'(?i)ferry terminal' [0x6]");
+
+ Way el = new Way(1);
+ el.addTag("name", "Ferry terminal");
+
+ GType type = getFirstType(rs, el);
+ assertNotNull("Expect rule to match", type);
+ assertEquals("Expect rule to match", 6, type.getType());
+ }
+
+ @Test(expected = SyntaxException.class)
+ public void testOnlyNotExists() {
+ makeRuleSet("highway != * [0x2]");
+ }
+
+ @Test(expected = SyntaxException.class)
+ public void testOnlyNotEqual() {
+ makeRuleSet("highway != 'primary' [0x2]");
+ }
+
+ @Test(expected = SyntaxException.class)
+ public void testOnlyNotAllowedTerms() {
+ makeRuleSet("highway != 'primary & a != bar & c !=*' [0x2]");
+ }
+
private List<GType> resolveList(RuleSet rs, Way el) {
- final List<GType> list = new ArrayList<GType>();
+ final List<GType> list = new ArrayList<>();
rs.resolveType(el, new TypeResult() {
public void add(Element el, GType type) {
list.add(type);
_______________________________________________
mkgmap-dev mailing list
[email protected]
http://www.mkgmap.org.uk/mailman/listinfo/mkgmap-dev
