[clang] 88e6208 - [libTooling] Update Transformer's `node` combinator to include the trailing semicolon for decls.

[Yitzhak Mandelbaum via cfe-commits]

Author: Yitzhak Mandelbaum
Date: 2020-11-20T18:11:50Z
New Revision: 88e62085624e7ec55bd4a41c6d77acdcb7f1d113

URL: https://github.com/llvm/llvm-project/commit/88e62085624e7ec55bd4a41c6d77acdcb7f1d113
DIFF: https://github.com/llvm/llvm-project/commit/88e62085624e7ec55bd4a41c6d77acdcb7f1d113.diff

LOG: [libTooling] Update Transformer's `node` combinator to include the trailing semicolon for decls.

Currently, `node` only includes the semicolon for (some) statements. However,
declarations have the same issue of (potentially) trailing semicolons, so `node`
should behave the same for them.

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

Added: 
    

Modified: 
    clang-tools-extra/unittests/clang-tidy/TransformerClangTidyCheckTest.cpp
    clang/include/clang/Tooling/Transformer/RangeSelector.h
    clang/lib/Tooling/Transformer/RangeSelector.cpp
    clang/unittests/Tooling/TransformerTest.cpp

Removed: 
    


################################################################################
diff  --git a/clang-tools-extra/unittests/clang-tidy/TransformerClangTidyCheckTest.cpp b/clang-tools-extra/unittests/clang-tidy/TransformerClangTidyCheckTest.cpp
index 76de7711cee9..536d6f8ef275 100644
--- a/clang-tools-extra/unittests/clang-tidy/TransformerClangTidyCheckTest.cpp
+++ b/clang-tools-extra/unittests/clang-tidy/TransformerClangTidyCheckTest.cpp
@@ -148,7 +148,7 @@ Optional<RewriteRule> noSkip(const LangOptions &LangOpts,
   if (Options.get("Skip", "false") == "true")
     return None;
   return tooling::makeRule(clang::ast_matchers::functionDecl(),
-                           change(cat("void nothing()")), cat("no message"));
+                           changeTo(cat("void nothing();")), cat("no message"));
 }
 
 class ConfigurableCheck : public TransformerClangTidyCheck {

diff  --git a/clang/include/clang/Tooling/Transformer/RangeSelector.h b/clang/include/clang/Tooling/Transformer/RangeSelector.h
index e070c0e7e2e6..c6ca193123d1 100644
--- a/clang/include/clang/Tooling/Transformer/RangeSelector.h
+++ b/clang/include/clang/Tooling/Transformer/RangeSelector.h
@@ -61,8 +61,8 @@ inline RangeSelector between(RangeSelector R1, RangeSelector R2) {
   return enclose(after(std::move(R1)), before(std::move(R2)));
 }
 
-/// Selects a node, including trailing semicolon (for non-expression
-/// statements). \p ID is the node's binding in the match result.
+/// Selects a node, including trailing semicolon, if any (for declarations and
+/// non-expression statements). \p ID is the node's binding in the match result.
 RangeSelector node(std::string ID);
 
 /// Selects a node, including trailing semicolon (always). Useful for selecting

diff  --git a/clang/lib/Tooling/Transformer/RangeSelector.cpp b/clang/lib/Tooling/Transformer/RangeSelector.cpp
index ce6f5fb9b444..0f3138db218a 100644
--- a/clang/lib/Tooling/Transformer/RangeSelector.cpp
+++ b/clang/lib/Tooling/Transformer/RangeSelector.cpp
@@ -142,7 +142,8 @@ RangeSelector transformer::node(std::string ID) {
     Expected<DynTypedNode> Node = getNode(Result.Nodes, ID);
     if (!Node)
       return Node.takeError();
-    return Node->get<Stmt>() != nullptr && Node->get<Expr>() == nullptr
+    return (Node->get<Decl>() != nullptr ||
+            (Node->get<Stmt>() != nullptr && Node->get<Expr>() == nullptr))
                ? tooling::getExtendedRange(*Node, tok::TokenKind::semi,
                                            *Result.Context)
                : CharSourceRange::getTokenRange(Node->getSourceRange());

diff  --git a/clang/unittests/Tooling/TransformerTest.cpp b/clang/unittests/Tooling/TransformerTest.cpp
index 773420c015cd..e4fcc210782f 100644
--- a/clang/unittests/Tooling/TransformerTest.cpp
+++ b/clang/unittests/Tooling/TransformerTest.cpp
@@ -1353,7 +1353,7 @@ void instantiate()
 
   // Changes the 'int' in 'S', but not the 'T' in 'TemplStruct':
   testRule(makeRule(traverse(TK_IgnoreUnlessSpelledInSource, MatchedField),
-                    changeTo(cat("safe_int ", name("theField")))),
+                    changeTo(cat("safe_int ", name("theField"), ";"))),
            NonTemplatesInput + TemplatesInput,
            NonTemplatesExpected + TemplatesInput);
 
@@ -1378,7 +1378,7 @@ void instantiate()
 
   // Changes the 'int' in 'S', and (incorrectly) the 'T' in 'TemplStruct':
   testRule(makeRule(traverse(TK_AsIs, MatchedField),
-                    changeTo(cat("safe_int ", name("theField")))),
+                    changeTo(cat("safe_int ", name("theField"), ";"))),
 
            NonTemplatesInput + TemplatesInput,
            NonTemplatesExpected + IncorrectTemplatesExpected);
@@ -1589,7 +1589,7 @@ TEST_F(TransformerTest, MultipleFiles) {
                                            Changes[0].getReplacements());
   ASSERT_TRUE(static_cast<bool>(UpdatedCode))
       << "Could not update code: " << llvm::toString(UpdatedCode.takeError());
-  EXPECT_EQ(format(*UpdatedCode), format(R"cc(;)cc"));
+  EXPECT_EQ(format(*UpdatedCode), "");
 
   ASSERT_EQ(Changes[1].getFilePath(), "input.cc");
   EXPECT_THAT(Changes[1].getInsertedHeaders(), IsEmpty());
@@ -1598,8 +1598,7 @@ TEST_F(TransformerTest, MultipleFiles) {
       Source, Changes[1].getReplacements());
   ASSERT_TRUE(static_cast<bool>(UpdatedCode))
       << "Could not update code: " << llvm::toString(UpdatedCode.takeError());
-  EXPECT_EQ(format(*UpdatedCode), format(R"cc(#include "input.h"
-                        ;)cc"));
+  EXPECT_EQ(format(*UpdatedCode), format("#include \"input.h\"\n"));
 }
 
 TEST_F(TransformerTest, AddIncludeMultipleFiles) {