[clang-format] Keep the ObjC selector name and @selector together
#160739
Conversation
This file contains hidden or bidirectional Unicode text that may be interpreted or compiled differently than what appears below. To review, open the file in an editor that reveals hidden Unicode characters.
Learn more about bidirectional Unicode characters
Fixes llvm#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.
|
@llvm/pr-subscribers-clang-format Author: None (sstwcw) ChangesFixes #36459. after - (void)test {
if ([object
respondsToSelector:@<!-- -->selector(
selectorNameThatIsReallyLong:param1:param2:)])
return;
}before - (void)test {
if ([object respondsToSelector:@<!-- -->selector
(selectorNameThatIsReallyLong:param1:param2:)])
return;
}Before this patch, the Full diff: https://github.com/llvm/llvm-project/pull/160739.diff 5 Files Affected:
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) {
|
HazardyKnusperkeks
approved these changes
Sep 25, 2025
owenca
reviewed
Sep 27, 2025
owenca
approved these changes
Oct 1, 2025
Sign up for free
to join this conversation on GitHub.
Already have an account?
Sign in to comment
4 participants
Add this suggestion to a batch that can be applied as a single commit.
This suggestion is invalid because no changes were made to the code.
Suggestions cannot be applied while the pull request is closed.
Suggestions cannot be applied while viewing a subset of changes.
Only one suggestion per line can be applied in a batch.
Add this suggestion to a batch that can be applied as a single commit.
Applying suggestions on deleted lines is not supported.
You must change the existing code in this line in order to create a valid suggestion.
Outdated suggestions cannot be applied.
This suggestion has been applied or marked resolved.
Suggestions cannot be applied from pending reviews.
Suggestions cannot be applied on multi-line comments.
Suggestions cannot be applied while the pull request is queued to merge.
Suggestion cannot be applied right now. Please check back later.
Fixes #36459.
after
before
Before this patch, the
ObjCMethodExprtype 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@selectorby 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.