[clang-tidy] readability-identifier-naming checks configs for included files

When checking for the style of a decl that isn't in the main file, the check will now search for the configuration that the included files uses to gather the style for its decls.

This can be useful to silence warnings in header files that follow a different naming convention without using header-filter to silence all warnings(even from other checks) in the header file.

Reviewed By: aaron.ballman, gribozavr2

Differential Revision: https://reviews.llvm.org/D84814
This commit is contained in:
Nathan James 2020-08-01 10:35:13 +01:00
parent 75f134eec1
commit 4888c9ce97
No known key found for this signature in database
GPG Key ID: CC007AFCDA90AA5F
11 changed files with 201 additions and 68 deletions

View File

@ -8,6 +8,7 @@
#include "IdentifierNamingCheck.h"
#include "../GlobList.h"
#include "clang/AST/CXXInheritance.h"
#include "clang/Lex/PPCallbacks.h"
#include "clang/Lex/Preprocessor.h"
@ -15,7 +16,8 @@
#include "llvm/ADT/DenseMapInfo.h"
#include "llvm/Support/Debug.h"
#include "llvm/Support/Error.h"
#include "llvm/Support/Format.h"
#include "llvm/Support/FormatVariadic.h"
#include "llvm/Support/Path.h"
#include "llvm/Support/Regex.h"
#define DEBUG_TYPE "clang-tidy"
@ -119,41 +121,47 @@ static StringRef const StyleNames[] = {
#undef NAMING_KEYS
// clang-format on
static std::vector<llvm::Optional<IdentifierNamingCheck::NamingStyle>>
getNamingStyles(const ClangTidyCheck::OptionsView &Options) {
std::vector<llvm::Optional<IdentifierNamingCheck::NamingStyle>> Styles;
Styles.reserve(StyleNames->size());
for (auto const &StyleName : StyleNames) {
auto CaseOptional = Options.getOptional<IdentifierNamingCheck::CaseType>(
(StyleName + "Case").str());
auto Prefix = Options.get((StyleName + "Prefix").str(), "");
auto Postfix = Options.get((StyleName + "Suffix").str(), "");
if (CaseOptional || !Prefix.empty() || !Postfix.empty())
Styles.emplace_back(IdentifierNamingCheck::NamingStyle{
std::move(CaseOptional), std::move(Prefix), std::move(Postfix)});
else
Styles.emplace_back(llvm::None);
}
return Styles;
}
IdentifierNamingCheck::IdentifierNamingCheck(StringRef Name,
ClangTidyContext *Context)
: RenamerClangTidyCheck(Name, Context),
: RenamerClangTidyCheck(Name, Context), Context(Context), CheckName(Name),
GetConfigPerFile(Options.get("GetConfigPerFile", true)),
IgnoreFailedSplit(Options.get("IgnoreFailedSplit", false)),
IgnoreMainLikeFunctions(Options.get("IgnoreMainLikeFunctions", false)) {
for (auto const &Name : StyleNames) {
auto CaseOptional = [&]() -> llvm::Optional<CaseType> {
auto ValueOr = Options.get<CaseType>((Name + "Case").str());
if (ValueOr)
return *ValueOr;
llvm::logAllUnhandledErrors(
llvm::handleErrors(ValueOr.takeError(),
[](const MissingOptionError &) -> llvm::Error {
return llvm::Error::success();
}),
llvm::errs(), "warning: ");
return llvm::None;
}();
auto prefix = Options.get((Name + "Prefix").str(), "");
auto postfix = Options.get((Name + "Suffix").str(), "");
if (CaseOptional || !prefix.empty() || !postfix.empty()) {
NamingStyles.push_back(NamingStyle(CaseOptional, prefix, postfix));
} else {
NamingStyles.push_back(llvm::None);
}
}
auto IterAndInserted = NamingStylesCache.try_emplace(
llvm::sys::path::parent_path(Context->getCurrentFile()),
getNamingStyles(Options));
assert(IterAndInserted.second && "Couldn't insert Style");
// Holding a reference to the data in the vector is safe as it should never
// move.
MainFileStyle = IterAndInserted.first->getValue();
}
IdentifierNamingCheck::~IdentifierNamingCheck() = default;
void IdentifierNamingCheck::storeOptions(ClangTidyOptions::OptionMap &Opts) {
RenamerClangTidyCheck::storeOptions(Opts);
ArrayRef<llvm::Optional<NamingStyle>> NamingStyles =
getStyleForFile(Context->getCurrentFile());
for (size_t i = 0; i < SK_Count; ++i) {
if (NamingStyles[i]) {
if (NamingStyles[i]->Case) {
@ -166,7 +174,7 @@ void IdentifierNamingCheck::storeOptions(ClangTidyOptions::OptionMap &Opts) {
NamingStyles[i]->Suffix);
}
}
Options.store(Opts, "GetConfigPerFile", GetConfigPerFile);
Options.store(Opts, "IgnoreFailedSplit", IgnoreFailedSplit);
Options.store(Opts, "IgnoreMainLikeFunctions", IgnoreMainLikeFunctions);
}
@ -374,8 +382,7 @@ fixupWithStyle(StringRef Name,
static StyleKind findStyleKind(
const NamedDecl *D,
const std::vector<llvm::Optional<IdentifierNamingCheck::NamingStyle>>
&NamingStyles,
ArrayRef<llvm::Optional<IdentifierNamingCheck::NamingStyle>> NamingStyles,
bool IgnoreMainLikeFunctions) {
assert(D && D->getIdentifier() && !D->getName().empty() && !D->isImplicit() &&
"Decl must be an explicit identifier with a name.");
@ -652,63 +659,56 @@ static StyleKind findStyleKind(
return SK_Invalid;
}
llvm::Optional<RenamerClangTidyCheck::FailureInfo>
IdentifierNamingCheck::GetDeclFailureInfo(const NamedDecl *Decl,
const SourceManager &SM) const {
StyleKind SK = findStyleKind(Decl, NamingStyles, IgnoreMainLikeFunctions);
if (SK == SK_Invalid)
static llvm::Optional<RenamerClangTidyCheck::FailureInfo> getFailureInfo(
StringRef Name, SourceLocation Location,
ArrayRef<llvm::Optional<IdentifierNamingCheck::NamingStyle>> NamingStyles,
StyleKind SK, const SourceManager &SM, bool IgnoreFailedSplit) {
if (SK == SK_Invalid || !NamingStyles[SK])
return None;
if (!NamingStyles[SK])
return None;
const NamingStyle &Style = *NamingStyles[SK];
StringRef Name = Decl->getName();
const IdentifierNamingCheck::NamingStyle &Style = *NamingStyles[SK];
if (matchesStyle(Name, Style))
return None;
std::string KindName = fixupWithCase(StyleNames[SK], CT_LowerCase);
std::string KindName =
fixupWithCase(StyleNames[SK], IdentifierNamingCheck::CT_LowerCase);
std::replace(KindName.begin(), KindName.end(), '_', ' ');
std::string Fixup = fixupWithStyle(Name, Style);
if (StringRef(Fixup).equals(Name)) {
if (!IgnoreFailedSplit) {
LLVM_DEBUG(llvm::dbgs()
<< Decl->getBeginLoc().printToString(SM)
<< llvm::format(": unable to split words for %s '%s'\n",
KindName.c_str(), Name.str().c_str()));
LLVM_DEBUG(Location.print(llvm::dbgs(), SM);
llvm::dbgs()
<< llvm::formatv(": unable to split words for {0} '{1}'\n",
KindName, Name));
}
return None;
}
return FailureInfo{std::move(KindName), std::move(Fixup)};
return RenamerClangTidyCheck::FailureInfo{std::move(KindName),
std::move(Fixup)};
}
llvm::Optional<RenamerClangTidyCheck::FailureInfo>
IdentifierNamingCheck::GetDeclFailureInfo(const NamedDecl *Decl,
const SourceManager &SM) const {
SourceLocation Loc = Decl->getLocation();
ArrayRef<llvm::Optional<NamingStyle>> NamingStyles =
getStyleForFile(SM.getFilename(Loc));
return getFailureInfo(
Decl->getName(), Loc, NamingStyles,
findStyleKind(Decl, NamingStyles, IgnoreMainLikeFunctions), SM,
IgnoreFailedSplit);
}
llvm::Optional<RenamerClangTidyCheck::FailureInfo>
IdentifierNamingCheck::GetMacroFailureInfo(const Token &MacroNameTok,
const SourceManager &SM) const {
if (!NamingStyles[SK_MacroDefinition])
return None;
SourceLocation Loc = MacroNameTok.getLocation();
StringRef Name = MacroNameTok.getIdentifierInfo()->getName();
const NamingStyle &Style = *NamingStyles[SK_MacroDefinition];
if (matchesStyle(Name, Style))
return None;
std::string KindName =
fixupWithCase(StyleNames[SK_MacroDefinition], CT_LowerCase);
std::replace(KindName.begin(), KindName.end(), '_', ' ');
std::string Fixup = fixupWithStyle(Name, Style);
if (StringRef(Fixup).equals(Name)) {
if (!IgnoreFailedSplit) {
LLVM_DEBUG(llvm::dbgs()
<< MacroNameTok.getLocation().printToString(SM)
<< llvm::format(": unable to split words for %s '%s'\n",
KindName.c_str(), Name.str().c_str()));
}
return None;
}
return FailureInfo{std::move(KindName), std::move(Fixup)};
return getFailureInfo(MacroNameTok.getIdentifierInfo()->getName(), Loc,
getStyleForFile(SM.getFilename(Loc)),
SK_MacroDefinition, SM, IgnoreFailedSplit);
}
RenamerClangTidyCheck::DiagInfo
@ -720,6 +720,21 @@ IdentifierNamingCheck::GetDiagInfo(const NamingCheckId &ID,
}};
}
ArrayRef<llvm::Optional<IdentifierNamingCheck::NamingStyle>>
IdentifierNamingCheck::getStyleForFile(StringRef FileName) const {
if (!GetConfigPerFile)
return MainFileStyle;
auto &Styles = NamingStylesCache[llvm::sys::path::parent_path(FileName)];
if (Styles.empty()) {
ClangTidyOptions Options = Context->getOptionsForFile(FileName);
if (Options.Checks && GlobList(*Options.Checks).contains(CheckName))
Styles = getNamingStyles({CheckName, Options.CheckOptions});
else
Styles.resize(SK_Count, None);
}
return Styles;
}
} // namespace readability
} // namespace tidy
} // namespace clang

View File

@ -10,6 +10,7 @@
#define LLVM_CLANG_TOOLS_EXTRA_CLANG_TIDY_READABILITY_IDENTIFIERNAMINGCHECK_H
#include "../utils/RenamerClangTidyCheck.h"
#include "llvm/ADT/Optional.h"
namespace clang {
class MacroInfo;
@ -69,7 +70,17 @@ private:
DiagInfo GetDiagInfo(const NamingCheckId &ID,
const NamingCheckFailure &Failure) const override;
std::vector<llvm::Optional<NamingStyle>> NamingStyles;
ArrayRef<llvm::Optional<NamingStyle>>
getStyleForFile(StringRef FileName) const;
/// Stores the style options as a vector, indexed by the specified \ref
/// StyleKind, for a given directory.
mutable llvm::StringMap<std::vector<llvm::Optional<NamingStyle>>>
NamingStylesCache;
ArrayRef<llvm::Optional<NamingStyle>> MainFileStyle;
ClangTidyContext *const Context;
const std::string CheckName;
const bool GetConfigPerFile;
const bool IgnoreFailedSplit;
const bool IgnoreMainLikeFunctions;
};

View File

@ -67,7 +67,14 @@ The improvements are...
Improvements to clang-tidy
--------------------------
The improvements are...
Changes in existing checks
^^^^^^^^^^^^^^^^^^^^^^^^^^
- Improved :doc:`readability-identifier-naming
<clang-tidy/checks/readability-identifier-naming>` check.
Added an option `GetConfigPerFile` to support including files which use
different naming styles.
Improvements to include-fixer
-----------------------------

View File

@ -51,6 +51,7 @@ The following options are describe below:
- :option:`EnumCase`, :option:`EnumPrefix`, :option:`EnumSuffix`
- :option:`EnumConstantCase`, :option:`EnumConstantPrefix`, :option:`EnumConstantSuffix`
- :option:`FunctionCase`, :option:`FunctionPrefix`, :option:`FunctionSuffix`
- :option:`GetConfigPerFile`
- :option:`GlobalConstantCase`, :option:`GlobalConstantPrefix`, :option:`GlobalConstantSuffix`
- :option:`GlobalConstantPointerCase`, :option:`GlobalConstantPointerPrefix`, :option:`GlobalConstantPointerSuffix`
- :option:`GlobalFunctionCase`, :option:`GlobalFunctionPrefix`, :option:`GlobalFunctionSuffix`
@ -713,6 +714,13 @@ After:
char pre_my_function_string_post();
.. option:: GetConfigPerFile
When `true` the check will look for the configuration for where an
identifier is declared. Useful for when included header files use a
different style.
Default value is `true`.
.. option:: GlobalConstantCase
When defined, the check will ensure global constant names conform to the

View File

@ -0,0 +1,5 @@
Checks: -readability-identifier-naming
CheckOptions:
- key: readability-identifier-naming.GlobalFunctionCase
value: lower_case

View File

@ -0,0 +1,3 @@
void disabled_style_1();
void disabledStyle2();
void DISABLED_STYLE_3();

View File

@ -0,0 +1,5 @@
Checks: readability-identifier-naming
CheckOptions:
- key: readability-identifier-naming.GlobalFunctionCase
value: lower_case

View File

@ -0,0 +1,5 @@
void style_first_good();
void styleFirstBad();

View File

@ -0,0 +1,5 @@
Checks: readability-identifier-naming
CheckOptions:
- key: readability-identifier-naming.GlobalFunctionCase
value: UPPER_CASE

View File

@ -0,0 +1,5 @@
void STYLE_SECOND_GOOD();
void styleSecondBad();

View File

@ -0,0 +1,64 @@
// Setup header directory
// RUN: rm -rf %theaders
// RUN: mkdir %theaders
// RUN: cp -R %S/Inputs/readability-identifier-naming/. %theaders
// C++11 isn't explicitly required, but failing to specify a standard means the
// check will run multiple times for different standards. This will cause the
// second test to fail as the header file will be changed during the first run.
// InheritParentConfig is needed to look for the clang-tidy configuration files.
// RUN: %check_clang_tidy -check-suffixes=ENABLED,SHARED -std=c++11 %s \
// RUN: readability-identifier-naming %t -- \
// RUN: -config='{ InheritParentConfig: true, CheckOptions: [ \
// RUN: {key: readability-identifier-naming.FunctionCase, value: camelBack}, \
// RUN: {key: readability-identifier-naming.GetConfigPerFile, value: true} \
// RUN: ]}' -header-filter='.*' -- -I%theaders
// On DISABLED run, everything should be made 'camelBack'.
// RUN: cp -R %S/Inputs/readability-identifier-naming/. %theaders
// RUN: %check_clang_tidy -check-suffixes=DISABLED,SHARED -std=c++11 %s \
// RUN: readability-identifier-naming %t -- \
// RUN: -config='{ InheritParentConfig: true, CheckOptions: [ \
// RUN: {key: readability-identifier-naming.FunctionCase, value: camelBack}, \
// RUN: {key: readability-identifier-naming.GetConfigPerFile, value: false} \
// RUN: ]}' -header-filter='.*' -- -I%theaders
#include "global-style-disabled/header.h"
#include "global-style1/header.h"
#include "global-style2/header.h"
// CHECK-MESSAGES-ENABLED-DAG: global-style1/header.h:5:6: warning: invalid case style for global function 'styleFirstBad'
// CHECK-MESSAGES-ENABLED-DAG: global-style2/header.h:5:6: warning: invalid case style for global function 'styleSecondBad'
// CHECK-MESSAGES-DISABLED-DAG: global-style1/header.h:3:6: warning: invalid case style for function 'style_first_good'
// CHECK-MESSAGES-DISABLED-DAG: global-style2/header.h:3:6: warning: invalid case style for function 'STYLE_SECOND_GOOD'
// CHECK-MESSAGES-DISABLED-DAG: global-style-disabled/header.h:1:6: warning: invalid case style for function 'disabled_style_1'
// CHECK-MESSAGES-DISABLED-DAG: global-style-disabled/header.h:3:6: warning: invalid case style for function 'DISABLED_STYLE_3'
void goodStyle() {
style_first_good();
STYLE_SECOND_GOOD();
// CHECK-FIXES-DISABLED: styleFirstGood();
// CHECK-FIXES-DISABLED-NEXT: styleSecondGood();
}
// CHECK-MESSAGES-SHARED-DAG: :[[@LINE+1]]:6: warning: invalid case style for function 'bad_style'
void bad_style() {
styleFirstBad();
styleSecondBad();
}
// CHECK-FIXES-SHARED: void badStyle() {
// CHECK-FIXES-DISABLED-NEXT: styleFirstBad();
// CHECK-FIXES-ENABLED-NEXT: style_first_bad();
// CHECK-FIXES-DISABLED-NEXT: styleSecondBad();
// CHECK-FIXES-ENABLED-NEXT: STYLE_SECOND_BAD();
// CHECK-FIXES-SHARED-NEXT: }
void expectNoStyle() {
disabled_style_1();
disabledStyle2();
DISABLED_STYLE_3();
// CHECK-FIXES-DISABLED: disabledStyle1();
// CHECK-FIXES-DISABLED-NEXT: disabledStyle2();
// CHECK-FIXES-DISABLED-NEXT: disabledStyle3();
}