Index: clang-tidy/ClangTidy.cpp =================================================================== --- clang-tidy/ClangTidy.cpp +++ clang-tidy/ClangTidy.cpp @@ -340,6 +340,7 @@ Context.setSourceManager(&Compiler.getSourceManager()); Context.setCurrentFile(File); Context.setASTContext(&Compiler.getASTContext()); + Context.setPreprocessor(&Compiler.getPreprocessor()); auto WorkingDir = Compiler.getSourceManager() .getFileManager() Index: clang-tidy/ClangTidyDiagnosticConsumer.h =================================================================== --- clang-tidy/ClangTidyDiagnosticConsumer.h +++ clang-tidy/ClangTidyDiagnosticConsumer.h @@ -19,11 +19,14 @@ #include "llvm/ADT/StringMap.h" #include "llvm/Support/Regex.h" #include "llvm/Support/Timer.h" +#include +#include namespace clang { class ASTContext; class CompilerInstance; +class Preprocessor; namespace ast_matchers { class MatchFinder; } @@ -32,6 +35,7 @@ } namespace tidy { +class NolintCommentsCollector; /// \brief A detected error complete with information to display diagnostic and /// automatic fix. @@ -131,13 +135,24 @@ /// \brief Sets ASTContext for the current translation unit. void setASTContext(ASTContext *Context); + /// \brief Subscribes the context to the preprocessor events if necessary. + void setPreprocessor(Preprocessor *preprocessor); + /// \brief Gets the language options from the AST context. const LangOptions &getLangOpts() const { return LangOpts; } + /// \brief Checks if a check with the specified name exists. + bool hasCheck(StringRef CheckName); + /// \brief Returns the name of the clang-tidy check which produced this /// diagnostic ID. StringRef getCheckName(unsigned DiagnosticID) const; + /// \brief Returns the name of diagnostics as it will be output in report + /// for the specified diagnostic ID. It can be either a check name or + /// clang diagnostics with "clang-diagnostic-" prefix. + std::string getDiagnosticsName(unsigned DiagnosticID) const; + /// \brief Returns \c true if the check is enabled for the \c CurrentFile. /// /// The \c CurrentFile can be changed using \c setCurrentFile. @@ -187,7 +202,7 @@ } private: - // Calls setDiagnosticsEngine() and storeError(). + // Calls setDiagnosticsEngine(), storeError() and getNolintCollector(). friend class ClangTidyDiagnosticConsumer; friend class ClangTidyPluginAction; @@ -198,6 +213,9 @@ /// \brief Store an \p Error. void storeError(const ClangTidyError &Error); + // \brief Provides access to the \c NOLINT/NOLINTNEXTLINE info collector. + NolintCommentsCollector& getNolintCollector(); + std::vector Errors; DiagnosticsEngine *DiagEngine; std::unique_ptr OptionsProvider; @@ -217,6 +235,9 @@ llvm::DenseMap CheckNamesByDiagnosticID; ProfileData *Profile; + + std::unique_ptr NolintCollector; + std::vector AllCheckNamesCache; }; /// \brief A diagnostic consumer that turns each \c Diagnostic into a @@ -241,8 +262,17 @@ private: void finalizeLastError(); + bool IsLineMarkedWithNolintInMacro(SourceManager &SM, SourceLocation Loc, + StringRef CheckName); + bool LineIsMarkedWithNolint(SourceManager &SM, SourceLocation Loc, + StringRef CheckName); + bool IsNolintFound(StringRef NolintDirectiveText, StringRef Line, + StringRef CheckName); + void removeIncompatibleErrors(SmallVectorImpl &Errors) const; + void reportRedundantNolintComments(); + /// \brief Returns the \c HeaderFilter constructed for the options set in the /// context. llvm::Regex *getHeaderFilter(); @@ -260,6 +290,9 @@ bool LastErrorRelatesToUserCode; bool LastErrorPassesLineFilter; bool LastErrorWasIgnored; + // Unfortunately, line numbers are not stored in Errors, so gather them + // separately to process NOLINT misuse. + std::unordered_map> DiagIDsToLineNumbers; }; } // end namespace tidy Index: clang-tidy/ClangTidyDiagnosticConsumer.cpp =================================================================== --- clang-tidy/ClangTidyDiagnosticConsumer.cpp +++ clang-tidy/ClangTidyDiagnosticConsumer.cpp @@ -18,11 +18,16 @@ #include "ClangTidyDiagnosticConsumer.h" #include "ClangTidyOptions.h" +#include "ClangTidy.h" +#include "ClangTidyModule.h" #include "clang/AST/ASTDiagnostic.h" #include "clang/Basic/DiagnosticOptions.h" #include "clang/Frontend/DiagnosticRenderer.h" +#include "clang/Lex/Preprocessor.h" +#include "llvm/ADT/Hashing.h" #include "llvm/ADT/STLExtras.h" #include "llvm/ADT/SmallString.h" +#include #include #include using namespace clang; @@ -105,6 +110,67 @@ private: ClangTidyError &Error; }; + +const StringRef ClangDiagnosticsPrefix = "clang-diagnostic-"; +const StringRef NolintCheckName = "nolint-usage"; +enum class NolintCommentType : uint8_t { + Nolint, + NolintNextLine, +}; +const StringRef NolintComment = "NOLINT"; +const StringRef NolintNextLineComment = "NOLINTNEXTLINE"; + +class NolintCommentParser { +public: + NolintCommentParser(StringRef NolintDirectiveText, StringRef Line) { + if ((NolintIndex = Line.find(NolintDirectiveText)) != StringRef::npos) { + 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 ((ClosingBracketFound = BracketEndIndex != StringRef::npos)) { + StringRef ChecksStr = + Line.substr(BracketIndex, BracketEndIndex - BracketIndex); + // Allow disabling all the checks with "*". + if (ChecksStr != "*") { + // Allow specifying a few check names, delimited with comma. + ChecksStr.split(CheckNames, ',', -1, false); + llvm::transform(CheckNames, CheckNames.begin(), + [](StringRef S) { return S.trim(); }); + } + } + } + } + } + + bool isNolintFound() const { + return NolintIndex != StringRef::npos; + } + + size_t getNolintIndex() const { + assert(isNolintFound()); + return NolintIndex; + } + + bool isAppliedToAllChecks() const { + return CheckNames.empty(); + } + + const SmallVector& getCheckNames() const { + assert(isNolintFound()); + assert(!isAppliedToAllChecks()); + return CheckNames; + } + + bool isClosingBracketFound() const { + return ClosingBracketFound; + } +private: + SmallVector CheckNames; + size_t NolintIndex = 0; + bool ClosingBracketFound = true; +}; } // end anonymous namespace ClangTidyError::ClangTidyError(StringRef CheckName, @@ -176,10 +242,103 @@ llvm::StringMap Cache; }; +class clang::tidy::NolintCommentsCollector : public CommentHandler { +public: + struct NolintTargetInfo { + unsigned LineNumber; + StringRef CheckName; + }; + struct NolintCommentInfo { + SourceLocation Location; + NolintCommentType Type; + }; + static const StringRef AnyCheck; + +private: + struct NolintTargetInfoHasher { + size_t operator() (const NolintTargetInfo &i) const { + return llvm::hash_combine(i.LineNumber, i.CheckName); + } + }; + struct NolintTargetInfoEq { + bool operator()(const NolintTargetInfo &i1, + const NolintTargetInfo &i2) const { + return i1.LineNumber == i2.LineNumber && i1.CheckName == i2.CheckName; + } + }; + +public: + NolintCommentsCollector(ClangTidyContext* Context) + : Context(Context) {} + + using NolintMap = std::unordered_map; + NolintMap takeNolintMap() { + NolintMap result; + std::swap(result, NolintComments); + return result; + } + +private: + bool HandleComment(Preprocessor &PP, SourceRange Range) override { + SourceManager &SrcMgr = PP.getSourceManager(); + StringRef Text = Lexer::getSourceText(CharSourceRange::getCharRange(Range), + SrcMgr, PP.getLangOpts()); + // Search for NOLINTNEXTLINE first (as NOLINT could be a part of it). + StringRef CommentName = NolintNextLineComment; + NolintCommentType CommentType = NolintCommentType::NolintNextLine; + unsigned LineLocationOffset = 1; + NolintCommentParser NolintParser(CommentName, Text); + if (!NolintParser.isNolintFound()) { + // Search for NOLINT otherwise. + CommentName = NolintComment; + CommentType = NolintCommentType::Nolint; + LineLocationOffset = 0; + NolintParser = NolintCommentParser(CommentName, Text); + } + if (NolintParser.isNolintFound()) { + SourceLocation NolintLoc = + Range.getBegin().getLocWithOffset(NolintParser.getNolintIndex()); + unsigned LineNo = SrcMgr.getExpansionLineNumber(Range.getBegin()) + LineLocationOffset; + static const SmallVector AllChecks = { AnyCheck }; + for (StringRef CheckName : NolintParser.isAppliedToAllChecks() ? + AllChecks : NolintParser.getCheckNames()) { + // Don't report diagnostics to itself. + if (CheckName == NolintCheckName) + continue; + if (!CheckName.empty() && !Context->hasCheck(CheckName) && + !CheckName.startswith(ClangDiagnosticsPrefix)) { + Context->diag(NolintCheckName, NolintLoc, + "unknown check name '%0' is specified for %1 comment") + << CheckName << CommentName; + } else { + NolintComments.emplace(NolintTargetInfo{ LineNo, CheckName }, + NolintCommentInfo{NolintLoc, CommentType}); + } + } + if (!NolintParser.isClosingBracketFound()) { + Context->diag(NolintCheckName, NolintLoc, + "%0 comment has an opening parenthesis and no closing one, " + "so all the diagnostics will be silenced for the line") + << CommentName; + } + } + return false; + } + +private: + NolintMap NolintComments; + ClangTidyContext* Context; +}; +const StringRef NolintCommentsCollector::AnyCheck; + ClangTidyContext::ClangTidyContext( std::unique_ptr OptionsProvider) : DiagEngine(nullptr), OptionsProvider(std::move(OptionsProvider)), - Profile(nullptr) { + Profile(nullptr), + NolintCollector(llvm::make_unique(this)) { // Before the first translation unit we can get errors related to command-line // parsing, use empty string for the file name in this case. setCurrentFile(""); @@ -218,6 +377,12 @@ LangOpts = Context->getLangOpts(); } +void ClangTidyContext::setPreprocessor(Preprocessor *preprocessor) { + if (preprocessor && isCheckEnabled(NolintCheckName)) { + preprocessor->addCommentHandler(NolintCollector.get()); + } +} + const ClangTidyGlobalOptions &ClangTidyContext::getGlobalOptions() const { return OptionsProvider->getGlobalOptions(); } @@ -250,6 +415,23 @@ Errors.push_back(Error); } +NolintCommentsCollector& ClangTidyContext::getNolintCollector() { + return *NolintCollector; +} + +// This method is not const, because it fills cache on first demand. +// It is possible to fill cache in constructor or make cache volatile +// and make this method const, but they seem like worse solutions. +bool ClangTidyContext::hasCheck(StringRef CheckName) { + if (AllCheckNamesCache.empty()) { + ClangTidyASTConsumerFactory Factory(*this); + AllCheckNamesCache = Factory.getCheckNames(); + assert(std::is_sorted(AllCheckNamesCache.begin(), AllCheckNamesCache.end())); + } + return std::binary_search(AllCheckNamesCache.begin(), + AllCheckNamesCache.end(), CheckName); +} + StringRef ClangTidyContext::getCheckName(unsigned DiagnosticID) const { llvm::DenseMap::const_iterator I = CheckNamesByDiagnosticID.find(DiagnosticID); @@ -258,6 +440,14 @@ return ""; } +std::string ClangTidyContext::getDiagnosticsName(unsigned DiagnosticID) const { + StringRef WarningOption = + DiagEngine->getDiagnosticIDs()->getWarningOptionForDiag(DiagnosticID); + return !WarningOption.empty() + ? (ClangDiagnosticsPrefix + WarningOption).str() + : getCheckName(DiagnosticID).str(); +} + ClangTidyDiagnosticConsumer::ClangTidyDiagnosticConsumer( ClangTidyContext &Ctx, bool RemoveIncompatibleErrors) : Context(Ctx), RemoveIncompatibleErrors(RemoveIncompatibleErrors), @@ -291,38 +481,25 @@ LastErrorPassesLineFilter = false; } -static bool IsNOLINTFound(StringRef NolintDirectiveText, StringRef Line, - unsigned DiagID, const ClangTidyContext &Context) { - const size_t NolintIndex = Line.find(NolintDirectiveText); - if (NolintIndex == StringRef::npos) +bool ClangTidyDiagnosticConsumer::IsNolintFound(StringRef NolintDirectiveText, + StringRef Line, + StringRef CheckName) { + NolintCommentParser NolintParser(NolintDirectiveText, Line); + if (!NolintParser.isNolintFound()) { 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 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; + if (NolintParser.isAppliedToAllChecks()) { + // NOLINT without check name shouldn't block NolintCheck itself, because + // otherwise there will never be diagnostics on "// NOLINT". + return CheckName != NolintCheckName; + } + const SmallVector& CheckNames = NolintParser.getCheckNames(); + return llvm::find(CheckNames, CheckName) != CheckNames.end(); } -static bool LineIsMarkedWithNOLINT(SourceManager &SM, SourceLocation Loc, - unsigned DiagID, - const ClangTidyContext &Context) { +bool ClangTidyDiagnosticConsumer::LineIsMarkedWithNolint(SourceManager &SM, + SourceLocation Loc, + StringRef CheckName) { bool Invalid; const char *CharacterData = SM.getCharacterData(Loc, &Invalid); if (Invalid) @@ -333,7 +510,7 @@ while (*P != '\0' && *P != '\r' && *P != '\n') ++P; StringRef RestOfLine(CharacterData, P - CharacterData + 1); - if (IsNOLINTFound("NOLINT", RestOfLine, DiagID, Context)) + if (IsNolintFound(NolintComment, RestOfLine, CheckName)) return true; // Check if there's a NOLINTNEXTLINE on the previous line. @@ -360,17 +537,16 @@ --P; RestOfLine = StringRef(P, LineEnd - P + 1); - if (IsNOLINTFound("NOLINTNEXTLINE", RestOfLine, DiagID, Context)) + if (IsNolintFound(NolintNextLineComment, RestOfLine, CheckName)) return true; return false; } -static bool LineIsMarkedWithNOLINTinMacro(SourceManager &SM, SourceLocation Loc, - unsigned DiagID, - const ClangTidyContext &Context) { +bool ClangTidyDiagnosticConsumer::IsLineMarkedWithNolintInMacro( + SourceManager &SM, SourceLocation Loc, StringRef CheckName) { while (true) { - if (LineIsMarkedWithNOLINT(SM, Loc, DiagID, Context)) + if (LineIsMarkedWithNolint(SM, Loc, CheckName)) return true; if (!Loc.isMacroID()) return false; @@ -384,33 +560,31 @@ if (LastErrorWasIgnored && DiagLevel == DiagnosticsEngine::Note) return; - if (Info.getLocation().isValid() && DiagLevel != DiagnosticsEngine::Error && - DiagLevel != DiagnosticsEngine::Fatal && - LineIsMarkedWithNOLINTinMacro(Diags->getSourceManager(), - Info.getLocation(), Info.getID(), - Context)) { - ++Context.Stats.ErrorsIgnoredNOLINT; - // Ignored a warning, should ignore related notes as well - LastErrorWasIgnored = true; - return; + std::string CheckName = Context.getDiagnosticsName(Info.getID()); + SourceLocation DiagLocation = Info.getLocation(); + if (DiagLocation.isValid()) { + // This is necessary for finding redundant NOLINT comments. Fill collection + // here, as there won't be another change if diagnostics is filtered. + DiagIDsToLineNumbers[Info.getID()].push_back( + Info.getSourceManager().getExpansionLineNumber(DiagLocation)); + + if (DiagLevel != DiagnosticsEngine::Error && + DiagLevel != DiagnosticsEngine::Fatal && + IsLineMarkedWithNolintInMacro(Diags->getSourceManager(), DiagLocation, + CheckName)) { + ++Context.Stats.ErrorsIgnoredNOLINT; + // Ignored a warning, should ignore related notes as well. + LastErrorWasIgnored = true; + return; + } } LastErrorWasIgnored = false; // Count warnings/errors. DiagnosticConsumer::HandleDiagnostic(DiagLevel, Info); - if (DiagLevel == DiagnosticsEngine::Note) { - assert(!Errors.empty() && - "A diagnostic note can only be appended to a message."); - } else { + if (DiagLevel != DiagnosticsEngine::Note) { finalizeLastError(); - StringRef WarningOption = - Context.DiagEngine->getDiagnosticIDs()->getWarningOptionForDiag( - Info.getID()); - std::string CheckName = !WarningOption.empty() - ? ("clang-diagnostic-" + WarningOption).str() - : Context.getCheckName(Info.getID()).str(); - if (CheckName.empty()) { // This is a compiler diagnostic without a warning option. Assign check // name based on its level. @@ -441,6 +615,9 @@ Context.treatAsError(CheckName); Errors.emplace_back(CheckName, Level, Context.getCurrentBuildDirectory(), IsWarningAsError); + } else { + assert(!Errors.empty() && + "A diagnostic note can only be appended to a message."); } ClangTidyDiagnosticRenderer Converter( @@ -641,6 +818,51 @@ } } +void ClangTidyDiagnosticConsumer::reportRedundantNolintComments() { + NolintCommentsCollector::NolintMap NolintMap = + Context.getNolintCollector().takeNolintMap(); + if (NolintMap.empty()) + return; + for (const auto& DiagIDToLineNumbers : DiagIDsToLineNumbers) { + std::string CheckName = Context.getDiagnosticsName(DiagIDToLineNumbers.first); + if (CheckName.empty()) + continue; + for (unsigned LineNumber : DiagIDToLineNumbers.second) { + NolintMap.erase({LineNumber, CheckName}); + NolintMap.erase({LineNumber, NolintCommentsCollector::AnyCheck}); + } + } + for (const auto& NolintEntry : NolintMap) { + StringRef Message; + if (NolintEntry.first.CheckName == NolintCommentsCollector::AnyCheck) { + switch (NolintEntry.second.Type) { + case NolintCommentType::Nolint: + Message = "there is no diagnostics on this line, " + "the NOLINT comment is redundant"; + break; + case NolintCommentType::NolintNextLine: + Message = "there is no diagnostics on the following line, " + "the NOLINTNEXTLINE comment is redundant"; + break; + } + Context.diag(NolintCheckName, NolintEntry.second.Location, Message); + } else { + switch (NolintEntry.second.Type) { + case NolintCommentType::Nolint: + Message = "there is no diagnostics for the '%0' check on this line, " + "the NOLINT comment is redundant"; + break; + case NolintCommentType::NolintNextLine: + Message = "there is no diagnostics for the '%0' check on the " + "following line, the NOLINTNEXTLINE comment is redundant"; + break; + } + Context.diag(NolintCheckName, NolintEntry.second.Location, Message) + << NolintEntry.first.CheckName; + } + } +} + namespace { struct LessClangTidyError { bool operator()(const ClangTidyError &LHS, const ClangTidyError &RHS) const { @@ -661,6 +883,7 @@ // Flushes the internal diagnostics buffer to the ClangTidyContext. void ClangTidyDiagnosticConsumer::finish() { + reportRedundantNolintComments(); finalizeLastError(); std::sort(Errors.begin(), Errors.end(), LessClangTidyError()); @@ -673,4 +896,5 @@ for (const ClangTidyError &Error : Errors) Context.storeError(Error); Errors.clear(); + DiagIDsToLineNumbers.clear(); } Index: test/clang-tidy/nolint-usage.cpp =================================================================== --- /dev/null +++ test/clang-tidy/nolint-usage.cpp @@ -0,0 +1,171 @@ +// RUN: %check_clang_tidy %s nolint-usage,google-explicit-constructor,misc-unused-parameters %t -- + +// Case: NO_LINT on line with an error. +class A1 { A1(int i); }; // NOLINT + +// Case: NO_LINT on line without errors. +class A2 { explicit A2(int i); }; // NOLINT +// CHECK-MESSAGES: :[[@LINE-1]]:38: warning: there is no diagnostics on this line, the NOLINT comment is redundant + +// Case: NO_LINT for the specific check on line with an error on it. +class A3 { A3(int i); }; // NOLINT(google-explicit-constructor) + +// Case: NO_LINT for the specific check on line with an error on another check. +class A4 { A4(int i); }; // NOLINT(misc-unused-parameters) +// CHECK-MESSAGES: :[[@LINE-1]]:12: warning: single-argument constructors must be marked explicit +// CHECK-MESSAGES: :[[@LINE-2]]:29: warning: there is no diagnostics for the 'misc-unused-parameters' check on this line, the NOLINT comment is redundant + +// Case: NO_LINT for the specific check on line without errors. +class A5 { explicit A5(int i); }; // NOLINT(google-explicit-constructor) +// CHECK-MESSAGES: :[[@LINE-1]]:38: warning: there is no diagnostics for the 'google-explicit-constructor' check on this line, the NOLINT comment is redundant + +// Case: NO_LINT for unknown check on line with an error on some check. +class A6 { A6(int i); }; // NOLINT(unknown-check) +// CHECK-MESSAGES: :[[@LINE-1]]:12: warning: single-argument constructors must be marked explicit +// CHECK-MESSAGES: :[[@LINE-2]]:29: warning: unknown check name 'unknown-check' is specified for NOLINT comment + +// Case: NO_LINT for unknown check on line without errors. +class A7 { explicit A7(int i); }; // NOLINT(unknown-check) +// CHECK-MESSAGES: :[[@LINE-1]]:38: warning: unknown check name 'unknown-check' is specified for NOLINT comment + +// Case: NO_LINT with not closed parenthesis without check names. +class A8 { A8(int i); }; // NOLINT( +// CHECK-MESSAGES: :[[@LINE-1]]:29: warning: NOLINT comment has an opening parenthesis and no closing one, so all the diagnostics will be silenced for the line + +// Case: NO_LINT with not closed parenthesis with check names. +class A9 { A9(int i); }; // NOLINT(unknown-check +// CHECK-MESSAGES: :[[@LINE-1]]:29: warning: NOLINT comment has an opening parenthesis and no closing one, so all the diagnostics will be silenced for the line + +// Case: NO_LINT with clang diagnostics +int f() { + int i = 0; // NOLINT +// CHECK-MESSAGES: :[[@LINE-1]]:17: warning: there is no diagnostics on this line, the NOLINT comment is redundant + int j = 0; // NOLINT(clang-diagnostic-unused-variable) +// CHECK-MESSAGES: :[[@LINE-1]]:17: warning: there is no diagnostics for the 'clang-diagnostic-unused-variable' check on this line, the NOLINT comment is redundant + return i + j; +} + +#define MACRO(X) class X { explicit X(int i); }; + +// Case: NO_LINT on macro instantiation line +MACRO(M1) // NOLINT +// CHECK-MESSAGES: :[[@LINE-1]]:14: warning: there is no diagnostics on this line, the NOLINT comment is redundant + +// Case: NO_LINT with the specific check on macro instantiation line +MACRO(M2) // NOLINT(google-explicit-constructor) +// CHECK-MESSAGES: :[[@LINE-1]]:14: warning: there is no diagnostics for the 'google-explicit-constructor' check on this line, the NOLINT comment is redundant + +// Case: NO_LINT on macro line +#define MACRO2(X) class X { explicit X(int i); }; // NOLINT +MACRO2(M3) +// CHECK-MESSAGES: :[[@LINE-2]]:54: warning: there is no diagnostics on this line, the NOLINT comment is redundant + +// Case: NO_LINT with the specific check on macro line +#define MACRO3(X) class X { explicit X(int i); }; // NOLINT(google-explicit-constructor) +MACRO(M4) +// CHECK-MESSAGES: :[[@LINE-2]]:54: warning: there is no diagnostics for the 'google-explicit-constructor' check on this line, the NOLINT comment is redundant + +// Case: NO_LINT with nolint-usage on line without errors. +class B1 { explicit B1(int i); }; // NOLINT(nolint-usage) + +// Case: NO_LINT with the specific check and nolint-usage on line without errors. +class B2 { explicit B2(int i); }; // NOLINT(google-explicit-constructor, nolint-usage) + +// Case: NO_LINT with the unknown check and nolint-usage on line without errors. +class B3 { explicit B3(int i); }; // NOLINT(unknown-check, nolint-usage) + + +// Case: NO_LINT_NEXT_LINE before line with an error. +// NOLINTNEXTLINE +class C1 { C1(int i); }; + +// Case: NO_LINT_NEXT_LINE before line without errors. +// NOLINTNEXTLINE +class C2 { explicit C2(int i); }; +// CHECK-MESSAGES: :[[@LINE-2]]:4: warning: there is no diagnostics on the following line, the NOLINTNEXTLINE comment is redundant + +// Case: NO_LINT_NEXT_LINE for the specific check before line with an error on it. +// NOLINTNEXTLINE(google-explicit-constructor) +class C3 { C3(int i); }; + +// Case: NO_LINT_NEXT_LINE for the specific check before line with an error on another check. +// NOLINTNEXTLINE(misc-unused-parameters) +class C4 { C4(int i); }; +// CHECK-MESSAGES: :[[@LINE-2]]:4: warning: there is no diagnostics for the 'misc-unused-parameters' check on the following line, the NOLINTNEXTLINE comment is redundant +// CHECK-MESSAGES: :[[@LINE-2]]:12: warning: single-argument constructors must be marked explicit + +// Case: NO_LINT_NEXT_LINE for the specific check before line without errors. +// NOLINTNEXTLINE(google-explicit-constructor) +class C5 { explicit C5(int i); }; +// CHECK-MESSAGES: :[[@LINE-2]]:4: warning: there is no diagnostics for the 'google-explicit-constructor' check on the following line, the NOLINTNEXTLINE comment is redundant + +// Case: NO_LINT_NEXT_LINE for unknown check before line with an error on some check. +// NOLINTNEXTLINE(unknown-check) +class C6 { C6(int i); }; +// CHECK-MESSAGES: :[[@LINE-2]]:4: warning: unknown check name 'unknown-check' is specified for NOLINTNEXTLINE comment +// CHECK-MESSAGES: :[[@LINE-2]]:12: warning: single-argument constructors must be marked explicit + +// Case: NO_LINT_NEXT_LINE for unknown check before line without errors. +// NOLINTNEXTLINE(unknown-check) +class C7 { explicit C7(int i); }; +// CHECK-MESSAGES: :[[@LINE-2]]:4: warning: unknown check name 'unknown-check' is specified for NOLINTNEXTLINE comment + +// Case: NO_LINT_NEXT_LINE with not closed parenthesis without check names. +// NOLINTNEXTLINE( +class C8 { C8(int i); }; +// CHECK-MESSAGES: :[[@LINE-2]]:4: warning: NOLINTNEXTLINE comment has an opening parenthesis and no closing one, so all the diagnostics will be silenced for the line + +// Case: NO_LINT_NEXT_LINE with not closed parenthesis with check names. +// NOLINTNEXTLINE(unknown-check +class C9 { C9(int i); }; +// CHECK-MESSAGES: :[[@LINE-2]]:4: warning: NOLINTNEXTLINE comment has an opening parenthesis and no closing one, so all the diagnostics will be silenced for the line + +// Case: NO_LINT_NEXT_LINE with clang diagnostics +int g() { + // NOLINTNEXTLINE + int i = 0; +// CHECK-MESSAGES: :[[@LINE-2]]:6: warning: there is no diagnostics on the following line, the NOLINTNEXTLINE comment is redundant + // NOLINTNEXTLINE(clang-diagnostic-unused-variable) + int j = 0; +// CHECK-MESSAGES: :[[@LINE-2]]:6: warning: there is no diagnostics for the 'clang-diagnostic-unused-variable' check on the following line, the NOLINTNEXTLINE comment is redundant + return i + j; +} + +#define MACRO(X) class X { explicit X(int i); }; + +// Case: NO_LINT_NEXT_LINE before macro instantiation line +// NOLINTNEXTLINE +MACRO(MM1) +// CHECK-MESSAGES: :[[@LINE-2]]:4: warning: there is no diagnostics on the following line, the NOLINTNEXTLINE comment is redundant + +// Case: NO_LINT_NEXT_LINE with the specific check before macro instantiation line +// NOLINTNEXTLINE(google-explicit-constructor) +MACRO(MM2) +// CHECK-MESSAGES: :[[@LINE-2]]:4: warning: there is no diagnostics for the 'google-explicit-constructor' check on the following line, the NOLINTNEXTLINE comment is redundant + +// Case: NO_LINT_NEXT_LINE before macro line +// NOLINTNEXTLINE +#define MACRO2(X) class X { explicit X(int i); }; +MACRO2(MM3) +// CHECK-MESSAGES: :[[@LINE-3]]:4: warning: there is no diagnostics on the following line, the NOLINTNEXTLINE comment is redundant + +// Case: NO_LINT_NEXT_LINE with the specific check before macro line +// NOLINTNEXTLINE(google-explicit-constructor) +#define MACRO3(X) class X { explicit X(int i); }; +MACRO(MM4) +// CHECK-MESSAGES: :[[@LINE-3]]:4: warning: there is no diagnostics for the 'google-explicit-constructor' check on the following line, the NOLINTNEXTLINE comment is redundant + +// Case: NO_LINT_NEXT_LINE with nolint-usage before line without errors. +// NOLINTNEXTLINE(nolint-usage) +// NOLINTNEXTLINE +class D1 { explicit D1(int i); }; + +// Case: NO_LINT_NEXT_LINE with the specific check and nolint-usage before line without errors. +// NOLINTNEXTLINE(nolint-usage) +// NOLINTNEXTLINE(google-explicit-constructor) +class D2 { explicit D2(int i); }; + +// Case: NO_LINT_NEXT_LINE with the unknown check and nolint-usage before line without errors. +// NOLINTNEXTLINE(nolint-usage) +// NOLINTNEXTLINE(unknown-check) +class D3 { explicit D3(int i); }; Index: test/clang-tidy/nolint.cpp =================================================================== --- test/clang-tidy/nolint.cpp +++ test/clang-tidy/nolint.cpp @@ -30,6 +30,9 @@ int i; // CHECK-MESSAGES: :[[@LINE-1]]:7: warning: unused variable 'i' [clang-diagnostic-unused-variable] int j; // NOLINT + int k; // NOLINT(clang-diagnostic-unused-variable) + int l; // NOLINT(clang-diagnostic-literal-conversion) +// CHECK-MESSAGES: :[[@LINE-1]]:7: warning: unused variable 'l' [clang-diagnostic-unused-variable] } #define MACRO(X) class X { X(int i); }; @@ -46,4 +49,4 @@ #define DOUBLE_MACRO MACRO(H) // NOLINT DOUBLE_MACRO -// CHECK-MESSAGES: Suppressed 12 warnings (12 NOLINT) +// CHECK-MESSAGES: Suppressed 13 warnings (13 NOLINT) Index: test/clang-tidy/nolintnextline.cpp =================================================================== --- test/clang-tidy/nolintnextline.cpp +++ test/clang-tidy/nolintnextline.cpp @@ -24,6 +24,18 @@ class C5 { C5(int i); }; +void f() { + int i; +// CHECK-MESSAGES: :[[@LINE-1]]:7: warning: unused variable 'i' [clang-diagnostic-unused-variable] + // NOLINTNEXTLINE + int j; + // NOLINTNEXTLINE(clang-diagnostic-unused-variable) + int k; + // NOLINTNEXTLINE(clang-diagnostic-literal-conversion) + int l; +// CHECK-MESSAGES: :[[@LINE-1]]:7: warning: unused variable 'l' [clang-diagnostic-unused-variable] +} + // NOLINTNEXTLINE class D { D(int i); }; @@ -44,6 +56,6 @@ // NOLINTNEXTLINE MACRO_NOARG -// CHECK-MESSAGES: Suppressed 8 warnings (8 NOLINT) +// CHECK-MESSAGES: Suppressed 10 warnings (10 NOLINT) -// RUN: %check_clang_tidy %s google-explicit-constructor %t -- +// RUN: %check_clang_tidy %s google-explicit-constructor,clang-diagnostic-unused-variable %t -- -extra-arg=-Wunused-variable --