[clang-tidy] introducing cppcoreguidelines-enum-all-caps check

[omtcyf0]

Summary

This patch introduces cppcoreguidelines-enum-all-caps, which flags all ALL_CAPS enumerations.

As written in C++ Core Guidelines: enumerations should not be named using ALL_CAPS to avoid clashes with macros.

This rule is part of the "Enumerations" profile of the C++ Core Guidelines, see
https://github.com/isocpp/CppCoreGuidelines/blob/master/CppCoreGuidelines.md#-enum5-dont-use-all_caps-for-enumerators

[alexfh]

There's already the readability-identifier-naming check that is highly configurable. It might miss the "use any naming style except for this one" functionality, but I'd prefer it to be added to that check rather than implementing another check for naming styles. After the required functionality is added to the readability-identifier-naming check, we can just register it in the cppcoreguidelines module with a custom name (corresponding to the rule) and a custom configuration that only enables verification of enumerator names (see GoogleModule::getModuleOptions() for an example of how to do this).

[omtcyf0]

@alexfh, Thanks for the feedback!

What you said seems reasonable to me, I'll follow your advice and come up with the proposed solution!

clang-tidy/cppcoreguidelines/CMakeLists.txt

 set(LLVM_LINK_COMPONENTS support)
 
 add_clang_library(clangTidyCppCoreGuidelinesModule
   CppCoreGuidelinesTidyModule.cpp
+  EnumAllCapsCheck.cpp
   ProBoundsArrayToPointerDecayCheck.cpp
   ProBoundsConstantArrayIndexCheck.cpp
   ProBoundsPointerArithmeticCheck.cpp
   ProTypeConstCastCheck.cpp
   ProTypeCstyleCastCheck.cpp
   ProTypeReinterpretCastCheck.cpp
   ProTypeStaticCastDowncastCheck.cpp
   ProTypeUnionAccessCheck.cpp

clang-tidy/cppcoreguidelines/CppCoreGuidelinesTidyModule.cpp

 //===--- CppCoreGuidelinesModule.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 "../ClangTidy.h"
 #include "../ClangTidyModule.h"
 #include "../ClangTidyModuleRegistry.h"
 #include "../misc/AssignOperatorSignatureCheck.h"
+#include "EnumAllCapsCheck.h"
 #include "ProBoundsArrayToPointerDecayCheck.h"
 #include "ProBoundsConstantArrayIndexCheck.h"
 #include "ProBoundsPointerArithmeticCheck.h"
 #include "ProTypeConstCastCheck.h"
 #include "ProTypeCstyleCastCheck.h"
 #include "ProTypeReinterpretCastCheck.h"
 #include "ProTypeStaticCastDowncastCheck.h"
 #include "ProTypeUnionAccessCheck.h"
 #include "ProTypeVarargCheck.h"
 
 namespace clang {
 namespace tidy {
 namespace cppcoreguidelines {
 
 /// A module containing checks of the C++ Core Guidelines
 class CppCoreGuidelinesModule : public ClangTidyModule {
 public:
   void addCheckFactories(ClangTidyCheckFactories &CheckFactories) override {
+    CheckFactories.registerCheck<EnumAllCapsCheck>(
+        "cppcoreguidelines-enum-all-caps");
     CheckFactories.registerCheck<ProBoundsArrayToPointerDecayCheck>(
         "cppcoreguidelines-pro-bounds-array-to-pointer-decay");
     CheckFactories.registerCheck<ProBoundsConstantArrayIndexCheck>(
         "cppcoreguidelines-pro-bounds-constant-array-index");
     CheckFactories.registerCheck<ProBoundsPointerArithmeticCheck>(
         "cppcoreguidelines-pro-bounds-pointer-arithmetic");
     CheckFactories.registerCheck<ProTypeConstCastCheck>(
         "cppcoreguidelines-pro-type-const-cast");

clang-tidy/cppcoreguidelines/EnumAllCapsCheck.h

+//===--- EnumAllCapsCheck.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_CPPCOREGUIDELINES_ENUMALLCAPS_H
+#define LLVM_CLANG_TOOLS_EXTRA_CLANG_TIDY_CPPCOREGUIDELINES_ENUMALLCAPS_H
+
+#include "../ClangTidy.h"
+
+namespace clang {
+namespace tidy {
+namespace cppcoreguidelines {
+
+/// This check flags all ALL_CAPS enumerations
+///
+/// For the user-facing documentation see:
+/// http://clang.llvm.org/extra/clang-tidy/checks/cppcoreguidelines-enum-all-caps.html
+class EnumAllCapsCheck : public ClangTidyCheck {
+public:
+  EnumAllCapsCheck(StringRef Name, ClangTidyContext *Context)
+      : ClangTidyCheck(Name, Context) {}
+  void registerMatchers(ast_matchers::MatchFinder *Finder) override;
+  void check(const ast_matchers::MatchFinder::MatchResult &Result) override;
+};
+
+} // namespace cppcoreguidelines
+} // namespace tidy
+} // namespace clang
+
+#endif // LLVM_CLANG_TOOLS_EXTRA_CLANG_TIDY_CPPCOREGUIDELINES_ENUMALLCAPS_H
+

clang-tidy/cppcoreguidelines/EnumAllCapsCheck.cpp

+//===--- EnumAllCapsCheck.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 "EnumAllCapsCheck.h"
+#include "clang/AST/ASTContext.h"
+#include "clang/ASTMatchers/ASTMatchFinder.h"
+
+#include <cctype>
+#include <string>
+#include <algorithm>
+
+using namespace clang::ast_matchers;
+
+namespace clang {
+namespace tidy {
+namespace cppcoreguidelines {
+
+void EnumAllCapsCheck::registerMatchers(MatchFinder *Finder) {
+  Finder->addMatcher(enumConstantDecl().bind("enum"), this);
+}
+
+void EnumAllCapsCheck::check(const MatchFinder::MatchResult &Result) {
+  const auto *MatchedDecl = Result.Nodes.getNodeAs<EnumConstantDecl>("enum");
+  unsigned AlphabeticLetters = 0;
+  bool AllCapsed = true;
+  for (auto symbol : MatchedDecl->getName()) {
+    if (std::isalpha(symbol)) {
+      AlphabeticLetters++;
+      if (!std::isupper(symbol)) {
+        AllCapsed = false;
+      }
+    }
+  }
+  if (!AllCapsed) {
+    return;
+  }
+  if (AlphabeticLetters <= 1) {
+    return;
+  }
+  std::string HintedLowercaseName = MatchedDecl->getName();
+  std::transform(HintedLowercaseName.begin(), HintedLowercaseName.end(),
+                 HintedLowercaseName.begin(), ::tolower);
+  diag(MatchedDecl->getLocation(), "Don’t use ALL_CAPS for enumerators")
+      << FixItHint::CreateInsertion(MatchedDecl->getLocation(),
+                                    HintedLowercaseName);
+}
+}
+} // namespace tidy
+} // namespace clang

docs/clang-tidy/checks/cppcoreguidelines-enum-all-caps.rst

+cppcoreguidelines-enum-all-caps
+=============================
+
+This check flags all ALL_CAPS enumerations.
+
+Enumerations should not be named using ALL_CAPS to avoid clashes with macros.
+
+This rule is part of the "Enumerations" profile of the C++ Core Guidelines, see
+https://github.com/isocpp/CppCoreGuidelines/blob/master/CppCoreGuidelines.md#-enum5-dont-use-all_caps-for-enumerators

docs/clang-tidy/checks/list.rst

 List of clang-tidy Checks
 =========================
 
 .. toctree::   
    cert-setlongjmp
    cert-static-object-exception
    cert-thrown-exception-type
    cert-variadic-function-def
+   cppcoreguidelines-enum-all-caps
    cppcoreguidelines-pro-bounds-array-to-pointer-decay
    cppcoreguidelines-pro-bounds-constant-array-index
    cppcoreguidelines-pro-bounds-pointer-arithmetic
    cppcoreguidelines-pro-type-const-cast
    cppcoreguidelines-pro-type-cstyle-cast
    cppcoreguidelines-pro-type-reinterpret-cast
    cppcoreguidelines-pro-type-static-cast-downcast
    cppcoreguidelines-pro-type-union-access

test/clang-tidy/cppcoreguidelines-enum-all-caps.cpp

+// RUN: clang-tidy -checks=cppcoreguidelines-enum-all-caps %s
+
+enum color {
+RED = 0xFF0000,
+// CHECK-MESSAGES: :[[@LINE-1]]:6: warning: Don’t use ALL_CAPS for enumerators [cppcoreguidelines-enum-all-caps]
+// CHECK-FIXES: red
+BLUE_COLOR = 0x0000FF,
+// CHECK-MESSAGES: :[[@LINE-1]]:6: warning: Don’t use ALL_CAPS for enumerators [cppcoreguidelines-enum-all-caps]
+// CHECK-FIXES: blue_color
+COOL_GREEN_123
+// CHECK-MESSAGES: :[[@LINE-1]]:6: warning: Don’t use ALL_CAPS for enumerators [cppcoreguidelines-enum-all-caps]
+// CHECK-FIXES: cool_green_123
+};
+
+// These are OK
+enum star_wars_character {
+  Chewbacca = 0,
+  Yoda = 1,
+  Darth_Vader = 2
+};