clang-format: [JS] do not clean up duplicated commas.

[mprobst]

Summary

In JavaScript, duplicated commas have semantic meaning.

x = [a,,b];

The statement above creates an array with three entries, the middle being undefined. Because clang-format should not change semantics, disable this cleanup in JS.

[mprobst]

• strip debug statements

[djasper]

lib/Format/Format.cpp

1085

As all the cleanups that aren't comma clean ups are cleanups for constructor initializers, I think we should just completely opt out of cleans for JavaScript for now?

[mprobst]

• disable all cleanups in JS.

[mprobst]

lib/Format/Format.cpp

1085

Agreed, done.

[djasper]

lib/Format/Format.cpp

1924

Maybe we should actually use Style.Language.isCpp(). These cleanups aren't tested for any other language.

[mprobst]

• only clean up C++

lib/Format/Format.cpp

analyze(TokenAnnotator &Annotator,
     // To determine if some redundant code is actually introduced by
     // replacements(e.g. deletions), we need to come up with a more
     // sophisticated way of computing affected ranges.
     AffectedRangeMgr.computeAffectedLines(AnnotatedLines.begin(),
                                           AnnotatedLines.end());
 
     checkEmptyNamespace(AnnotatedLines);
 
+    llvm::dbgs() << "Language style is " << Style.Language << "\n";
+
+    // Trailing or duplicated commas have semantic meaning in JS, e.g.
+    //     x = [a,,b];
+    // creates an array with three elements, one of them undefined.
+    bool CleanUpCommas = Style.Language != FormatStyle::LK_JavaScript;
+
     for (auto &Line : AnnotatedLines) {
       if (Line->Affected) {
+        if (CleanUpCommas)
           cleanupRight(Line->First, tok::comma, tok::comma);
         cleanupRight(Line->First, TT_CtorInitializerColon, tok::comma);
+        if (CleanUpCommas) {
           cleanupRight(Line->First, tok::l_paren, tok::comma);
           cleanupLeft(Line->First, tok::comma, tok::r_paren);
+        }
         cleanupLeft(Line->First, TT_CtorInitializerComma, tok::l_brace);
         cleanupLeft(Line->First, TT_CtorInitializerColon, tok::l_brace);
         cleanupLeft(Line->First, TT_CtorInitializerColon, tok::equal);
       }
     }
 
     return generateFixes();
   }
tooling::Replacements reformat(const FormatStyle &Style, StringRef Code,
   Formatter Format(*Env, Expanded, Status);
   return Format.process();
 }
 
 tooling::Replacements cleanup(const FormatStyle &Style, StringRef Code,
                               ArrayRef<tooling::Range> Ranges,
                               StringRef FileName) {
   std::unique_ptr<Environment> Env =
       Environment::CreateVirtualEnvironment(Code, FileName, Ranges);
   Cleaner Clean(*Env, Style);
   return Clean.process();
 }
 
 tooling::Replacements reformat(const FormatStyle &Style, StringRef Code,
                                ArrayRef<tooling::Range> Ranges,
                                StringRef FileName, bool *IncompleteFormat) {
   FormattingAttemptStatus Status;

unittests/Format/CleanupTest.cpp

std::string cleanup(llvm::StringRef Code,
 
     auto Result = applyAllReplacements(Code, Replaces);
     EXPECT_TRUE(static_cast<bool>(Result));
     return *Result;
   }
 
   // Returns code after cleanup around \p Offsets.
   std::string cleanupAroundOffsets(llvm::ArrayRef<unsigned> Offsets,
-                                   llvm::StringRef Code) {
+                                   llvm::StringRef Code,
+                                   const FormatStyle &Style = getLLVMStyle()) {
     std::vector<tooling::Range> Ranges;
     for (auto Offset : Offsets)
       Ranges.push_back(tooling::Range(Offset, 0));
-    return cleanup(Code, Ranges);
+    return cleanup(Code, Ranges, Style);
   }
 };
 
 TEST_F(CleanupTest, DeleteEmptyNamespaces) {
   std::string Code = "namespace A {\n"
                      "namespace B {\n"
                      "} // namespace B\n"
                      "} // namespace A\n\n"
TEST_F(CleanupTest, ListRedundantComma) {
   std::string Expected = "void f() { std::vector<int> v = {1,2,3,{4,5}}; }";
   EXPECT_EQ(Expected, cleanupAroundOffsets({40}, Code));
 
   Code = "int main() { f(1,,2,3,,4);}";
   Expected = "int main() { f(1,2,3,4);}";
   EXPECT_EQ(Expected, cleanupAroundOffsets({17, 22}, Code));
 }
 
+TEST_F(CleanupTest, ListRedundantCommaJavaScript) {
+  std::string Code = "function f() { var x = [a, b, , c]; }";
+  std::string Expected = "function f() { var x = [a, b, , c]; }";
+  const FormatStyle &Style = getGoogleStyle(FormatStyle::LK_JavaScript);
+      llvm::dbgs() << "Language style is " << Style.Language << "\n";
+
+  EXPECT_EQ(Expected, cleanupAroundOffsets({30}, Code, Style));
+}
+
 TEST_F(CleanupTest, TrailingCommaInParens) {
   std::string Code = "int main() { f(,1,,2,3,f(1,2,),4,,);}";
   std::string Expected = "int main() { f(1,2,3,f(1,2),4);}";
   EXPECT_EQ(Expected, cleanupAroundOffsets({15, 18, 29, 33}, Code));
 }
 
 TEST_F(CleanupTest, TrailingCommaInBraces) {
   // Trainling comma is allowed in brace list.