[clang-tools-extra] 8e21557 - [clang-tidy]Add new check readability-avoid-nested-conditional-operator (#78022)
via cfe-commits
cfe-commits at lists.llvm.org
Mon Jan 15 02:19:51 PST 2024
Author: Congcong Cai
Date: 2024-01-15T18:19:47+08:00
New Revision: 8e21557d0401a0046ff110daa50f21d02b71a2ee
URL: https://github.com/llvm/llvm-project/commit/8e21557d0401a0046ff110daa50f21d02b71a2ee
DIFF: https://github.com/llvm/llvm-project/commit/8e21557d0401a0046ff110daa50f21d02b71a2ee.diff
LOG: [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.
Added:
clang-tools-extra/clang-tidy/readability/AvoidNestedConditionalOperatorCheck.cpp
clang-tools-extra/clang-tidy/readability/AvoidNestedConditionalOperatorCheck.h
clang-tools-extra/docs/clang-tidy/checks/readability/avoid-nested-conditional-operator.rst
clang-tools-extra/test/clang-tidy/checkers/readability/avoid-nested-conditional-operator.cpp
Modified:
clang-tools-extra/clang-tidy/readability/CMakeLists.txt
clang-tools-extra/clang-tidy/readability/ReadabilityTidyModule.cpp
clang-tools-extra/docs/ReleaseNotes.rst
clang-tools-extra/docs/clang-tidy/checks/list.rst
Removed:
################################################################################
diff --git a/clang-tools-extra/clang-tidy/readability/AvoidNestedConditionalOperatorCheck.cpp b/clang-tools-extra/clang-tidy/readability/AvoidNestedConditionalOperatorCheck.cpp
new file mode 100644
index 00000000000000..2ea03c9070526f
--- /dev/null
+++ b/clang-tools-extra/clang-tidy/readability/AvoidNestedConditionalOperatorCheck.cpp
@@ -0,0 +1,52 @@
+//===--- AvoidNestedConditionalOperatorCheck.cpp - clang-tidy ----------===//
+//
+// Part of the LLVM Project, under the Apache License v2.0 with LLVM Exceptions.
+// See https://llvm.org/LICENSE.txt for license information.
+// SPDX-License-Identifier: Apache-2.0 WITH LLVM-exception
+//
+//===----------------------------------------------------------------------===//
+
+#include "AvoidNestedConditionalOperatorCheck.h"
+#include "clang/ASTMatchers/ASTMatchFinder.h"
+#include "clang/ASTMatchers/ASTMatchers.h"
+#include "clang/Basic/DiagnosticIDs.h"
+
+using namespace clang::ast_matchers;
+
+namespace clang::tidy::readability {
+
+void AvoidNestedConditionalOperatorCheck::registerMatchers(
+ MatchFinder *Finder) {
+ Finder->addMatcher(
+ conditionalOperator(
+ anyOf(
+ hasCondition(ignoringParenCasts(
+ conditionalOperator().bind("nested-conditional-operator"))),
+ hasTrueExpression(ignoringParenCasts(
+ conditionalOperator().bind("nested-conditional-operator"))),
+ hasFalseExpression(ignoringParenCasts(
+ conditionalOperator().bind("nested-conditional-operator")))))
+ .bind("conditional-operator"),
+ this);
+}
+
+void AvoidNestedConditionalOperatorCheck::check(
+ const MatchFinder::MatchResult &Result) {
+ const auto *CO =
+ Result.Nodes.getNodeAs<ConditionalOperator>("conditional-operator");
+ const auto *NCO = Result.Nodes.getNodeAs<ConditionalOperator>(
+ "nested-conditional-operator");
+ assert(CO);
+ assert(NCO);
+
+ if (CO->getBeginLoc().isMacroID() || NCO->getBeginLoc().isMacroID())
+ return;
+
+ diag(NCO->getBeginLoc(),
+ "conditional operator is used as sub-expression of parent conditional "
+ "operator, refrain from using nested conditional operators");
+ diag(CO->getBeginLoc(), "parent conditional operator here",
+ DiagnosticIDs::Note);
+}
+
+} // namespace clang::tidy::readability
diff --git a/clang-tools-extra/clang-tidy/readability/AvoidNestedConditionalOperatorCheck.h b/clang-tools-extra/clang-tidy/readability/AvoidNestedConditionalOperatorCheck.h
new file mode 100644
index 00000000000000..9010156de6ce2d
--- /dev/null
+++ b/clang-tools-extra/clang-tidy/readability/AvoidNestedConditionalOperatorCheck.h
@@ -0,0 +1,33 @@
+//===--- AvoidNestedConditionalOperatorCheck.h - clang-tidy -----*- C++ -*-===//
+//
+// Part of the LLVM Project, under the Apache License v2.0 with LLVM Exceptions.
+// See https://llvm.org/LICENSE.txt for license information.
+// SPDX-License-Identifier: Apache-2.0 WITH LLVM-exception
+//
+//===----------------------------------------------------------------------===//
+
+#ifndef LLVM_CLANG_TOOLS_EXTRA_CLANG_TIDY_READABILITY_AVOID_NESTED_CONDITIONAL_OPERATOR_CHECK_H
+#define LLVM_CLANG_TOOLS_EXTRA_CLANG_TIDY_READABILITY_AVOID_NESTED_CONDITIONAL_OPERATOR_CHECK_H
+
+#include "../ClangTidyCheck.h"
+
+namespace clang::tidy::readability {
+
+/// Identifies instances of nested conditional operators in the code.
+///
+/// For the user-facing documentation see:
+/// http://clang.llvm.org/extra/clang-tidy/checks/readability/avoid-nested-conditional-operator.html
+class AvoidNestedConditionalOperatorCheck : public ClangTidyCheck {
+public:
+ AvoidNestedConditionalOperatorCheck(StringRef Name, ClangTidyContext *Context)
+ : ClangTidyCheck(Name, Context) {}
+ void registerMatchers(ast_matchers::MatchFinder *Finder) override;
+ void check(const ast_matchers::MatchFinder::MatchResult &Result) override;
+ std::optional<TraversalKind> getCheckTraversalKind() const override {
+ return TK_IgnoreUnlessSpelledInSource;
+ }
+};
+
+} // namespace clang::tidy::readability
+
+#endif // LLVM_CLANG_TOOLS_EXTRA_CLANG_TIDY_READABILITY_AVOID_NESTED_CONDITIONAL_OPERATOR_CHECK_H
diff --git a/clang-tools-extra/clang-tidy/readability/CMakeLists.txt b/clang-tools-extra/clang-tidy/readability/CMakeLists.txt
index 408c822b861c5f..fa571d5dd7650d 100644
--- a/clang-tools-extra/clang-tidy/readability/CMakeLists.txt
+++ b/clang-tools-extra/clang-tidy/readability/CMakeLists.txt
@@ -5,6 +5,7 @@ set(LLVM_LINK_COMPONENTS
add_clang_library(clangTidyReadabilityModule
AvoidConstParamsInDecls.cpp
+ AvoidNestedConditionalOperatorCheck.cpp
AvoidReturnWithVoidValueCheck.cpp
AvoidUnconditionalPreprocessorIfCheck.cpp
BracesAroundStatementsCheck.cpp
diff --git a/clang-tools-extra/clang-tidy/readability/ReadabilityTidyModule.cpp b/clang-tools-extra/clang-tidy/readability/ReadabilityTidyModule.cpp
index 0b0aad7c0dcb36..f769752c5de5fa 100644
--- a/clang-tools-extra/clang-tidy/readability/ReadabilityTidyModule.cpp
+++ b/clang-tools-extra/clang-tidy/readability/ReadabilityTidyModule.cpp
@@ -10,6 +10,7 @@
#include "../ClangTidyModule.h"
#include "../ClangTidyModuleRegistry.h"
#include "AvoidConstParamsInDecls.h"
+#include "AvoidNestedConditionalOperatorCheck.h"
#include "AvoidReturnWithVoidValueCheck.h"
#include "AvoidUnconditionalPreprocessorIfCheck.h"
#include "BracesAroundStatementsCheck.h"
@@ -64,6 +65,8 @@ class ReadabilityModule : public ClangTidyModule {
void addCheckFactories(ClangTidyCheckFactories &CheckFactories) override {
CheckFactories.registerCheck<AvoidConstParamsInDecls>(
"readability-avoid-const-params-in-decls");
+ CheckFactories.registerCheck<AvoidNestedConditionalOperatorCheck>(
+ "readability-avoid-nested-conditional-operator");
CheckFactories.registerCheck<AvoidReturnWithVoidValueCheck>(
"readability-avoid-return-with-void-value");
CheckFactories.registerCheck<AvoidUnconditionalPreprocessorIfCheck>(
diff --git a/clang-tools-extra/docs/ReleaseNotes.rst b/clang-tools-extra/docs/ReleaseNotes.rst
index 344a7ed5798835..a235a7d02592e8 100644
--- a/clang-tools-extra/docs/ReleaseNotes.rst
+++ b/clang-tools-extra/docs/ReleaseNotes.rst
@@ -224,6 +224,11 @@ New checks
Recommends the smallest possible underlying type for an ``enum`` or ``enum``
class based on the range of its enumerators.
+- New :doc:`readability-avoid-nested-conditional-operator
+ <clang-tidy/checks/readability/avoid-nested-conditional-operator>` check.
+
+ Identifies instances of nested conditional operators in the code.
+
- New :doc:`readability-avoid-return-with-void-value
<clang-tidy/checks/readability/avoid-return-with-void-value>` check.
diff --git a/clang-tools-extra/docs/clang-tidy/checks/list.rst b/clang-tools-extra/docs/clang-tidy/checks/list.rst
index 2f86121ad87299..5f21449cfc3df4 100644
--- a/clang-tools-extra/docs/clang-tidy/checks/list.rst
+++ b/clang-tools-extra/docs/clang-tidy/checks/list.rst
@@ -337,6 +337,7 @@ Clang-Tidy Checks
:doc:`portability-simd-intrinsics <portability/simd-intrinsics>`,
:doc:`portability-std-allocator-const <portability/std-allocator-const>`,
:doc:`readability-avoid-const-params-in-decls <readability/avoid-const-params-in-decls>`, "Yes"
+ :doc:`readability-avoid-nested-conditional-operator <readability/avoid-nested-conditional-operator>`,
:doc:`readability-avoid-return-with-void-value <readability/avoid-return-with-void-value>`,
:doc:`readability-avoid-unconditional-preprocessor-if <readability/avoid-unconditional-preprocessor-if>`,
:doc:`readability-braces-around-statements <readability/braces-around-statements>`, "Yes"
diff --git a/clang-tools-extra/docs/clang-tidy/checks/readability/avoid-nested-conditional-operator.rst b/clang-tools-extra/docs/clang-tidy/checks/readability/avoid-nested-conditional-operator.rst
new file mode 100644
index 00000000000000..44b74283292ce0
--- /dev/null
+++ b/clang-tools-extra/docs/clang-tidy/checks/readability/avoid-nested-conditional-operator.rst
@@ -0,0 +1,21 @@
+.. title:: clang-tidy - readability-avoid-nested-conditional-operator
+
+readability-avoid-nested-conditional-operator
+=============================================
+
+Identifies instances of nested conditional operators in the code.
+
+Nested conditional operators, also known as ternary operators, can contribute
+to reduced code readability and comprehension. So they should be split as
+several statements and stored the intermediate results in temporary variable.
+
+Examples:
+
+.. code-block:: c++
+
+ int NestInConditional = (condition1 ? true1 : false1) ? true2 : false2;
+ int NestInTrue = condition1 ? (condition2 ? true1 : false1) : false2;
+ int NestInFalse = condition1 ? true1 : condition2 ? true2 : false1;
+
+This check implements part of `AUTOSAR C++14 Rule A5-16-1
+<https://www.autosar.org/fileadmin/standards/R22-11/AP/AUTOSAR_RS_CPP14Guidelines.pdf>`_.
diff --git a/clang-tools-extra/test/clang-tidy/checkers/readability/avoid-nested-conditional-operator.cpp b/clang-tools-extra/test/clang-tidy/checkers/readability/avoid-nested-conditional-operator.cpp
new file mode 100644
index 00000000000000..847df08199e349
--- /dev/null
+++ b/clang-tools-extra/test/clang-tidy/checkers/readability/avoid-nested-conditional-operator.cpp
@@ -0,0 +1,24 @@
+// RUN: %check_clang_tidy %s readability-avoid-nested-conditional-operator %t
+
+int NestInConditional = (true ? true : false) ? 1 : 2;
+// CHECK-MESSAGES: :[[@LINE-1]]:26: warning: conditional operator is used as sub-expression of parent conditional operator, refrain from using nested conditional operators
+// CHECK-MESSAGES: :[[@LINE-2]]:25: note: parent conditional operator here
+
+int NestInTrue = true ? (true ? 1 : 2) : 2;
+// CHECK-MESSAGES: :[[@LINE-1]]:26: warning: conditional operator is used as sub-expression of parent conditional operator, refrain from using nested conditional operators
+// CHECK-MESSAGES: :[[@LINE-2]]:18: note: parent conditional operator here
+
+int NestInFalse = true ? 1 : true ? 1 : 2;
+// CHECK-MESSAGES: :[[@LINE-1]]:30: warning: conditional operator is used as sub-expression of parent conditional operator, refrain from using nested conditional operators
+// CHECK-MESSAGES: :[[@LINE-2]]:19: note: parent conditional operator here
+int NestInFalse2 = true ? 1 : (true ? 1 : 2);
+// CHECK-MESSAGES: :[[@LINE-1]]:32: warning: conditional operator is used as sub-expression of parent conditional operator, refrain from using nested conditional operators
+// CHECK-MESSAGES: :[[@LINE-2]]:20: note: parent conditional operator here
+
+int NestWithParensis = true ? 1 : ((((true ? 1 : 2))));
+// CHECK-MESSAGES: :[[@LINE-1]]:39: warning: conditional operator is used as sub-expression of parent conditional operator, refrain from using nested conditional operators
+// CHECK-MESSAGES: :[[@LINE-2]]:24: note: parent conditional operator here
+
+#define CONDITIONAL_EXPR (true ? 1 : 2)
+// not diag for macro since it will not reduce readability
+int NestWithMacro = true ? CONDITIONAL_EXPR : 2;
More information about the cfe-commits
mailing list