diff --git a/clang-tools-extra/clang-tidy/misc/CMakeLists.txt b/clang-tools-extra/clang-tidy/misc/CMakeLists.txt --- a/clang-tools-extra/clang-tidy/misc/CMakeLists.txt +++ b/clang-tools-extra/clang-tidy/misc/CMakeLists.txt @@ -4,6 +4,7 @@ DefinitionsInHeadersCheck.cpp MiscTidyModule.cpp MisplacedConstCheck.cpp + MissingHeaderFileDeclarationCheck.cpp NewDeleteOverloadsCheck.cpp NonCopyableObjects.cpp NonPrivateMemberVariablesInClassesCheck.cpp diff --git a/clang-tools-extra/clang-tidy/misc/MiscTidyModule.cpp b/clang-tools-extra/clang-tidy/misc/MiscTidyModule.cpp --- a/clang-tools-extra/clang-tidy/misc/MiscTidyModule.cpp +++ b/clang-tools-extra/clang-tidy/misc/MiscTidyModule.cpp @@ -11,6 +11,7 @@ #include "../ClangTidyModuleRegistry.h" #include "DefinitionsInHeadersCheck.h" #include "MisplacedConstCheck.h" +#include "MissingHeaderFileDeclarationCheck.h" #include "NewDeleteOverloadsCheck.h" #include "NonCopyableObjects.h" #include "NonPrivateMemberVariablesInClassesCheck.h" @@ -33,6 +34,8 @@ CheckFactories.registerCheck( "misc-definitions-in-headers"); CheckFactories.registerCheck("misc-misplaced-const"); + CheckFactories.registerCheck( + "misc-missing-header-file-declaration"); CheckFactories.registerCheck( "misc-new-delete-overloads"); CheckFactories.registerCheck( diff --git a/clang-tools-extra/clang-tidy/misc/MissingHeaderFileDeclarationCheck.h b/clang-tools-extra/clang-tidy/misc/MissingHeaderFileDeclarationCheck.h new file mode 100644 --- /dev/null +++ b/clang-tools-extra/clang-tidy/misc/MissingHeaderFileDeclarationCheck.h @@ -0,0 +1,37 @@ +//===--- MissingHeaderFileDeclarationCheck.h - clang-tidy -------*- C++ -*-===// +// +// Part of the LLVM Project, under the Apache License v2.0 with LLVM Exceptions. +// See https://llvm.org/LICENSE.txt for license information. +// SPDX-License-Identifier: Apache-2.0 WITH LLVM-exception +// +//===----------------------------------------------------------------------===// + +#ifndef LLVM_CLANG_TOOLS_EXTRA_CLANG_TIDY_MISC_MISSINGHEADERFILEDECLARATIONCHECK_H +#define LLVM_CLANG_TOOLS_EXTRA_CLANG_TIDY_MISC_MISSINGHEADERFILEDECLARATIONCHECK_H + +#include "../ClangTidyCheck.h" + +namespace clang { +namespace tidy { +namespace misc { + +/// Flags definitions with external linkage in source files that don't have a +/// declaration in the corresponding header file. +/// +/// For the user-facing documentation see: +/// http://clang.llvm.org/extra/clang-tidy/checks/misc-missing-header-file-declaration.html +class MissingHeaderFileDeclarationCheck : public ClangTidyCheck { +public: + MissingHeaderFileDeclarationCheck(StringRef Name, ClangTidyContext *Context); + void storeOptions(ClangTidyOptions::OptionMap &Opts) override; + void registerMatchers(ast_matchers::MatchFinder *Finder) override; + void check(const ast_matchers::MatchFinder::MatchResult &Result) override; +private: + const bool CheckAnyHeader; +}; + +} // namespace misc +} // namespace tidy +} // namespace clang + +#endif // LLVM_CLANG_TOOLS_EXTRA_CLANG_TIDY_MISC_MISSINGHEADERFILEDECLARATIONCHECK_H diff --git a/clang-tools-extra/clang-tidy/misc/MissingHeaderFileDeclarationCheck.cpp b/clang-tools-extra/clang-tidy/misc/MissingHeaderFileDeclarationCheck.cpp new file mode 100644 --- /dev/null +++ b/clang-tools-extra/clang-tidy/misc/MissingHeaderFileDeclarationCheck.cpp @@ -0,0 +1,120 @@ +//===--- MissingHeaderFileDeclarationCheck.cpp - clang-tidy ---------------===// +// +// Part of the LLVM Project, under the Apache License v2.0 with LLVM Exceptions. +// See https://llvm.org/LICENSE.txt for license information. +// SPDX-License-Identifier: Apache-2.0 WITH LLVM-exception +// +//===----------------------------------------------------------------------===// + +#include "MissingHeaderFileDeclarationCheck.h" +#include "clang/ASTMatchers/ASTMatchFinder.h" +#include "clang/ASTMatchers/ASTMatchers.h" +#include "clang/Basic/SourceLocation.h" +#include "llvm/ADT/Optional.h" +#include "llvm/ADT/StringSwitch.h" +#include + +using namespace clang::ast_matchers; + +namespace clang { +namespace tidy { +namespace misc { + +struct DecomposedFileName { + StringRef NamePart; + StringRef Extension; + + bool isHeader() const { + return llvm::StringSwitch(Extension) + .CasesLower("h", "hpp", "hxx", true) + .Default(false); + } +}; + +llvm::Optional decomposeFileName(StringRef FileName) { + // Undecided about using llvm::sys::path + StringRef::size_type Pos = FileName.find_last_of("\\/"); + if (Pos != StringRef::npos) { + FileName = FileName.drop_front(Pos + 1); + } + Pos = FileName.find_last_of('.'); + if (Pos == StringRef::npos) + return None; + return DecomposedFileName{FileName.take_front(Pos), + FileName.drop_front(Pos + 1)}; +} + +template +static void checkDefinition(const Redecl &Decl, const SourceManager &SM, + bool AnyHeader, StringRef PrettTypeName, + DiagEmitter Diags) { + static_assert(std::is_base_of, Redecl>::value, + "Not a Redeclarable Decl"); + static_assert(std::is_base_of::value, "Not a Named Decl"); + llvm::Optional File = + decomposeFileName(SM.getFilename(Decl.getBeginLoc())); + if (!File || File->isHeader()) + // File being detected as a header would occur if clang-tidy was being + // invoked on a header file. + // If the file is a header and this gets tripped, probably not a good thing + // either, definitions in headers is a receipe for disaster, but that't not + // what this specific check is looking for. + return; + for (const auto *RD : Decl.redecls()) { + llvm::Optional ThisFile = + decomposeFileName(SM.getFilename(RD->getBeginLoc())); + if (!ThisFile || !ThisFile->isHeader()) + continue; + if (AnyHeader || File->NamePart.equals_lower(ThisFile->NamePart)) + return; // Found a good candidate for matching decl + } + Diags(Decl.getLocation(), + "%0 %1 is defined with external linkage but has no corresponding " + "declaration in %select{the corresponding|any}2 header file") + << PrettTypeName << cast(&Decl) << static_cast(AnyHeader); +} + +MissingHeaderFileDeclarationCheck::MissingHeaderFileDeclarationCheck( + StringRef Name, ClangTidyContext *Context) + : ClangTidyCheck(Name, Context), + CheckAnyHeader(Options.get("CheckAnyHeader", false)) {} + +void MissingHeaderFileDeclarationCheck::storeOptions( + ClangTidyOptions::OptionMap &Opts) { + Options.store(Opts, "CheckAnyHeader", CheckAnyHeader); +} + +void MissingHeaderFileDeclarationCheck::registerMatchers(MatchFinder *Finder) { + if (getLangOpts().ObjC) + return; + Finder->addMatcher(varDecl(hasExternalFormalLinkage(), isDefinition(), + isExpansionInMainFile(), unless(isConstexpr())) + .bind("VarDecl"), + this); + Finder->addMatcher( + functionDecl( + hasExternalFormalLinkage(), isDefinition(), isExpansionInMainFile(), + unless(anyOf(isConstexpr(), isInline(), cxxMethodDecl(), isMain()))) + .bind("FunctionDecl"), + this); +} + +void MissingHeaderFileDeclarationCheck::check( + const MatchFinder::MatchResult &Result) { + auto DiagEmitter = [&](SourceLocation Loc, StringRef Msg) { + return diag(Loc, Msg); + }; + + if (const auto *VD = Result.Nodes.getNodeAs("VarDecl")) { + checkDefinition(*VD, *Result.SourceManager, CheckAnyHeader, "Variable", + DiagEmitter); + } + if (const auto *FD = Result.Nodes.getNodeAs("FunctionDecl")) { + checkDefinition(*FD, *Result.SourceManager, CheckAnyHeader, "Function", + DiagEmitter); + } +} + +} // namespace misc +} // namespace tidy +} // namespace clang diff --git a/clang-tools-extra/docs/ReleaseNotes.rst b/clang-tools-extra/docs/ReleaseNotes.rst --- a/clang-tools-extra/docs/ReleaseNotes.rst +++ b/clang-tools-extra/docs/ReleaseNotes.rst @@ -88,6 +88,12 @@ Flags use of the `C` standard library functions ``memset``, ``memcpy`` and ``memcmp`` and similar derivatives on non-trivial types. +- New :doc:`misc-missing-header-file-declaration + ` check. + + Flags definitions with external linkage in source files that don't have a + declaration in the corresponding header file + New aliases ^^^^^^^^^^^ diff --git a/clang-tools-extra/docs/clang-tidy/checks/list.rst b/clang-tools-extra/docs/clang-tidy/checks/list.rst --- a/clang-tools-extra/docs/clang-tidy/checks/list.rst +++ b/clang-tools-extra/docs/clang-tidy/checks/list.rst @@ -188,6 +188,7 @@ `llvm-twine-local `_, "Yes" `misc-definitions-in-headers `_, "Yes" `misc-misplaced-const `_, + `misc-missing-header-file-declaration `_, "Yes" `misc-new-delete-overloads `_, `misc-non-copyable-objects `_, `misc-non-private-member-variables-in-classes `_, diff --git a/clang-tools-extra/docs/clang-tidy/checks/misc-missing-header-file-declaration.rst b/clang-tools-extra/docs/clang-tidy/checks/misc-missing-header-file-declaration.rst new file mode 100644 --- /dev/null +++ b/clang-tools-extra/docs/clang-tidy/checks/misc-missing-header-file-declaration.rst @@ -0,0 +1,15 @@ +.. title:: clang-tidy - misc-missing-header-file-declaration + +misc-missing-header-file-declaration +==================================== + +Flags variable and function definitions with external linkage in source files +that don't have a declaration in the corresponding header file. + +Options +------- + +.. option:: CheckAnyHeader + + If set to `1` the check will look in any header file for a matching + declaration. diff --git a/clang-tools-extra/test/clang-tidy/checkers/Inputs/misc-missing-header-file-declaration/misc-missing-header-file-declaration.cpp.tmp.h b/clang-tools-extra/test/clang-tidy/checkers/Inputs/misc-missing-header-file-declaration/misc-missing-header-file-declaration.cpp.tmp.h new file mode 100644 --- /dev/null +++ b/clang-tools-extra/test/clang-tidy/checkers/Inputs/misc-missing-header-file-declaration/misc-missing-header-file-declaration.cpp.tmp.h @@ -0,0 +1,24 @@ +#ifndef MISC_MISSING_HEADER_FILE_DECLARATON_H +#define MISC_MISSING_HEADER_FILE_DECLARATON_H + +extern bool DeclInHeader; +extern bool DeclInBoth; + +extern void declInHeader(); +extern void declInBoth(); + +struct Foo { + static int Bar; +}; + +namespace ns1 { +extern bool NS; +extern void nS(); +} // namespace ns1 + +namespace ns2 { +extern bool NS; +extern void nS(); +} // namespace ns2 + +#endif \ No newline at end of file diff --git a/clang-tools-extra/test/clang-tidy/checkers/Inputs/misc-missing-header-file-declaration/wrong_header.h b/clang-tools-extra/test/clang-tidy/checkers/Inputs/misc-missing-header-file-declaration/wrong_header.h new file mode 100644 --- /dev/null +++ b/clang-tools-extra/test/clang-tidy/checkers/Inputs/misc-missing-header-file-declaration/wrong_header.h @@ -0,0 +1,7 @@ +#ifndef WRONG_HEADER +#define WRONG_HEADER + +extern bool DeclInWrongHeader; +extern void declInWrongHeader(); + +#endif \ No newline at end of file diff --git a/clang-tools-extra/test/clang-tidy/checkers/misc-missing-header-file-declaration-any-header.cpp b/clang-tools-extra/test/clang-tidy/checkers/misc-missing-header-file-declaration-any-header.cpp new file mode 100644 --- /dev/null +++ b/clang-tools-extra/test/clang-tidy/checkers/misc-missing-header-file-declaration-any-header.cpp @@ -0,0 +1,79 @@ +// RUN: %check_clang_tidy %s misc-missing-header-file-declaration %t -- \ +// RUN: -config='{CheckOptions: \ +// RUN: [{key: misc-missing-header-file-declaration.CheckAnyHeader, value: 1}]}' \ +// RUN: -- -I%S/Inputs/misc-missing-header-file-declaration + +// NOTE in this file everything is actually in the wrong header file, as the +// corresponding header is called +// 'misc-missing-header-file-declaration-any-header.cpp.tmp.h' + +#include "misc-missing-header-file-declaration.cpp.tmp.h" +// Include file has to be named *.cpp.tmp.h to account for the script renaming +// this file to *.cpp.tmp.cpp. +// Outside this test environment it will work normally +// -> +#include "wrong_header.h" + +// These declarations should be ignored by the check as they are in the same +// file. +extern bool DeclInSource; +extern void declInSource(); + +// These declarations should be ignored by the check as they are in the same +// file, however there is a corresponding decl in the header that will prevent +// a failing check. +extern bool DeclInBoth; +extern void declInBoth(); + +// No external linkage so no warning +static bool StaticOK = false; +constexpr bool ConstexprOK = false; +inline void inlineOK() {} +static void staticOK() {} +constexpr bool constexprOK() { return true; } + +// External linkage but decl in header so no warning +bool DeclInHeader = false; +bool DeclInBoth = false; +void declInHeader() {} +void declInBoth() {} + + +//Decls don't appear in corresponding header so issue a warning +bool DeclInSource = false; +// CHECK-MESSAGES: :[[@LINE-1]]:6: warning: Variable 'DeclInSource' is defined with external linkage +bool NoDecl = false; +// CHECK-MESSAGES: :[[@LINE-1]]:6: warning: Variable 'NoDecl' is defined with external linkage +void declInSource() {} +// CHECK-MESSAGES: :[[@LINE-1]]:6: warning: Function 'declInSource' is defined with external linkage +void noDecl() {} +// CHECK-MESSAGES: :[[@LINE-1]]:6: warning: Function 'noDecl' is defined with external linkage + +// Checking any header, this is declared so no warning +bool DeclInWrongHeader = false; +void declInWrongHeader(){} + + +// Decls in an anonymous namespace don't have external linkage, so no warning +// should be emitted +namespace { +bool AnonNS = false; +void anonNS() {} +} // namespace + +// Ensure in namespace definitions are correctly resolved +namespace ns1 { +bool NS = false; +void nS() {} +} // namespace ns1 + +// Ensure out of namespace definitions are correctly resolved +bool /*namespace*/ns2::NS = false; +void /*namespace*/ns2::nS() {} + +// Static class members declared in the header shouldn't be warned on. +int /*struct*/Foo::Bar = 0; + +// main is special, don't warn for it. +int main() { +} \ No newline at end of file diff --git a/clang-tools-extra/test/clang-tidy/checkers/misc-missing-header-file-declaration.cpp b/clang-tools-extra/test/clang-tidy/checkers/misc-missing-header-file-declaration.cpp new file mode 100644 --- /dev/null +++ b/clang-tools-extra/test/clang-tidy/checkers/misc-missing-header-file-declaration.cpp @@ -0,0 +1,74 @@ +// RUN: %check_clang_tidy %s misc-missing-header-file-declaration %t -- \ +// RUN: -- -I%S/Inputs/misc-missing-header-file-declaration + +#include "misc-missing-header-file-declaration.cpp.tmp.h" +// Include file has to be named *.cpp.tmp.h to account for the script renaming +// this file to *.cpp.tmp.cpp. +// Outside this test environment it will work normally +// -> +#include "wrong_header.h" + +// These declarations should be ignored by the check as they are in the same +// file. +extern bool DeclInSource; +extern void declInSource(); + +// These declarations should be ignored by the check as they are in the same +// file, however there is a corresponding decl in the header that will prevent +// a failing check. +extern bool DeclInBoth; +extern void declInBoth(); + +// No external linkage so no warning +static bool StaticOK = false; +constexpr bool ConstexprOK = false; +inline void inlineOK() {} +static void staticOK() {} +constexpr bool constexprOK() { return true; } + +// External linkage but decl in header so no warning +bool DeclInHeader = false; +bool DeclInBoth = false; +void declInHeader() {} +void declInBoth() {} + + +//Decls don't appear in corresponding header so issue a warning +bool DeclInSource = false; +// CHECK-MESSAGES: :[[@LINE-1]]:6: warning: Variable 'DeclInSource' is defined with external linkage +bool NoDecl = false; +// CHECK-MESSAGES: :[[@LINE-1]]:6: warning: Variable 'NoDecl' is defined with external linkage +void declInSource() {} +// CHECK-MESSAGES: :[[@LINE-1]]:6: warning: Function 'declInSource' is defined with external linkage +void noDecl() {} +// CHECK-MESSAGES: :[[@LINE-1]]:6: warning: Function 'noDecl' is defined with external linkage + +bool DeclInWrongHeader = false; +// CHECK-MESSAGES: :[[@LINE-1]]:6: warning: Variable 'DeclInWrongHeader' is defined with external linkage +void declInWrongHeader(){} +// CHECK-MESSAGES: :[[@LINE-1]]:6: warning: Function 'declInWrongHeader' is defined with external linkage + + +// Decls in an anonymous namespace don't have external linkage, so no warning +// should be emitted +namespace { +bool AnonNS = false; +void anonNS() {} +} // namespace + +// Ensure in namespace definitions are correctly resolved +namespace ns1 { +bool NS = false; +void nS() {} +} // namespace ns1 + +// Ensure out of namespace definitions are correctly resolved +bool /*namespace*/ns2::NS = false; +void /*namespace*/ns2::nS() {} + +// Static class members declared in the header shouldn't be warned on. +int /*struct*/Foo::Bar = 0; + +// main is special, don't warn for it. +int main() { +} \ No newline at end of file