Clang: Properly apply fix-its from header files

Diagnostic may also include fix-its for the header
which require different handling.

Task-number: QTCREATORBUG-20517
Change-Id: I3e745622801be3fa2856d968b0c7a2a2aeb89b50
Reviewed-by: David Schulz <david.schulz@qt.io>

src/plugins/clangtools/clangfixitsrefactoringchanges.cpp

@@ -42,11 +42,11 @@ using namespace Utils;
 namespace ClangTools {
 namespace Internal {
 
-int FixitsRefactoringFile::position(unsigned line, unsigned column) const
+int FixitsRefactoringFile::position(const QString &filePath, unsigned line, unsigned column) const
 {
     QTC_ASSERT(line != 0, return -1);
     QTC_ASSERT(column != 0, return -1);
-    return document()->findBlockByNumber(line - 1).position() + column - 1;
+    return document(filePath)->findBlockByNumber(line - 1).position() + column - 1;
 }
 
 static QDebug operator<<(QDebug debug, const ReplacementOperation &op)
@@ -71,23 +71,22 @@ bool FixitsRefactoringFile::apply()
 
     QTC_ASSERT(!m_filePath.isEmpty(), return false);
 
-    // Check for permissions
-    if (!QFileInfo(m_filePath).isWritable())
-        return false; // Error file not writable
-
     // Apply changes
-    QTextDocument *doc = document();
-    QTextCursor cursor(doc);
-
     for (int i=0; i < m_replacementOperations.size(); ++i) {
         ReplacementOperation &op = *m_replacementOperations[i];
         if (op.apply) {
+            // Check for permissions
+            if (!QFileInfo(op.fileName).isWritable())
+                return false; // Error file not writable
+
             qCDebug(fixitsLog) << " " << i << "Applying" << op;
 
             // Shift subsequent operations that are affected
             shiftAffectedReplacements(op, i + 1);
 
             // Apply
+            QTextDocument *doc = document(op.fileName);
+            QTextCursor cursor(doc);
             cursor.setPosition(op.pos);
             cursor.setPosition(op.pos + op.length, QTextCursor::KeepAnchor);
             cursor.insertText(op.text);
@@ -99,40 +98,45 @@ bool FixitsRefactoringFile::apply()
         return false; // Error reading file
 
     QString error;
-    if (!m_textFileFormat.writeFile(m_filePath, doc->toPlainText(), &error)) {
+    for (auto it = m_documents.begin(); it != m_documents.end(); ++it) {
-        qCDebug(fixitsLog) << "ERROR: Could not write file" << m_filePath << ":" << error;
+        if (!m_textFileFormat.writeFile(it.key(), it.value()->toPlainText(), &error)) {
+            qCDebug(fixitsLog) << "ERROR: Could not write file" << it.key() << ":" << error;
             return false; // Error writing file
         }
+    }
 
     return true;
 }
 
-QTextDocument *FixitsRefactoringFile::document() const
+QTextDocument *FixitsRefactoringFile::document(const QString &filePath) const
 {
-    if (!m_document) {
+    if (m_documents.find(filePath) == m_documents.end()) {
         QString fileContents;
-        if (!m_filePath.isEmpty()) {
+        if (!filePath.isEmpty()) {
             QString error;
             QTextCodec *defaultCodec = Core::EditorManager::defaultTextCodec();
             TextFileFormat::ReadResult result = TextFileFormat::readFile(
-                        m_filePath, defaultCodec,
+                        filePath, defaultCodec,
                         &fileContents, &m_textFileFormat,
                         &error);
             if (result != TextFileFormat::ReadSuccess) {
-                qCDebug(fixitsLog) << "ERROR: Could not read " << m_filePath << ":" << error;
+                qCDebug(fixitsLog) << "ERROR: Could not read " << filePath << ":" << error;
                 m_textFileFormat.codec = nullptr;
             }
         }
         // always make a QTextDocument to avoid excessive null checks
-        m_document = new QTextDocument(fileContents);
+        m_documents[filePath] = new QTextDocument(fileContents);
     }
-    return m_document;
+    return m_documents[filePath];
 }
 
 void FixitsRefactoringFile::shiftAffectedReplacements(const ReplacementOperation &op, int startIndex)
 {
     for (int i = startIndex; i < m_replacementOperations.size(); ++i) {
         ReplacementOperation &current = *m_replacementOperations[i];
+        if (op.fileName != current.fileName)
+            continue;
+
         ReplacementOperation before = current;
 
         if (op.pos <= current.pos)

src/plugins/clangtools/clangfixitsrefactoringchanges.h

@@ -41,6 +41,7 @@ public:
     int pos = -1;
     int length = -1;
     QString text;
+    QString fileName;
     bool apply = false;
 };
 using ReplacementOperations = QVector<ReplacementOperation *>;
@@ -52,20 +53,21 @@ class FixitsRefactoringFile
 public:
     FixitsRefactoringFile() = default;
     FixitsRefactoringFile(const QString &filePath) : m_filePath(filePath) {}
+    ~FixitsRefactoringFile() { qDeleteAll(m_documents); }
 
     bool isValid() const { return !m_filePath.isEmpty(); }
-    int position(unsigned line, unsigned column) const;
+    int position(const QString &filePath, unsigned line, unsigned column) const;
 
     void setReplacements(const ReplacementOperations &ops) { m_replacementOperations = ops; }
     bool apply();
 
 private:
-    QTextDocument *document() const;
+    QTextDocument *document(const QString &filePath) const;
     void shiftAffectedReplacements(const ReplacementOperation &op, int startIndex);
 
     QString m_filePath;
     mutable Utils::TextFileFormat m_textFileFormat;
-    mutable QTextDocument *m_document = nullptr;
+    mutable QHash<QString, QTextDocument *> m_documents;
     ReplacementOperations m_replacementOperations; // Not owned.
 };
 

src/plugins/clangtools/clangtidyclazytool.cpp

@@ -119,13 +119,14 @@ public:
 
             const Debugger::DiagnosticLocation start = step.ranges.first();
             const Debugger::DiagnosticLocation end = step.ranges.last();
-            const int startPos = file.position(start.line, start.column);
+            const int startPos = file.position(start.filePath, start.line, start.column);
-            const int endPos = file.position(end.line, end.column);
+            const int endPos = file.position(start.filePath, end.line, end.column);
 
             auto op = new ReplacementOperation;
             op->pos = startPos;
             op->length = endPos - startPos;
             op->text = step.message;
+            op->fileName = start.filePath;
             op->apply = apply;
 
             replacements += op;