-
Notifications
You must be signed in to change notification settings - Fork 15.4k
[clang-format] Add ObjCSpaceBeforeMethodDeclColon option to control space before Objective-C method return type #170579
New issue
Have a question about this project? Sign up for a free GitHub account to open an issue and contact its maintainers and the community.
By clicking “Sign up for GitHub”, you agree to our terms of service and privacy statement. We’ll occasionally send you account related emails.
Already on GitHub? Sign in to your account
base: main
Are you sure you want to change the base?
Conversation
…pace before Objective-C method return type This patch introduces the ObjCSpaceBeforeMethodDeclColon style option, allowing users to add or remove a space between the '-'/'+' and the return type in Objective-C method declarations (e.g., '- (void)method' vs '-(void)method'). Includes documentation and unit tests.
|
Thank you for submitting a Pull Request (PR) to the LLVM Project! This PR will be automatically labeled and the relevant teams will be notified. If you wish to, you can add reviewers by using the "Reviewers" section on this page. If this is not working for you, it is probably because you do not have write permissions for the repository. In which case you can instead tag reviewers by name in a comment by using If you have received no comments on your PR for a week, you can request a review by "ping"ing the PR by adding a comment “Ping”. The common courtesy "ping" rate is once a week. Please remember that you are asking for valuable time from other developers. If you have further questions, they may be answered by the LLVM GitHub User Guide. You can also ask questions in a comment on this PR, on the LLVM Discord or on the forums. |
|
@llvm/pr-subscribers-clang-format Author: None (vkpatel186) Changes[clang-format] Add ObjCSpaceBeforeMethodDeclColon option to control space before Objective-C method return type This patch introduces the ObjCSpaceBeforeMethodDeclColon style option, allowing users to add or remove a space between the '-'/'+' and the return type in Objective-C method declarations (e.g., '- (void)method' vs '-(void)method'). Includes documentation and unit tests. Full diff: https://github.com/llvm/llvm-project/pull/170579.diff 5 Files Affected:
diff --git a/clang/docs/ClangFormatStyleOptions.rst b/clang/docs/ClangFormatStyleOptions.rst
index 4f81a084dd65b..dcd59a8820231 100644
--- a/clang/docs/ClangFormatStyleOptions.rst
+++ b/clang/docs/ClangFormatStyleOptions.rst
@@ -5538,6 +5538,12 @@ the configuration (without a prefix: ``Auto``).
Add a space after ``@property`` in Objective-C, i.e. use
``@property (readonly)`` instead of ``@property(readonly)``.
+.. _ObjCSpaceBeforeMethodDeclColon:
+
+**ObjCSpaceBeforeMethodDeclColon** (``Boolean``) :versionbadge:`clang-format 23` :ref:`¶ <ObjCSpaceBeforeMethodDeclColon>`
+ Add or remove a space between the '-'/'+' and the return type in Objective-C method declarations,
+ i.e. use '- (void)method' instead of '-(void)method'.
+
.. _ObjCSpaceBeforeProtocolList:
**ObjCSpaceBeforeProtocolList** (``Boolean``) :versionbadge:`clang-format 3.7` :ref:`¶ <ObjCSpaceBeforeProtocolList>`
diff --git a/clang/include/clang/Format/Format.h b/clang/include/clang/Format/Format.h
index c7e57d47f9ed1..a0c7310f80ff4 100644
--- a/clang/include/clang/Format/Format.h
+++ b/clang/include/clang/Format/Format.h
@@ -3936,6 +3936,11 @@ struct FormatStyle {
/// \version 3.7
bool ObjCSpaceAfterProperty;
+ /// Add or remove a space between the '-'/'+' and the return type in Objective-C method declarations,
+ /// i.e. use '- (void)method' instead of '-(void)method'.
+ /// \version 23
+ bool ObjCSpaceBeforeMethodDeclColon;
+
/// Add a space in front of an Objective-C protocol list, i.e. use
/// ``Foo <Protocol>`` instead of ``Foo<Protocol>``.
/// \version 3.7
@@ -5845,7 +5850,8 @@ struct FormatStyle {
VerilogBreakBetweenInstancePorts ==
R.VerilogBreakBetweenInstancePorts &&
WhitespaceSensitiveMacros == R.WhitespaceSensitiveMacros &&
- WrapNamespaceBodyWithEmptyLines == R.WrapNamespaceBodyWithEmptyLines;
+ WrapNamespaceBodyWithEmptyLines == R.WrapNamespaceBodyWithEmptyLines &&
+ ObjCSpaceBeforeMethodDeclColon == R.ObjCSpaceBeforeMethodDeclColon;
}
std::optional<FormatStyle> GetLanguageStyle(LanguageKind Language) const;
diff --git a/clang/lib/Format/Format.cpp b/clang/lib/Format/Format.cpp
index f0e9aff2fd21a..db184da18cc99 100644
--- a/clang/lib/Format/Format.cpp
+++ b/clang/lib/Format/Format.cpp
@@ -1353,6 +1353,8 @@ template <> struct MappingTraits<FormatStyle> {
Style.WhitespaceSensitiveMacros);
IO.mapOptional("WrapNamespaceBodyWithEmptyLines",
Style.WrapNamespaceBodyWithEmptyLines);
+ IO.mapOptional("ObjCSpaceBeforeMethodDeclColon",
+ Style.ObjCSpaceBeforeMethodDeclColon);
// If AlwaysBreakAfterDefinitionReturnType was specified but
// BreakAfterReturnType was not, initialize the latter from the former for
@@ -1788,6 +1790,7 @@ FormatStyle getLLVMStyle(FormatStyle::LanguageKind Language) {
LLVMStyle.ObjCBlockIndentWidth = 2;
LLVMStyle.ObjCBreakBeforeNestedBlockParam = true;
LLVMStyle.ObjCSpaceAfterProperty = false;
+ LLVMStyle.ObjCSpaceBeforeMethodDeclColon = true;
LLVMStyle.ObjCSpaceBeforeProtocolList = true;
LLVMStyle.PackConstructorInitializers = FormatStyle::PCIS_BinPack;
LLVMStyle.PointerAlignment = FormatStyle::PAS_Right;
diff --git a/clang/lib/Format/TokenAnnotator.cpp b/clang/lib/Format/TokenAnnotator.cpp
index 79cfa73001e54..af48d804b4518 100644
--- a/clang/lib/Format/TokenAnnotator.cpp
+++ b/clang/lib/Format/TokenAnnotator.cpp
@@ -5437,7 +5437,7 @@ bool TokenAnnotator::spaceRequiredBefore(const AnnotatedLine &Line,
return Right.hasWhitespaceBefore();
if (Line.Type == LT_ObjCMethodDecl) {
if (Left.is(TT_ObjCMethodSpecifier))
- return true;
+ return Style.ObjCSpaceBeforeMethodDeclColon;
if (Left.is(tok::r_paren) && Left.isNot(TT_AttributeRParen) &&
canBeObjCSelectorComponent(Right)) {
// Don't space between ')' and <id> or ')' and 'new'. 'new' is not a
diff --git a/clang/unittests/Format/FormatTest.cpp b/clang/unittests/Format/FormatTest.cpp
index 3ff784035dd44..bd98051872fcf 100644
--- a/clang/unittests/Format/FormatTest.cpp
+++ b/clang/unittests/Format/FormatTest.cpp
@@ -15623,6 +15623,22 @@ TEST_F(FormatTest, FormatForObjectiveCMethodDecls) {
verifyGoogleFormat("- foo:(int)foo;");
}
+TEST_F(FormatTest, SpaceBeforeObjCMethodDeclColon) {
+ FormatStyle Style = getLLVMStyle();
+ verifyFormat("- (void)method;", "-(void)method;", Style);
+ verifyFormat("+ (int)foo:(int)x;", "+ (int) foo:(int)x;", Style);
+ verifyFormat("- foo;", "-foo;", Style);
+ verifyFormat("- foo:(int)f;", "-foo:(int)f;", Style);
+
+ Style.ObjCSpaceBeforeMethodDeclColon = false;
+ verifyFormat("-(void)method;", "- (void) method;", Style);
+ verifyFormat("+(int)foo:(int)x;", "+ (int)foo:(int)x;", Style);
+ verifyFormat("+(int)foo:(int)x;", "+ (int)foo:(int)x;", Style);
+
+ verifyFormat("-foo;", "- foo;", Style);
+ verifyFormat("-foo:(int)f;", "- foo:(int)f;", Style);
+}
+
TEST_F(FormatTest, BreaksStringLiterals) {
// FIXME: unstable test case
EXPECT_EQ("\"some text \"\n"
|
HazardyKnusperkeks
left a comment
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
Please add a note to the release notes.
clang/include/clang/Format/Format.h
Outdated
|
|
||
| /// Add or remove a space between the '-'/'+' and the return type in Objective-C method declarations, | ||
| /// i.e. use '- (void)method' instead of '-(void)method'. | ||
| /// \version 23 |
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
That still would be 22.
clang/include/clang/Format/Format.h
Outdated
| bool ObjCSpaceAfterProperty; | ||
|
|
||
| /// Add or remove a space between the '-'/'+' and the return type in Objective-C method declarations, | ||
| /// i.e. use '- (void)method' instead of '-(void)method'. |
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
Please add a code section with true vs false.
clang/include/clang/Format/Format.h
Outdated
| WhitespaceSensitiveMacros == R.WhitespaceSensitiveMacros && | ||
| WrapNamespaceBodyWithEmptyLines == R.WrapNamespaceBodyWithEmptyLines; | ||
| WrapNamespaceBodyWithEmptyLines == R.WrapNamespaceBodyWithEmptyLines && | ||
| ObjCSpaceBeforeMethodDeclColon == R.ObjCSpaceBeforeMethodDeclColon; |
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
Please sort alphabetically.
clang/lib/Format/Format.cpp
Outdated
| Style.WhitespaceSensitiveMacros); | ||
| IO.mapOptional("WrapNamespaceBodyWithEmptyLines", | ||
| Style.WrapNamespaceBodyWithEmptyLines); | ||
| IO.mapOptional("ObjCSpaceBeforeMethodDeclColon", |
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
Sort
|
|
||
| TEST_F(FormatTest, SpaceBeforeObjCMethodDeclColon) { | ||
| FormatStyle Style = getLLVMStyle(); | ||
| verifyFormat("- (void)method;", "-(void)method;", Style); |
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
| verifyFormat("- (void)method;", "-(void)method;", Style); | |
| verifyFormat("- (void)method;", Style); |
Should suffice, or what is the benefit of the other tests?
| verifyFormat("- foo:(int)f;", "-foo:(int)f;", Style); | ||
|
|
||
| Style.ObjCSpaceBeforeMethodDeclColon = false; | ||
| verifyFormat("-(void)method;", "- (void) method;", Style); |
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
| verifyFormat("-(void)method;", "- (void) method;", Style); | |
| verifyFormat("-(void)method;", Style); |
Same here.
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
Just wanted to confirm that these changes don’t impact any other code paths. I aimed to cover all possible combinations.
|
✅ With the latest revision this PR passed the C/C++ code formatter. |
| ObjCPropertyAttributeOrder == R.ObjCPropertyAttributeOrder && | ||
| ObjCSpaceAfterProperty == R.ObjCSpaceAfterProperty && | ||
| ObjCSpaceBeforeProtocolList == R.ObjCSpaceBeforeProtocolList && | ||
| ObjCSpaceBeforeMethodDeclColon == R.ObjCSpaceBeforeMethodDeclColon && |
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
Still one line up. :)
owenca
left a comment
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
Please link a public style guide that wants no space after the +/- prefix. See https://clang.llvm.org/docs/ClangFormatStyleOptions.html#adding-additional-style-options. Also, add a CHECK_PARSE_BOOL test.
| /// -(void)method vs. - (void)method | ||
| /// \endcode | ||
| /// \version 22 | ||
| bool ObjCSpaceBeforeMethodDeclColon; |
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
How about ObjCSpaceAfterMethodDeclarationPrefix instead?
| FormatStyle Style = getLLVMStyle(); | ||
| verifyFormat("- (void)method;", "-(void)method;", Style); | ||
| verifyFormat("+ (int)foo:(int)x;", "+ (int) foo:(int)x;", Style); | ||
| verifyFormat("- foo;", "-foo;", Style); | ||
| verifyFormat("- foo:(int)f;", "-foo:(int)f;", Style); | ||
|
|
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
| FormatStyle Style = getLLVMStyle(); | |
| verifyFormat("- (void)method;", "-(void)method;", Style); | |
| verifyFormat("+ (int)foo:(int)x;", "+ (int) foo:(int)x;", Style); | |
| verifyFormat("- foo;", "-foo;", Style); | |
| verifyFormat("- foo:(int)f;", "-foo:(int)f;", Style); | |
| auto Style = getLLVMStyle(); | |
| EXPECT_TRUE(Style. ObjCSpaceBeforeMethodDeclColon); |
| verifyFormat("+(int)foo:(int)x;", "+ (int)foo:(int)x;", Style); | ||
| verifyFormat("+(int)foo:(int)x;", "+ (int)foo:(int)x;", Style); | ||
|
|
||
| verifyFormat("-foo;", "- foo;", Style); | ||
| verifyFormat("-foo:(int)f;", "- foo:(int)f;", Style); |
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
Delete.
[clang-format] Add ObjCSpaceBeforeMethodDeclColon option to control space before Objective-C method return type
This patch introduces the ObjCSpaceBeforeMethodDeclColon style option, allowing users to add or remove a space between the '-'/'+' and the return type in Objective-C method declarations (e.g., '- (void)method' vs '-(void)method').
Includes documentation and unit tests.