Parse lower case and, or and not as operators
Previously, we only recognized AND, OR and NOT as boolean
operators. This seems overly strict as users might
unintentionally use the lower case variants. The problem
right now is that Gerrit parses the query with lower-case
operators and silently treats them as words for full
text search using the default query.
With this change, we modify the grammar and add tests to
recognize lower cased operators as well.
Release-Notes: Recognize lower case operators in search
Google-Bug: b/226563292
Change-Id: Ibf39d4d7cbdf60521b28276fb528cf680a20bbd9
diff --git a/Documentation/user-search.txt b/Documentation/user-search.txt
index bae083b..f716cb0 100644
--- a/Documentation/user-search.txt
+++ b/Documentation/user-search.txt
@@ -712,17 +712,20 @@
`-is:starred` is the exact opposite of `is:starred` and will
therefore return changes that are *not* starred by the current user.
-The operator `NOT` (in all caps) is a synonym.
+The operator `NOT` (in all caps) or `not` (all lower case) is a
+synonym.
=== AND
-The boolean operator `AND` (in all caps) can be used to join two
-other operators together. This results in a restriction of the
-results, returning only changes that match both operators.
+The boolean operator `AND` (in all caps) or `and` (all lower case)
+can be used to join two other operators together. This results in
+a restriction of the results, returning only changes that match both
+operators.
=== OR
-The boolean operator `OR` (in all caps) can be used to find changes
-that match either operator. This increases the number of results
-that are returned, as more changes are considered.
+The boolean operator `OR` (in all caps) or `or` (all lower case)
+can be used to find changes that match either operator. This
+increases the number of results that are returned, as more changes
+are considered.
[[labels]]
diff --git a/antlr3/com/google/gerrit/index/query/Query.g b/antlr3/com/google/gerrit/index/query/Query.g
index c8587df..7f868e2 100644
--- a/antlr3/com/google/gerrit/index/query/Query.g
+++ b/antlr3/com/google/gerrit/index/query/Query.g
@@ -142,9 +142,9 @@
| EXACT_PHRASE
;
-AND: 'AND' ;
-OR: 'OR' ;
-NOT: 'NOT' ;
+AND: 'AND' | 'and';
+OR: 'OR' | 'or' ;
+NOT: 'NOT' | 'not' ;
COLON: ':' ;
diff --git a/javatests/com/google/gerrit/index/query/QueryParserTest.java b/javatests/com/google/gerrit/index/query/QueryParserTest.java
index f478803..0574746 100644
--- a/javatests/com/google/gerrit/index/query/QueryParserTest.java
+++ b/javatests/com/google/gerrit/index/query/QueryParserTest.java
@@ -19,6 +19,8 @@
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.OR;
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 +244,36 @@
assertThat(r).child(0).hasText("A backslash \\ in phrase");
}
+ @Test
+ public void upperCaseParsedAsOperators() throws Exception {
+ Tree r = parse("project:foo:bar AND file:baz");
+ assertThat(r).hasType(AND);
+ assertThat(r).hasChildCount(2);
+
+ r = parse("project:foo:bar OR file:baz");
+ assertThat(r).hasType(OR);
+ assertThat(r).hasChildCount(2);
+
+ r = parse("NOT project:foo:bar");
+ assertThat(r).hasType(NOT);
+ assertThat(r).hasChildCount(1);
+ }
+
+ @Test
+ public void lowerCaseParsedAsOperators() throws Exception {
+ Tree r = parse("project:foo:bar and file:baz");
+ assertThat(r).hasType(AND);
+ assertThat(r).hasChildCount(2);
+
+ r = parse("project:foo:bar or file:baz");
+ assertThat(r).hasType(OR);
+ assertThat(r).hasChildCount(2);
+
+ r = parse("not project:foo:bar");
+ assertThat(r).hasType(NOT);
+ assertThat(r).hasChildCount(1);
+ }
+
private static void assertParseFails(String query) {
assertThrows(QueryParseException.class, () -> parse(query));
}
diff --git a/javatests/com/google/gerrit/server/query/change/AbstractQueryChangesTest.java b/javatests/com/google/gerrit/server/query/change/AbstractQueryChangesTest.java
index 55c7921..6d9f916 100644
--- a/javatests/com/google/gerrit/server/query/change/AbstractQueryChangesTest.java
+++ b/javatests/com/google/gerrit/server/query/change/AbstractQueryChangesTest.java
@@ -354,6 +354,18 @@
}
@Test
+ public void byStatusOr() throws Exception {
+ TestRepository<Repo> repo = createProject("repo");
+ ChangeInserter ins1 = newChangeWithStatus(repo, Change.Status.NEW);
+ Change change1 = insert(repo, ins1);
+ ChangeInserter ins2 = newChangeWithStatus(repo, Change.Status.MERGED);
+ Change change2 = insert(repo, ins2);
+
+ assertQuery("status:new OR status:merged", change2, change1);
+ assertQuery("status:new or status:merged", change2, change1);
+ }
+
+ @Test
public void byStatusOpen() throws Exception {
TestRepository<Repo> repo = createProject("repo");
ChangeInserter ins1 = newChangeWithStatus(repo, Change.Status.NEW);
