Added a 'RefactorConditionVariables' option to control how the check handles condition variables
Reviewed By: aaron.ballman
Differential Revision: https://reviews.llvm.org/D82824
static const char ThrowStr[] = "throw";
static const char WarningMessage[] = "do not use 'else' after '%0'";
static const char WarnOnUnfixableStr[] = "WarnOnUnfixable";
+static const char WarnOnConditionVariablesStr[] = "WarnOnConditionVariables";
const DeclRefExpr *findUsage(const Stmt *Node, int64_t DeclIdentifier) {
if (!Node)
ElseAfterReturnCheck::ElseAfterReturnCheck(StringRef Name,
ClangTidyContext *Context)
: ClangTidyCheck(Name, Context),
- WarnOnUnfixable(Options.get(WarnOnUnfixableStr, true)) {}
+ WarnOnUnfixable(Options.get(WarnOnUnfixableStr, true)),
+ WarnOnConditionVariables(Options.get(WarnOnConditionVariablesStr, true)) {
+}
void ElseAfterReturnCheck::storeOptions(ClangTidyOptions::OptionMap &Opts) {
Options.store(Opts, WarnOnUnfixableStr, WarnOnUnfixable);
+ Options.store(Opts, WarnOnConditionVariablesStr, WarnOnConditionVariables);
}
void ElseAfterReturnCheck::registerMatchers(MatchFinder *Finder) {
}
if (checkConditionVarUsageInElse(If) != nullptr) {
+ if (!WarnOnConditionVariables)
+ return;
if (IsLastInScope) {
// If the if statement is the last statement its enclosing statements
// scope, we can pull the decl out of the if statement.
}
if (checkInitDeclUsageInElse(If) != nullptr) {
+ if (!WarnOnConditionVariables)
+ return;
if (IsLastInScope) {
// If the if statement is the last statement its enclosing statements
// scope, we can pull the decl out of the if statement.
private:
const bool WarnOnUnfixable;
+ const bool WarnOnConditionVariables;
};
} // namespace readability
Now checks ``std::basic_string_view`` by default.
+- Improved :doc:`readability-else-after-return
+ <clang-tidy/checks/readability-else-after-return>` check now supports a
+ `WarnOnConditionVariables` option to control whether to refactor condition
+ variables where possible.
+
- Improved :doc:`readability-identifier-naming
<clang-tidy/checks/readability-identifier-naming>` check.
}
}
+Options
+-------
+
+.. option:: WarnOnUnfixable
+
+ When `true`, emit a warning for cases where the check can't output a
+ Fix-It. These can occur with declarations inside the ``else`` branch that
+ would have an extended lifetime if the ``else`` branch was removed.
+ Default value is `true`.
+
+.. option:: WarnOnConditionVariables
+
+ When `true`, the check will attempt to refactor a variable defined inside
+ the condition of the ``if`` statement that is used in the ``else`` branch
+ defining them just before the ``if`` statement. This can only be done if
+ the ``if`` statement is the last statement in its parents scope.
+ Default value is `true`.
-This check helps to enforce this `LLVM Coding Standards recommendation
-<https://llvm.org/docs/CodingStandards.html#don-t-use-else-after-a-return>`_.
--- /dev/null
+// RUN: %check_clang_tidy %s readability-else-after-return %t -- \
+// RUN: -config='{CheckOptions: [ \
+// RUN: {key: readability-else-after-return.WarnOnConditionVariables, value: false}, \
+// RUN: ]}'
+
+bool foo(int Y) {
+ // Excess scopes are here so that the check would have to opportunity to
+ // refactor the variable out of the condition.
+
+ // Expect warnings here as we don't need to move declaration of 'X' out of the
+ // if condition as its not used in the else.
+ {
+ if (int X = Y)
+ return X < 0;
+ else
+ // CHECK-MESSAGES: :[[@LINE-1]]:5: warning: do not use 'else' after 'return'
+ return false;
+ }
+ {
+ if (int X = Y; X)
+ return X < 0;
+ else
+ // CHECK-MESSAGES: :[[@LINE-1]]:5: warning: do not use 'else' after 'return'
+ return false;
+ }
+
+ // Expect no warnings for these cases, as even though its safe to move
+ // declaration of 'X' out of the if condition, that has been disabled
+ // by the options.
+ {
+ if (int X = Y)
+ return false;
+ else
+ return X < 0;
+ }
+ {
+ if (int X = Y; X)
+ return false;
+ else
+ return X < 0;
+ }
+}