[Clang][Comments] Parse `<img src=""/>` in doc comments correctly

[egorzhdan]

Summary

This is a valid HTML5 tag. Previously it triggered a Clang error (HTML start tag prematurely ended, expected attribute name or '>') since Clang was treating /> as a text token. This was happening because after lexing the closing quote (") the lexer state was reset to "Normal" while the tag was not actually closed yet: > was not yet parsed at that point.

rdar://91464292

[gribozavr2]

clang/test/Sema/warn-documentation.cpp

78

Could you also add these tests to clang/test/Index/comment-to-html-xml-conversion.cpp to show that the whole tag is captured correctly? (for example, the ">" is correctly captured as a part of the tag, not as plain text)

[egorzhdan]

• Add tests for conversion of doc comments to HTML/XML
• Fix a bug in StringRef -> CXString conversion logic for an empty string

[egorzhdan]

clang/test/Sema/warn-documentation.cpp

78

Thanks, added a few tests there.

[gribozavr2]

clang/tools/libclang/CXString.cpp

85 ↗

(On Diff #456674)

Please split this change into a separate patch and add a unit test.

[egorzhdan]

clang/tools/libclang/CXString.cpp

85 ↗

(On Diff #456674)

Do you know a good way to add a unit-test for this? This function only gets called from within libclang, it isn't exposed to clients of libclang, including libclangTest. So I think a unit test would probably need to invoke the same CXComment API that c-index-test invokes while running comment-to-html-xml-conversion.cpp. Would that be worth it?

[gribozavr2]

clang/tools/libclang/CXString.cpp

85 ↗

(On Diff #456674)

It is unfortunate that the headers for CXString are not visible to llvm-project/clang/unittests/libclang/LibclangTest.cpp.

It wouldn't make sense to use the CXComment API because that's exactly what comment-to-html-xml-conversion.cpp does. A benefit of the unit test would be clear, direct testing of CXString.

Anyhow, please split the CXString change to a separate patch (without tests, unfortunately).

[egorzhdan]

Rebase to apply fixes to string conversion

clang/lib/AST/CommentLexer.cpp

void Lexer::lexHTMLStartTag(Token &T) {
   BufferPtr = skipWhitespace(BufferPtr, CommentEnd);
   if (BufferPtr == CommentEnd) {
     State = LS_Normal;
     return;
   }
 
   C = *BufferPtr;
   if (!isHTMLIdentifierStartingCharacter(C) &&
-      C != '=' && C != '\"' && C != '\'' && C != '>') {
+      C != '=' && C != '\"' && C != '\'' && C != '>' && C != '/') {
     State = LS_Normal;
     return;
   }
 }
 
 void Lexer::setupAndLexHTMLEndTag(Token &T) {
   assert(BufferPtr[0] == '<' && BufferPtr[1] == '/');
 

clang/test/Index/comment-to-html-xml-conversion.cpp

 
 // CHECK: comment-to-html-xml-conversion.cpp:[[@LINE-2]]:6: FunctionDecl=comment_to_html_conversion_37:{{.*}} FullCommentAsHTML=[<p class="para-brief"> <span id="A"></span></p>] FullCommentAsXML=[<Function file="{{[^"]+}}comment-to-html-xml-conversion.cpp" line="[[@LINE-2]]" column="6"><Name>comment_to_html_conversion_37</Name><USR>c:@F@comment_to_html_conversion_37#</USR><Declaration>void comment_to_html_conversion_37()</Declaration><Abstract><Para> <anchor id="A"></anchor></Para></Abstract></Function>]
 // CHECK-NEXT:  CommentAST=[
 // CHECK-NEXT:    (CXComment_FullComment
 // CHECK-NEXT:       (CXComment_Paragraph
 // CHECK-NEXT:         (CXComment_Text Text=[ ] IsWhitespace)
 // CHECK-NEXT:         (CXComment_InlineCommand CommandName=[anchor] RenderAnchor Arg[0]=A)))]
 
+/// Aaa bbb<img/>
+void comment_to_html_conversion_38();
+
+// CHECK: comment-to-html-xml-conversion.cpp:[[@LINE-2]]:6: FunctionDecl=comment_to_html_conversion_38:{{.*}} FullCommentAsHTML=[<p class="para-brief"> Aaa bbb<img/></p>] FullCommentAsXML=[<Function file="{{[^"]+}}comment-to-html-xml-conversion.cpp" line="[[@LINE-2]]" column="6"><Name>comment_to_html_conversion_38</Name><USR>c:@F@comment_to_html_conversion_38#</USR><Declaration>void comment_to_html_conversion_38()</Declaration><Abstract><Para> Aaa bbb<rawHTML><![CDATA[<img/>]]></rawHTML></Para></Abstract></Function>]
+// CHECK-NEXT:  CommentAST=[
+// CHECK-NEXT:    (CXComment_FullComment
+// CHECK-NEXT:       (CXComment_Paragraph
+// CHECK-NEXT:         (CXComment_Text Text=[ Aaa bbb])
+// CHECK-NEXT:         (CXComment_HTMLStartTag Name=[img] SelfClosing)
+
+/// Aaa ccc<img />
+void comment_to_html_conversion_39();
+
+// CHECK: comment-to-html-xml-conversion.cpp:[[@LINE-2]]:6: FunctionDecl=comment_to_html_conversion_39:{{.*}} FullCommentAsHTML=[<p class="para-brief"> Aaa ccc<img/></p>] FullCommentAsXML=[<Function file="{{[^"]+}}comment-to-html-xml-conversion.cpp" line="[[@LINE-2]]" column="6"><Name>comment_to_html_conversion_39</Name><USR>c:@F@comment_to_html_conversion_39#</USR><Declaration>void comment_to_html_conversion_39()</Declaration><Abstract><Para> Aaa ccc<rawHTML><![CDATA[<img/>]]></rawHTML></Para></Abstract></Function>]
+// CHECK-NEXT:  CommentAST=[
+// CHECK-NEXT:    (CXComment_FullComment
+// CHECK-NEXT:       (CXComment_Paragraph
+// CHECK-NEXT:         (CXComment_Text Text=[ Aaa ccc])
+// CHECK-NEXT:         (CXComment_HTMLStartTag Name=[img] SelfClosing)
+
 /// Aaa ccc<img src="">
 void comment_to_html_conversion_40();
 
 // CHECK: comment-to-html-xml-conversion.cpp:[[@LINE-2]]:6: FunctionDecl=comment_to_html_conversion_40:{{.*}} FullCommentAsHTML=[<p class="para-brief"> Aaa ccc<img src></p>] FullCommentAsXML=[<Function file="{{[^"]+}}comment-to-html-xml-conversion.cpp" line="[[@LINE-2]]" column="6"><Name>comment_to_html_conversion_40</Name><USR>c:@F@comment_to_html_conversion_40#</USR><Declaration>void comment_to_html_conversion_40()</Declaration><Abstract><Para> Aaa ccc<rawHTML><![CDATA[<img src>]]></rawHTML></Para></Abstract></Function>]
 // CHECK-NEXT:  CommentAST=[
 // CHECK-NEXT:    (CXComment_FullComment
 // CHECK-NEXT:       (CXComment_Paragraph
 // CHECK-NEXT:         (CXComment_Text Text=[ Aaa ccc])
 // CHECK-NEXT:         (CXComment_HTMLStartTag Name=[img] Attrs: src=)
 
+/// Aaa ccc<img src="path">
+void comment_to_html_conversion_41();
+
+// CHECK: comment-to-html-xml-conversion.cpp:[[@LINE-2]]:6: FunctionDecl=comment_to_html_conversion_41:{{.*}} FullCommentAsHTML=[<p class="para-brief"> Aaa ccc<img src="path"></p>] FullCommentAsXML=[<Function file="{{[^"]+}}comment-to-html-xml-conversion.cpp" line="[[@LINE-2]]" column="6"><Name>comment_to_html_conversion_41</Name><USR>c:@F@comment_to_html_conversion_41#</USR><Declaration>void comment_to_html_conversion_41()</Declaration><Abstract><Para> Aaa ccc<rawHTML><![CDATA[<img src="path">]]></rawHTML></Para></Abstract></Function>]
+// CHECK-NEXT:  CommentAST=[
+// CHECK-NEXT:    (CXComment_FullComment
+// CHECK-NEXT:       (CXComment_Paragraph
+// CHECK-NEXT:         (CXComment_Text Text=[ Aaa ccc])
+// CHECK-NEXT:         (CXComment_HTMLStartTag Name=[img] Attrs: src=path)
+
+/// Aaa ccc<img src="path"/>
+void comment_to_html_conversion_42();
+
+// CHECK: comment-to-html-xml-conversion.cpp:[[@LINE-2]]:6: FunctionDecl=comment_to_html_conversion_42:{{.*}} FullCommentAsHTML=[<p class="para-brief"> Aaa ccc<img src="path"/></p>] FullCommentAsXML=[<Function file="{{[^"]+}}comment-to-html-xml-conversion.cpp" line="[[@LINE-2]]" column="6"><Name>comment_to_html_conversion_42</Name><USR>c:@F@comment_to_html_conversion_42#</USR><Declaration>void comment_to_html_conversion_42()</Declaration><Abstract><Para> Aaa ccc<rawHTML><![CDATA[<img src="path"/>]]></rawHTML></Para></Abstract></Function>]
+// CHECK-NEXT:  CommentAST=[
+// CHECK-NEXT:    (CXComment_FullComment
+// CHECK-NEXT:       (CXComment_Paragraph
+// CHECK-NEXT:         (CXComment_Text Text=[ Aaa ccc])
+// CHECK-NEXT:         (CXComment_HTMLStartTag Name=[img] Attrs: src=path SelfClosing)
+
+/// Aaa ddd<img src=""/>
+void comment_to_html_conversion_43();
+
+// CHECK: comment-to-html-xml-conversion.cpp:[[@LINE-2]]:6: FunctionDecl=comment_to_html_conversion_43:{{.*}} FullCommentAsHTML=[<p class="para-brief"> Aaa ddd<img src/></p>] FullCommentAsXML=[<Function file="{{[^"]+}}comment-to-html-xml-conversion.cpp" line="[[@LINE-2]]" column="6"><Name>comment_to_html_conversion_43</Name><USR>c:@F@comment_to_html_conversion_43#</USR><Declaration>void comment_to_html_conversion_43()</Declaration><Abstract><Para> Aaa ddd<rawHTML><![CDATA[<img src/>]]></rawHTML></Para></Abstract></Function>]
+// CHECK-NEXT:  CommentAST=[
+// CHECK-NEXT:    (CXComment_FullComment
+// CHECK-NEXT:       (CXComment_Paragraph
+// CHECK-NEXT:         (CXComment_Text Text=[ Aaa ddd])
+// CHECK-NEXT:         (CXComment_HTMLStartTag Name=[img] Attrs: src= SelfClosing)
+
 /// Aaa.
 class comment_to_xml_conversion_01 {
 // CHECK: comment-to-html-xml-conversion.cpp:[[@LINE-1]]:7: ClassDecl=comment_to_xml_conversion_01:{{.*}} FullCommentAsXML=[<Class file="{{[^"]+}}comment-to-html-xml-conversion.cpp" line="[[@LINE-1]]" column="7"><Name>comment_to_xml_conversion_01</Name><USR>c:@S@comment_to_xml_conversion_01</USR><Declaration>class comment_to_xml_conversion_01 {}</Declaration><Abstract><Para> Aaa.</Para></Abstract></Class>]
 
   /// \param aaa Blah blah.
   comment_to_xml_conversion_01(int aaa);
 
 // CHECK: comment-to-html-xml-conversion.cpp:[[@LINE-2]]:3: CXXConstructor=comment_to_xml_conversion_01:{{.*}} FullCommentAsXML=[<Function isInstanceMethod="1" file="{{[^"]+}}comment-to-html-xml-conversion.cpp" line="[[@LINE-2]]" column="3"><Name>comment_to_xml_conversion_01</Name><USR>c:@S@comment_to_xml_conversion_01@F@comment_to_xml_conversion_01#I#</USR><Declaration>comment_to_xml_conversion_01(int aaa)</Declaration><Parameters><Parameter><Name>aaa</Name><Index>0</Index><Direction isExplicit="0">in</Direction><Discussion><Para> Blah blah.</Para></Discussion></Parameter></Parameters></Function>]

clang/test/Sema/warn-documentation.cpp

  * fff ggg.
  */
 int test_html10(int);
 
 // expected-warning@+1 {{HTML end tag 'br' is forbidden}}
 /// <br></br>
 int test_html11(int);
 
+/// Aaa bbb<img/>
+int test_html12(int);
+
+/// Aaa bbb<img />
+int test_html13(int);
+
+/// Aaa bbb<img src="">
+int test_html14(int);
+
+/// Aaa bbb<img src=""/>
+int test_html15(int);
+
+/// Aaa bbb<img src="" />
+int test_html16(int);
+
 /// <blockquote>Meow</blockquote>
 int test_html_nesting1(int);
 
 /// <b><i>Meow</i></b>
 int test_html_nesting2(int);
 
 /// <p>Aaa<br>
 /// Bbb</p>