[clang-tools-extra] f2daa32 - [clangd] Remove potential prefix from enum value names (#83412)
via cfe-commits
cfe-commits at lists.llvm.org
Thu May 2 04:19:03 PDT 2024
Author: Christian Kandeler
Date: 2024-05-02T13:18:59+02:00
New Revision: f2daa32fcaa061daa04546c343198d876e635def
URL: https://github.com/llvm/llvm-project/commit/f2daa32fcaa061daa04546c343198d876e635def
DIFF: https://github.com/llvm/llvm-project/commit/f2daa32fcaa061daa04546c343198d876e635def.diff
LOG: [clangd] Remove potential prefix from enum value names (#83412)
... when converting unscoped to scoped enums.
With traditional enums, a popular technique to guard against potential
name clashes is to use the enum name as a pseudo-namespace, like this:
enum MyEnum { MyEnumValue1, MyEnumValue2 };
With scoped enums, this makes no sense, making it extremely unlikely
that the user wants to keep such a prefix when modernizing. Therefore,
our tweak now removes it.
Added:
Modified:
clang-tools-extra/clangd/refactor/tweaks/ScopifyEnum.cpp
clang-tools-extra/clangd/unittests/tweaks/ScopifyEnumTests.cpp
clang-tools-extra/docs/ReleaseNotes.rst
Removed:
################################################################################
diff --git a/clang-tools-extra/clangd/refactor/tweaks/ScopifyEnum.cpp b/clang-tools-extra/clangd/refactor/tweaks/ScopifyEnum.cpp
index e36b3249bc7b92..2be2bbc422af75 100644
--- a/clang-tools-extra/clangd/refactor/tweaks/ScopifyEnum.cpp
+++ b/clang-tools-extra/clangd/refactor/tweaks/ScopifyEnum.cpp
@@ -40,15 +40,12 @@ namespace {
/// void f() { E e1 = EV1; }
///
/// After:
-/// enum class E { EV1, EV2 };
-/// void f() { E e1 = E::EV1; }
+/// enum class E { V1, V2 };
+/// void f() { E e1 = E::V1; }
///
/// Note that the respective project code might not compile anymore
/// if it made use of the now-gone implicit conversion to int.
/// This is out of scope for this tweak.
-///
-/// TODO: In the above example, we could detect that the values
-/// start with the enum name, and remove that prefix.
class ScopifyEnum : public Tweak {
const char *id() const final;
@@ -63,7 +60,8 @@ class ScopifyEnum : public Tweak {
std::function<tooling::Replacement(StringRef, StringRef, unsigned)>;
llvm::Error addClassKeywordToDeclarations();
llvm::Error scopifyEnumValues();
- llvm::Error scopifyEnumValue(const EnumConstantDecl &CD, StringRef Prefix);
+ llvm::Error scopifyEnumValue(const EnumConstantDecl &CD, StringRef EnumName,
+ bool StripPrefix);
llvm::Expected<StringRef> getContentForFile(StringRef FilePath);
unsigned getOffsetFromPosition(const Position &Pos, StringRef Content) const;
llvm::Error addReplacementForReference(const ReferencesResult::Reference &Ref,
@@ -125,25 +123,45 @@ llvm::Error ScopifyEnum::addClassKeywordToDeclarations() {
}
llvm::Error ScopifyEnum::scopifyEnumValues() {
- std::string PrefixToInsert(D->getName());
- PrefixToInsert += "::";
- for (auto E : D->enumerators()) {
- if (auto Err = scopifyEnumValue(*E, PrefixToInsert))
+ StringRef EnumName(D->getName());
+ bool StripPrefix = true;
+ for (const EnumConstantDecl *E : D->enumerators()) {
+ if (!E->getName().starts_with(EnumName)) {
+ StripPrefix = false;
+ break;
+ }
+ }
+ for (const EnumConstantDecl *E : D->enumerators()) {
+ if (auto Err = scopifyEnumValue(*E, EnumName, StripPrefix))
return Err;
}
return llvm::Error::success();
}
llvm::Error ScopifyEnum::scopifyEnumValue(const EnumConstantDecl &CD,
- StringRef Prefix) {
+ StringRef EnumName,
+ bool StripPrefix) {
for (const auto &Ref :
findReferences(*S->AST, getPosition(CD), 0, S->Index, false)
.References) {
- if (Ref.Attributes & ReferencesResult::Declaration)
+ if (Ref.Attributes & ReferencesResult::Declaration) {
+ if (StripPrefix) {
+ const auto MakeReplacement = [&EnumName](StringRef FilePath,
+ StringRef Content,
+ unsigned Offset) {
+ unsigned Length = EnumName.size();
+ if (Content[Offset + Length] == '_')
+ ++Length;
+ return tooling::Replacement(FilePath, Offset, Length, {});
+ };
+ if (auto Err = addReplacementForReference(Ref, MakeReplacement))
+ return Err;
+ }
continue;
+ }
- const auto MakeReplacement = [&Prefix](StringRef FilePath,
- StringRef Content, unsigned Offset) {
+ const auto MakeReplacement = [&](StringRef FilePath, StringRef Content,
+ unsigned Offset) {
const auto IsAlreadyScoped = [Content, Offset] {
if (Offset < 2)
return false;
@@ -164,9 +182,18 @@ llvm::Error ScopifyEnum::scopifyEnumValue(const EnumConstantDecl &CD,
}
return false;
};
- return IsAlreadyScoped()
- ? tooling::Replacement()
- : tooling::Replacement(FilePath, Offset, 0, Prefix);
+ if (StripPrefix) {
+ const int ExtraLength =
+ Content[Offset + EnumName.size()] == '_' ? 1 : 0;
+ if (IsAlreadyScoped())
+ return tooling::Replacement(FilePath, Offset,
+ EnumName.size() + ExtraLength, {});
+ return tooling::Replacement(FilePath, Offset + EnumName.size(),
+ ExtraLength, "::");
+ }
+ return IsAlreadyScoped() ? tooling::Replacement()
+ : tooling::Replacement(FilePath, Offset, 0,
+ EnumName.str() + "::");
};
if (auto Err = addReplacementForReference(Ref, MakeReplacement))
return Err;
diff --git a/clang-tools-extra/clangd/unittests/tweaks/ScopifyEnumTests.cpp b/clang-tools-extra/clangd/unittests/tweaks/ScopifyEnumTests.cpp
index b5a964a5a26d80..5da059faaf5e8c 100644
--- a/clang-tools-extra/clangd/unittests/tweaks/ScopifyEnumTests.cpp
+++ b/clang-tools-extra/clangd/unittests/tweaks/ScopifyEnumTests.cpp
@@ -26,7 +26,7 @@ enum ^E;
)cpp");
}
-TEST_F(ScopifyEnumTest, ApplyTest) {
+TEST_F(ScopifyEnumTest, ApplyTestWithPrefix) {
std::string Original = R"cpp(
enum ^E { EV1, EV2, EV3 };
enum E;
@@ -39,13 +39,69 @@ E func(E in)
}
)cpp";
std::string Expected = R"cpp(
-enum class E { EV1, EV2, EV3 };
+enum class E { V1, V2, V3 };
enum class E;
E func(E in)
{
- E out = E::EV1;
- if (in == E::EV2)
- out = E::EV3;
+ E out = E::V1;
+ if (in == E::V2)
+ out = E::V3;
+ return out;
+}
+)cpp";
+ FileName = "Test.cpp";
+ SCOPED_TRACE(Original);
+ EXPECT_EQ(apply(Original), Expected);
+}
+
+TEST_F(ScopifyEnumTest, ApplyTestWithPrefixAndUnderscore) {
+ std::string Original = R"cpp(
+enum ^E { E_V1, E_V2, E_V3 };
+enum E;
+E func(E in)
+{
+ E out = E_V1;
+ if (in == E_V2)
+ out = E::E_V3;
+ return out;
+}
+)cpp";
+ std::string Expected = R"cpp(
+enum class E { V1, V2, V3 };
+enum class E;
+E func(E in)
+{
+ E out = E::V1;
+ if (in == E::V2)
+ out = E::V3;
+ return out;
+}
+)cpp";
+ FileName = "Test.cpp";
+ SCOPED_TRACE(Original);
+ EXPECT_EQ(apply(Original), Expected);
+}
+
+TEST_F(ScopifyEnumTest, ApplyTestWithoutPrefix) {
+ std::string Original = R"cpp(
+enum ^E { V1, V2, V3 };
+enum E;
+E func(E in)
+{
+ E out = V1;
+ if (in == V2)
+ out = E::V3;
+ return out;
+}
+)cpp";
+ std::string Expected = R"cpp(
+enum class E { V1, V2, V3 };
+enum class E;
+E func(E in)
+{
+ E out = E::V1;
+ if (in == E::V2)
+ out = E::V3;
return out;
}
)cpp";
diff --git a/clang-tools-extra/docs/ReleaseNotes.rst b/clang-tools-extra/docs/ReleaseNotes.rst
index d59da3a61b7b61..fb7c45aa759cdd 100644
--- a/clang-tools-extra/docs/ReleaseNotes.rst
+++ b/clang-tools-extra/docs/ReleaseNotes.rst
@@ -69,6 +69,9 @@ Code completion
Code actions
^^^^^^^^^^^^
+- The tweak for turning unscoped into scoped enums now removes redundant prefixes
+ from the enum values.
+
Signature help
^^^^^^^^^^^^^^
More information about the cfe-commits
mailing list