jbcoe created this revision.
jbcoe added a project: clang-tools-extra.
Herald added subscribers: JDevlieghere, mgorny.
Add a check to find enums used in `==` or `!=` expressions. Using a switch
statement leads to more easily maintained code.
Repository:
rL LLVM
https://reviews.llvm.org/D30896
Files:
clang-tools-extra/clang-tidy/misc/CMakeLists.txt
clang-tools-extra/clang-tidy/misc/MiscTidyModule.cpp
clang-tools-extra/clang-tidy/misc/PreferSwitchForEnumsCheck.cpp
clang-tools-extra/clang-tidy/misc/PreferSwitchForEnumsCheck.h
clang-tools-extra/docs/clang-tidy/checks/list.rst
clang-tools-extra/docs/clang-tidy/checks/misc-prefer-switch-for-enums.rst
clang-tools-extra/test/clang-tidy/misc-prefer-switch-for-enums.cpp
Index: clang-tools-extra/test/clang-tidy/misc-prefer-switch-for-enums.cpp
===================================================================
--- /dev/null
+++ clang-tools-extra/test/clang-tidy/misc-prefer-switch-for-enums.cpp
@@ -0,0 +1,40 @@
+// RUN: %check_clang_tidy %s misc-prefer-switch-for-enums %t
+
+enum class kind { a, b, c, d };
+
+int foo(kind k)
+{
+ if (k == kind::a)
+ // CHECK-MESSAGES: :[[@LINE-1]]:9: warning: prefer a switch statement for enum-value checks [misc-prefer-switch-for-enums]
+ return -1;
+ return 1;
+}
+
+int antifoo(kind k)
+{
+ if (k != kind::a)
+ // CHECK-MESSAGES: :[[@LINE-1]]:9: warning: prefer a switch statement for enum-value checks [misc-prefer-switch-for-enums]
+ return 1;
+ return -1;
+}
+
+int bar(int i)
+{
+ if (i == 0)
+ return -1;
+ return 1;
+}
+
+int foobar(kind k)
+{
+ switch(k)
+ {
+ case kind::a:
+ return -1;
+ case kind::b:
+ case kind::c:
+ case kind::d:
+ return 1;
+ }
+}
+
Index: clang-tools-extra/docs/clang-tidy/checks/misc-prefer-switch-for-enums.rst
===================================================================
--- /dev/null
+++ clang-tools-extra/docs/clang-tidy/checks/misc-prefer-switch-for-enums.rst
@@ -0,0 +1,40 @@
+.. title:: clang-tidy - misc-prefer-switch-for-enums
+
+misc-prefer-switch-for-enums
+============================
+
+This check finds enum values used in ``==`` and ``!=`` expressions.
+
+A switch statement will robustly identify unhandled enum cases with a compiler
+warning. No such warning exists for control flow based on enum value
+comparison. Use of switches rather than if statements leads to easier to
+maintain code as adding values to an enum will trigger compiler warnings for
+unhandled cases.
+
+
+.. code-block:: c++
+
+ enum class kind { a, b, c};
+
+ int foo(kind k) {
+ return k == kind::a
+ ? 1
+ : -1;
+ }
+
+is more maintainably (but more verbosely) written as:
+
+.. code-block:: c++
+
+ enum class kind { a, b, c};
+
+ int foo(kind k) {
+ switch(k) {
+ case kind::a:
+ return 1;
+ case kind::b:
+ case kind::c:
+ return -1;
+ }
+ }
+
Index: clang-tools-extra/docs/clang-tidy/checks/list.rst
===================================================================
--- clang-tools-extra/docs/clang-tidy/checks/list.rst
+++ clang-tools-extra/docs/clang-tidy/checks/list.rst
@@ -78,6 +78,7 @@
misc-new-delete-overloads
misc-noexcept-move-constructor
misc-non-copyable-objects
+ misc-prefer-switch-for-enums
misc-redundant-expression
misc-sizeof-container
misc-sizeof-expression
Index: clang-tools-extra/clang-tidy/misc/PreferSwitchForEnumsCheck.h
===================================================================
--- /dev/null
+++ clang-tools-extra/clang-tidy/misc/PreferSwitchForEnumsCheck.h
@@ -0,0 +1,36 @@
+//===--- PreferSwitchForEnumsCheck.h - clang-tidy----------------*- C++ -*-===//
+//
+// The LLVM Compiler Infrastructure
+//
+// This file is distributed under the University of Illinois Open Source
+// License. See LICENSE.TXT for details.
+//
+//===----------------------------------------------------------------------===//
+
+#ifndef LLVM_CLANG_TOOLS_EXTRA_CLANG_TIDY_MISC_PREFER_SWITCH_FOR_ENUMS_H
+#define LLVM_CLANG_TOOLS_EXTRA_CLANG_TIDY_MISC_PREFER_SWITCH_FOR_ENUMS_H
+
+#include "../ClangTidy.h"
+
+namespace clang {
+namespace tidy {
+namespace misc {
+
+/// Find places where an enumeration value is used in `==` or `!=`.
+/// Using `switch` leads to more maintainable code.
+///
+/// For the user-facing documentation see:
+/// http://clang.llvm.org/extra/clang-tidy/checks/misc-prefer-switch-for-enums.html
+class PreferSwitchForEnumsCheck : public ClangTidyCheck {
+public:
+ PreferSwitchForEnumsCheck(StringRef Name, ClangTidyContext *Context)
+ : ClangTidyCheck(Name, Context) {}
+ void registerMatchers(ast_matchers::MatchFinder *Finder) override;
+ void check(const ast_matchers::MatchFinder::MatchResult &Result) override;
+};
+
+} // namespace misc
+} // namespace tidy
+} // namespace clang
+
+#endif // LLVM_CLANG_TOOLS_EXTRA_CLANG_TIDY_MISC_PREFER_SWITCH_FOR_ENUMS_H
Index: clang-tools-extra/clang-tidy/misc/PreferSwitchForEnumsCheck.cpp
===================================================================
--- /dev/null
+++ clang-tools-extra/clang-tidy/misc/PreferSwitchForEnumsCheck.cpp
@@ -0,0 +1,38 @@
+//===--- PreferSwitchForEnumsCheck.cpp - clang-tidy------------------------===//
+//
+// The LLVM Compiler Infrastructure
+//
+// This file is distributed under the University of Illinois Open Source
+// License. See LICENSE.TXT for details.
+//
+//===----------------------------------------------------------------------===//
+
+#include "PreferSwitchForEnumsCheck.h"
+#include "clang/AST/ASTContext.h"
+#include "clang/ASTMatchers/ASTMatchFinder.h"
+
+using namespace clang::ast_matchers;
+
+namespace clang {
+namespace tidy {
+namespace misc {
+
+void PreferSwitchForEnumsCheck::registerMatchers(MatchFinder *Finder) {
+ // FIXME: Add matchers.
+ Finder->addMatcher(
+ expr(binaryOperator(
+ anyOf(hasOperatorName("=="), hasOperatorName("!=")),
+ anyOf(hasRHS(declRefExpr(hasDeclaration(enumConstantDecl()))),
+ hasLHS(declRefExpr(hasDeclaration(enumConstantDecl()))))))
+ .bind("x"),
+ this);
+}
+
+void PreferSwitchForEnumsCheck::check(const MatchFinder::MatchResult &Result) {
+ const auto *MatchedDecl = Result.Nodes.getNodeAs<Expr>("x");
+ diag(MatchedDecl->getExprLoc(), "prefer a switch statement for enum-value checks");
+}
+
+} // namespace misc
+} // namespace tidy
+} // namespace clang
Index: clang-tools-extra/clang-tidy/misc/MiscTidyModule.cpp
===================================================================
--- clang-tools-extra/clang-tidy/misc/MiscTidyModule.cpp
+++ clang-tools-extra/clang-tidy/misc/MiscTidyModule.cpp
@@ -31,6 +31,7 @@
#include "NewDeleteOverloadsCheck.h"
#include "NoexceptMoveConstructorCheck.h"
#include "NonCopyableObjects.h"
+#include "PreferSwitchForEnumsCheck.h"
#include "RedundantExpressionCheck.h"
#include "SizeofContainerCheck.h"
#include "SizeofExpressionCheck.h"
@@ -66,6 +67,8 @@
CheckFactories.registerCheck<AssertSideEffectCheck>(
"misc-assert-side-effect");
CheckFactories.registerCheck<MisplacedConstCheck>("misc-misplaced-const");
+ CheckFactories.registerCheck<PreferSwitchForEnumsCheck>(
+ "misc-prefer-switch-for-enums");
CheckFactories.registerCheck<UnconventionalAssignOperatorCheck>(
"misc-unconventional-assign-operator");
CheckFactories.registerCheck<BoolPointerImplicitConversionCheck>(
Index: clang-tools-extra/clang-tidy/misc/CMakeLists.txt
===================================================================
--- clang-tools-extra/clang-tidy/misc/CMakeLists.txt
+++ clang-tools-extra/clang-tidy/misc/CMakeLists.txt
@@ -4,6 +4,7 @@
ArgumentCommentCheck.cpp
AssertSideEffectCheck.cpp
MisplacedConstCheck.cpp
+ PreferSwitchForEnumsCheck.cpp
UnconventionalAssignOperatorCheck.cpp
BoolPointerImplicitConversionCheck.cpp
DanglingHandleCheck.cpp
_______________________________________________
cfe-commits mailing list
cfe-commits@lists.llvm.org
http://lists.llvm.org/cgi-bin/mailman/listinfo/cfe-commits
