Backticks on identifiers should be separate tokens

grynspan commented 1 year ago

Description

If I have an expression like so:

let x = y.`z`()

Then the syntax node representing z contains the surrounding backticks. I then need to strip these off manually to get at the actual symbol name so I can use it in other contexts like context.makeUniqueName() or diagnostic messages. These backticks should probably be represented by separate tokens, no?

Steps to Reproduce

No response

ahoppen commented 1 year ago

Tracked in Apple’s issue tracker as rdar://112672035

grynspan commented 1 year ago

Or perhaps they should be excluded from .text. Not sure the ideal approach here, but having to manually filter them is slowly driving me to madness. :)

ahoppen commented 1 year ago

It makes sense that the backticks are part of the identifier token because the `z` token is formed in the lexer including the backticks and they thus are part of the identifier token. And we used to represent backticks as trivia at some point years ago but moved away from that for reasons that I can’t recall right now, but there were reasons.

I absolutely agree that we need to have an API on TokenSyntax that removes the trivia. We briefly touched upon this here: https://github.com/apple/swift-syntax/pull/1816#discussion_r1238428108

The suggestion is that we introduce something like the following and then you could use token.identifier.

struct Identifier {
  var name: String

  init(_ token: TokenSyntax) {
     // FIXME: Handle backticks
    self.name = token.text
  }
}

extension TokenSyntax {
  var identifier: Identifier {
    Identifier(self)
  }
}

grynspan commented 1 year ago

Oh, sure—I am maybe fixating on a specific technical solution here. Feel free to come up with a better idea! :)

adammcarter commented 6 months ago

This looks interesting!

I'm currently looking at macros and getting in to swift syntax and would love to contribute my first issue/fix and this one seems a nice isolated one to kick things off

Aware this is 8 months ago so things might have changed but sounds like this is all still viable as above if I were to look in to it? 👨🏻‍💻

ahoppen commented 6 months ago

Yes, the issue is still open and still applies.

adammcarter commented 6 months ago

I'm just looking in to this now, from what I can tell from this issue and the one linked by @ahoppen here: https://github.com/apple/swift-syntax/pull/1816#discussion_r1238428108 ...

It looks like the criteria here is:

Avoid using backticks as their own trivia

we used to represent backticks as trivia at some point years ago but moved away from that for reasons that I can’t recall right now, but there were reasons.
Create a new Identifier type that extracts backticks when creating the name (as per the example here https://github.com/apple/swift-syntax/issues/1936#issuecomment-1649144729)

I'm going to look at this now and see how I get on

adammcarter commented 6 months ago

Hey @ahoppen / @grynspan 👋🏻

I have a branch with the above changes and soon I'll be ready to push but I don't yet have permissions - is someone able to help out? Thanks!

grynspan commented 6 months ago

What you should do is fork the repository (via GitHub) to your account, then move the branch to your fork. From there, GitHub will let you open a PR back to the original repo.

If that's not working, we can look into it.

Matejkob commented 6 months ago

Hi there! It's fantastic to hear that you've prepared changes to contribute!

To submit a Pull Request, you don't need permissions. Here's a quick guide on how to proceed:

Fork this repository.
Clone your fork to your local machine.
Create a new branch in your local repository.
Commit your changes to this new branch.
Push the branch to the origin of your fork.

Once you've pushed your branch, you should see an option on this repository's page to create a PR from a branch in your fork.

For more detailed guidance on contributing to this project, I recommend checking out these resources:

Thanks for your contribution!

adammcarter commented 6 months ago

Amazing thanks both! 👌🏻

Apologies, I did read the contributing guides but didn't see anything re forking the repo. Did I scroll straight past it and it was right in front of me? Or are these steps not included?

If it's the latter, is it worth putting those instructions in that CONTRIBUTING guide @Matejkob ?

Matejkob commented 6 months ago

You're welcome, and no worries at all!

To the best of my knowledge, you're right: neither of the documents explicitly mentions the step about forking the repository. I included those links thinking they might offer additional insights on other aspects of contributing, not realizing the forking step wasn't covered. My apologies for any confusion that may have caused.

It's a good suggestion to add a section about forking to the CONTRIBUTING guide.

grynspan commented 6 months ago

Can you file an issue against the Swift repo about that oversight? Thanks! 😄

adammcarter commented 6 months ago

No worries at all!

Issue opened below for the following repos: