[clang-tools-extra] [clangd] Remove potential prefix from enum value names (PR #83412)
https://github.com/ckandeler closed https://github.com/llvm/llvm-project/pull/83412 ___ cfe-commits mailing list cfe-commits@lists.llvm.org https://lists.llvm.org/cgi-bin/mailman/listinfo/cfe-commits
[clang-tools-extra] [clangd] Remove potential prefix from enum value names (PR #83412)
https://github.com/ckandeler reopened https://github.com/llvm/llvm-project/pull/83412 ___ cfe-commits mailing list cfe-commits@lists.llvm.org https://lists.llvm.org/cgi-bin/mailman/listinfo/cfe-commits
[clang-tools-extra] [clangd] Remove potential prefix from enum value names (PR #83412)
https://github.com/ckandeler closed https://github.com/llvm/llvm-project/pull/83412 ___ cfe-commits mailing list cfe-commits@lists.llvm.org https://lists.llvm.org/cgi-bin/mailman/listinfo/cfe-commits
[clang-tools-extra] [clangd] Remove potential prefix from enum value names (PR #83412)
https://github.com/5chmidti approved this pull request. LGTM https://github.com/llvm/llvm-project/pull/83412 ___ cfe-commits mailing list cfe-commits@lists.llvm.org https://lists.llvm.org/cgi-bin/mailman/listinfo/cfe-commits
[clang-tools-extra] [clangd] Remove potential prefix from enum value names (PR #83412)
ckandeler wrote: Underscore separators get removed now. https://github.com/llvm/llvm-project/pull/83412 ___ cfe-commits mailing list cfe-commits@lists.llvm.org https://lists.llvm.org/cgi-bin/mailman/listinfo/cfe-commits
[clang-tools-extra] [clangd] Remove potential prefix from enum value names (PR #83412)
https://github.com/ckandeler updated
https://github.com/llvm/llvm-project/pull/83412
>From 349edc160e9c13068c42b35321814296bb713a09 Mon Sep 17 00:00:00 2001
From: Christian Kandeler
Date: Thu, 29 Feb 2024 12:26:52 +0100
Subject: [PATCH] [clangd] Remove potential prefix from enum value names
... 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.
---
.../clangd/refactor/tweaks/ScopifyEnum.cpp| 61 -
.../unittests/tweaks/ScopifyEnumTests.cpp | 66 +--
clang-tools-extra/docs/ReleaseNotes.rst | 3 +
3 files changed, 108 insertions(+), 22 deletions(-)
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;
llvm::Error addClassKeywordToDeclarations();
llvm::Error scopifyEnumValues();
- llvm::Error scopifyEnumValue(const EnumConstantDecl , StringRef Prefix);
+ llvm::Error scopifyEnumValue(const EnumConstantDecl , StringRef EnumName,
+ bool StripPrefix);
llvm::Expected getContentForFile(StringRef FilePath);
unsigned getOffsetFromPosition(const Position , StringRef Content) const;
llvm::Error addReplacementForReference(const ReferencesResult::Reference
,
@@ -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 ,
- StringRef Prefix) {
+ StringRef EnumName,
+ bool StripPrefix) {
for (const auto :
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 = [](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 = [](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 ,
}
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())
+
[clang-tools-extra] [clangd] Remove potential prefix from enum value names (PR #83412)
https://github.com/5chmidti commented: When the enumerators start with the enum name, but the names contain a `_` as a separator, then applying this tweak will result in `_`-prefixed enumerators. Can you please handle that case as well and remove the `_`? Otherwise, this looks good to me https://github.com/llvm/llvm-project/pull/83412 ___ cfe-commits mailing list cfe-commits@lists.llvm.org https://lists.llvm.org/cgi-bin/mailman/listinfo/cfe-commits
[clang-tools-extra] [clangd] Remove potential prefix from enum value names (PR #83412)
ckandeler wrote: Incorporated review comments. https://github.com/llvm/llvm-project/pull/83412 ___ cfe-commits mailing list cfe-commits@lists.llvm.org https://lists.llvm.org/cgi-bin/mailman/listinfo/cfe-commits
[clang-tools-extra] [clangd] Remove potential prefix from enum value names (PR #83412)
https://github.com/ckandeler updated
https://github.com/llvm/llvm-project/pull/83412
>From 78393d26e62f2f9a30f366501f8e61b1a32d6af4 Mon Sep 17 00:00:00 2001
From: Christian Kandeler
Date: Thu, 29 Feb 2024 12:26:52 +0100
Subject: [PATCH] [clangd] Remove potential prefix from enum value names
... 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.
---
.../clangd/refactor/tweaks/ScopifyEnum.cpp| 55 +--
.../unittests/tweaks/ScopifyEnumTests.cpp | 38 +++--
clang-tools-extra/docs/ReleaseNotes.rst | 3 +
3 files changed, 74 insertions(+), 22 deletions(-)
diff --git a/clang-tools-extra/clangd/refactor/tweaks/ScopifyEnum.cpp
b/clang-tools-extra/clangd/refactor/tweaks/ScopifyEnum.cpp
index e36b3249bc7b92..d44f2bb862ed07 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;
llvm::Error addClassKeywordToDeclarations();
llvm::Error scopifyEnumValues();
- llvm::Error scopifyEnumValue(const EnumConstantDecl , StringRef Prefix);
+ llvm::Error scopifyEnumValue(const EnumConstantDecl , StringRef EnumName,
+ bool StripPrefix);
llvm::Expected getContentForFile(StringRef FilePath);
unsigned getOffsetFromPosition(const Position , StringRef Content) const;
llvm::Error addReplacementForReference(const ReferencesResult::Reference
,
@@ -125,25 +123,42 @@ 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 ,
- StringRef Prefix) {
+ StringRef EnumName,
+ bool StripPrefix) {
for (const auto :
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 = [](StringRef FilePath,
+ StringRef /* Content */,
+ unsigned Offset) {
+ return tooling::Replacement(FilePath, Offset, EnumName.size(), {});
+};
+if (auto Err = addReplacementForReference(Ref, MakeReplacement))
+ return Err;
+ }
continue;
+}
-const auto MakeReplacement = [](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 +179,15 @@ llvm::Error ScopifyEnum::scopifyEnumValue(const
EnumConstantDecl ,
}
return false;
};
- return IsAlreadyScoped()
- ? tooling::Replacement()
- : tooling::Replacement(FilePath, Offset, 0, Prefix);
+ if (StripPrefix) {
+if (IsAlreadyScoped())
+ return tooling::Replacement(FilePath, Offset, EnumName.size(), {});
+return tooling::Replacement(FilePath, Offset + EnumName.size(), 0,
+"::");
+ }
[clang-tools-extra] [clangd] Remove potential prefix from enum value names (PR #83412)
https://github.com/5chmidti edited https://github.com/llvm/llvm-project/pull/83412 ___ cfe-commits mailing list cfe-commits@lists.llvm.org https://lists.llvm.org/cgi-bin/mailman/listinfo/cfe-commits
[clang-tools-extra] [clangd] Remove potential prefix from enum value names (PR #83412)
https://github.com/5chmidti commented: I think this change could use a release note in the docs, could you please add one? https://github.com/llvm/llvm-project/pull/83412 ___ cfe-commits mailing list cfe-commits@lists.llvm.org https://lists.llvm.org/cgi-bin/mailman/listinfo/cfe-commits
[clang-tools-extra] [clangd] Remove potential prefix from enum value names (PR #83412)
@@ -125,25 +123,42 @@ llvm::Error ScopifyEnum::addClassKeywordToDeclarations() {
}
llvm::Error ScopifyEnum::scopifyEnumValues() {
- std::string PrefixToInsert(D->getName());
- PrefixToInsert += "::";
+ StringRef EnumName(D->getName());
+ bool StripPrefix = true;
+ for (auto E : D->enumerators()) {
5chmidti wrote:
Please use `const EnumConstantDecl *` instead of `auto`.
https://github.com/llvm/llvm-project/pull/83412
___
cfe-commits mailing list
cfe-commits@lists.llvm.org
https://lists.llvm.org/cgi-bin/mailman/listinfo/cfe-commits
[clang-tools-extra] [clangd] Remove potential prefix from enum value names (PR #83412)
@@ -125,25 +123,42 @@ llvm::Error ScopifyEnum::addClassKeywordToDeclarations() {
}
llvm::Error ScopifyEnum::scopifyEnumValues() {
- std::string PrefixToInsert(D->getName());
- PrefixToInsert += "::";
+ StringRef EnumName(D->getName());
+ bool StripPrefix = true;
+ for (auto E : D->enumerators()) {
+if (!E->getName().starts_with(EnumName)) {
+ StripPrefix = false;
+ break;
+}
+ }
for (auto E : D->enumerators()) {
5chmidti wrote:
This is not in your changed lines, but please change this `auto` to `const
EnumConstantDecl *` as well.
https://github.com/llvm/llvm-project/pull/83412
___
cfe-commits mailing list
cfe-commits@lists.llvm.org
https://lists.llvm.org/cgi-bin/mailman/listinfo/cfe-commits
[clang-tools-extra] [clangd] Remove potential prefix from enum value names (PR #83412)
https://github.com/ckandeler updated
https://github.com/llvm/llvm-project/pull/83412
>From 01f74ddece947755938ccecbcc5f9d18a41eb793 Mon Sep 17 00:00:00 2001
From: Christian Kandeler
Date: Thu, 29 Feb 2024 12:26:52 +0100
Subject: [PATCH] [clangd] Remove potential prefix from enum value names
... 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.
---
.../clangd/refactor/tweaks/ScopifyEnum.cpp| 53 +--
.../unittests/tweaks/ScopifyEnumTests.cpp | 38 +++--
2 files changed, 70 insertions(+), 21 deletions(-)
diff --git a/clang-tools-extra/clangd/refactor/tweaks/ScopifyEnum.cpp
b/clang-tools-extra/clangd/refactor/tweaks/ScopifyEnum.cpp
index e36b3249bc7b92..8e13ae52d121a6 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;
llvm::Error addClassKeywordToDeclarations();
llvm::Error scopifyEnumValues();
- llvm::Error scopifyEnumValue(const EnumConstantDecl , StringRef Prefix);
+ llvm::Error scopifyEnumValue(const EnumConstantDecl , StringRef EnumName,
+ bool StripPrefix);
llvm::Expected getContentForFile(StringRef FilePath);
unsigned getOffsetFromPosition(const Position , StringRef Content) const;
llvm::Error addReplacementForReference(const ReferencesResult::Reference
,
@@ -125,25 +123,42 @@ llvm::Error ScopifyEnum::addClassKeywordToDeclarations() {
}
llvm::Error ScopifyEnum::scopifyEnumValues() {
- std::string PrefixToInsert(D->getName());
- PrefixToInsert += "::";
+ StringRef EnumName(D->getName());
+ bool StripPrefix = true;
+ for (auto E : D->enumerators()) {
+if (!E->getName().starts_with(EnumName)) {
+ StripPrefix = false;
+ break;
+}
+ }
for (auto E : D->enumerators()) {
-if (auto Err = scopifyEnumValue(*E, PrefixToInsert))
+if (auto Err = scopifyEnumValue(*E, EnumName, StripPrefix))
return Err;
}
return llvm::Error::success();
}
llvm::Error ScopifyEnum::scopifyEnumValue(const EnumConstantDecl ,
- StringRef Prefix) {
+ StringRef EnumName,
+ bool StripPrefix) {
for (const auto :
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 = [](StringRef FilePath,
+ StringRef /* Content */,
+ unsigned Offset) {
+ return tooling::Replacement(FilePath, Offset, EnumName.size(), {});
+};
+if (auto Err = addReplacementForReference(Ref, MakeReplacement))
+ return Err;
+ }
continue;
+}
-const auto MakeReplacement = [](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 +179,15 @@ llvm::Error ScopifyEnum::scopifyEnumValue(const
EnumConstantDecl ,
}
return false;
};
- return IsAlreadyScoped()
- ? tooling::Replacement()
- : tooling::Replacement(FilePath, Offset, 0, Prefix);
+ if (StripPrefix) {
+if (IsAlreadyScoped())
+ return tooling::Replacement(FilePath, Offset, EnumName.size(), {});
+return tooling::Replacement(FilePath, Offset + EnumName.size(), 0,
+"::");
+ }
+ return IsAlreadyScoped() ? tooling::Replacement()
+ : tooling::Replacement(FilePath, Offset,
[clang-tools-extra] [clangd] Remove potential prefix from enum value names (PR #83412)
llvmbot wrote:
@llvm/pr-subscribers-clangd
Author: Christian Kandeler (ckandeler)
Changes
... 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.
---
Full diff: https://github.com/llvm/llvm-project/pull/83412.diff
2 Files Affected:
- (modified) clang-tools-extra/clangd/refactor/tweaks/ScopifyEnum.cpp (+35-14)
- (modified) clang-tools-extra/clangd/unittests/tweaks/ScopifyEnumTests.cpp
(+4-4)
``diff
diff --git a/clang-tools-extra/clangd/refactor/tweaks/ScopifyEnum.cpp
b/clang-tools-extra/clangd/refactor/tweaks/ScopifyEnum.cpp
index e36b3249bc7b92..5c042ee7b782b9 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;
llvm::Error addClassKeywordToDeclarations();
llvm::Error scopifyEnumValues();
- llvm::Error scopifyEnumValue(const EnumConstantDecl , StringRef Prefix);
+ llvm::Error scopifyEnumValue(const EnumConstantDecl , StringRef EnumName,
+ bool StripPrefix);
llvm::Expected getContentForFile(StringRef FilePath);
unsigned getOffsetFromPosition(const Position , StringRef Content) const;
llvm::Error addReplacementForReference(const ReferencesResult::Reference
,
@@ -125,25 +123,42 @@ llvm::Error ScopifyEnum::addClassKeywordToDeclarations() {
}
llvm::Error ScopifyEnum::scopifyEnumValues() {
- std::string PrefixToInsert(D->getName());
- PrefixToInsert += "::";
+ StringRef EnumName(D->getName());
+ bool StripPrefix = true;
+ for (auto E : D->enumerators()) {
+if (!E->getName().starts_with(EnumName)) {
+ StripPrefix = false;
+ break;
+}
+ }
for (auto E : D->enumerators()) {
-if (auto Err = scopifyEnumValue(*E, PrefixToInsert))
+if (auto Err = scopifyEnumValue(*E, EnumName, StripPrefix))
return Err;
}
return llvm::Error::success();
}
llvm::Error ScopifyEnum::scopifyEnumValue(const EnumConstantDecl ,
- StringRef Prefix) {
+ StringRef EnumName,
+ bool StripPrefix) {
for (const auto :
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 = [](StringRef FilePath,
+ StringRef /* Content */,
+ unsigned Offset) {
+ return tooling::Replacement(FilePath, Offset, EnumName.size(), {});
+};
+if (auto Err = addReplacementForReference(Ref, MakeReplacement))
+ return Err;
+ }
continue;
+}
-const auto MakeReplacement = [](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 +179,15 @@ llvm::Error ScopifyEnum::scopifyEnumValue(const
EnumConstantDecl ,
}
return false;
};
+ if (StripPrefix) {
+if (IsAlreadyScoped())
+ return tooling::Replacement(FilePath, Offset, EnumName.size(), {});
+return tooling::Replacement(FilePath, Offset + EnumName.size(), 0,
+"::");
+ }
return IsAlreadyScoped()
? tooling::Replacement()
- : tooling::Replacement(FilePath, Offset, 0, Prefix);
+ : tooling::Replacement(FilePath, Offset, 0, EnumName);
};
if (auto Err = addReplacementForReference(Ref, MakeReplacement))
return Err;
diff --git a/clang-tools-extra/clangd/unittests/tweaks/ScopifyEnumTests.cpp
[clang-tools-extra] [clangd] Remove potential prefix from enum value names (PR #83412)
https://github.com/ckandeler created
https://github.com/llvm/llvm-project/pull/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.
>From c687b73939821eba285b35e50c241738ba7915e4 Mon Sep 17 00:00:00 2001
From: Christian Kandeler
Date: Thu, 29 Feb 2024 12:26:52 +0100
Subject: [PATCH] [clangd] Remove potential prefix from enum value names
... 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.
---
.../clangd/refactor/tweaks/ScopifyEnum.cpp| 49 +--
.../unittests/tweaks/ScopifyEnumTests.cpp | 8 +--
2 files changed, 39 insertions(+), 18 deletions(-)
diff --git a/clang-tools-extra/clangd/refactor/tweaks/ScopifyEnum.cpp
b/clang-tools-extra/clangd/refactor/tweaks/ScopifyEnum.cpp
index e36b3249bc7b92..5c042ee7b782b9 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;
llvm::Error addClassKeywordToDeclarations();
llvm::Error scopifyEnumValues();
- llvm::Error scopifyEnumValue(const EnumConstantDecl , StringRef Prefix);
+ llvm::Error scopifyEnumValue(const EnumConstantDecl , StringRef EnumName,
+ bool StripPrefix);
llvm::Expected getContentForFile(StringRef FilePath);
unsigned getOffsetFromPosition(const Position , StringRef Content) const;
llvm::Error addReplacementForReference(const ReferencesResult::Reference
,
@@ -125,25 +123,42 @@ llvm::Error ScopifyEnum::addClassKeywordToDeclarations() {
}
llvm::Error ScopifyEnum::scopifyEnumValues() {
- std::string PrefixToInsert(D->getName());
- PrefixToInsert += "::";
+ StringRef EnumName(D->getName());
+ bool StripPrefix = true;
+ for (auto E : D->enumerators()) {
+if (!E->getName().starts_with(EnumName)) {
+ StripPrefix = false;
+ break;
+}
+ }
for (auto E : D->enumerators()) {
-if (auto Err = scopifyEnumValue(*E, PrefixToInsert))
+if (auto Err = scopifyEnumValue(*E, EnumName, StripPrefix))
return Err;
}
return llvm::Error::success();
}
llvm::Error ScopifyEnum::scopifyEnumValue(const EnumConstantDecl ,
- StringRef Prefix) {
+ StringRef EnumName,
+ bool StripPrefix) {
for (const auto :
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 = [](StringRef FilePath,
+ StringRef /* Content */,
+ unsigned Offset) {
+ return tooling::Replacement(FilePath, Offset, EnumName.size(), {});
+};
+if (auto Err = addReplacementForReference(Ref, MakeReplacement))
+ return Err;
+ }
continue;
+}
-const auto MakeReplacement = [](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 +179,15 @@ llvm::Error ScopifyEnum::scopifyEnumValue(const
EnumConstantDecl ,
}
return false;
};
+ if (StripPrefix) {
+if (IsAlreadyScoped())
+ return tooling::Replacement(FilePath, Offset, EnumName.size(), {});
+
