r298913 - Added `applyAtomicChanges` function.

Eric Liu via cfe-commits cfe-commits at lists.llvm.org
Fri Apr 7 02:45:58 PDT 2017


Thanks Adrian!

I tried turning LLVM_ENABLE_LOCAL_SUBMODULE_VISIBILITY off but couldn't
reproduce the cycle on my local machine. Could you take a look at my patch
https://reviews.llvm.org/D30777? It includes clangFormat header from
clangToolingRefactor, and I couldn't see how the cycle was formed on the
GreenDragon bot (Clang_Tooling -> Clang_Format -> Clang_Tooling). Thanks!

On Thu, Mar 30, 2017 at 8:49 PM Adrian Prantl <aprantl at apple.com> wrote:

> Note that the green dragon bot doesn't use local submodule visibility, so
> every #include pulls in the entire clang module that header belongs to.
> Does this explain what you are seeing? (You should be able to reproduce
> with cmake -DLLVM_ENABLE_LOCAL_SUBMODULE_VISIBILITY=0)
>
> -- adrian
>
> On Mar 30, 2017, at 11:44 AM, Juergen Ributzka <juergen at ributzka.de>
> wrote:
>
> [+ Adrian]
>
> Adrian knows more about the bot setup.
>
> On Wed, Mar 29, 2017 at 8:04 AM, Eric Liu <ioeric at google.com> wrote:
>
> Hi Juergen, thanks for taking care of this, but I'm wondering if this
> build bot is using a different set of build rules? The error message says "Clang_Tooling
> -> Clang_Format -> Clang_Tooling"; however, the actual dependency is
> clangToolingRefactor -> clangFormat -> clangToolingCore, which seems fine
> for me. I guess the module build uses build rules with lower granularity.
>
> - Eric
>
> On Wed, Mar 29, 2017 at 2:39 AM Juergen Ributzka <juergen at ributzka.de>
> wrote:
>
> I reverted the commit in r298967. Please fix the cyclic dependency issue
> found here:
>
> http://lab.llvm.org:8080/green/job/clang-stage2-cmake-modulesRDA_build/4776/
>
> Cheers,
> Juergen
>
> On Tue, Mar 28, 2017 at 6:05 AM, Eric Liu via cfe-commits <
> cfe-commits at lists.llvm.org> wrote:
>
> Author: ioeric
> Date: Tue Mar 28 08:05:32 2017
> New Revision: 298913
>
> URL: http://llvm.org/viewvc/llvm-project?rev=298913&view=rev
> Log:
> Added `applyAtomicChanges` function.
>
> Summary: ... which applies a set of `AtomicChange`s on code.
>
> Reviewers: klimek, djasper
>
> Reviewed By: djasper
>
> Subscribers: cfe-commits
>
> Differential Revision: https://reviews.llvm.org/D30777
>
> Modified:
>     cfe/trunk/include/clang/Tooling/Refactoring/AtomicChange.h
>     cfe/trunk/lib/Tooling/Refactoring/AtomicChange.cpp
>     cfe/trunk/unittests/Tooling/RefactoringTest.cpp
>
> Modified: cfe/trunk/include/clang/Tooling/Refactoring/AtomicChange.h
> URL:
> http://llvm.org/viewvc/llvm-project/cfe/trunk/include/clang/Tooling/Refactoring/AtomicChange.h?rev=298913&r1=298912&r2=298913&view=diff
>
> ==============================================================================
> --- cfe/trunk/include/clang/Tooling/Refactoring/AtomicChange.h (original)
> +++ cfe/trunk/include/clang/Tooling/Refactoring/AtomicChange.h Tue Mar 28
> 08:05:32 2017
> @@ -16,6 +16,7 @@
>  #define LLVM_CLANG_TOOLING_REFACTOR_ATOMICCHANGE_H
>
>  #include "clang/Basic/SourceManager.h"
> +#include "clang/Format/Format.h"
>  #include "clang/Tooling/Core/Replacement.h"
>  #include "llvm/ADT/StringRef.h"
>  #include "llvm/Support/Error.h"
> @@ -123,6 +124,39 @@ private:
>    tooling::Replacements Replaces;
>  };
>
> +// Defines specs for applying changes.
> +struct ApplyChangesSpec {
> +  // If true, cleans up redundant/erroneous code around changed code with
> +  // clang-format's cleanup functionality, e.g. redundant commas around
> deleted
> +  // parameter or empty namespaces introduced by deletions.
> +  bool Cleanup = true;
> +
> +  format::FormatStyle Style = format::getNoStyle();
> +
> +  // Options for selectively formatting changes with clang-format:
> +  // kAll: Format all changed lines.
> +  // kNone: Don't format anything.
> +  // kViolations: Format lines exceeding the `ColumnLimit` in `Style`.
> +  enum FormatOption { kAll, kNone, kViolations };
> +
> +  FormatOption Format = kNone;
> +};
> +
> +/// \brief Applies all AtomicChanges in \p Changes to the \p Code.
> +///
> +/// This completely ignores the file path in each change and replaces
> them with
> +/// \p FilePath, i.e. callers are responsible for ensuring all changes
> are for
> +/// the same file.
> +///
> +/// \returns The changed code if all changes are applied successfully;
> +/// otherwise, an llvm::Error carrying llvm::StringError is returned (the
> Error
> +/// message can be converted to string with `llvm::toString()` and the
> +/// error_code should be ignored).
> +llvm::Expected<std::string>
> +applyAtomicChanges(llvm::StringRef FilePath, llvm::StringRef Code,
> +                   llvm::ArrayRef<AtomicChange> Changes,
> +                   const ApplyChangesSpec &Spec);
> +
>  } // end namespace tooling
>  } // end namespace clang
>
>
> Modified: cfe/trunk/lib/Tooling/Refactoring/AtomicChange.cpp
> URL:
> http://llvm.org/viewvc/llvm-project/cfe/trunk/lib/Tooling/Refactoring/AtomicChange.cpp?rev=298913&r1=298912&r2=298913&view=diff
>
> ==============================================================================
> --- cfe/trunk/lib/Tooling/Refactoring/AtomicChange.cpp (original)
> +++ cfe/trunk/lib/Tooling/Refactoring/AtomicChange.cpp Tue Mar 28 08:05:32
> 2017
> @@ -84,6 +84,116 @@ template <> struct MappingTraits<clang::
>
>  namespace clang {
>  namespace tooling {
> +namespace {
> +
> +// Returns true if there is any line that violates \p ColumnLimit in range
> +// [Start, End].
> +bool violatesColumnLimit(llvm::StringRef Code, unsigned ColumnLimit,
> +                         unsigned Start, unsigned End) {
> +  auto StartPos = Code.rfind('\n', Start);
> +  StartPos = (StartPos == llvm::StringRef::npos) ? 0 : StartPos + 1;
> +
> +  auto EndPos = Code.find("\n", End);
> +  if (EndPos == llvm::StringRef::npos)
> +    EndPos = Code.size();
> +
> +  llvm::SmallVector<llvm::StringRef, 8> Lines;
> +  Code.substr(StartPos, EndPos - StartPos).split(Lines, '\n');
> +  for (llvm::StringRef Line : Lines)
> +    if (Line.size() > ColumnLimit)
> +      return true;
> +  return false;
> +}
> +
> +std::vector<Range>
> +getRangesForFormating(llvm::StringRef Code, unsigned ColumnLimit,
> +                      ApplyChangesSpec::FormatOption Format,
> +                      const clang::tooling::Replacements &Replaces) {
> +  // kNone suppresses formatting entirely.
> +  if (Format == ApplyChangesSpec::kNone)
> +    return {};
> +  std::vector<clang::tooling::Range> Ranges;
> +  // This works assuming that replacements are ordered by offset.
> +  // FIXME: use `getAffectedRanges()` to calculate when it does not
> include '\n'
> +  // at the end of an insertion in affected ranges.
> +  int Offset = 0;
> +  for (const clang::tooling::Replacement &R : Replaces) {
> +    int Start = R.getOffset() + Offset;
> +    int End = Start + R.getReplacementText().size();
> +    if (!R.getReplacementText().empty() &&
> +        R.getReplacementText().back() == '\n' && R.getLength() == 0 &&
> +        R.getOffset() > 0 && R.getOffset() <= Code.size() &&
> +        Code[R.getOffset() - 1] == '\n')
> +      // If we are inserting at the start of a line and the replacement
> ends in
> +      // a newline, we don't need to format the subsequent line.
> +      --End;
> +    Offset += R.getReplacementText().size() - R.getLength();
> +
> +    if (Format == ApplyChangesSpec::kAll ||
> +        violatesColumnLimit(Code, ColumnLimit, Start, End))
> +      Ranges.emplace_back(Start, End - Start);
> +  }
> +  return Ranges;
> +}
> +
> +inline llvm::Error make_string_error(const llvm::Twine &Message) {
> +  return llvm::make_error<llvm::StringError>(Message,
> +
>  llvm::inconvertibleErrorCode());
> +}
> +
> +// Creates replacements for inserting/deleting #include headers.
> +llvm::Expected<Replacements>
> +createReplacementsForHeaders(llvm::StringRef FilePath, llvm::StringRef
> Code,
> +                             llvm::ArrayRef<AtomicChange> Changes,
> +                             const format::FormatStyle &Style) {
> +  // Create header insertion/deletion replacements to be cleaned up
> +  // (i.e. converted to real insertion/deletion replacements).
> +  Replacements HeaderReplacements;
> +  for (const auto &Change : Changes) {
> +    for (llvm::StringRef Header : Change.getInsertedHeaders()) {
> +      std::string EscapedHeader =
> +          Header.startswith("<") || Header.startswith("\"")
> +              ? Header.str()
> +              : ("\"" + Header + "\"").str();
> +      std::string ReplacementText = "#include " + EscapedHeader;
> +      // Offset UINT_MAX and length 0 indicate that the replacement is a
> header
> +      // insertion.
> +      llvm::Error Err = HeaderReplacements.add(
> +          tooling::Replacement(FilePath, UINT_MAX, 0, ReplacementText));
> +      if (Err)
> +        return std::move(Err);
> +    }
> +    for (const std::string &Header : Change.getRemovedHeaders()) {
> +      // Offset UINT_MAX and length 1 indicate that the replacement is a
> header
> +      // deletion.
> +      llvm::Error Err =
> +          HeaderReplacements.add(Replacement(FilePath, UINT_MAX, 1,
> Header));
> +      if (Err)
> +        return std::move(Err);
> +    }
> +  }
> +
> +  // cleanupAroundReplacements() converts header insertions/deletions into
> +  // actual replacements that add/remove headers at the right location.
> +  return clang::format::cleanupAroundReplacements(Code,
> HeaderReplacements,
> +                                                  Style);
> +}
> +
> +// Combine replacements in all Changes as a `Replacements`. This ignores
> the
> +// file path in all replacements and replaces them with \p FilePath.
> +llvm::Expected<Replacements>
> +combineReplacementsInChanges(llvm::StringRef FilePath,
> +                             llvm::ArrayRef<AtomicChange> Changes) {
> +  Replacements Replaces;
> +  for (const auto &Change : Changes)
> +    for (const auto &R : Change.getReplacements())
> +      if (auto Err = Replaces.add(Replacement(
> +              FilePath, R.getOffset(), R.getLength(),
> R.getReplacementText())))
> +        return std::move(Err);
> +  return Replaces;
> +}
> +
> +} // end namespace
>
>  AtomicChange::AtomicChange(const SourceManager &SM,
>                             SourceLocation KeyPosition) {
> @@ -168,5 +278,74 @@ void AtomicChange::removeHeader(llvm::St
>    RemovedHeaders.push_back(Header);
>  }
>
> +llvm::Expected<std::string>
> +applyAtomicChanges(llvm::StringRef FilePath, llvm::StringRef Code,
> +                   llvm::ArrayRef<AtomicChange> Changes,
> +                   const ApplyChangesSpec &Spec) {
> +  llvm::Expected<Replacements> HeaderReplacements =
> +      createReplacementsForHeaders(FilePath, Code, Changes, Spec.Style);
> +  if (!HeaderReplacements)
> +    return make_string_error(
> +        "Failed to create replacements for header changes: " +
> +        llvm::toString(HeaderReplacements.takeError()));
> +
> +  llvm::Expected<Replacements> Replaces =
> +      combineReplacementsInChanges(FilePath, Changes);
> +  if (!Replaces)
> +    return make_string_error("Failed to combine replacements in all
> changes: " +
> +                             llvm::toString(Replaces.takeError()));
> +
> +  Replacements AllReplaces = std::move(*Replaces);
> +  for (const auto &R : *HeaderReplacements) {
> +    llvm::Error Err = AllReplaces.add(R);
> +    if (Err)
> +      return make_string_error(
> +          "Failed to combine existing replacements with header
> replacements: " +
> +          llvm::toString(std::move(Err)));
> +  }
> +
> +  if (Spec.Cleanup) {
> +    llvm::Expected<Replacements> CleanReplaces =
> +        format::cleanupAroundReplacements(Code, AllReplaces, Spec.Style);
> +    if (!CleanReplaces)
> +      return make_string_error("Failed to cleanup around replacements: " +
> +                               llvm::toString(CleanReplaces.takeError()));
> +    AllReplaces = std::move(*CleanReplaces);
> +  }
> +
> +  // Apply all replacements.
> +  llvm::Expected<std::string> ChangedCode =
> +      applyAllReplacements(Code, AllReplaces);
> +  if (!ChangedCode)
> +    return make_string_error("Failed to apply all replacements: " +
> +                             llvm::toString(ChangedCode.takeError()));
> +
> +  // Sort inserted headers. This is done even if other formatting is
> turned off
> +  // as incorrectly sorted headers are always just wrong, it's not a
> matter of
> +  // taste.
> +  Replacements HeaderSortingReplacements = format::sortIncludes(
> +      Spec.Style, *ChangedCode, AllReplaces.getAffectedRanges(),
> FilePath);
> +  ChangedCode = applyAllReplacements(*ChangedCode,
> HeaderSortingReplacements);
> +  if (!ChangedCode)
> +    return make_string_error(
> +        "Failed to apply replacements for sorting includes: " +
> +        llvm::toString(ChangedCode.takeError()));
> +
> +  AllReplaces = AllReplaces.merge(HeaderSortingReplacements);
> +
> +  std::vector<Range> FormatRanges = getRangesForFormating(
> +      *ChangedCode, Spec.Style.ColumnLimit, Spec.Format, AllReplaces);
> +  if (!FormatRanges.empty()) {
> +    Replacements FormatReplacements =
> +        format::reformat(Spec.Style, *ChangedCode, FormatRanges,
> FilePath);
> +    ChangedCode = applyAllReplacements(*ChangedCode, FormatReplacements);
> +    if (!ChangedCode)
> +      return make_string_error(
> +          "Failed to apply replacements for formatting changed code: " +
> +          llvm::toString(ChangedCode.takeError()));
> +  }
> +  return ChangedCode;
> +}
> +
>  } // end namespace tooling
>  } // end namespace clang
>
> Modified: cfe/trunk/unittests/Tooling/RefactoringTest.cpp
> URL:
> http://llvm.org/viewvc/llvm-project/cfe/trunk/unittests/Tooling/RefactoringTest.cpp?rev=298913&r1=298912&r2=298913&view=diff
>
> ==============================================================================
> --- cfe/trunk/unittests/Tooling/RefactoringTest.cpp (original)
> +++ cfe/trunk/unittests/Tooling/RefactoringTest.cpp Tue Mar 28 08:05:32
> 2017
> @@ -1290,5 +1290,435 @@ TEST_F(AtomicChangeTest, InsertAfterWith
>        Replacement(Context.Sources, SourceLocation(), 0, "b")));
>  }
>
> +class ApplyAtomicChangesTest : public ::testing::Test {
> +protected:
> +  ApplyAtomicChangesTest() : FilePath("file.cc") {
> +    Spec.Cleanup = true;
> +    Spec.Format = ApplyChangesSpec::kAll;
> +    Spec.Style = format::getLLVMStyle();
> +  }
> +
> +  ~ApplyAtomicChangesTest() override {}
> +
> +  void setInput(llvm::StringRef Input) {
> +    Code = Input;
> +    FID = Context.createInMemoryFile(FilePath, Code);
> +  }
> +
> +  SourceLocation getLoc(unsigned Offset) const {
> +    return
> Context.Sources.getLocForStartOfFile(FID).getLocWithOffset(Offset);
> +  }
> +
> +  AtomicChange replacementToAtomicChange(llvm::StringRef Key, unsigned
> Offset,
> +                                         unsigned Length,
> +                                         llvm::StringRef Text) {
> +    AtomicChange Change(FilePath, Key);
> +    llvm::Error Err =
> +        Change.replace(Context.Sources, getLoc(Offset), Length, Text);
> +    EXPECT_FALSE(Err);
> +    return Change;
> +  }
> +
> +  std::string rewrite(bool FailureExpected = false) {
> +    llvm::Expected<std::string> ChangedCode =
> +        applyAtomicChanges(FilePath, Code, Changes, Spec);
> +    EXPECT_EQ(FailureExpected, !ChangedCode);
> +    if (!ChangedCode) {
> +      llvm::errs() << "Failed to apply changes: "
> +                   << llvm::toString(ChangedCode.takeError()) << "\n";
> +      return "";
> +    }
> +    return *ChangedCode;
> +  }
> +
> +  RewriterTestContext Context;
> +  FileID FID;
> +  ApplyChangesSpec Spec;
> +  std::string Code;
> +  std::string FilePath;
> +  llvm::SmallVector<AtomicChange, 8> Changes;
> +};
> +
> +TEST_F(ApplyAtomicChangesTest, BasicRefactoring) {
> +  setInput("int a;");
> +  AtomicChange Change(FilePath, "key1");
> +  Changes.push_back(replacementToAtomicChange("key1", 4, 1, "b"));
> +  EXPECT_EQ("int b;", rewrite());
> +}
> +
> +TEST_F(ApplyAtomicChangesTest, SeveralRefactorings) {
> +  setInput("int a;\n"
> +           "int b;");
> +  Changes.push_back(replacementToAtomicChange("key1", 0, 3, "float"));
> +  Changes.push_back(replacementToAtomicChange("key2", 4, 1, "f"));
> +  Changes.push_back(replacementToAtomicChange("key3", 11, 1, "g"));
> +  Changes.push_back(replacementToAtomicChange("key4", 7, 3, "float"));
> +  EXPECT_EQ("float f;\n"
> +            "float g;",
> +            rewrite());
> +}
> +
> +TEST_F(ApplyAtomicChangesTest, IgnorePathsInRefactorings) {
> +  setInput("int a;\n"
> +           "int b;");
> +  Changes.push_back(replacementToAtomicChange("key1", 4, 1, "aa"));
> +
> +  FileID ID = Context.createInMemoryFile("AnotherFile", "12345678912345");
> +  Changes.emplace_back("AnotherFile", "key2");
> +  auto Err = Changes.back().replace(
> +      Context.Sources,
> +      Context.Sources.getLocForStartOfFile(ID).getLocWithOffset(11), 1,
> "bb");
> +  ASSERT_TRUE(!Err);
> +  EXPECT_EQ("int aa;\n"
> +            "int bb;",
> +            rewrite());
> +}
> +
> +TEST_F(ApplyAtomicChangesTest, AppliesDuplicateInsertions) {
> +  setInput("int a;");
> +  Changes.push_back(replacementToAtomicChange("key1", 5, 0, "b"));
> +  Changes.push_back(replacementToAtomicChange("key2", 5, 0, "b"));
> +  EXPECT_EQ("int abb;", rewrite());
> +}
> +
> +TEST_F(ApplyAtomicChangesTest, BailsOnOverlappingRefactorings) {
> +  setInput("int a;");
> +  Changes.push_back(replacementToAtomicChange("key1", 0, 5, "float f"));
> +  Changes.push_back(replacementToAtomicChange("key2", 4, 1, "b"));
> +  EXPECT_EQ("", rewrite(/*FailureExpected=*/true));
> +}
> +
> +TEST_F(ApplyAtomicChangesTest, BasicReformatting) {
> +  setInput("int  a;");
> +  Changes.push_back(replacementToAtomicChange("key1", 5, 1, "b"));
> +  EXPECT_EQ("int b;", rewrite());
> +}
> +
> +TEST_F(ApplyAtomicChangesTest, OnlyFormatWhenViolateColumnLimits) {
> +  Spec.Format = ApplyChangesSpec::kViolations;
> +  Spec.Style.ColumnLimit = 8;
> +  setInput("int  a;\n"
> +           "int    a;\n"
> +           "int  aaaaaaaa;\n");
> +  Changes.push_back(replacementToAtomicChange("key1", 5, 1, "x"));
> +  Changes.push_back(replacementToAtomicChange("key2", 15, 1, "x"));
> +  Changes.push_back(replacementToAtomicChange("key3", 23, 8, "xx"));
> +  EXPECT_EQ("int  x;\n"
> +            "int x;\n"
> +            "int  xx;\n",
> +            rewrite());
> +}
> +
> +TEST_F(ApplyAtomicChangesTest, LastLineViolateColumnLimits) {
> +  Spec.Format = ApplyChangesSpec::kViolations;
> +  Spec.Style.ColumnLimit = 8;
> +  setInput("int  a;\n"
> +           "int    a;");
> +  Changes.push_back(replacementToAtomicChange("key1", 0, 1, "i"));
> +  Changes.push_back(replacementToAtomicChange("key2", 15, 2, "y;"));
> +  EXPECT_EQ("int  a;\n"
> +            "int y;",
> +            rewrite());
> +}
> +
> +TEST_F(ApplyAtomicChangesTest, LastLineWithNewlineViolateColumnLimits) {
> +  Spec.Format = ApplyChangesSpec::kViolations;
> +  Spec.Style.ColumnLimit = 8;
> +  setInput("int  a;\n"
> +           "int   a;\n");
> +  Changes.push_back(replacementToAtomicChange("key1", 0, 1, "i"));
> +  Changes.push_back(replacementToAtomicChange("key2", 14, 3, "y;\n"));
> +  EXPECT_EQ("int  a;\n"
> +            "int   y;\n",
> +            rewrite());
> +}
> +
> +TEST_F(ApplyAtomicChangesTest, Longer) {
> +  setInput("int  a;");
> +  Changes.push_back(replacementToAtomicChange("key1", 5, 1, "bbb"));
> +  EXPECT_EQ("int bbb;", rewrite());
> +}
> +
> +TEST_F(ApplyAtomicChangesTest, Shorter) {
> +  setInput("int  aaa;");
> +  Changes.push_back(replacementToAtomicChange("key1", 5, 3, "b"));
> +  EXPECT_EQ("int b;", rewrite());
> +}
> +
> +TEST_F(ApplyAtomicChangesTest, OnlyFormatChangedLines) {
> +  setInput("int  aaa;\n"
> +           "int a = b;\n"
> +           "int  bbb;");
> +  Changes.push_back(replacementToAtomicChange("key1", 14, 1, "b"));
> +  EXPECT_EQ("int  aaa;\n"
> +            "int b = b;\n"
> +            "int  bbb;",
> +            rewrite());
> +}
> +
> +TEST_F(ApplyAtomicChangesTest, DisableFormatting) {
> +  Spec.Format = ApplyChangesSpec::kNone;
> +  setInput("int  aaa;\n"
> +           "int a   = b;\n"
> +           "int  bbb;");
> +  Changes.push_back(replacementToAtomicChange("key1", 14, 1, "b"));
> +  EXPECT_EQ("int  aaa;\n"
> +            "int b   = b;\n"
> +            "int  bbb;",
> +            rewrite());
> +}
> +
> +TEST_F(ApplyAtomicChangesTest, AdaptsToLocalPointerStyle) {
> +  setInput("int *aaa;\n"
> +           "int *bbb;");
> +  Changes.push_back(replacementToAtomicChange("key1", 0, 0, "int*
> ccc;\n"));
> +  EXPECT_EQ("int *ccc;\n"
> +            "int *aaa;\n"
> +            "int *bbb;",
> +            rewrite());
> +}
> +
> +TEST_F(ApplyAtomicChangesTest, AcceptsSurroundingFormatting) {
> +  setInput("   int  aaa;\n"
> +           "   int a = b;\n"
> +           "   int  bbb;");
> +  Changes.push_back(replacementToAtomicChange("key1", 20, 1, "b"));
> +  EXPECT_EQ("   int  aaa;\n"
> +            "   int b = b;\n"
> +            "   int  bbb;",
> +            rewrite());
> +}
> +
> +TEST_F(ApplyAtomicChangesTest, BailsOutOnConflictingChanges) {
> +  setInput("int c;\n"
> +           "int f;");
> +  // Insertions at the same offset are only allowed in the same
> AtomicChange.
> +  Changes.push_back(replacementToAtomicChange("key1", 0, 0, "int a;\n"));
> +  Changes.push_back(replacementToAtomicChange("key2", 0, 0, "int b;\n"));
> +  EXPECT_EQ("", rewrite(/*FailureExpected=*/true));
> +}
> +
> +TEST_F(ApplyAtomicChangesTest, InsertsNewIncludesInRightOrder) {
> +  setInput("int a;");
> +  Changes.emplace_back(FilePath, "key1");
> +  Changes.back().addHeader("b");
> +  Changes.back().addHeader("c");
> +  Changes.emplace_back(FilePath, "key2");
> +  Changes.back().addHeader("a");
> +  EXPECT_EQ("#include \"a\"\n"
> +            "#include \"b\"\n"
> +            "#include \"c\"\n"
> +            "int a;",
> +            rewrite());
> +}
> +
> +TEST_F(ApplyAtomicChangesTest, RemoveAndSortIncludes) {
> +  setInput(R"(
> +#include "a"
> +#include "b"
> +#include "c"
> +
> +int a;
> +      )");
> +  Changes.emplace_back(FilePath, "key1");
> +  Changes.back().removeHeader("b");
> +  EXPECT_EQ(R"(
> +#include "a"
> +#include "c"
> +
> +int a;
> +      )",
> +            rewrite());
> +}
> +TEST_F(ApplyAtomicChangesTest, InsertsSystemIncludes) {
> +  setInput("#include <asys>\n"
> +           "#include <csys>\n"
> +           "\n"
> +           "#include \"a\"\n"
> +           "#include \"c\"\n");
> +  Changes.emplace_back(FilePath, "key1");
> +  Changes.back().addHeader("<asys>"); // Already exists.
> +  Changes.back().addHeader("<b>");
> +  Changes.back().addHeader("<d>");
> +  Changes.back().addHeader("\"b-already-escaped\"");
> +  EXPECT_EQ("#include <asys>\n"
> +            "#include <b>\n"
> +            "#include <csys>\n"
> +            "#include <d>\n"
> +            "\n"
> +            "#include \"a\"\n"
> +            "#include \"b-already-escaped\"\n"
> +            "#include \"c\"\n",
> +            rewrite());
> +}
> +
> +TEST_F(ApplyAtomicChangesTest, RemoveSystemIncludes) {
> +  setInput(R"(
> +#include <a>
> +#include <b>
> +
> +#include "c"
> +
> +int a;
> +      )");
> +  Changes.emplace_back(FilePath, "key1");
> +  Changes.back().removeHeader("<a>");
> +  EXPECT_EQ(R"(
> +#include <b>
> +
> +#include "c"
> +
> +int a;
> +      )",
> +            rewrite());
> +}
> +
> +TEST_F(ApplyAtomicChangesTest,
> +       DoNotFormatFollowingLinesIfSeparatedWithNewline) {
> +  setInput("#ifndef __H__\n"
> +           "#define __H__\n"
> +           "#include \"b\"\n"
> +           "\n"
> +           "int  a;\n"
> +           "int  a;\n"
> +           "int  a;\n"
> +           "#endif // __H__\n");
> +  Changes.push_back(replacementToAtomicChange("key1",
> +                                              llvm::StringRef("#ifndef
> __H__\n"
> +                                                              "#define
> __H__\n"
> +                                                              "\n"
> +                                                              "#include
> \"b\"\n"
> +                                                              "int  a;\n"
> +                                                              "int  ")
> +                                                  .size(),
> +                                              1, "b"));
> +  Changes.back().addHeader("a");
> +  EXPECT_EQ("#ifndef __H__\n"
> +            "#define __H__\n"
> +            "#include \"a\"\n"
> +            "#include \"b\"\n"
> +            "\n"
> +            "int  a;\n"
> +            "int b;\n"
> +            "int  a;\n"
> +            "#endif // __H__\n",
> +            rewrite());
> +}
> +
> +TEST_F(ApplyAtomicChangesTest, FormatsCorrectLineWhenHeaderIsRemoved) {
> +  setInput("#include \"a\"\n"
> +           "\n"
> +           "int  a;\n"
> +           "int  a;\n"
> +           "int  a;");
> +  Changes.push_back(replacementToAtomicChange("key1", 27, 1, "b"));
> +  Changes.back().removeHeader("a");
> +  EXPECT_EQ("\n"
> +            "int  a;\n"
> +            "int b;\n"
> +            "int  a;",
> +            rewrite());
> +}
> +
> +TEST_F(ApplyAtomicChangesTest, CleansUpCtorInitializers) {
> +  setInput("A::A() : a(), b() {}\n"
> +           "A::A() : a(), b() {}\n"
> +           "A::A() : a(), b() {}\n"
> +           "A::A() : a()/**/, b() {}\n"
> +           "A::A() : a()  ,// \n"
> +           "   /**/    b()    {}");
> +  Changes.emplace_back(FilePath, "key1");
> +  auto Err = Changes.back().replace(Context.Sources, getLoc(9), 3, "");
> +  ASSERT_TRUE(!Err);
> +  Err = Changes.back().replace(Context.Sources, getLoc(35), 3, "");
> +  ASSERT_TRUE(!Err);
> +  Err = Changes.back().replace(Context.Sources, getLoc(51), 3, "");
> +  ASSERT_TRUE(!Err);
> +  Err = Changes.back().replace(Context.Sources, getLoc(56), 3, "");
> +  ASSERT_TRUE(!Err);
> +  Err = Changes.back().replace(Context.Sources, getLoc(72), 3, "");
> +  ASSERT_TRUE(!Err);
> +  Err = Changes.back().replace(Context.Sources, getLoc(97), 3, "");
> +  ASSERT_TRUE(!Err);
> +  Err = Changes.back().replace(Context.Sources, getLoc(118), 3, "");
> +  ASSERT_TRUE(!Err);
> +  EXPECT_EQ("A::A() : b() {}\n"
> +            "A::A() : a() {}\n"
> +            "A::A() {}\n"
> +            "A::A() : b() {}\n"
> +            "A::A() {}",
> +            rewrite());
> +}
> +
> +TEST_F(ApplyAtomicChangesTest, CleansUpParameterLists) {
> +  setInput("void f(int i, float f, string s);\n"
> +           "f(1, 2.0f, \"a\");\n"
> +           "g(1, 1);");
> +  Changes.emplace_back(FilePath, "key1");
> +  auto Err = Changes.back().replace(Context.Sources, getLoc(7), 5, "");
> +  ASSERT_TRUE(!Err);
> +  Err = Changes.back().replace(Context.Sources, getLoc(23), 8, "");
> +  ASSERT_TRUE(!Err);
> +  Err = Changes.back().replace(Context.Sources, getLoc(36), 1, "");
> +  ASSERT_TRUE(!Err);
> +  Err = Changes.back().replace(Context.Sources, getLoc(45), 3, "");
> +  ASSERT_TRUE(!Err);
> +  Err = Changes.back().replace(Context.Sources, getLoc(53), 1, "");
> +  ASSERT_TRUE(!Err);
> +  Err = Changes.back().replace(Context.Sources, getLoc(56), 1, "");
> +  ASSERT_TRUE(!Err);
> +  EXPECT_EQ("void f(float f);\n"
> +            "f(2.0f);\n"
> +            "g();",
> +            rewrite());
> +}
> +
> +TEST_F(ApplyAtomicChangesTest, DisableCleanup) {
> +  Spec.Cleanup = false;
> +  setInput("void f(int i, float f, string s);\n"
> +           "f(1, 2.0f, \"a\");\n"
> +           "g(1, 1);");
> +  Changes.emplace_back(FilePath, "key1");
> +  auto Err = Changes.back().replace(Context.Sources, getLoc(7), 5, "");
> +  ASSERT_TRUE(!Err);
> +  Err = Changes.back().replace(Context.Sources, getLoc(23), 8, "");
> +  ASSERT_TRUE(!Err);
> +  Err = Changes.back().replace(Context.Sources, getLoc(36), 1, "");
> +  ASSERT_TRUE(!Err);
> +  Err = Changes.back().replace(Context.Sources, getLoc(45), 3, "");
> +  ASSERT_TRUE(!Err);
> +  Err = Changes.back().replace(Context.Sources, getLoc(53), 1, "");
> +  ASSERT_TRUE(!Err);
> +  Err = Changes.back().replace(Context.Sources, getLoc(56), 1, "");
> +  ASSERT_TRUE(!Err);
> +  EXPECT_EQ("void f(, float f, );\n"
> +            "f(, 2.0f, );\n"
> +            "g(, );",
> +            rewrite());
> +}
> +
> +TEST_F(ApplyAtomicChangesTest, EverythingDeleted) {
> +  setInput("int a;");
> +  Changes.push_back(replacementToAtomicChange("key1", 0, 6, ""));
> +  EXPECT_EQ("", rewrite());
> +}
> +
> +TEST_F(ApplyAtomicChangesTest, DoesNotDeleteInserts) {
> +  setInput("int a;\n"
> +           "int b;");
> +  Changes.emplace_back(FilePath, "key1");
> +  auto Err = Changes.back().replace(Context.Sources, getLoc(4), 1, "");
> +  ASSERT_TRUE(!Err);
> +  Err = Changes.back().replace(Context.Sources, getLoc(4), 0, "b");
> +  ASSERT_TRUE(!Err);
> +  Err = Changes.back().replace(Context.Sources, getLoc(11), 0, "a");
> +  ASSERT_TRUE(!Err);
> +  Err = Changes.back().replace(Context.Sources, getLoc(11), 1, "");
> +  ASSERT_TRUE(!Err);
> +  EXPECT_EQ("int b;\n"
> +            "int a;",
> +            rewrite());
> +}
> +
>  } // end namespace tooling
>  } // end namespace clang
>
>
> _______________________________________________
> cfe-commits mailing list
> cfe-commits at lists.llvm.org
> http://lists.llvm.org/cgi-bin/mailman/listinfo/cfe-commits
>
>
>
>
>
-------------- next part --------------
An HTML attachment was scrubbed...
URL: <http://lists.llvm.org/pipermail/cfe-commits/attachments/20170407/bb729a05/attachment-0001.html>


More information about the cfe-commits mailing list