diff options
author | Aaron Ballman <aaron@aaronballman.com> | 2017-12-14 16:13:57 +0000 |
---|---|---|
committer | Aaron Ballman <aaron@aaronballman.com> | 2017-12-14 16:13:57 +0000 |
commit | ffd3d6489a842b5e2b5da6eed099b3c730ebb9ba (patch) | |
tree | 7d3415d2d9d315418d5d3c72a8200980b8441efe | |
parent | 64b8582ecca09bf279c5a815a82486befcf50341 (diff) |
Add support for NOLINT and NOLINTNEXTLINE comments mentioning specific check names.
Supports a comma-separated list of check names to be disabled on the given line. Also supports * as a wildcard to disable all lint diagnostic messages on that line.
Patch by Anton (xgsa).
git-svn-id: https://llvm.org/svn/llvm-project/clang-tools-extra/trunk@320713 91177308-0d34-0410-b5e6-96231b3b80d8
-rw-r--r-- | clang-tidy/ClangTidyDiagnosticConsumer.cpp | 49 | ||||
-rw-r--r-- | docs/ReleaseNotes.rst | 2 | ||||
-rw-r--r-- | docs/clang-tidy/index.rst | 50 | ||||
-rw-r--r-- | test/clang-tidy/nolint.cpp | 15 | ||||
-rw-r--r-- | test/clang-tidy/nolintnextline.cpp | 20 |
5 files changed, 124 insertions, 12 deletions
diff --git a/clang-tidy/ClangTidyDiagnosticConsumer.cpp b/clang-tidy/ClangTidyDiagnosticConsumer.cpp index 44f78b8a..1939a36a 100644 --- a/clang-tidy/ClangTidyDiagnosticConsumer.cpp +++ b/clang-tidy/ClangTidyDiagnosticConsumer.cpp @@ -21,6 +21,7 @@ #include "clang/AST/ASTDiagnostic.h" #include "clang/Basic/DiagnosticOptions.h" #include "clang/Frontend/DiagnosticRenderer.h" +#include "llvm/ADT/STLExtras.h" #include "llvm/ADT/SmallString.h" #include <tuple> #include <vector> @@ -290,7 +291,38 @@ void ClangTidyDiagnosticConsumer::finalizeLastError() { LastErrorPassesLineFilter = false; } -static bool LineIsMarkedWithNOLINT(SourceManager &SM, SourceLocation Loc) { +static bool IsNOLINTFound(StringRef NolintDirectiveText, StringRef Line, + unsigned DiagID, const ClangTidyContext &Context) { + const size_t NolintIndex = Line.find(NolintDirectiveText); + if (NolintIndex == StringRef::npos) + return false; + + size_t BracketIndex = NolintIndex + NolintDirectiveText.size(); + // Check if the specific checks are specified in brackets. + if (BracketIndex < Line.size() && Line[BracketIndex] == '(') { + ++BracketIndex; + const size_t BracketEndIndex = Line.find(')', BracketIndex); + if (BracketEndIndex != StringRef::npos) { + StringRef ChecksStr = + Line.substr(BracketIndex, BracketEndIndex - BracketIndex); + // Allow disabling all the checks with "*". + if (ChecksStr != "*") { + StringRef CheckName = Context.getCheckName(DiagID); + // Allow specifying a few check names, delimited with comma. + SmallVector<StringRef, 1> Checks; + ChecksStr.split(Checks, ',', -1, false); + llvm::transform(Checks, Checks.begin(), + [](StringRef S) { return S.trim(); }); + return llvm::find(Checks, CheckName) != Checks.end(); + } + } + } + return true; +} + +static bool LineIsMarkedWithNOLINT(SourceManager &SM, SourceLocation Loc, + unsigned DiagID, + const ClangTidyContext &Context) { bool Invalid; const char *CharacterData = SM.getCharacterData(Loc, &Invalid); if (Invalid) @@ -301,8 +333,7 @@ static bool LineIsMarkedWithNOLINT(SourceManager &SM, SourceLocation Loc) { while (*P != '\0' && *P != '\r' && *P != '\n') ++P; StringRef RestOfLine(CharacterData, P - CharacterData + 1); - // FIXME: Handle /\bNOLINT\b(\([^)]*\))?/ as cpplint.py does. - if (RestOfLine.find("NOLINT") != StringRef::npos) + if (IsNOLINTFound("NOLINT", RestOfLine, DiagID, Context)) return true; // Check if there's a NOLINTNEXTLINE on the previous line. @@ -329,16 +360,17 @@ static bool LineIsMarkedWithNOLINT(SourceManager &SM, SourceLocation Loc) { --P; RestOfLine = StringRef(P, LineEnd - P + 1); - if (RestOfLine.find("NOLINTNEXTLINE") != StringRef::npos) + if (IsNOLINTFound("NOLINTNEXTLINE", RestOfLine, DiagID, Context)) return true; return false; } -static bool LineIsMarkedWithNOLINTinMacro(SourceManager &SM, - SourceLocation Loc) { +static bool LineIsMarkedWithNOLINTinMacro(SourceManager &SM, SourceLocation Loc, + unsigned DiagID, + const ClangTidyContext &Context) { while (true) { - if (LineIsMarkedWithNOLINT(SM, Loc)) + if (LineIsMarkedWithNOLINT(SM, Loc, DiagID, Context)) return true; if (!Loc.isMacroID()) return false; @@ -355,7 +387,8 @@ void ClangTidyDiagnosticConsumer::HandleDiagnostic( if (Info.getLocation().isValid() && DiagLevel != DiagnosticsEngine::Error && DiagLevel != DiagnosticsEngine::Fatal && LineIsMarkedWithNOLINTinMacro(Diags->getSourceManager(), - Info.getLocation())) { + Info.getLocation(), Info.getID(), + Context)) { ++Context.Stats.ErrorsIgnoredNOLINT; // Ignored a warning, should ignore related notes as well LastErrorWasIgnored = true; diff --git a/docs/ReleaseNotes.rst b/docs/ReleaseNotes.rst index f60e812b..ea312d93 100644 --- a/docs/ReleaseNotes.rst +++ b/docs/ReleaseNotes.rst @@ -261,6 +261,8 @@ Improvements to clang-tidy - `hicpp-use-nullptr <http://clang.llvm.org/extra/clang-tidy/checks/hicpp-use-nullptr.html>`_ - `hicpp-vararg <http://clang.llvm.org/extra/clang-tidy/checks/hicpp-vararg.html>`_ +- Added the ability to suppress specific checks (or all checks) in a ``NOLINT`` or ``NOLINTNEXTLINE`` comment. + Improvements to include-fixer ----------------------------- diff --git a/docs/clang-tidy/index.rst b/docs/clang-tidy/index.rst index 642a2df2..35292d00 100644 --- a/docs/clang-tidy/index.rst +++ b/docs/clang-tidy/index.rst @@ -250,6 +250,56 @@ An overview of all the command-line options: value: 'some value' ... +:program:`clang-tidy` diagnostics are intended to call out code that does +not adhere to a coding standard, or is otherwise problematic in some way. +However, if it is known that the code is correct, the check-specific ways +to silence the diagnostics could be used, if they are available (e.g. +bugprone-use-after-move can be silenced by re-initializing the variable after +it has been moved out, misc-string-integer-assignment can be suppressed by +explicitly casting the integer to char, readability-implicit-bool-conversion +can also be suppressed by using explicit casts, etc.). If they are not +available or if changing the semantics of the code is not desired, +the ``NOLINT`` or ``NOLINTNEXTLINE`` comments can be used instead. For example: + +.. code-block:: c++ + + class Foo + { + // Silent all the diagnostics for the line + Foo(int param); // NOLINT + + // Silent only the specified checks for the line + Foo(double param); // NOLINT(google-explicit-constructor, google-runtime-int) + + // Silent only the specified diagnostics for the next line + // NOLINTNEXTLINE(google-explicit-constructor, google-runtime-int) + Foo(bool param); + }; + +The formal syntax of ``NOLINT``/``NOLINTNEXTLINE`` is the following: + +.. parsed-literal:: + + lint-comment: + lint-command + lint-command lint-args + + lint-args: + **(** check-name-list **)** + + check-name-list: + *check-name* + check-name-list **,** *check-name* + + lint-command: + **NOLINT** + **NOLINTNEXTLINE** + +Note that whitespaces between ``NOLINT``/``NOLINTNEXTLINE`` and the opening +parenthesis are not allowed (in this case the comment will be treated just as +``NOLINT``/``NOLINTNEXTLINE``), whereas in check names list (inside +the parenthesis) whitespaces can be used and will be ignored. + .. _LibTooling: http://clang.llvm.org/docs/LibTooling.html .. _How To Setup Tooling For LLVM: http://clang.llvm.org/docs/HowToSetupToolingForLLVM.html diff --git a/test/clang-tidy/nolint.cpp b/test/clang-tidy/nolint.cpp index 7e828bfd..893e8192 100644 --- a/test/clang-tidy/nolint.cpp +++ b/test/clang-tidy/nolint.cpp @@ -13,7 +13,18 @@ class A { A(int i); }; class B { B(int i); }; // NOLINT -class C { C(int i); }; // NOLINT(we-dont-care-about-categories-yet) +class C { C(int i); }; // NOLINT(for-some-other-check) +// CHECK-MESSAGES: :[[@LINE-1]]:11: warning: single-argument constructors must be marked explicit + +class C1 { C1(int i); }; // NOLINT(*) + +class C2 { C2(int i); }; // NOLINT(not-closed-bracket-is-treated-as-skip-all + +class C3 { C3(int i); }; // NOLINT(google-explicit-constructor) + +class C4 { C4(int i); }; // NOLINT(some-check, google-explicit-constructor) + +class C5 { C5(int i); }; // NOLINT without-brackets-skip-all, another-check void f() { int i; @@ -35,4 +46,4 @@ MACRO_NOLINT #define DOUBLE_MACRO MACRO(H) // NOLINT DOUBLE_MACRO -// CHECK-MESSAGES: Suppressed 8 warnings (8 NOLINT) +// CHECK-MESSAGES: Suppressed 12 warnings (12 NOLINT) diff --git a/test/clang-tidy/nolintnextline.cpp b/test/clang-tidy/nolintnextline.cpp index d18f335c..a97928ae 100644 --- a/test/clang-tidy/nolintnextline.cpp +++ b/test/clang-tidy/nolintnextline.cpp @@ -4,8 +4,24 @@ class A { A(int i); }; // NOLINTNEXTLINE class B { B(int i); }; -// NOLINTNEXTLINE(we-dont-care-about-categories-yet) +// NOLINTNEXTLINE(for-some-other-check) class C { C(int i); }; +// CHECK-MESSAGES: :[[@LINE-1]]:11: warning: single-argument constructors must be marked explicit + +// NOLINTNEXTLINE(*) +class C1 { C1(int i); }; + +// NOLINTNEXTLINE(not-closed-bracket-is-treated-as-skip-all +class C2 { C2(int i); }; + +// NOLINTNEXTLINE(google-explicit-constructor) +class C3 { C3(int i); }; + +// NOLINTNEXTLINE(some-check, google-explicit-constructor) +class C4 { C4(int i); }; + +// NOLINTNEXTLINE without-brackets-skip-all, another-check +class C5 { C5(int i); }; // NOLINTNEXTLINE @@ -28,6 +44,6 @@ MACRO(G) // NOLINTNEXTLINE MACRO_NOARG -// CHECK-MESSAGES: Suppressed 4 warnings (4 NOLINT) +// CHECK-MESSAGES: Suppressed 8 warnings (8 NOLINT) // RUN: %check_clang_tidy %s google-explicit-constructor %t -- |