[clangd] use existing functions for code locations in the scopify enum tweak

[5chmidti]

Clangd already implements some utility functions for converting between
SourceLocations, Positions and Offsets into a buffer.

[llvmbot]

@llvm/pr-subscribers-clang-tools-extra

@llvm/pr-subscribers-clangd

Author: Julian Schmidt (5chmidti)

Changes

Clangd already implements some utility functions for converting between
SourceLocations, Positions and Offsets into a buffer.

---

Full diff: https://github.com/llvm/llvm-project/pull/88737.diff

1 Files Affected:

• (modified) clang-tools-extra/clangd/refactor/tweaks/ScopifyEnum.cpp (+11-27)

diff --git a/clang-tools-extra/clangd/refactor/tweaks/ScopifyEnum.cpp b/clang-tools-extra/clangd/refactor/tweaks/ScopifyEnum.cpp
index e36b3249bc7b92..0c51c1b39a8401 100644
--- a/clang-tools-extra/clangd/refactor/tweaks/ScopifyEnum.cpp
+++ b/clang-tools-extra/clangd/refactor/tweaks/ScopifyEnum.cpp
@@ -65,12 +65,10 @@ class ScopifyEnum : public Tweak {
   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;
@@ -109,7 +107,8 @@ Expected<Tweak::Effect> ScopifyEnum::apply(const Selection &Inputs) {
 
 llvm::Error ScopifyEnum::addClassKeywordToDeclarations() {
   for (const auto &Ref :
-       findReferences(*S->AST, getPosition(*D), 0, S->Index, false)
+       findReferences(*S->AST, sourceLocToPosition(*SM, D->getBeginLoc()), 0,
+                      S->Index, false)
            .References) {
     if (!(Ref.Attributes & ReferencesResult::Declaration))
       continue;
@@ -137,7 +136,8 @@ llvm::Error ScopifyEnum::scopifyEnumValues() {
 llvm::Error ScopifyEnum::scopifyEnumValue(const EnumConstantDecl &CD,
                                           StringRef Prefix) {
   for (const auto &Ref :
-       findReferences(*S->AST, getPosition(CD), 0, S->Index, false)
+       findReferences(*S->AST, sourceLocToPosition(*SM, CD.getBeginLoc()), 0,
+                      S->Index, false)
            .References) {
     if (Ref.Attributes & ReferencesResult::Declaration)
       continue;
@@ -187,27 +187,19 @@ llvm::Expected<StringRef> ScopifyEnum::getContentForFile(StringRef FilePath) {
   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);
+  llvm::Expected<StringRef> Content = getContentForFile(FilePath);
   if (!Content)
     return Content.takeError();
-  unsigned Offset = getOffsetFromPosition(Ref.Loc.range.start, *Content);
-  tooling::Replacement Replacement = GetReplacement(FilePath, *Content, Offset);
+  llvm::Expected<size_t> Offset =
+      positionToOffset(*Content, Ref.Loc.range.start);
+  if (!Offset)
+    return Offset.takeError();
+  tooling::Replacement Replacement =
+      GetReplacement(FilePath, *Content, *Offset);
   if (Replacement.isApplicable())
     return addReplacement(FilePath, *Content, Replacement);
   return llvm::Error::success();
@@ -223,13 +215,5 @@ ScopifyEnum::addReplacement(StringRef FilePath, StringRef Content,
   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

[5chmidti]

@ckandeler while opening up your other changes in my editor, I wanted to see if some of the logic around Positions and Offsets could be simplified. It looks like clangd already implements some of the functionality in the SourceCode.h file. This shouldn't actually change the behavior, so I didn't add a release note.(And this is unrelated to your change, which is why I didn't suggest it there).