Skip to content

Update swift syntax #1217

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

Closed
wants to merge 1 commit into from
Closed

Conversation

kimdv
Copy link
Contributor

@kimdv kimdv commented Jan 11, 2023

No description provided.

@kimdv kimdv requested a review from ahoppen as a code owner January 11, 2023 09:47
@kimdv kimdv force-pushed the kimdv/update-swift-syntax branch from fb801de to a790d59 Compare January 11, 2023 09:51
@@ -26,7 +26,7 @@ open class BasicFormat: SyntaxRewriter {
Trivia(pieces: [.newlines(1), indentation])
}

private var lastRewrittenToken: TokenSyntax?
private var lastRewrittenToken: TokenSyntax?
Copy link
Contributor Author

Choose a reason for hiding this comment

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

Could we in basic format make if next token == nil return no trailing space

Copy link
Member

@ahoppen ahoppen left a comment

Choose a reason for hiding this comment

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

There are a few regressions in terms of formatting IMO. It would be nice to fix them in BasicFormat in the future but this doesn’t have to block this PR.

@@ -48,13 +48,13 @@ open class BasicFormat: SyntaxRewriter {
open override func visit(_ node: TokenSyntax) -> TokenSyntax {
var leadingTrivia = node.leadingTrivia
var trailingTrivia = node.trailingTrivia
if requiresLeadingSpace(node) && leadingTrivia.isEmpty && lastRewrittenToken?.trailingTrivia.isEmpty != false {
if requiresLeadingSpace(node) && leadingTrivia.isEmpty && lastRewrittenToken? .trailingTrivia.isEmpty != false {
Copy link
Member

Choose a reason for hiding this comment

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

😬 This seems like a regression. It’s fine now but it would be nice to fix this in BasicFormat for the future

@@ -28,7 +28,7 @@ extension AccessorDecl {
/// A convenience initializer that allows:
/// - Initializing syntax collections using result builders
/// - Initializing tokens without default text using strings
public init(leadingTrivia: Trivia? = nil, unexpectedBeforeAttributes: UnexpectedNodes? = nil, attributes: AttributeList? = nil, unexpectedBetweenAttributesAndModifier: UnexpectedNodes? = nil, modifier: DeclModifier? = nil, unexpectedBetweenModifierAndAccessorKind: UnexpectedNodes? = nil, accessorKind: Token, unexpectedBetweenAccessorKindAndParameter: UnexpectedNodes? = nil, parameter: AccessorParameter? = nil, unexpectedBetweenParameterAndAsyncKeyword: UnexpectedNodes? = nil, asyncKeyword: Keyword?, unexpectedBetweenAsyncKeywordAndThrowsKeyword: UnexpectedNodes? = nil, throwsKeyword: Token? = nil, unexpectedBetweenThrowsKeywordAndBody: UnexpectedNodes? = nil, @CodeBlockItemListBuilder bodyBuilder: () -> CodeBlockItemListSyntax? = {
public init(leadingTrivia: Trivia? = nil, unexpectedBeforeAttributes: UnexpectedNodes? = nil, attributes: AttributeList? = nil, unexpectedBetweenAttributesAndModifier: UnexpectedNodes? = nil, modifier: DeclModifier? = nil, unexpectedBetweenModifierAndAccessorKind: UnexpectedNodes? = nil, accessorKind: Token, unexpectedBetweenAccessorKindAndParameter: UnexpectedNodes? = nil, parameter: AccessorParameter? = nil, unexpectedBetweenParameterAndAsyncKeyword: UnexpectedNodes? = nil, asyncKeyword: Keyword? , unexpectedBetweenAsyncKeywordAndThrowsKeyword: UnexpectedNodes? = nil, throwsKeyword: Token? = nil, unexpectedBetweenThrowsKeywordAndBody: UnexpectedNodes? = nil, @CodeBlockItemListBuilder bodyBuilder: () -> CodeBlockItemListSyntax? = {
Copy link
Member

Choose a reason for hiding this comment

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

Same here, seems like an unfortunate regression.

Copy link
Contributor Author

Choose a reason for hiding this comment

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

leadingTrivia += .space
}
if requiresTrailingSpace(node) && trailingTrivia.isEmpty {
trailingTrivia += .space
}
if let keyPath = getKeyPath(Syntax(node)), requiresLeadingNewline(keyPath), !(leadingTrivia.first?.isNewline ?? false) {
if let keyPath = getKeyPath(Syntax(node)), requiresLeadingNewline(keyPath), !(leadingTrivia.first? .isNewline ?? false) {
Copy link
Contributor Author

Choose a reason for hiding this comment

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

We missed some cases here.
Will open a new PR

Copy link
Contributor Author

Choose a reason for hiding this comment

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

@ahoppen
Copy link
Member

ahoppen commented Jan 13, 2023

Closing in favor of #1230.

@ahoppen ahoppen closed this Jan 13, 2023
@kimdv kimdv deleted the kimdv/update-swift-syntax branch January 13, 2023 21:42
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