hokein created this revision.
hokein added a reviewer: ilya-biryukov.
Herald added subscribers: usaxena95, kadircet, arphaman, jkorous, MaskRay.
Herald added a project: clang.
We used to scan the code everytime when computing the LSP position to the offset
(respect the LSP encoding).
Repository:
rG LLVM Github Monorepo
https://reviews.llvm.org/D70441
Files:
clang-tools-extra/clangd/SourceCode.cpp
clang-tools-extra/clangd/SourceCode.h
clang-tools-extra/clangd/refactor/Rename.cpp
clang-tools-extra/clangd/refactor/Rename.h
clang-tools-extra/clangd/unittests/RenameTests.cpp
clang-tools-extra/clangd/unittests/SourceCodeTests.cpp
Index: clang-tools-extra/clangd/unittests/SourceCodeTests.cpp
===================================================================
--- clang-tools-extra/clangd/unittests/SourceCodeTests.cpp
+++ clang-tools-extra/clangd/unittests/SourceCodeTests.cpp
@@ -77,6 +77,17 @@
EXPECT_EQ(lspLength("😂"), 1UL);
}
+TEST(SourceCodeTests, byteOffset) {
+ // UTF-16 encoding.
+ EXPECT_THAT_EXPECTED(byteOffset("", 0UL), llvm::HasValue(0));
+ EXPECT_THAT_EXPECTED(byteOffset("ascii", 5UL), llvm::HasValue(5));
+ // BMP
+ EXPECT_THAT_EXPECTED(byteOffset("↓", 1UL), llvm::HasValue(3));
+ EXPECT_THAT_EXPECTED(byteOffset("Â¥", 1UL), llvm::HasValue(2));
+ // astral
+ EXPECT_THAT_EXPECTED(byteOffset("😂", 2UL), llvm::HasValue(4));
+}
+
// The = → 🡆 below are ASCII (1 byte), BMP (3 bytes), and astral (4 bytes).
const char File[] = R"(0:0 = 0
1:0 → 8
Index: clang-tools-extra/clangd/unittests/RenameTests.cpp
===================================================================
--- clang-tools-extra/clangd/unittests/RenameTests.cpp
+++ clang-tools-extra/clangd/unittests/RenameTests.cpp
@@ -638,6 +638,21 @@
UnorderedElementsAre(Pair(Eq(Path), Eq(expectedResult(Code, NewName)))));
}
+TEST(CrossFileRenameTests, BuildRenameEdits) {
+ Annotations Code("[[😂]]");
+ auto LSPRange = Code.range();
+ auto Edit = buildRenameEdit(Code.code(), {LSPRange}, "abc");
+ ASSERT_TRUE(bool(Edit));
+ ASSERT_EQ(1UL, Edit->Replacements.size());
+ EXPECT_EQ(4UL, Edit->Replacements.begin()->getLength());
+
+ LSPRange.end = {-1, 100}; // out of range
+ Edit = buildRenameEdit(Code.code(), {LSPRange}, "abc");
+ EXPECT_FALSE(Edit);
+ EXPECT_THAT(llvm::toString(Edit.takeError()),
+ testing::HasSubstr("out of range"));
+}
+
} // namespace
} // namespace clangd
} // namespace clang
Index: clang-tools-extra/clangd/refactor/Rename.h
===================================================================
--- clang-tools-extra/clangd/refactor/Rename.h
+++ clang-tools-extra/clangd/refactor/Rename.h
@@ -47,6 +47,13 @@
/// in another file (per the index).
llvm::Expected<FileEdits> rename(const RenameInputs &RInputs);
+/// Generates rename edits that replaces all given occurreneces with the
+/// NewName.
+/// Exposed for testing only.
+llvm::Expected<Edit> buildRenameEdit(llvm::StringRef InitialCode,
+ const std::vector<Range> &Occurrences,
+ llvm::StringRef NewName);
+
} // namespace clangd
} // namespace clang
Index: clang-tools-extra/clangd/refactor/Rename.cpp
===================================================================
--- clang-tools-extra/clangd/refactor/Rename.cpp
+++ clang-tools-extra/clangd/refactor/Rename.cpp
@@ -11,6 +11,7 @@
#include "FindTarget.h"
#include "Logger.h"
#include "ParsedAST.h"
+#include "Protocol.h"
#include "Selection.h"
#include "SourceCode.h"
#include "index/SymbolCollector.h"
@@ -19,6 +20,7 @@
#include "clang/Basic/SourceLocation.h"
#include "clang/Tooling/Refactoring/Rename/USRFindingAction.h"
#include "llvm/ADT/STLExtras.h"
+#include "llvm/Support/Errc.h"
#include "llvm/Support/Error.h"
namespace clang {
@@ -297,34 +299,6 @@
return AffectedFiles;
}
-llvm::Expected<std::pair<size_t, size_t>> toRangeOffset(const clangd::Range &R,
- llvm::StringRef Code) {
- auto StartOffset = positionToOffset(Code, R.start);
- if (!StartOffset)
- return StartOffset.takeError();
- auto EndOffset = positionToOffset(Code, R.end);
- if (!EndOffset)
- return EndOffset.takeError();
- return std::make_pair(*StartOffset, *EndOffset);
-};
-
-llvm::Expected<Edit> buildRenameEdit(llvm::StringRef InitialCode,
- const std::vector<Range> &Occurrences,
- llvm::StringRef NewName) {
- tooling::Replacements RenameEdit;
- for (const Range &Occurrence : Occurrences) {
- // FIXME: !positionToOffset is O(N), optimize it.
- auto RangeOffset = toRangeOffset(Occurrence, InitialCode);
- if (!RangeOffset)
- return RangeOffset.takeError();
- auto ByteLength = RangeOffset->second - RangeOffset->first;
- if (auto Err = RenameEdit.add(tooling::Replacement(
- InitialCode, RangeOffset->first, ByteLength, NewName)))
- return std::move(Err);
- }
- return Edit(InitialCode, std::move(RenameEdit));
-}
-
// Index-based rename, it renames all occurrences outside of the main file.
//
// The cross-file rename is purely based on the index, as we don't want to
@@ -366,7 +340,6 @@
elog("Fail to read file content: {0}", AffectedFileCode.takeError());
continue;
}
-
auto RenameEdit = buildRenameEdit(*AffectedFileCode,
FileAndOccurrences.getValue(), NewName);
if (!RenameEdit)
@@ -465,5 +438,65 @@
return Results;
}
+llvm::Expected<Edit> buildRenameEdit(llvm::StringRef InitialCode,
+ const std::vector<Range> &Occurrences,
+ llvm::StringRef NewName) {
+ struct Line {
+ llvm::StringRef Content; // withiout trailing '\n'.
+ size_t StartOffset; // the offset where the line starts
+ };
+ std::vector<Line> LineTable; // index is the line number (0-based).
+ // Build the line table.
+ llvm::StringRef CurrentLine, Rest = InitialCode;
+ size_t Offset = 0;
+ while (!Rest.empty()) {
+ std::tie(CurrentLine, Rest) = Rest.split('\n');
+ LineTable.push_back({CurrentLine, Offset});
+ Offset += CurrentLine.size() + 1;
+ }
+ // Helper functions for converting to offset.
+ auto PositionOffset =
+ [&LineTable](const Position &P) -> llvm::Expected<size_t> {
+ if (P.line < 0 || P.line >= (int)LineTable.size()) {
+ return llvm::make_error<llvm::StringError>(
+ llvm::formatv("Line value is out of range ({0})", P.line),
+ llvm::errc::invalid_argument);
+ }
+ if (P.character < 0) {
+ return llvm::make_error<llvm::StringError>(
+ llvm::formatv("Character value is out of range ({0})", P.character),
+ llvm::errc::invalid_argument);
+ }
+ const auto &CurrentLine = LineTable[P.line];
+ auto OffsetInLine = byteOffset(CurrentLine.Content, P.character);
+ if (!OffsetInLine)
+ return OffsetInLine.takeError();
+ return CurrentLine.StartOffset + *OffsetInLine;
+ };
+ auto ToRangeOffset =
+ [PositionOffset](
+ const Range &R) -> llvm::Expected<std::pair<size_t, size_t>> {
+ auto StartOffset = PositionOffset(R.start);
+ if (!StartOffset)
+ return StartOffset.takeError();
+ auto EndOffset = PositionOffset(R.end);
+ if (!EndOffset)
+ return EndOffset.takeError();
+ return std::make_pair(*StartOffset, *EndOffset);
+ };
+
+ tooling::Replacements RenameEdit;
+ for (const Range &Occurrence : Occurrences) {
+ auto ROffset = ToRangeOffset(Occurrence);
+ if (!ROffset)
+ return ROffset.takeError();
+ auto ByteLength = ROffset->second - ROffset->first;
+ if (auto Err = RenameEdit.add(tooling::Replacement(
+ InitialCode, ROffset->first, ByteLength, NewName)))
+ return std::move(Err);
+ }
+ return Edit(InitialCode, std::move(RenameEdit));
+}
+
} // namespace clangd
} // namespace clang
Index: clang-tools-extra/clangd/SourceCode.h
===================================================================
--- clang-tools-extra/clangd/SourceCode.h
+++ clang-tools-extra/clangd/SourceCode.h
@@ -49,6 +49,10 @@
// Use of UTF-16 may be overridden by kCurrentOffsetEncoding.
size_t lspLength(StringRef Code);
+/// Converts the LSP column character (in UTF-16 code units) to the byte offset.
+/// Use of UTF-16 may be overridden by kCurrentOffsetEncoding.
+llvm::Expected<size_t> byteOffset(StringRef LineCode, int LSPCharacter);
+
/// Turn a [line, column] pair into an offset in Code.
///
/// If P.character exceeds the line length, returns the offset at end-of-line.
Index: clang-tools-extra/clangd/SourceCode.cpp
===================================================================
--- clang-tools-extra/clangd/SourceCode.cpp
+++ clang-tools-extra/clangd/SourceCode.cpp
@@ -152,6 +152,18 @@
return Count;
}
+llvm::Expected<size_t> byteOffset(llvm::StringRef LineCode, int LSPCharacter) {
+ bool Valid = false;
+ size_t ByteInLine =
+ measureUnits(LineCode, LSPCharacter, lspEncoding(), Valid);
+ if (!Valid)
+ return llvm::make_error<llvm::StringError>(
+ llvm::formatv("{0} offset {1} is invalid for code: {2}", lspEncoding(),
+ LSPCharacter, LineCode),
+ llvm::errc::invalid_argument);
+ return ByteInLine;
+}
+
llvm::Expected<size_t> positionToOffset(llvm::StringRef Code, Position P,
bool AllowColumnsBeyondLineLength) {
if (P.line < 0)
_______________________________________________
cfe-commits mailing list
cfe-commits@lists.llvm.org
https://lists.llvm.org/cgi-bin/mailman/listinfo/cfe-commits
