[clang] 50bdd60 - [clang-format] [PR46130] When editing a file with unbalance {} the namespace comment fixer can incorrectly comment the wrong closing brace

[via cfe-commits]

Author: mydeveloperday
Date: 2020-05-30T13:15:27+01:00
New Revision: 50bdd60731130dbde81fa477ba8916c58039d73a

URL: https://github.com/llvm/llvm-project/commit/50bdd60731130dbde81fa477ba8916c58039d73a
DIFF: https://github.com/llvm/llvm-project/commit/50bdd60731130dbde81fa477ba8916c58039d73a.diff

LOG: [clang-format] [PR46130] When editing a file with unbalance {} the namespace comment fixer can incorrectly comment the wrong closing brace

Summary:
https://bugs.llvm.org/show_bug.cgi?id=46130   from Twitter https://twitter.com/ikautak/status/1265998988232159232

I have seen this myself many times.. if you have format on save and you work in an editor where you are constantly saving (:w muscle memory)

If you are in the middle of editing and somehow you've missed a { or } in your code, somewhere, often way below where you are at the bottom of your file the namespace comment fixer will have put the namespace on the previous closing brace.

This leads to you having to fix up the bottom of the file.

This revision prevents that happening by performing an initial pass of the tokens and simply counting the number of `{` and `}`  and ensuring they balance.

If they don't balance we don't do any namespace fixing as it will likely be unstable and incorrect.

Reviewed By: curdeius

Subscribers: cfe-commits

Tags: #clang, #clang-format

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

Added: 
    

Modified: 
    clang/lib/Format/NamespaceEndCommentsFixer.cpp
    clang/unittests/Format/NamespaceEndCommentsFixerTest.cpp

Removed: 
    


################################################################################
diff  --git a/clang/lib/Format/NamespaceEndCommentsFixer.cpp b/clang/lib/Format/NamespaceEndCommentsFixer.cpp
index 92707150fcdb..97de45bd1965 100644
--- a/clang/lib/Format/NamespaceEndCommentsFixer.cpp
+++ b/clang/lib/Format/NamespaceEndCommentsFixer.cpp
@@ -205,6 +205,23 @@ std::pair<tooling::Replacements, unsigned> NamespaceEndCommentsFixer::analyze(
   const SourceManager &SourceMgr = Env.getSourceManager();
   AffectedRangeMgr.computeAffectedLines(AnnotatedLines);
   tooling::Replacements Fixes;
+
+  // Spin through the lines and ensure we have balanced braces.
+  int Braces = 0;
+  for (size_t I = 0, E = AnnotatedLines.size(); I != E; ++I) {
+    FormatToken *Tok = AnnotatedLines[I]->First;
+    while (Tok) {
+      Braces += Tok->is(tok::l_brace) ? 1 : Tok->is(tok::r_brace) ? -1 : 0;
+      Tok = Tok->Next;
+    }
+  }
+  // Don't attempt to comment unbalanced braces or this can
+  // lead to comments being placed on the closing brace which isn't
+  // the matching brace of the namespace. (occurs during incomplete editing).
+  if (Braces != 0) {
+    return {Fixes, 0};
+  }
+
   std::string AllNamespaceNames = "";
   size_t StartLineIndex = SIZE_MAX;
   StringRef NamespaceTokenText;

diff  --git a/clang/unittests/Format/NamespaceEndCommentsFixerTest.cpp b/clang/unittests/Format/NamespaceEndCommentsFixerTest.cpp
index fee8597b4330..463afa67e8b0 100644
--- a/clang/unittests/Format/NamespaceEndCommentsFixerTest.cpp
+++ b/clang/unittests/Format/NamespaceEndCommentsFixerTest.cpp
@@ -1089,6 +1089,34 @@ TEST_F(NamespaceEndCommentsFixerTest, HandlesInlineAtEndOfLine_PR32438) {
                                     "void d() {\n"
                                     "}\n"));
 }
+
+TEST_F(NamespaceEndCommentsFixerTest, IgnoreUnbalanced) {
+  EXPECT_EQ("namespace A {\n"
+            "class Foo {\n"
+            "}\n"
+            "}// namespace A\n",
+            fixNamespaceEndComments("namespace A {\n"
+                                    "class Foo {\n"
+                                    "}\n"
+                                    "}\n"));
+  EXPECT_EQ("namespace A {\n"
+            "class Foo {\n"
+            "}\n",
+            fixNamespaceEndComments("namespace A {\n"
+                                    "class Foo {\n"
+                                    "}\n"));
+
+  EXPECT_EQ("namespace A {\n"
+            "class Foo {\n"
+            "}\n"
+            "}\n"
+            "}\n",
+            fixNamespaceEndComments("namespace A {\n"
+                                    "class Foo {\n"
+                                    "}\n"
+                                    "}\n"
+                                    "}\n"));
+}
 } // end namespace
 } // end namespace format
 } // end namespace clang