Update macro call arg tests with comment test cases.

Filed bug for the one formatter_test test case that is still wrong. issues #225 PiperOrigin-RevId: 302728312
chipsalliance · Mar 24, 2020 · 0dac7a3 · 0dac7a3
1 parent 67beaba
commit 0dac7a3
Show file tree

Hide file tree

Showing 3 changed files with 24 additions and 11 deletions.
diff --git a/verilog/formatting/formatter_test.cc b/verilog/formatting/formatter_test.cc
@@ -343,10 +343,15 @@ static const std::initializer_list<FormatterTestCase> kFormatterTestCases = {
      "    aa,  //bb\n"
      "    bb,  // cc\n"
      "    cc)\n"},
-    {// macro call with argument including comment
-     "`FOO(aa, bb//cc\ndd)\n",
-     "`FOO(aa, bb//cc\n"
-     "dd)\n"},  // TODO(fangism): Improve formatting arguments with comments
+    {// macro call with argument including trailing EOL comment
+     "`FOO(aa, bb,//cc\ndd)\n",
+     "`FOO(aa, bb,  //cc\n"
+     "     dd)\n"},
+    {// macro call with argument including EOL comment on own line
+     "`FOOOO(aa, bb,\n//cc\ndd)\n",
+     "`FOOOO(aa, bb,  //cc\n"
+     // TODO(b/152324712): //cc comment should start its own line
+     "       dd)\n"},
     {"  // leading comment\n"
      "  `define   FOO    \\\n"  // multiline macro definition
      "1\n"

diff --git a/verilog/formatting/tree_unwrapper.cc b/verilog/formatting/tree_unwrapper.cc
@@ -1506,6 +1506,7 @@ void TreeUnwrapper::Visit(const verible::SyntaxTreeLeaf& leaf) {
       // Pick one, even if both work.
       if (current_context_.DirectParentIsOneOf({
               // NodeEnum:xxxx                             // due to element:
+              NodeEnum::kMacroArgList,         // MacroArg
               NodeEnum::kPortDeclarationList,  // kPort, kPortDeclaration
               NodeEnum::kPortActualList,       // kActualNamedPort,
                                                // kActualPositionalPort

diff --git a/verilog/formatting/tree_unwrapper_test.cc b/verilog/formatting/tree_unwrapper_test.cc
@@ -3349,24 +3349,31 @@ const TreeUnwrapperTestData kUnwrapPreprocessorTestCases[] = {
         "macro call with comments in argument list",
         "`FOO(aa, //aa\nbb , // bb\ncc)\n",
         N(0, L(0, {"`FOO", "("}),
-          N(2, L(2, {"aa"}), L(2, {",", "//aa"}), L(2, {"bb"}),
-            L(2, {",", "// bb"}), L(2, {"cc", ")"}))),
+          N(2,                           //
+            L(2, {"aa", ",", "//aa"}),   //
+            L(2, {"bb", ",", "// bb"}),  //
+            L(2, {"cc", ")"}))),
     },
 
     {
         "macro call with comment before first argument",
         "`FOO(// aa\naa, // bb\nbb, // cc\ncc)\n",
         N(0, L(0, {"`FOO", "(", "// aa"}),
-          N(2, L(2, {"aa"}), L(2, {",", "// bb"}), L(2, {"bb"}),
-            L(2, {",", "// cc"}), L(2, {"cc", ")"}))),
+          N(2,                           //
+            L(2, {"aa", ",", "// bb"}),  //
+            L(2, {"bb", ",", "// cc"}),  //
+            L(2, {"cc", ")"}))),
     },
 
     {
-        // TODO(fangism): Improve formatting arguments with comments
         "macro call with argument including comment",
-        "`FOO(aa, bb // cc\ndd)\n",
+        "`FOO(aa, bb,\n// cc\ndd)\n",
         N(0, L(0, {"`FOO", "("}),
-          N(2, L(2, {"aa", ","}), L(2, {"bb // cc\ndd", ")"}))),
+          N(2,                  //
+            L(2, {"aa", ","}),  //
+            L(2, {"bb", ","}),  //
+            L(2, {"// cc"}),    // indented to same level as surrounding args
+            L(2, {"dd", ")"}))),
     },
 
     {