[LibTooling] Add function to translate and validate source range for editing

[ymandel]

Summary

Adds the function getRangeForEdit to validate that a given source range is
editable and, if needed, translate it into a range in the source file (for
example, if it's sourced in macro expansions).

[ymandel]

define matcher instead of operator==

[ilya-biryukov]

LGTM

clang/include/clang/Tooling/Refactoring/SourceCode.h

80

NIT: I can see why passing two structs instead of a single ASTContext is annoying, but still suggest adding an overload that accepts a SourceManager and LangOptions.
(The current overload would trivially call into the other one).

The reason is that ASTContext is not available where you would need this function.
In any case, having the ASTContext overload is convenient.

[ymandel]

Added overload.

[ymandel]

clang/include/clang/Tooling/Refactoring/SourceCode.h

80

good point!

clang/include/clang/Tooling/Refactoring/SourceCode.h

 ///   getExtendedText(*S2.getThen(), tok::TokenKind::semi, Context)
 ///     = getText(S2, Context) = "{ return 3; }"
 /// \endcode
 template <typename T>
 StringRef getExtendedText(const T &Node, tok::TokenKind Next,
                           ASTContext &Context) {
   return getText(getExtendedRange(Node, Next, Context), Context);
 }
+
+// Attempts to resolve the given range to one that can be edited by a rewrite;
+// generally, one that starts and ends within a particular file. It supports
+// a limited set of cases involving source locations in macro expansions.
+llvm::Optional<CharSourceRange>
+getRangeForEdit(const CharSourceRange &EditRange, const ASTContext &Context);
 } // namespace tooling
 } // namespace clang
 #endif // LLVM_CLANG_TOOLING_REFACTOR_SOURCE_CODE_H

clang/lib/Tooling/Refactoring/SourceCode.cpp

CharSourceRange clang::tooling::maybeExtendRange(CharSourceRange Range,
                                                  tok::TokenKind Next,
                                                  ASTContext &Context) {
   Optional<Token> Tok = Lexer::findNextToken(
       Range.getEnd(), Context.getSourceManager(), Context.getLangOpts());
   if (!Tok || !Tok->is(Next))
     return Range;
   return CharSourceRange::getTokenRange(Range.getBegin(), Tok->getLocation());
 }
+
+llvm::Optional<CharSourceRange>
+clang::tooling::getRangeForEdit(const CharSourceRange &EditRange,
+                                const ASTContext &Context) {
+  const SourceManager &SM = Context.getSourceManager();
+  // FIXME: makeFileCharRange() has the disadvantage of stripping off "identity"
+  // macros. For example, if we're looking to rewrite the int literal 3 to 6,
+  // and we have the following definition:
+  //    #define DO_NOTHING(x) x
+  // then
+  //    foo(DO_NOTHING(3))
+  // will be rewritten to
+  //    foo(6)
+  // rather than the arguably better
+  //    foo(DO_NOTHING(6))
+  // Decide whether the current behavior is desirable and modify if not.
+  CharSourceRange Range =
+      Lexer::makeFileCharRange(EditRange, SM, Context.getLangOpts());
+  if (Range.isInvalid())
+    return None;
+
+  if (Range.getBegin().isMacroID() || Range.getEnd().isMacroID())
+    return None;
+  if (SM.isInSystemHeader(Range.getBegin()) ||
+      SM.isInSystemHeader(Range.getEnd()))
+    return None;
+
+  std::pair<FileID, unsigned> BeginInfo = SM.getDecomposedLoc(Range.getBegin());
+  std::pair<FileID, unsigned> EndInfo = SM.getDecomposedLoc(Range.getEnd());
+  if (BeginInfo.first != EndInfo.first ||
+      BeginInfo.second > EndInfo.second)
+    return None;
+
+  return Range;
+}

clang/unittests/Tooling/SourceCodeTest.cpp

 //===- unittest/Tooling/SourceCodeTest.cpp --------------------------------===//
 //
 // 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 "clang/Tooling/Refactoring/SourceCode.h"
 #include "TestVisitor.h"
 #include "clang/Basic/Diagnostic.h"
-#include "clang/Tooling/Refactoring/SourceCode.h"
+#include "llvm/Testing/Support/Annotations.h"
+#include "llvm/Testing/Support/SupportHelpers.h"
 
 using namespace clang;
 
-using tooling::getText;
+using llvm::ValueIs;
 using tooling::getExtendedText;
+using tooling::getRangeForEdit;
+using tooling::getText;
+
+namespace clang {
+inline bool operator==(const CharSourceRange &LHS, const CharSourceRange &RHS) {
+  return LHS.isTokenRange() == RHS.isTokenRange() &&
+         LHS.getBegin() == RHS.getBegin() && LHS.getEnd() == RHS.getEnd();
+}
+}
 
 namespace {
 
+struct IntLitVisitor : TestVisitor<IntLitVisitor> {
+  bool VisitIntegerLiteral(IntegerLiteral *Expr) {
+    OnIntLit(Expr, Context);
+    return true;
+  }
+
+  std::function<void(IntegerLiteral *, ASTContext *Context)> OnIntLit;
+};
+
 struct CallsVisitor : TestVisitor<CallsVisitor> {
   bool VisitCallExpr(CallExpr *Expr) {
     OnCall(Expr, Context);
     return true;
   }
 
   std::function<void(CallExpr *, ASTContext *Context)> OnCall;
 };
 
+static CharSourceRange AsRange(const SourceManager &SM,
+                                         llvm::Annotations::Range R) {
+  return CharSourceRange::getCharRange(
+      SM.getLocForStartOfFile(SM.getMainFileID()).getLocWithOffset(R.Begin),
+      SM.getLocForStartOfFile(SM.getMainFileID()).getLocWithOffset(R.End));
+}
+
 TEST(SourceCodeTest, getText) {
   CallsVisitor Visitor;
 
   Visitor.OnCall = [](CallExpr *CE, ASTContext *Context) {
     EXPECT_EQ("foo(x, y)", getText(*CE, *Context));
   };
   Visitor.runOver("void foo(int x, int y) { foo(x, y); }");
 
TEST(SourceCodeTest, getExtendedText) {
   Visitor.OnCall = [](CallExpr *CE, ASTContext *Context) {
     EXPECT_EQ("foo()", getExtendedText(*CE, tok::TokenKind::semi, *Context));
   };
   Visitor.runOver("bool foo() { if (foo()) return true; return false; }");
   Visitor.runOver("void foo() { int x; for (;; foo()) ++x; }");
   Visitor.runOver("int foo() { return foo() + 3; }");
 }
 
+TEST(SourceCodeTest, EditRangeWithMacroExpansionsShouldSucceed) {
+  // The call expression, whose range we are extracting, includes two macro
+  // expansions.
+  llvm::Annotations Code(R"cpp(
+#define M(a) a * 13
+int foo(int x, int y);
+int a = $r[[foo(M(1), M(2))]];
+)cpp");
+
+  CallsVisitor Visitor;
+
+  Visitor.OnCall = [&Code](CallExpr *CE, ASTContext *Context) {
+    auto Range = CharSourceRange::getTokenRange(CE->getSourceRange());
+    EXPECT_THAT(getRangeForEdit(Range, *Context),
+                ValueIs(AsRange(Context->getSourceManager(), Code.range("r"))));
+  };
+  Visitor.runOver(Code.code());
+}
+
+TEST(SourceCodeTest, EditWholeMacroExpansionShouldSucceed) {
+  llvm::Annotations Code(R"cpp(
+#define FOO 10
+int a = $r[[FOO]];
+)cpp");
+
+  IntLitVisitor Visitor;
+  Visitor.OnIntLit = [&Code](IntegerLiteral *Expr, ASTContext *Context) {
+    auto Range = CharSourceRange::getTokenRange(Expr->getSourceRange());
+    EXPECT_THAT(getRangeForEdit(Range, *Context),
+                ValueIs(AsRange(Context->getSourceManager(), Code.range("r"))));
+  };
+  Visitor.runOver(Code.code());
+}
+
+TEST(SourceCodeTest, EditPartialMacroExpansionShouldFail) {
+  std::string Code = R"cpp(
+#define BAR 10+
+int c = BAR 3.0;
+)cpp";
+
+  IntLitVisitor Visitor;
+  Visitor.OnIntLit = [](IntegerLiteral *Expr, ASTContext *Context) {
+    auto Range = CharSourceRange::getTokenRange(Expr->getSourceRange());
+    EXPECT_FALSE(getRangeForEdit(Range, *Context).hasValue());
+  };
+  Visitor.runOver(Code);
+}
+
+TEST(SourceCodeTest, EditWholeMacroArgShouldSucceed) {
+  llvm::Annotations Code(R"cpp(
+#define FOO(a) a + 7.0;
+int a = FOO($r[[10]]);
+)cpp");
+
+  IntLitVisitor Visitor;
+  Visitor.OnIntLit = [&Code](IntegerLiteral *Expr, ASTContext *Context) {
+    auto Range = CharSourceRange::getTokenRange(Expr->getSourceRange());
+    EXPECT_THAT(getRangeForEdit(Range, *Context),
+                ValueIs(AsRange(Context->getSourceManager(), Code.range("r"))));
+  };
+  Visitor.runOver(Code.code());
+}
+
+TEST(SourceCodeTest, EditPartialMacroArgShouldSucceed) {
+  llvm::Annotations Code(R"cpp(
+#define FOO(a) a + 7.0;
+int a = FOO($r[[10]] + 10.0);
+)cpp");
+
+  IntLitVisitor Visitor;
+  Visitor.OnIntLit = [&Code](IntegerLiteral *Expr, ASTContext *Context) {
+    auto Range = CharSourceRange::getTokenRange(Expr->getSourceRange());
+    EXPECT_THAT(getRangeForEdit(Range, *Context),
+                ValueIs(AsRange(Context->getSourceManager(), Code.range("r"))));
+  };
+  Visitor.runOver(Code.code());
+}
+
 } // end anonymous namespace