jaredgrubb updated this revision to Diff 558010. jaredgrubb added a comment.
Adjusting the Style comments, since the behavior changed and were not updated to reflect that leading/trailing comments no longer have special handling. CHANGES SINCE LAST ACTION https://reviews.llvm.org/D150083/new/ https://reviews.llvm.org/D150083 Files: clang/docs/ClangFormatStyleOptions.rst clang/docs/ReleaseNotes.rst clang/include/clang/Format/Format.h clang/lib/Format/CMakeLists.txt clang/lib/Format/Format.cpp clang/lib/Format/ObjCPropertyAttributeOrderFixer.cpp clang/lib/Format/ObjCPropertyAttributeOrderFixer.h clang/unittests/Format/CMakeLists.txt clang/unittests/Format/ObjCPropertyAttributeOrderFixerTest.cpp
Index: clang/unittests/Format/ObjCPropertyAttributeOrderFixerTest.cpp =================================================================== --- /dev/null +++ clang/unittests/Format/ObjCPropertyAttributeOrderFixerTest.cpp @@ -0,0 +1,412 @@ +//===- 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); +} + +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, RemovesDuplicateAttributes) { + FormatStyle Style = getLLVMStyle(); + Style.Language = FormatStyle::LK_ObjC; + 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, 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;", + "#define MACRO @property() int p;", Style); + verifyFormat("#define MACRO @property(a) int p;", + "#define MACRO @property(a, 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 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/lib/Format/ObjCPropertyAttributeOrderFixer.h =================================================================== --- /dev/null +++ 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; + + 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 *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 Index: clang/lib/Format/ObjCPropertyAttributeOrderFixer.cpp =================================================================== --- /dev/null +++ clang/lib/Format/ObjCPropertyAttributeOrderFixer.cpp @@ -0,0 +1,197 @@ +//===--- 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 { + // 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) { + 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) + if (Tok->Next->is(tok::equal)) { + Tok = Tok->Next; + if (!Tok->Next->is(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 no need to sort unless there's more than one thing. + 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(); + const 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; + 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"; + } +} + +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->isNot(Keywords.kw_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->Next, RParenTok); + + // Return the final token since we can skip past everything in between. + return 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) { + if (!Line->Affected) + 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/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" @@ -1037,6 +1038,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); @@ -3708,6 +3711,13 @@ }); } + 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) { 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/include/clang/Format/Format.h =================================================================== --- clang/include/clang/Format/Format.h +++ clang/include/clang/Format/Format.h @@ -3192,6 +3192,30 @@ /// \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. 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 @@ -4748,6 +4772,7 @@ ObjCBlockIndentWidth == R.ObjCBlockIndentWidth && ObjCBreakBeforeNestedBlockParam == R.ObjCBreakBeforeNestedBlockParam && + ObjCPropertyAttributeOrder == R.ObjCPropertyAttributeOrder && ObjCSpaceAfterProperty == R.ObjCSpaceAfterProperty && ObjCSpaceBeforeProtocolList == R.ObjCSpaceBeforeProtocolList && PackConstructorInitializers == R.PackConstructorInitializers && Index: clang/docs/ReleaseNotes.rst =================================================================== --- clang/docs/ReleaseNotes.rst +++ clang/docs/ReleaseNotes.rst @@ -840,6 +840,8 @@ - Add ``AllowBreakBeforeNoexceptSpecifier`` option. - Add ``AllowShortCompoundRequirementOnASingleLine`` option. - Change ``BreakAfterAttributes`` from ``Never`` to ``Leave`` in LLVM style. +- Add ``ObjCPropertyAttributeOrder`` which can be used to sort ObjC property + attributes (like ``nonatomic, strong, nullable``). libclang -------- Index: clang/docs/ClangFormatStyleOptions.rst =================================================================== --- clang/docs/ClangFormatStyleOptions.rst +++ clang/docs/ClangFormatStyleOptions.rst @@ -4139,6 +4139,33 @@ }] } +.. _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. Duplicate attributes will be removed, but no other conflict + checking is performed. 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>`
_______________________________________________ cfe-commits mailing list cfe-commits@lists.llvm.org https://lists.llvm.org/cgi-bin/mailman/listinfo/cfe-commits