barcisz updated this revision to Diff 461955.
barcisz added a comment.
Tests for the utility
Repository:
rG LLVM Github Monorepo
CHANGES SINCE LAST ACTION
https://reviews.llvm.org/D133942/new/
https://reviews.llvm.org/D133942
Files:
clang-tools-extra/clang-tidy/utils/FixItHintUtils.cpp
clang-tools-extra/clang-tidy/utils/FixItHintUtils.h
clang-tools-extra/unittests/clang-tidy/CMakeLists.txt
clang-tools-extra/unittests/clang-tidy/FixItHintUtilsTest.cpp
Index: clang-tools-extra/unittests/clang-tidy/FixItHintUtilsTest.cpp
===================================================================
--- /dev/null
+++ clang-tools-extra/unittests/clang-tidy/FixItHintUtilsTest.cpp
@@ -0,0 +1,101 @@
+#include "utils/FixItHintUtils.h"
+#include "ClangTidyCheck.h"
+#include "ClangTidyTest.h"
+#include "utils/LexerUtils.h"
+#include "gtest/gtest.h"
+
+#define REGISTER_TEST_MATCHER(TestSuiteName, MatcherType) \
+ class TestSuiteName : public ClangTidyCheck { \
+ public: \
+ TestSuiteName(llvm::StringRef Name, \
+ clang::tidy::ClangTidyContext *Context) \
+ : ClangTidyCheck(Name, Context) {} \
+ void registerMatchers(clang::ast_matchers::MatchFinder *Finder) override { \
+ Finder->addMatcher(getMatcher(), this); \
+ } \
+ void check( \
+ const clang::ast_matchers::MatchFinder::MatchResult &Result) override; \
+ \
+ private: \
+ clang::ast_matchers::internal::Matcher<MatcherType> getMatcher(); \
+ }; \
+ clang::ast_matchers::internal::Matcher<MatcherType> \
+ TestSuiteName::getMatcher()
+
+#define CHECK_TEST_MATCHER(TestSuiteName) \
+ void TestSuiteName::check( \
+ const clang::ast_matchers::MatchFinder::MatchResult &Result)
+
+#define RUN_TEST_MATCHER(TestSuiteName, TestName, SourceCode, TargetCode) \
+ TEST(TestSuiteName, TestName) { \
+ EXPECT_EQ(TargetCode, runCheckOnCode<TestSuiteName>(SourceCode)); \
+ }
+
+namespace clang {
+namespace tidy {
+namespace test {
+
+using namespace ast_matchers;
+
+REGISTER_TEST_MATCHER(AddSubsequentStatementUtil, Stmt) {
+ return stmt(forEach(callExpr().bind("call"))).bind("parent");
+}
+
+CHECK_TEST_MATCHER(AddSubsequentStatementUtil) {
+ const Stmt *const Node = Result.Nodes.getNodeAs<Stmt>("call");
+ const Stmt *const ParentNode = Result.Nodes.getNodeAs<Stmt>("parent");
+ EXPECT_TRUE(Node);
+ EXPECT_TRUE(ParentNode);
+ const auto NodeTerminator = utils::lexer::findNextTerminator(
+ Node->getEndLoc(), Result.Context->getSourceManager(),
+ Result.Context->getLangOpts());
+ auto Range = SourceRange(Node->getBeginLoc(), NodeTerminator);
+ diag(Node->getBeginLoc(), "") << utils::fixit::addSubsequentStatement(
+ Range, *ParentNode, "foo()", *Result.Context);
+}
+
+RUN_TEST_MATCHER(AddSubsequentStatementUtil, CompoundStatementParent,
+ "void foo() {\n foo();\n}",
+ "void foo() {\n foo();\n foo();\n}")
+
+RUN_TEST_MATCHER(AddSubsequentStatementUtil,
+ CompoundStatementParentWithComments,
+ "void foo() {\n \tfoo(); //some /* comments */\n}",
+ "void foo() {\n \tfoo(); //some /* comments */\n \tfoo();\n}")
+
+RUN_TEST_MATCHER(AddSubsequentStatementUtil, IfStatementParent,
+ "void foo() {\n if(true)\n foo();\n}",
+ "void foo() {\n if(true) {\n foo();\n foo();\n }\n}")
+
+RUN_TEST_MATCHER(
+ AddSubsequentStatementUtil, IfStatementParentWithComments,
+ "void foo() {\n if(true)\n foo(); //some /* comments */\n}",
+ "void foo() {\n if(true) {\n foo(); //some /* comments */\n "
+ "foo();\n }\n}")
+
+RUN_TEST_MATCHER(
+ AddSubsequentStatementUtil, IfStatementSameLineParent,
+ "void foo() {\n if(true) foo();\n}",
+ "void foo() {\n if(true) { foo();\n foo();\n }\n}")
+
+RUN_TEST_MATCHER(AddSubsequentStatementUtil,
+ IfStatementSameLineParentWithComments,
+ "void foo() {\n if(true) foo(); //some /* comments */\n}",
+ "void foo() {\n if(true) { foo(); //some /* comments */\n "
+ " foo();\n }\n}")
+
+RUN_TEST_MATCHER(AddSubsequentStatementUtil, IfElseStatementParent,
+ "void foo() {\n if(true)\n foo();\n else\n foo();\n}",
+ "void foo() {\n if(true) {\n foo();\n foo();\n } else "
+ "{\n foo();\n foo();\n }\n}")
+
+RUN_TEST_MATCHER(
+ AddSubsequentStatementUtil, IfElseStatementParentWithComments,
+ "void foo() {\n if(true)\n foo(); //some /* comments */\n else\n "
+ "foo(); //some /* comments */\n}",
+ "void foo() {\n if(true) {\n foo(); //some /* comments */\n "
+ "foo();\n } else {\n foo(); //some /* comments */\n foo();\n }\n}")
+
+} // namespace test
+} // namespace tidy
+} // namespace clang
Index: clang-tools-extra/unittests/clang-tidy/CMakeLists.txt
===================================================================
--- clang-tools-extra/unittests/clang-tidy/CMakeLists.txt
+++ clang-tools-extra/unittests/clang-tidy/CMakeLists.txt
@@ -22,6 +22,7 @@
ClangTidyOptionsTest.cpp
DeclRefExprUtilsTest.cpp
IncludeInserterTest.cpp
+ FixItHintUtilsTest.cpp
GlobListTest.cpp
GoogleModuleTest.cpp
LLVMModuleTest.cpp
Index: clang-tools-extra/clang-tidy/utils/FixItHintUtils.h
===================================================================
--- clang-tools-extra/clang-tidy/utils/FixItHintUtils.h
+++ clang-tools-extra/clang-tidy/utils/FixItHintUtils.h
@@ -11,7 +11,9 @@
#include "clang/AST/ASTContext.h"
#include "clang/AST/Decl.h"
+#include "clang/Basic/SourceManagerInternals.h"
#include "clang/Sema/DeclSpec.h"
+#include "clang/Tooling/FixIt.h"
namespace clang {
namespace tidy {
@@ -46,6 +48,17 @@
DeclSpec::TQ Qualifier,
QualifierTarget CT = QualifierTarget::Pointee,
QualifierPolicy CP = QualifierPolicy::Left);
+
+/// \brief Adds a statement to be executed right after this statement .
+/// Is designed for taking potential comments or statements in the same line
+/// into account. The statement should not be an expression that's part of
+/// another statement. The statement range should include the terminator
+/// (semicolon).
+llvm::SmallVector<FixItHint, 1>
+addSubsequentStatement(SourceRange stmtRangeWithTerminator,
+ const Stmt &parentStmt, llvm::StringRef nextStmt,
+ ASTContext &context);
+
} // namespace fixit
} // namespace utils
} // namespace tidy
Index: clang-tools-extra/clang-tidy/utils/FixItHintUtils.cpp
===================================================================
--- clang-tools-extra/clang-tidy/utils/FixItHintUtils.cpp
+++ clang-tools-extra/clang-tidy/utils/FixItHintUtils.cpp
@@ -223,6 +223,154 @@
return None;
}
+
+static unsigned int getLineNumber(SourceLocation Loc, SourceManager& SM) {
+ FileID FID;
+ unsigned int Offset;
+ std::tie(FID, Offset) = SM.getDecomposedLoc(Loc);
+ return SM.getLineNumber(FID, Offset);
+}
+
+static std::string getIndent(SourceLocation sLoc, ASTContext& context) {
+ auto& SM = context.getSourceManager();
+
+ const auto sLocLineNo = getLineNumber(sLoc, SM);
+
+ auto indentation_template = tooling::fixit::internal::getText(
+ CharSourceRange::getCharRange(SourceRange(
+ SM.translateLineCol(SM.getFileID(sLoc), sLocLineNo, 1), sLoc)),
+ context);
+
+ std::string indentation;
+ indentation.reserve(indentation_template.size());
+ std::transform(
+ indentation_template.begin(),
+ indentation_template.end(),
+ std::back_inserter(indentation),
+ [](char c) { return isspace(c) ? c : ' '; });
+ return indentation;
+}
+
+llvm::SmallVector<FixItHint, 1> addSubsequentStatement(
+ SourceRange stmtRangeWithTerminator,
+ const Stmt& parentStmt,
+ llvm::StringRef nextStmt,
+ ASTContext& context) {
+ auto& SM = context.getSourceManager();
+ auto langOpts = context.getLangOpts();
+
+ const auto stmtEndLineNo =
+ getLineNumber(stmtRangeWithTerminator.getEnd(), SM);
+
+ // Find the first token's data for which the next token is
+ // either a line apart or is not a comment
+ SourceLocation lastTokenEndLoc =
+ stmtRangeWithTerminator.getEnd().getLocWithOffset(1);
+ auto lastTokenLine = stmtEndLineNo;
+ bool insertNewLine = true;
+ while (true) {
+ llvm::Optional<Token> tokenOption = Lexer::findNextToken(
+ lastTokenEndLoc.getLocWithOffset(-1), SM, langOpts, true);
+ if (!tokenOption) {
+ return llvm::SmallVector<FixItHint, 1>();
+ }
+ if (tokenOption->is(tok::eof)) {
+ insertNewLine = false;
+ break;
+ }
+ const auto tokenBeginLineNo = getLineNumber(tokenOption->getLocation(), SM);
+
+ if (tokenOption->isNot(tok::comment)) {
+ insertNewLine = tokenBeginLineNo != stmtEndLineNo;
+ break;
+ }
+ if (tokenBeginLineNo > lastTokenLine) {
+ break;
+ }
+
+ lastTokenEndLoc = tokenOption->getEndLoc();
+ lastTokenLine = getLineNumber(tokenOption->getEndLoc(), SM);
+ }
+
+ bool isEnclosedWithBrackets =
+ parentStmt.getStmtClass() == Stmt::CompoundStmtClass;
+
+ // Generating the FixItHint
+ // There are 5 scenarios that we have to take into account:
+ // 1. The statement is enclosed in brackets but the next statement is
+ // in the same line - insert the new statement right after the previous one
+ // 2. The statement is not enclosed in brackets and the next statement is
+ // in the same line - same as 1. and enclose both statements in brackets
+ // on the same line
+ // 3. The statement is enclosed in brackets and the next statement is
+ // on subsequent lines - skip all the comments in this line
+ // 4. The statement is not enclosed in brackets but the next statement is on
+ // subsequent lines - same as 3. and enclose the statements with
+ // google-style multiline brackets (opening bracket right after the parent
+ // statement and closing bracket on a new line after the new statement).
+ // 5. The statement is not enclosed in brackets but the next statement is on
+ // subsequent lines and the main statement is before an else token - same
+ // as 4. but the closing bracket is put on the same line as the else
+ // statement
+ if (!insertNewLine) {
+ if (isEnclosedWithBrackets) {
+ // Case 1.
+ return llvm::SmallVector<FixItHint, 1>{FixItHint::CreateInsertion(
+ stmtRangeWithTerminator.getEnd().getLocWithOffset(1),
+ (llvm::Twine(" ") + nextStmt.str() + ";").str())};
+ } else {
+ // Case 2.
+ return llvm::SmallVector<FixItHint, 1>{
+ FixItHint::CreateInsertion(stmtRangeWithTerminator.getBegin(), "{"),
+ FixItHint::CreateInsertion(
+ stmtRangeWithTerminator.getEnd().getLocWithOffset(1),
+ (llvm::Twine(" ") + nextStmt.str() + ";}").str())};
+ }
+ } else {
+ if (isEnclosedWithBrackets) {
+ // Case 3.
+ return llvm::SmallVector<FixItHint, 1>{FixItHint::CreateInsertion(
+ lastTokenEndLoc,
+ (llvm::Twine("\n") +
+ getIndent(stmtRangeWithTerminator.getBegin(), context) +
+ nextStmt.str() + ";")
+ .str())};
+ } else {
+ const auto previousTokenEndLoc =
+ tidy::utils::lexer::getPreviousToken(
+ stmtRangeWithTerminator.getBegin(), SM, context.getLangOpts())
+ .getEndLoc();
+ auto nextStmtIndent =
+ getIndent(stmtRangeWithTerminator.getBegin(), context);
+
+ if (getLineNumber(previousTokenEndLoc, SM) ==
+ getLineNumber(stmtRangeWithTerminator.getBegin(), SM)) {
+ nextStmtIndent += " ";
+ }
+ auto nextToken = Lexer::findNextToken(lastTokenEndLoc, SM, langOpts);
+ if (!nextToken || nextToken->getRawIdentifier() != "else") {
+ // Case 4.
+ return llvm::SmallVector<FixItHint, 1>{
+ FixItHint::CreateInsertion(previousTokenEndLoc, " {"),
+ FixItHint::CreateInsertion(
+ lastTokenEndLoc,
+ (llvm::Twine("\n") + nextStmtIndent + nextStmt.str() + ";\n" +
+ getIndent(parentStmt.getBeginLoc(), context) + "}")
+ .str())};
+ } else {
+ // Case 5.
+ return llvm::SmallVector<FixItHint, 1>{
+ FixItHint::CreateInsertion(previousTokenEndLoc, " {"),
+ FixItHint::CreateInsertion(
+ lastTokenEndLoc,
+ (llvm::Twine("\n") + nextStmtIndent + nextStmt.str() + ";")
+ .str()),
+ FixItHint::CreateInsertion(nextToken->getLocation(), "} ")};
+ }
+ }
+ }
+}
+
} // namespace fixit
} // namespace utils
} // namespace tidy
_______________________________________________
cfe-commits mailing list
cfe-commits@lists.llvm.org
https://lists.llvm.org/cgi-bin/mailman/listinfo/cfe-commits
