Author: Danny Mösch Date: 2024-04-07T13:28:29+02:00 New Revision: 7a4e89761a13bfad27a2614ecea5e8698f50336c
URL: https://github.com/llvm/llvm-project/commit/7a4e89761a13bfad27a2614ecea5e8698f50336c DIFF: https://github.com/llvm/llvm-project/commit/7a4e89761a13bfad27a2614ecea5e8698f50336c.diff LOG: [clang-tidy] Add fix-its to `readability-avoid-return-with-void-value` check (#81420) Added: clang-tools-extra/clang-tidy/utils/BracesAroundStatement.cpp clang-tools-extra/clang-tidy/utils/BracesAroundStatement.h Modified: clang-tools-extra/clang-tidy/readability/AvoidReturnWithVoidValueCheck.cpp clang-tools-extra/clang-tidy/readability/BracesAroundStatementsCheck.cpp clang-tools-extra/clang-tidy/readability/BracesAroundStatementsCheck.h clang-tools-extra/clang-tidy/utils/CMakeLists.txt clang-tools-extra/docs/ReleaseNotes.rst clang-tools-extra/test/clang-tidy/checkers/readability/avoid-return-with-void-value.cpp Removed: ################################################################################ diff --git a/clang-tools-extra/clang-tidy/readability/AvoidReturnWithVoidValueCheck.cpp b/clang-tools-extra/clang-tidy/readability/AvoidReturnWithVoidValueCheck.cpp index e3400f614fa564..48bca41f4a3b1e 100644 --- a/clang-tools-extra/clang-tidy/readability/AvoidReturnWithVoidValueCheck.cpp +++ b/clang-tools-extra/clang-tidy/readability/AvoidReturnWithVoidValueCheck.cpp @@ -7,19 +7,18 @@ //===----------------------------------------------------------------------===// #include "AvoidReturnWithVoidValueCheck.h" -#include "clang/AST/Stmt.h" -#include "clang/ASTMatchers/ASTMatchFinder.h" -#include "clang/ASTMatchers/ASTMatchers.h" +#include "../utils/BracesAroundStatement.h" +#include "../utils/LexerUtils.h" using namespace clang::ast_matchers; namespace clang::tidy::readability { -static constexpr auto IgnoreMacrosName = "IgnoreMacros"; -static constexpr auto IgnoreMacrosDefault = true; +static constexpr char IgnoreMacrosName[] = "IgnoreMacros"; +static const bool IgnoreMacrosDefault = true; -static constexpr auto StrictModeName = "StrictMode"; -static constexpr auto StrictModeDefault = true; +static constexpr char StrictModeName[] = "StrictMode"; +static const bool StrictModeDefault = true; AvoidReturnWithVoidValueCheck::AvoidReturnWithVoidValueCheck( StringRef Name, ClangTidyContext *Context) @@ -32,7 +31,10 @@ void AvoidReturnWithVoidValueCheck::registerMatchers(MatchFinder *Finder) { Finder->addMatcher( returnStmt( hasReturnValue(allOf(hasType(voidType()), unless(initListExpr()))), - optionally(hasParent(compoundStmt().bind("compound_parent")))) + optionally(hasParent( + compoundStmt( + optionally(hasParent(functionDecl().bind("function_parent")))) + .bind("compound_parent")))) .bind("void_return"), this); } @@ -42,10 +44,30 @@ void AvoidReturnWithVoidValueCheck::check( const auto *VoidReturn = Result.Nodes.getNodeAs<ReturnStmt>("void_return"); if (IgnoreMacros && VoidReturn->getBeginLoc().isMacroID()) return; - if (!StrictMode && !Result.Nodes.getNodeAs<CompoundStmt>("compound_parent")) + const auto *SurroundingBlock = + Result.Nodes.getNodeAs<CompoundStmt>("compound_parent"); + if (!StrictMode && !SurroundingBlock) return; - diag(VoidReturn->getBeginLoc(), "return statement within a void function " - "should not have a specified return value"); + DiagnosticBuilder Diag = diag(VoidReturn->getBeginLoc(), + "return statement within a void function " + "should not have a specified return value"); + const SourceLocation SemicolonPos = utils::lexer::findNextTerminator( + VoidReturn->getEndLoc(), *Result.SourceManager, getLangOpts()); + if (SemicolonPos.isInvalid()) + return; + if (!SurroundingBlock) { + const auto BraceInsertionHints = utils::getBraceInsertionsHints( + VoidReturn, getLangOpts(), *Result.SourceManager, + VoidReturn->getBeginLoc()); + if (BraceInsertionHints) + Diag << BraceInsertionHints.openingBraceFixIt() + << BraceInsertionHints.closingBraceFixIt(); + } + Diag << FixItHint::CreateRemoval(VoidReturn->getReturnLoc()); + if (!Result.Nodes.getNodeAs<FunctionDecl>("function_parent") || + SurroundingBlock->body_back() != VoidReturn) + Diag << FixItHint::CreateInsertion(SemicolonPos.getLocWithOffset(1), + " return;", true); } void AvoidReturnWithVoidValueCheck::storeOptions( diff --git a/clang-tools-extra/clang-tidy/readability/BracesAroundStatementsCheck.cpp b/clang-tools-extra/clang-tidy/readability/BracesAroundStatementsCheck.cpp index 81ca33cbbdfb4b..85bd9c1e4f9a04 100644 --- a/clang-tools-extra/clang-tidy/readability/BracesAroundStatementsCheck.cpp +++ b/clang-tools-extra/clang-tidy/readability/BracesAroundStatementsCheck.cpp @@ -7,6 +7,7 @@ //===----------------------------------------------------------------------===// #include "BracesAroundStatementsCheck.h" +#include "../utils/BracesAroundStatement.h" #include "../utils/LexerUtils.h" #include "clang/AST/ASTContext.h" #include "clang/ASTMatchers/ASTMatchers.h" @@ -17,12 +18,10 @@ using namespace clang::ast_matchers; namespace clang::tidy::readability { static tok::TokenKind getTokenKind(SourceLocation Loc, const SourceManager &SM, - const ASTContext *Context) { + const LangOptions &LangOpts) { Token Tok; - SourceLocation Beginning = - Lexer::GetBeginningOfToken(Loc, SM, Context->getLangOpts()); - const bool Invalid = - Lexer::getRawToken(Beginning, Tok, SM, Context->getLangOpts()); + SourceLocation Beginning = Lexer::GetBeginningOfToken(Loc, SM, LangOpts); + const bool Invalid = Lexer::getRawToken(Beginning, Tok, SM, LangOpts); assert(!Invalid && "Expected a valid token."); if (Invalid) @@ -33,64 +32,21 @@ static tok::TokenKind getTokenKind(SourceLocation Loc, const SourceManager &SM, static SourceLocation forwardSkipWhitespaceAndComments(SourceLocation Loc, const SourceManager &SM, - const ASTContext *Context) { + const LangOptions &LangOpts) { assert(Loc.isValid()); for (;;) { while (isWhitespace(*SM.getCharacterData(Loc))) Loc = Loc.getLocWithOffset(1); - tok::TokenKind TokKind = getTokenKind(Loc, SM, Context); + tok::TokenKind TokKind = getTokenKind(Loc, SM, LangOpts); if (TokKind != tok::comment) return Loc; // Fast-forward current token. - Loc = Lexer::getLocForEndOfToken(Loc, 0, SM, Context->getLangOpts()); + Loc = Lexer::getLocForEndOfToken(Loc, 0, SM, LangOpts); } } -static SourceLocation findEndLocation(const Stmt &S, const SourceManager &SM, - const ASTContext *Context) { - SourceLocation Loc = - utils::lexer::getUnifiedEndLoc(S, SM, Context->getLangOpts()); - if (!Loc.isValid()) - return Loc; - - // Start searching right after S. - Loc = Loc.getLocWithOffset(1); - - for (;;) { - assert(Loc.isValid()); - while (isHorizontalWhitespace(*SM.getCharacterData(Loc))) { - Loc = Loc.getLocWithOffset(1); - } - - if (isVerticalWhitespace(*SM.getCharacterData(Loc))) { - // EOL, insert brace before. - break; - } - tok::TokenKind TokKind = getTokenKind(Loc, SM, Context); - if (TokKind != tok::comment) { - // Non-comment token, insert brace before. - break; - } - - SourceLocation TokEndLoc = - Lexer::getLocForEndOfToken(Loc, 0, SM, Context->getLangOpts()); - SourceRange TokRange(Loc, TokEndLoc); - StringRef Comment = Lexer::getSourceText( - CharSourceRange::getTokenRange(TokRange), SM, Context->getLangOpts()); - if (Comment.starts_with("/*") && Comment.contains('\n')) { - // Multi-line block comment, insert brace before. - break; - } - // else: Trailing comment, insert brace after the newline. - - // Fast-forward current token. - Loc = TokEndLoc; - } - return Loc; -} - BracesAroundStatementsCheck::BracesAroundStatementsCheck( StringRef Name, ClangTidyContext *Context) : ClangTidyCheck(Name, Context), @@ -124,7 +80,7 @@ void BracesAroundStatementsCheck::check( } else if (const auto *S = Result.Nodes.getNodeAs<DoStmt>("do")) { checkStmt(Result, S->getBody(), S->getDoLoc(), S->getWhileLoc()); } else if (const auto *S = Result.Nodes.getNodeAs<WhileStmt>("while")) { - SourceLocation StartLoc = findRParenLoc(S, SM, Context); + SourceLocation StartLoc = findRParenLoc(S, SM, Context->getLangOpts()); if (StartLoc.isInvalid()) return; checkStmt(Result, S->getBody(), StartLoc); @@ -133,7 +89,7 @@ void BracesAroundStatementsCheck::check( if (S->isConsteval()) return; - SourceLocation StartLoc = findRParenLoc(S, SM, Context); + SourceLocation StartLoc = findRParenLoc(S, SM, Context->getLangOpts()); if (StartLoc.isInvalid()) return; if (ForceBracesStmts.erase(S)) @@ -156,7 +112,7 @@ template <typename IfOrWhileStmt> SourceLocation BracesAroundStatementsCheck::findRParenLoc(const IfOrWhileStmt *S, const SourceManager &SM, - const ASTContext *Context) { + const LangOptions &LangOpts) { // Skip macros. if (S->getBeginLoc().isMacroID()) return {}; @@ -170,14 +126,14 @@ BracesAroundStatementsCheck::findRParenLoc(const IfOrWhileStmt *S, } SourceLocation PastCondEndLoc = - Lexer::getLocForEndOfToken(CondEndLoc, 0, SM, Context->getLangOpts()); + Lexer::getLocForEndOfToken(CondEndLoc, 0, SM, LangOpts); if (PastCondEndLoc.isInvalid()) return {}; SourceLocation RParenLoc = - forwardSkipWhitespaceAndComments(PastCondEndLoc, SM, Context); + forwardSkipWhitespaceAndComments(PastCondEndLoc, SM, LangOpts); if (RParenLoc.isInvalid()) return {}; - tok::TokenKind TokKind = getTokenKind(RParenLoc, SM, Context); + tok::TokenKind TokKind = getTokenKind(RParenLoc, SM, LangOpts); if (TokKind != tok::r_paren) return {}; return RParenLoc; @@ -188,86 +144,23 @@ BracesAroundStatementsCheck::findRParenLoc(const IfOrWhileStmt *S, bool BracesAroundStatementsCheck::checkStmt( const MatchFinder::MatchResult &Result, const Stmt *S, SourceLocation StartLoc, SourceLocation EndLocHint) { - while (const auto *AS = dyn_cast<AttributedStmt>(S)) S = AS->getSubStmt(); - const SourceManager &SM = *Result.SourceManager; - const ASTContext *Context = Result.Context; - - // 1) If there's a corresponding "else" or "while", the check inserts "} " - // right before that token. - // 2) If there's a multi-line block comment starting on the same line after - // the location we're inserting the closing brace at, or there's a non-comment - // token, the check inserts "\n}" right before that token. - // 3) Otherwise the check finds the end of line (possibly after some block or - // line comments) and inserts "\n}" right before that EOL. - if (!S || isa<CompoundStmt>(S)) { - // Already inside braces. - return false; - } - - // When TreeTransform, Stmt in constexpr IfStmt will be transform to NullStmt. - // This NullStmt can be detected according to beginning token. - const SourceLocation StmtBeginLoc = S->getBeginLoc(); - if (isa<NullStmt>(S) && StmtBeginLoc.isValid() && - getTokenKind(StmtBeginLoc, SM, Context) == tok::l_brace) - return false; - - if (StartLoc.isInvalid()) - return false; - - // Convert StartLoc to file location, if it's on the same macro expansion - // level as the start of the statement. We also need file locations for - // Lexer::getLocForEndOfToken working properly. - StartLoc = Lexer::makeFileCharRange( - CharSourceRange::getCharRange(StartLoc, S->getBeginLoc()), SM, - Context->getLangOpts()) - .getBegin(); - if (StartLoc.isInvalid()) - return false; - StartLoc = - Lexer::getLocForEndOfToken(StartLoc, 0, SM, Context->getLangOpts()); - - // StartLoc points at the location of the opening brace to be inserted. - SourceLocation EndLoc; - std::string ClosingInsertion; - if (EndLocHint.isValid()) { - EndLoc = EndLocHint; - ClosingInsertion = "} "; - } else { - EndLoc = findEndLocation(*S, SM, Context); - ClosingInsertion = "\n}"; - } - - assert(StartLoc.isValid()); - - // Don't require braces for statements spanning less than certain number of - // lines. - if (ShortStatementLines && !ForceBracesStmts.erase(S)) { - unsigned StartLine = SM.getSpellingLineNumber(StartLoc); - unsigned EndLine = SM.getSpellingLineNumber(EndLoc); - if (EndLine - StartLine < ShortStatementLines) + const auto BraceInsertionHints = utils::getBraceInsertionsHints( + S, Result.Context->getLangOpts(), *Result.SourceManager, StartLoc, + EndLocHint); + if (BraceInsertionHints) { + if (ShortStatementLines && !ForceBracesStmts.erase(S) && + BraceInsertionHints.resultingCompoundLineExtent(*Result.SourceManager) < + ShortStatementLines) return false; + auto Diag = diag(BraceInsertionHints.DiagnosticPos, + "statement should be inside braces"); + if (BraceInsertionHints.offersFixIts()) + Diag << BraceInsertionHints.openingBraceFixIt() + << BraceInsertionHints.closingBraceFixIt(); } - - auto Diag = diag(StartLoc, "statement should be inside braces"); - - // Change only if StartLoc and EndLoc are on the same macro expansion level. - // This will also catch invalid EndLoc. - // Example: LLVM_DEBUG( for(...) do_something() ); - // In this case fix-it cannot be provided as the semicolon which is not - // visible here is part of the macro. Adding braces here would require adding - // another semicolon. - if (Lexer::makeFileCharRange( - CharSourceRange::getTokenRange(SourceRange( - SM.getSpellingLoc(StartLoc), SM.getSpellingLoc(EndLoc))), - SM, Context->getLangOpts()) - .isInvalid()) - return false; - - Diag << FixItHint::CreateInsertion(StartLoc, " {") - << FixItHint::CreateInsertion(EndLoc, ClosingInsertion); return true; } diff --git a/clang-tools-extra/clang-tidy/readability/BracesAroundStatementsCheck.h b/clang-tools-extra/clang-tidy/readability/BracesAroundStatementsCheck.h index 249aa1aaaa9154..4cd37a7b2dd6cc 100644 --- a/clang-tools-extra/clang-tidy/readability/BracesAroundStatementsCheck.h +++ b/clang-tools-extra/clang-tidy/readability/BracesAroundStatementsCheck.h @@ -52,7 +52,7 @@ class BracesAroundStatementsCheck : public ClangTidyCheck { SourceLocation EndLocHint = SourceLocation()); template <typename IfOrWhileStmt> SourceLocation findRParenLoc(const IfOrWhileStmt *S, const SourceManager &SM, - const ASTContext *Context); + const LangOptions &LangOpts); std::optional<TraversalKind> getCheckTraversalKind() const override { return TK_IgnoreUnlessSpelledInSource; } diff --git a/clang-tools-extra/clang-tidy/utils/BracesAroundStatement.cpp b/clang-tools-extra/clang-tidy/utils/BracesAroundStatement.cpp new file mode 100644 index 00000000000000..2a3b7bed08c1e0 --- /dev/null +++ b/clang-tools-extra/clang-tidy/utils/BracesAroundStatement.cpp @@ -0,0 +1,168 @@ +//===--- BracesAroundStatement.cpp - clang-tidy -------- ------------------===// +// +// 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 +// +//===----------------------------------------------------------------------===// +/// +/// \file +/// This file provides utilities to put braces around a statement. +/// +//===----------------------------------------------------------------------===// + +#include "BracesAroundStatement.h" +#include "../utils/LexerUtils.h" +#include "LexerUtils.h" +#include "clang/AST/ASTContext.h" +#include "clang/Basic/CharInfo.h" +#include "clang/Basic/LangOptions.h" +#include "clang/Lex/Lexer.h" + +namespace clang::tidy::utils { + +BraceInsertionHints::operator bool() const { return DiagnosticPos.isValid(); } + +bool BraceInsertionHints::offersFixIts() const { + return OpeningBracePos.isValid() && ClosingBracePos.isValid(); +} + +unsigned BraceInsertionHints::resultingCompoundLineExtent( + const SourceManager &SourceMgr) const { + return SourceMgr.getSpellingLineNumber(ClosingBracePos) - + SourceMgr.getSpellingLineNumber(OpeningBracePos); +} + +FixItHint BraceInsertionHints::openingBraceFixIt() const { + return OpeningBracePos.isValid() + ? FixItHint::CreateInsertion(OpeningBracePos, " {") + : FixItHint(); +} + +FixItHint BraceInsertionHints::closingBraceFixIt() const { + return ClosingBracePos.isValid() + ? FixItHint::CreateInsertion(ClosingBracePos, ClosingBrace) + : FixItHint(); +} + +static tok::TokenKind getTokenKind(SourceLocation Loc, const SourceManager &SM, + const LangOptions &LangOpts) { + Token Tok; + SourceLocation Beginning = Lexer::GetBeginningOfToken(Loc, SM, LangOpts); + const bool Invalid = Lexer::getRawToken(Beginning, Tok, SM, LangOpts); + assert(!Invalid && "Expected a valid token."); + + if (Invalid) + return tok::NUM_TOKENS; + + return Tok.getKind(); +} + +static SourceLocation findEndLocation(const Stmt &S, const SourceManager &SM, + const LangOptions &LangOpts) { + SourceLocation Loc = lexer::getUnifiedEndLoc(S, SM, LangOpts); + if (!Loc.isValid()) + return Loc; + + // Start searching right after S. + Loc = Loc.getLocWithOffset(1); + + for (;;) { + assert(Loc.isValid()); + while (isHorizontalWhitespace(*SM.getCharacterData(Loc))) { + Loc = Loc.getLocWithOffset(1); + } + + if (isVerticalWhitespace(*SM.getCharacterData(Loc))) { + // EOL, insert brace before. + break; + } + tok::TokenKind TokKind = getTokenKind(Loc, SM, LangOpts); + if (TokKind != tok::comment) { + // Non-comment token, insert brace before. + break; + } + + SourceLocation TokEndLoc = Lexer::getLocForEndOfToken(Loc, 0, SM, LangOpts); + SourceRange TokRange(Loc, TokEndLoc); + StringRef Comment = Lexer::getSourceText( + CharSourceRange::getTokenRange(TokRange), SM, LangOpts); + if (Comment.starts_with("/*") && Comment.contains('\n')) { + // Multi-line block comment, insert brace before. + break; + } + // else: Trailing comment, insert brace after the newline. + + // Fast-forward current token. + Loc = TokEndLoc; + } + return Loc; +} + +BraceInsertionHints getBraceInsertionsHints(const Stmt *const S, + const LangOptions &LangOpts, + const SourceManager &SM, + SourceLocation StartLoc, + SourceLocation EndLocHint) { + // 1) If there's a corresponding "else" or "while", the check inserts "} " + // right before that token. + // 2) If there's a multi-line block comment starting on the same line after + // the location we're inserting the closing brace at, or there's a non-comment + // token, the check inserts "\n}" right before that token. + // 3) Otherwise the check finds the end of line (possibly after some block or + // line comments) and inserts "\n}" right before that EOL. + if (!S || isa<CompoundStmt>(S)) { + // Already inside braces. + return {}; + } + + // When TreeTransform, Stmt in constexpr IfStmt will be transform to NullStmt. + // This NullStmt can be detected according to beginning token. + const SourceLocation StmtBeginLoc = S->getBeginLoc(); + if (isa<NullStmt>(S) && StmtBeginLoc.isValid() && + getTokenKind(StmtBeginLoc, SM, LangOpts) == tok::l_brace) + return {}; + + if (StartLoc.isInvalid()) + return {}; + + // Convert StartLoc to file location, if it's on the same macro expansion + // level as the start of the statement. We also need file locations for + // Lexer::getLocForEndOfToken working properly. + StartLoc = Lexer::makeFileCharRange( + CharSourceRange::getCharRange(StartLoc, S->getBeginLoc()), SM, + LangOpts) + .getBegin(); + if (StartLoc.isInvalid()) + return {}; + StartLoc = Lexer::getLocForEndOfToken(StartLoc, 0, SM, LangOpts); + + // StartLoc points at the location of the opening brace to be inserted. + SourceLocation EndLoc; + std::string ClosingInsertion; + if (EndLocHint.isValid()) { + EndLoc = EndLocHint; + ClosingInsertion = "} "; + } else { + EndLoc = findEndLocation(*S, SM, LangOpts); + ClosingInsertion = "\n}"; + } + + assert(StartLoc.isValid()); + + // Change only if StartLoc and EndLoc are on the same macro expansion level. + // This will also catch invalid EndLoc. + // Example: LLVM_DEBUG( for(...) do_something() ); + // In this case fix-it cannot be provided as the semicolon which is not + // visible here is part of the macro. Adding braces here would require adding + // another semicolon. + if (Lexer::makeFileCharRange( + CharSourceRange::getTokenRange(SourceRange( + SM.getSpellingLoc(StartLoc), SM.getSpellingLoc(EndLoc))), + SM, LangOpts) + .isInvalid()) + return {StartLoc}; + return {StartLoc, EndLoc, ClosingInsertion}; +} + +} // namespace clang::tidy::utils diff --git a/clang-tools-extra/clang-tidy/utils/BracesAroundStatement.h b/clang-tools-extra/clang-tidy/utils/BracesAroundStatement.h new file mode 100644 index 00000000000000..cb1c06c7aa1a1a --- /dev/null +++ b/clang-tools-extra/clang-tidy/utils/BracesAroundStatement.h @@ -0,0 +1,75 @@ +//===--- BracesAroundStatement.h - clang-tidy ------- -----------*- 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 +// +//===----------------------------------------------------------------------===// +/// +/// \file +/// This file provides utilities to put braces around a statement. +/// +//===----------------------------------------------------------------------===// + +#include "clang/AST/Stmt.h" +#include "clang/Basic/Diagnostic.h" +#include "clang/Basic/SourceLocation.h" +#include "clang/Basic/SourceManager.h" + +namespace clang::tidy::utils { + +/// A provider of fix-it hints to insert opening and closing braces. An instance +/// of this type is the result of calling `getBraceInsertionsHints` below. +struct BraceInsertionHints { + /// The position of a potential diagnostic. It coincides with the position of + /// the opening brace to insert, but can also just be the place to show a + /// diagnostic in case braces cannot be inserted automatically. + SourceLocation DiagnosticPos; + + /// Constructor for a no-hint. + BraceInsertionHints() = default; + + /// Constructor for a valid hint that cannot insert braces automatically. + BraceInsertionHints(SourceLocation DiagnosticPos) + : DiagnosticPos(DiagnosticPos) {} + + /// Constructor for a hint offering fix-its for brace insertion. Both + /// positions must be valid. + BraceInsertionHints(SourceLocation OpeningBracePos, + SourceLocation ClosingBracePos, std::string ClosingBrace) + : DiagnosticPos(OpeningBracePos), OpeningBracePos(OpeningBracePos), + ClosingBracePos(ClosingBracePos), ClosingBrace(ClosingBrace) { + assert(offersFixIts()); + } + + /// Indicates whether the hint provides at least the position of a diagnostic. + operator bool() const; + + /// Indicates whether the hint provides fix-its to insert braces. + bool offersFixIts() const; + + /// The number of lines between the inserted opening brace and its closing + /// counterpart. + unsigned resultingCompoundLineExtent(const SourceManager &SourceMgr) const; + + /// Fix-it to insert an opening brace. + FixItHint openingBraceFixIt() const; + + /// Fix-it to insert a closing brace. + FixItHint closingBraceFixIt() const; + +private: + SourceLocation OpeningBracePos; + SourceLocation ClosingBracePos; + std::string ClosingBrace; +}; + +/// Create fix-it hints for braces that wrap the given statement when applied. +/// The algorithm computing them respects comment before and after the statement +/// and adds line breaks before the braces accordingly. +BraceInsertionHints +getBraceInsertionsHints(const Stmt *const S, const LangOptions &LangOpts, + const SourceManager &SM, SourceLocation StartLoc, + SourceLocation EndLocHint = SourceLocation()); + +} // namespace clang::tidy::utils diff --git a/clang-tools-extra/clang-tidy/utils/CMakeLists.txt b/clang-tools-extra/clang-tidy/utils/CMakeLists.txt index f0160fa9df7487..9cff7d475425d7 100644 --- a/clang-tools-extra/clang-tidy/utils/CMakeLists.txt +++ b/clang-tools-extra/clang-tidy/utils/CMakeLists.txt @@ -6,6 +6,7 @@ set(LLVM_LINK_COMPONENTS add_clang_library(clangTidyUtils Aliasing.cpp ASTUtils.cpp + BracesAroundStatement.cpp DeclRefExprUtils.cpp DesignatedInitializers.cpp ExceptionAnalyzer.cpp diff --git a/clang-tools-extra/docs/ReleaseNotes.rst b/clang-tools-extra/docs/ReleaseNotes.rst index 34bad7e624630c..2babb1406b9776 100644 --- a/clang-tools-extra/docs/ReleaseNotes.rst +++ b/clang-tools-extra/docs/ReleaseNotes.rst @@ -255,6 +255,10 @@ Changes in existing checks analyzed, se the check now handles the common patterns `const auto e = (*vector_ptr)[i]` and `const auto e = vector_ptr->at(i);`. +- Improved :doc:`readability-avoid-return-with-void-value + <clang-tidy/checks/readability/avoid-return-with-void-value>` check by adding + fix-its. + - Improved :doc:`readability-duplicate-include <clang-tidy/checks/readability/duplicate-include>` check by excluding include directives that form the filename using macro. diff --git a/clang-tools-extra/test/clang-tidy/checkers/readability/avoid-return-with-void-value.cpp b/clang-tools-extra/test/clang-tidy/checkers/readability/avoid-return-with-void-value.cpp index f00407c99ce570..7c948dba3e8f7c 100644 --- a/clang-tools-extra/test/clang-tidy/checkers/readability/avoid-return-with-void-value.cpp +++ b/clang-tools-extra/test/clang-tidy/checkers/readability/avoid-return-with-void-value.cpp @@ -12,23 +12,30 @@ void f2() { return f1(); // CHECK-MESSAGES: :[[@LINE-1]]:5: warning: return statement within a void function should not have a specified return value [readability-avoid-return-with-void-value] // CHECK-MESSAGES-LENIENT: :[[@LINE-2]]:5: warning: return statement within a void function should not have a specified return value [readability-avoid-return-with-void-value] + // CHECK-FIXES: f1(); } void f3(bool b) { if (b) return f1(); // CHECK-MESSAGES: :[[@LINE-1]]:12: warning: return statement within a void function should not have a specified return value [readability-avoid-return-with-void-value] + // CHECK-FIXES: if (b) { f1(); return; + // CHECK-NEXT: } return f2(); // CHECK-MESSAGES: :[[@LINE-1]]:5: warning: return statement within a void function should not have a specified return value [readability-avoid-return-with-void-value] // CHECK-MESSAGES-LENIENT: :[[@LINE-2]]:5: warning: return statement within a void function should not have a specified return value [readability-avoid-return-with-void-value] + // CHECK-FIXES: f2(); + // CHECK-FIXES-LENIENT: f2(); } template<class T> T f4() {} void f5() { - return f4<void>(); - // CHECK-MESSAGES: :[[@LINE-1]]:5: warning: return statement within a void function should not have a specified return value [readability-avoid-return-with-void-value] - // CHECK-MESSAGES-LENIENT: :[[@LINE-2]]:5: warning: return statement within a void function should not have a specified return value [readability-avoid-return-with-void-value] + { return f4<void>(); } + // CHECK-MESSAGES: :[[@LINE-1]]:7: warning: return statement within a void function should not have a specified return value [readability-avoid-return-with-void-value] + // CHECK-MESSAGES-LENIENT: :[[@LINE-2]]:7: warning: return statement within a void function should not have a specified return value [readability-avoid-return-with-void-value] + // CHECK-FIXES: { f4<void>(); return; } + // CHECK-FIXES-LENIENT: { f4<void>(); return; } } void f6() { return; } @@ -41,6 +48,8 @@ void f9() { return (void)f7(); // CHECK-MESSAGES: :[[@LINE-1]]:5: warning: return statement within a void function should not have a specified return value [readability-avoid-return-with-void-value] // CHECK-MESSAGES-LENIENT: :[[@LINE-2]]:5: warning: return statement within a void function should not have a specified return value [readability-avoid-return-with-void-value] + // CHECK-FIXES: (void)f7(); + // CHECK-FIXES-LENIENT: (void)f7(); } #define RETURN_VOID return (void)1 @@ -50,12 +59,12 @@ void f10() { // CHECK-MESSAGES-INCLUDE-MACROS: :[[@LINE-1]]:5: warning: return statement within a void function should not have a specified return value [readability-avoid-return-with-void-value] } -template <typename A> +template <typename A> struct C { C(A) {} }; -template <class T> +template <class T> C<T> f11() { return {}; } using VOID = void; @@ -66,4 +75,36 @@ VOID f13() { return f12(); // CHECK-MESSAGES: :[[@LINE-1]]:5: warning: return statement within a void function should not have a specified return value [readability-avoid-return-with-void-value] // CHECK-MESSAGES-LENIENT: :[[@LINE-2]]:5: warning: return statement within a void function should not have a specified return value [readability-avoid-return-with-void-value] + // CHECK-FIXES: f12(); return; + // CHECK-FIXES-LENIENT: f12(); return; + (void)1; +} + +void f14() { + return /* comment */ f1() /* comment */ ; + // CHECK-MESSAGES: :[[@LINE-1]]:5: warning: return statement within a void function should not have a specified return value [readability-avoid-return-with-void-value] + // CHECK-MESSAGES-LENIENT: :[[@LINE-2]]:5: warning: return statement within a void function should not have a specified return value [readability-avoid-return-with-void-value] + // CHECK-FIXES: /* comment */ f1() /* comment */ ; return; + // CHECK-FIXES-LENIENT: /* comment */ f1() /* comment */ ; return; + (void)1; +} + +void f15() { + return/*comment*/f1()/*comment*/;//comment + // CHECK-MESSAGES: :[[@LINE-1]]:5: warning: return statement within a void function should not have a specified return value [readability-avoid-return-with-void-value] + // CHECK-MESSAGES-LENIENT: :[[@LINE-2]]:5: warning: return statement within a void function should not have a specified return value [readability-avoid-return-with-void-value] + // CHECK-FIXES: /*comment*/f1()/*comment*/; return;//comment + // CHECK-FIXES-LENIENT: /*comment*/f1()/*comment*/; return;//comment + (void)1; +} + +void f16(bool b) { + if (b) return f1(); + // CHECK-MESSAGES: :[[@LINE-1]]:12: warning: return statement within a void function should not have a specified return value [readability-avoid-return-with-void-value] + // CHECK-FIXES: if (b) { f1(); return; + // CHECK-NEXT: } + else return f2(); + // CHECK-MESSAGES: :[[@LINE-1]]:10: warning: return statement within a void function should not have a specified return value [readability-avoid-return-with-void-value] + // CHECK-FIXES: else { f2(); return; + // CHECK-NEXT: } } _______________________________________________ cfe-commits mailing list cfe-commits@lists.llvm.org https://lists.llvm.org/cgi-bin/mailman/listinfo/cfe-commits
