[Parser] Emit diagnostics for @ AttributeName and @\nAttributeName

[TiagoMaiaL]

Goal

To fix the second part of issue #1395. The first one (for unexpected whitespace in between the macro declaration), is being fixed by #1395.

Notes

There are some TODOs I've placed to seek assistance from reviewers. Specifically, I don't know how I would synthesize a RawTypeSyntax, as I didn't find an easy way to access its text (without the leading trivia). Any help is appreciated. Thank you.

[ahoppen]

I think the following should do the job

// `withLeadingTrivia` only returns `nil` if there is no token in `attributeName`. 
// But since `attributeName` has leadingTriviaLength != 0 there must be trivia and thus a token.
// So we can safely force-unwrap here.
attributeName.raw.withLeadingTrivia([], arena: self.arena)!.cast(RawTypeSyntax.self)

For this to work you need to mark the withLeadingTrivia (and for consistency also withTrailingTrivia) functions in RawSyntax as @_spi(RawSyntax) public.

I also added the following to RawSyntaxProtocol. I think I’ve wanted to add it a couple of times already but somehow never did.

  /// Cast to the specified raw syntax type and trap if this node is not of that type.
  func cast<Node: RawSyntaxNodeProtocol>(_ syntaxType: Node.Type) -> Node {
    return self.as(Node.self)!
  }

[TiagoMaiaL]

@ahoppen, do you know why this round-trip failure is happening? Maybe the fixed attribute name should be marked as missing? I didn't find a way to do it.

testInvalidNewlineBetweenAtSignAndIdenfifierIsDiagnosed(): failed - Actual output (+) differed from expected output (-):
–@
–MyAttribute
+@MyAttribute
 func foo() {}

Source failed to round-trip.

Actual syntax tree:
SourceFileSyntax
├─statements: CodeBlockItemListSyntax
│ ╰─[0]: CodeBlockItemSyntax
│   ╰─item: FunctionDeclSyntax
│     ├─attributes: AttributeListSyntax
│     │ ╰─[0]: AttributeSyntax
│     │   ├─atSignToken: atSign
│     │   ╰─attributeName: SimpleTypeIdentifierSyntax
│     │     ╰─name: identifier("MyAttribute")
│     ├─funcKeyword: keyword(SwiftSyntax.Keyword.func)
│     ├─identifier: identifier("foo")
│     ├─signature: FunctionSignatureSyntax
│     │ ╰─input: ParameterClauseSyntax
│     │   ├─leftParen: leftParen
│     │   ├─parameterList: FunctionParameterListSyntax
│     │   ╰─rightParen: rightParen
│     ╰─body: CodeBlockSyntax
│       ├─leftBrace: leftBrace
│       ├─statements: CodeBlockItemListSyntax
│       ╰─rightBrace: rightBrace
╰─eofToken: eof

testInvalidNewlineBetweenAtSignAndIdenfifierIsDiagnosed(): failed - Expected 1 diagnostics but received 0:

[ahoppen]

@TiagoMaiaL Just wanted to let you know that I’m making a similar change in #1665 and you might be able to also use handleExtraneousWhitespaceError from it for your PR.

[ahoppen]

Two more minor comments I saw while reading it again, otherwise LGTM.

[ahoppen]

Thank you

[ahoppen]

@swift-ci Please test

[ahoppen]

I suspect this is where the round-trip failures is coming from. You have already stored attributeName in unexpectedBetweenAtSignAndAttributeName and now you’re adding it again with SourcePresence.present. Although it doesn’t quite match the failure message, we have the node twice in the tree now, once with trivia and once without trivia.

The best solution here is a little bit ugly but it’s the best I can come up with: We want to shove the first token of the attributeName (which contains the leading trivia) into unexpectedBetweenAtSignAndAttributeName and instead synthesize a new missing token that doesn’t have leading trivia for the first token in attributeName.

Getting the first token to put into unexpectedBetweenAtSignAndAttributeName should be as easy as doing attributeName.firstToken(viewMode: .sourceAccurate).

To replace the first token in attributeName you need to write a SyntaxRewriter. On the first token that it encounters, it should generate a corresponding missing token without leading trivia. After that, it doesn’t rewrite any further tokens. SyntaxRewriter operates on the Syntax, not RawSyntax level, so you will need to convert attributeName to a Syntax and extract the RawSyntax from the rewritten node. Also, you need to make sure that the rewritten first token is allocated in the same SyntaxArena as all the other tokens. #1383 contains a change to SyntaxRewriter that allows that.

[TiagoMaiaL]

Getting the first token to put into unexpectedBetweenAtSignAndAttributeName should be as easy as doing attributeName.firstToken(viewMode: .sourceAccurate).

The only way I managed to do this was to first declare RawSyntax.firstToken as @_spi(RawSyntax) public and then do the following:

    } else if attributeName.raw.leadingTriviaByteLength != 0,
      let firstAttributeNameToken = attributeName.raw.firstToken(viewMode: .sourceAccurate)
    {
      unexpectedBetweenAtSignAndAttributeName = RawUnexpectedNodesSyntax(
        firstAttributeNameToken.withPresence(.present, arena: self.arena)
      )
      ...

[ahoppen]

Can you push your changes and then I’ll take a look at them.

[ahoppen]

@TiagoMaiaL Are you still working on this? If so, could you rebase on top of main and look at the round-trip failure?

[TiagoMaiaL]

@TiagoMaiaL Are you still working on this? If so, could you rebase on top of main and look at the round-trip failure?

Yes, I'll do it soon. I need to solve some conflicts first, as it's been a while since I don't touch this PR.

[adammcarter]

Hey all, is this still open?

I'm just looking for a nice first issue and looks like this is done but not merged and it's been a while so don't want to step on any toes! 🦶🏻

[ahoppen]

This should have been fixed by #2466