[clang-tools-extra] r352796 - [clangd] A code action to swap branches of an if statement
Ilya Biryukov via cfe-commits
cfe-commits at lists.llvm.org
Thu Jan 31 13:30:05 PST 2019
Author: ibiryukov
Date: Thu Jan 31 13:30:05 2019
New Revision: 352796
URL: http://llvm.org/viewvc/llvm-project?rev=352796&view=rev
Log:
[clangd] A code action to swap branches of an if statement
Reviewers: sammccall
Reviewed By: sammccall
Subscribers: llvm-commits, mgorny, ioeric, MaskRay, jkorous, arphaman, kadircet, cfe-commits
Tags: #clang
Differential Revision: https://reviews.llvm.org/D56611
Added:
clang-tools-extra/trunk/clangd/refactor/tweaks/SwapIfBranches.cpp
clang-tools-extra/trunk/unittests/clangd/TweakTests.cpp
Removed:
clang-tools-extra/trunk/clangd/refactor/tweaks/Dummy.cpp
Modified:
clang-tools-extra/trunk/clangd/SourceCode.cpp
clang-tools-extra/trunk/clangd/SourceCode.h
clang-tools-extra/trunk/clangd/refactor/tweaks/CMakeLists.txt
clang-tools-extra/trunk/unittests/clangd/CMakeLists.txt
Modified: clang-tools-extra/trunk/clangd/SourceCode.cpp
URL: http://llvm.org/viewvc/llvm-project/clang-tools-extra/trunk/clangd/SourceCode.cpp?rev=352796&r1=352795&r2=352796&view=diff
==============================================================================
--- clang-tools-extra/trunk/clangd/SourceCode.cpp (original)
+++ clang-tools-extra/trunk/clangd/SourceCode.cpp Thu Jan 31 13:30:05 2019
@@ -11,6 +11,8 @@
#include "clang/AST/ASTContext.h"
#include "clang/Basic/SourceManager.h"
#include "clang/Lex/Lexer.h"
+#include "llvm/ADT/None.h"
+#include "llvm/ADT/StringRef.h"
#include "llvm/Support/Errc.h"
#include "llvm/Support/Error.h"
#include "llvm/Support/Path.h"
@@ -141,6 +143,69 @@ Position sourceLocToPosition(const Sourc
return P;
}
+bool isValidFileRange(const SourceManager &Mgr, SourceRange R) {
+ if (!R.getBegin().isValid() || !R.getEnd().isValid())
+ return false;
+
+ FileID BeginFID;
+ size_t BeginOffset = 0;
+ std::tie(BeginFID, BeginOffset) = Mgr.getDecomposedLoc(R.getBegin());
+
+ FileID EndFID;
+ size_t EndOffset = 0;
+ std::tie(EndFID, EndOffset) = Mgr.getDecomposedLoc(R.getEnd());
+
+ return BeginFID.isValid() && BeginFID == EndFID && BeginOffset <= EndOffset;
+}
+
+bool halfOpenRangeContains(const SourceManager &Mgr, SourceRange R,
+ SourceLocation L) {
+ assert(isValidFileRange(Mgr, R));
+
+ FileID BeginFID;
+ size_t BeginOffset = 0;
+ std::tie(BeginFID, BeginOffset) = Mgr.getDecomposedLoc(R.getBegin());
+ size_t EndOffset = Mgr.getFileOffset(R.getEnd());
+
+ FileID LFid;
+ size_t LOffset;
+ std::tie(LFid, LOffset) = Mgr.getDecomposedLoc(L);
+ return BeginFID == LFid && BeginOffset <= LOffset && LOffset < EndOffset;
+}
+
+bool halfOpenRangeTouches(const SourceManager &Mgr, SourceRange R,
+ SourceLocation L) {
+ return L == R.getEnd() || halfOpenRangeContains(Mgr, R, L);
+}
+
+llvm::Optional<SourceRange> toHalfOpenFileRange(const SourceManager &Mgr,
+ const LangOptions &LangOpts,
+ SourceRange R) {
+ auto Begin = Mgr.getFileLoc(R.getBegin());
+ if (Begin.isInvalid())
+ return llvm::None;
+ auto End = Mgr.getFileLoc(R.getEnd());
+ if (End.isInvalid())
+ return llvm::None;
+ End = Lexer::getLocForEndOfToken(End, 0, Mgr, LangOpts);
+
+ SourceRange Result(Begin, End);
+ if (!isValidFileRange(Mgr, Result))
+ return llvm::None;
+ return Result;
+}
+
+llvm::StringRef toSourceCode(const SourceManager &SM, SourceRange R) {
+ assert(isValidFileRange(SM, R));
+ bool Invalid = false;
+ auto *Buf = SM.getBuffer(SM.getFileID(R.getBegin()), &Invalid);
+ assert(!Invalid);
+
+ size_t BeginOffset = SM.getFileOffset(R.getBegin());
+ size_t EndOffset = SM.getFileOffset(R.getEnd());
+ return Buf->getBuffer().substr(BeginOffset, EndOffset - BeginOffset);
+}
+
llvm::Expected<SourceLocation> sourceLocationInMainFile(const SourceManager &SM,
Position P) {
llvm::StringRef Code = SM.getBuffer(SM.getMainFileID())->getBuffer();
@@ -169,8 +234,7 @@ std::pair<size_t, size_t> offsetToClangL
return {Lines + 1, Offset - StartOfLine + 1};
}
-std::pair<llvm::StringRef, llvm::StringRef>
-splitQualifiedName(llvm::StringRef QName) {
+std::pair<StringRef, StringRef> splitQualifiedName(StringRef QName) {
size_t Pos = QName.rfind("::");
if (Pos == llvm::StringRef::npos)
return {llvm::StringRef(), QName};
Modified: clang-tools-extra/trunk/clangd/SourceCode.h
URL: http://llvm.org/viewvc/llvm-project/clang-tools-extra/trunk/clangd/SourceCode.h?rev=352796&r1=352795&r2=352796&view=diff
==============================================================================
--- clang-tools-extra/trunk/clangd/SourceCode.h (original)
+++ clang-tools-extra/trunk/clangd/SourceCode.h Thu Jan 31 13:30:05 2019
@@ -14,6 +14,7 @@
#define LLVM_CLANG_TOOLS_EXTRA_CLANGD_SOURCECODE_H
#include "Protocol.h"
#include "clang/Basic/Diagnostic.h"
+#include "clang/Basic/LangOptions.h"
#include "clang/Basic/SourceLocation.h"
#include "clang/Basic/SourceManager.h"
#include "clang/Format/Format.h"
@@ -61,6 +62,46 @@ Position sourceLocToPosition(const Sourc
llvm::Expected<SourceLocation> sourceLocationInMainFile(const SourceManager &SM,
Position P);
+/// Turns a token range into a half-open range and checks its correctness.
+/// The resulting range will have only valid source location on both sides, both
+/// of which are file locations.
+///
+/// File locations always point to a particular offset in a file, i.e. they
+/// never refer to a location inside a macro expansion. Turning locations from
+/// macro expansions into file locations is ambiguous - one can use
+/// SourceManager::{getExpansion|getFile|getSpelling}Loc. This function
+/// calls SourceManager::getFileLoc on both ends of \p R to do the conversion.
+///
+/// User input (e.g. cursor position) is expressed as a file location, so this
+/// function can be viewed as a way to normalize the ranges used in the clang
+/// AST so that they are comparable with ranges coming from the user input.
+llvm::Optional<SourceRange> toHalfOpenFileRange(const SourceManager &Mgr,
+ const LangOptions &LangOpts,
+ SourceRange R);
+
+/// Returns true iff all of the following conditions hold:
+/// - start and end locations are valid,
+/// - start and end locations are file locations from the same file
+/// (i.e. expansion locations are not taken into account).
+/// - start offset <= end offset.
+/// FIXME: introduce a type for source range with this invariant.
+bool isValidFileRange(const SourceManager &Mgr, SourceRange R);
+
+/// Returns true iff \p L is contained in \p R.
+/// EXPECTS: isValidFileRange(R) == true, L is a file location.
+bool halfOpenRangeContains(const SourceManager &Mgr, SourceRange R,
+ SourceLocation L);
+
+/// Returns true iff \p L is contained in \p R or \p L is equal to the end point
+/// of \p R.
+/// EXPECTS: isValidFileRange(R) == true, L is a file location.
+bool halfOpenRangeTouches(const SourceManager &Mgr, SourceRange R,
+ SourceLocation L);
+
+/// Returns the source code covered by the source range.
+/// EXPECTS: isValidFileRange(R) == true.
+llvm::StringRef toSourceCode(const SourceManager &SM, SourceRange R);
+
// Converts a half-open clang source range to an LSP range.
// Note that clang also uses closed source ranges, which this can't handle!
Range halfOpenToRange(const SourceManager &SM, CharSourceRange R);
Modified: clang-tools-extra/trunk/clangd/refactor/tweaks/CMakeLists.txt
URL: http://llvm.org/viewvc/llvm-project/clang-tools-extra/trunk/clangd/refactor/tweaks/CMakeLists.txt?rev=352796&r1=352795&r2=352796&view=diff
==============================================================================
--- clang-tools-extra/trunk/clangd/refactor/tweaks/CMakeLists.txt (original)
+++ clang-tools-extra/trunk/clangd/refactor/tweaks/CMakeLists.txt Thu Jan 31 13:30:05 2019
@@ -8,6 +8,5 @@ include_directories(${CMAKE_CURRENT_SOUR
# $<TARGET_OBJECTS:obj.clangDaemonTweaks> to a list of sources, see
# clangd/tool/CMakeLists.txt for an example.
add_clang_library(clangDaemonTweaks OBJECT
- Dummy.cpp # FIXME: to avoid CMake errors due to empty inputs, remove when a
- # first tweak lands.
+ SwapIfBranches.cpp
)
Removed: clang-tools-extra/trunk/clangd/refactor/tweaks/Dummy.cpp
URL: http://llvm.org/viewvc/llvm-project/clang-tools-extra/trunk/clangd/refactor/tweaks/Dummy.cpp?rev=352795&view=auto
==============================================================================
--- clang-tools-extra/trunk/clangd/refactor/tweaks/Dummy.cpp (original)
+++ clang-tools-extra/trunk/clangd/refactor/tweaks/Dummy.cpp (removed)
@@ -1,9 +0,0 @@
-//===--- Dummy.cpp -----------------------------------------------*- C++-*-===//
-//
-// The LLVM Compiler Infrastructure
-//
-// This file is distributed under the University of Illinois Open Source
-// License. See LICENSE.TXT for details.
-//
-//===----------------------------------------------------------------------===//
-// Does nothing, only here to avoid cmake errors for empty libraries.
\ No newline at end of file
Added: clang-tools-extra/trunk/clangd/refactor/tweaks/SwapIfBranches.cpp
URL: http://llvm.org/viewvc/llvm-project/clang-tools-extra/trunk/clangd/refactor/tweaks/SwapIfBranches.cpp?rev=352796&view=auto
==============================================================================
--- clang-tools-extra/trunk/clangd/refactor/tweaks/SwapIfBranches.cpp (added)
+++ clang-tools-extra/trunk/clangd/refactor/tweaks/SwapIfBranches.cpp Thu Jan 31 13:30:05 2019
@@ -0,0 +1,132 @@
+//===--- SwapIfBranches.cpp --------------------------------------*- C++-*-===//
+//
+// 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 "ClangdUnit.h"
+#include "Logger.h"
+#include "SourceCode.h"
+#include "refactor/Tweak.h"
+#include "clang/AST/ASTContext.h"
+#include "clang/AST/RecursiveASTVisitor.h"
+#include "clang/AST/Stmt.h"
+#include "clang/Basic/LangOptions.h"
+#include "clang/Basic/SourceLocation.h"
+#include "clang/Basic/SourceManager.h"
+#include "clang/Lex/Lexer.h"
+#include "clang/Tooling/Core/Replacement.h"
+#include "llvm/ADT/None.h"
+#include "llvm/ADT/Optional.h"
+#include "llvm/ADT/StringRef.h"
+#include "llvm/Support/Casting.h"
+#include "llvm/Support/Error.h"
+
+namespace clang {
+namespace clangd {
+namespace {
+/// Swaps the 'then' and 'else' branch of the if statement.
+/// Before:
+/// if (foo) { return 10; } else { continue; }
+/// ^^^^^^^ ^^^^
+/// After:
+/// if (foo) { continue; } else { return 10; }
+class SwapIfBranches : public Tweak {
+public:
+ TweakID id() const override final;
+
+ bool prepare(const Selection &Inputs) override;
+ Expected<tooling::Replacements> apply(const Selection &Inputs) override;
+ std::string title() const override;
+
+private:
+ IfStmt *If = nullptr;
+};
+
+REGISTER_TWEAK(SwapIfBranches);
+
+class FindIfUnderCursor : public RecursiveASTVisitor<FindIfUnderCursor> {
+public:
+ FindIfUnderCursor(ASTContext &Ctx, SourceLocation CursorLoc, IfStmt *&Result)
+ : Ctx(Ctx), CursorLoc(CursorLoc), Result(Result) {}
+
+ bool VisitIfStmt(IfStmt *If) {
+ // Check if the cursor is in the range of 'if (cond)'.
+ // FIXME: this does not contain the closing paren, add it too.
+ auto R = toHalfOpenFileRange(
+ Ctx.getSourceManager(), Ctx.getLangOpts(),
+ SourceRange(If->getIfLoc(), If->getCond()->getEndLoc().isValid()
+ ? If->getCond()->getEndLoc()
+ : If->getIfLoc()));
+ if (R && halfOpenRangeTouches(Ctx.getSourceManager(), *R, CursorLoc)) {
+ Result = If;
+ return false;
+ }
+ // Check the range of 'else'.
+ R = toHalfOpenFileRange(Ctx.getSourceManager(), Ctx.getLangOpts(),
+ SourceRange(If->getElseLoc()));
+ if (R && halfOpenRangeTouches(Ctx.getSourceManager(), *R, CursorLoc)) {
+ Result = If;
+ return false;
+ }
+
+ return true;
+ }
+
+private:
+ ASTContext &Ctx;
+ SourceLocation CursorLoc;
+ IfStmt *&Result;
+};
+} // namespace
+
+bool SwapIfBranches::prepare(const Selection &Inputs) {
+ auto &Ctx = Inputs.AST.getASTContext();
+ FindIfUnderCursor(Ctx, Inputs.Cursor, If).TraverseAST(Ctx);
+ if (!If)
+ return false;
+
+ // avoid dealing with single-statement brances, they require careful handling
+ // to avoid changing semantics of the code (i.e. dangling else).
+ if (!If->getThen() || !llvm::isa<CompoundStmt>(If->getThen()) ||
+ !If->getElse() || !llvm::isa<CompoundStmt>(If->getElse()))
+ return false;
+ return true;
+}
+
+Expected<tooling::Replacements> SwapIfBranches::apply(const Selection &Inputs) {
+ auto &Ctx = Inputs.AST.getASTContext();
+ auto &SrcMgr = Ctx.getSourceManager();
+
+ auto ThenRng = toHalfOpenFileRange(SrcMgr, Ctx.getLangOpts(),
+ If->getThen()->getSourceRange());
+ if (!ThenRng)
+ return llvm::createStringError(
+ llvm::inconvertibleErrorCode(),
+ "Could not obtain range of the 'then' branch. Macros?");
+ auto ElseRng = toHalfOpenFileRange(SrcMgr, Ctx.getLangOpts(),
+ If->getElse()->getSourceRange());
+ if (!ElseRng)
+ return llvm::createStringError(
+ llvm::inconvertibleErrorCode(),
+ "Could not obtain range of the 'else' branch. Macros?");
+
+ auto ThenCode = toSourceCode(SrcMgr, *ThenRng);
+ auto ElseCode = toSourceCode(SrcMgr, *ElseRng);
+
+ tooling::Replacements Result;
+ if (auto Err = Result.add(tooling::Replacement(Ctx.getSourceManager(),
+ ThenRng->getBegin(),
+ ThenCode.size(), ElseCode)))
+ return std::move(Err);
+ if (auto Err = Result.add(tooling::Replacement(Ctx.getSourceManager(),
+ ElseRng->getBegin(),
+ ElseCode.size(), ThenCode)))
+ return std::move(Err);
+ return Result;
+}
+
+std::string SwapIfBranches::title() const { return "Swap if branches"; }
+} // namespace clangd
+} // namespace clang
Modified: clang-tools-extra/trunk/unittests/clangd/CMakeLists.txt
URL: http://llvm.org/viewvc/llvm-project/clang-tools-extra/trunk/unittests/clangd/CMakeLists.txt?rev=352796&r1=352795&r2=352796&view=diff
==============================================================================
--- clang-tools-extra/trunk/unittests/clangd/CMakeLists.txt (original)
+++ clang-tools-extra/trunk/unittests/clangd/CMakeLists.txt Thu Jan 31 13:30:05 2019
@@ -45,8 +45,11 @@ add_extra_unittest(ClangdTests
TestTU.cpp
ThreadingTests.cpp
TraceTests.cpp
+ TweakTests.cpp
URITests.cpp
XRefsTests.cpp
+
+ $<TARGET_OBJECTS:obj.clangDaemonTweaks>
)
target_link_libraries(ClangdTests
Added: clang-tools-extra/trunk/unittests/clangd/TweakTests.cpp
URL: http://llvm.org/viewvc/llvm-project/clang-tools-extra/trunk/unittests/clangd/TweakTests.cpp?rev=352796&view=auto
==============================================================================
--- clang-tools-extra/trunk/unittests/clangd/TweakTests.cpp (added)
+++ clang-tools-extra/trunk/unittests/clangd/TweakTests.cpp Thu Jan 31 13:30:05 2019
@@ -0,0 +1,158 @@
+//===-- TweakTests.cpp ------------------------------------------*- C++ -*-===//
+//
+// The LLVM Compiler Infrastructure
+//
+// This file is distributed under the University of Illinois Open Source
+// License. See LICENSE.TXT for details.
+//
+//===----------------------------------------------------------------------===//
+
+#include "Annotations.h"
+#include "SourceCode.h"
+#include "TestTU.h"
+#include "refactor/Tweak.h"
+#include "clang/AST/Expr.h"
+#include "clang/Rewrite/Core/Rewriter.h"
+#include "clang/Tooling/Core/Replacement.h"
+#include "llvm/ADT/StringRef.h"
+#include "llvm/Support/Error.h"
+#include "llvm/Testing/Support/Error.h"
+#include "gmock/gmock.h"
+#include "gtest/gtest.h"
+#include <cassert>
+
+using llvm::Failed;
+using llvm::HasValue;
+using llvm::Succeeded;
+using ::testing::IsEmpty;
+using ::testing::Not;
+
+namespace clang {
+namespace clangd {
+namespace {
+
+std::string markRange(llvm::StringRef Code, Range R) {
+ size_t Begin = llvm::cantFail(positionToOffset(Code, R.start));
+ size_t End = llvm::cantFail(positionToOffset(Code, R.end));
+ assert(Begin <= End);
+ if (Begin == End) // Mark a single point.
+ return (Code.substr(0, Begin) + "^" + Code.substr(Begin)).str();
+ // Mark a range.
+ return (Code.substr(0, Begin) + "[[" + Code.substr(Begin, End - Begin) +
+ "]]" + Code.substr(End))
+ .str();
+}
+
+void checkAvailable(TweakID ID, llvm::StringRef Input, bool Available) {
+ Annotations Code(Input);
+ ASSERT_TRUE(0 < Code.points().size() || 0 < Code.ranges().size())
+ << "no points of interest specified";
+ TestTU TU;
+ TU.Filename = "foo.cpp";
+ TU.Code = Code.code();
+
+ ParsedAST AST = TU.build();
+
+ auto CheckOver = [&](Range Selection) {
+ auto CursorLoc = llvm::cantFail(sourceLocationInMainFile(
+ AST.getASTContext().getSourceManager(), Selection.start));
+ auto T = prepareTweak(ID, Tweak::Selection{Code.code(), AST, CursorLoc});
+ if (Available)
+ EXPECT_THAT_EXPECTED(T, Succeeded())
+ << "code is " << markRange(Code.code(), Selection);
+ else
+ EXPECT_THAT_EXPECTED(T, Failed())
+ << "code is " << markRange(Code.code(), Selection);
+ };
+ for (auto P : Code.points())
+ CheckOver(Range{P, P});
+ for (auto R : Code.ranges())
+ CheckOver(R);
+}
+
+/// Checks action is available at every point and range marked in \p Input.
+void checkAvailable(TweakID ID, llvm::StringRef Input) {
+ return checkAvailable(ID, Input, /*Available=*/true);
+}
+
+/// Same as checkAvailable, but checks the action is not available.
+void checkNotAvailable(TweakID ID, llvm::StringRef Input) {
+ return checkAvailable(ID, Input, /*Available=*/false);
+}
+llvm::Expected<std::string> apply(TweakID ID, llvm::StringRef Input) {
+ Annotations Code(Input);
+ Range SelectionRng;
+ if (Code.points().size() != 0) {
+ assert(Code.ranges().size() == 0 &&
+ "both a cursor point and a selection range were specified");
+ SelectionRng = Range{Code.point(), Code.point()};
+ } else {
+ SelectionRng = Code.range();
+ }
+ TestTU TU;
+ TU.Filename = "foo.cpp";
+ TU.Code = Code.code();
+
+ ParsedAST AST = TU.build();
+ auto CursorLoc = llvm::cantFail(sourceLocationInMainFile(
+ AST.getASTContext().getSourceManager(), SelectionRng.start));
+ Tweak::Selection S = {Code.code(), AST, CursorLoc};
+
+ auto T = prepareTweak(ID, S);
+ if (!T)
+ return T.takeError();
+ auto Replacements = (*T)->apply(S);
+ if (!Replacements)
+ return Replacements.takeError();
+ return applyAllReplacements(Code.code(), *Replacements);
+}
+
+void checkTransform(llvm::StringRef ID, llvm::StringRef Input,
+ llvm::StringRef Output) {
+ EXPECT_THAT_EXPECTED(apply(ID, Input), HasValue(Output))
+ << "action id is" << ID;
+}
+
+TEST(TweakTest, SwapIfBranches) {
+ llvm::StringLiteral ID = "SwapIfBranches";
+
+ checkAvailable(ID, R"cpp(
+ void test() {
+ ^i^f^^(^t^r^u^e^) { return 100; } ^e^l^s^e^ { continue; }
+ }
+ )cpp");
+
+ checkNotAvailable(ID, R"cpp(
+ void test() {
+ if (true) {^return ^100;^ } else { ^continue^;^ }
+ }
+ )cpp");
+
+ llvm::StringLiteral Input = R"cpp(
+ void test() {
+ ^if (true) { return 100; } else { continue; }
+ }
+ )cpp";
+ llvm::StringLiteral Output = R"cpp(
+ void test() {
+ if (true) { continue; } else { return 100; }
+ }
+ )cpp";
+ checkTransform(ID, Input, Output);
+
+ Input = R"cpp(
+ void test() {
+ ^if () { return 100; } else { continue; }
+ }
+ )cpp";
+ Output = R"cpp(
+ void test() {
+ if () { continue; } else { return 100; }
+ }
+ )cpp";
+ checkTransform(ID, Input, Output);
+}
+
+} // namespace
+} // namespace clangd
+} // namespace clang
More information about the cfe-commits
mailing list