ajohnson-uoregon created this revision.
ajohnson-uoregon added reviewers: jdoerfert, rsmith, klimek.
ajohnson-uoregon requested review of this revision.
Herald added a project: clang.
Herald added a subscriber: cfe-commits.
...-rewriterope/3970
[clang][Rewriter] split test for replace after insert to be more understandable
and also a fix to make the test correct
Repository:
rG LLVM Github Monorepo
https://reviews.llvm.org/D107503
Files:
clang/include/clang/Rewrite/Core/Rewriter.h
clang/lib/Rewrite/Rewriter.cpp
clang/unittests/Rewrite/RewriterTest.cpp
Index: clang/unittests/Rewrite/RewriterTest.cpp
===================================================================
--- clang/unittests/Rewrite/RewriterTest.cpp
+++ clang/unittests/Rewrite/RewriterTest.cpp
@@ -62,6 +62,7 @@
// Check that correct text is replaced for each range type. Ranges remain in
// terms of the original text but include the new text.
StringRef Code = "int main(int argc, char *argv[]) { return argc; }";
+ // foogc; }
// replace char range with "foo" ^~
// get ^~~~~ = "foogc;"
// replace token range with "bar" ^~++
@@ -77,4 +78,54 @@
EXPECT_EQ(T.Rewrite.getRewrittenText(T.makeCharRange(42, 47)), "0;");
}
+TEST(Rewriter, ReplaceAfterInsertNoOpts) {
+ // Check that remove/insert after insert behaves correctly based on the
+ // RewriteOptions passed. This should erase extra text.
+
+ StringRef code = "int main(int argc, char *argv[]) { return argc; } double "
+ "f(int text_to_erase) {}";
+ // insert "int answer = 42;" ^
+ // replace with 42 ^
+ // extra text to erase ^
+ std::unique_ptr<ASTUnit> AST = tooling::buildASTFromCode(code);
+ ASTContext &C = AST->getASTContext();
+ Rewriter Rewrite = Rewriter(C.getSourceManager(), C.getLangOpts());
+ SourceLocation FileStart = AST->getStartOfMainFileID();
+ SourceLocation insert_point = FileStart.getLocWithOffset(34);
+ Rewrite.InsertText(insert_point, "int answer = 42;");
+ SourceRange replace_range = SourceRange(FileStart.getLocWithOffset(34),
+ FileStart.getLocWithOffset(45));
+ Rewrite.ReplaceText(replace_range, "42;");
+ EXPECT_EQ(
+ Rewrite.getRewrittenText(SourceRange(FileStart.getLocWithOffset(33),
+ FileStart.getLocWithOffset(65))),
+ "{int answer = 42;42; text_to_erase");
+}
+
+TEST(Rewriter, ReplaceAfterInsertOpts) {
+ // Check that remove/insert after insert behaves correctly based on the
+ // RewriteOptions passed. This should NOT erase extra text.
+
+ StringRef code = "int main(int argc, char *argv[]) { return argc; } double "
+ "f(int text_to_erase) {}";
+ // insert "int answer = 42;" ^
+ // replace with 42 ^
+ // extra text to erase ^
+ std::unique_ptr<ASTUnit> AST = tooling::buildASTFromCode(code);
+ ASTContext &C = AST->getASTContext();
+ Rewriter Rewrite = Rewriter(C.getSourceManager(), C.getLangOpts());
+ SourceLocation FileStart = AST->getStartOfMainFileID();
+ SourceLocation insert_point = FileStart.getLocWithOffset(34);
+ Rewrite.InsertText(insert_point, "int answer = 42;");
+ SourceRange replace_range = SourceRange(FileStart.getLocWithOffset(34),
+ FileStart.getLocWithOffset(46));
+ Rewriter::RewriteOptions opts;
+ opts.IncludeInsertsAtBeginOfRange = false;
+ Rewrite.ReplaceText(replace_range, "42;", opts);
+ EXPECT_EQ(
+ Rewrite.getRewrittenText(SourceRange(FileStart.getLocWithOffset(33),
+ FileStart.getLocWithOffset(58))),
+ "{int answer = 42;42; } double f(");
+}
+
} // anonymous namespace
Index: clang/lib/Rewrite/Rewriter.cpp
===================================================================
--- clang/lib/Rewrite/Rewriter.cpp
+++ clang/lib/Rewrite/Rewriter.cpp
@@ -327,13 +327,13 @@
return false;
}
-bool Rewriter::ReplaceText(SourceRange range, SourceRange replacementRange) {
+bool Rewriter::ReplaceText(SourceRange range, SourceRange replacementRange, RewriteOptions opts) {
if (!isRewritable(range.getBegin())) return true;
if (!isRewritable(range.getEnd())) return true;
if (replacementRange.isInvalid()) return true;
SourceLocation start = range.getBegin();
- unsigned origLength = getRangeSize(range);
- unsigned newLength = getRangeSize(replacementRange);
+ unsigned origLength = getRangeSize(range, opts);
+ unsigned newLength = getRangeSize(replacementRange, opts);
FileID FID;
unsigned newOffs = getLocationOffsetAndFileID(replacementRange.getBegin(),
FID);
Index: clang/include/clang/Rewrite/Core/Rewriter.h
===================================================================
--- clang/include/clang/Rewrite/Core/Rewriter.h
+++ clang/include/clang/Rewrite/Core/Rewriter.h
@@ -160,22 +160,31 @@
/// ReplaceText - This method replaces a range of characters in the input
/// buffer with a new string. This is effectively a combined "remove/insert"
- /// operation.
- bool ReplaceText(CharSourceRange range, StringRef NewStr) {
- return ReplaceText(range.getBegin(), getRangeSize(range), NewStr);
+ /// operation. To ensure correct computation of the size of the range to
+ /// replace, pass RewriteOptions with IncludeInsertsAtBeginOfRange = false
+ /// (unless different behavior is desired).
+ bool ReplaceText(CharSourceRange range, StringRef NewStr,
+ RewriteOptions opts = RewriteOptions()) {
+ return ReplaceText(range.getBegin(), getRangeSize(range, opts), NewStr);
}
/// ReplaceText - This method replaces a range of characters in the input
/// buffer with a new string. This is effectively a combined "remove/insert"
- /// operation.
- bool ReplaceText(SourceRange range, StringRef NewStr) {
- return ReplaceText(range.getBegin(), getRangeSize(range), NewStr);
+ /// operation. To ensure correct computation of the size of the range to
+ /// replace, pass RewriteOptions with IncludeInsertsAtBeginOfRange = false
+ /// (unless different behavior is desired).
+ bool ReplaceText(SourceRange range, StringRef NewStr,
+ RewriteOptions opts = RewriteOptions()) {
+ return ReplaceText(range.getBegin(), getRangeSize(range, opts), NewStr);
}
/// ReplaceText - This method replaces a range of characters in the input
/// buffer with a new string. This is effectively a combined "remove/insert"
- /// operation.
- bool ReplaceText(SourceRange range, SourceRange replacementRange);
+ /// operation. To ensure correct computation of the size of the ranges,
+ /// pass RewriteOptions with IncludeInsertsAtBeginOfRange = false (unless
+ /// different behavior is desired).
+ bool ReplaceText(SourceRange range, SourceRange replacementRange,
+ RewriteOptions opts = RewriteOptions());
/// Increase indentation for the lines between the given source range.
/// To determine what the indentation should be, 'parentIndent' is used
_______________________________________________
cfe-commits mailing list
cfe-commits@lists.llvm.org
https://lists.llvm.org/cgi-bin/mailman/listinfo/cfe-commits
