r333553 - [clang-format/ObjC] Correctly parse Objective-C methods with 'class' in name
Ben Hamilton via cfe-commits
cfe-commits at lists.llvm.org
Wed May 30 08:21:38 PDT 2018
Author: benhamilton
Date: Wed May 30 08:21:38 2018
New Revision: 333553
URL: http://llvm.org/viewvc/llvm-project?rev=333553&view=rev
Log:
[clang-format/ObjC] Correctly parse Objective-C methods with 'class' in name
Summary:
Please take a close look at this CL. I haven't touched much of
`UnwrappedLineParser` before, so I may have gotten things wrong.
Previously, clang-format would incorrectly format the following:
```
@implementation Foo
- (Class)class {
}
- (void)foo {
}
@end
```
as:
```
@implementation Foo
- (Class)class {
}
- (void)foo {
}
@end
```
The problem is whenever `UnwrappedLineParser::parseStructuralElement()`
sees any of the keywords `class`, `struct`, or `enum`, it calls
`parseRecord()` to parse them as a C/C++ record.
This causes subsequent lines to be parsed incorrectly, which
causes them to be indented incorrectly.
In Objective-C/Objective-C++, these keywords are valid selector
components.
This diff fixes the issue by explicitly handling `+` and `-` lines
inside `@implementation` / `@interface` / `@protocol` blocks
and parsing them as Objective-C methods.
Test Plan: New tests added. Ran tests with:
make -j16 FormatTests && ./tools/clang/unittests/Format/FormatTests
Reviewers: jolesiak, klimek
Reviewed By: jolesiak, klimek
Subscribers: klimek, cfe-commits, Wizard
Differential Revision: https://reviews.llvm.org/D47095
Modified:
cfe/trunk/lib/Format/UnwrappedLineParser.cpp
cfe/trunk/lib/Format/UnwrappedLineParser.h
cfe/trunk/unittests/Format/FormatTestObjC.cpp
Modified: cfe/trunk/lib/Format/UnwrappedLineParser.cpp
URL: http://llvm.org/viewvc/llvm-project/cfe/trunk/lib/Format/UnwrappedLineParser.cpp?rev=333553&r1=333552&r2=333553&view=diff
==============================================================================
--- cfe/trunk/lib/Format/UnwrappedLineParser.cpp (original)
+++ cfe/trunk/lib/Format/UnwrappedLineParser.cpp Wed May 30 08:21:38 2018
@@ -2130,6 +2130,24 @@ void UnwrappedLineParser::parseRecord(bo
// "} n, m;" will end up in one unwrapped line.
}
+void UnwrappedLineParser::parseObjCMethod() {
+ assert(FormatTok->Tok.isOneOf(tok::l_paren, tok::identifier) &&
+ "'(' or identifier expected.");
+ do {
+ if (FormatTok->Tok.is(tok::semi)) {
+ nextToken();
+ addUnwrappedLine();
+ return;
+ } else if (FormatTok->Tok.is(tok::l_brace)) {
+ parseBlock(/*MustBeDeclaration=*/false);
+ addUnwrappedLine();
+ return;
+ } else {
+ nextToken();
+ }
+ } while (!eof());
+}
+
void UnwrappedLineParser::parseObjCProtocolList() {
assert(FormatTok->Tok.is(tok::less) && "'<' expected.");
do {
@@ -2157,6 +2175,9 @@ void UnwrappedLineParser::parseObjCUntil
// Ignore stray "}". parseStructuralElement doesn't consume them.
nextToken();
addUnwrappedLine();
+ } else if (FormatTok->isOneOf(tok::minus, tok::plus)) {
+ nextToken();
+ parseObjCMethod();
} else {
parseStructuralElement();
}
Modified: cfe/trunk/lib/Format/UnwrappedLineParser.h
URL: http://llvm.org/viewvc/llvm-project/cfe/trunk/lib/Format/UnwrappedLineParser.h?rev=333553&r1=333552&r2=333553&view=diff
==============================================================================
--- cfe/trunk/lib/Format/UnwrappedLineParser.h (original)
+++ cfe/trunk/lib/Format/UnwrappedLineParser.h Wed May 30 08:21:38 2018
@@ -120,6 +120,7 @@ private:
// parses the record as a child block, i.e. if the class declaration is an
// expression.
void parseRecord(bool ParseAsExpr = false);
+ void parseObjCMethod();
void parseObjCProtocolList();
void parseObjCUntilAtEnd();
void parseObjCInterfaceOrImplementation();
Modified: cfe/trunk/unittests/Format/FormatTestObjC.cpp
URL: http://llvm.org/viewvc/llvm-project/cfe/trunk/unittests/Format/FormatTestObjC.cpp?rev=333553&r1=333552&r2=333553&view=diff
==============================================================================
--- cfe/trunk/unittests/Format/FormatTestObjC.cpp (original)
+++ cfe/trunk/unittests/Format/FormatTestObjC.cpp Wed May 30 08:21:38 2018
@@ -328,7 +328,14 @@ TEST_F(FormatTestObjC, FormatObjCInterfa
"}\n"
"+ (id)init;\n"
"@end");
-
+ verifyFormat("@interface Foo\n"
+ "- (void)foo {\n"
+ "}\n"
+ "@end\n"
+ "@implementation Bar\n"
+ "- (void)bar {\n"
+ "}\n"
+ "@end");
Style.ColumnLimit = 40;
verifyFormat("@interface ccccccccccccc () <\n"
" ccccccccccccc, ccccccccccccc,\n"
@@ -969,6 +976,59 @@ TEST_F(FormatTestObjC, ObjCCxxKeywords)
verifyFormat("MACRO(new:)\n");
verifyFormat("MACRO(delete:)\n");
verifyFormat("foo = @{MACRO(new:) : MACRO(delete:)}\n");
+ verifyFormat("@implementation Foo\n"
+ "// Testing\n"
+ "- (Class)class {\n"
+ "}\n"
+ "- (void)foo {\n"
+ "}\n"
+ "@end\n");
+ verifyFormat("@implementation Foo\n"
+ "- (Class)class {\n"
+ "}\n"
+ "- (void)foo {\n"
+ "}\n"
+ "@end");
+ verifyFormat("@implementation Foo\n"
+ "+ (Class)class {\n"
+ "}\n"
+ "- (void)foo {\n"
+ "}\n"
+ "@end");
+ verifyFormat("@implementation Foo\n"
+ "- (Class)class:(Class)klass {\n"
+ "}\n"
+ "- (void)foo {\n"
+ "}\n"
+ "@end");
+ verifyFormat("@implementation Foo\n"
+ "+ (Class)class:(Class)klass {\n"
+ "}\n"
+ "- (void)foo {\n"
+ "}\n"
+ "@end");
+
+ verifyFormat("@interface Foo\n"
+ "// Testing\n"
+ "- (Class)class;\n"
+ "- (void)foo;\n"
+ "@end\n");
+ verifyFormat("@interface Foo\n"
+ "- (Class)class;\n"
+ "- (void)foo;\n"
+ "@end");
+ verifyFormat("@interface Foo\n"
+ "+ (Class)class;\n"
+ "- (void)foo;\n"
+ "@end");
+ verifyFormat("@interface Foo\n"
+ "- (Class)class:(Class)klass;\n"
+ "- (void)foo;\n"
+ "@end");
+ verifyFormat("@interface Foo\n"
+ "+ (Class)class:(Class)klass;\n"
+ "- (void)foo;\n"
+ "@end");
}
TEST_F(FormatTestObjC, ObjCLiterals) {
More information about the cfe-commits
mailing list