From 61cb48524c6386f2d3f402e65e76408de66e2222 Mon Sep 17 00:00:00 2001 From: sstwcw Date: Thu, 25 Sep 2025 16:29:40 +0000 Subject: [PATCH 1/2] [clang-format] Keep the ObjC selector name and `@selector` together Fixes #36459. after ```Objective-C - (void)test { if ([object respondsToSelector:@selector( selectorNameThatIsReallyLong:param1:param2:)]) return; } ``` before ```Objective-C - (void)test { if ([object respondsToSelector:@selector (selectorNameThatIsReallyLong:param1:param2:)]) return; } ``` Before this patch, the `ObjCMethodExpr` type was assigned to many kinds of tokens. The rule for allowing breaking the line before the colon on line TokenAnnotator.cpp:6290 was intended for method declarations and calls. It matched the parenthesis following `@selector` by mistake. To fix the problem, this patch adds a new type for `@selector`. Most of the special things in the code related to the old type is intended for other constructs. So most of the code related to the old type is not changed in this patch. --- clang/lib/Format/Format.cpp | 2 +- clang/lib/Format/FormatToken.h | 11 ++++++++ clang/lib/Format/TokenAnnotator.cpp | 21 +++++++------- clang/unittests/Format/FormatTestObjC.cpp | 6 ++++ clang/unittests/Format/TokenAnnotatorTest.cpp | 28 +++++++++++++++++++ 5 files changed, 56 insertions(+), 12 deletions(-) diff --git a/clang/lib/Format/Format.cpp b/clang/lib/Format/Format.cpp index 835071dbe715d..248f72d19e846 100644 --- a/clang/lib/Format/Format.cpp +++ b/clang/lib/Format/Format.cpp @@ -3199,7 +3199,7 @@ class ObjCHeaderStyleGuesser : public TokenAnalyzer { Keywords.kw_NS_OPTIONS, TT_ObjCBlockLBrace, TT_ObjCBlockLParen, TT_ObjCDecl, TT_ObjCForIn, TT_ObjCMethodExpr, TT_ObjCMethodSpecifier, - TT_ObjCProperty)) { + TT_ObjCProperty, TT_ObjCSelector)) { LLVM_DEBUG(llvm::dbgs() << "Detected ObjC at location " << FormatTok->Tok.getLocation().printToString( diff --git a/clang/lib/Format/FormatToken.h b/clang/lib/Format/FormatToken.h index e04b0e7af10c0..9b9af89b54220 100644 --- a/clang/lib/Format/FormatToken.h +++ b/clang/lib/Format/FormatToken.h @@ -127,9 +127,17 @@ namespace format { TYPE(ObjCBlockLParen) \ TYPE(ObjCDecl) \ TYPE(ObjCForIn) \ + /* The square brackets surrounding a method call, the colon separating the \ + * method or parameter name and the argument inside the square brackets, and \ + * the colon separating the method or parameter name and the type inside the \ + * method declaration. */ \ TYPE(ObjCMethodExpr) \ + /* The '+' or '-' at the start of the line. */ \ TYPE(ObjCMethodSpecifier) \ TYPE(ObjCProperty) \ + /* The parentheses following '@selector' and the colon following the method \ + * or parameter name inside the parentheses. */ \ + TYPE(ObjCSelector) \ TYPE(ObjCStringLiteral) \ TYPE(OverloadedOperator) \ TYPE(OverloadedOperatorLParen) \ @@ -146,6 +154,9 @@ namespace format { TYPE(RequiresExpression) \ TYPE(RequiresExpressionLBrace) \ TYPE(RequiresExpressionLParen) \ + /* The hash key in languages that have hash literals, not including the \ + * field name in the C++ struct literal. Also the method or parameter name \ + * in the Objective-C method declaration or call. */ \ TYPE(SelectorName) \ TYPE(StartOfName) \ TYPE(StatementAttributeLikeMacro) \ diff --git a/clang/lib/Format/TokenAnnotator.cpp b/clang/lib/Format/TokenAnnotator.cpp index 6a8286da73442..c047769eefcfd 100644 --- a/clang/lib/Format/TokenAnnotator.cpp +++ b/clang/lib/Format/TokenAnnotator.cpp @@ -321,13 +321,13 @@ class AnnotatingParser { return parseUntouchableParens(); } - bool StartsObjCMethodExpr = false; + bool StartsObjCSelector = false; if (!Style.isVerilog()) { if (FormatToken *MaybeSel = OpeningParen.Previous) { // @selector( starts a selector. if (MaybeSel->is(tok::objc_selector) && MaybeSel->Previous && MaybeSel->Previous->is(tok::at)) { - StartsObjCMethodExpr = true; + StartsObjCSelector = true; } } } @@ -451,10 +451,8 @@ class AnnotatingParser { } } - if (StartsObjCMethodExpr) { - Contexts.back().ColonIsObjCMethodExpr = true; - OpeningParen.setType(TT_ObjCMethodExpr); - } + if (StartsObjCSelector) + OpeningParen.setType(TT_ObjCSelector); // MightBeFunctionType and ProbablyFunctionType are used for // function pointer and reference types as well as Objective-C @@ -513,8 +511,8 @@ class AnnotatingParser { } } - if (StartsObjCMethodExpr) { - CurrentToken->setType(TT_ObjCMethodExpr); + if (StartsObjCSelector) { + CurrentToken->setType(TT_ObjCSelector); if (Contexts.back().FirstObjCSelectorName) { Contexts.back().FirstObjCSelectorName->LongestObjCSelectorName = Contexts.back().LongestObjCSelectorName; @@ -1454,7 +1452,7 @@ class AnnotatingParser { Next->Next->is(tok::colon)))) { // This handles a special macro in ObjC code where selectors including // the colon are passed as macro arguments. - Tok->setType(TT_ObjCMethodExpr); + Tok->setType(TT_ObjCSelector); } break; case tok::pipe: @@ -4609,7 +4607,7 @@ bool TokenAnnotator::spaceRequiredBetween(const AnnotatedLine &Line, return false; } if (Left.is(tok::colon)) - return Left.isNot(TT_ObjCMethodExpr); + return !Left.isOneOf(TT_ObjCSelector, TT_ObjCMethodExpr); if (Left.is(tok::coloncolon)) return false; if (Left.is(tok::less) || Right.isOneOf(tok::greater, tok::less)) { @@ -5465,7 +5463,7 @@ bool TokenAnnotator::spaceRequiredBefore(const AnnotatedLine &Line, // `private:` and `public:`. if (!Right.getNextNonComment()) return false; - if (Right.is(TT_ObjCMethodExpr)) + if (Right.isOneOf(TT_ObjCSelector, TT_ObjCMethodExpr)) return false; if (Left.is(tok::question)) return false; @@ -6289,6 +6287,7 @@ bool TokenAnnotator::canBreakBefore(const AnnotatedLine &Line, return Style.BreakInheritanceList == FormatStyle::BILS_AfterColon; if (Right.is(TT_InheritanceColon)) return Style.BreakInheritanceList != FormatStyle::BILS_AfterColon; + // When the method parameter has no name, allow breaking before the colon. if (Right.is(TT_ObjCMethodExpr) && Right.isNot(tok::r_square) && Left.isNot(TT_SelectorName)) { return true; diff --git a/clang/unittests/Format/FormatTestObjC.cpp b/clang/unittests/Format/FormatTestObjC.cpp index f7f73db62045c..5b7ca60366f1e 100644 --- a/clang/unittests/Format/FormatTestObjC.cpp +++ b/clang/unittests/Format/FormatTestObjC.cpp @@ -763,6 +763,12 @@ TEST_F(FormatTestObjC, FormatObjCMethodExpr) { " backing:NSBackingStoreBuffered\n" " defer:NO]);\n" "}"); + verifyFormat(R"(- (void)test { + if ([object + respondsToSelector:@selector( + selectorNameThatIsReallyLong:param1:param2:)]) + return; +})"); verifyFormat("[contentsContainer replaceSubview:[subviews objectAtIndex:0]\n" " with:contentsNativeView];"); diff --git a/clang/unittests/Format/TokenAnnotatorTest.cpp b/clang/unittests/Format/TokenAnnotatorTest.cpp index 4c43a963632a6..6bf34749d2799 100644 --- a/clang/unittests/Format/TokenAnnotatorTest.cpp +++ b/clang/unittests/Format/TokenAnnotatorTest.cpp @@ -1929,6 +1929,34 @@ TEST_F(TokenAnnotatorTest, UnderstandsObjCMethodExpr) { ASSERT_EQ(Tokens.size(), 20u) << Tokens; EXPECT_TOKEN(Tokens[9], tok::l_square, TT_ObjCMethodExpr); EXPECT_TOKEN(Tokens[15], tok::greater, TT_BinaryOperator); + Tokens = annotate("a = @selector(name:);"); + ASSERT_EQ(Tokens.size(), 10u) << Tokens; + EXPECT_TOKEN(Tokens[4], tok::l_paren, TT_ObjCSelector); + EXPECT_TOKEN(Tokens[6], tok::colon, TT_ObjCSelector); + EXPECT_TOKEN(Tokens[7], tok::r_paren, TT_ObjCSelector); + Tokens = + annotate("[object respondsToSelector:@selector(name:param1:param2:)\n" + " respondsToSelector:@selector(name:param1:param2:)];"); + ASSERT_EQ(Tokens.size(), 29u) << Tokens; + EXPECT_TOKEN(Tokens[0], tok::l_square, TT_ObjCMethodExpr); + EXPECT_TOKEN(Tokens[3], tok::colon, TT_ObjCMethodExpr); + EXPECT_TOKEN(Tokens[6], tok::l_paren, TT_ObjCSelector); + EXPECT_TOKEN(Tokens[8], tok::colon, TT_ObjCSelector); + EXPECT_TOKEN(Tokens[10], tok::colon, TT_ObjCSelector); + EXPECT_TOKEN(Tokens[12], tok::colon, TT_ObjCSelector); + EXPECT_TOKEN(Tokens[13], tok::r_paren, TT_ObjCSelector); + EXPECT_TOKEN(Tokens[15], tok::colon, TT_ObjCMethodExpr); + EXPECT_TOKEN(Tokens[18], tok::l_paren, TT_ObjCSelector); + EXPECT_TOKEN(Tokens[20], tok::colon, TT_ObjCSelector); + EXPECT_TOKEN(Tokens[22], tok::colon, TT_ObjCSelector); + EXPECT_TOKEN(Tokens[24], tok::colon, TT_ObjCSelector); + EXPECT_TOKEN(Tokens[25], tok::r_paren, TT_ObjCSelector); + EXPECT_TOKEN(Tokens[26], tok::r_square, TT_ObjCMethodExpr); + Tokens = annotate("[a b:c];"); + ASSERT_EQ(Tokens.size(), 8u) << Tokens; + EXPECT_TOKEN(Tokens[0], tok::l_square, TT_ObjCMethodExpr); + EXPECT_TOKEN(Tokens[3], tok::colon, TT_ObjCMethodExpr); + EXPECT_TOKEN(Tokens[5], tok::r_square, TT_ObjCMethodExpr); } TEST_F(TokenAnnotatorTest, UnderstandsObjCMethodDecl) { From ebb6d85943ccacdd63e86631802ac2055685804e Mon Sep 17 00:00:00 2001 From: sstwcw Date: Wed, 1 Oct 2025 02:33:09 +0000 Subject: [PATCH 2/2] Address comments --- clang/unittests/Format/FormatTestObjC.cpp | 15 +++++++++------ clang/unittests/Format/TokenAnnotatorTest.cpp | 3 +++ 2 files changed, 12 insertions(+), 6 deletions(-) diff --git a/clang/unittests/Format/FormatTestObjC.cpp b/clang/unittests/Format/FormatTestObjC.cpp index 5b7ca60366f1e..700d7cf8efca6 100644 --- a/clang/unittests/Format/FormatTestObjC.cpp +++ b/clang/unittests/Format/FormatTestObjC.cpp @@ -763,12 +763,15 @@ TEST_F(FormatTestObjC, FormatObjCMethodExpr) { " backing:NSBackingStoreBuffered\n" " defer:NO]);\n" "}"); - verifyFormat(R"(- (void)test { - if ([object - respondsToSelector:@selector( - selectorNameThatIsReallyLong:param1:param2:)]) - return; -})"); + Style.ColumnLimit = 63; + verifyFormat( + "- (void)test {\n" + " if ([object\n" + " respondsToSelector:@selector(\n" + " selectorName:param1:param2:)])\n" + " return;\n" + "}"); + Style.ColumnLimit = PreviousColumnLimit; verifyFormat("[contentsContainer replaceSubview:[subviews objectAtIndex:0]\n" " with:contentsNativeView];"); diff --git a/clang/unittests/Format/TokenAnnotatorTest.cpp b/clang/unittests/Format/TokenAnnotatorTest.cpp index 6bf34749d2799..781a0d687f8cf 100644 --- a/clang/unittests/Format/TokenAnnotatorTest.cpp +++ b/clang/unittests/Format/TokenAnnotatorTest.cpp @@ -1929,11 +1929,13 @@ TEST_F(TokenAnnotatorTest, UnderstandsObjCMethodExpr) { ASSERT_EQ(Tokens.size(), 20u) << Tokens; EXPECT_TOKEN(Tokens[9], tok::l_square, TT_ObjCMethodExpr); EXPECT_TOKEN(Tokens[15], tok::greater, TT_BinaryOperator); + Tokens = annotate("a = @selector(name:);"); ASSERT_EQ(Tokens.size(), 10u) << Tokens; EXPECT_TOKEN(Tokens[4], tok::l_paren, TT_ObjCSelector); EXPECT_TOKEN(Tokens[6], tok::colon, TT_ObjCSelector); EXPECT_TOKEN(Tokens[7], tok::r_paren, TT_ObjCSelector); + Tokens = annotate("[object respondsToSelector:@selector(name:param1:param2:)\n" " respondsToSelector:@selector(name:param1:param2:)];"); @@ -1952,6 +1954,7 @@ TEST_F(TokenAnnotatorTest, UnderstandsObjCMethodExpr) { EXPECT_TOKEN(Tokens[24], tok::colon, TT_ObjCSelector); EXPECT_TOKEN(Tokens[25], tok::r_paren, TT_ObjCSelector); EXPECT_TOKEN(Tokens[26], tok::r_square, TT_ObjCMethodExpr); + Tokens = annotate("[a b:c];"); ASSERT_EQ(Tokens.size(), 8u) << Tokens; EXPECT_TOKEN(Tokens[0], tok::l_square, TT_ObjCMethodExpr);