diff --git a/clang-tools-extra/clang-tidy/utils/FixItHintUtils.h b/clang-tools-extra/clang-tidy/utils/FixItHintUtils.h --- a/clang-tools-extra/clang-tidy/utils/FixItHintUtils.h +++ b/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 +addSubsequentStatement(SourceRange stmtRangeWithTerminator, + const Stmt &parentStmt, llvm::StringRef nextStmt, + ASTContext &context); + } // namespace fixit } // namespace utils } // namespace tidy diff --git a/clang-tools-extra/clang-tidy/utils/FixItHintUtils.cpp b/clang-tools-extra/clang-tidy/utils/FixItHintUtils.cpp --- a/clang-tools-extra/clang-tidy/utils/FixItHintUtils.cpp +++ b/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 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 tokenOption = Lexer::findNextToken( + lastTokenEndLoc.getLocWithOffset(-1), SM, langOpts, true); + if (!tokenOption) { + return llvm::SmallVector(); + } + 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::CreateInsertion( + stmtRangeWithTerminator.getEnd().getLocWithOffset(1), + (llvm::Twine(" ") + nextStmt.str() + ";").str())}; + } else { + // Case 2. + return llvm::SmallVector{ + FixItHint::CreateInsertion(stmtRangeWithTerminator.getBegin(), "{"), + FixItHint::CreateInsertion( + stmtRangeWithTerminator.getEnd().getLocWithOffset(1), + (llvm::Twine(" ") + nextStmt.str() + ";}").str())}; + } + } else { + if (isEnclosedWithBrackets) { + // Case 3. + return llvm::SmallVector{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::CreateInsertion(previousTokenEndLoc, " {"), + FixItHint::CreateInsertion( + lastTokenEndLoc, + (llvm::Twine("\n") + nextStmtIndent + nextStmt.str() + ";\n" + + getIndent(parentStmt.getBeginLoc(), context) + "}") + .str())}; + } else { + // Case 5. + return llvm::SmallVector{ + FixItHint::CreateInsertion(previousTokenEndLoc, " {"), + FixItHint::CreateInsertion( + lastTokenEndLoc, + (llvm::Twine("\n") + nextStmtIndent + nextStmt.str() + ";") + .str()), + FixItHint::CreateInsertion(nextToken->getLocation(), "} ")}; + } + } + } +} + } // namespace fixit } // namespace utils } // namespace tidy diff --git a/clang-tools-extra/unittests/clang-tidy/CMakeLists.txt b/clang-tools-extra/unittests/clang-tidy/CMakeLists.txt --- a/clang-tools-extra/unittests/clang-tidy/CMakeLists.txt +++ b/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 diff --git a/clang-tools-extra/unittests/clang-tidy/FixItHintUtilsTest.cpp b/clang-tools-extra/unittests/clang-tidy/FixItHintUtilsTest.cpp new file mode 100644 --- /dev/null +++ b/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 getMatcher(); \ + }; \ + clang::ast_matchers::internal::Matcher \ + 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(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("call"); + const Stmt *const ParentNode = Result.Nodes.getNodeAs("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