Skip to content

[SourceKit] Add subscript to doc structure (SR-5035) #11258

New issue

Have a question about this project? Sign up for a free GitHub account to open an issue and contact its maintainers and the community.

By clicking “Sign up for GitHub”, you agree to our terms of service and privacy statement. We’ll occasionally send you account related emails.

Already on GitHub? Sign in to your account

Merged

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! 🙌

@@ -146,6 +147,7 @@ struct SyntaxStructureNode {
case SyntaxStructureKind::StaticVariable:
case SyntaxStructureKind::ClassVariable:
case SyntaxStructureKind::Parameter:
case SyntaxStructureKind::Subscript:
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