[clang-tools-extra] 7a4e897 - [clang-tidy] Add fix-its to `readability-avoid-return-with-void-value` check (#81420)
via cfe-commits
cfe-commits at lists.llvm.org
Sun Apr 7 04:28:32 PDT 2024
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: }
}
More information about the cfe-commits
mailing list