PostQuery::AST: fix #to_infix to not add unnecessary parentheses.

* Fix the `#to_infix` method to not add unnecessary parentheses around subexpressions.
* Fix metatags to add quotes around values when necessary.

app/logical/post_query/ast.rb

@@ -168,7 +168,7 @@ class PostQuery
         in [:tag, name]
           name
         in [:metatag, name, value]
-          "#{name}:#{value}"
+          "#{name}:#{quoted_value}"
         in [:wildcard, name]
           "(wildcard #{name})"
         in [type, *args]
@@ -180,7 +180,7 @@ class PostQuery
       def to_infix
         case self
         in [:all]
-          "all"
+          ""
         in [:none]
           "none"
         in [:wildcard, name]
@@ -188,19 +188,15 @@ class PostQuery
         in [:tag, name]
           name
         in [:metatag, name, value]
-          "#{name}:#{value}"
-        in [:not, a]
-          "-#{a.to_infix}"
-        in [:opt, a]
-          "~#{a.to_infix}"
-        in [:and, a]
-          a.to_infix
-        in [:or, a]
-          a.to_infix
-        in [:and, *a]
-          "(#{a.map(&:to_infix).join(" ")})"
-        in [:or, *a]
-          "(#{a.map(&:to_infix).join(" or ")})"
+          "#{name}:#{quoted_value}"
+        in :not, child
+          child.term? ? "-#{child.to_infix}" : "-(#{child.to_infix})"
+        in :opt, child
+          child.term? ? "~#{child.to_infix}" : "~(#{child.to_infix})"
+        in :and, *children
+          children.map { _1.children.many? ? "(#{_1.to_infix})" : _1.to_infix }.join(" ")
+        in :or, *children
+          children.map { _1.children.many? ? "(#{_1.to_infix})" : _1.to_infix }.join(" or ")
         end
       end
 
@@ -266,6 +262,17 @@ class PostQuery
         args.second if metatag?
       end
 
+      # @return [String, nil] The value of the metatag as a quoted string, if a metatag node.
+      def quoted_value
+        return nil unless metatag?
+
+        if value.include?(" ") || value.starts_with?('"') || value.empty?
+          %Q{"#{value.gsub(/"/, '\\"')}"}
+        else
+          value
+        end
+      end
+
       # @return [Array<AST>] The child nodes, if the node has children.
       def children
         term? ? [] : args

app/logical/post_query/parser.rb

@@ -33,6 +33,8 @@ require "strscan"
 
 class PostQuery
   class Parser
+    extend Memoist
+
     class Error < StandardError; end
 
     METATAG_NAME_REGEX = /(#{PostQueryBuilder::METATAGS.join("|")}):/i
@@ -265,5 +267,7 @@ class PostQuery
         AST.new(type, args)
       end
     end
+
+    memoize :parse, :parse!
   end
 end

test/unit/post_query_parser_test.rb

@@ -5,6 +5,10 @@ class PostQueryParserTest < ActiveSupport::TestCase
     assert_equal(expected, PostQuery::Parser.parse(input).simplify.to_sexp)
   end
 
+  def to_infix(string)
+    PostQuery::Parser.parse(string).to_infix
+  end
+
   context "PostQueryParser:" do
     should "parse empty queries correctly" do
       assert_parse_equals("all", "")
@@ -60,10 +64,11 @@ class PostQueryParserTest < ActiveSupport::TestCase
       assert_parse_equals("fav:(a)", "fav:(a)")
       assert_parse_equals("fav:(a", "(fav:(a)")
 
-      assert_parse_equals('source:foo bar', 'source:"foo bar"')
+      assert_parse_equals('source:"foo bar"', 'source:"foo bar"')
       assert_parse_equals('source:foobar"(', 'source:foobar"(')
-      assert_parse_equals('source:', 'source:""')
-      assert_parse_equals(%q{source:don't say "lazy" okay}, %q{source:"don't say \"lazy\" okay"})
+      assert_parse_equals('source:""', 'source:""')
+      assert_parse_equals('source:"\""', 'source:"\""')
+      assert_parse_equals(%q{source:"don't say \"lazy\" okay"}, %q{source:"don't say \"lazy\" okay"})
       assert_parse_equals(%q{(and source:foo)bar a)}, %q{(a (source:"foo)bar"))})
     end
 
@@ -293,5 +298,45 @@ class PostQueryParserTest < ActiveSupport::TestCase
       assert_parse_equals("none", 'source:"foo')
       assert_parse_equals("none", 'source:"foo bar')
     end
+
+    context "#to_infix" do
+      should "work" do
+        assert_equal("", to_infix(""))
+        assert_equal("", to_infix(" "))
+
+        assert_equal("a", to_infix("a"))
+        assert_equal("a b", to_infix("a b"))
+        assert_equal("a b c", to_infix("a b c"))
+
+        assert_equal("-a", to_infix("-a"))
+        assert_equal("-(-a)", to_infix("-(-a)"))
+        assert_equal("-(-(-a))", to_infix("-(-(-a))"))
+
+        assert_equal("~a", to_infix("~a"))
+        assert_equal("~a ~b", to_infix("~a ~b"))
+        assert_equal("~a ~(b ~c)", to_infix("~a ~(b ~c)"))
+
+        assert_equal("a b", to_infix("a and b"))
+        assert_equal("a (b c)", to_infix("a and b and c"))
+        assert_equal("a (b (c d))", to_infix("a and b and c and d"))
+
+        assert_equal("a or b", to_infix("a or b"))
+        assert_equal("a or (b or c)", to_infix("a or b or c"))
+        assert_equal("a or (b or (c or d))", to_infix("a or b or c or d"))
+
+        assert_equal("(a b) or (c d)", to_infix("a b or c d"))
+        assert_equal("(~a ~b) (~c ~d)", to_infix("~a ~b and ~c ~d"))
+        assert_equal("(a or b) (c or d)", to_infix("(a or b) and (c or d)"))
+
+        assert_equal("a source:b", to_infix("a source:b"))
+        assert_equal('a source:"b c"', to_infix('a source:"b c"'))
+        assert_equal('a source:"\\""', to_infix('a source:"\\""'))
+        assert_equal('a source:""', to_infix('a source:""'))
+
+        assert_equal("a* b", to_infix("a* b"))
+
+        assert_equal("none", to_infix("("))
+      end
+    end
   end
 end