Allow plugins with hyphens in their names to define query operators
Currently plugins are allowed to have hyphens in their names, but
they cannot define search operators as Gerrit does not allow hyphens
in field names. So, this update can very well be considered a bug
fix.
Although hyphens appear often in existing queries (for example,
hyphen as the first character is reserved for NOT and it may also
appear in label names and values), allowing them in field names
provides a lot of value given that there are plenty of plugins on
gerrit-review.googlesource.com itself which are named with hyphens.
Since hyphen as a prefix to the field name is reserved for NOT, in
this change we do not allow field names to start with hyphens, and
to be consistent, field names are also not allowed to end with a
hyphen.
Also, supporting hyphens in field names shouldn't introduce any new
risks that aren't already present with underscores.
Release-Notes: Plugins named with hyphens can now define query operators
Change-Id: I11872953bd362373dcbe54b4eeef26a85ab19340
diff --git a/antlr3/com/google/gerrit/index/query/Query.g b/antlr3/com/google/gerrit/index/query/Query.g
index c8587df..0994d9b 100644
--- a/antlr3/com/google/gerrit/index/query/Query.g
+++ b/antlr3/com/google/gerrit/index/query/Query.g
@@ -152,8 +152,10 @@
: ( ' ' | '\r' | '\t' | '\n' ) { $channel=HIDDEN; }
;
+fragment LOWERCASE_AND_UNDERSCORE: ('a'..'z' | '_')+ ;
+
FIELD_NAME
- : ('a'..'z' | '_')+
+ : LOWERCASE_AND_UNDERSCORE ( '-' LOWERCASE_AND_UNDERSCORE )*
;
EXACT_PHRASE
diff --git a/javatests/com/google/gerrit/index/query/QueryParserTest.java b/javatests/com/google/gerrit/index/query/QueryParserTest.java
index f478803..2ff56a8 100644
--- a/javatests/com/google/gerrit/index/query/QueryParserTest.java
+++ b/javatests/com/google/gerrit/index/query/QueryParserTest.java
@@ -19,6 +19,7 @@
import static com.google.gerrit.index.query.QueryParser.DEFAULT_FIELD;
import static com.google.gerrit.index.query.QueryParser.EXACT_PHRASE;
import static com.google.gerrit.index.query.QueryParser.FIELD_NAME;
+import static com.google.gerrit.index.query.QueryParser.NOT;
import static com.google.gerrit.index.query.QueryParser.SINGLE_WORD;
import static com.google.gerrit.index.query.QueryParser.parse;
import static com.google.gerrit.index.query.testing.TreeSubject.assertThat;
@@ -242,6 +243,119 @@
assertThat(r).child(0).hasText("A backslash \\ in phrase");
}
+ @Test
+ public void fieldNameWithNot() throws Exception {
+ Tree r = parse("-foo:bar");
+ assertThat(r).hasType(NOT);
+ assertThat(r).hasText("NOT");
+ assertThat(r).hasChildCount(1);
+ assertThat(r).child(0).hasType(FIELD_NAME);
+ assertThat(r).child(0).hasText("foo");
+ assertThat(r).child(0).hasChildCount(1);
+ assertThat(r).child(0).child(0).hasType(SINGLE_WORD);
+ assertThat(r).child(0).child(0).hasText("bar");
+ assertThat(r).child(0).child(0).hasNoChildren();
+ }
+
+ @Test
+ public void fieldNameWithDigit() throws Exception {
+ Tree r = parse("foo9:bar");
+ assertThat(r).hasType(DEFAULT_FIELD);
+ assertThat(r).hasChildCount(3);
+ assertThat(r).child(0).hasType(SINGLE_WORD);
+ assertThat(r).child(0).hasText("foo9");
+ assertThat(r).child(0).hasNoChildren();
+ assertThat(r).child(1).hasType(COLON);
+ assertThat(r).child(1).hasText(":");
+ assertThat(r).child(1).hasNoChildren();
+ assertThat(r).child(2).hasType(SINGLE_WORD);
+ assertThat(r).child(2).hasText("bar");
+ assertThat(r).child(2).hasNoChildren();
+ }
+
+ @Test
+ public void fieldNameWithUnderscore() throws Exception {
+ Tree r = parse("foo_bar:baz");
+ assertThat(r).hasType(FIELD_NAME);
+ assertThat(r).hasText("foo_bar");
+ assertThat(r).hasChildCount(1);
+ assertThat(r).child(0).hasType(SINGLE_WORD);
+ assertThat(r).child(0).hasText("baz");
+ assertThat(r).child(0).hasNoChildren();
+ }
+
+ @Test
+ public void fieldNameWithHyphen() throws Exception {
+ Tree r = parse("foo-bar:baz");
+ assertThat(r).hasType(FIELD_NAME);
+ assertThat(r).hasText("foo-bar");
+ assertThat(r).hasChildCount(1);
+ assertThat(r).child(0).hasType(SINGLE_WORD);
+ assertThat(r).child(0).hasText("baz");
+ assertThat(r).child(0).hasNoChildren();
+ }
+
+ @Test
+ public void fieldNameEndingWithHyphen() throws Exception {
+ Tree r = parse("foo-:bar");
+ assertThat(r).hasType(DEFAULT_FIELD);
+ assertThat(r).hasChildCount(3);
+ assertThat(r).child(0).hasType(SINGLE_WORD);
+ assertThat(r).child(0).hasText("foo-");
+ assertThat(r).child(0).hasNoChildren();
+ assertThat(r).child(1).hasType(COLON);
+ assertThat(r).child(1).hasText(":");
+ assertThat(r).child(1).hasNoChildren();
+ assertThat(r).child(2).hasType(SINGLE_WORD);
+ assertThat(r).child(2).hasText("bar");
+ assertThat(r).child(2).hasNoChildren();
+ }
+
+ @Test
+ public void fieldNameWithNotAndHyphen() throws Exception {
+ Tree r = parse("-foo-bar:baz");
+ assertThat(r).hasType(NOT);
+ assertThat(r).hasText("NOT");
+ assertThat(r).hasChildCount(1);
+ assertThat(r).child(0).hasType(FIELD_NAME);
+ assertThat(r).child(0).hasText("foo-bar");
+ assertThat(r).child(0).hasChildCount(1);
+ assertThat(r).child(0).child(0).hasType(SINGLE_WORD);
+ assertThat(r).child(0).child(0).hasText("baz");
+ assertThat(r).child(0).child(0).hasNoChildren();
+ }
+
+ @Test
+ public void fieldNameWithNotAndEndingWithHyphen() throws Exception {
+ Tree r = parse("-foo-bar-:baz");
+ assertThat(r).hasType(NOT);
+ assertThat(r).hasChildCount(1);
+ assertThat(r).child(0).hasType(DEFAULT_FIELD);
+ assertThat(r).child(0).hasChildCount(3);
+ assertThat(r).child(0).child(0).hasType(SINGLE_WORD);
+ assertThat(r).child(0).child(0).hasText("foo-bar-");
+ assertThat(r).child(0).child(0).hasNoChildren();
+ assertThat(r).child(0).child(1).hasType(COLON);
+ assertThat(r).child(0).child(1).hasText(":");
+ assertThat(r).child(0).child(1).hasNoChildren();
+ assertThat(r).child(0).child(2).hasType(SINGLE_WORD);
+ assertThat(r).child(0).child(2).hasText("baz");
+ assertThat(r).child(0).child(2).hasNoChildren();
+ }
+
+ @Test
+ public void fieldNameWithMiscellaneousCharacters() throws Exception {
+ Tree r = parse("-foo-bar_-baz_:qux");
+ assertThat(r).hasType(NOT);
+ assertThat(r).hasChildCount(1);
+ assertThat(r).child(0).hasType(FIELD_NAME);
+ assertThat(r).child(0).hasText("foo-bar_-baz_");
+ assertThat(r).child(0).hasChildCount(1);
+ assertThat(r).child(0).child(0).hasType(SINGLE_WORD);
+ assertThat(r).child(0).child(0).hasText("qux");
+ assertThat(r).child(0).child(0).hasNoChildren();
+ }
+
private static void assertParseFails(String query) {
assertThrows(QueryParseException.class, () -> parse(query));
}
