Allow newlines in AST Matchers in clang-query files

Reviewers: aaron.ballman

Subscribers: cfe-commits

Tags: #clang

Differential Revision: https://reviews.llvm.org/D71842

clang-tools-extra/clang-query/Query.cpp

@@ -101,9 +101,24 @@ bool MatchQuery::run(llvm::raw_ostream &OS, QuerySession &QS) const {
     Finder.matchAST(AST->getASTContext());
 
     if (QS.PrintMatcher) {
-      std::string prefixText = "Matcher: ";
+      SmallVector<StringRef, 4> Lines;
-      OS << "\n  " << prefixText << Source << "\n";
+      Source.split(Lines, "\n");
-      OS << "  " << std::string(prefixText.size() + Source.size(), '=') << '\n';
+      auto FirstLine = Lines[0];
+      Lines.erase(Lines.begin(), Lines.begin() + 1);
+      while (!Lines.empty() && Lines.back().empty()) {
+        Lines.resize(Lines.size() - 1);
+      }
+      unsigned MaxLength = FirstLine.size();
+      std::string PrefixText = "Matcher: ";
+      OS << "\n  " << PrefixText << FirstLine;
+
+      for (auto Line : Lines) {
+        OS << "\n" << std::string(PrefixText.size() + 2, ' ') << Line;
+        MaxLength = std::max<int>(MaxLength, Line.rtrim().size());
+      }
+
+      OS << "\n"
+         << "  " << std::string(PrefixText.size() + MaxLength, '=') << "\n\n";
     }
 
     for (auto MI = Matches.begin(), ME = Matches.end(); MI != ME; ++MI) {

clang-tools-extra/clang-query/Query.h

@@ -44,6 +44,7 @@ struct Query : llvm::RefCountedBase<Query> {
   /// \return false if an error occurs, otherwise return true.
   virtual bool run(llvm::raw_ostream &OS, QuerySession &QS) const = 0;
 
+  StringRef RemainingContent;
   const QueryKind Kind;
 };
 

clang-tools-extra/clang-query/QueryParser.cpp

@@ -26,7 +26,10 @@ namespace query {
 // is found before End, return StringRef().  Begin is adjusted to exclude the
 // lexed region.
 StringRef QueryParser::lexWord() {
-  Line = Line.ltrim();
+  Line = Line.drop_while([](char c) {
+    // Don't trim newlines.
+    return StringRef(" \t\v\f\r").contains(c);
+  });
 
   if (Line.empty())
     // Even though the Line is empty, it contains a pointer and
@@ -34,12 +37,12 @@ StringRef QueryParser::lexWord() {
     // code completion.
     return Line;
 
-  if (Line.front() == '#') {
+  StringRef Word;
-    Line = {};
+  if (Line.front() == '#')
-    return StringRef();
+    Word = Line.substr(0, 1);
-  }
+  else
+    Word = Line.take_until(isWhitespace);
 
-  StringRef Word = Line.take_until(isWhitespace);
   Line = Line.drop_front(Word.size());
   return Word;
 }
@@ -125,9 +128,25 @@ template <typename QueryType> QueryRef QueryParser::parseSetOutputKind() {
 }
 
 QueryRef QueryParser::endQuery(QueryRef Q) {
-  const StringRef Extra = Line;
+  StringRef Extra = Line;
-  if (!lexWord().empty())
+  StringRef ExtraTrimmed = Extra.drop_while(
-    return new InvalidQuery("unexpected extra input: '" + Extra + "'");
+      [](char c) { return StringRef(" \t\v\f\r").contains(c); });
+
+  if ((!ExtraTrimmed.empty() && ExtraTrimmed[0] == '\n') ||
+      (ExtraTrimmed.size() >= 2 && ExtraTrimmed[0] == '\r' &&
+       ExtraTrimmed[1] == '\n'))
+    Q->RemainingContent = Extra;
+  else {
+    StringRef TrailingWord = lexWord();
+    if (!TrailingWord.empty() && TrailingWord.front() == '#') {
+      Line = Line.drop_until([](char c) { return c == '\n'; });
+      Line = Line.drop_while([](char c) { return c == '\n'; });
+      return endQuery(Q);
+    }
+    if (!TrailingWord.empty()) {
+      return new InvalidQuery("unexpected extra input: '" + Extra + "'");
+    }
+  }
   return Q;
 }
 
@@ -193,7 +212,11 @@ QueryRef QueryParser::doParse() {
   switch (QKind) {
   case PQK_Comment:
   case PQK_NoOp:
-    return new NoOpQuery;
+    Line = Line.drop_until([](char c) { return c == '\n'; });
+    Line = Line.drop_while([](char c) { return c == '\n'; });
+    if (Line.empty())
+      return new NoOpQuery;
+    return doParse();
 
   case PQK_Help:
     return endQuery(new HelpQuery);
@@ -217,7 +240,9 @@ QueryRef QueryParser::doParse() {
       return makeInvalidQueryFromDiagnostics(Diag);
     }
 
-    return new LetQuery(Name, Value);
+    auto *Q = new LetQuery(Name, Value);
+    Q->RemainingContent = Line;
+    return Q;
   }
 
   case PQK_Match: {
@@ -226,12 +251,17 @@ QueryRef QueryParser::doParse() {
 
     Diagnostics Diag;
     auto MatcherSource = Line.trim();
+    auto OrigMatcherSource = MatcherSource;
     Optional<DynTypedMatcher> Matcher = Parser::parseMatcherExpression(
         MatcherSource, nullptr, &QS.NamedValues, &Diag);
     if (!Matcher) {
       return makeInvalidQueryFromDiagnostics(Diag);
     }
-    return new MatchQuery(MatcherSource, *Matcher);
+    auto ActualSource = OrigMatcherSource.slice(0, OrigMatcherSource.size() -
+                                                       MatcherSource.size());
+    auto *Q = new MatchQuery(ActualSource, *Matcher);
+    Q->RemainingContent = MatcherSource;
+    return Q;
   }
 
   case PQK_Set: {

clang-tools-extra/clang-query/tool/ClangQuery.cpp

@@ -69,13 +69,16 @@ bool runCommandsInFile(const char *ExeName, std::string const &FileName,
     llvm::errs() << ExeName << ": cannot open " << FileName << "\n";
     return 1;
   }
-  while (Input.good()) {
-    std::string Line;
-    std::getline(Input, Line);
 
-    QueryRef Q = QueryParser::parse(Line, QS);
+  std::string FileContent((std::istreambuf_iterator<char>(Input)),
+                          std::istreambuf_iterator<char>());
+
+  StringRef FileContentRef(FileContent);
+  while (!FileContentRef.empty()) {
+    QueryRef Q = QueryParser::parse(FileContentRef, QS);
     if (!Q->run(llvm::outs(), QS))
       return true;
+    FileContentRef = Q->RemainingContent;
   }
   return false;
 }

clang-tools-extra/unittests/clang-query/QueryParserTest.cpp

@@ -230,3 +230,104 @@ TEST_F(QueryParserTest, Complete) {
   EXPECT_EQ("et ", Comps[0].TypedText);
   EXPECT_EQ("let", Comps[0].DisplayText);
 }
+
+TEST_F(QueryParserTest, Multiline) {
+
+  // Single string with multiple commands
+  QueryRef Q = parse(R"matcher(
+set bind-root false
+set output dump
+    )matcher");
+
+  ASSERT_TRUE(isa<SetQuery<bool>>(Q));
+
+  Q = parse(Q->RemainingContent);
+  ASSERT_TRUE(isa<SetExclusiveOutputQuery>(Q));
+
+  // Missing newline
+  Q = parse(R"matcher(
+set bind-root false set output dump
+    )matcher");
+
+  ASSERT_TRUE(isa<InvalidQuery>(Q));
+  EXPECT_EQ("unexpected extra input: ' set output dump\n    '",
+            cast<InvalidQuery>(Q)->ErrStr);
+
+  // Commands which do their own parsing
+  Q = parse(R"matcher(
+let fn functionDecl(hasName("foo"))
+match callExpr(callee(functionDecl()))
+    )matcher");
+
+  ASSERT_TRUE(isa<LetQuery>(Q));
+
+  Q = parse(Q->RemainingContent);
+  ASSERT_TRUE(isa<MatchQuery>(Q));
+
+  // Multi-line matcher
+  Q = parse(R"matcher(
+match callExpr(callee(
+    functionDecl().bind("fn")
+    ))
+
+    )matcher");
+
+  ASSERT_TRUE(isa<MatchQuery>(Q));
+
+  // Comment locations
+  Q = parse(R"matcher(
+#nospacecomment
+# Leading comment
+match callExpr ( # Trailing comment
+            # Comment alone on line
+
+            callee(
+            functionDecl(
+            ).bind(
+            "fn"
+            )
+            )) # Comment trailing close
+# Comment after match
+    )matcher");
+
+  ASSERT_TRUE(isa<MatchQuery>(Q));
+
+  // \r\n
+  Q = parse("set bind-root false\r\nset output dump");
+
+  ASSERT_TRUE(isa<SetQuery<bool>>(Q));
+
+  Q = parse(Q->RemainingContent);
+  ASSERT_TRUE(isa<SetExclusiveOutputQuery>(Q));
+
+  // Leading and trailing space in lines
+  Q = parse("  set bind-root false  \r\n  set output dump  ");
+
+  ASSERT_TRUE(isa<SetQuery<bool>>(Q));
+
+  Q = parse(Q->RemainingContent);
+  ASSERT_TRUE(isa<SetExclusiveOutputQuery>(Q));
+
+  // Incomplete commands
+  Q = parse("set\nbind-root false");
+
+  ASSERT_TRUE(isa<InvalidQuery>(Q));
+  EXPECT_EQ("expected variable name", cast<InvalidQuery>(Q)->ErrStr);
+
+  Q = parse("set bind-root\nfalse");
+
+  ASSERT_TRUE(isa<InvalidQuery>(Q));
+  EXPECT_EQ("expected 'true' or 'false', got ''",
+            cast<InvalidQuery>(Q)->ErrStr);
+
+  Q = parse(R"matcher(
+match callExpr
+(
+)
+    )matcher");
+
+  ASSERT_TRUE(isa<InvalidQuery>(Q));
+  EXPECT_EQ("1:9: Error parsing matcher. Found token <NewLine> "
+            "while looking for '('.",
+            cast<InvalidQuery>(Q)->ErrStr);
+}

clang/include/clang/ASTMatchers/Dynamic/Parser.h

@@ -164,16 +164,14 @@ public:
   ///   description of the error.
   ///   The caller takes ownership of the DynTypedMatcher object returned.
   static llvm::Optional<DynTypedMatcher>
-  parseMatcherExpression(StringRef MatcherCode, Sema *S,
+  parseMatcherExpression(StringRef &MatcherCode, Sema *S,
-                         const NamedValueMap *NamedValues,
+                         const NamedValueMap *NamedValues, Diagnostics *Error);
-                         Diagnostics *Error);
   static llvm::Optional<DynTypedMatcher>
-  parseMatcherExpression(StringRef MatcherCode, Sema *S,
+  parseMatcherExpression(StringRef &MatcherCode, Sema *S, Diagnostics *Error) {
-                         Diagnostics *Error) {
     return parseMatcherExpression(MatcherCode, S, nullptr, Error);
   }
   static llvm::Optional<DynTypedMatcher>
-  parseMatcherExpression(StringRef MatcherCode, Diagnostics *Error) {
+  parseMatcherExpression(StringRef &MatcherCode, Diagnostics *Error) {
     return parseMatcherExpression(MatcherCode, nullptr, Error);
   }
 
@@ -189,14 +187,14 @@ public:
   /// \param NamedValues A map of precomputed named values.  This provides
   ///   the dictionary for the <NamedValue> rule of the grammar.
   ///   If null, it is ignored.
-  static bool parseExpression(StringRef Code, Sema *S,
+  static bool parseExpression(StringRef &Code, Sema *S,
                               const NamedValueMap *NamedValues,
                               VariantValue *Value, Diagnostics *Error);
-  static bool parseExpression(StringRef Code, Sema *S,
+  static bool parseExpression(StringRef &Code, Sema *S, VariantValue *Value,
-                              VariantValue *Value, Diagnostics *Error) {
+                              Diagnostics *Error) {
     return parseExpression(Code, S, nullptr, Value, Error);
   }
-  static bool parseExpression(StringRef Code, VariantValue *Value,
+  static bool parseExpression(StringRef &Code, VariantValue *Value,
                               Diagnostics *Error) {
     return parseExpression(Code, nullptr, Value, Error);
   }
@@ -213,14 +211,14 @@ public:
   /// \return The list of completions, which may be empty if there are no
   /// available completions or if an error occurred.
   static std::vector<MatcherCompletion>
-  completeExpression(StringRef Code, unsigned CompletionOffset, Sema *S,
+  completeExpression(StringRef &Code, unsigned CompletionOffset, Sema *S,
                      const NamedValueMap *NamedValues);
   static std::vector<MatcherCompletion>
-  completeExpression(StringRef Code, unsigned CompletionOffset, Sema *S) {
+  completeExpression(StringRef &Code, unsigned CompletionOffset, Sema *S) {
     return completeExpression(Code, CompletionOffset, S, nullptr);
   }
   static std::vector<MatcherCompletion>
-  completeExpression(StringRef Code, unsigned CompletionOffset) {
+  completeExpression(StringRef &Code, unsigned CompletionOffset) {
     return completeExpression(Code, CompletionOffset, nullptr);
   }
 

clang/lib/ASTMatchers/Dynamic/Parser.cpp

@@ -38,6 +38,7 @@ struct Parser::TokenInfo {
   /// Different possible tokens.
   enum TokenKind {
     TK_Eof,
+    TK_NewLine,
     TK_OpenParen,
     TK_CloseParen,
     TK_Comma,
@@ -65,12 +66,12 @@ const char* const Parser::TokenInfo::ID_Bind = "bind";
 /// Simple tokenizer for the parser.
 class Parser::CodeTokenizer {
 public:
-  explicit CodeTokenizer(StringRef MatcherCode, Diagnostics *Error)
+  explicit CodeTokenizer(StringRef &MatcherCode, Diagnostics *Error)
       : Code(MatcherCode), StartOfLine(MatcherCode), Error(Error) {
     NextToken = getNextToken();
   }
 
-  CodeTokenizer(StringRef MatcherCode, Diagnostics *Error,
+  CodeTokenizer(StringRef &MatcherCode, Diagnostics *Error,
                 unsigned CodeCompletionOffset)
       : Code(MatcherCode), StartOfLine(MatcherCode), Error(Error),
         CodeCompletionLocation(MatcherCode.data() + CodeCompletionOffset) {
@@ -87,6 +88,19 @@ public:
     return ThisToken;
   }
 
+  TokenInfo SkipNewlines() {
+    while (NextToken.Kind == TokenInfo::TK_NewLine)
+      NextToken = getNextToken();
+    return NextToken;
+  }
+
+  TokenInfo consumeNextTokenIgnoreNewlines() {
+    SkipNewlines();
+    if (NextToken.Kind == TokenInfo::TK_Eof)
+      return NextToken;
+    return consumeNextToken();
+  }
+
   TokenInfo::TokenKind nextTokenKind() const { return NextToken.Kind; }
 
 private:
@@ -110,9 +124,8 @@ private:
 
     switch (Code[0]) {
     case '#':
-      Result.Kind = TokenInfo::TK_Eof;
+      Code = Code.drop_until([](char c) { return c == '\n'; });
-      Result.Text = "";
+      return getNextToken();
-      return Result;
     case ',':
       Result.Kind = TokenInfo::TK_Comma;
       Result.Text = Code.substr(0, 1);
@@ -123,6 +136,13 @@ private:
       Result.Text = Code.substr(0, 1);
       Code = Code.drop_front();
       break;
+    case '\n':
+      ++Line;
+      StartOfLine = Code.drop_front();
+      Result.Kind = TokenInfo::TK_NewLine;
+      Result.Text = Code.substr(0, 1);
+      Code = Code.drop_front();
+      break;
     case '(':
       Result.Kind = TokenInfo::TK_OpenParen;
       Result.Text = Code.substr(0, 1);
@@ -277,13 +297,10 @@ private:
 
   /// Consume all leading whitespace from \c Code.
   void consumeWhitespace() {
-    while (!Code.empty() && isWhitespace(Code[0])) {
+    Code = Code.drop_while([](char c) {
-      if (Code[0] == '\n') {
+      // Don't trim newlines.
-        ++Line;
+      return StringRef(" \t\v\f\r").contains(c);
-        StartOfLine = Code.drop_front();
+    });
-      }
-      Code = Code.drop_front();
-    }
   }
 
   SourceLocation currentLocation() {
@@ -293,7 +310,7 @@ private:
     return Location;
   }
 
-  StringRef Code;
+  StringRef &Code;
   StringRef StartOfLine;
   unsigned Line = 1;
   Diagnostics *Error;
@@ -337,6 +354,13 @@ struct Parser::ScopedContextEntry {
 bool Parser::parseIdentifierPrefixImpl(VariantValue *Value) {
   const TokenInfo NameToken = Tokenizer->consumeNextToken();
 
+  if (Tokenizer->nextTokenKind() == TokenInfo::TK_NewLine) {
+    Error->addError(Tokenizer->peekNextToken().Range,
+                    Error->ET_ParserNoOpenParen)
+        << "NewLine";
+    return false;
+  }
+
   if (Tokenizer->nextTokenKind() != TokenInfo::TK_OpenParen) {
     // Parse as a named value.
     if (const VariantValue NamedValue =
@@ -368,6 +392,7 @@ bool Parser::parseIdentifierPrefixImpl(VariantValue *Value) {
     // unknown named value.
     if ((Tokenizer->nextTokenKind() == TokenInfo::TK_Comma ||
          Tokenizer->nextTokenKind() == TokenInfo::TK_CloseParen ||
+         Tokenizer->nextTokenKind() == TokenInfo::TK_NewLine ||
          Tokenizer->nextTokenKind() == TokenInfo::TK_Eof) &&
         !S->lookupMatcherCtor(NameToken.Text)) {
       Error->addError(NameToken.Range, Error->ET_RegistryValueNotFound)
@@ -377,6 +402,8 @@ bool Parser::parseIdentifierPrefixImpl(VariantValue *Value) {
     // Otherwise, fallback to the matcher parser.
   }
 
+  Tokenizer->SkipNewlines();
+
   // Parse as a matcher expression.
   return parseMatcherExpressionImpl(NameToken, Value);
 }
@@ -392,8 +419,8 @@ bool Parser::parseBindID(std::string &BindID) {
   }
 
   const TokenInfo OpenToken = Tokenizer->consumeNextToken();
-  const TokenInfo IDToken = Tokenizer->consumeNextToken();
+  const TokenInfo IDToken = Tokenizer->consumeNextTokenIgnoreNewlines();
-  const TokenInfo CloseToken = Tokenizer->consumeNextToken();
+  const TokenInfo CloseToken = Tokenizer->consumeNextTokenIgnoreNewlines();
 
   // TODO: We could use different error codes for each/some to be more
   //       explicit about the syntax error.
@@ -443,6 +470,8 @@ bool Parser::parseMatcherExpressionImpl(const TokenInfo &NameToken,
   std::vector<ParserValue> Args;
   TokenInfo EndToken;
 
+  Tokenizer->SkipNewlines();
+
   {
     ScopedContextEntry SCE(this, Ctor ? *Ctor : nullptr);
 
@@ -466,12 +495,14 @@ bool Parser::parseMatcherExpressionImpl(const TokenInfo &NameToken,
                                NameToken.Text, NameToken.Range,
                                Args.size() + 1);
       ParserValue ArgValue;
+      Tokenizer->SkipNewlines();
       ArgValue.Text = Tokenizer->peekNextToken().Text;
       ArgValue.Range = Tokenizer->peekNextToken().Range;
       if (!parseExpressionImpl(&ArgValue.Value)) {
         return false;
       }
 
+      Tokenizer->SkipNewlines();
       Args.push_back(ArgValue);
       SCE.nextArg();
     }
@@ -531,7 +562,7 @@ std::vector<MatcherCompletion> Parser::getNamedValueCompletions(
 }
 
 void Parser::addExpressionCompletions() {
-  const TokenInfo CompToken = Tokenizer->consumeNextToken();
+  const TokenInfo CompToken = Tokenizer->consumeNextTokenIgnoreNewlines();
   assert(CompToken.Kind == TokenInfo::TK_CodeCompletion);
 
   // We cannot complete code if there is an invalid element on the context
@@ -575,7 +606,9 @@ bool Parser::parseExpressionImpl(VariantValue *Value) {
   case TokenInfo::TK_Error:
     // This error was already reported by the tokenizer.
     return false;
-
+  case TokenInfo::TK_NewLine:
+    llvm_unreachable("Newline should never be found here");
+    return false;
   case TokenInfo::TK_OpenParen:
   case TokenInfo::TK_CloseParen:
   case TokenInfo::TK_Comma:
@@ -624,13 +657,14 @@ std::vector<MatcherCompletion> Parser::RegistrySema::getMatcherCompletions(
   return Registry::getMatcherCompletions(AcceptedTypes);
 }
 
-bool Parser::parseExpression(StringRef Code, Sema *S,
+bool Parser::parseExpression(StringRef &Code, Sema *S,
                              const NamedValueMap *NamedValues,
                              VariantValue *Value, Diagnostics *Error) {
   CodeTokenizer Tokenizer(Code, Error);
   if (!Parser(&Tokenizer, S, NamedValues, Error).parseExpressionImpl(Value))
     return false;
-  if (Tokenizer.peekNextToken().Kind != TokenInfo::TK_Eof) {
+  auto NT = Tokenizer.peekNextToken();
+  if (NT.Kind != TokenInfo::TK_Eof && NT.Kind != TokenInfo::TK_NewLine) {
     Error->addError(Tokenizer.peekNextToken().Range,
                     Error->ET_ParserTrailingCode);
     return false;
@@ -639,7 +673,7 @@ bool Parser::parseExpression(StringRef Code, Sema *S,
 }
 
 std::vector<MatcherCompletion>
-Parser::completeExpression(StringRef Code, unsigned CompletionOffset, Sema *S,
+Parser::completeExpression(StringRef &Code, unsigned CompletionOffset, Sema *S,
                            const NamedValueMap *NamedValues) {
   Diagnostics Error;
   CodeTokenizer Tokenizer(Code, &Error, CompletionOffset);
@@ -659,7 +693,7 @@ Parser::completeExpression(StringRef Code, unsigned CompletionOffset, Sema *S,
 }
 
 llvm::Optional<DynTypedMatcher>
-Parser::parseMatcherExpression(StringRef Code, Sema *S,
+Parser::parseMatcherExpression(StringRef &Code, Sema *S,
                                const NamedValueMap *NamedValues,
                                Diagnostics *Error) {
   VariantValue Value;

clang/unittests/ASTMatchers/Dynamic/ParserTest.cpp

@@ -207,10 +207,12 @@ Parser::NamedValueMap getTestNamedValues() {
 
 TEST(ParserTest, FullParserTest) {
   Diagnostics Error;
-  llvm::Optional<DynTypedMatcher> VarDecl(Parser::parseMatcherExpression(
+
+  StringRef Code =
       "varDecl(hasInitializer(binaryOperator(hasLHS(integerLiteral()),"
-      "                                      hasOperatorName(\"+\"))))",
+      "                                      hasOperatorName(\"+\"))))";
-      &Error));
+  llvm::Optional<DynTypedMatcher> VarDecl(
+      Parser::parseMatcherExpression(Code, &Error));
   EXPECT_EQ("", Error.toStringFull());
   Matcher<Decl> M = VarDecl->unconditionalConvertTo<Decl>();
   EXPECT_TRUE(matches("int x = 1 + false;", M));
@@ -218,8 +220,9 @@ TEST(ParserTest, FullParserTest) {
   EXPECT_FALSE(matches("int x = 1 - false;", M));
   EXPECT_FALSE(matches("int x = true - 1;", M));
 
-  llvm::Optional<DynTypedMatcher> HasParameter(Parser::parseMatcherExpression(
+  Code = "functionDecl(hasParameter(1, hasName(\"x\")))";
-      "functionDecl(hasParameter(1, hasName(\"x\")))", &Error));
+  llvm::Optional<DynTypedMatcher> HasParameter(
+      Parser::parseMatcherExpression(Code, &Error));
   EXPECT_EQ("", Error.toStringFull());
   M = HasParameter->unconditionalConvertTo<Decl>();
 
@@ -228,20 +231,18 @@ TEST(ParserTest, FullParserTest) {
 
   // Test named values.
   auto NamedValues = getTestNamedValues();
+
+  Code = "functionDecl(hasParamA, hasParameter(1, hasName(nameX)))";
   llvm::Optional<DynTypedMatcher> HasParameterWithNamedValues(
-      Parser::parseMatcherExpression(
+      Parser::parseMatcherExpression(Code, nullptr, &NamedValues, &Error));
-          "functionDecl(hasParamA, hasParameter(1, hasName(nameX)))",
-          nullptr, &NamedValues, &Error));
   EXPECT_EQ("", Error.toStringFull());
   M = HasParameterWithNamedValues->unconditionalConvertTo<Decl>();
 
   EXPECT_TRUE(matches("void f(int a, int x);", M));
   EXPECT_FALSE(matches("void f(int x, int a);", M));
 
-
+  Code = "hasInitializer(\n    binaryOperator(hasLHS(\"A\")))";
-  EXPECT_TRUE(!Parser::parseMatcherExpression(
+  EXPECT_TRUE(!Parser::parseMatcherExpression(Code, &Error).hasValue());
-                   "hasInitializer(\n    binaryOperator(hasLHS(\"A\")))",
-                   &Error).hasValue());
   EXPECT_EQ("1:1: Error parsing argument 1 for matcher hasInitializer.\n"
             "2:5: Error parsing argument 1 for matcher binaryOperator.\n"
             "2:20: Error building matcher hasLHS.\n"
@@ -252,9 +253,11 @@ TEST(ParserTest, FullParserTest) {
 
 TEST(ParserTest, VariadicMatchTest) {
   Diagnostics Error;
-  llvm::Optional<DynTypedMatcher> OM(Parser::parseMatcherExpression(
+
-      "stmt(objcMessageExpr(hasAnySelector(\"methodA\", \"methodB:\")))",
+  StringRef Code =
-      &Error));
+      "stmt(objcMessageExpr(hasAnySelector(\"methodA\", \"methodB:\")))";
+  llvm::Optional<DynTypedMatcher> OM(
+      Parser::parseMatcherExpression(Code, &Error));
   EXPECT_EQ("", Error.toStringFull());
   auto M = OM->unconditionalConvertTo<Stmt>();
   EXPECT_TRUE(matchesObjC("@interface I @end "
@@ -324,15 +327,132 @@ TEST(ParserTest, OverloadErrors) {
             ParseWithError("callee(\"A\")"));
 }
 
+TEST(ParserTest, ParseMultiline) {
+  StringRef Code;
+
+  llvm::Optional<DynTypedMatcher> M;
+  {
+    Code = R"matcher(varDecl(
+  hasName("foo")
+  )
+)matcher";
+    Diagnostics Error;
+    EXPECT_TRUE(Parser::parseMatcherExpression(Code, &Error).hasValue());
+  }
+
+  {
+    Code = R"matcher(varDecl(
+  # Internal comment
+  hasName("foo") # Internal comment
+# Internal comment
+  )
+)matcher";
+    Diagnostics Error;
+    EXPECT_TRUE(Parser::parseMatcherExpression(Code, &Error).hasValue());
+  }
+
+  {
+    Code = R"matcher(decl().bind(
+  "paramName")
+)matcher";
+    Diagnostics Error;
+    EXPECT_TRUE(Parser::parseMatcherExpression(Code, &Error).hasValue());
+  }
+
+  {
+    Code = R"matcher(decl().bind(
+  "paramName"
+  )
+)matcher";
+    Diagnostics Error;
+    EXPECT_TRUE(Parser::parseMatcherExpression(Code, &Error).hasValue());
+  }
+
+  {
+    Code = R"matcher(decl(decl()
+, decl()))matcher";
+    Diagnostics Error;
+    EXPECT_TRUE(Parser::parseMatcherExpression(Code, &Error).hasValue());
+  }
+
+  {
+    Code = R"matcher(decl(decl(),
+decl()))matcher";
+    Diagnostics Error;
+    EXPECT_TRUE(Parser::parseMatcherExpression(Code, &Error).hasValue());
+  }
+
+  {
+    Code = "namedDecl(hasName(\"n\"\n))";
+    Diagnostics Error;
+    EXPECT_TRUE(Parser::parseMatcherExpression(Code, &Error).hasValue());
+  }
+
+  {
+    Diagnostics Error;
+
+    auto NamedValues = getTestNamedValues();
+
+    Code = R"matcher(hasParamA.bind
+  ("paramName")
+)matcher";
+    M = Parser::parseMatcherExpression(Code, nullptr, &NamedValues, &Error);
+    EXPECT_FALSE(M.hasValue());
+    EXPECT_EQ("1:15: Malformed bind() expression.", Error.toStringFull());
+  }
+
+  {
+    Diagnostics Error;
+
+    auto NamedValues = getTestNamedValues();
+
+    Code = R"matcher(hasParamA.
+  bind("paramName")
+)matcher";
+    M = Parser::parseMatcherExpression(Code, nullptr, &NamedValues, &Error);
+    EXPECT_FALSE(M.hasValue());
+    EXPECT_EQ("1:11: Malformed bind() expression.", Error.toStringFull());
+  }
+
+  {
+    Diagnostics Error;
+
+    Code = R"matcher(varDecl
+()
+)matcher";
+    M = Parser::parseMatcherExpression(Code, nullptr, nullptr, &Error);
+    EXPECT_FALSE(M.hasValue());
+    EXPECT_EQ("1:8: Error parsing matcher. Found token "
+              "<NewLine> while looking for '('.",
+              Error.toStringFull());
+  }
+
+  // Correct line/column numbers
+  {
+    Diagnostics Error;
+
+    Code = R"matcher(varDecl(
+  doesNotExist()
+  )
+)matcher";
+    M = Parser::parseMatcherExpression(Code, nullptr, nullptr, &Error);
+    EXPECT_FALSE(M.hasValue());
+    EXPECT_EQ(R"error(1:1: Error parsing argument 1 for matcher varDecl.
+2:3: Matcher not found: doesNotExist)error",
+              Error.toStringFull());
+  }
+}
+
 TEST(ParserTest, CompletionRegistry) {
-  std::vector<MatcherCompletion> Comps =
+  StringRef Code = "while";
-      Parser::completeExpression("while", 5);
+  std::vector<MatcherCompletion> Comps = Parser::completeExpression(Code, 5);
   ASSERT_EQ(1u, Comps.size());
   EXPECT_EQ("Stmt(", Comps[0].TypedText);
   EXPECT_EQ("Matcher<Stmt> whileStmt(Matcher<WhileStmt>...)",
             Comps[0].MatcherDecl);
 
-  Comps = Parser::completeExpression("whileStmt().", 12);
+  Code = "whileStmt().";
+  Comps = Parser::completeExpression(Code, 12);
   ASSERT_EQ(1u, Comps.size());
   EXPECT_EQ("bind(\"", Comps[0].TypedText);
   EXPECT_EQ("bind", Comps[0].MatcherDecl);
@@ -380,9 +500,9 @@ TEST(ParserTest, ParseBindOnLet) {
   Diagnostics Error;
 
   {
+    StringRef Code = "hasParamA.bind(\"parmABinding\")";
     llvm::Optional<DynTypedMatcher> TopLevelLetBinding(
-        Parser::parseMatcherExpression("hasParamA.bind(\"parmABinding\")",
+        Parser::parseMatcherExpression(Code, nullptr, &NamedValues, &Error));
-                                       nullptr, &NamedValues, &Error));
     EXPECT_EQ("", Error.toStringFull());
     auto M = TopLevelLetBinding->unconditionalConvertTo<Decl>();
 
@@ -395,10 +515,9 @@ TEST(ParserTest, ParseBindOnLet) {
   }
 
   {
+    StringRef Code = "functionDecl(hasParamA.bind(\"parmABinding\"))";
     llvm::Optional<DynTypedMatcher> NestedLetBinding(
-        Parser::parseMatcherExpression(
+        Parser::parseMatcherExpression(Code, nullptr, &NamedValues, &Error));
-            "functionDecl(hasParamA.bind(\"parmABinding\"))", nullptr,
-            &NamedValues, &Error));
     EXPECT_EQ("", Error.toStringFull());
     auto M = NestedLetBinding->unconditionalConvertTo<Decl>();