[clang-tools-extra] 7a73da4 - [clang-tidy] Add support for optional parameters in config.
Piotr Zegar via cfe-commits
cfe-commits at lists.llvm.org
Mon Oct 9 12:59:07 PDT 2023
Author: Felix
Date: 2023-10-09T19:58:44Z
New Revision: 7a73da4c85a12341752a4573c55ebff46ba20db0
URL: https://github.com/llvm/llvm-project/commit/7a73da4c85a12341752a4573c55ebff46ba20db0
DIFF: https://github.com/llvm/llvm-project/commit/7a73da4c85a12341752a4573c55ebff46ba20db0.diff
LOG: [clang-tidy] Add support for optional parameters in config.
If a parameter value is either 'none', 'null', 'false', '-1'
or '', we will in that case use the default value.
Reviewed By: PiotrZSL
Differential Revision: https://reviews.llvm.org/D159436
Added:
Modified:
clang-tools-extra/clang-tidy/ClangTidyCheck.h
clang-tools-extra/clang-tidy/readability/FunctionSizeCheck.cpp
clang-tools-extra/clang-tidy/readability/FunctionSizeCheck.h
clang-tools-extra/docs/ReleaseNotes.rst
clang-tools-extra/docs/clang-tidy/checks/readability/function-size.rst
clang-tools-extra/test/clang-tidy/checkers/readability/function-size.cpp
clang-tools-extra/test/clang-tidy/infrastructure/config-files.cpp
Removed:
################################################################################
diff --git a/clang-tools-extra/clang-tidy/ClangTidyCheck.h b/clang-tools-extra/clang-tidy/ClangTidyCheck.h
index 39b9f9fa865b76a..656a2f008f6e0e5 100644
--- a/clang-tools-extra/clang-tidy/ClangTidyCheck.h
+++ b/clang-tools-extra/clang-tidy/ClangTidyCheck.h
@@ -184,8 +184,8 @@ class ClangTidyCheck : public ast_matchers::MatchFinder::MatchCallback {
/// integral type ``T``.
///
/// Reads the option with the check-local name \p LocalName from the
- /// ``CheckOptions``. If the corresponding key is not present, return
- /// ``std::nullopt``.
+ /// ``CheckOptions``. If the corresponding key is not present,
+ /// return ``std::nullopt``.
///
/// If the corresponding key can't be parsed as a ``T``, emit a
/// diagnostic and return ``std::nullopt``.
@@ -201,6 +201,31 @@ class ClangTidyCheck : public ast_matchers::MatchFinder::MatchCallback {
return std::nullopt;
}
+ /// Read a named option from the ``Context`` and parse it as an
+ /// integral type ``T``.
+ ///
+ /// Reads the option with the check-local name \p LocalName from the
+ /// ``CheckOptions``. If the corresponding key is `none`, `null`,
+ /// `-1` or empty, return ``std::nullopt``. If the corresponding
+ /// key is not present, return \p Default.
+ ///
+ /// If the corresponding key can't be parsed as a ``T``, emit a
+ /// diagnostic and return \p Default.
+ template <typename T>
+ std::enable_if_t<std::is_integral_v<T>, std::optional<T>>
+ get(StringRef LocalName, std::optional<T> Default) const {
+ if (std::optional<StringRef> Value = get(LocalName)) {
+ if (Value == "" || Value == "none" || Value == "null" ||
+ (std::is_unsigned_v<T> && Value == "-1"))
+ return std::nullopt;
+ T Result{};
+ if (!StringRef(*Value).getAsInteger(10, Result))
+ return Result;
+ diagnoseBadIntegerOption(NamePrefix + LocalName, *Value);
+ }
+ return Default;
+ }
+
/// Read a named option from the ``Context`` and parse it as an
/// integral type ``T``.
///
@@ -245,6 +270,39 @@ class ClangTidyCheck : public ast_matchers::MatchFinder::MatchCallback {
return std::nullopt;
}
+ /// Read a named option from the ``Context`` and parse it as an
+ /// integral type ``T``.
+ ///
+ /// Reads the option with the check-local name \p LocalName from local or
+ /// global ``CheckOptions``. Gets local option first. If local is not
+ /// present, falls back to get global option. If global option is not
+ /// present either, return \p Default. If the value value was found
+ /// and equals ``none``, ``null``, ``-1`` or empty, return ``std::nullopt``.
+ ///
+ /// If the corresponding key can't be parsed as a ``T``, emit a
+ /// diagnostic and return \p Default.
+ template <typename T>
+ std::enable_if_t<std::is_integral_v<T>, std::optional<T>>
+ getLocalOrGlobal(StringRef LocalName, std::optional<T> Default) const {
+ std::optional<StringRef> ValueOr = get(LocalName);
+ bool IsGlobal = false;
+ if (!ValueOr) {
+ IsGlobal = true;
+ ValueOr = getLocalOrGlobal(LocalName);
+ if (!ValueOr)
+ return Default;
+ }
+ T Result{};
+ if (ValueOr == "" || ValueOr == "none" || ValueOr == "null" ||
+ (std::is_unsigned_v<T> && ValueOr == "-1"))
+ return std::nullopt;
+ if (!StringRef(*ValueOr).getAsInteger(10, Result))
+ return Result;
+ diagnoseBadIntegerOption(
+ IsGlobal ? Twine(LocalName) : NamePrefix + LocalName, *ValueOr);
+ return Default;
+ }
+
/// Read a named option from the ``Context`` and parse it as an
/// integral type ``T``.
///
@@ -286,8 +344,8 @@ class ClangTidyCheck : public ast_matchers::MatchFinder::MatchCallback {
/// enum type ``T``.
///
/// Reads the option with the check-local name \p LocalName from the
- /// ``CheckOptions``. If the corresponding key is not present, return
- /// \p Default.
+ /// ``CheckOptions``. If the corresponding key is not present,
+ /// return \p Default.
///
/// If the corresponding key can't be parsed as a ``T``, emit a
/// diagnostic and return \p Default.
@@ -356,6 +414,19 @@ class ClangTidyCheck : public ast_matchers::MatchFinder::MatchCallback {
storeInt(Options, LocalName, Value);
}
+ /// Stores an option with the check-local name \p LocalName with
+ /// integer value \p Value to \p Options. If the value is empty
+ /// stores ``
+ template <typename T>
+ std::enable_if_t<std::is_integral_v<T>>
+ store(ClangTidyOptions::OptionMap &Options, StringRef LocalName,
+ std::optional<T> Value) const {
+ if (Value)
+ storeInt(Options, LocalName, *Value);
+ else
+ store(Options, LocalName, "none");
+ }
+
/// Stores an option with the check-local name \p LocalName as the string
/// representation of the Enum \p Value to \p Options.
///
diff --git a/clang-tools-extra/clang-tidy/readability/FunctionSizeCheck.cpp b/clang-tools-extra/clang-tidy/readability/FunctionSizeCheck.cpp
index 3c803959caf80be..3313bcb39b7f351 100644
--- a/clang-tools-extra/clang-tidy/readability/FunctionSizeCheck.cpp
+++ b/clang-tools-extra/clang-tidy/readability/FunctionSizeCheck.cpp
@@ -126,12 +126,16 @@ class FunctionASTVisitor : public RecursiveASTVisitor<FunctionASTVisitor> {
FunctionSizeCheck::FunctionSizeCheck(StringRef Name, ClangTidyContext *Context)
: ClangTidyCheck(Name, Context),
- LineThreshold(Options.get("LineThreshold", -1U)),
- StatementThreshold(Options.get("StatementThreshold", 800U)),
- BranchThreshold(Options.get("BranchThreshold", -1U)),
- ParameterThreshold(Options.get("ParameterThreshold", -1U)),
- NestingThreshold(Options.get("NestingThreshold", -1U)),
- VariableThreshold(Options.get("VariableThreshold", -1U)) {}
+ LineThreshold(Options.get("LineThreshold", DefaultLineThreshold)),
+ StatementThreshold(
+ Options.get("StatementThreshold", DefaultStatementThreshold)),
+ BranchThreshold(Options.get("BranchThreshold", DefaultBranchThreshold)),
+ ParameterThreshold(
+ Options.get("ParameterThreshold", DefaultParameterThreshold)),
+ NestingThreshold(
+ Options.get("NestingThreshold", DefaultNestingThreshold)),
+ VariableThreshold(
+ Options.get("VariableThreshold", DefaultVariableThreshold)) {}
void FunctionSizeCheck::storeOptions(ClangTidyOptions::OptionMap &Opts) {
Options.store(Opts, "LineThreshold", LineThreshold);
@@ -155,7 +159,7 @@ void FunctionSizeCheck::check(const MatchFinder::MatchResult &Result) {
const auto *Func = Result.Nodes.getNodeAs<FunctionDecl>("func");
FunctionASTVisitor Visitor;
- Visitor.Info.NestingThreshold = NestingThreshold;
+ Visitor.Info.NestingThreshold = NestingThreshold.value_or(-1);
Visitor.TraverseDecl(const_cast<FunctionDecl *>(Func));
auto &FI = Visitor.Info;
@@ -173,49 +177,51 @@ void FunctionSizeCheck::check(const MatchFinder::MatchResult &Result) {
unsigned ActualNumberParameters = Func->getNumParams();
- if (FI.Lines > LineThreshold || FI.Statements > StatementThreshold ||
- FI.Branches > BranchThreshold ||
- ActualNumberParameters > ParameterThreshold ||
- !FI.NestingThresholders.empty() || FI.Variables > VariableThreshold) {
+ if ((LineThreshold && FI.Lines > LineThreshold) ||
+ (StatementThreshold && FI.Statements > StatementThreshold) ||
+ (BranchThreshold && FI.Branches > BranchThreshold) ||
+ (ParameterThreshold && ActualNumberParameters > ParameterThreshold) ||
+ !FI.NestingThresholders.empty() ||
+ (VariableThreshold && FI.Variables > VariableThreshold)) {
diag(Func->getLocation(),
"function %0 exceeds recommended size/complexity thresholds")
<< Func;
}
- if (FI.Lines > LineThreshold) {
+ if (LineThreshold && FI.Lines > LineThreshold) {
diag(Func->getLocation(),
"%0 lines including whitespace and comments (threshold %1)",
DiagnosticIDs::Note)
- << FI.Lines << LineThreshold;
+ << FI.Lines << LineThreshold.value();
}
- if (FI.Statements > StatementThreshold) {
+ if (StatementThreshold && FI.Statements > StatementThreshold) {
diag(Func->getLocation(), "%0 statements (threshold %1)",
DiagnosticIDs::Note)
- << FI.Statements << StatementThreshold;
+ << FI.Statements << StatementThreshold.value();
}
- if (FI.Branches > BranchThreshold) {
+ if (BranchThreshold && FI.Branches > BranchThreshold) {
diag(Func->getLocation(), "%0 branches (threshold %1)", DiagnosticIDs::Note)
- << FI.Branches << BranchThreshold;
+ << FI.Branches << BranchThreshold.value();
}
- if (ActualNumberParameters > ParameterThreshold) {
+ if (ParameterThreshold && ActualNumberParameters > ParameterThreshold) {
diag(Func->getLocation(), "%0 parameters (threshold %1)",
DiagnosticIDs::Note)
- << ActualNumberParameters << ParameterThreshold;
+ << ActualNumberParameters << ParameterThreshold.value();
}
for (const auto &CSPos : FI.NestingThresholders) {
diag(CSPos, "nesting level %0 starts here (threshold %1)",
DiagnosticIDs::Note)
- << NestingThreshold + 1 << NestingThreshold;
+ << NestingThreshold.value() + 1 << NestingThreshold.value();
}
- if (FI.Variables > VariableThreshold) {
+ if (VariableThreshold && FI.Variables > VariableThreshold) {
diag(Func->getLocation(), "%0 variables (threshold %1)",
DiagnosticIDs::Note)
- << FI.Variables << VariableThreshold;
+ << FI.Variables << VariableThreshold.value();
}
}
diff --git a/clang-tools-extra/clang-tidy/readability/FunctionSizeCheck.h b/clang-tools-extra/clang-tidy/readability/FunctionSizeCheck.h
index 5f3fb9a35dd5410..106c69ff0739370 100644
--- a/clang-tools-extra/clang-tidy/readability/FunctionSizeCheck.h
+++ b/clang-tools-extra/clang-tidy/readability/FunctionSizeCheck.h
@@ -41,12 +41,23 @@ class FunctionSizeCheck : public ClangTidyCheck {
void check(const ast_matchers::MatchFinder::MatchResult &Result) override;
private:
- const unsigned LineThreshold;
- const unsigned StatementThreshold;
- const unsigned BranchThreshold;
- const unsigned ParameterThreshold;
- const unsigned NestingThreshold;
- const unsigned VariableThreshold;
+ const std::optional<unsigned> LineThreshold;
+ const std::optional<unsigned> StatementThreshold;
+ const std::optional<unsigned> BranchThreshold;
+ const std::optional<unsigned> ParameterThreshold;
+ const std::optional<unsigned> NestingThreshold;
+ const std::optional<unsigned> VariableThreshold;
+
+ static constexpr std::optional<unsigned> DefaultLineThreshold = std::nullopt;
+ static constexpr std::optional<unsigned> DefaultStatementThreshold = 800U;
+ static constexpr std::optional<unsigned> DefaultBranchThreshold =
+ std::nullopt;
+ static constexpr std::optional<unsigned> DefaultParameterThreshold =
+ std::nullopt;
+ static constexpr std::optional<unsigned> DefaultNestingThreshold =
+ std::nullopt;
+ static constexpr std::optional<unsigned> DefaultVariableThreshold =
+ std::nullopt;
};
} // namespace clang::tidy::readability
diff --git a/clang-tools-extra/docs/ReleaseNotes.rst b/clang-tools-extra/docs/ReleaseNotes.rst
index 60b82f4c50dd715..03e5dc6f164af2a 100644
--- a/clang-tools-extra/docs/ReleaseNotes.rst
+++ b/clang-tools-extra/docs/ReleaseNotes.rst
@@ -312,6 +312,10 @@ Changes in existing checks
detect comparison between string and empty string literals and support
``length()`` method as an alternative to ``size()``.
+- Improved :doc:`readability-function-size
+ <clang-tidy/checks/readability/function-size>` check configuration to use
+ `none` rather than `-1` to disable some parameters.
+
- Improved :doc:`readability-identifier-naming
<clang-tidy/checks/readability/identifier-naming>` check to issue accurate
warnings when a type's forward declaration precedes its definition.
@@ -334,7 +338,6 @@ Changes in existing checks
<clang-tidy/checks/readability/static-accessed-through-instance>` check to
identify calls to static member functions with out-of-class inline definitions.
-
Removed checks
^^^^^^^^^^^^^^
diff --git a/clang-tools-extra/docs/clang-tidy/checks/readability/function-size.rst b/clang-tools-extra/docs/clang-tidy/checks/readability/function-size.rst
index 3360fbd5f1e6303..133bd3e9c8cbe06 100644
--- a/clang-tools-extra/docs/clang-tidy/checks/readability/function-size.rst
+++ b/clang-tools-extra/docs/clang-tidy/checks/readability/function-size.rst
@@ -12,7 +12,7 @@ Options
.. option:: LineThreshold
- Flag functions exceeding this number of lines. The default is `-1` (ignore
+ Flag functions exceeding this number of lines. The default is `none` (ignore
the number of lines).
.. option:: StatementThreshold
@@ -24,22 +24,22 @@ Options
.. option:: BranchThreshold
Flag functions exceeding this number of control statements. The default is
- `-1` (ignore the number of branches).
+ `none` (ignore the number of branches).
.. option:: ParameterThreshold
Flag functions that exceed a specified number of parameters. The default
- is `-1` (ignore the number of parameters).
+ is `none` (ignore the number of parameters).
.. option:: NestingThreshold
Flag compound statements which create next nesting level after
`NestingThreshold`. This may
diff er significantly from the expected value
- for macro-heavy code. The default is `-1` (ignore the nesting level).
+ for macro-heavy code. The default is `none` (ignore the nesting level).
.. option:: VariableThreshold
Flag functions exceeding this number of variables declared in the body.
- The default is `-1` (ignore the number of variables).
+ The default is `none` (ignore the number of variables).
Please note that function parameters and variables declared in lambdas,
GNU Statement Expressions, and nested class inline functions are not counted.
diff --git a/clang-tools-extra/test/clang-tidy/checkers/readability/function-size.cpp b/clang-tools-extra/test/clang-tidy/checkers/readability/function-size.cpp
index b6a79465d981c6b..45b2604b43d0363 100644
--- a/clang-tools-extra/test/clang-tidy/checkers/readability/function-size.cpp
+++ b/clang-tools-extra/test/clang-tidy/checkers/readability/function-size.cpp
@@ -8,6 +8,16 @@
// RUN: readability-function-size.VariableThreshold: 1 \
// RUN: }}'
+
+// RUN: %check_clang_tidy -check-suffixes=OPTIONAL %s readability-function-size %t -- \
+// RUN: -config='{CheckOptions: { \
+// RUN: readability-function-size.StatementThreshold: "-1", \
+// RUN: readability-function-size.BranchThreshold: "5", \
+// RUN: readability-function-size.ParameterThreshold: "none", \
+// RUN: readability-function-size.NestingThreshold: "", \
+// RUN: readability-function-size.VariableThreshold: "" \
+// RUN: }}'
+
// Bad formatting is intentional, don't run clang-format over the whole file!
void foo1() {
@@ -103,9 +113,11 @@ void baz0() { // 1
// check that nested if's are not reported. this was broken initially
void nesting_if() { // 1
// CHECK-MESSAGES: :[[@LINE-1]]:6: warning: function 'nesting_if' exceeds recommended size/complexity
- // CHECK-MESSAGES: :[[@LINE-2]]:6: note: 23 lines including whitespace and comments (threshold 0)
+ // CHECK-MESSAGES: :[[@LINE-2]]:6: note: 25 lines including whitespace and comments (threshold 0)
// CHECK-MESSAGES: :[[@LINE-3]]:6: note: 18 statements (threshold 0)
// CHECK-MESSAGES: :[[@LINE-4]]:6: note: 6 branches (threshold 0)
+ // CHECK-MESSAGES-OPTIONAL: :[[@LINE-5]]:6: warning: function 'nesting_if' exceeds recommended size/complexity
+ // CHECK-MESSAGES-OPTIONAL: :[[@LINE-6]]:6: note: 6 branches (threshold 5)
if (true) { // 2
int j;
} else if (true) { // 2
@@ -123,7 +135,7 @@ void nesting_if() { // 1
} else if (true) { // 2
int j;
}
- // CHECK-MESSAGES: :[[@LINE-22]]:6: note: 6 variables (threshold 1)
+ // CHECK-MESSAGES: :[[@LINE-24]]:6: note: 6 variables (threshold 1)
}
// however this should warn
diff --git a/clang-tools-extra/test/clang-tidy/infrastructure/config-files.cpp b/clang-tools-extra/test/clang-tidy/infrastructure/config-files.cpp
index cc2dbc464e2d72a..ab4f3becb7a9fc5 100644
--- a/clang-tools-extra/test/clang-tidy/infrastructure/config-files.cpp
+++ b/clang-tools-extra/test/clang-tidy/infrastructure/config-files.cpp
@@ -55,5 +55,12 @@
// CHECK-CHILD6: Checks: {{.*-llvm-qualified-auto'? *$}}
// CHECK-CHILD6-NOT: modernize-use-using.IgnoreMacros
+// RUN: clang-tidy -dump-config \
+// RUN: --config='{CheckOptions: {readability-function-size.LineThreshold: ""}, \
+// RUN: Checks: readability-function-size}' \
+// RUN: %S/Inputs/config-files/4/44/- -- | FileCheck %s -check-prefix=CHECK-CHILD7
+// CHECK-CHILD7: readability-function-size.LineThreshold: none
+
+
// Validate that check options are printed in alphabetical order:
// RUN: clang-tidy --checks="-*,readability-identifier-naming" --dump-config %S/Inputs/config-files/- -- | grep "readability-identifier-naming\." | sort --check
More information about the cfe-commits
mailing list