Skip to content

SwiftSyntax: allow any Syntax nodes to access their leading/trailing trivia. #15278

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
merged 3 commits into from
Mar 15, 2018
Merged

SwiftSyntax: allow any Syntax nodes to access their leading/trailing trivia. #15278

merged 3 commits into from
Mar 15, 2018

Conversation

nkcsgexi
Copy link
Contributor

Although libSyntax is designed in a way that trivia is attached to
tokens, we shouldn't restrict clients to access trivia only from a token.
An example can be doc-comment, which conceptually attaches to a declaration rather
than the start token of a declaration, like at-attributes.

…trivia.

Although libSyntax is designed in a way that trivia is attached to
tokens, we shouldn't restrict clients to access trivia only from a token.
An example can be doc-comment, which conceptually attaches to a declaration rather
than the start token of a declaration, like at-attributes.
@nkcsgexi
Copy link
Contributor Author

@swift-ci please smoke test

@nkcsgexi
Copy link
Contributor Author

@harlanhaskins @rintaro Any objection to this?

PositionTests.test("Trivias") {
expectDoesNotThrow({
var l = [CodeBlockItemSyntax]()
let leading = Trivia.newlines(1).appending(TriviaPiece.backticks(1))
Copy link
Contributor

Choose a reason for hiding this comment

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

This could be cleaner as:

let leading = Trivia(pieces: [
  .newlines(1),
  .backticks(1),
  .docLineComment("/// some comment")
])

let leading = Trivia.newlines(1).appending(TriviaPiece.backticks(1))
.appending(TriviaPiece.docLineComment("/// some comment"))
let trailing = Trivia.docLineComment("/// This is comment\n")
let idx = 5
Copy link
Contributor

Choose a reason for hiding this comment

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

Probably would be a good idea to Make the ReturnStmt here then just say

let l = [CodeBlockItemSyntax](repeating: returnStmt, count: 5)

for _ in 0...idx {
l.append(SyntaxFactory.makeCodeBlockItem(
item: SyntaxFactory.makeReturnStmt(
returnKeyword: SyntaxFactory.makeToken(.returnKeyword, presence: .present)
Copy link
Contributor

@harlanhaskins harlanhaskins Mar 15, 2018

Choose a reason for hiding this comment

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

ReturnStmtSyntax {
  $0.useReturnKeyword(
    SyntaxFactory.makeReturnKeyword(
      leadingTrivia: leading, 
      trailingTrivia: trailing))
}

@nkcsgexi
Copy link
Contributor Author

@swift-ci please smoke test

/// The textual byte length of this node exluding leading and trailing trivia.
public var byteSizeAfterTrimmingTrivia: Int {
return data.byteSize -
(leadingTrivia == nil ? 0 : leadingTrivia!.byteSize) -
Copy link
Contributor

Choose a reason for hiding this comment

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

These should probably be leadingTrivia?.byteSize ?? 0 to avoid the force-unwrap.

@nkcsgexi
Copy link
Contributor Author

@swift-ci please smoke test

@nkcsgexi nkcsgexi merged commit 1c9f1c4 into swiftlang:master Mar 15, 2018
@nkcsgexi nkcsgexi deleted the trivia-syntax-node branch March 15, 2018 22:31
@harlanhaskins
Copy link
Contributor

🎉

maldahleh pushed a commit to maldahleh/swift that referenced this pull request Oct 26, 2020
…trivia. (swiftlang#15278)

Although libSyntax is designed in a way that trivia is attached to
tokens, we shouldn't restrict clients to access trivia only from a token.
An example can be doc-comment, which conceptually attaches to a declaration rather
than the start token of a declaration, like at-attributes.
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.

2 participants