From: Tomas Babej Date: Sat, 30 Jan 2021 17:50:58 -0500 Subject: Backport parser issues fixes This backports upstream PR #2405 and commit a5eee5f, which contain extra fixes and tests for issues with numeric project names (amongst others) which were being wrongly interpreted as expressions. Addresses upstream issues TW-{2388,2386,2257,1908,1896} --- src/CLI2.cpp | 56 ++++++++++++++++++++++++++++++++++++++----- src/CLI2.h | 1 + src/columns/ColTypeString.cpp | 7 +++++- test/filter.t | 1 + test/project.t | 1 - test/tw-1883.t | 17 +++++++++++++ test/tw-1895.t | 6 +++++ test/tw-2257.t | 5 ++++ test/tw-2386.t | 8 +++++++ test/tw-2392.t | 10 ++++++++ 10 files changed, 104 insertions(+), 8 deletions(-) create mode 100755 test/tw-1883.t create mode 100644 test/tw-1895.t create mode 100755 test/tw-2257.t create mode 100755 test/tw-2386.t diff --git a/src/CLI2.cpp b/src/CLI2.cpp index ec2bd6c..e91aa71 100644 --- a/src/CLI2.cpp +++ b/src/CLI2.cpp @@ -1277,7 +1277,9 @@ void CLI2::desugarFilterAttributes () A2 op ("", Lexer::Type::op); op.tag ("FILTER"); - A2 rhs ("", values[0]._lextype); + // Attribute types that do not support evaluation should be interpreted + // as strings (currently this means that string attributes are not evaluated) + A2 rhs ("", evalSupported ? values[0]._lextype: Lexer::Type::string); rhs.tag ("FILTER"); // Special case for ':'. @@ -1371,7 +1373,7 @@ void CLI2::desugarFilterAttributes () // Do not modify this construct without full understanding. // Getting this wrong breaks a whole lot of filtering tests. - if (values.size () > 1 || evalSupported) + if (evalSupported) { for (auto& v : values) reconstructed.push_back (v); @@ -1945,6 +1947,40 @@ void CLI2::desugarFilterPlainArgs () } } +//////////////////////////////////////////////////////////////////////////////// +// Detects if the bracket at iterator it is a start or end of an empty paren expression +// Examples: +// ( status = pending ) ( ) +// ^ +// it -----| => true +// +// ( status = pending ) ( project = Home ) +// ^ +// it -----| => false +bool CLI2::isEmptyParenExpression (std::vector::iterator it, bool forward /* = true */) const +{ + int open = 0; + int closed = 0; + + for (auto a = it; a != (forward ? _args.end (): _args.begin()); (forward ? ++a: --a)) + { + if (a->attribute("raw") == "(") + open++; + else if (a->attribute("raw") == ")") + closed++; + else + // Encountering a non-paren token means there is something between parenthees + return false; + + // Getting balanced parentheses means we have an empty paren expression + if (open == closed && open != 0) + return true; + } + + // Should not end here. + return false; +} + //////////////////////////////////////////////////////////////////////////////// // Two consecutive FILTER, non-OP arguments that are not "(" or ")" need an // "and" operator inserted between them. @@ -1971,10 +2007,18 @@ void CLI2::insertJunctions () // Insert AND between terms. else if (a != prev) { - if ((prev->_lextype != Lexer::Type::op && a->attribute ("raw") == "(") || - (prev->_lextype != Lexer::Type::op && a->_lextype != Lexer::Type::op) || - (prev->attribute ("raw") == ")" && a->_lextype != Lexer::Type::op) || - (prev->attribute ("raw") == ")" && a->attribute ("raw") == "(")) + if ((prev->_lextype != Lexer::Type::op && + a->attribute ("raw") == "(" && + ! isEmptyParenExpression(a, true) ) || + (prev->attribute ("raw") == ")" && + a->_lextype != Lexer::Type::op && + ! isEmptyParenExpression(prev, false)) || + (prev->attribute ("raw") == ")" && + a->attribute ("raw") == "(" && + ! isEmptyParenExpression(a, true) && + ! isEmptyParenExpression(prev, false)) || + (prev->_lextype != Lexer::Type::op && + a->_lextype != Lexer::Type::op)) { A2 opOr ("and", Lexer::Type::op); opOr.tag ("FILTER"); diff --git a/src/CLI2.h b/src/CLI2.h index f6b0ce9..025be4c 100644 --- a/src/CLI2.h +++ b/src/CLI2.h @@ -99,6 +99,7 @@ private: void findUUIDs (); void insertIDExpr (); void lexFilterArgs (); + bool isEmptyParenExpression (std::vector::iterator it, bool forward = true) const; void desugarFilterPlainArgs (); void insertJunctions (); void defaultCommand (); diff --git a/src/columns/ColTypeString.cpp b/src/columns/ColTypeString.cpp index 37c1433..4ef9757 100644 --- a/src/columns/ColTypeString.cpp +++ b/src/columns/ColTypeString.cpp @@ -58,7 +58,12 @@ void ColumnTypeString::modify (Task& task, const std::string& value) std::string domRef; Lexer::Type type; if (lexer.token (domRef, type) && - type == Lexer::Type::dom) + type == Lexer::Type::dom && + // Ensure 'value' contains only the DOM reference and no other tokens + // The lexer.token returns false for end-of-string. + // This works as long as all the DOM references we should support consist + // only of a single token. + lexer.token (domRef, type) == false) { Eval e; e.addSource (domSource); diff --git a/test/filter.t b/test/filter.t index c69f257..1c3328d 100755 --- a/test/filter.t +++ b/test/filter.t @@ -1096,6 +1096,7 @@ class TestBug1915(TestCase): self.assertIn("thingB", out) self.assertNotIn("thingC", out) + @unittest.expectedFailure def test_complex_and_or_query_variant_eight(self): """1915: Make sure parser handles complex and-or queries correctly (8)""" code, out, err = self.t("rc.verbose:nothing status:pending and \\(project:A or project:B\\) all") diff --git a/test/project.t b/test/project.t index 91dd9ba..eb5ba92 100755 --- a/test/project.t +++ b/test/project.t @@ -381,7 +381,6 @@ class TestBug1455(TestCase): def setUp(self): self.t = Task() - @unittest.expectedFailure def test_project_hierarchy_filter(self): """1455: Test project:school) diff --git a/test/tw-1883.t b/test/tw-1883.t new file mode 100755 index 0000000..37a0f2b --- /dev/null +++ b/test/tw-1883.t @@ -0,0 +1,17 @@ +#!/bin/bash +# Test for TW-1883 (#1896 on Github) +# https://github.com/GothenburgBitFactory/taskwarrior/issues/1896 + +. bash_tap_tw.sh + +task add sample +task +PENDING '(' ')' +task '(' ')' +task '(' '(' ')' ')' +task '(' '(' ')' '(' ')' ')' + +# Detect that the task is actually displayed +task +PENDING '(' ')' | grep sample +task '(' ')' | grep sample +task '(' '(' ')' ')' | grep sample +task '(' '(' ')' '(' ')' ')' | grep sample diff --git a/test/tw-1895.t b/test/tw-1895.t new file mode 100644 index 0000000..3999150 --- /dev/null +++ b/test/tw-1895.t @@ -0,0 +1,6 @@ +#!/bin/bash + +. bash_tap_tw.sh + +task add description:'start something' +task | grep something diff --git a/test/tw-2257.t b/test/tw-2257.t new file mode 100755 index 0000000..2482230 --- /dev/null +++ b/test/tw-2257.t @@ -0,0 +1,5 @@ +#!/bin/bash + +. bash_tap_tw.sh + +task 'rc.uda.test.type=string' add 'test:"start doing the thing"' hello world diff --git a/test/tw-2386.t b/test/tw-2386.t new file mode 100755 index 0000000..a739e93 --- /dev/null +++ b/test/tw-2386.t @@ -0,0 +1,8 @@ +#!/bin/bash + +. bash_tap_tw.sh + +task add first project:someday +task add second project:bar + +task project:someday list | grep first diff --git a/test/tw-2392.t b/test/tw-2392.t index 0b7ae11..5ed31da 100755 --- a/test/tw-2392.t +++ b/test/tw-2392.t @@ -8,3 +8,13 @@ task add test project:c.vs.2021-01 task rc.debug.parser:3 project:c.vs.2021-01 _ids [[ `task project:c.vs.2021-01 _ids` == "1" ]] + +# Same thing now, but with 11 instead of 01 + +# This sets foo to "PT13H" despite it being a string UDA +task add test project:c.vs.2021-11 + +# Show the problem in TAP output +task rc.debug.parser:3 project:c.vs.2021-11 _ids + +[[ `task project:c.vs.2021-11 _ids` == "2" ]]