Skip to content
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

Jump to bottom

[SR-11099] SourceKit's editor open request in Swift 5.1 returns invalid name in some cases #53494

Closed
jpsim opened this issue Jul 10, 2019 · 12 comments
Closed

[SR-11099] SourceKit's editor open request in Swift 5.1 returns invalid name in some cases #53494

jpsim opened this issue Jul 10, 2019 · 12 comments
Assignees
@nathawes
Labels
bug A deviation from expected or documented behavior. Also: expected but undesirable behavior. regression swift 5.1

Comments

@jpsim
Copy link
Contributor

jpsim commented Jul 10, 2019

Previous ID SR-11099
Radar rdar://problem/55183943
Original Reporter @jpsim
Type Bug
Status Resolved
Resolution Done
Environment

Xcode 11.0 Build version 11M362v

Additional Detail from JIRA
Votes 1
Component/s Source Tooling
Labels Bug, 5.1Regression
Assignee @nathawes
Priority Medium

md5: e2a8d425e923105798883af0c7d26afe

Issue Description:

I have a case where SourceKit's editor open request will return an invalid name with a length of 2^32 − 1. This is a regression compared to Swift 5.0.

$ cat file.swift
_ = #"\#(0)"#
$ cat request.yml
key.request: source.request.editor.open
key.name: "/Users/jsimard/Downloads/file.swift"
key.sourcefile: "/Users/jsimard/Downloads/file.swift"
$ DEVELOPER_DIR=/Applications/Xcode-11.app/Contents/Developer xcodebuild -version
Xcode 11.0
Build version 11M362v
$ DEVELOPER_DIR=/Applications/Xcode-11.app/Contents/Developer sourcekitten request --yaml request.yml | jq '."key.substructure"'
[
  {
    "key.bodylength": 0,
    "key.bodyoffset": 13,
    "key.kind": "source.lang.swift.stmt.brace",
    "key.length": 9,
    "key.namelength": 0,
    "key.nameoffset": 0,
    "key.offset": 4,
    "key.substructure": [
      {
        "key.bodylength": 1,
        "key.bodyoffset": 9,
        "key.kind": "source.lang.swift.expr.call",
        "key.length": 3,
        "key.name": "(0)\"#\n",
        "key.namelength": 4294967295,
        "key.nameoffset": 8,
        "key.offset": 8
      }
    ]
  }
]
$ DEVELOPER_DIR=/Applications/Xcode-10.2.1.app/Contents/Developer xcodebuild -version
Xcode 10.2.1
Build version 10E1001
$ DEVELOPER_DIR=/Applications/Xcode-10.2.1.app/Contents/Developer sourcekitten request --yaml request.yml | jq '."key.substructure"'
[
  {
    "key.bodylength": 0,
    "key.bodyoffset": 13,
    "key.kind": "source.lang.swift.stmt.brace",
    "key.length": 9,
    "key.namelength": 0,
    "key.nameoffset": 0,
    "key.offset": 4,
    "key.substructure": [
      {
        "key.bodylength": 1,
        "key.bodyoffset": 9,
        "key.kind": "source.lang.swift.expr.call",
        "key.length": 3,
        "key.namelength": 0,
        "key.nameoffset": 0,
        "key.offset": 8
      }
    ]
  }
]
@jpsim
Copy link
Contributor Author

jpsim commented Sep 8, 2019

I forgot to follow up on this earlier in the 5.1 development cycle, but better late than never.

Any chance this can get fixed in a following release? cc @benlangmuir @akyrtzi

@benlangmuir
Copy link
Member

@nathawes do you recognize this?

@benlangmuir
Copy link
Member

@swift-ci create

@nathawes
Copy link
Collaborator

nathawes commented Sep 9, 2019

Yes! This is due to the new handling of interpolated string literals and should be fixed by #26354 which I forgot to actually land... I need to follow up on a few review comments, but will try to do that later today.

@jpsim
Copy link
Contributor Author

jpsim commented Sep 11, 2019

Thanks for the fix @nathawes! It looks like CI passed on your PR. Is there any value in adding a regression test for the minimal case I shared in this ticket?

I'd also love to know if it would be possible to consider including this fix in the next 5.1.x patch release.

@nathawes
Copy link
Collaborator

No worries @jpsim! The test in the PR covers a pretty similar case (a regular interpolated string instead of a raw interpolated string) so it's probably not worth it but certainly doesn't hurt to add if you'd like to. The master PR is merged now and I've pulled a minimal version of the fix into the swift-5.1-branch here: #27186.

@akyrtzi
Copy link
Member

akyrtzi commented Nov 6, 2019

@jpsim could you verify the issue is fixed with the latest 5.1 OSS toolchain?

@jpsim
Copy link
Contributor Author

jpsim commented Nov 6, 2019

It's still an issue with Swift 5.1.1 from Xcode 11.2:

$ DEVELOPER_DIR=/Applications/Xcode-11p2.app/Contents/Developer xcrun swift --version
Apple Swift version 5.1.1 (swiftlang-1100.0.274.1 clang-1100.0.33.9)
$ DEVELOPER_DIR=/Applications/Xcode-11p2.app/Contents/Developer sourcekitten request --yaml request.yml | jq '."key.substructure"' 
[
  {
    "key.bodylength": 0,
    "key.bodyoffset": 13,
    "key.kind": "source.lang.swift.stmt.brace",
    "key.length": 9,
    "key.namelength": 0,
    "key.nameoffset": 0,
    "key.offset": 4,
    "key.substructure": [
      {
        "key.bodylength": 1,
        "key.bodyoffset": 9,
        "key.kind": "source.lang.swift.expr.call",
        "key.length": 3,
        "key.name": "(0)\"#\n",
        "key.namelength": 4294967295,
        "key.nameoffset": 8,
        "key.offset": 8
      }
    ]
  }
]

Checking with the latest 5.1 OSS toolchain now...

@jpsim
Copy link
Contributor Author

jpsim commented Nov 6, 2019

Somewhat concerning, but the latest 5.1 OSS toolchain doesn't contain anything in the key.substructure member:

$ TOOLCHAIN_DIR=/Library/Developer/Toolchains/swift-5.1-DEVELOPMENT-SNAPSHOT-2019-11-05-a.xctoolchain sourcekitten request --yaml request.yml                           
{
  "key.diagnostic_stage" : "source.diagnostic.stage.swift.parse",
  "key.length" : 14,
  "key.offset" : 0,
  "key.substructure" : [

  ],
  "key.syntaxmap" : [
    {
      "key.kind" : "source.lang.swift.syntaxtype.keyword",
      "key.length" : 1,
      "key.offset" : 0
    },
    {
      "key.kind" : "source.lang.swift.syntaxtype.string",
      "key.length" : 2,
      "key.offset" : 4
    },
    {
      "key.kind" : "source.lang.swift.syntaxtype.string_interpolation_anchor",
      "key.length" : 1,
      "key.offset" : 8
    },
    {
      "key.kind" : "source.lang.swift.syntaxtype.number",
      "key.length" : 1,
      "key.offset" : 9
    },
    {
      "key.kind" : "source.lang.swift.syntaxtype.string_interpolation_anchor",
      "key.length" : 1,
      "key.offset" : 10
    },
    {
      "key.kind" : "source.lang.swift.syntaxtype.string",
      "key.length" : 2,
      "key.offset" : 11
    }
  ]
}

@nathawes
Copy link
Collaborator

nathawes commented Nov 7, 2019

@jpsim what substructure were you expecting it to have in this case? The brace statement and call it had previously were considered a bug because they're not actually present in the source.

@jpsim
Copy link
Contributor Author

jpsim commented Nov 7, 2019

You're absolutely right, this is now working as expected. Thanks for following up!

@nathawes
Copy link
Collaborator

nathawes commented Nov 7, 2019

Thanks for checking!

@swift-ci swift-ci transferred this issue from apple/swift-issues Apr 25, 2022
This issue was closed.
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Assignees

@nathawes nathawes

Labels
bug A deviation from expected or documented behavior. Also: expected but undesirable behavior. regression swift 5.1
Projects
None yet
Milestone
No milestone
Development

No branches or pull requests
5 participants
@akyrtzi @jpsim @benlangmuir @AnthonyLatsis @nathawes