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) { _______________________________________________ cfe-commits mailing list cfe-commits@lists.llvm.org http://lists.llvm.org/cgi-bin/mailman/listinfo/cfe-commits