HomePhabricator

[clang-tidy] Add fix descriptions to clang-tidy checks.

Description

[clang-tidy] Add fix descriptions to clang-tidy checks.

Summary:
Motivation/Context: in the code review system integrating with clang-tidy,
clang-tidy doesn't provide a human-readable description of the fix. Usually
developers have to preview a code diff (before vs after apply the fix) to
understand what the fix does before applying a fix.

This patch proposes that each clang-tidy check provides a short and
actional fix description that can be shown in the UI, so that users can know
what the fix does without previewing diff.

This patch extends clang-tidy framework to support fix descriptions (will add implementations for
existing checks in the future). Fix descriptions and fixes are emitted via diagnostic::Note (rather than
attaching the main warning diagnostic).

Before this patch:

void MyCheck::check(...) {
   ...
   diag(loc, "my check warning") <<  FixtItHint::CreateReplacement(...);
}

After:

void MyCheck::check(...) {
   ...
   diag(loc, "my check warning"); // Emit a check warning
   diag(loc, "fix description", DiagnosticIDs::Note) << FixtItHint::CreateReplacement(...); // Emit a diagnostic note and a fix
}

Reviewers: sammccall, alexfh

Reviewed By: alexfh

Subscribers: MyDeveloperDay, Eugene.Zelenko, aaron.ballman, JonasToth, xazax.hun, jdoerfert, cfe-commits

Tags: #clang-tools-extra, #clang

Differential Revision: https://reviews.llvm.org/D59932

Details

Committed
hokeinApr 17 2019, 5:53 AM
Reviewer
alexfh
Differential Revision
D59932: [clang-tidy] Add fix descriptions to clang-tidy checks.
Parents
rL358575: [clangd] Include textual diagnostic ID as Diagnostic.code.
Branches
Unknown
Tags
Unknown

Event Timeline

A few post-commit comments.

/cfe/trunk/include/clang/Tooling/Core/Diagnostic.h
96

Use Doxygen comments (///).

97

s/Return/\returns/
s/attached/are attached/

/clang-tools-extra/trunk/clang-tidy/ClangTidy.cpp
136

Please use early continue here to avoid extra nesting.

174

Could you remind me why we

  1. use Error.Message.Fix instead of ChosenFix
  2. don't check for ApplyFixes here

?

hokein marked an inline comment as done.Apr 17 2019, 10:57 AM

A few post-commit comments.

Thanks, will address them tomorrow.

/clang-tools-extra/trunk/clang-tidy/ClangTidy.cpp
174

Yes, it is intended.

reportFix is used to print the fix to the console, clang-tidy would print all alternative fixes (Error.Message, and Error.Notes); when clang-tidy applies Fix, we only choose a single one, that is ChosenFix.