[clang-tidy] Add check name to YAML export (clang-tools-extra part)

Add a field indicating the associated check for every replacement to the YAML
report generated with the '-export-fixes' option.  Update
clang-apply-replacements to handle the new format.

Patch by Alpha Abdoulaye!

Differential revision: https://reviews.llvm.org/D26137

llvm-svn: 290893
This commit is contained in:
Alexander Kornienko 2017-01-03 14:36:13 +00:00
parent 9bab199146
commit 563de799e3
16 changed files with 242 additions and 177 deletions

View File

@ -16,6 +16,7 @@
#ifndef LLVM_CLANG_APPLYREPLACEMENTS_H
#define LLVM_CLANG_APPLYREPLACEMENTS_H
#include "clang/Tooling/Core/Diagnostic.h"
#include "clang/Tooling/Refactoring.h"
#include "llvm/ADT/StringMap.h"
#include "llvm/ADT/StringRef.h"
@ -43,6 +44,9 @@ typedef std::vector<clang::tooling::TranslationUnitReplacements> TUReplacements;
/// \brief Collection of TranslationUnitReplacement files.
typedef std::vector<std::string> TUReplacementFiles;
/// \brief Collection of TranslationUniDiagnostics.
typedef std::vector<clang::tooling::TranslationUnitDiagnostics> TUDiagnostics;
/// \brief Map mapping file name to Replacements targeting that file.
typedef llvm::DenseMap<const clang::FileEntry *,
std::vector<clang::tooling::Replacement>>
@ -58,8 +62,8 @@ typedef llvm::DenseMap<const clang::FileEntry *,
/// \param[in] Directory Directory to begin search for serialized
/// TranslationUnitReplacements.
/// \param[out] TUs Collection of all found and deserialized
/// TranslationUnitReplacements.
/// \param[out] TURFiles Collection of all TranslationUnitReplacement files
/// TranslationUnitReplacements or TranslationUnitDiagnostics.
/// \param[out] TUFiles Collection of all TranslationUnitReplacement files
/// found in \c Directory.
/// \param[in] Diagnostics DiagnosticsEngine used for error output.
///
@ -67,7 +71,11 @@ typedef llvm::DenseMap<const clang::FileEntry *,
/// directory structure.
std::error_code collectReplacementsFromDirectory(
const llvm::StringRef Directory, TUReplacements &TUs,
TUReplacementFiles &TURFiles, clang::DiagnosticsEngine &Diagnostics);
TUReplacementFiles &TUFiles, clang::DiagnosticsEngine &Diagnostics);
std::error_code collectReplacementsFromDirectory(
const llvm::StringRef Directory, TUDiagnostics &TUs,
TUReplacementFiles &TUFiles, clang::DiagnosticsEngine &Diagnostics);
/// \brief Deduplicate, check for conflicts, and apply all Replacements stored
/// in \c TUs. If conflicts occur, no Replacements are applied.
@ -75,7 +83,8 @@ std::error_code collectReplacementsFromDirectory(
/// \post For all (key,value) in GroupedReplacements, value[i].getOffset() <=
/// value[i+1].getOffset().
///
/// \param[in] TUs Collection of TranslationUnitReplacements to merge,
/// \param[in] TUs Collection of TranslationUnitReplacements or
/// TranslationUnitDiagnostics to merge,
/// deduplicate, and test for conflicts.
/// \param[out] GroupedReplacements Container grouping all Replacements by the
/// file they target.
@ -88,6 +97,10 @@ bool mergeAndDeduplicate(const TUReplacements &TUs,
FileToReplacementsMap &GroupedReplacements,
clang::SourceManager &SM);
bool mergeAndDeduplicate(const TUDiagnostics &TUs,
FileToReplacementsMap &GroupedReplacements,
clang::SourceManager &SM);
// FIXME: Remove this function after changing clang-apply-replacements to use
// Replacements class.
bool applyAllReplacements(const std::vector<tooling::Replacement> &Replaces,

View File

@ -20,6 +20,7 @@
#include "clang/Format/Format.h"
#include "clang/Lex/Lexer.h"
#include "clang/Rewrite/Core/Rewriter.h"
#include "clang/Tooling/DiagnosticsYaml.h"
#include "clang/Tooling/ReplacementsYaml.h"
#include "llvm/ADT/ArrayRef.h"
#include "llvm/Support/FileSystem.h"
@ -37,7 +38,7 @@ namespace replace {
std::error_code collectReplacementsFromDirectory(
const llvm::StringRef Directory, TUReplacements &TUs,
TUReplacementFiles &TURFiles, clang::DiagnosticsEngine &Diagnostics) {
TUReplacementFiles &TUFiles, clang::DiagnosticsEngine &Diagnostics) {
using namespace llvm::sys::fs;
using namespace llvm::sys::path;
@ -54,7 +55,7 @@ std::error_code collectReplacementsFromDirectory(
if (extension(I->path()) != ".yaml")
continue;
TURFiles.push_back(I->path());
TUFiles.push_back(I->path());
ErrorOr<std::unique_ptr<MemoryBuffer>> Out =
MemoryBuffer::getFile(I->path());
@ -79,6 +80,51 @@ std::error_code collectReplacementsFromDirectory(
return ErrorCode;
}
std::error_code
collectReplacementsFromDirectory(const llvm::StringRef Directory,
TUDiagnostics &TUs, TUReplacementFiles &TUFiles,
clang::DiagnosticsEngine &Diagnostics) {
using namespace llvm::sys::fs;
using namespace llvm::sys::path;
std::error_code ErrorCode;
for (recursive_directory_iterator I(Directory, ErrorCode), E;
I != E && !ErrorCode; I.increment(ErrorCode)) {
if (filename(I->path())[0] == '.') {
// Indicate not to descend into directories beginning with '.'
I.no_push();
continue;
}
if (extension(I->path()) != ".yaml")
continue;
TUFiles.push_back(I->path());
ErrorOr<std::unique_ptr<MemoryBuffer>> Out =
MemoryBuffer::getFile(I->path());
if (std::error_code BufferError = Out.getError()) {
errs() << "Error reading " << I->path() << ": " << BufferError.message()
<< "\n";
continue;
}
yaml::Input YIn(Out.get()->getBuffer(), nullptr, &eatDiagnostics);
tooling::TranslationUnitDiagnostics TU;
YIn >> TU;
if (YIn.error()) {
// File doesn't appear to be a header change description. Ignore it.
continue;
}
// Only keep files that properly parse.
TUs.push_back(TU);
}
return ErrorCode;
}
/// \brief Dumps information for a sequence of conflicting Replacements.
///
/// \param[in] File FileEntry for the file the conflicting Replacements are
@ -256,6 +302,34 @@ bool mergeAndDeduplicate(const TUReplacements &TUs,
return !deduplicateAndDetectConflicts(GroupedReplacements, SM);
}
bool mergeAndDeduplicate(const TUDiagnostics &TUs,
FileToReplacementsMap &GroupedReplacements,
clang::SourceManager &SM) {
// Group all replacements by target file.
std::set<StringRef> Warned;
for (const auto &TU : TUs) {
for (const auto &D : TU.Diagnostics) {
for (const auto &Fix : D.Fix) {
for (const tooling::Replacement &R : Fix.second) {
// Use the file manager to deduplicate paths. FileEntries are
// automatically canonicalized.
const FileEntry *Entry = SM.getFileManager().getFile(R.getFilePath());
if (!Entry && Warned.insert(R.getFilePath()).second) {
errs() << "Described file '" << R.getFilePath()
<< "' doesn't exist. Ignoring...\n";
continue;
}
GroupedReplacements[Entry].push_back(R);
}
}
}
}
// Ask clang to deduplicate and report conflicts.
return !deduplicateAndDetectConflicts(GroupedReplacements, SM);
}
bool applyReplacements(const FileToReplacementsMap &GroupedReplacements,
clang::Rewriter &Rewrites) {

View File

@ -66,7 +66,7 @@ static cl::opt<std::string>
cl::init("LLVM"), cl::cat(FormattingCategory));
namespace {
// Helper object to remove the TUReplacement files (triggered by
// Helper object to remove the TUReplacement and TUDiagnostic (triggered by
// "remove-change-desc-files" command line option) when exiting current scope.
class ScopedFileRemover {
public:
@ -211,11 +211,16 @@ int main(int argc, char **argv) {
if (DoFormat)
FormatStyle = format::getStyle(FormatStyleOpt, FormatStyleConfig, "LLVM");
TUReplacements TUs;
TUReplacementFiles TURFiles;
TUReplacements TURs;
TUReplacementFiles TUFiles;
std::error_code ErrorCode =
collectReplacementsFromDirectory(Directory, TUs, TURFiles, Diagnostics);
collectReplacementsFromDirectory(Directory, TURs, TUFiles, Diagnostics);
TUDiagnostics TUDs;
TUFiles.clear();
ErrorCode =
collectReplacementsFromDirectory(Directory, TUDs, TUFiles, Diagnostics);
if (ErrorCode) {
errs() << "Trouble iterating over directory '" << Directory
@ -227,13 +232,15 @@ int main(int argc, char **argv) {
// command line option) when exiting main().
std::unique_ptr<ScopedFileRemover> Remover;
if (RemoveTUReplacementFiles)
Remover.reset(new ScopedFileRemover(TURFiles, Diagnostics));
Remover.reset(new ScopedFileRemover(TUFiles, Diagnostics));
FileManager Files((FileSystemOptions()));
SourceManager SM(Diagnostics, Files);
FileToReplacementsMap GroupedReplacements;
if (!mergeAndDeduplicate(TUs, GroupedReplacements, SM))
if (!mergeAndDeduplicate(TURs, GroupedReplacements, SM))
return 1;
if (!mergeAndDeduplicate(TUDs, GroupedReplacements, SM))
return 1;
Rewriter ReplacementsRewriter(SM, LangOptions());

View File

@ -35,6 +35,7 @@
#include "clang/Rewrite/Frontend/FrontendActions.h"
#include "clang/StaticAnalyzer/Core/BugReporter/PathDiagnostic.h"
#include "clang/StaticAnalyzer/Frontend/AnalysisConsumer.h"
#include "clang/Tooling/DiagnosticsYaml.h"
#include "clang/Tooling/Refactoring.h"
#include "clang/Tooling/ReplacementsYaml.h"
#include "clang/Tooling/Tooling.h"
@ -102,14 +103,14 @@ public:
SourceManager &getSourceManager() { return SourceMgr; }
void reportDiagnostic(const ClangTidyError &Error) {
const ClangTidyMessage &Message = Error.Message;
const tooling::DiagnosticMessage &Message = Error.Message;
SourceLocation Loc = getLocation(Message.FilePath, Message.FileOffset);
// Contains a pair for each attempted fix: location and whether the fix was
// applied successfully.
SmallVector<std::pair<SourceLocation, bool>, 4> FixLocations;
{
auto Level = static_cast<DiagnosticsEngine::Level>(Error.DiagLevel);
std::string Name = Error.CheckName;
std::string Name = Error.DiagnosticName;
if (Error.IsWarningAsError) {
Name += ",-warnings-as-errors";
Level = DiagnosticsEngine::Error;
@ -177,7 +178,7 @@ public:
Diags.Report(Fix.first, Fix.second ? diag::note_fixit_applied
: diag::note_fixit_failed);
}
for (const ClangTidyMessage &Note : Error.Notes)
for (const auto &Note : Error.Notes)
reportNote(Note);
}
@ -229,9 +230,8 @@ private:
return SourceMgr.getLocForStartOfFile(ID).getLocWithOffset(Offset);
}
void reportNote(const ClangTidyMessage &Message) {
void reportNote(const tooling::DiagnosticMessage &Message) {
SourceLocation Loc = getLocation(Message.FilePath, Message.FileOffset);
DiagnosticBuilder Diag =
Diags.Report(Loc, Diags.getCustomDiagID(DiagnosticsEngine::Note, "%0"))
<< Message.Message;
}
@ -562,18 +562,18 @@ void handleErrors(const std::vector<ClangTidyError> &Errors, bool Fix,
WarningsAsErrorsCount += Reporter.getWarningsAsErrorsCount();
}
void exportReplacements(const std::vector<ClangTidyError> &Errors,
void exportReplacements(const llvm::StringRef MainFilePath,
const std::vector<ClangTidyError> &Errors,
raw_ostream &OS) {
TranslationUnitReplacements TUR;
for (const ClangTidyError &Error : Errors) {
for (const auto &FileAndFixes : Error.Fix)
TUR.Replacements.insert(TUR.Replacements.end(),
FileAndFixes.second.begin(),
FileAndFixes.second.end());
TranslationUnitDiagnostics TUD;
TUD.MainSourceFile = MainFilePath;
for (const auto &Error : Errors) {
tooling::Diagnostic Diag = Error;
TUD.Diagnostics.insert(TUD.Diagnostics.end(), Diag);
}
yaml::Output YAML(OS);
YAML << TUR;
YAML << TUD;
}
} // namespace tidy

View File

@ -242,7 +242,8 @@ void handleErrors(const std::vector<ClangTidyError> &Errors, bool Fix,
/// \brief Serializes replacements into YAML and writes them to the specified
/// output stream.
void exportReplacements(const std::vector<ClangTidyError> &Errors,
void exportReplacements(StringRef MainFilePath,
const std::vector<ClangTidyError> &Errors,
raw_ostream &OS);
} // end namespace tidy

View File

@ -7,8 +7,8 @@
//
//===----------------------------------------------------------------------===//
///
/// \file This file implements ClangTidyDiagnosticConsumer, ClangTidyMessage,
/// ClangTidyContext and ClangTidyError classes.
/// \file This file implements ClangTidyDiagnosticConsumer, ClangTidyContext
/// and ClangTidyError classes.
///
/// This tool uses the Clang Tooling infrastructure, see
/// http://clang.llvm.org/docs/HowToSetupToolingForLLVM.html
@ -45,13 +45,13 @@ protected:
// FIXME: Remove this once there's a better way to pass check names than
// appending the check name to the message in ClangTidyContext::diag and
// using getCustomDiagID.
std::string CheckNameInMessage = " [" + Error.CheckName + "]";
std::string CheckNameInMessage = " [" + Error.DiagnosticName + "]";
if (Message.endswith(CheckNameInMessage))
Message = Message.substr(0, Message.size() - CheckNameInMessage.size());
ClangTidyMessage TidyMessage = Loc.isValid()
? ClangTidyMessage(Message, *SM, Loc)
: ClangTidyMessage(Message);
auto TidyMessage = Loc.isValid()
? tooling::DiagnosticMessage(Message, *SM, Loc)
: tooling::DiagnosticMessage(Message);
if (Level == DiagnosticsEngine::Note) {
Error.Notes.push_back(TidyMessage);
return;
@ -110,23 +110,11 @@ private:
};
} // end anonymous namespace
ClangTidyMessage::ClangTidyMessage(StringRef Message)
: Message(Message), FileOffset(0) {}
ClangTidyMessage::ClangTidyMessage(StringRef Message,
const SourceManager &Sources,
SourceLocation Loc)
: Message(Message) {
assert(Loc.isValid() && Loc.isFileID());
FilePath = Sources.getFilename(Loc);
FileOffset = Sources.getFileOffset(Loc);
}
ClangTidyError::ClangTidyError(StringRef CheckName,
ClangTidyError::Level DiagLevel,
bool IsWarningAsError, StringRef BuildDirectory)
: CheckName(CheckName), BuildDirectory(BuildDirectory),
DiagLevel(DiagLevel), IsWarningAsError(IsWarningAsError) {}
StringRef BuildDirectory, bool IsWarningAsError)
: tooling::Diagnostic(CheckName, DiagLevel, BuildDirectory),
IsWarningAsError(IsWarningAsError) {}
// Returns true if GlobList starts with the negative indicator ('-'), removes it
// from the GlobList.
@ -260,7 +248,7 @@ ClangTidyDiagnosticConsumer::ClangTidyDiagnosticConsumer(ClangTidyContext &Ctx)
void ClangTidyDiagnosticConsumer::finalizeLastError() {
if (!Errors.empty()) {
ClangTidyError &Error = Errors.back();
if (!Context.getChecksFilter().contains(Error.CheckName) &&
if (!Context.getChecksFilter().contains(Error.DiagnosticName) &&
Error.DiagLevel != ClangTidyError::Error) {
++Context.Stats.ErrorsIgnoredCheckFilter;
Errors.pop_back();
@ -366,8 +354,8 @@ void ClangTidyDiagnosticConsumer::HandleDiagnostic(
bool IsWarningAsError =
DiagLevel == DiagnosticsEngine::Warning &&
Context.getWarningAsErrorFilter().contains(CheckName);
Errors.push_back(ClangTidyError(CheckName, Level, IsWarningAsError,
Context.getCurrentBuildDirectory()));
Errors.emplace_back(CheckName, Level, Context.getCurrentBuildDirectory(),
IsWarningAsError);
}
ClangTidyDiagnosticRenderer Converter(
@ -532,10 +520,9 @@ void ClangTidyDiagnosticConsumer::removeIncompatibleErrors(
// FIXME: Handle empty intervals, such as those from insertions.
if (Begin == End)
continue;
FileEvents[FilePath].push_back(
Event(Begin, End, Event::ET_Begin, I, Sizes[I]));
FileEvents[FilePath].push_back(
Event(Begin, End, Event::ET_End, I, Sizes[I]));
auto &Events = FileEvents[FilePath];
Events.emplace_back(Begin, End, Event::ET_Begin, I, Sizes[I]);
Events.emplace_back(Begin, End, Event::ET_End, I, Sizes[I]);
}
}
}
@ -562,9 +549,8 @@ void ClangTidyDiagnosticConsumer::removeIncompatibleErrors(
for (unsigned I = 0; I < Errors.size(); ++I) {
if (!Apply[I]) {
Errors[I].Fix.clear();
Errors[I].Notes.push_back(
ClangTidyMessage("this fix will not be applied because"
" it overlaps with another fix"));
Errors[I].Notes.emplace_back(
"this fix will not be applied because it overlaps with another fix");
}
}
}
@ -572,8 +558,8 @@ void ClangTidyDiagnosticConsumer::removeIncompatibleErrors(
namespace {
struct LessClangTidyError {
bool operator()(const ClangTidyError &LHS, const ClangTidyError &RHS) const {
const ClangTidyMessage &M1 = LHS.Message;
const ClangTidyMessage &M2 = RHS.Message;
const tooling::DiagnosticMessage &M1 = LHS.Message;
const tooling::DiagnosticMessage &M2 = RHS.Message;
return std::tie(M1.FilePath, M1.FileOffset, M1.Message) <
std::tie(M2.FilePath, M2.FileOffset, M2.Message);

View File

@ -13,6 +13,7 @@
#include "ClangTidyOptions.h"
#include "clang/Basic/Diagnostic.h"
#include "clang/Basic/SourceManager.h"
#include "clang/Tooling/Core/Diagnostic.h"
#include "clang/Tooling/Refactoring.h"
#include "llvm/ADT/DenseMap.h"
#include "llvm/ADT/StringMap.h"
@ -32,18 +33,6 @@ class CompilationDatabase;
namespace tidy {
/// \brief A message from a clang-tidy check.
///
/// Note that this is independent of a \c SourceManager.
struct ClangTidyMessage {
ClangTidyMessage(StringRef Message = "");
ClangTidyMessage(StringRef Message, const SourceManager &Sources,
SourceLocation Loc);
std::string Message;
std::string FilePath;
unsigned FileOffset;
};
/// \brief A detected error complete with information to display diagnostic and
/// automatic fix.
///
@ -51,31 +40,10 @@ struct ClangTidyMessage {
/// dependency on a SourceManager.
///
/// FIXME: Make Diagnostics flexible enough to support this directly.
struct ClangTidyError {
enum Level {
Warning = DiagnosticsEngine::Warning,
Error = DiagnosticsEngine::Error
};
struct ClangTidyError : tooling::Diagnostic {
ClangTidyError(StringRef CheckName, Level DiagLevel, StringRef BuildDirectory,
bool IsWarningAsError);
ClangTidyError(StringRef CheckName, Level DiagLevel, bool IsWarningAsError,
StringRef BuildDirectory);
std::string CheckName;
ClangTidyMessage Message;
// Fixes grouped by file path.
llvm::StringMap<tooling::Replacements> Fix;
SmallVector<ClangTidyMessage, 1> Notes;
// A build directory of the diagnostic source file.
//
// It's an absolute path which is `directory` field of the source file in
// compilation database. If users don't specify the compilation database
// directory, it is the current directory where clang-tidy runs.
//
// Note: it is empty in unittest.
std::string BuildDirectory;
Level DiagLevel;
bool IsWarningAsError;
};

View File

@ -403,7 +403,7 @@ static int clangTidyMain(int argc, const char **argv) {
llvm::errs() << "Error opening output file: " << EC.message() << '\n';
return 1;
}
exportReplacements(Errors, OS);
exportReplacements(FilePath.str(), Errors, OS);
}
printStats(Stats);

View File

@ -1,5 +1,7 @@
---
MainSourceFile: source1.cpp
Diagnostics:
- DiagnosticName: test-basic
Replacements:
- FilePath: $(path)/basic.h
Offset: 242

View File

@ -1,5 +1,7 @@
---
MainSourceFile: source2.cpp
Diagnostics:
- DiagnosticName: test-basic
Replacements:
- FilePath: $(path)/basic.h
Offset: 148

View File

@ -1,5 +1,7 @@
---
MainSourceFile: source1.cpp
Diagnostics:
- DiagnosticName: test-conflict
Replacements:
- FilePath: $(path)/common.h
Offset: 106

View File

@ -1,5 +1,7 @@
---
MainSourceFile: source2.cpp
Diagnostics:
- DiagnosticName: test-conflict
Replacements:
- FilePath: $(path)/common.h
Offset: 106

View File

@ -1,5 +1,7 @@
---
MainSourceFile: source1.cpp
Diagnostics:
- DiagnosticName: test-conflict
Replacements:
- FilePath: $(path)/common.h
Offset: 169

View File

@ -1,5 +1,7 @@
---
MainSourceFile: source1.cpp
Diagnostics:
- DiagnosticName: test-crlf
Replacements:
- FilePath: $(path)/crlf.cpp
Offset: 79

View File

@ -1,5 +1,7 @@
---
MainSourceFile: no.cpp
Diagnostics:
- DiagnosticName: test-no
Replacements:
- FilePath: $(path)/no.cpp
Offset: 94

View File

@ -2,6 +2,8 @@
# so that formatting happens correctly.
---
MainSourceFile: yes.cpp
Diagnostics:
- DiagnosticName: test-yes
Replacements:
- FilePath: $(path)/yes.cpp
Offset: 494