diff --git a/clang-tools-extra/clang-tidy/ClangTidyCheck.h b/clang-tools-extra/clang-tidy/ClangTidyCheck.h --- a/clang-tools-extra/clang-tidy/ClangTidyCheck.h +++ b/clang-tools-extra/clang-tidy/ClangTidyCheck.h @@ -184,8 +184,8 @@ /// 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 or empty, + /// return ``std::nullopt``. /// /// If the corresponding key can't be parsed as a ``T``, emit a /// diagnostic and return ``std::nullopt``. @@ -193,6 +193,9 @@ std::enable_if_t, std::optional> get(StringRef LocalName) const { if (std::optional Value = get(LocalName)) { + if (Value == "" || Value == "none" || Value == "null" || + (std::is_unsigned_v && Value == "-1")) + return std::nullopt; T Result{}; if (!StringRef(*Value).getAsInteger(10, Result)) return Result; @@ -238,6 +241,9 @@ return std::nullopt; } T Result{}; + if (ValueOr == "" || ValueOr == "none" || ValueOr == "null" || + (std::is_unsigned_v && ValueOr == "-1")) + return std::nullopt; if (!StringRef(*ValueOr).getAsInteger(10, Result)) return Result; diagnoseBadIntegerOption( @@ -286,8 +292,8 @@ /// 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 or empty, + /// return \p Default. /// /// If the corresponding key can't be parsed as a ``T``, emit a /// diagnostic and return \p Default. @@ -356,6 +362,19 @@ 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 + std::enable_if_t> + store(ClangTidyOptions::OptionMap &Options, StringRef LocalName, + std::optional 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.h b/clang-tools-extra/clang-tidy/readability/FunctionSizeCheck.h --- a/clang-tools-extra/clang-tidy/readability/FunctionSizeCheck.h +++ b/clang-tools-extra/clang-tidy/readability/FunctionSizeCheck.h @@ -17,21 +17,21 @@ /// /// These options are supported: /// -/// * `LineThreshold` - flag functions exceeding this number of lines. The -/// default is `-1` (ignore the number of lines). +/// * `LineThreshold` - flag functions exceeding this number of lines. This +/// parameter is disabled by default. /// * `StatementThreshold` - flag functions exceeding this number of /// statements. This may differ significantly from the number of lines for /// macro-heavy code. The default is `800`. /// * `BranchThreshold` - flag functions exceeding this number of control -/// statements. The default is `-1` (ignore the number of branches). +/// statements. This parameter is disabled by default. /// * `ParameterThreshold` - flag functions having a high number of -/// parameters. The default is `-1` (ignore the number of parameters). +/// parameters. This parameter is disabled by default. /// * `NestingThreshold` - flag compound statements which create next nesting /// level after `NestingThreshold`. This may differ significantly from the -/// expected value for macro-heavy code. The default is `-1` (ignore the -/// nesting level). +/// expected value for macro-heavy code. This parameter is disabled by +/// default. /// * `VariableThreshold` - flag functions having a high number of variable -/// declarations. The default is `-1` (ignore the number of variables). +/// declarations. This parameter is disabled by default. class FunctionSizeCheck : public ClangTidyCheck { public: FunctionSizeCheck(StringRef Name, ClangTidyContext *Context); @@ -41,12 +41,12 @@ 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 LineThreshold; + const std::optional StatementThreshold; + const std::optional BranchThreshold; + const std::optional ParameterThreshold; + const std::optional NestingThreshold; + const std::optional VariableThreshold; }; } // namespace clang::tidy::readability diff --git a/clang-tools-extra/clang-tidy/readability/FunctionSizeCheck.cpp b/clang-tools-extra/clang-tidy/readability/FunctionSizeCheck.cpp --- a/clang-tools-extra/clang-tidy/readability/FunctionSizeCheck.cpp +++ b/clang-tools-extra/clang-tidy/readability/FunctionSizeCheck.cpp @@ -126,12 +126,12 @@ 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")), + StatementThreshold(Options.get("StatementThreshold", 800U)), + BranchThreshold(Options.get("BranchThreshold")), + ParameterThreshold(Options.get("ParameterThreshold")), + NestingThreshold(Options.get("NestingThreshold")), + VariableThreshold(Options.get("VariableThreshold")) {} void FunctionSizeCheck::storeOptions(ClangTidyOptions::OptionMap &Opts) { Options.store(Opts, "LineThreshold", LineThreshold); @@ -155,7 +155,7 @@ const auto *Func = Result.Nodes.getNodeAs("func"); FunctionASTVisitor Visitor; - Visitor.Info.NestingThreshold = NestingThreshold; + Visitor.Info.NestingThreshold = NestingThreshold.value_or(-1); Visitor.TraverseDecl(const_cast(Func)); auto &FI = Visitor.Info; @@ -173,49 +173,51 @@ 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/docs/ReleaseNotes.rst b/clang-tools-extra/docs/ReleaseNotes.rst --- a/clang-tools-extra/docs/ReleaseNotes.rst +++ b/clang-tools-extra/docs/ReleaseNotes.rst @@ -113,6 +113,11 @@ - Improved `--dump-config` to print check options in alphabetical order. +- Added support for optional parameters. Parameters that previously used -1 to disable + their effect can now be set to `none`, `null`, or left empty to get the same + behaviour. It the parameter does not support optional values, the default value + will be used instead. + New 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 --- 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,8 +12,8 @@ .. option:: LineThreshold - Flag functions exceeding this number of lines. The default is `-1` (ignore - the number of lines). + Flag functions exceeding this number of lines. This parameter is disabled + by default. .. option:: StatementThreshold @@ -23,23 +23,23 @@ .. option:: BranchThreshold - Flag functions exceeding this number of control statements. The default is - `-1` (ignore the number of branches). + Flag functions exceeding this number of control statements. This parameter + is disabled by default. .. option:: ParameterThreshold - Flag functions that exceed a specified number of parameters. The default - is `-1` (ignore the number of parameters). + Flag functions that exceed a specified number of parameters. This parameter + is disabled by default. .. option:: NestingThreshold Flag compound statements which create next nesting level after `NestingThreshold`. This may differ significantly from the expected value - for macro-heavy code. The default is `-1` (ignore the nesting level). + for macro-heavy code. This parameter is disabled by default. .. option:: VariableThreshold Flag functions exceeding this number of variables declared in the body. - The default is `-1` (ignore the number of variables). + This parameter is disabled by default. 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/infrastructure/config-files.cpp b/clang-tools-extra/test/clang-tidy/infrastructure/config-files.cpp --- 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 diff --git a/clang-tools-extra/test/clang-tidy/infrastructure/optional-parameter.cpp b/clang-tools-extra/test/clang-tidy/infrastructure/optional-parameter.cpp new file mode 100644 --- /dev/null +++ b/clang-tools-extra/test/clang-tidy/infrastructure/optional-parameter.cpp @@ -0,0 +1,19 @@ +// RUN: %check_clang_tidy %s bugprone-easily-swappable-parameters %t \ +// RUN: -config='{CheckOptions: { \ +// RUN: bugprone-easily-swappable-parameters.MinimumLength: "", \ +// RUN: }}' -- + +// RUN: %check_clang_tidy %s bugprone-easily-swappable-parameters %t \ +// RUN: -config='{CheckOptions: { \ +// RUN: bugprone-easily-swappable-parameters.MinimumLength: "none", \ +// RUN: }}' -- + +// RUN: %check_clang_tidy %s bugprone-easily-swappable-parameters %t \ +// RUN: -config='{CheckOptions: { \ +// RUN: bugprone-easily-swappable-parameters.MinimumLength: "null", \ +// RUN: }}' -- + +void a(int b, int c) {} +// CHECK-MESSAGES: :[[@LINE-1]]:8: warning: 2 adjacent parameters of 'a' of similar type ('int') are easily swapped by mistake [bugprone-easily-swappable-parameters] +// CHECK-MESSAGES: :[[@LINE-2]]:12: note: the first parameter in the range is 'b' +// CHECK-MESSAGES: :[[@LINE-3]]:19: note: the last parameter in the range is 'c'