Comment parsing: Don't recognize commands in single-line double quotation

[aaronpuchert]

Summary

This is consistent with the behavior of Doxygen, and allows users to
write strings with C escapes or document input/output formats containing
special characters (@ or \) without escaping them, which might be
confusing. For example, if a function wants to document its expected
input format as "user@host" it doesn't have to write user\@host instead,
which would look right in the documentation but confusing in the code.
Now users can just use double quotes (which they might do anyway).

This fixes a lot of false positives of -Wdocumentation-unknown-command,
but it could also fix issues with -Wdocumentation if the text triggers
an actual command.

[aaronpuchert]

Ping @gribozavr2.

[gribozavr2]

clang/lib/AST/CommentLexer.cpp

278

I'd suggest refactoring to a while (true) if you don't mind.

290

Does Doxygen understand escaped quotes?

/// "@hello\"@hello"

[aaronpuchert]

clang/lib/AST/CommentLexer.cpp

278

I was imitating lexVerbatimBlockFirstLine, but I don't mind either way.

290

Vim syntax highlighting suggests it does, but at least my version actually doesn't: "@c a \" @c b" is translated to "@c a \" <code>b"</code>. So the escaped quote seems to end the quotation.

485

This is what I was imitating.

[gribozavr2]

clang/lib/AST/CommentLexer.cpp

278

I see. No worries then.

290

Got it, thanks for checking!

clang/include/clang/AST/CommentLexer.h

private:
 
   DiagnosticBuilder Diag(SourceLocation Loc, unsigned DiagID) {
     return Diags.Report(Loc, DiagID);
   }
 
   /// Eat string matching regexp \code \s*\* \endcode.
   void skipLineStartingDecorations();
 
+  /// Skip over pure text.
+  const char *skipTextToken();
+
   /// Lex comment text, including commands if ParseCommands is set to true.
   void lexCommentText(Token &T);
 
   void setupAndLexVerbatimBlock(Token &T, const char *TextBegin, char Marker,
                                 const CommandInfo *Info);
 
   void lexVerbatimBlockFirstLine(Token &T);
 

clang/lib/AST/CommentLexer.cpp

void Lexer::formTokenWithChars(Token &Result, const char *TokEnd,
   Result.setLength(TokLen);
 #ifndef NDEBUG
   Result.TextPtr = "<UNSET>";
   Result.IntVal = 7;
 #endif
   BufferPtr = TokEnd;
 }
 
+const char *Lexer::skipTextToken() {
+  const char *TokenPtr = BufferPtr;
+  assert(TokenPtr < CommentEnd);
+  StringRef TokStartSymbols = ParseCommands ? "\n\r\\@\"&<" : "\n\r";
+
+again:
+  size_t End =
+      StringRef(TokenPtr, CommentEnd - TokenPtr).find_first_of(TokStartSymbols);
+  if (End == StringRef::npos)
+    return CommentEnd;
+
+  // Doxygen doesn't recognize any commands in a one-line double quotation.
+  // If we don't find an ending quotation mark, we pretend it never began.
+  if (*(TokenPtr + End) == '\"') {
+    TokenPtr += End + 1;
+    End = StringRef(TokenPtr, CommentEnd - TokenPtr).find_first_of("\n\r\"");
+    if (End != StringRef::npos && *(TokenPtr + End) == '\"')
+      TokenPtr += End + 1;
+    goto again;
+  }
+  return TokenPtr + End;
+}
+
 void Lexer::lexCommentText(Token &T) {
   assert(CommentState == LCS_InsideBCPLComment ||
          CommentState == LCS_InsideCComment);
 
   // Handles lexing non-command text, i.e. text and newline.
   auto HandleNonCommandToken = [&]() -> void {
     assert(State == LS_Normal);
 
     const char *TokenPtr = BufferPtr;
     assert(TokenPtr < CommentEnd);
     switch (*TokenPtr) {
       case '\n':
       case '\r':
           TokenPtr = skipNewline(TokenPtr, CommentEnd);
           formTokenWithChars(T, TokenPtr, tok::newline);
 
           if (CommentState == LCS_InsideCComment)
             skipLineStartingDecorations();
           return;
 
-      default: {
-          StringRef TokStartSymbols = ParseCommands ? "\n\r\\@&<" : "\n\r";
-          size_t End = StringRef(TokenPtr, CommentEnd - TokenPtr)
-                           .find_first_of(TokStartSymbols);
-          if (End != StringRef::npos)
-            TokenPtr += End;
-          else
-            TokenPtr = CommentEnd;
-          formTextToken(T, TokenPtr);
-          return;
-      }
+      default:
+        return formTextToken(T, skipTextToken());
     }
   };
 
   if (!ParseCommands)
     return HandleNonCommandToken();
 
   switch (State) {
   case LS_Normal:
if (BufferPtr != CommentEnd &&
     State = LS_VerbatimBlockBody;
     return;
   }
 
   State = LS_VerbatimBlockFirstLine;
 }
 
 void Lexer::lexVerbatimBlockFirstLine(Token &T) {
 again:
   assert(BufferPtr < CommentEnd);
 
   // FIXME: It would be better to scan the text once, finding either the block
   // end command or newline.
   //
   // Extract current line.
   const char *Newline = findNewline(BufferPtr, CommentEnd);
   StringRef Line(BufferPtr, Newline - BufferPtr);

clang/test/Sema/warn-documentation-unknown-command.cpp

 // RUN: %clang_cc1 -std=c++11 -fsyntax-only -Wdocumentation-unknown-command -verify %s
 // RUN: %clang_cc1 -std=c++11 -fsyntax-only -Werror -Wno-documentation-unknown-command %s
 
 // expected-warning@+1 {{unknown command tag name}}
 /// aaa \unknown
 int test_unknown_comand_1;
 
 // expected-warning@+1 {{unknown command tag name 'retur'; did you mean 'return'?}}
 /// \retur aaa
 int test_unknown_comand_2();
 
+/// We don't recognize commands in double quotes: "\n\t @unknown2".
+int test_unknown_comand_3();
+
+// expected-warning@+2 {{unknown command tag name}}
+// expected-warning@+2 {{unknown command tag name}}
+/// But it has to be a single line: "\unknown3
+/// @unknown4" (Doxygen treats multi-line quotes inconsistently.)
+int test_unknown_comand_4();
+
 // RUN: c-index-test -test-load-source all -Wdocumentation-unknown-command %s > /dev/null 2> %t.err
 // RUN: FileCheck < %t.err -check-prefix=CHECK-RANGE %s
 // CHECK-RANGE: warn-documentation-unknown-command.cpp:5:9:{5:9-5:17}: warning: unknown command tag name
 // CHECK-RANGE: warn-documentation-unknown-command.cpp:9:5:{9:5-9:11}: warning: unknown command tag name 'retur'; did you mean 'return'?

clang/test/Sema/warn-documentation.cpp

 /// \brief
 /// 
 /// \returns Aaa
 int test_block_command5(int);
 
 /// \brief \c Aaa
 int test_block_command6(int);
 
+// We don't recognize comments in double quotes.
+/// "\brief \returns Aaa"
+int test_block_command7(int);
+
+// But only if they're single-line. (Doxygen treats multi-line quotes inconsistently.)
+// expected-warning@+1 {{empty paragraph passed to '\brief' command}}
+/// "\brief
+/// \returns Aaa"
+int test_block_command8(int);
+
 // expected-warning@+5 {{duplicated command '\brief'}} expected-note@+1 {{previous command '\brief' here}}
 /// \brief Aaa
 ///
 /// Bbb
 ///
 /// \brief Ccc
 int test_duplicate_brief1(int);