Skip to content

Conversation

marcelofabri
Copy link
Contributor

@marcelofabri marcelofabri commented Jul 29, 2017

This adds subscript to the structure reported by SourceKit.

For example, for this file:

class Foo {
    subscript(index: Int) -> Int {
        get {
            return 0
        }
        set {
            print(newValue)
        }
    }
}

We get this structure now:

{
  key.offset: 0,
  key.length: 104,
  key.diagnostic_stage: source.diagnostic.stage.swift.parse,
  key.substructure: [
    {
      key.kind: source.lang.swift.decl.class,
      key.accessibility: source.lang.swift.accessibility.internal,
      key.name: "Foo",
      key.offset: 0,
      key.length: 103,
      key.runtime_name: "_TtC8__main__3Foo",
      key.nameoffset: 6,
      key.namelength: 3,
      key.bodyoffset: 11,
      key.bodylength: 91,
      key.substructure: [
        {
          key.kind: source.lang.swift.decl.function.subscript,
          key.accessibility: source.lang.swift.accessibility.internal,
          key.setter_accessibility: source.lang.swift.accessibility.internal,
          key.name: "subscript(_:)",
          key.offset: 13,
          key.length: 88,
          key.nameoffset: 0,
          key.namelength: 0,
          key.bodyoffset: 43,
          key.bodylength: 57,
          key.substructure: [
            {
              key.kind: source.lang.swift.decl.var.parameter,
              key.name: "index",
              key.offset: 23,
              key.length: 10,
              key.typename: "Int",
              key.nameoffset: 0,
              key.namelength: 0
            }
          ]
        },
        {
          key.kind: source.lang.swift.expr.call,
          key.name: "print",
          key.offset: 79,
          key.length: 15,
          key.nameoffset: 79,
          key.namelength: 5,
          key.bodyoffset: 85,
          key.bodylength: 8
        }
      ]
    }
  ]
}

Resolves SR-5035.

This wouldn't be possible without the help from @johnfairh and @CodaFi. Thanks so much! 🙌

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'm not sure if this is the proper solution or if it's better to:

  1. Rename this function
  2. Whenever using isVariable also check if Kind == Subscript

Copy link
Contributor

Choose a reason for hiding this comment

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

what happens if we don't add this line here?

Copy link
Contributor Author

Choose a reason for hiding this comment

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

the syntax kinds of get and set inside subscripts become identifier instead of keyword

Copy link
Contributor

Choose a reason for hiding this comment

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

This accessor (isVariable()) is only being used in one place in the Syntax Model and only for things that have substructure that needs to be walked. Would it be OK to just rename it hasSubstructure() or something?

Copy link
Contributor

Choose a reason for hiding this comment

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

I agree with @CodaFi . Renaming it sounds a right approach to me.

Copy link
Contributor Author

Choose a reason for hiding this comment

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

Renamed and amended the commit.

@marcelofabri
Copy link
Contributor Author

cc @nkcsgexi

@CodaFi CodaFi requested a review from nkcsgexi July 31, 2017 21:35
@marcelofabri marcelofabri force-pushed the add-subscript-to-sourcekit-structure branch from 84d1437 to c4dad0c Compare July 31, 2017 22:00
@nkcsgexi
Copy link
Contributor

@swift-ci please smoke test and merge

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.

4 participants