llvmorg-github-actions[bot] wrote:
<!--LLVM PR SUMMARY COMMENT--> @llvm/pr-subscribers-clangd @llvm/pr-subscribers-clang-tools-extra Author: Zeyi Xu (zeyi2) <details> <summary>Changes</summary> The readability-redundant-parentheses check emitted fix-its that simply removed both parentheses. Tools that apply those fix-its directly could join adjacent tokens and produce invalid code, e.g. `return(0)` becoming `return0`. Replace the opening parenthesis with a space when removing it would merge identifier characters across the removed token. AI Usage: Test assisted by Codex. Closes https://github.com/llvm/llvm-project/issues/185108 --- Full diff: https://github.com/llvm/llvm-project/pull/202365.diff 3 Files Affected: - (modified) clang-tools-extra/clang-tidy/readability/RedundantParenthesesCheck.cpp (+20-1) - (modified) clang-tools-extra/clangd/unittests/DiagnosticsTests.cpp (+23) - (modified) clang-tools-extra/docs/ReleaseNotes.rst (+7-3) ``````````diff diff --git a/clang-tools-extra/clang-tidy/readability/RedundantParenthesesCheck.cpp b/clang-tools-extra/clang-tidy/readability/RedundantParenthesesCheck.cpp index c177c07b95a75..639e183f434b2 100644 --- a/clang-tools-extra/clang-tidy/readability/RedundantParenthesesCheck.cpp +++ b/clang-tools-extra/clang-tidy/readability/RedundantParenthesesCheck.cpp @@ -13,6 +13,7 @@ #include "clang/ASTMatchers/ASTMatchFinder.h" #include "clang/ASTMatchers/ASTMatchers.h" #include "clang/ASTMatchers/ASTMatchersMacros.h" +#include "clang/Lex/Lexer.h" #include <cassert> using namespace clang::ast_matchers; @@ -34,6 +35,23 @@ AST_MATCHER(ParenExpr, isInMacro) { } // namespace +static FixItHint createSpacedRemoval(SourceLocation Loc, + const SourceManager &SM, + const LangOptions &LangOpts) { + if (Loc.isValid() && !Loc.isMacroID()) { + auto LocInfo = SM.getDecomposedLoc(Loc); + bool Invalid = false; + StringRef Buffer = SM.getBufferData(LocInfo.first, &Invalid); + if (!Invalid && LocInfo.second > 0 && LocInfo.second + 1 < Buffer.size() && + Lexer::isAsciiIdentifierContinueChar(Buffer[LocInfo.second - 1], + LangOpts) && + Lexer::isAsciiIdentifierContinueChar(Buffer[LocInfo.second + 1], + LangOpts)) + return FixItHint::CreateReplacement(SourceRange(Loc, Loc), " "); + } + return FixItHint::CreateRemoval(Loc); +} + RedundantParenthesesCheck::RedundantParenthesesCheck(StringRef Name, ClangTidyContext *Context) : ClangTidyCheck(Name, Context), @@ -66,7 +84,8 @@ void RedundantParenthesesCheck::registerMatchers(MatchFinder *Finder) { void RedundantParenthesesCheck::check(const MatchFinder::MatchResult &Result) { const auto *PE = Result.Nodes.getNodeAs<ParenExpr>("dup"); diag(PE->getBeginLoc(), "redundant parentheses around expression") - << FixItHint::CreateRemoval(PE->getLParen()) + << createSpacedRemoval(PE->getLParen(), *Result.SourceManager, + getLangOpts()) << FixItHint::CreateRemoval(PE->getRParen()); } diff --git a/clang-tools-extra/clangd/unittests/DiagnosticsTests.cpp b/clang-tools-extra/clangd/unittests/DiagnosticsTests.cpp index 4258f7faf34fd..6d91ac1ef1e8e 100644 --- a/clang-tools-extra/clangd/unittests/DiagnosticsTests.cpp +++ b/clang-tools-extra/clangd/unittests/DiagnosticsTests.cpp @@ -359,6 +359,29 @@ TEST(DiagnosticsTest, ClangTidy) { "function 'bar' is within a recursive call chain")))); } +TEST(DiagnosticsTest, ClangTidyRedundantParenthesesFix) { + Annotations Test(R"cpp( + int func() { + return$lparen[[(]]0$rparen[[)]]; + } + )cpp"); + auto TU = TestTU::withCode(Test.code()); + TU.ClangTidyProvider = addTidyChecks("readability-redundant-parentheses"); + + clangd::Fix ExpectedFix; + ExpectedFix.Message = "redundant parentheses around expression"; + ExpectedFix.Edits.push_back(TextEdit{Test.range("lparen"), " "}); + ExpectedFix.Edits.push_back(TextEdit{Test.range("rparen"), ""}); + + EXPECT_THAT( + TU.build().getDiagnostics(), + ifTidyChecks(ElementsAre(AllOf( + Diag(Test.range("lparen"), "redundant parentheses around expression"), + diagSource(Diag::ClangTidy), + diagName("readability-redundant-parentheses"), + withFix(equalToFix(ExpectedFix)))))); +} + TEST(DiagnosticsTest, ClangTidyEOF) { // clang-format off Annotations Test(R"cpp( diff --git a/clang-tools-extra/docs/ReleaseNotes.rst b/clang-tools-extra/docs/ReleaseNotes.rst index 9703bb8f17208..13d771ec821bf 100644 --- a/clang-tools-extra/docs/ReleaseNotes.rst +++ b/clang-tools-extra/docs/ReleaseNotes.rst @@ -802,9 +802,13 @@ Changes in existing checks macros that may expand differently in other configurations. - Improved :doc:`readability-redundant-parentheses - <clang-tidy/checks/readability/redundant-parentheses>` check by fixing a - false positive for parentheses present around an overloaded operator in the - context of a binary operation. + <clang-tidy/checks/readability/redundant-parentheses>` check: + + - Fixed a false positive for parentheses present around an overloaded operator + in the context of a binary operation. + + - Fixed a bug where clients that apply fix-its without :program:`clang-tidy`'s + cleanup could produce invalid code by joining adjacent tokens. - Improved :doc:`readability-redundant-preprocessor <clang-tidy/checks/readability/redundant-preprocessor>` check by fixing a `````````` </details> https://github.com/llvm/llvm-project/pull/202365 _______________________________________________ cfe-commits mailing list [email protected] https://lists.llvm.org/cgi-bin/mailman/listinfo/cfe-commits
