[clang-format] ObjCPropertyAttributeOrder to sort ObjC property attributes

Add a style option to specify the order that property attributes should
appear in ObjC property declarations (property attributes are things like
`nonatomic, strong, nullable`).

Closes #71323.

Differential Revision: https://reviews.llvm.org/D150083

clang/docs/ClangFormatStyleOptions.rst

@@ -4211,6 +4211,32 @@ the configuration (without a prefix: ``Auto``).
              }]
      }
 
+.. _ObjCPropertyAttributeOrder:
+
+**ObjCPropertyAttributeOrder** (``List of Strings``) :versionbadge:`clang-format 18` :ref:`¶ <ObjCPropertyAttributeOrder>`
+  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. Comments between attributes will leave the attributes
+  untouched.
+
+  .. warning::
+
+   Using this option could lead to incorrect code formatting due to
+   clang-format's lack of complete semantic information. As such, extra
+   care should be taken to review code changes made by this option.
+
+  .. 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:`¶ <ObjCSpaceAfterProperty>`

clang/docs/ReleaseNotes.rst

@@ -984,6 +984,8 @@ clang-format
 - Add ``AllowShortCompoundRequirementOnASingleLine`` option.
 - Change ``BreakAfterAttributes`` from ``Never`` to ``Leave`` in LLVM style.
 - Add ``BreakAdjacentStringLiterals`` option.
+- Add ``ObjCPropertyAttributeOrder`` which can be used to sort ObjC property
+  attributes (like ``nonatomic, strong, nullable``).
 
 libclang
 --------

clang/include/clang/Format/Format.h

@@ -3264,6 +3264,29 @@ struct FormatStyle {
   /// \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. Comments between attributes will leave the attributes
+  /// untouched.
+  /// \warning
+  ///  Using this option could lead to incorrect code formatting due to
+  ///  clang-format's lack of complete semantic information. As such, extra
+  ///  care should be taken to review code changes made by this option.
+  /// \endwarning
+  /// \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 18
+  std::vector<std::string> ObjCPropertyAttributeOrder;
+
   /// Add a space after ``@property`` in Objective-C, i.e. use
   /// ``@property (readonly)`` instead of ``@property(readonly)``.
   /// \version 3.7
@@ -4821,6 +4844,7 @@ struct FormatStyle {
            ObjCBlockIndentWidth == R.ObjCBlockIndentWidth &&
            ObjCBreakBeforeNestedBlockParam ==
                R.ObjCBreakBeforeNestedBlockParam &&
+           ObjCPropertyAttributeOrder == R.ObjCPropertyAttributeOrder &&
            ObjCSpaceAfterProperty == R.ObjCSpaceAfterProperty &&
            ObjCSpaceBeforeProtocolList == R.ObjCSpaceBeforeProtocolList &&
            PackConstructorInitializers == R.PackConstructorInitializers &&

clang/lib/Format/CMakeLists.txt

@@ -12,6 +12,7 @@ add_clang_library(clangFormat
   MacroCallReconstructor.cpp
   MacroExpander.cpp
   NamespaceEndCommentsFixer.cpp
+  ObjCPropertyAttributeOrderFixer.cpp
   QualifierAlignmentFixer.cpp
   SortJavaScriptImports.cpp
   TokenAnalyzer.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"
@@ -1039,6 +1040,8 @@ template <> struct MappingTraits<FormatStyle> {
     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);
@@ -3711,6 +3714,13 @@ reformat(const FormatStyle &Style, StringRef Code,
     });
   }
 
+  if (Style.Language == FormatStyle::LK_ObjC &&
+      !Style.ObjCPropertyAttributeOrder.empty()) {
+    Passes.emplace_back([&](const Environment &Env) {
+      return ObjCPropertyAttributeOrderFixer(Env, Expanded).process();
+    });
+  }
+
   if (Style.isJavaScript() &&
       Style.JavaScriptQuotes != FormatStyle::JSQS_Leave) {
     Passes.emplace_back([&](const Environment &Env) {

clang/lib/Format/ObjCPropertyAttributeOrderFixer.cpp

@@ -0,0 +1,202 @@
+//===--- 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 "llvm/ADT/Sequence.h"
+
+#include <algorithm>
+
+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 (const auto &Property : Style.ObjCPropertyAttributeOrder)
+    SortOrderMap[Property] = 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 *BeginTok, const FormatToken *EndTok) const {
+  assert(BeginTok);
+  assert(EndTok);
+  assert(EndTok->Previous);
+
+  // If there are zero or one tokens, nothing to do.
+  if (BeginTok == EndTok || BeginTok->Next == EndTok)
+    return;
+
+  // Collect the attributes.
+  SmallVector<ObjCPropertyEntry, 8> PropertyAttributes;
+  for (auto Tok = BeginTok; Tok != EndTok; Tok = Tok->Next) {
+    assert(Tok);
+    if (Tok->is(tok::comma)) {
+      // Ignore the comma separators.
+      continue;
+    }
+
+    if (!isObjCPropertyAttribute(Tok)) {
+      // If we hit any other kind of token, just bail.
+      return;
+    }
+
+    // Memoize the attribute. (Note that 'class' is a legal attribute!)
+    PropertyAttributes.push_back({Tok->TokenText, StringRef{}});
+
+    // Also handle `getter=getFoo` attributes.
+    // (Note: no check needed against `EndTok`, since its type is not
+    // BinaryOperator or Identifier)
+    assert(Tok->Next);
+    if (Tok->Next->is(tok::equal)) {
+      Tok = Tok->Next;
+      assert(Tok->Next);
+      if (Tok->Next->isNot(tok::identifier)) {
+        // If we hit any other kind of token, just bail. It's unusual/illegal.
+        return;
+      }
+      Tok = Tok->Next;
+      PropertyAttributes.back().Value = Tok->TokenText;
+    }
+  }
+
+  // There's nothing to do unless there's more than one attribute.
+  if (PropertyAttributes.size() < 2)
+    return;
+
+  // Create a "remapping index" on how to reorder the attributes.
+  SmallVector<unsigned, 8> Indices =
+      llvm::to_vector<8>(llvm::seq<unsigned>(0, PropertyAttributes.size()));
+
+  // Sort the indices based on the priority stored in 'SortOrderMap'; use Max
+  // for missing values.
+  const auto SortOrderMax = Style.ObjCPropertyAttributeOrder.size();
+  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);
+  });
+
+  // If the property order is already correct, then no fix-up is needed.
+  if (llvm::is_sorted(Indices))
+    return;
+
+  // Generate the replacement text.
+  std::string NewText;
+  const auto AppendAttribute = [&](const ObjCPropertyEntry &PropertyEntry) {
+    NewText += PropertyEntry.Attribute;
+
+    if (!PropertyEntry.Value.empty()) {
+      NewText += "=";
+      NewText += PropertyEntry.Value;
+    }
+  };
+
+  AppendAttribute(PropertyAttributes[Indices[0]]);
+  for (unsigned Index : llvm::drop_begin(Indices)) {
+    NewText += ", ";
+    AppendAttribute(PropertyAttributes[Index]);
+  }
+
+  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";
+  }
+}
+
+void ObjCPropertyAttributeOrderFixer::analyzeObjCPropertyDecl(
+    const SourceManager &SourceMgr, const AdditionalKeywords &Keywords,
+    tooling::Replacements &Fixes, const FormatToken *Tok) const {
+  assert(Tok);
+
+  // Expect `property` to be the very next token or else just bail early.
+  const FormatToken *const PropertyTok = Tok->Next;
+  if (!PropertyTok || PropertyTok->isNot(Keywords.kw_property))
+    return;
+
+  // 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;
+
+  // Get the matching right-paren, the bounds for property attributes.
+  const FormatToken *const RParenTok = LParenTok->MatchingParen;
+  if (!RParenTok)
+    return;
+
+  sortPropertyAttributes(SourceMgr, Fixes, LParenTok->Next, RParenTok);
+}
+
+std::pair<tooling::Replacements, unsigned>
+ObjCPropertyAttributeOrderFixer::analyze(
+    TokenAnnotator & /*Annotator*/,
+    SmallVectorImpl<AnnotatedLine *> &AnnotatedLines,
+    FormatTokenLexer &Tokens) {
+  tooling::Replacements Fixes;
+  const AdditionalKeywords &Keywords = Tokens.getKeywords();
+  const SourceManager &SourceMgr = Env.getSourceManager();
+  AffectedRangeMgr.computeAffectedLines(AnnotatedLines);
+
+  for (AnnotatedLine *Line : AnnotatedLines) {
+    assert(Line);
+    if (!Line->Affected || Line->Type != LT_ObjCProperty)
+      continue;
+    FormatToken *First = Line->First;
+    assert(First);
+    if (First->Finalized)
+      continue;
+
+    const auto *Last = Line->Last;
+
+    for (const auto *Tok = First; Tok != Last; Tok = Tok->Next) {
+      assert(Tok);
+
+      // Skip until the `@` of a `@property` declaration.
+      if (Tok->isNot(TT_ObjCProperty))
+        continue;
+
+      analyzeObjCPropertyDecl(SourceMgr, Keywords, Fixes, Tok);
+
+      // There are never two `@property` in a line (they are split
+      // by other passes), so this pass can break after just one.
+      break;
+    }
+  }
+  return {Fixes, 0};
+}
+
+} // namespace format
+} // namespace clang

clang/lib/Format/ObjCPropertyAttributeOrderFixer.h

@@ -0,0 +1,51 @@
+//===--- 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"
+
+namespace clang {
+namespace format {
+
+class ObjCPropertyAttributeOrderFixer : public TokenAnalyzer {
+  llvm::StringMap<unsigned> SortOrderMap;
+
+  void analyzeObjCPropertyDecl(const SourceManager &SourceMgr,
+                               const AdditionalKeywords &Keywords,
+                               tooling::Replacements &Fixes,
+                               const FormatToken *Tok) const;
+
+  void sortPropertyAttributes(const SourceManager &SourceMgr,
+                              tooling::Replacements &Fixes,
+                              const FormatToken *BeginTok,
+                              const FormatToken *EndTok) const;
+
+  std::pair<tooling::Replacements, unsigned>
+  analyze(TokenAnnotator &Annotator,
+          SmallVectorImpl<AnnotatedLine *> &AnnotatedLines,
+          FormatTokenLexer &Tokens) override;
+
+public:
+  ObjCPropertyAttributeOrderFixer(const Environment &Env,
+                                  const FormatStyle &Style);
+};
+
+} // end namespace format
+} // end namespace clang
+
+#endif

clang/unittests/Format/CMakeLists.txt

@@ -28,6 +28,7 @@ add_clang_unittest(FormatTests
   MacroCallReconstructorTest.cpp
   MacroExpanderTest.cpp
   NamespaceEndCommentsFixerTest.cpp
+  ObjCPropertyAttributeOrderFixerTest.cpp
   QualifierFixerTest.cpp
   SortImportsTestJS.cpp
   SortImportsTestJava.cpp

clang/unittests/Format/ObjCPropertyAttributeOrderFixerTest.cpp

@@ -0,0 +1,423 @@
+//===- 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);
+  }
+
+  llvm::SpecificBumpPtrAllocator<FormatToken> Allocator;
+  std::vector<std::unique_ptr<llvm::MemoryBuffer>> Buffers;
+};
+
+TEST_F(ObjCPropertyAttributeOrderFixerTest, ParsesStyleOption) {
+  FormatStyle Style = {};
+  Style.Language = FormatStyle::LK_ObjC;
+
+  CHECK_PARSE("ObjCPropertyAttributeOrder: [class]", ObjCPropertyAttributeOrder,
+              std::vector<std::string>({"class"}));
+
+  CHECK_PARSE("ObjCPropertyAttributeOrder: ["
+              "class, direct, atomic, nonatomic, "
+              "assign, retain, strong, copy, weak, unsafe_unretained, "
+              "readonly, readwrite, getter, setter, "
+              "nullable, nonnull, null_resettable, null_unspecified"
+              "]",
+              ObjCPropertyAttributeOrder,
+              std::vector<std::string>({
+                  "class",
+                  "direct",
+                  "atomic",
+                  "nonatomic",
+                  "assign",
+                  "retain",
+                  "strong",
+                  "copy",
+                  "weak",
+                  "unsafe_unretained",
+                  "readonly",
+                  "readwrite",
+                  "getter",
+                  "setter",
+                  "nullable",
+                  "nonnull",
+                  "null_resettable",
+                  "null_unspecified",
+              }));
+}
+
+TEST_F(ObjCPropertyAttributeOrderFixerTest, SortsSpecifiedAttributes) {
+  FormatStyle Style = getLLVMStyle();
+  Style.Language = FormatStyle::LK_ObjC;
+  Style.ObjCPropertyAttributeOrder = {"a", "b", "c"};
+
+  // Zero: nothing to do, but is legal.
+  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);
+
+  // Check that properties preceded by @optional/@required work.
+  verifyFormat("@optional\n"
+               "@property(a, b) int p;",
+               "@optional @property(b, a) int p;", Style);
+  verifyFormat("@required\n"
+               "@property(a, b) int p;",
+               "@required @property(b, a) int p;", Style);
+
+  // Check two `@property`s on one-line are reflowed (by other passes)
+  // and both have their attributes reordered.
+  verifyFormat("@property(a, b) int p;\n"
+               "@property(a, b) int q;",
+               "@property(b, a) int p; @property(b, a) int q;", Style);
+}
+
+TEST_F(ObjCPropertyAttributeOrderFixerTest, SortsAttributesWithValues) {
+  FormatStyle Style = getLLVMStyle();
+  Style.Language = FormatStyle::LK_ObjC;
+  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.Language = FormatStyle::LK_ObjC;
+  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, HandlesDuplicatedAttributes) {
+  // Duplicated attributes aren't rejected by the compiler even if it's silly
+  // to do so. Preserve them and sort them best-effort.
+  FormatStyle Style = getLLVMStyle();
+  Style.Language = FormatStyle::LK_ObjC;
+  Style.ObjCPropertyAttributeOrder = {"a", "b", "c"};
+
+  // Just a dup and nothing else.
+  verifyFormat("@property(a, a) int p;", Style);
+
+  // A dup and something else.
+  verifyFormat("@property(a, a, b) int p;", "@property(a, b, a) int p;", Style);
+
+  // Duplicates using `=`: stable-sort irrespective of their value.
+  verifyFormat("@property(a=A, a=A, b=X, b=Y) int p;",
+               "@property(a=A, b=X, a=A, b=Y) int p;", Style);
+  verifyFormat("@property(a=A, a=A, b=Y, b=X) int p;",
+               "@property(a=A, b=Y, a=A, b=X) int p;", Style);
+  verifyFormat("@property(a, a=A, b=B, b) int p;",
+               "@property(a, b=B, a=A, b) int p;", Style);
+}
+
+TEST_F(ObjCPropertyAttributeOrderFixerTest, SortsInPPDirective) {
+  FormatStyle Style = getLLVMStyle();
+  Style.Language = FormatStyle::LK_ObjC;
+  Style.ObjCPropertyAttributeOrder = {"a", "b", "c"};
+
+  // Spot-check a few simple cases that require sorting in a macro definition.
+  verifyFormat("#define MACRO @property() int p;", Style);
+  verifyFormat("#define MACRO @property(a) int p;", Style);
+  verifyFormat("#define MACRO @property(a, b) int p;",
+               "#define MACRO @property(b, a) int p;", Style);
+  verifyFormat("#define MACRO @property(a, b, c) int p;",
+               "#define MACRO @property(c, b, a) 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();
+  Style.Language = FormatStyle::LK_ObjC;
+  Style.ObjCPropertyAttributeOrder = {"FIRST",
+                                      "class",
+                                      "direct",
+                                      "atomic",
+                                      "nonatomic",
+                                      "assign",
+                                      "retain",
+                                      "strong",
+                                      "copy",
+                                      "weak",
+                                      "unsafe_unretained",
+                                      "readonly",
+                                      "readwrite",
+                                      "getter",
+                                      "setter",
+                                      "nullable",
+                                      "nonnull",
+                                      "null_resettable",
+                                      "null_unspecified",
+                                      "LAST"};
+
+  // No change: specify all attributes in the correct order.
+  verifyFormat("@property(class, LAST) int p;", Style);
+  verifyFormat("@property(direct, LAST) int p;", Style);
+  verifyFormat("@property(atomic, LAST) int p;", Style);
+  verifyFormat("@property(nonatomic, LAST) int p;", Style);
+  verifyFormat("@property(assign, LAST) int p;", Style);
+  verifyFormat("@property(retain, LAST) int p;", Style);
+  verifyFormat("@property(strong, LAST) int p;", Style);
+  verifyFormat("@property(copy, LAST) int p;", Style);
+  verifyFormat("@property(weak, LAST) int p;", Style);
+  verifyFormat("@property(unsafe_unretained, LAST) int p;", Style);
+  verifyFormat("@property(readonly, LAST) int p;", Style);
+  verifyFormat("@property(readwrite, LAST) int p;", Style);
+  verifyFormat("@property(getter, LAST) int p;", Style);
+  verifyFormat("@property(setter, LAST) int p;", Style);
+  verifyFormat("@property(nullable, LAST) int p;", Style);
+  verifyFormat("@property(nonnull, LAST) int p;", Style);
+  verifyFormat("@property(null_resettable, LAST) int p;", Style);
+  verifyFormat("@property(null_unspecified, LAST) int p;", Style);
+
+  verifyFormat("@property(FIRST, class) int p;", Style);
+  verifyFormat("@property(FIRST, direct) int p;", Style);
+  verifyFormat("@property(FIRST, atomic) int p;", Style);
+  verifyFormat("@property(FIRST, nonatomic) int p;", Style);
+  verifyFormat("@property(FIRST, assign) int p;", Style);
+  verifyFormat("@property(FIRST, retain) int p;", Style);
+  verifyFormat("@property(FIRST, strong) int p;", Style);
+  verifyFormat("@property(FIRST, copy) int p;", Style);
+  verifyFormat("@property(FIRST, weak) int p;", Style);
+  verifyFormat("@property(FIRST, unsafe_unretained) int p;", Style);
+  verifyFormat("@property(FIRST, readonly) int p;", Style);
+  verifyFormat("@property(FIRST, readwrite) int p;", Style);
+  verifyFormat("@property(FIRST, getter) int p;", Style);
+  verifyFormat("@property(FIRST, setter) int p;", Style);
+  verifyFormat("@property(FIRST, nullable) int p;", Style);
+  verifyFormat("@property(FIRST, nonnull) int p;", Style);
+  verifyFormat("@property(FIRST, null_resettable) int p;", Style);
+  verifyFormat("@property(FIRST, null_unspecified) int p;", Style);
+
+  verifyFormat("@property(FIRST, class, LAST) int p;", Style);
+  verifyFormat("@property(FIRST, direct, LAST) int p;", Style);
+  verifyFormat("@property(FIRST, atomic, LAST) int p;", Style);
+  verifyFormat("@property(FIRST, nonatomic, LAST) int p;", Style);
+  verifyFormat("@property(FIRST, assign, LAST) int p;", Style);
+  verifyFormat("@property(FIRST, retain, LAST) int p;", Style);
+  verifyFormat("@property(FIRST, strong, LAST) int p;", Style);
+  verifyFormat("@property(FIRST, copy, LAST) int p;", Style);
+  verifyFormat("@property(FIRST, weak, LAST) int p;", Style);
+  verifyFormat("@property(FIRST, unsafe_unretained, LAST) int p;", Style);
+  verifyFormat("@property(FIRST, readonly, LAST) int p;", Style);
+  verifyFormat("@property(FIRST, readwrite, LAST) int p;", Style);
+  verifyFormat("@property(FIRST, getter, LAST) int p;", Style);
+  verifyFormat("@property(FIRST, setter, LAST) int p;", Style);
+  verifyFormat("@property(FIRST, nullable, LAST) int p;", Style);
+  verifyFormat("@property(FIRST, nonnull, LAST) int p;", Style);
+  verifyFormat("@property(FIRST, null_resettable, LAST) int p;", Style);
+  verifyFormat("@property(FIRST, null_unspecified, LAST) int p;", Style);
+
+  // Reorder: put 'FIRST' and/or 'LAST' in the wrong spot.
+  verifyFormat("@property(class, LAST) int p;", "@property(LAST, class) int p;",
+               Style);
+  verifyFormat("@property(direct, LAST) int p;",
+               "@property(LAST, direct) int p;", Style);
+  verifyFormat("@property(atomic, LAST) int p;",
+               "@property(LAST, atomic) int p;", Style);
+  verifyFormat("@property(nonatomic, LAST) int p;",
+               "@property(LAST, nonatomic) int p;", Style);
+  verifyFormat("@property(assign, LAST) int p;",
+               "@property(LAST, assign) int p;", Style);
+  verifyFormat("@property(retain, LAST) int p;",
+               "@property(LAST, retain) int p;", Style);
+  verifyFormat("@property(strong, LAST) int p;",
+               "@property(LAST, strong) int p;", Style);
+  verifyFormat("@property(copy, LAST) int p;", "@property(LAST, copy) int p;",
+               Style);
+  verifyFormat("@property(weak, LAST) int p;", "@property(LAST, weak) int p;",
+               Style);
+  verifyFormat("@property(unsafe_unretained, LAST) int p;",
+               "@property(LAST, unsafe_unretained) int p;", Style);
+  verifyFormat("@property(readonly, LAST) int p;",
+               "@property(LAST, readonly) int p;", Style);
+  verifyFormat("@property(readwrite, LAST) int p;",
+               "@property(LAST, readwrite) int p;", Style);
+  verifyFormat("@property(getter, LAST) int p;",
+               "@property(LAST, getter) int p;", Style);
+  verifyFormat("@property(setter, LAST) int p;",
+               "@property(LAST, setter) int p;", Style);
+  verifyFormat("@property(nullable, LAST) int p;",
+               "@property(LAST, nullable) int p;", Style);
+  verifyFormat("@property(nonnull, LAST) int p;",
+               "@property(LAST, nonnull) int p;", Style);
+  verifyFormat("@property(null_resettable, LAST) int p;",
+               "@property(LAST, null_resettable) int p;", Style);
+  verifyFormat("@property(null_unspecified, LAST) int p;",
+               "@property(LAST, null_unspecified) int p;", Style);
+
+  verifyFormat("@property(FIRST, class) int p;",
+               "@property(class, FIRST) int p;", Style);
+  verifyFormat("@property(FIRST, direct) int p;",
+               "@property(direct, FIRST) int p;", Style);
+  verifyFormat("@property(FIRST, atomic) int p;",
+               "@property(atomic, FIRST) int p;", Style);
+  verifyFormat("@property(FIRST, nonatomic) int p;",
+               "@property(nonatomic, FIRST) int p;", Style);
+  verifyFormat("@property(FIRST, assign) int p;",
+               "@property(assign, FIRST) int p;", Style);
+  verifyFormat("@property(FIRST, retain) int p;",
+               "@property(retain, FIRST) int p;", Style);
+  verifyFormat("@property(FIRST, strong) int p;",
+               "@property(strong, FIRST) int p;", Style);
+  verifyFormat("@property(FIRST, copy) int p;", "@property(copy, FIRST) int p;",
+               Style);
+  verifyFormat("@property(FIRST, weak) int p;", "@property(weak, FIRST) int p;",
+               Style);
+  verifyFormat("@property(FIRST, unsafe_unretained) int p;",
+               "@property(unsafe_unretained, FIRST) int p;", Style);
+  verifyFormat("@property(FIRST, readonly) int p;",
+               "@property(readonly, FIRST) int p;", Style);
+  verifyFormat("@property(FIRST, readwrite) int p;",
+               "@property(readwrite, FIRST) int p;", Style);
+  verifyFormat("@property(FIRST, getter) int p;",
+               "@property(getter, FIRST) int p;", Style);
+  verifyFormat("@property(FIRST, setter) int p;",
+               "@property(setter, FIRST) int p;", Style);
+  verifyFormat("@property(FIRST, nullable) int p;",
+               "@property(nullable, FIRST) int p;", Style);
+  verifyFormat("@property(FIRST, nonnull) int p;",
+               "@property(nonnull, FIRST) int p;", Style);
+  verifyFormat("@property(FIRST, null_resettable) int p;",
+               "@property(null_resettable, FIRST) int p;", Style);
+  verifyFormat("@property(FIRST, null_unspecified) int p;",
+               "@property(null_unspecified, FIRST) int p;", Style);
+
+  verifyFormat("@property(FIRST, class, LAST) int p;",
+               "@property(LAST, class, FIRST) int p;", Style);
+  verifyFormat("@property(FIRST, direct, LAST) int p;",
+               "@property(LAST, direct, FIRST) int p;", Style);
+  verifyFormat("@property(FIRST, atomic, LAST) int p;",
+               "@property(LAST, atomic, FIRST) int p;", Style);
+  verifyFormat("@property(FIRST, nonatomic, LAST) int p;",
+               "@property(LAST, nonatomic, FIRST) int p;", Style);
+  verifyFormat("@property(FIRST, assign, LAST) int p;",
+               "@property(LAST, assign, FIRST) int p;", Style);
+  verifyFormat("@property(FIRST, retain, LAST) int p;",
+               "@property(LAST, retain, FIRST) int p;", Style);
+  verifyFormat("@property(FIRST, strong, LAST) int p;",
+               "@property(LAST, strong, FIRST) int p;", Style);
+  verifyFormat("@property(FIRST, copy, LAST) int p;",
+               "@property(LAST, copy, FIRST) int p;", Style);
+  verifyFormat("@property(FIRST, weak, LAST) int p;",
+               "@property(LAST, weak, FIRST) int p;", Style);
+  verifyFormat("@property(FIRST, unsafe_unretained, LAST) int p;",
+               "@property(LAST, unsafe_unretained, FIRST) int p;", Style);
+  verifyFormat("@property(FIRST, readonly, LAST) int p;",
+               "@property(LAST, readonly, FIRST) int p;", Style);
+  verifyFormat("@property(FIRST, readwrite, LAST) int p;",
+               "@property(LAST, readwrite, FIRST) int p;", Style);
+  verifyFormat("@property(FIRST, getter, LAST) int p;",
+               "@property(LAST, getter, FIRST) int p;", Style);
+  verifyFormat("@property(FIRST, setter, LAST) int p;",
+               "@property(LAST, setter, FIRST) int p;", Style);
+  verifyFormat("@property(FIRST, nullable, LAST) int p;",
+               "@property(LAST, nullable, FIRST) int p;", Style);
+  verifyFormat("@property(FIRST, nonnull, LAST) int p;",
+               "@property(LAST, nonnull, FIRST) int p;", Style);
+  verifyFormat("@property(FIRST, null_resettable, LAST) int p;",
+               "@property(LAST, null_resettable, FIRST) int p;", Style);
+  verifyFormat("@property(FIRST, null_unspecified, LAST) int p;",
+               "@property(LAST, null_unspecified, FIRST) int p;", Style);
+}
+
+TEST_F(ObjCPropertyAttributeOrderFixerTest, HandlesCommentsAroundAttributes) {
+  FormatStyle Style = getLLVMStyle();
+  Style.Language = FormatStyle::LK_ObjC;
+  Style.ObjCPropertyAttributeOrder = {"a", "b"};
+
+  // Zero attributes but comments.
+  verifyFormat("@property(/* 1 */) int p;", Style);
+  verifyFormat("@property(/* 1 */ /* 2 */) int p;", Style);
+
+  // 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);
+
+  // No reordering if comments are encountered anywhere.
+  // (Each case represents a reordering that would have happened
+  // without the comment.)
+  verifyFormat("@property(/* before */ b, a) int p;", Style);
+  verifyFormat("@property(b, /* between */ a) int p;", Style);
+  verifyFormat("@property(b, a /* after */) int p;", Style);
+}
+
+} // namespace
+} // namespace test
+} // namespace format
+} // namespace clang