hokein updated this revision to Diff 193857.
hokein added a comment.
Remove a stale comment.
Repository:
rG LLVM Github Monorepo
CHANGES SINCE LAST ACTION
https://reviews.llvm.org/D59932/new/
https://reviews.llvm.org/D59932
Files:
clang-tools-extra/clang-apply-replacements/lib/Tooling/ApplyReplacements.cpp
clang-tools-extra/clang-tidy/ClangTidy.cpp
clang-tools-extra/clang-tidy/ClangTidyDiagnosticConsumer.cpp
clang-tools-extra/clang-tidy/add_new_check.py
clang-tools-extra/clang-tidy/misc/UnusedUsingDeclsCheck.cpp
clang-tools-extra/test/clang-apply-replacements/Inputs/basic/file1.yaml
clang-tools-extra/test/clang-apply-replacements/Inputs/basic/file2.yaml
clang-tools-extra/test/clang-apply-replacements/Inputs/conflict/file1.yaml
clang-tools-extra/test/clang-apply-replacements/Inputs/conflict/file2.yaml
clang-tools-extra/test/clang-apply-replacements/Inputs/conflict/file3.yaml
clang-tools-extra/test/clang-apply-replacements/Inputs/crlf/file1.yaml
clang-tools-extra/test/clang-apply-replacements/Inputs/format/no.yaml
clang-tools-extra/test/clang-apply-replacements/Inputs/format/yes.yaml
clang-tools-extra/test/clang-apply-replacements/Inputs/identical/file1.yaml
clang-tools-extra/test/clang-apply-replacements/Inputs/identical/file2.yaml
clang-tools-extra/test/clang-apply-replacements/Inputs/order-dependent/file1.yaml
clang-tools-extra/test/clang-apply-replacements/Inputs/order-dependent/file2.yaml
clang-tools-extra/test/clang-tidy/export-diagnostics.cpp
clang-tools-extra/unittests/clang-apply-replacements/ApplyReplacementsTest.cpp
clang-tools-extra/unittests/clang-tidy/ClangTidyTest.h
clang/include/clang/Tooling/Core/Diagnostic.h
clang/include/clang/Tooling/DiagnosticsYaml.h
clang/lib/Tooling/Core/Diagnostic.cpp
clang/unittests/Tooling/DiagnosticsYamlTest.cpp
Index: clang/unittests/Tooling/DiagnosticsYamlTest.cpp
===================================================================
--- clang/unittests/Tooling/DiagnosticsYamlTest.cpp
+++ clang/unittests/Tooling/DiagnosticsYamlTest.cpp
@@ -20,11 +20,13 @@
using clang::tooling::Diagnostic;
static DiagnosticMessage makeMessage(const std::string &Message, int FileOffset,
- const std::string &FilePath) {
+ const std::string &FilePath,
+ const StringMap<Replacements> &Fix) {
DiagnosticMessage DiagMessage;
DiagMessage.Message = Message;
DiagMessage.FileOffset = FileOffset;
DiagMessage.FilePath = FilePath;
+ DiagMessage.Fix = Fix;
return DiagMessage;
}
@@ -32,10 +34,52 @@
const std::string &Message, int FileOffset,
const std::string &FilePath,
const StringMap<Replacements> &Fix) {
- return Diagnostic(DiagnosticName, makeMessage(Message, FileOffset, FilePath),
- Fix, {}, Diagnostic::Warning, "path/to/build/directory");
+ return Diagnostic(DiagnosticName,
+ makeMessage(Message, FileOffset, FilePath, Fix), {},
+ Diagnostic::Warning, "path/to/build/directory");
}
+static const char *YAMLContent =
+ "---\n"
+ "MainSourceFile: 'path/to/source.cpp'\n"
+ "Diagnostics: \n"
+ " - DiagnosticName: 'diagnostic#1\'\n"
+ " DiagnosticMessage: \n"
+ " Message: 'message #1'\n"
+ " FilePath: 'path/to/source.cpp'\n"
+ " FileOffset: 55\n"
+ " Replacements: \n"
+ " - FilePath: 'path/to/source.cpp'\n"
+ " Offset: 100\n"
+ " Length: 12\n"
+ " ReplacementText: 'replacement #1'\n"
+ " - DiagnosticName: 'diagnostic#2'\n"
+ " DiagnosticMessage: \n"
+ " Message: 'message #2'\n"
+ " FilePath: 'path/to/header.h'\n"
+ " FileOffset: 60\n"
+ " Replacements: \n"
+ " - FilePath: 'path/to/header.h'\n"
+ " Offset: 62\n"
+ " Length: 2\n"
+ " ReplacementText: 'replacement #2'\n"
+ " - DiagnosticName: 'diagnostic#3'\n"
+ " DiagnosticMessage: \n"
+ " Message: 'message #3'\n"
+ " FilePath: 'path/to/source2.cpp'\n"
+ " FileOffset: 72\n"
+ " Replacements: []\n"
+ " Notes: \n"
+ " - Message: Note1\n"
+ " FilePath: 'path/to/note1.cpp'\n"
+ " FileOffset: 88\n"
+ " Replacements: []\n"
+ " - Message: Note2\n"
+ " FilePath: 'path/to/note2.cpp'\n"
+ " FileOffset: 99\n"
+ " Replacements: []\n"
+ "...\n";
+
TEST(DiagnosticsYamlTest, serializesDiagnostics) {
TranslationUnitDiagnostics TUD;
TUD.MainSourceFile = "path/to/source.cpp";
@@ -55,9 +99,9 @@
TUD.Diagnostics.push_back(makeDiagnostic("diagnostic#3", "message #3", 72,
"path/to/source2.cpp", {}));
TUD.Diagnostics.back().Notes.push_back(
- makeMessage("Note1", 88, "path/to/note1.cpp"));
+ makeMessage("Note1", 88, "path/to/note1.cpp", {}));
TUD.Diagnostics.back().Notes.push_back(
- makeMessage("Note2", 99, "path/to/note2.cpp"));
+ makeMessage("Note2", 99, "path/to/note2.cpp", {}));
std::string YamlContent;
raw_string_ostream YamlContentStream(YamlContent);
@@ -65,80 +109,12 @@
yaml::Output YAML(YamlContentStream);
YAML << TUD;
- EXPECT_EQ("---\n"
- "MainSourceFile: 'path/to/source.cpp'\n"
- "Diagnostics: \n"
- " - DiagnosticName: 'diagnostic#1\'\n"
- " Message: 'message #1'\n"
- " FileOffset: 55\n"
- " FilePath: 'path/to/source.cpp'\n"
- " Replacements: \n"
- " - FilePath: 'path/to/source.cpp'\n"
- " Offset: 100\n"
- " Length: 12\n"
- " ReplacementText: 'replacement #1'\n"
- " - DiagnosticName: 'diagnostic#2'\n"
- " Message: 'message #2'\n"
- " FileOffset: 60\n"
- " FilePath: 'path/to/header.h'\n"
- " Replacements: \n"
- " - FilePath: 'path/to/header.h'\n"
- " Offset: 62\n"
- " Length: 2\n"
- " ReplacementText: 'replacement #2'\n"
- " - DiagnosticName: 'diagnostic#3'\n"
- " Message: 'message #3'\n"
- " FileOffset: 72\n"
- " FilePath: 'path/to/source2.cpp'\n"
- " Notes: \n"
- " - Message: Note1\n"
- " FilePath: 'path/to/note1.cpp'\n"
- " FileOffset: 88\n"
- " - Message: Note2\n"
- " FilePath: 'path/to/note2.cpp'\n"
- " FileOffset: 99\n"
- " Replacements: []\n"
- "...\n",
- YamlContentStream.str());
+ EXPECT_EQ(YAMLContent, YamlContentStream.str());
}
TEST(DiagnosticsYamlTest, deserializesDiagnostics) {
- std::string YamlContent = "---\n"
- "MainSourceFile: path/to/source.cpp\n"
- "Diagnostics: \n"
- " - DiagnosticName: 'diagnostic#1'\n"
- " Message: 'message #1'\n"
- " FileOffset: 55\n"
- " FilePath: path/to/source.cpp\n"
- " Replacements: \n"
- " - FilePath: path/to/source.cpp\n"
- " Offset: 100\n"
- " Length: 12\n"
- " ReplacementText: 'replacement #1'\n"
- " - DiagnosticName: 'diagnostic#2'\n"
- " Message: 'message #2'\n"
- " FileOffset: 60\n"
- " FilePath: path/to/header.h\n"
- " Replacements: \n"
- " - FilePath: path/to/header.h\n"
- " Offset: 62\n"
- " Length: 2\n"
- " ReplacementText: 'replacement #2'\n"
- " - DiagnosticName: 'diagnostic#3'\n"
- " Message: 'message #3'\n"
- " FileOffset: 98\n"
- " FilePath: path/to/source.cpp\n"
- " Notes:\n"
- " - Message: Note1\n"
- " FilePath: 'path/to/note1.cpp'\n"
- " FileOffset: 66\n"
- " - Message: Note2\n"
- " FilePath: 'path/to/note2.cpp'\n"
- " FileOffset: 77\n"
- " Replacements: []\n"
- "...\n";
TranslationUnitDiagnostics TUDActual;
- yaml::Input YAML(YamlContent);
+ yaml::Input YAML(YAMLContent);
YAML >> TUDActual;
ASSERT_FALSE(YAML.error());
@@ -160,7 +136,7 @@
EXPECT_EQ("message #1", D1.Message.Message);
EXPECT_EQ(55u, D1.Message.FileOffset);
EXPECT_EQ("path/to/source.cpp", D1.Message.FilePath);
- std::vector<Replacement> Fixes1 = getFixes(D1.Fix);
+ std::vector<Replacement> Fixes1 = getFixes(D1.Message.Fix);
ASSERT_EQ(1u, Fixes1.size());
EXPECT_EQ("path/to/source.cpp", Fixes1[0].getFilePath());
EXPECT_EQ(100u, Fixes1[0].getOffset());
@@ -172,7 +148,7 @@
EXPECT_EQ("message #2", D2.Message.Message);
EXPECT_EQ(60u, D2.Message.FileOffset);
EXPECT_EQ("path/to/header.h", D2.Message.FilePath);
- std::vector<Replacement> Fixes2 = getFixes(D2.Fix);
+ std::vector<Replacement> Fixes2 = getFixes(D2.Message.Fix);
ASSERT_EQ(1u, Fixes2.size());
EXPECT_EQ("path/to/header.h", Fixes2[0].getFilePath());
EXPECT_EQ(62u, Fixes2[0].getOffset());
@@ -182,15 +158,15 @@
Diagnostic D3 = TUDActual.Diagnostics[2];
EXPECT_EQ("diagnostic#3", D3.DiagnosticName);
EXPECT_EQ("message #3", D3.Message.Message);
- EXPECT_EQ(98u, D3.Message.FileOffset);
- EXPECT_EQ("path/to/source.cpp", D3.Message.FilePath);
+ EXPECT_EQ(72u, D3.Message.FileOffset);
+ EXPECT_EQ("path/to/source2.cpp", D3.Message.FilePath);
EXPECT_EQ(2u, D3.Notes.size());
EXPECT_EQ("Note1", D3.Notes[0].Message);
- EXPECT_EQ(66u, D3.Notes[0].FileOffset);
+ EXPECT_EQ(88u, D3.Notes[0].FileOffset);
EXPECT_EQ("path/to/note1.cpp", D3.Notes[0].FilePath);
EXPECT_EQ("Note2", D3.Notes[1].Message);
- EXPECT_EQ(77u, D3.Notes[1].FileOffset);
+ EXPECT_EQ(99u, D3.Notes[1].FileOffset);
EXPECT_EQ("path/to/note2.cpp", D3.Notes[1].FilePath);
- std::vector<Replacement> Fixes3 = getFixes(D3.Fix);
+ std::vector<Replacement> Fixes3 = getFixes(D3.Message.Fix);
EXPECT_TRUE(Fixes3.empty());
}
Index: clang/lib/Tooling/Core/Diagnostic.cpp
===================================================================
--- clang/lib/Tooling/Core/Diagnostic.cpp
+++ clang/lib/Tooling/Core/Diagnostic.cpp
@@ -12,6 +12,7 @@
#include "clang/Tooling/Core/Diagnostic.h"
#include "clang/Basic/SourceManager.h"
+#include "llvm/ADT/STLExtras.h"
namespace clang {
namespace tooling {
@@ -40,11 +41,21 @@
Diagnostic::Diagnostic(llvm::StringRef DiagnosticName,
const DiagnosticMessage &Message,
- const llvm::StringMap<Replacements> &Fix,
const SmallVector<DiagnosticMessage, 1> &Notes,
Level DiagLevel, llvm::StringRef BuildDirectory)
- : DiagnosticName(DiagnosticName), Message(Message), Fix(Fix), Notes(Notes),
+ : DiagnosticName(DiagnosticName), Message(Message), Notes(Notes),
DiagLevel(DiagLevel), BuildDirectory(BuildDirectory) {}
+const llvm::StringMap<Replacements> *Diagnostic::getChosenFix() const {
+ if (!Message.Fix.empty())
+ return &Message.Fix;
+ auto Iter = llvm::find_if(Notes, [](const tooling::DiagnosticMessage &D) {
+ return !D.Fix.empty();
+ });
+ if (Iter != Notes.end())
+ return &Iter->Fix;
+ return nullptr;
+}
+
} // end namespace tooling
} // end namespace clang
Index: clang/include/clang/Tooling/DiagnosticsYaml.h
===================================================================
--- clang/include/clang/Tooling/DiagnosticsYaml.h
+++ clang/include/clang/Tooling/DiagnosticsYaml.h
@@ -31,6 +31,20 @@
Io.mapRequired("Message", M.Message);
Io.mapOptional("FilePath", M.FilePath);
Io.mapOptional("FileOffset", M.FileOffset);
+ std::vector<clang::tooling::Replacement> Fixes;
+ for (auto &Replacements : M.Fix) {
+ for (auto &Replacement : Replacements.second)
+ Fixes.push_back(Replacement);
+ }
+ Io.mapRequired("Replacements", Fixes);
+ for (auto &Fix : Fixes) {
+ llvm::Error Err = M.Fix[Fix.getFilePath()].add(Fix);
+ if (Err) {
+ // FIXME: Implement better conflict handling.
+ llvm::errs() << "Fix conflicts with existing fix: "
+ << llvm::toString(std::move(Err)) << "\n";
+ }
+ }
}
};
@@ -43,12 +57,11 @@
: DiagLevel(clang::tooling::Diagnostic::Level::Warning) {}
NormalizedDiagnostic(const IO &, const clang::tooling::Diagnostic &D)
- : DiagnosticName(D.DiagnosticName), Message(D.Message), Fix(D.Fix),
- Notes(D.Notes), DiagLevel(D.DiagLevel),
- BuildDirectory(D.BuildDirectory) {}
+ : DiagnosticName(D.DiagnosticName), Message(D.Message), Notes(D.Notes),
+ DiagLevel(D.DiagLevel), BuildDirectory(D.BuildDirectory) {}
clang::tooling::Diagnostic denormalize(const IO &) {
- return clang::tooling::Diagnostic(DiagnosticName, Message, Fix, Notes,
+ return clang::tooling::Diagnostic(DiagnosticName, Message, Notes,
DiagLevel, BuildDirectory);
}
@@ -64,28 +77,10 @@
MappingNormalization<NormalizedDiagnostic, clang::tooling::Diagnostic> Keys(
Io, D);
Io.mapRequired("DiagnosticName", Keys->DiagnosticName);
- Io.mapRequired("Message", Keys->Message.Message);
- Io.mapRequired("FileOffset", Keys->Message.FileOffset);
- Io.mapRequired("FilePath", Keys->Message.FilePath);
+ Io.mapRequired("DiagnosticMessage", Keys->Message);
Io.mapOptional("Notes", Keys->Notes);
// FIXME: Export properly all the different fields.
-
- std::vector<clang::tooling::Replacement> Fixes;
- for (auto &Replacements : Keys->Fix) {
- for (auto &Replacement : Replacements.second) {
- Fixes.push_back(Replacement);
- }
- }
- Io.mapRequired("Replacements", Fixes);
- for (auto &Fix : Fixes) {
- llvm::Error Err = Keys->Fix[Fix.getFilePath()].add(Fix);
- if (Err) {
- // FIXME: Implement better conflict handling.
- llvm::errs() << "Fix conflicts with existing fix: "
- << llvm::toString(std::move(Err)) << "\n";
- }
- }
}
};
Index: clang/include/clang/Tooling/Core/Diagnostic.h
===================================================================
--- clang/include/clang/Tooling/Core/Diagnostic.h
+++ clang/include/clang/Tooling/Core/Diagnostic.h
@@ -42,6 +42,9 @@
std::string Message;
std::string FilePath;
unsigned FileOffset;
+
+ /// Fixes for this diagnostic, grouped by file path.
+ llvm::StringMap<Replacements> Fix;
};
/// Represents the diagnostic with the level of severity and possible
@@ -58,19 +61,20 @@
StringRef BuildDirectory);
Diagnostic(llvm::StringRef DiagnosticName, const DiagnosticMessage &Message,
- const llvm::StringMap<Replacements> &Fix,
const SmallVector<DiagnosticMessage, 1> &Notes, Level DiagLevel,
llvm::StringRef BuildDirectory);
+ // Get the chosen fix to apply for this diagnostic.
+ // FIXME: currently we choose the first non-empty fix, extend it to support
+ // fix selection.
+ const llvm::StringMap<Replacements> *getChosenFix() const;
+
/// Name identifying the Diagnostic.
std::string DiagnosticName;
/// Message associated to the diagnostic.
DiagnosticMessage Message;
- /// Fixes to apply, grouped by file path.
- llvm::StringMap<Replacements> Fix;
-
/// Potential notes about the diagnostic.
SmallVector<DiagnosticMessage, 1> Notes;
Index: clang-tools-extra/unittests/clang-tidy/ClangTidyTest.h
===================================================================
--- clang-tools-extra/unittests/clang-tidy/ClangTidyTest.h
+++ clang-tools-extra/unittests/clang-tidy/ClangTidyTest.h
@@ -14,6 +14,7 @@
#include "clang/ASTMatchers/ASTMatchFinder.h"
#include "clang/Frontend/CompilerInstance.h"
#include "clang/Frontend/FrontendActions.h"
+#include "clang/Tooling/Core/Replacement.h"
#include "clang/Tooling/Refactoring.h"
#include "clang/Tooling/Tooling.h"
#include "llvm/ADT/Optional.h"
@@ -131,16 +132,17 @@
tooling::Replacements Fixes;
std::vector<ClangTidyError> Diags = DiagConsumer.take();
for (const ClangTidyError &Error : Diags) {
- for (const auto &FileAndFixes : Error.Fix) {
- for (const auto &Fix : FileAndFixes.second) {
- auto Err = Fixes.add(Fix);
- // FIXME: better error handling. Keep the behavior for now.
- if (Err) {
- llvm::errs() << llvm::toString(std::move(Err)) << "\n";
- return "";
+ if (const auto *ChosenFix = Error.getChosenFix())
+ for (const auto &FileAndFixes : *ChosenFix) {
+ for (const auto &Fix : FileAndFixes.second) {
+ auto Err = Fixes.add(Fix);
+ // FIXME: better error handling. Keep the behavior for now.
+ if (Err) {
+ llvm::errs() << llvm::toString(std::move(Err)) << "\n";
+ return "";
+ }
}
}
- }
}
if (Errors)
*Errors = std::move(Diags);
Index: clang-tools-extra/unittests/clang-apply-replacements/ApplyReplacementsTest.cpp
===================================================================
--- clang-tools-extra/unittests/clang-apply-replacements/ApplyReplacementsTest.cpp
+++ clang-tools-extra/unittests/clang-apply-replacements/ApplyReplacementsTest.cpp
@@ -26,7 +26,6 @@
TUs.push_back({MainSourceFile,
{{DiagnosticName,
Message,
- Replacements,
{},
Diagnostic::Warning,
BuildDirectory}}});
Index: clang-tools-extra/test/clang-tidy/export-diagnostics.cpp
===================================================================
--- clang-tools-extra/test/clang-tidy/export-diagnostics.cpp
+++ clang-tools-extra/test/clang-tidy/export-diagnostics.cpp
@@ -13,16 +13,19 @@
// CHECK-YAML-NEXT: MainSourceFile: '{{.*}}-input.cpp'
// CHECK-YAML-NEXT: Diagnostics:
// CHECK-YAML-NEXT: - DiagnosticName: clang-diagnostic-missing-prototypes
-// CHECK-YAML-NEXT: Message: 'no previous prototype for function ''ff'''
-// CHECK-YAML-NEXT: FileOffset: 30
-// CHECK-YAML-NEXT: FilePath: '{{.*}}-input.cpp'
+// CHECK-YAML-NEXT: DiagnosticMessage:
+// CHECK-YAML-NEXT: Message: 'no previous prototype for function
+// ''ff'''
+// CHECK-YAML-NEXT: FilePath: '{{.*}}-input.cpp'
+// CHECK-YAML-NEXT: FileOffset: 30
+// CHECK-YAML-NEXT: Replacements: []
// CHECK-YAML-NEXT: Notes:
// CHECK-YAML-NEXT: - Message: 'expanded from macro ''X'''
// CHECK-YAML-NEXT: FilePath: '{{.*}}-input.cpp'
// CHECK-YAML-NEXT: FileOffset: 18
+// CHECK-YAML-NEXT: Replacements: []
// CHECK-YAML-NEXT: - Message: expanded from here
// CHECK-YAML-NEXT: FilePath: ''
// CHECK-YAML-NEXT: FileOffset: 0
-// CHECK-YAML-NEXT: Replacements: []
+// CHECK-YAML-NEXT: Replacements: []
// CHECK-YAML-NEXT: ...
-
Index: clang-tools-extra/test/clang-apply-replacements/Inputs/order-dependent/file2.yaml
===================================================================
--- clang-tools-extra/test/clang-apply-replacements/Inputs/order-dependent/file2.yaml
+++ clang-tools-extra/test/clang-apply-replacements/Inputs/order-dependent/file2.yaml
@@ -2,12 +2,13 @@
MainSourceFile: order-dependent.cpp
Diagnostics:
- DiagnosticName: test-order-dependent-insertion
- Message: Fix
- FilePath: $(path)/order-dependent.cpp
- FileOffset: 12
- Replacements:
- - FilePath: $(path)/order-dependent.cpp
- Offset: 12
- Length: 0
- ReplacementText: '1'
+ DiagnosticMessage:
+ Message: Fix
+ FilePath: $(path)/order-dependent.cpp
+ FileOffset: 12
+ Replacements:
+ - FilePath: $(path)/order-dependent.cpp
+ Offset: 12
+ Length: 0
+ ReplacementText: '1'
...
Index: clang-tools-extra/test/clang-apply-replacements/Inputs/order-dependent/file1.yaml
===================================================================
--- clang-tools-extra/test/clang-apply-replacements/Inputs/order-dependent/file1.yaml
+++ clang-tools-extra/test/clang-apply-replacements/Inputs/order-dependent/file1.yaml
@@ -2,12 +2,13 @@
MainSourceFile: order-dependent.cpp
Diagnostics:
- DiagnosticName: test-order-dependent-insertion
- Message: Fix
- FilePath: $(path)/order-dependent.cpp
- FileOffset: 12
- Replacements:
- - FilePath: $(path)/order-dependent.cpp
- Offset: 12
- Length: 0
- ReplacementText: '0'
+ DiagnosticMessage:
+ Message: Fix
+ FilePath: $(path)/order-dependent.cpp
+ FileOffset: 12
+ Replacements:
+ - FilePath: $(path)/order-dependent.cpp
+ Offset: 12
+ Length: 0
+ ReplacementText: '0'
...
Index: clang-tools-extra/test/clang-apply-replacements/Inputs/identical/file2.yaml
===================================================================
--- clang-tools-extra/test/clang-apply-replacements/Inputs/identical/file2.yaml
+++ clang-tools-extra/test/clang-apply-replacements/Inputs/identical/file2.yaml
@@ -2,13 +2,14 @@
MainSourceFile: identical.cpp
Diagnostics:
- DiagnosticName: test-identical-insertion
- Message: Fix
- FilePath: $(path)/identical.cpp
- FileOffset: 12
- Replacements:
- - FilePath: $(path)/identical.cpp
- Offset: 12
- Length: 0
- ReplacementText: '0'
+ DiagnosticMessage:
+ Message: Fix
+ FilePath: $(path)/identical.cpp
+ FileOffset: 12
+ Replacements:
+ - FilePath: $(path)/identical.cpp
+ Offset: 12
+ Length: 0
+ ReplacementText: '0'
...
Index: clang-tools-extra/test/clang-apply-replacements/Inputs/identical/file1.yaml
===================================================================
--- clang-tools-extra/test/clang-apply-replacements/Inputs/identical/file1.yaml
+++ clang-tools-extra/test/clang-apply-replacements/Inputs/identical/file1.yaml
@@ -2,13 +2,14 @@
MainSourceFile: identical.cpp
Diagnostics:
- DiagnosticName: test-identical-insertion
- Message: Fix
- FilePath: $(path)/identical.cpp
- FileOffset: 12
- Replacements:
- - FilePath: $(path)/identical.cpp
- Offset: 12
- Length: 0
- ReplacementText: '0'
+ DiagnosticMessage:
+ Message: Fix
+ FilePath: $(path)/identical.cpp
+ FileOffset: 12
+ Replacements:
+ - FilePath: $(path)/identical.cpp
+ Offset: 12
+ Length: 0
+ ReplacementText: '0'
...
Index: clang-tools-extra/test/clang-apply-replacements/Inputs/format/yes.yaml
===================================================================
--- clang-tools-extra/test/clang-apply-replacements/Inputs/format/yes.yaml
+++ clang-tools-extra/test/clang-apply-replacements/Inputs/format/yes.yaml
@@ -4,24 +4,25 @@
MainSourceFile: yes.cpp
Diagnostics:
- DiagnosticName: test-yes
- Message: Fix
- FilePath: $(path)/yes.cpp
- FileOffset: 494
- Replacements:
- - FilePath: $(path)/yes.cpp
- Offset: 494
- Length: 1
- ReplacementText: nullptr
- - FilePath: $(path)/yes.cpp
- Offset: 410
- Length: 1
- ReplacementText: nullptr
- - FilePath: $(path)/yes.cpp
- Offset: 454
- Length: 1
- ReplacementText: nullptr
- - FilePath: $(path)/yes.cpp
- Offset: 108
- Length: 38
- ReplacementText: 'auto '
+ DiagnosticMessage:
+ Message: Fix
+ FilePath: $(path)/yes.cpp
+ FileOffset: 494
+ Replacements:
+ - FilePath: $(path)/yes.cpp
+ Offset: 494
+ Length: 1
+ ReplacementText: nullptr
+ - FilePath: $(path)/yes.cpp
+ Offset: 410
+ Length: 1
+ ReplacementText: nullptr
+ - FilePath: $(path)/yes.cpp
+ Offset: 454
+ Length: 1
+ ReplacementText: nullptr
+ - FilePath: $(path)/yes.cpp
+ Offset: 108
+ Length: 38
+ ReplacementText: 'auto '
...
Index: clang-tools-extra/test/clang-apply-replacements/Inputs/format/no.yaml
===================================================================
--- clang-tools-extra/test/clang-apply-replacements/Inputs/format/no.yaml
+++ clang-tools-extra/test/clang-apply-replacements/Inputs/format/no.yaml
@@ -2,12 +2,13 @@
MainSourceFile: no.cpp
Diagnostics:
- DiagnosticName: test-no
- Message: Fix
- FilePath: $(path)/no.cpp
- FileOffset: 94
- Replacements:
- - FilePath: $(path)/no.cpp
- Offset: 94
- Length: 3
- ReplacementText: 'auto '
+ DiagnosticMessage:
+ Message: Fix
+ FilePath: $(path)/no.cpp
+ FileOffset: 94
+ Replacements:
+ - FilePath: $(path)/no.cpp
+ Offset: 94
+ Length: 3
+ ReplacementText: 'auto '
...
Index: clang-tools-extra/test/clang-apply-replacements/Inputs/crlf/file1.yaml
===================================================================
--- clang-tools-extra/test/clang-apply-replacements/Inputs/crlf/file1.yaml
+++ clang-tools-extra/test/clang-apply-replacements/Inputs/crlf/file1.yaml
@@ -2,12 +2,13 @@
MainSourceFile: source1.cpp
Diagnostics:
- DiagnosticName: test-crlf
- Message: Fix
- FilePath: $(path)/crlf.cpp
- FileOffset: 79
- Replacements:
- - FilePath: $(path)/crlf.cpp
- Offset: 79
- Length: 1
- ReplacementText: nullptr
+ DiagnosticMessage:
+ Message: Fix
+ FilePath: $(path)/crlf.cpp
+ FileOffset: 79
+ Replacements:
+ - FilePath: $(path)/crlf.cpp
+ Offset: 79
+ Length: 1
+ ReplacementText: nullptr
...
Index: clang-tools-extra/test/clang-apply-replacements/Inputs/conflict/file3.yaml
===================================================================
--- clang-tools-extra/test/clang-apply-replacements/Inputs/conflict/file3.yaml
+++ clang-tools-extra/test/clang-apply-replacements/Inputs/conflict/file3.yaml
@@ -2,12 +2,13 @@
MainSourceFile: source1.cpp
Diagnostics:
- DiagnosticName: test-conflict
- Message: Fix
- FilePath: $(path)/common.h
- FileOffset: 169
- Replacements:
- - FilePath: $(path)/common.h
- Offset: 169
- Length: 0
- ReplacementText: "(int*)"
+ DiagnosticMessage:
+ Message: Fix
+ FilePath: $(path)/common.h
+ FileOffset: 169
+ Replacements:
+ - FilePath: $(path)/common.h
+ Offset: 169
+ Length: 0
+ ReplacementText: "(int*)"
...
Index: clang-tools-extra/test/clang-apply-replacements/Inputs/conflict/file2.yaml
===================================================================
--- clang-tools-extra/test/clang-apply-replacements/Inputs/conflict/file2.yaml
+++ clang-tools-extra/test/clang-apply-replacements/Inputs/conflict/file2.yaml
@@ -2,20 +2,21 @@
MainSourceFile: source2.cpp
Diagnostics:
- DiagnosticName: test-conflict
- Message: Fix
- FilePath: $(path)/common.h
- FileOffset: 106
- Replacements:
- - FilePath: $(path)/common.h
- Offset: 106
- Length: 26
- ReplacementText: 'int & elem : ints'
- - FilePath: $(path)/common.h
- Offset: 140
- Length: 7
- ReplacementText: elem
- - FilePath: $(path)/common.h
- Offset: 169
- Length: 1
- ReplacementText: nullptr
+ DiagnosticMessage:
+ Message: Fix
+ FilePath: $(path)/common.h
+ FileOffset: 106
+ Replacements:
+ - FilePath: $(path)/common.h
+ Offset: 106
+ Length: 26
+ ReplacementText: 'int & elem : ints'
+ - FilePath: $(path)/common.h
+ Offset: 140
+ Length: 7
+ ReplacementText: elem
+ - FilePath: $(path)/common.h
+ Offset: 169
+ Length: 1
+ ReplacementText: nullptr
...
Index: clang-tools-extra/test/clang-apply-replacements/Inputs/conflict/file1.yaml
===================================================================
--- clang-tools-extra/test/clang-apply-replacements/Inputs/conflict/file1.yaml
+++ clang-tools-extra/test/clang-apply-replacements/Inputs/conflict/file1.yaml
@@ -2,20 +2,21 @@
MainSourceFile: source1.cpp
Diagnostics:
- DiagnosticName: test-conflict
- Message: Fix
- FilePath: $(path)/common.h
- FileOffset: 106
- Replacements:
- - FilePath: $(path)/common.h
- Offset: 106
- Length: 26
- ReplacementText: 'auto & i : ints'
- - FilePath: $(path)/common.h
- Offset: 140
- Length: 7
- ReplacementText: i
- - FilePath: $(path)/common.h
- Offset: 160
- Length: 12
- ReplacementText: ''
+ DiagnosticMessage:
+ Message: Fix
+ FilePath: $(path)/common.h
+ FileOffset: 106
+ Replacements:
+ - FilePath: $(path)/common.h
+ Offset: 106
+ Length: 26
+ ReplacementText: 'auto & i : ints'
+ - FilePath: $(path)/common.h
+ Offset: 140
+ Length: 7
+ ReplacementText: i
+ - FilePath: $(path)/common.h
+ Offset: 160
+ Length: 12
+ ReplacementText: ''
...
Index: clang-tools-extra/test/clang-apply-replacements/Inputs/basic/file2.yaml
===================================================================
--- clang-tools-extra/test/clang-apply-replacements/Inputs/basic/file2.yaml
+++ clang-tools-extra/test/clang-apply-replacements/Inputs/basic/file2.yaml
@@ -2,12 +2,13 @@
MainSourceFile: source2.cpp
Diagnostics:
- DiagnosticName: test-basic
- Message: Fix
- FilePath: $(path)/basic.h
- FileOffset: 148
- Replacements:
- - FilePath: $(path)/../basic/basic.h
- Offset: 298
- Length: 1
- ReplacementText: elem
+ DiagnosticMessage:
+ Message: Fix
+ FilePath: $(path)/basic.h
+ FileOffset: 148
+ Replacements:
+ - FilePath: $(path)/../basic/basic.h
+ Offset: 298
+ Length: 1
+ ReplacementText: elem
...
Index: clang-tools-extra/test/clang-apply-replacements/Inputs/basic/file1.yaml
===================================================================
--- clang-tools-extra/test/clang-apply-replacements/Inputs/basic/file1.yaml
+++ clang-tools-extra/test/clang-apply-replacements/Inputs/basic/file1.yaml
@@ -2,24 +2,25 @@
MainSourceFile: source1.cpp
Diagnostics:
- DiagnosticName: test-basic
- Message: Fix
- FilePath: $(path)/basic.h
- FileOffset: 242
- Replacements:
- - FilePath: $(path)/basic.h
- Offset: 242
- Length: 26
- ReplacementText: 'auto & elem : ints'
- - FilePath: $(path)/basic.h
- Offset: 276
- Length: 22
- ReplacementText: ''
- - FilePath: $(path)/basic.h
- Offset: 298
- Length: 1
- ReplacementText: elem
- - FilePath: $(path)/../basic/basic.h
- Offset: 148
- Length: 0
- ReplacementText: 'override '
+ DiagnosticMessage:
+ Message: Fix
+ FilePath: $(path)/basic.h
+ FileOffset: 242
+ Replacements:
+ - FilePath: $(path)/basic.h
+ Offset: 242
+ Length: 26
+ ReplacementText: 'auto & elem : ints'
+ - FilePath: $(path)/basic.h
+ Offset: 276
+ Length: 22
+ ReplacementText: ''
+ - FilePath: $(path)/basic.h
+ Offset: 298
+ Length: 1
+ ReplacementText: elem
+ - FilePath: $(path)/../basic/basic.h
+ Offset: 148
+ Length: 0
+ ReplacementText: 'override '
...
Index: clang-tools-extra/clang-tidy/misc/UnusedUsingDeclsCheck.cpp
===================================================================
--- clang-tools-extra/clang-tidy/misc/UnusedUsingDeclsCheck.cpp
+++ clang-tools-extra/clang-tidy/misc/UnusedUsingDeclsCheck.cpp
@@ -154,7 +154,10 @@
for (const auto &Context : Contexts) {
if (!Context.IsUsed) {
diag(Context.FoundUsingDecl->getLocation(), "using decl %0 is unused")
- << Context.FoundUsingDecl
+ << Context.FoundUsingDecl;
+ // Emit a fix and a fix description of the check;
+ diag(Context.FoundUsingDecl->getLocation(),
+ /*FixDescription=*/"remove the using", DiagnosticIDs::Note)
<< FixItHint::CreateRemoval(Context.UsingDeclRange);
}
}
Index: clang-tools-extra/clang-tidy/add_new_check.py
===================================================================
--- clang-tools-extra/clang-tidy/add_new_check.py
+++ clang-tools-extra/clang-tidy/add_new_check.py
@@ -137,7 +137,8 @@
if (MatchedDecl->getName().startswith("awesome_"))
return;
diag(MatchedDecl->getLocation(), "function %%0 is insufficiently awesome")
- << MatchedDecl
+ << MatchedDecl;
+ diag(MatchedDecl->getLocation(), "insert 'awesome'", DiagnosticIDs::Note)
<< FixItHint::CreateInsertion(MatchedDecl->getLocation(), "awesome_");
}
Index: clang-tools-extra/clang-tidy/ClangTidyDiagnosticConsumer.cpp
===================================================================
--- clang-tools-extra/clang-tidy/ClangTidyDiagnosticConsumer.cpp
+++ clang-tools-extra/clang-tidy/ClangTidyDiagnosticConsumer.cpp
@@ -18,8 +18,10 @@
#include "ClangTidyDiagnosticConsumer.h"
#include "ClangTidyOptions.h"
#include "clang/AST/ASTDiagnostic.h"
+#include "clang/Basic/Diagnostic.h"
#include "clang/Basic/DiagnosticOptions.h"
#include "clang/Frontend/DiagnosticRenderer.h"
+#include "clang/Tooling/Core/Diagnostic.h"
#include "llvm/ADT/STLExtras.h"
#include "llvm/ADT/SmallString.h"
#include <tuple>
@@ -68,6 +70,9 @@
SmallVectorImpl<CharSourceRange> &Ranges,
ArrayRef<FixItHint> Hints) override {
assert(Loc.isValid());
+ tooling::DiagnosticMessage *DiagWithFix =
+ Level == DiagnosticsEngine::Note ? &Error.Notes.back() : &Error.Message;
+
for (const auto &FixIt : Hints) {
CharSourceRange Range = FixIt.RemoveRange;
assert(Range.getBegin().isValid() && Range.getEnd().isValid() &&
@@ -77,7 +82,8 @@
tooling::Replacement Replacement(Loc.getManager(), Range,
FixIt.CodeToInsert);
- llvm::Error Err = Error.Fix[Replacement.getFilePath()].add(Replacement);
+ llvm::Error Err =
+ DiagWithFix->Fix[Replacement.getFilePath()].add(Replacement);
// FIXME: better error handling (at least, don't let other replacements be
// applied).
if (Err) {
@@ -581,9 +587,17 @@
// Compute error sizes.
std::vector<int> Sizes;
- for (const auto &Error : Errors) {
+ std::vector<
+ std::pair<ClangTidyError *, llvm::StringMap<tooling::Replacements> *>>
+ ErrorFixes;
+ for (auto &Error : Errors) {
+ if (const auto *Fix = Error.getChosenFix())
+ ErrorFixes.emplace_back(
+ &Error, const_cast<llvm::StringMap<tooling::Replacements> *>(Fix));
+ }
+ for (const auto &ErrorAndFix : ErrorFixes) {
int Size = 0;
- for (const auto &FileAndReplaces : Error.Fix) {
+ for (const auto &FileAndReplaces : *ErrorAndFix.second) {
for (const auto &Replace : FileAndReplaces.second)
Size += Replace.getLength();
}
@@ -592,8 +606,8 @@
// Build events from error intervals.
std::map<std::string, std::vector<Event>> FileEvents;
- for (unsigned I = 0; I < Errors.size(); ++I) {
- for (const auto &FileAndReplace : Errors[I].Fix) {
+ for (unsigned I = 0; I < ErrorFixes.size(); ++I) {
+ for (const auto &FileAndReplace : *ErrorFixes[I].second) {
for (const auto &Replace : FileAndReplace.second) {
unsigned Begin = Replace.getOffset();
unsigned End = Begin + Replace.getLength();
@@ -608,7 +622,7 @@
}
}
- std::vector<bool> Apply(Errors.size(), true);
+ std::vector<bool> Apply(ErrorFixes.size(), true);
for (auto &FileAndEvents : FileEvents) {
std::vector<Event> &Events = FileAndEvents.second;
// Sweep.
@@ -627,10 +641,10 @@
assert(OpenIntervals == 0 && "Amount of begin/end points doesn't match");
}
- for (unsigned I = 0; I < Errors.size(); ++I) {
+ for (unsigned I = 0; I < ErrorFixes.size(); ++I) {
if (!Apply[I]) {
- Errors[I].Fix.clear();
- Errors[I].Notes.emplace_back(
+ ErrorFixes[I].second->clear();
+ ErrorFixes[I].first->Notes.emplace_back(
"this fix will not be applied because it overlaps with another fix");
}
}
Index: clang-tools-extra/clang-tidy/ClangTidy.cpp
===================================================================
--- clang-tools-extra/clang-tidy/ClangTidy.cpp
+++ clang-tools-extra/clang-tidy/ClangTidy.cpp
@@ -35,6 +35,7 @@
#include "clang/Lex/Preprocessor.h"
#include "clang/Rewrite/Frontend/FixItRewriter.h"
#include "clang/Rewrite/Frontend/FrontendActions.h"
+#include "clang/Tooling/Core/Diagnostic.h"
#if CLANG_ENABLE_STATIC_ANALYZER
#include "clang/StaticAnalyzer/Core/BugReporter/PathDiagnostic.h"
#include "clang/StaticAnalyzer/Frontend/AnalysisConsumer.h"
@@ -125,15 +126,17 @@
}
auto Diag = Diags.Report(Loc, Diags.getCustomDiagID(Level, "%0 [%1]"))
<< Message.Message << Name;
- for (const auto &FileAndReplacements : Error.Fix) {
- for (const auto &Repl : FileAndReplacements.second) {
- SourceLocation FixLoc;
- ++TotalFixes;
- bool CanBeApplied = false;
- if (Repl.isApplicable()) {
- SmallString<128> FixAbsoluteFilePath = Repl.getFilePath();
- Files.makeAbsolutePath(FixAbsoluteFilePath);
- if (ApplyFixes) {
+
+ const llvm::StringMap<Replacements> *ChosenFix = Error.getChosenFix();
+ if (ApplyFixes && ChosenFix) {
+ for (const auto &FileAndReplacements : *ChosenFix) {
+ for (const auto &Repl : FileAndReplacements.second) {
+ ++TotalFixes;
+ bool CanBeApplied = false;
+ if (Repl.isApplicable()) {
+ SourceLocation FixLoc;
+ SmallString<128> FixAbsoluteFilePath = Repl.getFilePath();
+ Files.makeAbsolutePath(FixAbsoluteFilePath);
tooling::Replacement R(FixAbsoluteFilePath, Repl.getOffset(),
Repl.getLength(),
Repl.getReplacementText());
@@ -158,28 +161,17 @@
llvm::errs()
<< "Can't resolve conflict, skipping the replacement.\n";
}
-
} else {
CanBeApplied = true;
++AppliedFixes;
}
+ FixLoc = getLocation(FixAbsoluteFilePath, Repl.getOffset());
+ FixLocations.push_back(std::make_pair(FixLoc, CanBeApplied));
}
- FixLoc = getLocation(FixAbsoluteFilePath, Repl.getOffset());
- SourceLocation FixEndLoc =
- FixLoc.getLocWithOffset(Repl.getLength());
- // Retrieve the source range for applicable fixes. Macro definitions
- // on the command line have locations in a virtual buffer and don't
- // have valid file paths and are therefore not applicable.
- CharSourceRange Range =
- CharSourceRange::getCharRange(SourceRange(FixLoc, FixEndLoc));
- Diag << FixItHint::CreateReplacement(Range,
- Repl.getReplacementText());
}
-
- if (ApplyFixes)
- FixLocations.push_back(std::make_pair(FixLoc, CanBeApplied));
}
}
+ reportFix(Diag, Error.Message.Fix);
}
for (auto Fix : FixLocations) {
Diags.Report(Fix.first, Fix.second ? diag::note_fixit_applied
@@ -250,10 +242,33 @@
return SourceMgr.getLocForStartOfFile(ID).getLocWithOffset(Offset);
}
+ void reportFix(const DiagnosticBuilder &Diag,
+ const llvm::StringMap<Replacements> &Fix) {
+ for (const auto &FileAndReplacements : Fix) {
+ for (const auto &Repl : FileAndReplacements.second) {
+ if (!Repl.isApplicable())
+ continue;
+ SmallString<128> FixAbsoluteFilePath = Repl.getFilePath();
+ Files.makeAbsolutePath(FixAbsoluteFilePath);
+ SourceLocation FixLoc =
+ getLocation(FixAbsoluteFilePath, Repl.getOffset());
+ SourceLocation FixEndLoc = FixLoc.getLocWithOffset(Repl.getLength());
+ // Retrieve the source range for applicable fixes. Macro definitions
+ // on the command line have locations in a virtual buffer and don't
+ // have valid file paths and are therefore not applicable.
+ CharSourceRange Range =
+ CharSourceRange::getCharRange(SourceRange(FixLoc, FixEndLoc));
+ Diag << FixItHint::CreateReplacement(Range, Repl.getReplacementText());
+ }
+ }
+ }
+
void reportNote(const tooling::DiagnosticMessage &Message) {
SourceLocation Loc = getLocation(Message.FilePath, Message.FileOffset);
- Diags.Report(Loc, Diags.getCustomDiagID(DiagnosticsEngine::Note, "%0"))
+ auto Diag =
+ Diags.Report(Loc, Diags.getCustomDiagID(DiagnosticsEngine::Note, "%0"))
<< Message.Message;
+ reportFix(Diag, Message.Fix);
}
FileManager Files;
Index: clang-tools-extra/clang-apply-replacements/lib/Tooling/ApplyReplacements.cpp
===================================================================
--- clang-tools-extra/clang-apply-replacements/lib/Tooling/ApplyReplacements.cpp
+++ clang-tools-extra/clang-apply-replacements/lib/Tooling/ApplyReplacements.cpp
@@ -169,9 +169,11 @@
for (const auto &TU : TUDs)
for (const auto &D : TU.Diagnostics)
- for (const auto &Fix : D.Fix)
- for (const tooling::Replacement &R : Fix.second)
- AddToGroup(R, true);
+ if (const auto *ChoosenFix = D.getChosenFix()) {
+ for (const auto &Fix : *ChoosenFix)
+ for (const tooling::Replacement &R : Fix.second)
+ AddToGroup(R, true);
+ }
// Sort replacements per file to keep consistent behavior when
// clang-apply-replacements run on differents machine.
_______________________________________________
cfe-commits mailing list
cfe-commits@lists.llvm.org
https://lists.llvm.org/cgi-bin/mailman/listinfo/cfe-commits
