[clang-tools-extra] r320713 - Add support for NOLINT and NOLINTNEXTLINE comments mentioning specific check names.
Aaron Ballman via cfe-commits
cfe-commits at lists.llvm.org
Thu Dec 14 08:13:57 PST 2017
Author: aaronballman
Date: Thu Dec 14 08:13:57 2017
New Revision: 320713
URL: http://llvm.org/viewvc/llvm-project?rev=320713&view=rev
Log:
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).
Modified:
clang-tools-extra/trunk/clang-tidy/ClangTidyDiagnosticConsumer.cpp
clang-tools-extra/trunk/docs/ReleaseNotes.rst
clang-tools-extra/trunk/docs/clang-tidy/index.rst
clang-tools-extra/trunk/test/clang-tidy/nolint.cpp
clang-tools-extra/trunk/test/clang-tidy/nolintnextline.cpp
Modified: clang-tools-extra/trunk/clang-tidy/ClangTidyDiagnosticConsumer.cpp
URL: http://llvm.org/viewvc/llvm-project/clang-tools-extra/trunk/clang-tidy/ClangTidyDiagnosticConsumer.cpp?rev=320713&r1=320712&r2=320713&view=diff
==============================================================================
--- clang-tools-extra/trunk/clang-tidy/ClangTidyDiagnosticConsumer.cpp (original)
+++ clang-tools-extra/trunk/clang-tidy/ClangTidyDiagnosticConsumer.cpp Thu Dec 14 08:13:57 2017
@@ -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::finali
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(Sourc
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(Sourc
--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::Handle
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;
Modified: clang-tools-extra/trunk/docs/ReleaseNotes.rst
URL: http://llvm.org/viewvc/llvm-project/clang-tools-extra/trunk/docs/ReleaseNotes.rst?rev=320713&r1=320712&r2=320713&view=diff
==============================================================================
--- clang-tools-extra/trunk/docs/ReleaseNotes.rst (original)
+++ clang-tools-extra/trunk/docs/ReleaseNotes.rst Thu Dec 14 08:13:57 2017
@@ -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
-----------------------------
Modified: clang-tools-extra/trunk/docs/clang-tidy/index.rst
URL: http://llvm.org/viewvc/llvm-project/clang-tools-extra/trunk/docs/clang-tidy/index.rst?rev=320713&r1=320712&r2=320713&view=diff
==============================================================================
--- clang-tools-extra/trunk/docs/clang-tidy/index.rst (original)
+++ clang-tools-extra/trunk/docs/clang-tidy/index.rst Thu Dec 14 08:13:57 2017
@@ -250,6 +250,56 @@ An overview of all the command-line opti
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
Modified: clang-tools-extra/trunk/test/clang-tidy/nolint.cpp
URL: http://llvm.org/viewvc/llvm-project/clang-tools-extra/trunk/test/clang-tidy/nolint.cpp?rev=320713&r1=320712&r2=320713&view=diff
==============================================================================
--- clang-tools-extra/trunk/test/clang-tidy/nolint.cpp (original)
+++ clang-tools-extra/trunk/test/clang-tidy/nolint.cpp Thu Dec 14 08:13:57 2017
@@ -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)
Modified: clang-tools-extra/trunk/test/clang-tidy/nolintnextline.cpp
URL: http://llvm.org/viewvc/llvm-project/clang-tools-extra/trunk/test/clang-tidy/nolintnextline.cpp?rev=320713&r1=320712&r2=320713&view=diff
==============================================================================
--- clang-tools-extra/trunk/test/clang-tidy/nolintnextline.cpp (original)
+++ clang-tools-extra/trunk/test/clang-tidy/nolintnextline.cpp Thu Dec 14 08:13:57 2017
@@ -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 --
More information about the cfe-commits
mailing list