[clang-tidy] Change checks that take enum configurations to use a new access method.

[njames93]

Summary

Change all checks that take enums as configuration to use enum specific methods in ClangTidyCheck::OptionsView.

[njames93]

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

154

Unsure about hardcoding the check-name as it could be ran under an alias. However there is no way for a ClangTidyCheck to get the name its ran as. ClangTidyCheck::CheckName is private, maybe a protected getter would be wise.

177

I feel its safer not assuming the fix and instead letting the user modify their configuration, WDYT?

[njames93]

I'm not too sure how many other checks are like this, but I feel that this functionality could maybe be brought out of this check and into the Options to let more checks that take enum like configurations to use it.

[aaron.ballman]

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

149–151

Some autos that don't have the type spelled out (also, a top-level const that can be dropped).

154

I agree that we should not hardcode the name of the check. I actually wonder if a better approach here is to call Context->diag() to generate an actual diagnostic?

155

acceptible -> acceptable

177

Agreed -- we don't recover in Clang by assuming the fix to the command line option spelling, either.

[njames93]

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

149–151

Copying from what was already there.

154

A few issues with that approach, firstly the ClangTidyCheck doesn't expose the ClangTidyContext to any derived classes. the other issue is it requires a SourceLocation to emit a diagnostic, Passing an invalid SourceLocation effectively suppresses any output, likewise any other SourceLocation will just confuse users.
I have got another implementation in the works that creates new methods in ClangTidyCheck::OptionsView that takes a mapping from StringRef to Enum and uses that for reading (and writing) enums from the configuration. This has access to the CheckName and can distinguish if the enum was read from local or global and adjust accordingly.

[njames93]

• Change checks that take enum configurations to use a new access method.

[aaron.ballman]

LGTM!

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

 
 static StringRef const StyleNames[] = {
 #define STRINGIZE(v) #v,
   NAMING_KEYS(STRINGIZE)
 #undef STRINGIZE
 };
 
 #undef NAMING_KEYS
+
+#define CASE_KEYS(m) \
+    m("aNy_CasE", CT_AnyCase) \
+    m("lower_case", CT_LowerCase) \
+    m("UPPER_CASE", CT_UpperCase) \
+    m("camelBack", CT_CamelBack) \
+    m("CamelCase", CT_CamelCase) \
+    m("Camel_Snake_Case", CT_CamelSnakeCase) \
+    m("camel_Snake_Back", CT_CamelSnakeBack)
 // clang-format on
 
 namespace {
 /// Callback supplies macros to IdentifierNamingCheck::checkMacro
 class IdentifierNamingCheckPPCallbacks : public PPCallbacks {
 public:
   IdentifierNamingCheckPPCallbacks(Preprocessor *PP,
                                    IdentifierNamingCheck *Check)

 
 IdentifierNamingCheck::IdentifierNamingCheck(StringRef Name,
                                              ClangTidyContext *Context)
     : RenamerClangTidyCheck(Name, Context),
       IgnoreFailedSplit(Options.get("IgnoreFailedSplit", 0)),
       IgnoreMainLikeFunctions(Options.get("IgnoreMainLikeFunctions", 0)) {
   auto const fromString = [](StringRef Str) {
     return llvm::StringSwitch<llvm::Optional<CaseType>>(Str)
-        .Case("aNy_CasE", CT_AnyCase)
-        .Case("lower_case", CT_LowerCase)
-        .Case("UPPER_CASE", CT_UpperCase)
-        .Case("camelBack", CT_CamelBack)
-        .Case("CamelCase", CT_CamelCase)
-        .Case("Camel_Snake_Case", CT_CamelSnakeCase)
-        .Case("camel_Snake_Back", CT_CamelSnakeBack)
+#define CASE(NAME, ENUM) .Case(NAME, ENUM)
+        CASE_KEYS(CASE)
+#undef CASE
             .Default(llvm::None);
   };
-
   for (auto const &Name : StyleNames) {
-    auto const caseOptional =
-        fromString(Options.get((Name + "Case").str(), ""));
+    auto CaseOptionName = (Name + "Case").str();
+    auto CaseValue = Options.get(CaseOptionName, "");
+    auto const caseOptional = fromString(CaseValue);
+    if (!caseOptional && !CaseValue.empty()) {
+      llvm::errs() << "warning: Invalid case style '" << CaseValue
+                   << "' for readability-identifier-naming." << CaseOptionName;
+      constexpr StringRef AcceptibleNames[] = {
+#define CASE(NAME, ENUM) NAME,
+          CASE_KEYS(CASE)
+#undef CASE
+      };
+      StringRef Closest = "";
+      unsigned EditDistance = -1u;
+      for (StringRef CheckMatch : AcceptibleNames) {
+        if (CheckMatch.equals_lower(
+                CaseValue)) { // bad case just suggest the fix straight away
+          EditDistance = 0;
+          Closest = CheckMatch;
+          break;
+        }
+        unsigned Distance =
+            llvm::StringRef(CaseValue).edit_distance(CheckMatch);
+        if (Distance < EditDistance) {
+          EditDistance = Distance;
+          Closest = CheckMatch;
+        }
+      }
+      if (EditDistance < 3)
+        llvm::errs() << ": did you mean '" << Closest << "'\n";
+      else
+        llvm::errs() << '\n';
+    }
     auto prefix = Options.get((Name + "Prefix").str(), "");
     auto postfix = Options.get((Name + "Suffix").str(), "");
 
     if (caseOptional || !prefix.empty() || !postfix.empty()) {
       NamingStyles.push_back(NamingStyle(caseOptional, prefix, postfix));
     } else {
       NamingStyles.push_back(llvm::None);
     }

clang-tools-extra/test/clang-tidy/checkers/readability-identifier-naming-case-validation.cpp

+// RUN: clang-tidy %s -checks=readability-identifier-naming \
+// RUN:   -config="{CheckOptions: [\
+// RUN:   {key: readability-identifier-naming.FunctionCase, value: camelback}, \
+// RUN:   {key: readability-identifier-naming.VariableCase, value: camelBack}, \
+// RUN:   {key: readability-identifier-naming.ClassCase, value: UUPER_CASE}, \
+// RUN:   {key: readability-identifier-naming.StructCase, value: CAMEL}, \
+// RUN:   {key: readability-identifier-naming.EnumCase, value: AnY_cASe}, \
+// RUN:   ]}" 2>&1 | FileCheck %s --implicit-check-not warning
+
+
+// CHECK-DAG: warning: Invalid case style 'camelback' for readability-identifier-naming.FunctionCase: did you mean 'camelBack'{{$}}
+// CHECK-DAG: warning: Invalid case style 'UUPER_CASE' for readability-identifier-naming.ClassCase: did you mean 'UPPER_CASE'{{$}}
+// Don't try to suggest an alternative for 'CAMEL'
+// CHECK-DAG: warning: Invalid case style 'CAMEL' for readability-identifier-naming.StructCase{{$}}
+// This fails on the EditDistance, but as it matches ignoring case suggest the correct value
+// CHECK-DAG: warning: Invalid case style 'AnY_cASe' for readability-identifier-naming.EnumCase: did you mean 'aNy_CasE'{{$}}