njames93 created this revision.
njames93 added reviewers: aaron.ballman, gribozavr2.
Herald added subscribers: cfe-commits, xazax.hun.
Herald added a project: clang.
njames93 requested review of this revision.
Herald added a subscriber: aheejin.
These methods abstract away Error handling when trying to read options that
can't be parsed by logging the error automatically and returning None.
Repository:
rG LLVM Github Monorepo
https://reviews.llvm.org/D84812
Files:
clang-tools-extra/clang-tidy/ClangTidyCheck.cpp
clang-tools-extra/clang-tidy/ClangTidyCheck.h
clang-tools-extra/clang-tidy/readability/IdentifierNamingCheck.cpp
clang-tools-extra/clang-tidy/readability/IdentifierNamingCheck.h
clang-tools-extra/test/clang-tidy/checkers/Inputs/readability-identifier-naming/global-style1/.clang-tidy
clang-tools-extra/test/clang-tidy/checkers/Inputs/readability-identifier-naming/global-style1/header.h
clang-tools-extra/test/clang-tidy/checkers/Inputs/readability-identifier-naming/global-style2/.clang-tidy
clang-tools-extra/test/clang-tidy/checkers/Inputs/readability-identifier-naming/global-style2/header.h
clang-tools-extra/test/clang-tidy/checkers/readability-identifier-naming-case-violation.cpp
clang-tools-extra/test/clang-tidy/checkers/readability-identifier-naming-multiple-styles.cpp
Index: clang-tools-extra/test/clang-tidy/checkers/readability-identifier-naming-multiple-styles.cpp
===================================================================
--- /dev/null
+++ clang-tools-extra/test/clang-tidy/checkers/readability-identifier-naming-multiple-styles.cpp
@@ -0,0 +1,34 @@
+// Setup header directory
+
+// RUN: rm -rf %theaders
+// RUN: mkdir %theaders
+// RUN: cp -R %S/Inputs/readability-identifier-naming/. %theaders
+
+// C++11 isnt 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.
+
+// RUN: %check_clang_tidy -std=c++11 %s readability-identifier-naming %t -- \
+// RUN: -config='{ InheritParentConfig: true, CheckOptions: [ \
+// RUN: {key: readability-identifier-naming.FunctionCase, value: camelBack} \
+// RUN: ]}' -header-filter='.*' -- -I%theaders
+
+#include "global-style1/header.h"
+#include "global-style2/header.h"
+// CHECK-MESSAGES-DAG: global-style1/header.h:5:6: warning: invalid case style for global function 'style1Bad'
+// CHECK-MESSAGES-DAG: global-style2/header.h:5:6: warning: invalid case style for global function 'style2Bad'
+
+void goodStyle() {
+ style1_good();
+ STYLE2_GOOD();
+}
+// CHECK-MESSAGES-DAG: :[[@LINE+1]]:6: warning: invalid case style for function 'bad_style'
+void bad_style() {
+ style1Bad();
+ style2Bad();
+}
+
+// CHECK-FIXES: void badStyle() {
+// CHECK-FIXES-NEXT: style1_bad();
+// CHECK-FIXES-NEXT: STYLE2_BAD();
+// CHECK-FIXES-NEXT: }
\ No newline at end of file
Index: clang-tools-extra/test/clang-tidy/checkers/readability-identifier-naming-case-violation.cpp
===================================================================
--- clang-tools-extra/test/clang-tidy/checkers/readability-identifier-naming-case-violation.cpp
+++ clang-tools-extra/test/clang-tidy/checkers/readability-identifier-naming-case-violation.cpp
@@ -5,11 +5,11 @@
// RUN: {key: readability-identifier-naming.ClassCase, value: UUPER_CASE}, \
// RUN: {key: readability-identifier-naming.StructCase, value: CAMEL}, \
// RUN: {key: readability-identifier-naming.EnumCase, value: AnY_cASe}, \
-// RUN: ]}" 2>&1 | FileCheck %s --implicit-check-not warning
+// RUN: ]}" 2>&1 | FileCheck %s --implicit-check-not error
-// CHECK-DAG: warning: invalid configuration value 'camelback' for option 'readability-identifier-naming.FunctionCase'; did you mean 'camelBack'?{{$}}
-// CHECK-DAG: warning: invalid configuration value 'UUPER_CASE' for option 'readability-identifier-naming.ClassCase'; did you mean 'UPPER_CASE'?{{$}}
+// CHECK-DAG: error: invalid configuration value 'camelback' for option 'readability-identifier-naming.FunctionCase'; did you mean 'camelBack'?{{$}}
+// CHECK-DAG: error: invalid configuration value 'UUPER_CASE' for option 'readability-identifier-naming.ClassCase'; did you mean 'UPPER_CASE'?{{$}}
// Don't try to suggest an alternative for 'CAMEL'
-// CHECK-DAG: warning: invalid configuration value 'CAMEL' for option 'readability-identifier-naming.StructCase'{{$}}
+// CHECK-DAG: error: invalid configuration value 'CAMEL' for option 'readability-identifier-naming.StructCase'{{$}}
// This fails on the EditDistance, but as it matches ignoring case suggest the correct value
-// CHECK-DAG: warning: invalid configuration value 'AnY_cASe' for option 'readability-identifier-naming.EnumCase'; did you mean 'aNy_CasE'?{{$}}
+// CHECK-DAG: error: invalid configuration value 'AnY_cASe' for option 'readability-identifier-naming.EnumCase'; did you mean 'aNy_CasE'?{{$}}
Index: clang-tools-extra/test/clang-tidy/checkers/Inputs/readability-identifier-naming/global-style2/header.h
===================================================================
--- /dev/null
+++ clang-tools-extra/test/clang-tidy/checkers/Inputs/readability-identifier-naming/global-style2/header.h
@@ -0,0 +1,5 @@
+
+
+void STYLE2_GOOD();
+
+void style2Bad();
\ No newline at end of file
Index: clang-tools-extra/test/clang-tidy/checkers/Inputs/readability-identifier-naming/global-style2/.clang-tidy
===================================================================
--- /dev/null
+++ clang-tools-extra/test/clang-tidy/checkers/Inputs/readability-identifier-naming/global-style2/.clang-tidy
@@ -0,0 +1,4 @@
+CheckOptions:
+ - key: readability-identifier-naming.GlobalFunctionCase
+ value: UPPER_CASE
+
Index: clang-tools-extra/test/clang-tidy/checkers/Inputs/readability-identifier-naming/global-style1/header.h
===================================================================
--- /dev/null
+++ clang-tools-extra/test/clang-tidy/checkers/Inputs/readability-identifier-naming/global-style1/header.h
@@ -0,0 +1,5 @@
+
+
+void style1_good();
+
+void style1Bad();
\ No newline at end of file
Index: clang-tools-extra/test/clang-tidy/checkers/Inputs/readability-identifier-naming/global-style1/.clang-tidy
===================================================================
--- /dev/null
+++ clang-tools-extra/test/clang-tidy/checkers/Inputs/readability-identifier-naming/global-style1/.clang-tidy
@@ -0,0 +1,4 @@
+CheckOptions:
+ - key: readability-identifier-naming.GlobalFunctionCase
+ value: lower_case
+
Index: clang-tools-extra/clang-tidy/readability/IdentifierNamingCheck.h
===================================================================
--- clang-tools-extra/clang-tidy/readability/IdentifierNamingCheck.h
+++ clang-tools-extra/clang-tidy/readability/IdentifierNamingCheck.h
@@ -59,6 +59,8 @@
std::string Suffix;
};
+ using OptionalStyle = llvm::Optional<NamingStyle>;
+
private:
llvm::Optional<FailureInfo>
GetDeclFailureInfo(const NamedDecl *Decl,
@@ -69,7 +71,12 @@
DiagInfo GetDiagInfo(const NamingCheckId &ID,
const NamingCheckFailure &Failure) const override;
- std::vector<llvm::Optional<NamingStyle>> NamingStyles;
+ ArrayRef<OptionalStyle> getStyleForFile(StringRef FileName) const;
+
+ mutable llvm::StringMap<std::vector<OptionalStyle>> NamingStylesCache;
+
+ ClangTidyContext *const Context;
+ const std::string CheckName;
const bool IgnoreFailedSplit;
const bool IgnoreMainLikeFunctions;
};
Index: clang-tools-extra/clang-tidy/readability/IdentifierNamingCheck.cpp
===================================================================
--- clang-tools-extra/clang-tidy/readability/IdentifierNamingCheck.cpp
+++ clang-tools-extra/clang-tidy/readability/IdentifierNamingCheck.cpp
@@ -15,7 +15,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 +120,41 @@
#undef NAMING_KEYS
// clang-format on
+static void
+populateNaimgStyle(std::vector<IdentifierNamingCheck::OptionalStyle> &Styles,
+ const ClangTidyCheck::OptionsView &Options) {
+ assert(Styles.empty() && "Styles should be empty here");
+ 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);
+ }
+}
+
IdentifierNamingCheck::IdentifierNamingCheck(StringRef Name,
ClangTidyContext *Context)
- : RenamerClangTidyCheck(Name, Context),
+ : RenamerClangTidyCheck(Name, Context), Context(Context), CheckName(Name),
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);
- }
- }
+ populateNaimgStyle(NamingStylesCache[llvm::sys::path::parent_path(
+ Context->getCurrentFile())],
+ Options);
}
IdentifierNamingCheck::~IdentifierNamingCheck() = default;
void IdentifierNamingCheck::storeOptions(ClangTidyOptions::OptionMap &Opts) {
RenamerClangTidyCheck::storeOptions(Opts);
+ ArrayRef<OptionalStyle> NamingStyles =
+ getStyleForFile(Context->getCurrentFile());
for (size_t i = 0; i < SK_Count; ++i) {
if (NamingStyles[i]) {
if (NamingStyles[i]->Case) {
@@ -372,11 +373,10 @@
return (Style.Prefix + Mid + Style.Suffix).str();
}
-static StyleKind findStyleKind(
- const NamedDecl *D,
- const std::vector<llvm::Optional<IdentifierNamingCheck::NamingStyle>>
- &NamingStyles,
- bool IgnoreMainLikeFunctions) {
+static StyleKind
+findStyleKind(const NamedDecl *D,
+ ArrayRef<IdentifierNamingCheck::OptionalStyle> NamingStyles,
+ bool IgnoreMainLikeFunctions) {
assert(D && D->getIdentifier() && !D->getName().empty() && !D->isImplicit() &&
"Decl must be an explicit identifier with a name.");
@@ -652,63 +652,56 @@
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)
- return None;
-
- if (!NamingStyles[SK])
+static llvm::Optional<RenamerClangTidyCheck::FailureInfo>
+getFailureInfo(StringRef Name, SourceLocation Location,
+ ArrayRef<IdentifierNamingCheck::OptionalStyle> NamingStyles,
+ StyleKind SK, const SourceManager &SM, bool IgnoreFailedSplit) {
+ if (SK == SK_Invalid || !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::GetMacroFailureInfo(const Token &MacroNameTok,
- const SourceManager &SM) const {
- if (!NamingStyles[SK_MacroDefinition])
- return None;
+IdentifierNamingCheck::GetDeclFailureInfo(const NamedDecl *Decl,
+ const SourceManager &SM) const {
+ SourceLocation Loc = Decl->getLocation();
+ ArrayRef<OptionalStyle> NamingStyles = getStyleForFile(SM.getFilename(Loc));
- StringRef Name = MacroNameTok.getIdentifierInfo()->getName();
- const NamingStyle &Style = *NamingStyles[SK_MacroDefinition];
- if (matchesStyle(Name, Style))
- return None;
+ return getFailureInfo(
+ Decl->getName(), Loc, NamingStyles,
+ findStyleKind(Decl, NamingStyles, IgnoreMainLikeFunctions), SM,
+ IgnoreFailedSplit);
+}
- std::string KindName =
- fixupWithCase(StyleNames[SK_MacroDefinition], CT_LowerCase);
- std::replace(KindName.begin(), KindName.end(), '_', ' ');
+llvm::Optional<RenamerClangTidyCheck::FailureInfo>
+IdentifierNamingCheck::GetMacroFailureInfo(const Token &MacroNameTok,
+ const SourceManager &SM) const {
+ SourceLocation Loc = MacroNameTok.getLocation();
+ ArrayRef<OptionalStyle> NamingStyles = getStyleForFile(SM.getFilename(Loc));
- 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,
+ NamingStyles, SK_MacroDefinition, SM,
+ IgnoreFailedSplit);
}
RenamerClangTidyCheck::DiagInfo
@@ -720,6 +713,15 @@
}};
}
+ArrayRef<IdentifierNamingCheck::OptionalStyle>
+IdentifierNamingCheck::getStyleForFile(StringRef FileName) const {
+ auto &Styles = NamingStylesCache[llvm::sys::path::parent_path(FileName)];
+ if (Styles.empty())
+ populateNaimgStyle(
+ Styles, {CheckName, Context->getOptionsForFile(FileName).CheckOptions});
+ return Styles;
+}
+
} // namespace readability
} // namespace tidy
} // namespace clang
Index: clang-tools-extra/clang-tidy/ClangTidyCheck.h
===================================================================
--- clang-tools-extra/clang-tidy/ClangTidyCheck.h
+++ clang-tools-extra/clang-tidy/ClangTidyCheck.h
@@ -268,7 +268,7 @@
if (llvm::Expected<T> ValueOr = get<T>(LocalName))
return *ValueOr;
else
- logErrToStdErr(ValueOr.takeError());
+ logOptionParsingError(ValueOr.takeError());
return Default;
}
@@ -314,7 +314,7 @@
if (llvm::Expected<T> ValueOr = getLocalOrGlobal<T>(LocalName))
return *ValueOr;
else
- logErrToStdErr(ValueOr.takeError());
+ logOptionParsingError(ValueOr.takeError());
return Default;
}
@@ -353,7 +353,7 @@
if (auto ValueOr = get<T>(LocalName, IgnoreCase))
return *ValueOr;
else
- logErrToStdErr(ValueOr.takeError());
+ logOptionParsingError(ValueOr.takeError());
return Default;
}
@@ -395,10 +395,35 @@
if (auto ValueOr = getLocalOrGlobal<T>(LocalName, IgnoreCase))
return *ValueOr;
else
- logErrToStdErr(ValueOr.takeError());
+ logOptionParsingError(ValueOr.takeError());
return Default;
}
+ /// Returns the value for the option \p LocalName represented as a ``T``.
+ /// If the option is missing returns None, if the option can't be parsed
+ /// as a ``T``, log that to stderr and return None.
+ template <typename T = std::string>
+ llvm::Optional<T> getOptional(StringRef LocalName) const {
+ if (auto ValueOr = get<T>(LocalName))
+ return *ValueOr;
+ else
+ logOptionParsingError(ValueOr.takeError());
+ return llvm::None;
+ }
+
+ /// Returns the value for the local or global option \p LocalName
+ /// represented as a ``T``.
+ /// If the option is missing returns None, if the
+ /// option can't be parsed as a ``T``, log that to stderr and return None.
+ template <typename T = std::string>
+ llvm::Optional<T> getOptionalLocalOrGlobal(StringRef LocalName) const {
+ if (auto ValueOr = getLocalOrGlobal<T>(LocalName))
+ return *ValueOr;
+ else
+ logOptionParsingError(ValueOr.takeError());
+ return llvm::None;
+ }
+
/// Stores an option with the check-local name \p LocalName with
/// string value \p Value to \p Options.
void store(ClangTidyOptions::OptionMap &Options, StringRef LocalName,
@@ -456,7 +481,8 @@
void storeInt(ClangTidyOptions::OptionMap &Options, StringRef LocalName,
int64_t Value) const;
- static void logErrToStdErr(llvm::Error &&Err);
+ /// Logs an Error to stderr if a \p Err is not a MissingOptionError.
+ static void logOptionParsingError(llvm::Error &&Err);
std::string NamePrefix;
const ClangTidyOptions::OptionMap &CheckOptions;
@@ -524,6 +550,19 @@
ClangTidyOptions::OptionMap &Options, StringRef LocalName,
bool Value) const;
+/// Returns the value for the option \p LocalName.
+/// If the option is missing returns None.
+template <>
+Optional<std::string> ClangTidyCheck::OptionsView::getOptional<std::string>(
+ StringRef LocalName) const;
+
+/// Returns the value for the local or global option \p LocalName.
+/// If the option is missing returns None.
+template <>
+Optional<std::string>
+ClangTidyCheck::OptionsView::getOptionalLocalOrGlobal<std::string>(
+ StringRef LocalName) const;
+
} // namespace tidy
} // namespace clang
Index: clang-tools-extra/clang-tidy/ClangTidyCheck.cpp
===================================================================
--- clang-tools-extra/clang-tidy/ClangTidyCheck.cpp
+++ clang-tools-extra/clang-tidy/ClangTidyCheck.cpp
@@ -10,6 +10,7 @@
#include "llvm/ADT/SmallString.h"
#include "llvm/ADT/StringRef.h"
#include "llvm/Support/Error.h"
+#include "llvm/Support/WithColor.h"
#include "llvm/Support/raw_ostream.h"
namespace clang {
@@ -126,7 +127,7 @@
llvm::Expected<bool> ValueOr = get<bool>(LocalName);
if (ValueOr)
return *ValueOr;
- logErrToStdErr(ValueOr.takeError());
+ logOptionParsingError(ValueOr.takeError());
return Default;
}
@@ -145,7 +146,7 @@
llvm::Expected<bool> ValueOr = getLocalOrGlobal<bool>(LocalName);
if (ValueOr)
return *ValueOr;
- logErrToStdErr(ValueOr.takeError());
+ logOptionParsingError(ValueOr.takeError());
return Default;
}
@@ -204,13 +205,36 @@
Iter->second.Value);
}
-void ClangTidyCheck::OptionsView::logErrToStdErr(llvm::Error &&Err) {
- llvm::logAllUnhandledErrors(
- llvm::handleErrors(std::move(Err),
- [](const MissingOptionError &) -> llvm::Error {
- return llvm::Error::success();
- }),
- llvm::errs(), "warning: ");
+void ClangTidyCheck::OptionsView::logOptionParsingError(llvm::Error &&Err) {
+ auto RemainingErrors = llvm::handleErrors(
+ std::move(Err), [](const MissingOptionError &) -> llvm::Error {
+ return llvm::Error::success();
+ });
+ if (RemainingErrors)
+ llvm::logAllUnhandledErrors(std::move(RemainingErrors),
+ llvm::WithColor::error());
}
+
+template <>
+Optional<std::string> ClangTidyCheck::OptionsView::getOptional<std::string>(
+ StringRef LocalName) const {
+ if (auto ValueOr = get(LocalName))
+ return *ValueOr;
+ else
+ consumeError(ValueOr.takeError());
+ return llvm::None;
+}
+
+template <>
+Optional<std::string>
+ClangTidyCheck::OptionsView::getOptionalLocalOrGlobal<std::string>(
+ StringRef LocalName) const {
+ if (auto ValueOr = getLocalOrGlobal(LocalName))
+ return *ValueOr;
+ else
+ consumeError(ValueOr.takeError());
+ return llvm::None;
+}
+
} // namespace tidy
} // namespace clang
_______________________________________________
cfe-commits mailing list
[email protected]
https://lists.llvm.org/cgi-bin/mailman/listinfo/cfe-commits
