Clang: Honor fixits own locations in ClangFixItOperation

The fixits have own ranges/locations and thus might address more than
one file.

Change-Id: I5ee59944bef588e763a91f054a60823593373a0e
Reviewed-by: David Schulz <david.schulz@qt.io>

src/plugins/clangcodemodel/clangdiagnostictooltipwidget.cpp

@@ -108,10 +108,9 @@ void openEditorAt(const ClangBackEnd::SourceLocationContainer &location)
                                       int(location.column() - 1));
 }
 
-void applyFixit(const ClangBackEnd::SourceLocationContainer &location,
+void applyFixit(const QVector<ClangBackEnd::FixItContainer> &fixits)
-                const QVector<ClangBackEnd::FixItContainer> &fixits)
 {
-    ClangCodeModel::ClangFixItOperation operation(location.filePath(), Utf8String(), fixits);
+    ClangCodeModel::ClangFixItOperation operation(Utf8String(), fixits);
 
     operation.perform();
 }
@@ -146,7 +145,7 @@ QWidget *createDiagnosticLabel(const ClangBackEnd::DiagnosticContainer &diagnost
     label->setTextFormat(Qt::RichText);
     QObject::connect(label, &QLabel::linkActivated, [location, fixits](const QString &action) {
         if (action == QLatin1String(LINK_ACTION_APPLY_FIX))
-            applyFixit(location, fixits);
+            applyFixit(fixits);
         else
             openEditorAt(location);
 

src/plugins/clangcodemodel/clangfixitoperation.cpp

@@ -27,16 +27,21 @@
 
 #include <texteditor/refactoringchanges.h>
 
+#include <utils/qtcassert.h>
+
 #include <QTextDocument>
 
 namespace ClangCodeModel {
 
-ClangFixItOperation::ClangFixItOperation(const Utf8String &filePath,
+using FileToFixits = QMap<QString, QVector<ClangBackEnd::FixItContainer>>;
+using FileToFixitsIterator = QMapIterator<QString, QVector<ClangBackEnd::FixItContainer>>;
+using RefactoringFilePtr = QSharedPointer<TextEditor::RefactoringFile>;
+
+ClangFixItOperation::ClangFixItOperation(
         const Utf8String &fixItText,
         const QVector<ClangBackEnd::FixItContainer> &fixItContainers)
-    : filePath(filePath),
+    : fixItText(fixItText)
-      fixItText(fixItText),
+    , fixItContainers(fixItContainers)
-      fixItContainers(fixItContainers)
 {
 }
 
@@ -50,20 +55,56 @@ QString ClangCodeModel::ClangFixItOperation::description() const
     return QStringLiteral("Apply Fix: ") + fixItText.toString();
 }
 
+static FileToFixits fixitsPerFile(const QVector<ClangBackEnd::FixItContainer> &fixItContainers)
+{
+    FileToFixits mapping;
+
+    for (const auto &fixItContainer : fixItContainers) {
+        const QString rangeStartFilePath = fixItContainer.range().start().filePath().toString();
+        const QString rangeEndFilePath = fixItContainer.range().end().filePath().toString();
+        QTC_CHECK(rangeStartFilePath == rangeEndFilePath);
+        mapping[rangeStartFilePath].append(fixItContainer);
+    }
+
+    return mapping;
+}
+
 void ClangFixItOperation::perform()
 {
     const TextEditor::RefactoringChanges refactoringChanges;
-    refactoringFile = refactoringChanges.file(filePath.toString());
+    const FileToFixits fileToFixIts = fixitsPerFile(fixItContainers);
-    refactoringFile->setChangeSet(changeSet());
+
-    refactoringFile->apply();
+    FileToFixitsIterator i(fileToFixIts);
+    while (i.hasNext()) {
+        i.next();
+        const QString filePath = i.key();
+        const QVector<ClangBackEnd::FixItContainer> fixits = i.value();
+
+        RefactoringFilePtr refactoringFile = refactoringChanges.file(filePath);
+        refactoringFiles.append(refactoringFile);
+
+        applyFixitsToFile(*refactoringFile, fixits);
+    }
 }
 
-QString ClangFixItOperation::refactoringFileContent_forTestOnly() const
+QString ClangFixItOperation::firstRefactoringFileContent_forTestOnly() const
 {
-    return refactoringFile->document()->toPlainText();
+    return refactoringFiles.first()->document()->toPlainText();
 }
 
-Utils::ChangeSet ClangFixItOperation::changeSet() const
+void ClangFixItOperation::applyFixitsToFile(
+        TextEditor::RefactoringFile &refactoringFile,
+        const QVector<ClangBackEnd::FixItContainer> fixItContainers)
+{
+    const Utils::ChangeSet changeSet = toChangeSet(refactoringFile, fixItContainers);
+
+    refactoringFile.setChangeSet(changeSet);
+    refactoringFile.apply();
+}
+
+Utils::ChangeSet ClangFixItOperation::toChangeSet(
+        TextEditor::RefactoringFile &refactoringFile,
+        const QVector<ClangBackEnd::FixItContainer> fixItContainers) const
 {
     Utils::ChangeSet changeSet;
 
@@ -71,8 +112,8 @@ Utils::ChangeSet ClangFixItOperation::changeSet() const
         const auto range = fixItContainer.range();
         const auto start = range.start();
         const auto end = range.end();
-        changeSet.replace(refactoringFile->position(start.line(), start.column()),
+        changeSet.replace(refactoringFile.position(start.line(), start.column()),
-                          refactoringFile->position(end.line(), end.column()),
+                          refactoringFile.position(end.line(), end.column()),
                           fixItContainer.text());
     }
 

src/plugins/clangcodemodel/clangfixitoperation.h

@@ -36,6 +36,7 @@
 
 namespace TextEditor
 {
+class RefactoringChanges;
 class RefactoringFile;
 }
 
@@ -44,23 +45,25 @@ namespace ClangCodeModel {
 class ClangFixItOperation : public TextEditor::QuickFixOperation
 {
 public:
-    ClangFixItOperation(const Utf8String &filePath,
+    ClangFixItOperation(const Utf8String &fixItText,
-                        const Utf8String &fixItText,
                         const QVector<ClangBackEnd::FixItContainer> &fixItContainers);
 
     int priority() const override;
     QString description() const override;
     void perform() override;
 
-    QString refactoringFileContent_forTestOnly() const;
+    QString firstRefactoringFileContent_forTestOnly() const;
 
 private:
-    Utils::ChangeSet changeSet() const;
+    void applyFixitsToFile(TextEditor::RefactoringFile &refactoringFile,
+                           const QVector<ClangBackEnd::FixItContainer> fixItContainers);
+    Utils::ChangeSet toChangeSet(
+            TextEditor::RefactoringFile &refactoringFile,
+            const QVector<ClangBackEnd::FixItContainer> fixItContainers) const;
 
 private:
-    Utf8String filePath;
     Utf8String fixItText;
-    QSharedPointer<TextEditor::RefactoringFile> refactoringFile;
+    QVector<QSharedPointer<TextEditor::RefactoringFile>> refactoringFiles;
     QVector<ClangBackEnd::FixItContainer> fixItContainers;
 };
 

src/plugins/clangcodemodel/clangfixitoperationsextractor.cpp

@@ -109,16 +109,13 @@ ClangFixItOperationsExtractor::extract(const QString &filePath, int line)
 }
 
 void ClangFixItOperationsExtractor::appendFixitOperation(
-        const QString &filePath,
         const QString &diagnosticText,
         const QVector<ClangBackEnd::FixItContainer> &fixits)
 {
     if (!fixits.isEmpty()) {
         const QString diagnosticTextTweaked = tweakedDiagnosticText(diagnosticText);
         TextEditor::QuickFixOperation::Ptr operation(
-                    new ClangFixItOperation(filePath,
+                    new ClangFixItOperation(diagnosticTextTweaked, fixits));
-                                            diagnosticTextTweaked,
-                                            fixits));
         operations.append(operation);
     }
 }
@@ -130,7 +127,7 @@ void ClangFixItOperationsExtractor::extractFromDiagnostic(
 {
     const QVector<ClangBackEnd::FixItContainer> fixIts = diagnosticContainer.fixIts();
     if (hasFixItAt(fixIts, filePath, line)) {
-        appendFixitOperation(filePath, diagnosticContainer.text().toString(), fixIts);
+        appendFixitOperation(diagnosticContainer.text().toString(), fixIts);
 
         foreach (const auto &child, diagnosticContainer.children())
             extractFromDiagnostic(child, filePath, line);

src/plugins/clangcodemodel/clangfixitoperationsextractor.h

@@ -42,8 +42,7 @@ private:
     void extractFromDiagnostic(const ClangBackEnd::DiagnosticContainer &diagnosticContainer,
                                const QString &filePath,
                                int line);
-    void appendFixitOperation(const QString &filePath,
+    void appendFixitOperation(const QString &diagnosticText,
-                              const QString &diagnosticText,
                               const QVector<ClangBackEnd::FixItContainer> &fixits);
 
 private:

tests/unit/unittest/clangfixitoperationtest.cpp

@@ -58,7 +58,7 @@ MATCHER_P(MatchText, expectedText,
           + " expected text:\n" + PrintToString(expectedText))
 {
     const ::ClangFixItOperation &operation = arg;
-    QString resultText = operation.refactoringFileContent_forTestOnly();
+    QString resultText = operation.firstRefactoringFileContent_forTestOnly();
 
     if (resultText != expectedText) {
         *result_listener << "\n" << resultText.toUtf8().constData();
@@ -95,7 +95,7 @@ protected:
 
 TEST_F(ClangFixItOperation, Description)
 {
-    ::ClangFixItOperation operation(semicolonFilePath, diagnosticText, {semicolonFixItContainer});
+    ::ClangFixItOperation operation(diagnosticText, {semicolonFixItContainer});
 
     ASSERT_THAT(operation.description(),
                 QStringLiteral("Apply Fix: expected ';' at end of declaration"));
@@ -103,7 +103,7 @@ TEST_F(ClangFixItOperation, Description)
 
 TEST_F(ClangFixItOperation, AppendSemicolon)
 {
-    ::ClangFixItOperation operation(semicolonFilePath, diagnosticText, {semicolonFixItContainer});
+    ::ClangFixItOperation operation(diagnosticText, {semicolonFixItContainer});
 
     operation.perform();
 
@@ -112,7 +112,7 @@ TEST_F(ClangFixItOperation, AppendSemicolon)
 
 TEST_F(ClangFixItOperation, ComparisonVersusAssignmentChooseComparison)
 {
-    ::ClangFixItOperation operation(compareFilePath, diagnosticText, {compareFixItContainer});
+    ::ClangFixItOperation operation(diagnosticText, {compareFixItContainer});
 
     operation.perform();
 
@@ -121,8 +121,7 @@ TEST_F(ClangFixItOperation, ComparisonVersusAssignmentChooseComparison)
 
 TEST_F(ClangFixItOperation, ComparisonVersusAssignmentChooseParentheses)
 {
-    ::ClangFixItOperation operation(compareFilePath,
+    ::ClangFixItOperation operation(diagnosticText,
-                                    diagnosticText,
                                     {assignmentFixItContainerParenLeft,
                                      assignmentFixItContainerParenRight});