Skip to content

Commit 5b37cdd

Browse files
committed
[clang-tidy] Introduce HeaderFileExtensions and ImplementationFileExtensions options
Re-introduce the patch that was reverted previously. In the first attempt, the checks would not be able to read from the global option, since getLocalOrGlobal only works with string types. Additional logic is needed in order to support both use cases in the transition period. All that logic will be removed when the local options are fully removed. We have a number of checks designed to analyze problems in header files only, for example: bugprone-suspicious-include google-build-namespaces llvm-header-guard misc-definitions-in-header ... All these checks duplicate the same logic and options to determine whether a location is placed in the main source file or in the header. More checks are coming up with similar requirements. Thus, to remove duplication, let's move this option to the top-level configuration of clang-tidy (since it's something all checks should share). Add a deprecation notice for all checks that use the local option, prompting to update to the global option. Differential Revision: https://reviews.llvm.org/D142655
1 parent 1235ed9 commit 5b37cdd

36 files changed

+365
-147
lines changed

clang-tools-extra/clang-tidy/ClangTidyDiagnosticConsumer.cpp

Lines changed: 19 additions & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -22,6 +22,7 @@
2222
#include "clang/AST/ASTContext.h"
2323
#include "clang/AST/ASTDiagnostic.h"
2424
#include "clang/AST/Attr.h"
25+
#include "clang/Basic/CharInfo.h"
2526
#include "clang/Basic/Diagnostic.h"
2627
#include "clang/Basic/DiagnosticOptions.h"
2728
#include "clang/Basic/FileManager.h"
@@ -222,12 +223,30 @@ void ClangTidyContext::setSourceManager(SourceManager *SourceMgr) {
222223
DiagEngine->setSourceManager(SourceMgr);
223224
}
224225

226+
static bool parseFileExtensions(llvm::ArrayRef<std::string> AllFileExtensions,
227+
FileExtensionsSet &FileExtensions) {
228+
FileExtensions.clear();
229+
for (StringRef Suffix : AllFileExtensions) {
230+
StringRef Extension = Suffix.trim();
231+
if (!llvm::all_of(Extension, isAlphanumeric))
232+
return false;
233+
FileExtensions.insert(Extension);
234+
}
235+
return true;
236+
}
237+
225238
void ClangTidyContext::setCurrentFile(StringRef File) {
226239
CurrentFile = std::string(File);
227240
CurrentOptions = getOptionsForFile(CurrentFile);
228241
CheckFilter = std::make_unique<CachedGlobList>(*getOptions().Checks);
229242
WarningAsErrorFilter =
230243
std::make_unique<CachedGlobList>(*getOptions().WarningsAsErrors);
244+
if (!parseFileExtensions(*getOptions().HeaderFileExtensions,
245+
HeaderFileExtensions))
246+
this->configurationDiag("Invalid header file extensions");
247+
if (!parseFileExtensions(*getOptions().ImplementationFileExtensions,
248+
ImplementationFileExtensions))
249+
this->configurationDiag("Invalid implementation file extensions");
231250
}
232251

233252
void ClangTidyContext::setASTContext(ASTContext *Context) {

clang-tools-extra/clang-tidy/ClangTidyDiagnosticConsumer.h

Lines changed: 12 additions & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -11,6 +11,7 @@
1111

1212
#include "ClangTidyOptions.h"
1313
#include "ClangTidyProfiling.h"
14+
#include "FileExtensionsSet.h"
1415
#include "NoLintDirectiveHandler.h"
1516
#include "clang/Basic/Diagnostic.h"
1617
#include "clang/Tooling/Core/Diagnostic.h"
@@ -160,6 +161,14 @@ class ClangTidyContext {
160161
/// \c CurrentFile.
161162
ClangTidyOptions getOptionsForFile(StringRef File) const;
162163

164+
const FileExtensionsSet &getHeaderFileExtensions() const {
165+
return HeaderFileExtensions;
166+
}
167+
168+
const FileExtensionsSet &getImplementationFileExtensions() const {
169+
return ImplementationFileExtensions;
170+
}
171+
163172
/// Returns \c ClangTidyStats containing issued and ignored diagnostic
164173
/// counters.
165174
const ClangTidyStats &getStats() const { return Stats; }
@@ -221,6 +230,9 @@ class ClangTidyContext {
221230
std::unique_ptr<CachedGlobList> CheckFilter;
222231
std::unique_ptr<CachedGlobList> WarningAsErrorFilter;
223232

233+
FileExtensionsSet HeaderFileExtensions;
234+
FileExtensionsSet ImplementationFileExtensions;
235+
224236
LangOptions LangOpts;
225237

226238
ClangTidyStats Stats;

clang-tools-extra/clang-tidy/ClangTidyOptions.cpp

Lines changed: 8 additions & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -122,6 +122,9 @@ template <> struct MappingTraits<ClangTidyOptions> {
122122
bool Ignored = false;
123123
IO.mapOptional("Checks", Options.Checks);
124124
IO.mapOptional("WarningsAsErrors", Options.WarningsAsErrors);
125+
IO.mapOptional("HeaderFileExtensions", Options.HeaderFileExtensions);
126+
IO.mapOptional("ImplementationFileExtensions",
127+
Options.ImplementationFileExtensions);
125128
IO.mapOptional("HeaderFilterRegex", Options.HeaderFilterRegex);
126129
IO.mapOptional("AnalyzeTemporaryDtors", Ignored); // deprecated
127130
IO.mapOptional("FormatStyle", Options.FormatStyle);
@@ -142,6 +145,8 @@ ClangTidyOptions ClangTidyOptions::getDefaults() {
142145
ClangTidyOptions Options;
143146
Options.Checks = "";
144147
Options.WarningsAsErrors = "";
148+
Options.HeaderFileExtensions = {"", "h", "hh", "hpp", "hxx"};
149+
Options.ImplementationFileExtensions = {"c", "cc", "cpp", "cxx"};
145150
Options.HeaderFilterRegex = "";
146151
Options.SystemHeaders = false;
147152
Options.FormatStyle = "none";
@@ -178,6 +183,9 @@ ClangTidyOptions &ClangTidyOptions::mergeWith(const ClangTidyOptions &Other,
178183
unsigned Order) {
179184
mergeCommaSeparatedLists(Checks, Other.Checks);
180185
mergeCommaSeparatedLists(WarningsAsErrors, Other.WarningsAsErrors);
186+
overrideValue(HeaderFileExtensions, Other.HeaderFileExtensions);
187+
overrideValue(ImplementationFileExtensions,
188+
Other.ImplementationFileExtensions);
181189
overrideValue(HeaderFilterRegex, Other.HeaderFilterRegex);
182190
overrideValue(SystemHeaders, Other.SystemHeaders);
183191
overrideValue(FormatStyle, Other.FormatStyle);

clang-tools-extra/clang-tidy/ClangTidyOptions.h

Lines changed: 8 additions & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -71,6 +71,14 @@ struct ClangTidyOptions {
7171
/// WarningsAsErrors filter.
7272
std::optional<std::string> WarningsAsErrors;
7373

74+
/// File extensions to consider to determine if a given diagnostic is located
75+
/// in a header file.
76+
std::optional<std::vector<std::string>> HeaderFileExtensions;
77+
78+
/// File extensions to consider to determine if a given diagnostic is located
79+
/// is located in an implementation file.
80+
std::optional<std::vector<std::string>> ImplementationFileExtensions;
81+
7482
/// Output warnings from headers matching this filter. Warnings from
7583
/// main files will always be displayed.
7684
std::optional<std::string> HeaderFilterRegex;
Lines changed: 19 additions & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -0,0 +1,19 @@
1+
//===--- FileExtensionsSet.h - clang-tidy -----------------------*- C++ -*-===//
2+
//
3+
// Part of the LLVM Project, under the Apache License v2.0 with LLVM Exceptions.
4+
// See https://llvm.org/LICENSE.txt for license information.
5+
// SPDX-License-Identifier: Apache-2.0 WITH LLVM-exception
6+
//
7+
//===----------------------------------------------------------------------===//
8+
9+
#ifndef LLVM_CLANG_TOOLS_EXTRA_CLANG_TIDY_FILE_EXTENSIONS_SET_H
10+
#define LLVM_CLANG_TOOLS_EXTRA_CLANG_TIDY_FILE_EXTENSIONS_SET_H
11+
12+
#include "llvm/ADT/SmallSet.h"
13+
#include "llvm/ADT/StringRef.h"
14+
15+
namespace clang::tidy {
16+
typedef llvm::SmallSet<llvm::StringRef, 5> FileExtensionsSet;
17+
} // namespace clang::tidy
18+
19+
#endif // LLVM_CLANG_TOOLS_EXTRA_CLANG_TIDY_FILE_EXTENSIONS_SET_H

clang-tools-extra/clang-tidy/bugprone/DynamicStaticInitializersCheck.cpp

Lines changed: 17 additions & 11 deletions
Original file line numberDiff line numberDiff line change
@@ -7,6 +7,7 @@
77
//===----------------------------------------------------------------------===//
88

99
#include "DynamicStaticInitializersCheck.h"
10+
#include "../utils/FileExtensionsUtils.h"
1011
#include "clang/AST/ASTContext.h"
1112
#include "clang/ASTMatchers/ASTMatchFinder.h"
1213

@@ -24,17 +25,22 @@ AST_MATCHER(clang::VarDecl, hasConstantDeclaration) {
2425
return false;
2526
}
2627

27-
DynamicStaticInitializersCheck::DynamicStaticInitializersCheck(StringRef Name,
28-
ClangTidyContext *Context)
29-
: ClangTidyCheck(Name, Context),
30-
RawStringHeaderFileExtensions(Options.getLocalOrGlobal(
31-
"HeaderFileExtensions", utils::defaultHeaderFileExtensions())) {
32-
if (!utils::parseFileExtensions(RawStringHeaderFileExtensions,
33-
HeaderFileExtensions,
34-
utils::defaultFileExtensionDelimiters())) {
35-
this->configurationDiag("Invalid header file extension: '%0'")
36-
<< RawStringHeaderFileExtensions;
37-
}
28+
DynamicStaticInitializersCheck::DynamicStaticInitializersCheck(
29+
StringRef Name, ClangTidyContext *Context)
30+
: ClangTidyCheck(Name, Context) {
31+
std::optional<StringRef> HeaderFileExtensionsOption =
32+
Options.get("HeaderFileExtensions");
33+
RawStringHeaderFileExtensions =
34+
HeaderFileExtensionsOption.value_or(utils::defaultHeaderFileExtensions());
35+
if (HeaderFileExtensionsOption) {
36+
if (!utils::parseFileExtensions(RawStringHeaderFileExtensions,
37+
HeaderFileExtensions,
38+
utils::defaultFileExtensionDelimiters())) {
39+
this->configurationDiag("Invalid header file extension: '%0'")
40+
<< RawStringHeaderFileExtensions;
41+
}
42+
} else
43+
HeaderFileExtensions = Context->getHeaderFileExtensions();
3844
}
3945

4046
void DynamicStaticInitializersCheck::storeOptions(

clang-tools-extra/clang-tidy/bugprone/DynamicStaticInitializersCheck.h

Lines changed: 3 additions & 3 deletions
Original file line numberDiff line numberDiff line change
@@ -10,7 +10,7 @@
1010
#define LLVM_CLANG_TOOLS_EXTRA_CLANG_TIDY_BUGPRONE_DYNAMIC_STATIC_INITIALIZERS_CHECK_H
1111

1212
#include "../ClangTidyCheck.h"
13-
#include "../utils/FileExtensionsUtils.h"
13+
#include "../FileExtensionsSet.h"
1414

1515
namespace clang::tidy::bugprone {
1616

@@ -34,8 +34,8 @@ class DynamicStaticInitializersCheck : public ClangTidyCheck {
3434
void check(const ast_matchers::MatchFinder::MatchResult &Result) override;
3535

3636
private:
37-
const StringRef RawStringHeaderFileExtensions;
38-
utils::FileExtensionsSet HeaderFileExtensions;
37+
StringRef RawStringHeaderFileExtensions;
38+
FileExtensionsSet HeaderFileExtensions;
3939
};
4040

4141
} // namespace clang::tidy::bugprone

clang-tools-extra/clang-tidy/bugprone/SuspiciousIncludeCheck.cpp

Lines changed: 30 additions & 19 deletions
Original file line numberDiff line numberDiff line change
@@ -7,6 +7,7 @@
77
//===----------------------------------------------------------------------===//
88

99
#include "SuspiciousIncludeCheck.h"
10+
#include "../utils/FileExtensionsUtils.h"
1011
#include "clang/AST/ASTContext.h"
1112
#include "clang/Lex/Preprocessor.h"
1213
#include <optional>
@@ -36,25 +37,35 @@ class SuspiciousIncludePPCallbacks : public PPCallbacks {
3637

3738
SuspiciousIncludeCheck::SuspiciousIncludeCheck(StringRef Name,
3839
ClangTidyContext *Context)
39-
: ClangTidyCheck(Name, Context),
40-
RawStringHeaderFileExtensions(Options.getLocalOrGlobal(
41-
"HeaderFileExtensions", utils::defaultHeaderFileExtensions())),
42-
RawStringImplementationFileExtensions(Options.getLocalOrGlobal(
43-
"ImplementationFileExtensions",
44-
utils::defaultImplementationFileExtensions())) {
45-
if (!utils::parseFileExtensions(RawStringImplementationFileExtensions,
46-
ImplementationFileExtensions,
47-
utils::defaultFileExtensionDelimiters())) {
48-
this->configurationDiag("Invalid implementation file extension: '%0'")
49-
<< RawStringImplementationFileExtensions;
50-
}
51-
52-
if (!utils::parseFileExtensions(RawStringHeaderFileExtensions,
53-
HeaderFileExtensions,
54-
utils::defaultFileExtensionDelimiters())) {
55-
this->configurationDiag("Invalid header file extension: '%0'")
56-
<< RawStringHeaderFileExtensions;
57-
}
40+
: ClangTidyCheck(Name, Context) {
41+
std::optional<StringRef> ImplementationFileExtensionsOption =
42+
Options.get("ImplementationFileExtensions");
43+
RawStringImplementationFileExtensions =
44+
ImplementationFileExtensionsOption.value_or(
45+
utils::defaultImplementationFileExtensions());
46+
if (ImplementationFileExtensionsOption) {
47+
if (!utils::parseFileExtensions(RawStringImplementationFileExtensions,
48+
ImplementationFileExtensions,
49+
utils::defaultFileExtensionDelimiters())) {
50+
this->configurationDiag("Invalid implementation file extension: '%0'")
51+
<< RawStringImplementationFileExtensions;
52+
}
53+
} else
54+
ImplementationFileExtensions = Context->getImplementationFileExtensions();
55+
56+
std::optional<StringRef> HeaderFileExtensionsOption =
57+
Options.get("HeaderFileExtensions");
58+
RawStringHeaderFileExtensions =
59+
HeaderFileExtensionsOption.value_or(utils::defaultHeaderFileExtensions());
60+
if (HeaderFileExtensionsOption) {
61+
if (!utils::parseFileExtensions(RawStringHeaderFileExtensions,
62+
HeaderFileExtensions,
63+
utils::defaultFileExtensionDelimiters())) {
64+
this->configurationDiag("Invalid header file extension: '%0'")
65+
<< RawStringHeaderFileExtensions;
66+
}
67+
} else
68+
HeaderFileExtensions = Context->getHeaderFileExtensions();
5869
}
5970

6071
void SuspiciousIncludeCheck::storeOptions(ClangTidyOptions::OptionMap &Opts) {

clang-tools-extra/clang-tidy/bugprone/SuspiciousIncludeCheck.h

Lines changed: 4 additions & 4 deletions
Original file line numberDiff line numberDiff line change
@@ -40,12 +40,12 @@ class SuspiciousIncludeCheck : public ClangTidyCheck {
4040
Preprocessor *ModuleExpanderPP) override;
4141
void storeOptions(ClangTidyOptions::OptionMap &Opts) override;
4242

43-
utils::FileExtensionsSet HeaderFileExtensions;
44-
utils::FileExtensionsSet ImplementationFileExtensions;
43+
FileExtensionsSet HeaderFileExtensions;
44+
FileExtensionsSet ImplementationFileExtensions;
4545

4646
private:
47-
const StringRef RawStringHeaderFileExtensions;
48-
const StringRef RawStringImplementationFileExtensions;
47+
StringRef RawStringHeaderFileExtensions;
48+
StringRef RawStringImplementationFileExtensions;
4949
};
5050

5151
} // namespace clang::tidy::bugprone

clang-tools-extra/clang-tidy/google/GlobalNamesInHeadersCheck.cpp

Lines changed: 14 additions & 9 deletions
Original file line numberDiff line numberDiff line change
@@ -18,15 +18,20 @@ namespace clang::tidy::google::readability {
1818

1919
GlobalNamesInHeadersCheck::GlobalNamesInHeadersCheck(StringRef Name,
2020
ClangTidyContext *Context)
21-
: ClangTidyCheck(Name, Context),
22-
RawStringHeaderFileExtensions(Options.getLocalOrGlobal(
23-
"HeaderFileExtensions", utils::defaultHeaderFileExtensions())) {
24-
if (!utils::parseFileExtensions(RawStringHeaderFileExtensions,
25-
HeaderFileExtensions,
26-
utils::defaultFileExtensionDelimiters())) {
27-
this->configurationDiag("Invalid header file extension: '%0'")
28-
<< RawStringHeaderFileExtensions;
29-
}
21+
: ClangTidyCheck(Name, Context) {
22+
std::optional<StringRef> HeaderFileExtensionsOption =
23+
Options.get("HeaderFileExtensions");
24+
RawStringHeaderFileExtensions =
25+
HeaderFileExtensionsOption.value_or(utils::defaultHeaderFileExtensions());
26+
if (HeaderFileExtensionsOption) {
27+
if (!utils::parseFileExtensions(RawStringHeaderFileExtensions,
28+
HeaderFileExtensions,
29+
utils::defaultFileExtensionDelimiters())) {
30+
this->configurationDiag("Invalid header file extension: '%0'")
31+
<< RawStringHeaderFileExtensions;
32+
}
33+
} else
34+
HeaderFileExtensions = Context->getHeaderFileExtensions();
3035
}
3136

3237
void GlobalNamesInHeadersCheck::storeOptions(

clang-tools-extra/clang-tidy/google/GlobalNamesInHeadersCheck.h

Lines changed: 2 additions & 2 deletions
Original file line numberDiff line numberDiff line change
@@ -35,8 +35,8 @@ class GlobalNamesInHeadersCheck : public ClangTidyCheck {
3535
void check(const ast_matchers::MatchFinder::MatchResult &Result) override;
3636

3737
private:
38-
const StringRef RawStringHeaderFileExtensions;
39-
utils::FileExtensionsSet HeaderFileExtensions;
38+
StringRef RawStringHeaderFileExtensions;
39+
FileExtensionsSet HeaderFileExtensions;
4040
};
4141

4242
} // namespace clang::tidy::google::readability

clang-tools-extra/clang-tidy/google/UnnamedNamespaceInHeaderCheck.cpp

Lines changed: 14 additions & 9 deletions
Original file line numberDiff line numberDiff line change
@@ -17,15 +17,20 @@ namespace clang::tidy::google::build {
1717

1818
UnnamedNamespaceInHeaderCheck::UnnamedNamespaceInHeaderCheck(
1919
StringRef Name, ClangTidyContext *Context)
20-
: ClangTidyCheck(Name, Context),
21-
RawStringHeaderFileExtensions(Options.getLocalOrGlobal(
22-
"HeaderFileExtensions", utils::defaultHeaderFileExtensions())) {
23-
if (!utils::parseFileExtensions(RawStringHeaderFileExtensions,
24-
HeaderFileExtensions,
25-
utils::defaultFileExtensionDelimiters())) {
26-
this->configurationDiag("Invalid header file extension: '%0'")
27-
<< RawStringHeaderFileExtensions;
28-
}
20+
: ClangTidyCheck(Name, Context) {
21+
std::optional<StringRef> HeaderFileExtensionsOption =
22+
Options.get("HeaderFileExtensions");
23+
RawStringHeaderFileExtensions =
24+
HeaderFileExtensionsOption.value_or(utils::defaultHeaderFileExtensions());
25+
if (HeaderFileExtensionsOption) {
26+
if (!utils::parseFileExtensions(RawStringHeaderFileExtensions,
27+
HeaderFileExtensions,
28+
utils::defaultFileExtensionDelimiters())) {
29+
this->configurationDiag("Invalid header file extension: '%0'")
30+
<< RawStringHeaderFileExtensions;
31+
}
32+
} else
33+
HeaderFileExtensions = Context->getHeaderFileExtensions();
2934
}
3035

3136
void UnnamedNamespaceInHeaderCheck::storeOptions(

clang-tools-extra/clang-tidy/google/UnnamedNamespaceInHeaderCheck.h

Lines changed: 2 additions & 2 deletions
Original file line numberDiff line numberDiff line change
@@ -41,8 +41,8 @@ class UnnamedNamespaceInHeaderCheck : public ClangTidyCheck {
4141
void check(const ast_matchers::MatchFinder::MatchResult &Result) override;
4242

4343
private:
44-
const StringRef RawStringHeaderFileExtensions;
45-
utils::FileExtensionsSet HeaderFileExtensions;
44+
StringRef RawStringHeaderFileExtensions;
45+
FileExtensionsSet HeaderFileExtensions;
4646
};
4747

4848
} // namespace clang::tidy::google::build

clang-tools-extra/clang-tidy/llvmlibc/InlineFunctionDeclCheck.cpp

Lines changed: 2 additions & 19 deletions
Original file line numberDiff line numberDiff line change
@@ -7,6 +7,7 @@
77
//===----------------------------------------------------------------------===//
88

99
#include "InlineFunctionDeclCheck.h"
10+
#include "../utils/FileExtensionsUtils.h"
1011
#include "clang/AST/ASTContext.h"
1112
#include "clang/ASTMatchers/ASTMatchFinder.h"
1213

@@ -19,30 +20,12 @@ namespace clang::tidy::llvm_libc {
1920
InlineFunctionDeclCheck::InlineFunctionDeclCheck(StringRef Name,
2021
ClangTidyContext *Context)
2122
: ClangTidyCheck(Name, Context),
22-
RawStringHeaderFileExtensions(Options.getLocalOrGlobal(
23-
"HeaderFileExtensions", utils::defaultHeaderFileExtensions())) {
24-
if (!utils::parseFileExtensions(RawStringHeaderFileExtensions,
25-
HeaderFileExtensions,
26-
utils::defaultFileExtensionDelimiters())) {
27-
this->configurationDiag("Invalid header file extension: '%0'")
28-
<< RawStringHeaderFileExtensions;
29-
}
30-
}
23+
HeaderFileExtensions(Context->getHeaderFileExtensions()) {}
3124

3225
void InlineFunctionDeclCheck::registerMatchers(MatchFinder *Finder) {
3326
Finder->addMatcher(decl(functionDecl()).bind("func_decl"), this);
3427
}
3528

36-
void InlineFunctionDeclCheck::storeOptions(ClangTidyOptions::OptionMap &Opts) {
37-
Options.store(Opts, "HeaderFileExtensions", RawStringHeaderFileExtensions);
38-
if (!utils::parseFileExtensions(RawStringHeaderFileExtensions,
39-
HeaderFileExtensions,
40-
utils::defaultFileExtensionDelimiters())) {
41-
this->configurationDiag("Invalid header file extension: '%0'")
42-
<< RawStringHeaderFileExtensions;
43-
}
44-
}
45-
4629
void InlineFunctionDeclCheck::check(const MatchFinder::MatchResult &Result) {
4730
const auto *FuncDecl = Result.Nodes.getNodeAs<FunctionDecl>("func_decl");
4831

0 commit comments

Comments
 (0)