[SyntaxTree] Test the List API

[eduucaldas]

Apply arc lint patch

[eduucaldas]

• [SyntaxTree] Split TreeTest and ListTest testing fixtures.

[eduucaldas]

I made a separate class for the tests on Lists, as it didn't share any methods with the tests for Trees. What do you think about that?

Should I also put the tests for lists in a different file, even though TreeTest.cpp could mean test Tree.h?

[gribozavr2]

clang/include/clang/Tooling/Syntax/Tree.h

224

I'd slightly prefer "null" b/c "nul" refers to the ASCII character. Feel free to add more spaces to make columns line up :)

clang/unittests/Tooling/Syntax/TreeTest.cpp

130

155

Would llvm::interleaveComma help?

331

All of the tests above are testing getElementsAsNodesAndDelimiters -- could you put that into the test names? Or do you plan adding more assertions in the end? Or is that the only meaningful assertion? (I think not, we could also test the low-level non-typed function at least.)

[eduucaldas]

Answer code review.

[eduucaldas]

.

[eduucaldas]

clang/include/clang/Tooling/Syntax/Tree.h

224

Thanks for providing my solution to my crazyness ^^

clang/include/clang/Tooling/Syntax/Tree.h

public:
   };
 
   using Tree::Tree;
   static bool classof(const Node *N);
   /// Returns the elements and corresponding delimiters. Missing elements
   /// and delimiters are represented as null pointers.
   ///
   /// For example, in a separated list:
-  /// "a, b, c" <=> [("a", ","), ("b", ","), ("c", null)]
-  /// "a, , c" <=> [("a", ","), (null, ","), ("c", ",)]
-  /// "a, b," <=> [("a", ","), ("b", ","), (null, null)]
+  /// "a, b, c"  <=> [("a" , ","), ("b" , "," ), ("c" , null)]
+  /// "a,  , c"  <=> [("a" , ","), (null, "," ), ("c" , null)]
+  /// "a, b  c"  <=> [("a" , ","), ("b" , null), ("c" , null)]
+  /// "a, b,"    <=> [("a" , ","), ("b" , "," ), (null, null)]
   ///
   /// In a terminated or maybe-terminated list:
-  /// "a, b," <=> [("a", ","), ("b", ",")]
+  /// "a; b; c;" <=> [("a" , ";"), ("b" , ";" ), ("c" , ";" )]
+  /// "a;  ; c;" <=> [("a" , ";"), (null, ";" ), ("c" , ";" )]
+  /// "a; b  c;" <=> [("a" , ";"), ("b" , null), ("c" , ";" )]
+  /// "a; b; c"  <=> [("a" , ";"), ("b" , ";" ), ("c" , null)]
   std::vector<ElementAndDelimiter<Node>> getElementsAsNodesAndDelimiters();
 
   /// Returns the elements of the list. Missing elements are represented
   /// as null pointers in the same way as in the return value of
   /// `getElementsAsNodesAndDelimiters()`.
   std::vector<Node *> getElementsAsNodes();
 
   // These can't be implemented with the information we have!

clang/unittests/Tooling/Syntax/TreeTest.cpp

 //===- TreeTest.cpp ---------------------------------------------*- C++ -*-===//
 //
 // Part of the LLVM Project, under the Apache License v2.0 with LLVM Exceptions.
 // See https://llvm.org/LICENSE.txt for license information.
 // SPDX-License-Identifier: Apache-2.0 WITH LLVM-exception
 //
 //===----------------------------------------------------------------------===//
 
 #include "clang/Tooling/Syntax/Tree.h"
 #include "TreeTestBase.h"
 #include "clang/Tooling/Syntax/BuildTree.h"
+#include "clang/Tooling/Syntax/Nodes.h"
+#include "llvm/ADT/STLExtras.h"
 #include "gtest/gtest.h"
 
 using namespace clang;
 using namespace clang::syntax;
 
 namespace {
 
 class TreeTest : public SyntaxTreeTest {
TEST_P(TreeTest, LastLeaf) {
   std::vector<const Node *> Leafs = {createLeaf(*Arena, tok::l_paren),
                                      createLeaf(*Arena, tok::r_paren)};
   for (const auto *Tree : generateAllTreesWithShape(Leafs, {3u})) {
     ASSERT_TRUE(Tree->findLastLeaf() != nullptr);
     EXPECT_EQ(Tree->findLastLeaf()->getToken()->kind(), tok::r_paren);
   }
 }
 
+class ListTest : public SyntaxTreeTest {
+private:
+  std::string dumpQuotedTokensOrNull(const Node *N) {
+    return N ? "'" +
+                   StringRef(N->dumpTokens(Arena->getSourceManager()))
+                       .trim()
+                       .str() +
+                   "'"
+             : "null";
+  }
+
+protected:
+  std::string
+  dumpElementsAndDelimiters(ArrayRef<List::ElementAndDelimiter<Node>> EDs) {
+    std::string Storage;
+    llvm::raw_string_ostream OS(Storage);
+
+    OS << "[";
+
+    llvm::interleaveComma(
+        EDs, OS, [&OS, this](const List::ElementAndDelimiter<Node> &ED) {
+          OS << "(" << dumpQuotedTokensOrNull(ED.element) << ", "
+             << dumpQuotedTokensOrNull(ED.delimiter) << ")";
+        });
+
+    OS << "]";
+
+    return OS.str();
+  }
+
+  std::string dumpNodes(ArrayRef<Node *> Nodes) {
+    std::string Storage;
+    llvm::raw_string_ostream OS(Storage);
+
+    OS << "[";
+
+    llvm::interleaveComma(Nodes, OS, [&OS, this](const Node *N) {
+      OS << dumpQuotedTokensOrNull(N);
+    });
+
+    OS << "]";
+
+    return OS.str();
+  }
+};
+
+INSTANTIATE_TEST_CASE_P(TreeTests, ListTest,
+                        ::testing::ValuesIn(allTestClangConfigs()), );
+
+/// "a, b, c"  <=> [("a", ","), ("b", ","), ("c", null)]
+TEST_P(ListTest, List_Separated_WellFormed) {
+  buildTree("", GetParam());
+
+  // "a, b, c"
+  auto *List = dyn_cast<syntax::List>(syntax::createTree(
+      *Arena,
+      {
+          {createLeaf(*Arena, tok::identifier, "a"), NodeRole::ListElement},
+          {createLeaf(*Arena, tok::comma), NodeRole::ListDelimiter},
+          {createLeaf(*Arena, tok::identifier, "b"), NodeRole::ListElement},
+          {createLeaf(*Arena, tok::comma), NodeRole::ListDelimiter},
+          {createLeaf(*Arena, tok::identifier, "c"), NodeRole::ListElement},
+      },
+      NodeKind::CallArguments));
+
+  EXPECT_EQ(dumpElementsAndDelimiters(List->getElementsAsNodesAndDelimiters()),
+            "[('a', ','), ('b', ','), ('c', null)]");
+  EXPECT_EQ(dumpNodes(List->getElementsAsNodes()), "['a', 'b', 'c']");
+}
+
+/// "a,  , c"  <=> [("a", ","), (null, ","), ("c", null)]
+TEST_P(ListTest, List_Separated_MissingElement) {
+  buildTree("", GetParam());
+
+  // "a,  , c"
+  auto *List = dyn_cast<syntax::List>(syntax::createTree(
+      *Arena,
+      {
+          {createLeaf(*Arena, tok::identifier, "a"), NodeRole::ListElement},
+          {createLeaf(*Arena, tok::comma), NodeRole::ListDelimiter},
+          {createLeaf(*Arena, tok::comma), NodeRole::ListDelimiter},
+          {createLeaf(*Arena, tok::identifier, "c"), NodeRole::ListElement},
+      },
+      NodeKind::CallArguments));
+
+  EXPECT_EQ(dumpElementsAndDelimiters(List->getElementsAsNodesAndDelimiters()),
+            "[('a', ','), (null, ','), ('c', null)]");
+  EXPECT_EQ(dumpNodes(List->getElementsAsNodes()), "['a', null, 'c']");
+}
+
+/// "a, b  c"  <=> [("a", ","), ("b", null), ("c", null)]
+TEST_P(ListTest, List_Separated_MissingDelimiter) {
+  buildTree("", GetParam());
+
+  // "a, b  c"
+  auto *List = dyn_cast<syntax::List>(syntax::createTree(
+      *Arena,
+      {
+          {createLeaf(*Arena, tok::identifier, "a"), NodeRole::ListElement},
+          {createLeaf(*Arena, tok::comma), NodeRole::ListDelimiter},
+          {createLeaf(*Arena, tok::identifier, "b"), NodeRole::ListElement},
+          {createLeaf(*Arena, tok::identifier, "c"), NodeRole::ListElement},
+      },
+      NodeKind::CallArguments));
+
+  EXPECT_EQ(dumpElementsAndDelimiters(List->getElementsAsNodesAndDelimiters()),
+            "[('a', ','), ('b', null), ('c', null)]");
+  EXPECT_EQ(dumpNodes(List->getElementsAsNodes()), "['a', 'b', 'c']");
+}
+
+/// "a, b,"    <=> [("a", ","), ("b", ","), (null, null)]
+TEST_P(ListTest, List_Separated_MissingLastElement) {
+  buildTree("", GetParam());
+
+  // "a, b, c"
+  auto *List = dyn_cast<syntax::List>(syntax::createTree(
+      *Arena,
+      {
+          {createLeaf(*Arena, tok::identifier, "a"), NodeRole::ListElement},
+          {createLeaf(*Arena, tok::comma), NodeRole::ListDelimiter},
+          {createLeaf(*Arena, tok::identifier, "b"), NodeRole::ListElement},
+          {createLeaf(*Arena, tok::comma), NodeRole::ListDelimiter},
+      },
+      NodeKind::CallArguments));
+
+  EXPECT_EQ(dumpElementsAndDelimiters(List->getElementsAsNodesAndDelimiters()),
+            "[('a', ','), ('b', ','), (null, null)]");
+  EXPECT_EQ(dumpNodes(List->getElementsAsNodes()), "['a', 'b', null]");
+}
+
+/// "a:: b:: c::" <=> [("a", "::"), ("b", "::"), ("c", "::")]
+TEST_P(ListTest, List_Terminated_WellFormed) {
+  if (!GetParam().isCXX()) {
+    return;
+  }
+  buildTree("", GetParam());
+
+  // "a:: b:: c::"
+  auto *List = dyn_cast<syntax::List>(syntax::createTree(
+      *Arena,
+      {
+          {createLeaf(*Arena, tok::identifier, "a"), NodeRole::ListElement},
+          {createLeaf(*Arena, tok::coloncolon), NodeRole::ListDelimiter},
+          {createLeaf(*Arena, tok::identifier, "b"), NodeRole::ListElement},
+          {createLeaf(*Arena, tok::coloncolon), NodeRole::ListDelimiter},
+          {createLeaf(*Arena, tok::identifier, "c"), NodeRole::ListElement},
+          {createLeaf(*Arena, tok::coloncolon), NodeRole::ListDelimiter},
+      },
+      NodeKind::NestedNameSpecifier));
+
+  EXPECT_EQ(dumpElementsAndDelimiters(List->getElementsAsNodesAndDelimiters()),
+            "[('a', '::'), ('b', '::'), ('c', '::')]");
+  EXPECT_EQ(dumpNodes(List->getElementsAsNodes()), "['a', 'b', 'c']");
+}
+
+/// "a::  :: c::" <=> [("a", "::"), (null, "::"), ("c", "::")]
+TEST_P(ListTest, List_Terminated_MissingElement) {
+  if (!GetParam().isCXX()) {
+    return;
+  }
+  buildTree("", GetParam());
+
+  // "a:: b:: c::"
+  auto *List = dyn_cast<syntax::List>(syntax::createTree(
+      *Arena,
+      {
+          {createLeaf(*Arena, tok::identifier, "a"), NodeRole::ListElement},
+          {createLeaf(*Arena, tok::coloncolon), NodeRole::ListDelimiter},
+          {createLeaf(*Arena, tok::coloncolon), NodeRole::ListDelimiter},
+          {createLeaf(*Arena, tok::identifier, "c"), NodeRole::ListElement},
+          {createLeaf(*Arena, tok::coloncolon), NodeRole::ListDelimiter},
+      },
+      NodeKind::NestedNameSpecifier));
+
+  EXPECT_EQ(dumpElementsAndDelimiters(List->getElementsAsNodesAndDelimiters()),
+            "[('a', '::'), (null, '::'), ('c', '::')]");
+  EXPECT_EQ(dumpNodes(List->getElementsAsNodes()), "['a', null, 'c']");
+}
+
+/// "a:: b  c::" <=> [("a", "::"), ("b", null), ("c", "::")]
+TEST_P(ListTest, List_Terminated_MissingDelimiter) {
+  if (!GetParam().isCXX()) {
+    return;
+  }
+  buildTree("", GetParam());
+
+  // "a:: b  c::"
+  auto *List = dyn_cast<syntax::List>(syntax::createTree(
+      *Arena,
+      {
+          {createLeaf(*Arena, tok::identifier, "a"), NodeRole::ListElement},
+          {createLeaf(*Arena, tok::coloncolon), NodeRole::ListDelimiter},
+          {createLeaf(*Arena, tok::identifier, "b"), NodeRole::ListElement},
+          {createLeaf(*Arena, tok::identifier, "c"), NodeRole::ListElement},
+          {createLeaf(*Arena, tok::coloncolon), NodeRole::ListDelimiter},
+      },
+      NodeKind::NestedNameSpecifier));
+
+  EXPECT_EQ(dumpElementsAndDelimiters(List->getElementsAsNodesAndDelimiters()),
+            "[('a', '::'), ('b', null), ('c', '::')]");
+  EXPECT_EQ(dumpNodes(List->getElementsAsNodes()), "['a', 'b', 'c']");
+}
+
+/// "a:: b:: c"  <=> [("a", "::"), ("b", "::"), ("c", null)]
+TEST_P(ListTest, List_Terminated_MissingLastDelimiter) {
+  if (!GetParam().isCXX()) {
+    return;
+  }
+  buildTree("", GetParam());
+
+  // "a:: b:: c"
+  auto *List = dyn_cast<syntax::List>(syntax::createTree(
+      *Arena,
+      {
+          {createLeaf(*Arena, tok::identifier, "a"), NodeRole::ListElement},
+          {createLeaf(*Arena, tok::coloncolon), NodeRole::ListDelimiter},
+          {createLeaf(*Arena, tok::identifier, "b"), NodeRole::ListElement},
+          {createLeaf(*Arena, tok::coloncolon), NodeRole::ListDelimiter},
+          {createLeaf(*Arena, tok::identifier, "c"), NodeRole::ListElement},
+      },
+      NodeKind::NestedNameSpecifier));
+
+  EXPECT_EQ(dumpElementsAndDelimiters(List->getElementsAsNodesAndDelimiters()),
+            "[('a', '::'), ('b', '::'), ('c', null)]");
+  EXPECT_EQ(dumpNodes(List->getElementsAsNodes()), "['a', 'b', 'c']");
+}
+
 } // namespace