Skip to content

[clangd] Add tweak for turning an unscoped into a scoped enum #69481

New issue

Have a question about this project? Sign up for a free GitHub account to open an issue and contact its maintainers and the community.

By clicking “Sign up for GitHub”, you agree to our terms of service and privacy statement. We’ll occasionally send you account related emails.

Already on GitHub? Sign in to your account

Merged
merged 1 commit into from
Nov 10, 2023
Merged
Show file tree
Hide file tree
Changes from all commits
Commits
File filter

Filter by extension

Filter by extension

Conversations
Failed to load comments.
Loading
Jump to
Jump to file
Failed to load files.
Loading
Diff view
Diff view
1 change: 1 addition & 0 deletions clang-tools-extra/clangd/refactor/tweaks/CMakeLists.txt
Original file line number Diff line number Diff line change
Expand Up @@ -27,6 +27,7 @@ add_clang_library(clangDaemonTweaks OBJECT
PopulateSwitch.cpp
RawStringLiteral.cpp
RemoveUsingNamespace.cpp
ScopifyEnum.cpp
SpecialMembers.cpp
SwapIfBranches.cpp

Expand Down
222 changes: 222 additions & 0 deletions clang-tools-extra/clangd/refactor/tweaks/ScopifyEnum.cpp
Original file line number Diff line number Diff line change
@@ -0,0 +1,222 @@
//===--- ScopifyEnum.cpp --------------------------------------- -*- C++-*-===//
//
// Part of the LLVM Project, under the Apache License v2.0 with LLVM Exceptions.
// See https://llvm.org/LICENSE.txt for license information.
// SPDX-License-Identifier: Apache-2.0 WITH LLVM-exception
//
//===----------------------------------------------------------------------===//

#include "ParsedAST.h"
#include "XRefs.h"
#include "refactor/Tweak.h"
#include "clang/Basic/SourceLocation.h"
#include "clang/Basic/SourceManager.h"
#include "clang/Tooling/Core/Replacement.h"
#include "llvm/ADT/StringRef.h"
#include "llvm/Support/Error.h"

#include <functional>

namespace clang::clangd {
namespace {

/// Turns an unscoped into a scoped enum type.
/// Before:
/// enum E { EV1, EV2 };
/// ^
/// void f() { E e1 = EV1; }
///
/// After:
/// enum class E { EV1, EV2 };
/// void f() { E e1 = E::EV1; }
///
/// 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;
std::string title() const override { return "Convert to scoped enum"; }
llvm::StringLiteral kind() const override {
return CodeAction::REFACTOR_KIND;
}
bool prepare(const Selection &Inputs) override;
Expected<Tweak::Effect> apply(const Selection &Inputs) override;

using MakeReplacement =
std::function<tooling::Replacement(StringRef, StringRef, unsigned)>;
llvm::Error addClassKeywordToDeclarations();
llvm::Error scopifyEnumValues();
llvm::Error scopifyEnumValue(const EnumConstantDecl &CD, StringRef Prefix);
llvm::Expected<StringRef> getContentForFile(StringRef FilePath);
unsigned getOffsetFromPosition(const Position &Pos, StringRef Content) const;
llvm::Error addReplacementForReference(const ReferencesResult::Reference &Ref,
const MakeReplacement &GetReplacement);
llvm::Error addReplacement(StringRef FilePath, StringRef Content,
const tooling::Replacement &Replacement);
Position getPosition(const Decl &D) const;

const EnumDecl *D = nullptr;
const Selection *S = nullptr;
SourceManager *SM = nullptr;
llvm::SmallVector<std::unique_ptr<llvm::MemoryBuffer>> ExtraBuffers;
llvm::StringMap<StringRef> ContentPerFile;
Effect E;
};

REGISTER_TWEAK(ScopifyEnum)

bool ScopifyEnum::prepare(const Selection &Inputs) {
if (!Inputs.AST->getLangOpts().CPlusPlus11)
return false;
const SelectionTree::Node *N = Inputs.ASTSelection.commonAncestor();
if (!N)
return false;
D = N->ASTNode.get<EnumDecl>();
return D && !D->isScoped() && D->isThisDeclarationADefinition();
}

Expected<Tweak::Effect> ScopifyEnum::apply(const Selection &Inputs) {
S = &Inputs;
SM = &S->AST->getSourceManager();
E.FormatEdits = false;
ContentPerFile.insert(std::make_pair(SM->getFilename(D->getLocation()),
SM->getBufferData(SM->getMainFileID())));

if (auto Err = addClassKeywordToDeclarations())
return Err;
if (auto Err = scopifyEnumValues())
return Err;

return E;
}

llvm::Error ScopifyEnum::addClassKeywordToDeclarations() {
for (const auto &Ref :
findReferences(*S->AST, getPosition(*D), 0, S->Index, false)
.References) {
if (!(Ref.Attributes & ReferencesResult::Declaration))
continue;

static const auto MakeReplacement = [](StringRef FilePath,
StringRef Content, unsigned Offset) {
return tooling::Replacement(FilePath, Offset, 0, "class ");
};
if (auto Err = addReplacementForReference(Ref, MakeReplacement))
return Err;
}
return llvm::Error::success();
}

llvm::Error ScopifyEnum::scopifyEnumValues() {
std::string PrefixToInsert(D->getName());
PrefixToInsert += "::";
for (auto E : D->enumerators()) {
if (auto Err = scopifyEnumValue(*E, PrefixToInsert))
return Err;
}
return llvm::Error::success();
}

llvm::Error ScopifyEnum::scopifyEnumValue(const EnumConstantDecl &CD,
StringRef Prefix) {
for (const auto &Ref :
findReferences(*S->AST, getPosition(CD), 0, S->Index, false)
.References) {
if (Ref.Attributes & ReferencesResult::Declaration)
continue;

const auto MakeReplacement = [&Prefix](StringRef FilePath,
StringRef Content, unsigned Offset) {
const auto IsAlreadyScoped = [Content, Offset] {
if (Offset < 2)
return false;
unsigned I = Offset;
while (--I > 0) {
switch (Content[I]) {
case ' ':
case '\t':
case '\n':
continue;
case ':':
if (Content[I - 1] == ':')
return true;
[[fallthrough]];
default:
return false;
}
}
return false;
};
return IsAlreadyScoped()
? tooling::Replacement()
: tooling::Replacement(FilePath, Offset, 0, Prefix);
};
if (auto Err = addReplacementForReference(Ref, MakeReplacement))
return Err;
}

return llvm::Error::success();
}

llvm::Expected<StringRef> ScopifyEnum::getContentForFile(StringRef FilePath) {
if (auto It = ContentPerFile.find(FilePath); It != ContentPerFile.end())
return It->second;
auto Buffer = S->FS->getBufferForFile(FilePath);
if (!Buffer)
return llvm::errorCodeToError(Buffer.getError());
StringRef Content = Buffer->get()->getBuffer();
ExtraBuffers.push_back(std::move(*Buffer));
ContentPerFile.insert(std::make_pair(FilePath, Content));
return Content;
}

unsigned int ScopifyEnum::getOffsetFromPosition(const Position &Pos,
StringRef Content) const {
unsigned int Offset = 0;

for (std::size_t LinesRemaining = Pos.line;
Offset < Content.size() && LinesRemaining;) {
if (Content[Offset++] == '\n')
--LinesRemaining;
}
return Offset + Pos.character;
}

llvm::Error
ScopifyEnum::addReplacementForReference(const ReferencesResult::Reference &Ref,
const MakeReplacement &GetReplacement) {
StringRef FilePath = Ref.Loc.uri.file();
auto Content = getContentForFile(FilePath);
if (!Content)
return Content.takeError();
unsigned Offset = getOffsetFromPosition(Ref.Loc.range.start, *Content);
tooling::Replacement Replacement = GetReplacement(FilePath, *Content, Offset);
if (Replacement.isApplicable())
return addReplacement(FilePath, *Content, Replacement);
return llvm::Error::success();
}

llvm::Error
ScopifyEnum::addReplacement(StringRef FilePath, StringRef Content,
const tooling::Replacement &Replacement) {
Edit &TheEdit = E.ApplyEdits[FilePath];
TheEdit.InitialCode = Content;
if (auto Err = TheEdit.Replacements.add(Replacement))
return Err;
return llvm::Error::success();
}

Position ScopifyEnum::getPosition(const Decl &D) const {
const SourceLocation Loc = D.getLocation();
Position Pos;
Pos.line = SM->getSpellingLineNumber(Loc) - 1;
Pos.character = SM->getSpellingColumnNumber(Loc) - 1;
return Pos;
}

} // namespace
} // namespace clang::clangd
1 change: 1 addition & 0 deletions clang-tools-extra/clangd/unittests/CMakeLists.txt
Original file line number Diff line number Diff line change
Expand Up @@ -131,6 +131,7 @@ add_unittest(ClangdUnitTests ClangdTests
tweaks/PopulateSwitchTests.cpp
tweaks/RawStringLiteralTests.cpp
tweaks/RemoveUsingNamespaceTests.cpp
tweaks/ScopifyEnumTests.cpp
tweaks/ShowSelectionTreeTests.cpp
tweaks/SpecialMembersTests.cpp
tweaks/SwapIfBranchesTests.cpp
Expand Down
58 changes: 58 additions & 0 deletions clang-tools-extra/clangd/unittests/tweaks/ScopifyEnumTests.cpp
Original file line number Diff line number Diff line change
@@ -0,0 +1,58 @@
//===-- DefineOutline.cpp ---------------------------------------*- C++ -*-===//
//
// Part of the LLVM Project, under the Apache License v2.0 with LLVM Exceptions.
// See https://llvm.org/LICENSE.txt for license information.
// SPDX-License-Identifier: Apache-2.0 WITH LLVM-exception
//
//===----------------------------------------------------------------------===//

#include "TweakTesting.h"
#include "gtest/gtest.h"

namespace clang::clangd {
namespace {

TWEAK_TEST(ScopifyEnum);

TEST_F(ScopifyEnumTest, TriggersOnUnscopedEnumDecl) {
FileName = "Test.hpp";
// Not available for scoped enum.
EXPECT_UNAVAILABLE(R"cpp(enum class ^E { V };)cpp");

// Not available for non-definition.
EXPECT_UNAVAILABLE(R"cpp(
enum E { V };
enum ^E;
)cpp");
}

TEST_F(ScopifyEnumTest, ApplyTest) {
std::string Original = R"cpp(
enum ^E { EV1, EV2, EV3 };
enum E;
E func(E in)
{
E out = EV1;
if (in == EV2)
out = E::EV3;
return out;
}
)cpp";
std::string Expected = R"cpp(
enum class E { EV1, EV2, EV3 };
enum class E;
E func(E in)
{
E out = E::EV1;
if (in == E::EV2)
out = E::EV3;
return out;
}
)cpp";
FileName = "Test.cpp";
SCOPED_TRACE(Original);
EXPECT_EQ(apply(Original), Expected);
}

} // namespace
} // namespace clang::clangd
1 change: 1 addition & 0 deletions clang-tools-extra/docs/ReleaseNotes.rst
Original file line number Diff line number Diff line change
Expand Up @@ -70,6 +70,7 @@ Code actions
^^^^^^^^^^^^

- The extract variable tweak gained support for extracting lambda expressions to a variable.
- A new tweak was added for turning unscoped into scoped enums.

Signature help
^^^^^^^^^^^^^^
Expand Down