Skip to content

Conversation

marcelofabri
Copy link
Contributor

This PR includes AccessLevel attributes inside key.attributes in structures.

This would make possible for tools like SwiftLint to get the ranges of these attributes, which can be useful for several rules:

  • Enforcing that every top level declaration has an explicit AccessLevel
  • Enforcing that all declarations have explicit AccessLevels
  • Enforcing that an AccessLevel is omitted when it can be inferred
  • Validating the order of "modifiers" (private, mutating, final, etc) of declarations
  • Enforcing that the setter AccessLevel is not the same as the general one (e.g. private(set) private foo: Int)

This PR also adds attributes for setter AccessLevel in 472fd78c43203a6c746db720373c79352a066a29. I'm not sure if this is the best way to do it, creating attributes like source.decl.attribute.setter_access.private, so feedbacks are welcome!

Resolves SR-5978.

// cc @nkcsgexi

static UIdent Attr_FilePrivate("source.decl.attribute.fileprivate");
static UIdent Attr_Internal("source.decl.attribute.internal");
static UIdent Attr_Public("source.decl.attribute.public");
static UIdent Attr_Open("source.decl.attribute.open");
Copy link
Contributor

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Any reason we are not adding these to ProtocolUIDs.def?

Copy link
Contributor Author

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Attributes are defined in Attr.def. The way that AccessLevel attributes work is via DECL_ATTR_ALIAS, which forces us to explicitly check the AccessLevel and pick the right UID.

Copy link
Contributor

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

ah, OK. Thanks for the explanation!

static UIdent Attr_FilePrivate("source.decl.attribute.setter_access.fileprivate");
static UIdent Attr_Internal("source.decl.attribute.setter_access.internal");
static UIdent Attr_Public("source.decl.attribute.setter_access.public");
static UIdent Attr_Open("source.decl.attribute.setter_access.open");
Copy link
Contributor

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Same for these new keys.

static UIdent Attr_Public("source.decl.attribute.setter_access.public");
static UIdent Attr_Open("source.decl.attribute.setter_access.open");

switch (cast<AbstractAccessControlAttr>(Attr)->getAccess()) {
Copy link
Contributor

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Is there a way we can share the two switch statements? It seems to me a logic duplication.

Copy link
Contributor Author

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

I agree that there's some logic duplication, but I can't think of a way of sharing those and being easier to read at the same time.

@nkcsgexi
Copy link
Contributor

@swift-ci please smoke test

@marcelofabri marcelofabri force-pushed the accesscontrol-attributes branch from 472fd78 to c8a677f Compare September 25, 2017 23:41
@nkcsgexi
Copy link
Contributor

@swift-ci please smoke test

@marcelofabri
Copy link
Contributor Author

@nkcsgexi can you please trigger @swift-ci again?

@nkcsgexi
Copy link
Contributor

nkcsgexi commented Oct 6, 2017

@swift-ci please smoke test

@marcelofabri
Copy link
Contributor Author

ping @nkcsgexi

Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
None yet
Projects
None yet
Development

Successfully merging this pull request may close these issues.

2 participants