Skip to content

Commit 8e21557

Browse files
authored
[clang-tidy]Add new check readability-avoid-nested-conditional-operator (#78022)
Finds nested conditional operator. Nested conditional operators lead code hard to understand, so they should be splited as several statement and stored in temporary varibale.
1 parent 3dff20c commit 8e21557

File tree

8 files changed

+140
-0
lines changed

8 files changed

+140
-0
lines changed
Lines changed: 52 additions & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -0,0 +1,52 @@
1+
//===--- AvoidNestedConditionalOperatorCheck.cpp - clang-tidy ----------===//
2+
//
3+
// Part of the LLVM Project, under the Apache License v2.0 with LLVM Exceptions.
4+
// See https://llvm.org/LICENSE.txt for license information.
5+
// SPDX-License-Identifier: Apache-2.0 WITH LLVM-exception
6+
//
7+
//===----------------------------------------------------------------------===//
8+
9+
#include "AvoidNestedConditionalOperatorCheck.h"
10+
#include "clang/ASTMatchers/ASTMatchFinder.h"
11+
#include "clang/ASTMatchers/ASTMatchers.h"
12+
#include "clang/Basic/DiagnosticIDs.h"
13+
14+
using namespace clang::ast_matchers;
15+
16+
namespace clang::tidy::readability {
17+
18+
void AvoidNestedConditionalOperatorCheck::registerMatchers(
19+
MatchFinder *Finder) {
20+
Finder->addMatcher(
21+
conditionalOperator(
22+
anyOf(
23+
hasCondition(ignoringParenCasts(
24+
conditionalOperator().bind("nested-conditional-operator"))),
25+
hasTrueExpression(ignoringParenCasts(
26+
conditionalOperator().bind("nested-conditional-operator"))),
27+
hasFalseExpression(ignoringParenCasts(
28+
conditionalOperator().bind("nested-conditional-operator")))))
29+
.bind("conditional-operator"),
30+
this);
31+
}
32+
33+
void AvoidNestedConditionalOperatorCheck::check(
34+
const MatchFinder::MatchResult &Result) {
35+
const auto *CO =
36+
Result.Nodes.getNodeAs<ConditionalOperator>("conditional-operator");
37+
const auto *NCO = Result.Nodes.getNodeAs<ConditionalOperator>(
38+
"nested-conditional-operator");
39+
assert(CO);
40+
assert(NCO);
41+
42+
if (CO->getBeginLoc().isMacroID() || NCO->getBeginLoc().isMacroID())
43+
return;
44+
45+
diag(NCO->getBeginLoc(),
46+
"conditional operator is used as sub-expression of parent conditional "
47+
"operator, refrain from using nested conditional operators");
48+
diag(CO->getBeginLoc(), "parent conditional operator here",
49+
DiagnosticIDs::Note);
50+
}
51+
52+
} // namespace clang::tidy::readability
Lines changed: 33 additions & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -0,0 +1,33 @@
1+
//===--- AvoidNestedConditionalOperatorCheck.h - clang-tidy -----*- C++ -*-===//
2+
//
3+
// Part of the LLVM Project, under the Apache License v2.0 with LLVM Exceptions.
4+
// See https://llvm.org/LICENSE.txt for license information.
5+
// SPDX-License-Identifier: Apache-2.0 WITH LLVM-exception
6+
//
7+
//===----------------------------------------------------------------------===//
8+
9+
#ifndef LLVM_CLANG_TOOLS_EXTRA_CLANG_TIDY_READABILITY_AVOID_NESTED_CONDITIONAL_OPERATOR_CHECK_H
10+
#define LLVM_CLANG_TOOLS_EXTRA_CLANG_TIDY_READABILITY_AVOID_NESTED_CONDITIONAL_OPERATOR_CHECK_H
11+
12+
#include "../ClangTidyCheck.h"
13+
14+
namespace clang::tidy::readability {
15+
16+
/// Identifies instances of nested conditional operators in the code.
17+
///
18+
/// For the user-facing documentation see:
19+
/// http://clang.llvm.org/extra/clang-tidy/checks/readability/avoid-nested-conditional-operator.html
20+
class AvoidNestedConditionalOperatorCheck : public ClangTidyCheck {
21+
public:
22+
AvoidNestedConditionalOperatorCheck(StringRef Name, ClangTidyContext *Context)
23+
: ClangTidyCheck(Name, Context) {}
24+
void registerMatchers(ast_matchers::MatchFinder *Finder) override;
25+
void check(const ast_matchers::MatchFinder::MatchResult &Result) override;
26+
std::optional<TraversalKind> getCheckTraversalKind() const override {
27+
return TK_IgnoreUnlessSpelledInSource;
28+
}
29+
};
30+
31+
} // namespace clang::tidy::readability
32+
33+
#endif // LLVM_CLANG_TOOLS_EXTRA_CLANG_TIDY_READABILITY_AVOID_NESTED_CONDITIONAL_OPERATOR_CHECK_H

clang-tools-extra/clang-tidy/readability/CMakeLists.txt

Lines changed: 1 addition & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -5,6 +5,7 @@ set(LLVM_LINK_COMPONENTS
55

66
add_clang_library(clangTidyReadabilityModule
77
AvoidConstParamsInDecls.cpp
8+
AvoidNestedConditionalOperatorCheck.cpp
89
AvoidReturnWithVoidValueCheck.cpp
910
AvoidUnconditionalPreprocessorIfCheck.cpp
1011
BracesAroundStatementsCheck.cpp

clang-tools-extra/clang-tidy/readability/ReadabilityTidyModule.cpp

Lines changed: 3 additions & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -10,6 +10,7 @@
1010
#include "../ClangTidyModule.h"
1111
#include "../ClangTidyModuleRegistry.h"
1212
#include "AvoidConstParamsInDecls.h"
13+
#include "AvoidNestedConditionalOperatorCheck.h"
1314
#include "AvoidReturnWithVoidValueCheck.h"
1415
#include "AvoidUnconditionalPreprocessorIfCheck.h"
1516
#include "BracesAroundStatementsCheck.h"
@@ -64,6 +65,8 @@ class ReadabilityModule : public ClangTidyModule {
6465
void addCheckFactories(ClangTidyCheckFactories &CheckFactories) override {
6566
CheckFactories.registerCheck<AvoidConstParamsInDecls>(
6667
"readability-avoid-const-params-in-decls");
68+
CheckFactories.registerCheck<AvoidNestedConditionalOperatorCheck>(
69+
"readability-avoid-nested-conditional-operator");
6770
CheckFactories.registerCheck<AvoidReturnWithVoidValueCheck>(
6871
"readability-avoid-return-with-void-value");
6972
CheckFactories.registerCheck<AvoidUnconditionalPreprocessorIfCheck>(

clang-tools-extra/docs/ReleaseNotes.rst

Lines changed: 5 additions & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -224,6 +224,11 @@ New checks
224224
Recommends the smallest possible underlying type for an ``enum`` or ``enum``
225225
class based on the range of its enumerators.
226226

227+
- New :doc:`readability-avoid-nested-conditional-operator
228+
<clang-tidy/checks/readability/avoid-nested-conditional-operator>` check.
229+
230+
Identifies instances of nested conditional operators in the code.
231+
227232
- New :doc:`readability-avoid-return-with-void-value
228233
<clang-tidy/checks/readability/avoid-return-with-void-value>` check.
229234

clang-tools-extra/docs/clang-tidy/checks/list.rst

Lines changed: 1 addition & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -337,6 +337,7 @@ Clang-Tidy Checks
337337
:doc:`portability-simd-intrinsics <portability/simd-intrinsics>`,
338338
:doc:`portability-std-allocator-const <portability/std-allocator-const>`,
339339
:doc:`readability-avoid-const-params-in-decls <readability/avoid-const-params-in-decls>`, "Yes"
340+
:doc:`readability-avoid-nested-conditional-operator <readability/avoid-nested-conditional-operator>`,
340341
:doc:`readability-avoid-return-with-void-value <readability/avoid-return-with-void-value>`,
341342
:doc:`readability-avoid-unconditional-preprocessor-if <readability/avoid-unconditional-preprocessor-if>`,
342343
:doc:`readability-braces-around-statements <readability/braces-around-statements>`, "Yes"
Lines changed: 21 additions & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -0,0 +1,21 @@
1+
.. title:: clang-tidy - readability-avoid-nested-conditional-operator
2+
3+
readability-avoid-nested-conditional-operator
4+
=============================================
5+
6+
Identifies instances of nested conditional operators in the code.
7+
8+
Nested conditional operators, also known as ternary operators, can contribute
9+
to reduced code readability and comprehension. So they should be split as
10+
several statements and stored the intermediate results in temporary variable.
11+
12+
Examples:
13+
14+
.. code-block:: c++
15+
16+
int NestInConditional = (condition1 ? true1 : false1) ? true2 : false2;
17+
int NestInTrue = condition1 ? (condition2 ? true1 : false1) : false2;
18+
int NestInFalse = condition1 ? true1 : condition2 ? true2 : false1;
19+
20+
This check implements part of `AUTOSAR C++14 Rule A5-16-1
21+
<https://www.autosar.org/fileadmin/standards/R22-11/AP/AUTOSAR_RS_CPP14Guidelines.pdf>`_.
Lines changed: 24 additions & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -0,0 +1,24 @@
1+
// RUN: %check_clang_tidy %s readability-avoid-nested-conditional-operator %t
2+
3+
int NestInConditional = (true ? true : false) ? 1 : 2;
4+
// CHECK-MESSAGES: :[[@LINE-1]]:26: warning: conditional operator is used as sub-expression of parent conditional operator, refrain from using nested conditional operators
5+
// CHECK-MESSAGES: :[[@LINE-2]]:25: note: parent conditional operator here
6+
7+
int NestInTrue = true ? (true ? 1 : 2) : 2;
8+
// CHECK-MESSAGES: :[[@LINE-1]]:26: warning: conditional operator is used as sub-expression of parent conditional operator, refrain from using nested conditional operators
9+
// CHECK-MESSAGES: :[[@LINE-2]]:18: note: parent conditional operator here
10+
11+
int NestInFalse = true ? 1 : true ? 1 : 2;
12+
// CHECK-MESSAGES: :[[@LINE-1]]:30: warning: conditional operator is used as sub-expression of parent conditional operator, refrain from using nested conditional operators
13+
// CHECK-MESSAGES: :[[@LINE-2]]:19: note: parent conditional operator here
14+
int NestInFalse2 = true ? 1 : (true ? 1 : 2);
15+
// CHECK-MESSAGES: :[[@LINE-1]]:32: warning: conditional operator is used as sub-expression of parent conditional operator, refrain from using nested conditional operators
16+
// CHECK-MESSAGES: :[[@LINE-2]]:20: note: parent conditional operator here
17+
18+
int NestWithParensis = true ? 1 : ((((true ? 1 : 2))));
19+
// CHECK-MESSAGES: :[[@LINE-1]]:39: warning: conditional operator is used as sub-expression of parent conditional operator, refrain from using nested conditional operators
20+
// CHECK-MESSAGES: :[[@LINE-2]]:24: note: parent conditional operator here
21+
22+
#define CONDITIONAL_EXPR (true ? 1 : 2)
23+
// not diag for macro since it will not reduce readability
24+
int NestWithMacro = true ? CONDITIONAL_EXPR : 2;

0 commit comments

Comments
 (0)