Index: clang/docs/ClangFormatStyleOptions.rst =================================================================== --- clang/docs/ClangFormatStyleOptions.rst +++ clang/docs/ClangFormatStyleOptions.rst @@ -3916,6 +3916,29 @@ }] } +.. _ObjCPropertyAttributeOrder: + +**ObjCPropertyAttributeOrder** (``List of Strings``) :versionbadge:`clang-format 17` :ref:`¶ ` + The order in which ObjC property attributes should appear. + + Attributes in code will be sorted in the order specified. Any attributes + encountered that are not mentioned in this array will be sorted last, in + stable order. Duplicate attributes will be removed, but no other conflict + checking is performed. A leading or trailing comment is allowed to the + whole set, but comments encountered between attributes will leave the + entire set untouched. + + + .. code-block:: yaml + + ObjCPropertyAttributeOrder: [ + class, direct, + atomic, nonatomic, + assign, retain, strong, copy, weak, unsafe_unretained, + readonly, readwrite, getter, setter, + nullable, nonnull, null_resettable, null_unspecified + ] + .. _ObjCSpaceAfterProperty: **ObjCSpaceAfterProperty** (``Boolean``) :versionbadge:`clang-format 3.7` :ref:`¶ ` Index: clang/docs/ReleaseNotes.rst =================================================================== --- clang/docs/ReleaseNotes.rst +++ clang/docs/ReleaseNotes.rst @@ -555,6 +555,8 @@ - Fix all known issues associated with ``LambdaBodyIndentation: OuterScope``. - Add ``BracedInitializerIndentWidth`` which can be used to configure the indentation level of the contents of braced init lists. +- Add ``ObjCPropertyAttributeOrder`` which can be used to sort ObjC property + attributes (like ``nonatomic, strong, nullable``). libclang -------- Index: clang/docs/tools/clang-formatted-files.txt =================================================================== --- clang/docs/tools/clang-formatted-files.txt +++ clang/docs/tools/clang-formatted-files.txt @@ -458,6 +458,8 @@ clang/lib/Format/Macros.h clang/lib/Format/NamespaceEndCommentsFixer.cpp clang/lib/Format/NamespaceEndCommentsFixer.h +clang/lib/Format/ObjCPropertyAttributeOrderFixer.h +clang/lib/Format/ObjCPropertyAttributeOrderFixer.cpp clang/lib/Format/QualifierAlignmentFixer.cpp clang/lib/Format/QualifierAlignmentFixer.h clang/lib/Format/SortJavaScriptImports.cpp @@ -672,6 +674,7 @@ clang/unittests/Format/FormatTestUtils.h clang/unittests/Format/MacroExpanderTest.cpp clang/unittests/Format/NamespaceEndCommentsFixerTest.cpp +clang/unittests/Format/ObjCPropertyAttributeOrderFixerTest.cpp clang/unittests/Format/QualifierFixerTest.cpp clang/unittests/Format/SortImportsTestJava.cpp clang/unittests/Format/SortImportsTestJS.cpp Index: clang/include/clang/Format/Format.h =================================================================== --- clang/include/clang/Format/Format.h +++ clang/include/clang/Format/Format.h @@ -2986,6 +2986,27 @@ /// \version 11 bool ObjCBreakBeforeNestedBlockParam; + /// The order in which ObjC property attributes should appear. + /// + /// Attributes in code will be sorted in the order specified. Any attributes + /// encountered that are not mentioned in this array will be sorted last, in + /// stable order. Duplicate attributes will be removed, but no other conflict + /// checking is performed. A leading or trailing comment is allowed to the + /// whole set, but comments encountered between attributes will leave the + /// entire set untouched. + /// + /// \code{.yaml} + /// ObjCPropertyAttributeOrder: [ + /// class, direct, + /// atomic, nonatomic, + /// assign, retain, strong, copy, weak, unsafe_unretained, + /// readonly, readwrite, getter, setter, + /// nullable, nonnull, null_resettable, null_unspecified + /// ] + /// \endcode + /// \version 17 + std::vector ObjCPropertyAttributeOrder; + /// Add a space after ``@property`` in Objective-C, i.e. use /// ``@property (readonly)`` instead of ``@property(readonly)``. /// \version 3.7 @@ -4379,6 +4400,7 @@ ObjCBlockIndentWidth == R.ObjCBlockIndentWidth && ObjCBreakBeforeNestedBlockParam == R.ObjCBreakBeforeNestedBlockParam && + ObjCPropertyAttributeOrder == R.ObjCPropertyAttributeOrder && ObjCSpaceAfterProperty == R.ObjCSpaceAfterProperty && ObjCSpaceBeforeProtocolList == R.ObjCSpaceBeforeProtocolList && PackConstructorInitializers == R.PackConstructorInitializers && Index: clang/lib/Format/CMakeLists.txt =================================================================== --- clang/lib/Format/CMakeLists.txt +++ clang/lib/Format/CMakeLists.txt @@ -12,6 +12,7 @@ MacroCallReconstructor.cpp MacroExpander.cpp NamespaceEndCommentsFixer.cpp + ObjCPropertyAttributeOrderFixer.cpp QualifierAlignmentFixer.cpp SortJavaScriptImports.cpp TokenAnalyzer.cpp Index: clang/lib/Format/Format.cpp =================================================================== --- clang/lib/Format/Format.cpp +++ clang/lib/Format/Format.cpp @@ -22,6 +22,7 @@ #include "FormatTokenLexer.h" #include "IntegerLiteralSeparatorFixer.h" #include "NamespaceEndCommentsFixer.h" +#include "ObjCPropertyAttributeOrderFixer.h" #include "QualifierAlignmentFixer.h" #include "SortJavaScriptImports.h" #include "TokenAnalyzer.h" @@ -952,6 +953,8 @@ IO.mapOptional("ObjCBlockIndentWidth", Style.ObjCBlockIndentWidth); IO.mapOptional("ObjCBreakBeforeNestedBlockParam", Style.ObjCBreakBeforeNestedBlockParam); + IO.mapOptional("ObjCPropertyAttributeOrder", + Style.ObjCPropertyAttributeOrder); IO.mapOptional("ObjCSpaceAfterProperty", Style.ObjCSpaceAfterProperty); IO.mapOptional("ObjCSpaceBeforeProtocolList", Style.ObjCSpaceBeforeProtocolList); @@ -3482,6 +3485,12 @@ }); } + if (!Style.ObjCPropertyAttributeOrder.empty()) { + Passes.emplace_back([&](const Environment &Env) { + return ObjCPropertyAttributeOrderFixer(Env, Expanded).process(); + }); + } + if (Style.InsertBraces) { FormatStyle S = Expanded; S.InsertBraces = true; Index: clang/lib/Format/ObjCPropertyAttributeOrderFixer.h =================================================================== --- /dev/null +++ clang/lib/Format/ObjCPropertyAttributeOrderFixer.h @@ -0,0 +1,53 @@ +//===--- ObjCPropertyAttributeOrderFixer.h ------------------------------*- C++ +//-*-===// +// +// Part of the LLVM Project, under the Apache License v2.0 with LLVM Exceptions. +// See https://llvm.org/LICENSE.txt for license information. +// SPDX-License-Identifier: Apache-2.0 WITH LLVM-exception +// +//===----------------------------------------------------------------------===// +/// +/// \file +/// This file declares ObjCPropertyAttributeOrderFixer, a TokenAnalyzer that +/// adjusts the order of attributes in an ObjC `@property(...)` declaration, +/// depending on the style. +/// +//===----------------------------------------------------------------------===// + +#ifndef LLVM_CLANG_LIB_FORMAT_OBJCPROPERTYATTRIBUTEORDERFIXER_H +#define LLVM_CLANG_LIB_FORMAT_OBJCPROPERTYATTRIBUTEORDERFIXER_H + +#include "TokenAnalyzer.h" +#include "llvm/ADT/StringMap.h" + +namespace clang { +namespace format { + +class ObjCPropertyAttributeOrderFixer : public TokenAnalyzer { + llvm::StringMap SortOrderMap; + unsigned SortOrderMax; + + const FormatToken *analyzeObjCPropertyDecl(const SourceManager &SourceMgr, + const AdditionalKeywords &Keywords, + tooling::Replacements &Fixes, + const FormatToken *Tok) const; + + void sortPropertyAttributes(const SourceManager &SourceMgr, + tooling::Replacements &Fixes, + const FormatToken *LParenTok, + const FormatToken *RParenTok) const; + +public: + ObjCPropertyAttributeOrderFixer(const Environment &Env, + const FormatStyle &Style); + + std::pair + analyze(TokenAnnotator &Annotator, + SmallVectorImpl &AnnotatedLines, + FormatTokenLexer &Tokens) override; +}; + +} // end namespace format +} // end namespace clang + +#endif Index: clang/lib/Format/ObjCPropertyAttributeOrderFixer.cpp =================================================================== --- /dev/null +++ clang/lib/Format/ObjCPropertyAttributeOrderFixer.cpp @@ -0,0 +1,205 @@ +//===--- ObjCPropertyAttributeOrderFixer.cpp -------------------*- C++--*-===// +// +// Part of the LLVM Project, under the Apache License v2.0 with LLVM Exceptions. +// See https://llvm.org/LICENSE.txt for license information. +// SPDX-License-Identifier: Apache-2.0 WITH LLVM-exception +// +//===----------------------------------------------------------------------===// +/// +/// \file +/// This file implements ObjCPropertyAttributeOrderFixer, a TokenAnalyzer that +/// adjusts the order of attributes in an ObjC `@property(...)` declaration, +/// depending on the style. +/// +//===----------------------------------------------------------------------===// + +#include "ObjCPropertyAttributeOrderFixer.h" + +#include "FormatToken.h" +#include "llvm/ADT/Sequence.h" +#include "llvm/Support/Debug.h" + +#include + +#define DEBUG_TYPE "format-objc-property-attribute-order-fixer" + +namespace clang { +namespace format { + +ObjCPropertyAttributeOrderFixer::ObjCPropertyAttributeOrderFixer( + const Environment &Env, const FormatStyle &Style) + : TokenAnalyzer(Env, Style) { + + // Create an "order priority" map to use to sort properties. + unsigned index = 0; + for (auto const &Property : Style.ObjCPropertyAttributeOrder) + SortOrderMap[Property] = index++; + // A sentinel value bigger than all others (used to sort unknown ones to the + // end). + SortOrderMax = index; +} + +struct ObjCPropertyEntry { + StringRef Attribute; // eg, "readwrite" + StringRef Value; // eg, the "foo" of the attribute "getter=foo" +}; + +static bool isObjCPropertyAttribute(const FormatToken *Tok) { + // Most attributes look like identifiers, but `class` is a keyword. + return Tok->isOneOf(tok::identifier, tok::kw_class); +} + +void ObjCPropertyAttributeOrderFixer::sortPropertyAttributes( + const SourceManager &SourceMgr, tooling::Replacements &Fixes, + const FormatToken *LParenTok, const FormatToken *RParenTok) const { + // Skip past any leading comments. + const FormatToken *const BeginTok = LParenTok->getNextNonComment(); + + // Block out any trailing comments. ("End" marks a left-closed interval, so + // store one-past-last) This will point to either the right-paren, or a + // comment (if there were multiple trailing comments). + const FormatToken *const EndTok = RParenTok->getPreviousNonComment()->Next; + assert(EndTok->isOneOf(tok::r_paren, tok::comment) && + "Expect the range to be bounded by comment or paren"); + + // If there are zero or one elements, nothing to do. + if (BeginTok == EndTok || BeginTok->Next == EndTok) + return; + + // Collect the attributes. + SmallVector PropertyAttributes; + for (auto Tok = BeginTok; Tok != EndTok; Tok = Tok->Next) { + if (Tok->is(tok::comma)) { + // Ignore the comma separators. + continue; + } else if (isObjCPropertyAttribute(Tok)) { + // Memoize the attribute. (Note that 'class' is a legal attribute!) + PropertyAttributes.push_back({Tok->TokenText.trim(), StringRef{}}); + + // Also handle `getter=getFoo` attributes. + // (Note: no check needed against `EndTok`, since its type is not + // BinaryOperator or Identifier) + if (Tok->Next->is(tok::equal)) { + Tok = Tok->Next; + if (Tok->Next->is(tok::identifier)) { + Tok = Tok->Next; + PropertyAttributes.back().Value = Tok->TokenText.trim(); + } else { + // If we hit any other kind of token, just bail. It's unusual/illegal. + return; + } + } + } else { + // If we hit any other kind of token, just bail. + return; + } + } + + // Create a "remapping index" on how to reorder the attributes. + SmallVector Indices = + llvm::to_vector<8>(llvm::seq(0, PropertyAttributes.size())); + + // Sort the indices based on the priority stored in 'SortOrderMap'; use Max + // for missing values. + auto sortIndex = [&](const StringRef &needle) -> unsigned { + auto i = SortOrderMap.find(needle); + return (i == SortOrderMap.end()) ? SortOrderMax : i->getValue(); + }; + llvm::stable_sort(Indices, [&](unsigned LHSI, unsigned RHSI) { + return sortIndex(PropertyAttributes[LHSI].Attribute) < + sortIndex(PropertyAttributes[RHSI].Attribute); + }); + + // Deduplicate the attributes. + Indices.erase(std::unique(Indices.begin(), Indices.end(), + [&](unsigned LHSI, unsigned RHSI) { + return PropertyAttributes[LHSI].Attribute == + PropertyAttributes[RHSI].Attribute; + }), + Indices.end()); + + // If there are no removals or shuffling, then don't suggest any fixup. + if (Indices.size() == PropertyAttributes.size() && llvm::is_sorted(Indices)) + return; + + // Generate the replacement text. + std::string NewText; + for (unsigned Index : Indices) { + if (!NewText.empty()) + NewText += ", "; + + NewText += PropertyAttributes[Index].Attribute; + + if (!PropertyAttributes[Index].Value.empty()) { + NewText += "="; + NewText += PropertyAttributes[Index].Value; + } + } + + auto Range = CharSourceRange::getCharRange( + BeginTok->getStartOfNonWhitespace(), EndTok->Previous->Tok.getEndLoc()); + auto Replacement = tooling::Replacement(SourceMgr, Range, NewText); + auto Err = Fixes.add(Replacement); + if (Err) { + llvm::errs() << "Error while reodering ObjC property attributes : " + << llvm::toString(std::move(Err)) << "\n"; + } +} + +const FormatToken *ObjCPropertyAttributeOrderFixer::analyzeObjCPropertyDecl( + const SourceManager &SourceMgr, const AdditionalKeywords &Keywords, + tooling::Replacements &Fixes, const FormatToken *const Tok) const { + // Expect `property` to be the very next token or else just bail early. + const FormatToken *const PropertyTok = Tok->Next; + if (!PropertyTok || PropertyTok->TokenText != "property") + return Tok; + + // Expect the opening paren to be the next token or else just bail early. + const FormatToken *const LParenTok = PropertyTok->getNextNonComment(); + if (!LParenTok || LParenTok->isNot(tok::l_paren)) + return Tok; + + // Get the matching right-paren, the bounds for property attributes. + const FormatToken *const RParenTok = LParenTok->MatchingParen; + if (!RParenTok) + return Tok; + + sortPropertyAttributes(SourceMgr, Fixes, LParenTok, RParenTok); + + // Return the final token since we can skip past everything in between. + return RParenTok; +} + +std::pair +ObjCPropertyAttributeOrderFixer::analyze( + TokenAnnotator & /*Annotator*/, + SmallVectorImpl &AnnotatedLines, + FormatTokenLexer &Tokens) { + tooling::Replacements Fixes; + const AdditionalKeywords &Keywords = Tokens.getKeywords(); + const SourceManager &SourceMgr = Env.getSourceManager(); + AffectedRangeMgr.computeAffectedLines(AnnotatedLines); + + for (AnnotatedLine *Line : AnnotatedLines) { + if (!Line->Affected || Line->InPPDirective) + continue; + FormatToken *First = Line->First; + assert(First); + if (First->Finalized) + continue; + + const auto *Last = Line->Last; + + for (const auto *Tok = First; Tok && Tok != Last && Tok->Next; + Tok = Tok->Next) { + // Skip until the `@` of a `@property` declaration. + if (Tok->isNot(TT_ObjCProperty)) + continue; + Tok = analyzeObjCPropertyDecl(SourceMgr, Keywords, Fixes, Tok); + } + } + return {Fixes, 0}; +} + +} // namespace format +} // namespace clang Index: clang/lib/Format/QualifierAlignmentFixer.h =================================================================== --- clang/lib/Format/QualifierAlignmentFixer.h +++ clang/lib/Format/QualifierAlignmentFixer.h @@ -1,4 +1,4 @@ -//===--- LeftRightQualifierAlignmentFixer.h ------------------------------*- C++ +//===--- QualifierAlignmentFixer.h ------------------------------*- C++ //-*-===// // // Part of the LLVM Project, under the Apache License v2.0 with LLVM Exceptions. @@ -8,7 +8,7 @@ //===----------------------------------------------------------------------===// /// /// \file -/// This file declares LeftRightQualifierAlignmentFixer, a TokenAnalyzer that +/// This file declares QualifierAlignmentFixer, a TokenAnalyzer that /// enforces either east or west const depending on the style. /// //===----------------------------------------------------------------------===// Index: clang/lib/Format/QualifierAlignmentFixer.cpp =================================================================== --- clang/lib/Format/QualifierAlignmentFixer.cpp +++ clang/lib/Format/QualifierAlignmentFixer.cpp @@ -1,4 +1,4 @@ -//===--- LeftRightQualifierAlignmentFixer.cpp -------------------*- C++--*-===// +//===--- QualifierAlignmentFixer.cpp ----------------------------*- C++--*-===// // // Part of the LLVM Project, under the Apache License v2.0 with LLVM Exceptions. // See https://llvm.org/LICENSE.txt for license information. @@ -7,7 +7,7 @@ //===----------------------------------------------------------------------===// /// /// \file -/// This file implements LeftRightQualifierAlignmentFixer, a TokenAnalyzer that +/// This file implements QualifierAlignmentFixer, a TokenAnalyzer that /// enforces either left or right const depending on the style. /// //===----------------------------------------------------------------------===// @@ -248,7 +248,7 @@ // The cases: // `Foo() const` -> `Foo() const` // `Foo() const final` -> `Foo() const final` - // `Foo() const override` -> `Foo() const final` + // `Foo() const override` -> `Foo() const override` // `Foo() const volatile override` -> `Foo() const volatile override` // `Foo() volatile const final` -> `Foo() const volatile final` if (PreviousCheck->is(tok::r_paren)) Index: clang/unittests/Format/CMakeLists.txt =================================================================== --- clang/unittests/Format/CMakeLists.txt +++ clang/unittests/Format/CMakeLists.txt @@ -28,6 +28,7 @@ MacroCallReconstructorTest.cpp MacroExpanderTest.cpp NamespaceEndCommentsFixerTest.cpp + ObjCPropertyAttributeOrderFixerTest.cpp QualifierFixerTest.cpp SortImportsTestJS.cpp SortImportsTestJava.cpp Index: clang/unittests/Format/ObjCPropertyAttributeOrderFixerTest.cpp =================================================================== --- /dev/null +++ clang/unittests/Format/ObjCPropertyAttributeOrderFixerTest.cpp @@ -0,0 +1,207 @@ +//===- unittest/Format/ObjCPropertyAttributeOrderFixerTest.cpp - unit tests +// +// Part of the LLVM Project, under the Apache License v2.0 with LLVM Exceptions. +// See https://llvm.org/LICENSE.txt for license information. +// SPDX-License-Identifier: Apache-2.0 WITH LLVM-exception +// +//===----------------------------------------------------------------------===// + +#include "../lib/Format/ObjCPropertyAttributeOrderFixer.h" +#include "FormatTestBase.h" +#include "TestLexer.h" + +#define DEBUG_TYPE "format-objc-property-attribute-order-fixer-test" + +namespace clang { +namespace format { +namespace test { +namespace { + +#define CHECK_PARSE(TEXT, FIELD, VALUE) \ + EXPECT_NE(VALUE, Style.FIELD) << "Initial value already the same!"; \ + EXPECT_EQ(0, parseConfiguration(TEXT, &Style).value()); \ + EXPECT_EQ(VALUE, Style.FIELD) << "Unexpected value after parsing!" + +#define FAIL_PARSE(TEXT, FIELD, VALUE) \ + EXPECT_NE(0, parseConfiguration(TEXT, &Style).value()); \ + EXPECT_EQ(VALUE, Style.FIELD) << "Unexpected value after parsing!" + +class ObjCPropertyAttributeOrderFixerTest : public FormatTestBase { +protected: + TokenList annotate(llvm::StringRef Code, + const FormatStyle &Style = getLLVMStyle()) { + return TestLexer(Allocator, Buffers, Style).annotate(Code); + } + + static std::vector getAllObjCAttributes() { + // These are all the ObjC property attributes that are currently supported in ObjC. + // The Fixer doesn't actually know these, it just accepts whatever tokens the user provides. + // These are specified here just to be exhaustive on the tokens that are expected, and to + // make sure they are handled correctly. For example, 'class' is a keyword, so it could + // get trapped in an unexpected way. + return { + "class", "direct", "atomic", "nonatomic", + "assign", "retain", "strong", "copy", "weak", "unsafe_unretained", + "readonly", "readwrite", "getter", "setter", + "nullable", "nonnull", "null_resettable", "null_unspecified", + }; + } + + llvm::SpecificBumpPtrAllocator Allocator; + std::vector> Buffers; +}; + +TEST_F(ObjCPropertyAttributeOrderFixerTest, ParsesStyleOption) { + FormatStyle Style = {}; + Style.Language = FormatStyle::LK_ObjC; + + CHECK_PARSE("ObjCPropertyAttributeOrder: [class]", ObjCPropertyAttributeOrder, + std::vector({"class"})); + + CHECK_PARSE("ObjCPropertyAttributeOrder: [" + llvm::join(getAllObjCAttributes(), ",") + "]", + ObjCPropertyAttributeOrder, + getAllObjCAttributes()); +} + +TEST_F(ObjCPropertyAttributeOrderFixerTest, SortsSpecifiedAttributes) { + FormatStyle Style = getLLVMStyle(); + Style.ObjCPropertyAttributeOrder = {"a", "b", "c"}; + + verifyFormat("@property() int p;", Style); + + // One: shouldn't move. + verifyFormat("@property(a) int p;", Style); + verifyFormat("@property(b) int p;", Style); + verifyFormat("@property(c) int p;", Style); + + // Two in correct order already: no change. + verifyFormat("@property(a, b) int p;", Style); + verifyFormat("@property(a, c) int p;", Style); + verifyFormat("@property(b, c) int p;", Style); + + // Three in correct order already: no change. + verifyFormat("@property(a, b, c) int p;", Style); + + // Two wrong order. + verifyFormat("@property(a, b) int p;", "@property(b, a) int p;", Style); + verifyFormat("@property(a, c) int p;", "@property(c, a) int p;", Style); + verifyFormat("@property(b, c) int p;", "@property(c, b) int p;", Style); + + // Three wrong order. + verifyFormat("@property(a, b, c) int p;", "@property(b, a, c) int p;", Style); + verifyFormat("@property(a, b, c) int p;", "@property(c, b, a) int p;", Style); +} + +TEST_F(ObjCPropertyAttributeOrderFixerTest, SortsAttributesWithValues) { + FormatStyle Style = getLLVMStyle(); + Style.ObjCPropertyAttributeOrder = {"a", "getter", "c"}; + + // No change + verifyFormat("@property(getter=G, c) int p;", Style); + verifyFormat("@property(a, getter=G) int p;", Style); + verifyFormat("@property(a, getter=G, c) int p;", Style); + + // Reorder + verifyFormat("@property(getter=G, c) int p;", "@property(c, getter=G) int p;", + Style); + verifyFormat("@property(a, getter=G) int p;", "@property(getter=G, a) int p;", + Style); + verifyFormat("@property(a, getter=G, c) int p;", + "@property(getter=G, c, a) int p;", Style); + + // Multiple set properties, including ones not recognized + verifyFormat("@property(a=A, c=C, x=X, y=Y) int p;", + "@property(c=C, x=X, y=Y, a=A) int p;", Style); +} + +TEST_F(ObjCPropertyAttributeOrderFixerTest, SortsUnspecifiedAttributesToBack) { + FormatStyle Style = getLLVMStyle(); + Style.ObjCPropertyAttributeOrder = {"a", "b", "c"}; + + verifyFormat("@property(x) int p;", Style); + + // No change in order. + verifyFormat("@property(a, x, y) int p;", Style); + verifyFormat("@property(b, x, y) int p;", Style); + verifyFormat("@property(a, b, c, x, y) int p;", Style); + + // Reorder one unrecognized one. + verifyFormat("@property(a, x) int p;", "@property(x, a) int p;", Style); + + // Prove the unrecognized ones have a stable sort order + verifyFormat("@property(a, b, x, y) int p;", "@property(x, b, y, a) int p;", + Style); + verifyFormat("@property(a, b, y, x) int p;", "@property(y, b, x, a) int p;", + Style); +} + +TEST_F(ObjCPropertyAttributeOrderFixerTest, RemovesDuplicateAttributes) { + FormatStyle Style = getLLVMStyle(); + Style.ObjCPropertyAttributeOrder = {"a", "b", "c"}; + + verifyFormat("@property(a) int p;", "@property(a, a) int p;", Style); + verifyFormat("@property(a) int p;", "@property(a, a, a, a) int p;", Style); + + verifyFormat("@property(a, b, c) int p;", + "@property(c, b, a, b, a, c) int p;", Style); + + verifyFormat("@property(a, b, c, x, y) int p;", + "@property(c, x, b, a, y, b, a, c, y) int p;", Style); +} + +TEST_F(ObjCPropertyAttributeOrderFixerTest, HandlesAllAttributes) { + // 'class' is the only attribute that is a keyword, so make sure it works too. + FormatStyle Style = getLLVMStyle(); + + for(auto const& Attribute: getAllObjCAttributes()) { + Style.ObjCPropertyAttributeOrder = { "FIRST", Attribute, "LAST" }; + + // No change: specify all attributes in the correct order. + verifyFormat("@property(" + Attribute + ", LAST) int p;", Style); + verifyFormat("@property(FIRST, " + Attribute + ") int p;", Style); + verifyFormat("@property(FIRST, " + Attribute + ", LAST) int p;", Style); + + // Reorder: put 'FIRST' and/or 'LAST' in the wrong spot. + verifyFormat("@property(" + Attribute + ", LAST) int p;", "@property(LAST, " + Attribute + ") int p;", Style); + verifyFormat("@property(FIRST, " + Attribute + ") int p;", "@property(" + Attribute + ", FIRST) int p;", Style); + verifyFormat("@property(FIRST, " + Attribute + ", LAST) int p;", "@property(LAST, " + Attribute + ", FIRST) int p;", Style); + } +} + +TEST_F(ObjCPropertyAttributeOrderFixerTest, HandlesCommentsAroundAttributes) { + FormatStyle Style = getLLVMStyle(); + Style.ObjCPropertyAttributeOrder = {"a", "b", "c"}; + + // Handle zero attributes but comments. + verifyFormat("@property(/* 1 */) int p;", Style); + verifyFormat("@property(/* 1 */ /* 2 */) int p;", Style); + + // Handle one attribute with comments before or after. + verifyFormat("@property(/* 1 */ a) int p;", Style); + verifyFormat("@property(a /* 2 */) int p;", Style); + verifyFormat("@property(/* 1 */ a /* 2 */) int p;", Style); + + // Handle reordering with comments, before or after or both. + verifyFormat("@property(/* 1 */ a, b, x, y) int p;", + "@property(/* 1 */ x, b, a, y) int p;", Style); + + verifyFormat("@property(a, b, x, y /* 2 */) int p;", + "@property(x, b, a, y /* 2 */) int p;", Style); + + verifyFormat("@property(/* 1 */ a, b, x, y /* 2 */) int p;", + "@property(/* 1 */ x, b, a, y /* 2 */) int p;", Style); + + verifyFormat("@property(/* 1 */ /* 2 */ a, b, x, y /* 3 */ /* 4 */) int p;", + "@property(/* 1 *//* 2 */ x,b,a,y /* 3 *//* 4 */) int p;", + Style); + + // Comments between properties cause the pass to bail. + verifyFormat("@property(a, /* 1 */ b) int p;", Style); + verifyFormat("@property(b, /* 1 */ a) int p;", Style); + verifyFormat("@property(b /* 1 */, a) int p;", Style); +} + +} // namespace +} // namespace test +} // namespace format +} // namespace clang Index: llvm/utils/gn/secondary/clang/lib/Format/BUILD.gn =================================================================== --- llvm/utils/gn/secondary/clang/lib/Format/BUILD.gn +++ llvm/utils/gn/secondary/clang/lib/Format/BUILD.gn @@ -20,6 +20,7 @@ "MacroCallReconstructor.cpp", "MacroExpander.cpp", "NamespaceEndCommentsFixer.cpp", + "ObjCPropertyAttributeOrderFixer.cpp", "QualifierAlignmentFixer.cpp", "SortJavaScriptImports.cpp", "TokenAnalyzer.cpp", Index: llvm/utils/gn/secondary/clang/unittests/Format/BUILD.gn =================================================================== --- llvm/utils/gn/secondary/clang/unittests/Format/BUILD.gn +++ llvm/utils/gn/secondary/clang/unittests/Format/BUILD.gn @@ -36,6 +36,7 @@ "MacroCallReconstructorTest.cpp", "MacroExpanderTest.cpp", "NamespaceEndCommentsFixerTest.cpp", + "ObjCPropertyAttributeOrderFixerTest.cpp", "QualifierFixerTest.cpp", "SortImportsTestJS.cpp", "SortImportsTestJava.cpp", Index: utils/bazel/llvm-project-overlay/clang/BUILD.bazel =================================================================== --- utils/bazel/llvm-project-overlay/clang/BUILD.bazel +++ utils/bazel/llvm-project-overlay/clang/BUILD.bazel @@ -1301,6 +1301,7 @@ "lib/Format/FormatTokenLexer.h", "lib/Format/FormatTokenSource.h", "lib/Format/Macros.h", + "lib/Format/ObjCPropertyAttributeOrderFixer.h", "lib/Format/QualifierAlignmentFixer.h", "lib/Format/UnwrappedLineParser.h", ] + glob([