[PATCH] D85697: [clang-tidy] Add cppcoreguidelines-prefer-scoped-enums-over-unscoped

[Nathan James via Phabricator via cfe-commits]

njames93 added a comment.

Is 'over-unscoped' really needed in the name, would just 'prefer-scoped-enum' be better, WDYT?

For the case of macro, you can check if the location is a macro using `SourceLocation::isMacroID()`.

For this to work you would also need to check every single usage of the the values in the enum to make sure they are converted to use the scoped access.
You're best bet would be to actually store a map indexed by unscoped enum decls with a set of all their locations and maybe a flag to say if the fix can be applied or not.
For instance a fix couldn't be applied if any of the usages or decls are in macros.
This map could then be checked using the `endOfTranslationUnit` virtual method, with all the diags and fixes being spawned there.



================
Comment at: clang-tools-extra/clang-tidy/cppcoreguidelines/PreferScopedEnumsOverUnscopedCheck.cpp:31-32
+  const auto *MatchedDecl = Result.Nodes.getNodeAs<EnumDecl>("enumDecls");
+  if (MatchedDecl->isScoped())
+    return;
+  diag(MatchedDecl->getLocation(), "enumeration %0 is not a scoped enumeration")
----------------
Please hoist this logic into the matcher.
```
enumDecl(unless(isScoped())).bind("enumDecls")````


================
Comment at: clang-tools-extra/docs/clang-tidy/checks/cppcoreguidelines-prefer-scoped-enums-over-unscoped.rst:20
+    B
+  }
+
----------------
Semi-colon after the struct definition.


================
Comment at: clang-tools-extra/docs/clang-tidy/checks/cppcoreguidelines-prefer-scoped-enums-over-unscoped.rst:29
+    B
+  }
+
----------------
Semi-colon after the struct definition.


================
Comment at: clang-tools-extra/test/clang-tidy/checkers/cppcoreguidelines-prefer-scoped-enums-over-unscoped.cpp:27-28
+enum struct ScopedEnumWithStruct {
+  SEWC_FirstValue,
+  SEWC_SecondValue,
+};
----------------
nit:


Repository:
  rG LLVM Github Monorepo

CHANGES SINCE LAST ACTION
  https://reviews.llvm.org/D85697/new/

https://reviews.llvm.org/D85697