clang-format: [JS] merge requoting replacements.

[mprobst]

Summary

When formatting source code that needs both requoting and reindentation,
merge the replacements to avoid erroring out for conflicting replacements.

[djasper]

Basically looks good.

lib/Format/Format.cpp

806 ↗

(On Diff #70066)

Call this "RequoteChanges".

831 ↗

(On Diff #70066)

Just

return RequoteChanges.merge(Whitespaces.generateReplacements());

[mprobst]

lib/Format/Format.cpp

806 ↗

(On Diff #70066)

Done, went with RequoteReplaces (which is what we use in other places for the tooling::Replacements variable).

831 ↗

(On Diff #70066)

Done.

Somewhat related, this API was very surprising – it passes down a Replacements object, but then the caller takes every item of the returned result and adds it back into the object, causing conflicts. So the only way to correctly use the API is to ignore the parameter, or return an empty dummy object. I've changed the API to not pass down the parameter to make this a bit less misleading.

cfe/trunk/lib/Format/Format.cpp

 public:
   Formatter(const Environment &Env, const FormatStyle &Style,
             bool *IncompleteFormat)
       : TokenAnalyzer(Env, Style), IncompleteFormat(IncompleteFormat) {}
 
   tooling::Replacements
   analyze(TokenAnnotator &Annotator,
           SmallVectorImpl<AnnotatedLine *> &AnnotatedLines,
-          FormatTokenLexer &Tokens, tooling::Replacements &Result) override {
+          FormatTokenLexer &Tokens) override {
+    tooling::Replacements RequoteReplaces;
     deriveLocalStyle(AnnotatedLines);
     AffectedRangeMgr.computeAffectedLines(AnnotatedLines.begin(),
                                           AnnotatedLines.end());
 
     if (Style.Language == FormatStyle::LK_JavaScript &&
         Style.JavaScriptQuotes != FormatStyle::JSQS_Leave)
-      requoteJSStringLiteral(AnnotatedLines, Result);
+      requoteJSStringLiteral(AnnotatedLines, RequoteReplaces);
 
     for (unsigned i = 0, e = AnnotatedLines.size(); i != e; ++i) {
       Annotator.calculateFormattingInformation(*AnnotatedLines[i]);
     }
 
     Annotator.setCommentLineLevels(AnnotatedLines);
 
     WhitespaceManager Whitespaces(
         Env.getSourceManager(), Style,
         inputUsesCRLF(Env.getSourceManager().getBufferData(Env.getFileID())));
     ContinuationIndenter Indenter(Style, Tokens.getKeywords(),
                                   Env.getSourceManager(), Whitespaces, Encoding,
                                   BinPackInconclusiveFunctions);
     UnwrappedLineFormatter(&Indenter, &Whitespaces, Style, Tokens.getKeywords(),
                            IncompleteFormat)
         .format(AnnotatedLines);
-    return Whitespaces.generateReplacements();
+    return RequoteReplaces.merge(Whitespaces.generateReplacements());
   }
 
 private:
   // If the last token is a double/single-quoted string literal, generates a
   // replacement with a single/double quoted string literal, re-escaping the
   // contents in the process.
   void requoteJSStringLiteral(SmallVectorImpl<AnnotatedLine *> &Lines,
                               tooling::Replacements &Result) {
public:
   Cleaner(const Environment &Env, const FormatStyle &Style)
       : TokenAnalyzer(Env, Style),
         DeletedTokens(FormatTokenLess(Env.getSourceManager())) {}
 
   // FIXME: eliminate unused parameters.
   tooling::Replacements
   analyze(TokenAnnotator &Annotator,
           SmallVectorImpl<AnnotatedLine *> &AnnotatedLines,
-          FormatTokenLexer &Tokens, tooling::Replacements &Result) override {
+          FormatTokenLexer &Tokens) override {
     // FIXME: in the current implementation the granularity of affected range
     // is an annotated line. However, this is not sufficient. Furthermore,
     // redundant code introduced by replacements does not necessarily
     // intercept with ranges of replacements that result in the redundancy.
     // 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(),

cfe/trunk/lib/Format/SortJavaScriptImports.cpp

 public:
   JavaScriptImportSorter(const Environment &Env, const FormatStyle &Style)
       : TokenAnalyzer(Env, Style),
         FileContents(Env.getSourceManager().getBufferData(Env.getFileID())) {}
 
   tooling::Replacements
   analyze(TokenAnnotator &Annotator,
           SmallVectorImpl<AnnotatedLine *> &AnnotatedLines,
-          FormatTokenLexer &Tokens, tooling::Replacements &) override {
+          FormatTokenLexer &Tokens) override {
     tooling::Replacements Result;
     AffectedRangeMgr.computeAffectedLines(AnnotatedLines.begin(),
                                           AnnotatedLines.end());
 
     const AdditionalKeywords &Keywords = Tokens.getKeywords();
     SmallVector<JsModuleReference, 16> References;
     AnnotatedLine *FirstNonImportLine;
     std::tie(References, FirstNonImportLine) =
private:
   // Parses module references in the given lines. Returns the module references,
   // and a pointer to the first "main code" line if that is adjacent to the
   // affected lines of module references, nullptr otherwise.
   std::pair<SmallVector<JsModuleReference, 16>, AnnotatedLine*>
   parseModuleReferences(const AdditionalKeywords &Keywords,
                         SmallVectorImpl<AnnotatedLine *> &AnnotatedLines) {
     SmallVector<JsModuleReference, 16> References;
     SourceLocation Start;
-    bool FoundLines = false;
     AnnotatedLine *FirstNonImportLine = nullptr;
     bool AnyImportAffected = false;
     for (auto Line : AnnotatedLines) {
       Current = Line->First;
       LineEnd = Line->Last;
       skipComments();
       if (Start.isInvalid() || References.empty())
         // After the first file level comment, consider line comments to be part
         // of the import that immediately follows them by using the previously
         // set Start.
         Start = Line->First->Tok.getLocation();
       if (!Current)
         continue; // Only comments on this line.
-      FoundLines = true;
       JsModuleReference Reference;
       Reference.Range.setBegin(Start);
       if (!parseModuleReference(Keywords, Reference)) {
         FirstNonImportLine = Line;
         break;
       }
       AnyImportAffected = AnyImportAffected || Line->Affected;
       Reference.Range.setEnd(LineEnd->Tok.getEndLoc());

cfe/trunk/lib/Format/TokenAnalyzer.h

public:
   TokenAnalyzer(const Environment &Env, const FormatStyle &Style);
 
   tooling::Replacements process();
 
 protected:
   virtual tooling::Replacements
   analyze(TokenAnnotator &Annotator,
           SmallVectorImpl<AnnotatedLine *> &AnnotatedLines,
-          FormatTokenLexer &Tokens, tooling::Replacements &Result) = 0;
+          FormatTokenLexer &Tokens) = 0;
 
   void consumeUnwrappedLine(const UnwrappedLine &TheLine) override;
 
   void finishRun() override;
 
   FormatStyle Style;
   // Stores Style, FileID and SourceManager etc.
   const Environment &Env;

cfe/trunk/lib/Format/TokenAnalyzer.cpp

for (unsigned Run = 0, RunE = UnwrappedLines.size(); Run + 1 != RunE; ++Run) {
 
     TokenAnnotator Annotator(Style, Tokens.getKeywords());
     for (unsigned i = 0, e = UnwrappedLines[Run].size(); i != e; ++i) {
       AnnotatedLines.push_back(new AnnotatedLine(UnwrappedLines[Run][i]));
       Annotator.annotate(*AnnotatedLines.back());
     }
 
     tooling::Replacements RunResult =
-        analyze(Annotator, AnnotatedLines, Tokens, Result);
+        analyze(Annotator, AnnotatedLines, Tokens);
 
     DEBUG({
       llvm::dbgs() << "Replacements for run " << Run << ":\n";
       for (tooling::Replacements::const_iterator I = RunResult.begin(),
                                                  E = RunResult.end();
            I != E; ++I) {
         llvm::dbgs() << I->toString() << "\n";
       }

cfe/trunk/unittests/Format/FormatTestJS.cpp

verifyFormat("// clang-format off\n"
                "// clang-format on\n"
                "let x = 'single';\n",
                "// clang-format off\n"
                "let x = \"double\";\n"
                "// clang-format on\n"
                "let x = \"single\";\n");
 }
 
+TEST_F(FormatTestJS, RequoteAndIndent) {
+  verifyFormat("let x = someVeryLongFunctionThatGoesOnAndOn(\n"
+               "    'double quoted string that needs wrapping');",
+               "let x = someVeryLongFunctionThatGoesOnAndOn("
+               "\"double quoted string that needs wrapping\");");
+}
+
 TEST_F(FormatTestJS, RequoteStringsDouble) {
   FormatStyle DoubleQuotes = getGoogleStyle(FormatStyle::LK_JavaScript);
   DoubleQuotes.JavaScriptQuotes = FormatStyle::JSQS_Double;
   verifyFormat("var x = \"foo\";", DoubleQuotes);
   verifyFormat("var x = \"foo\";", "var x = 'foo';", DoubleQuotes);
   verifyFormat("var x = \"fo'o\";", "var x = 'fo\\'o';", DoubleQuotes);
 }