Skip to content

Remove SwiftSyntaxParser module #1544

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 1 commit into from
Apr 17, 2023

Conversation

ahoppen
Copy link
Member

@ahoppen ahoppen commented Apr 14, 2023

This removes the SwiftSyntaxParser module, which was just a wrapper around SwiftParser to ease the transition for clients that previously used SwiftSyntaxParser. Everybody should be using SwiftParser by now, so remove the deprecated module.

This migrates all test cases that still provide value to other modules.

@ahoppen ahoppen requested a review from bnbarham April 14, 2023 20:26
@ahoppen ahoppen requested a review from keith as a code owner April 14, 2023 20:26
@ahoppen ahoppen requested a review from DougGregor April 14, 2023 20:26
@ahoppen
Copy link
Member Author

ahoppen commented Apr 14, 2023

@swift-ci Please test macOS

Copy link
Contributor

@bnbarham bnbarham left a comment

Choose a reason for hiding this comment

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

💃

@@ -1,3 +0,0 @@
// A closure without a signature. The test will ensure it stays the same after
Copy link
Contributor

Choose a reason for hiding this comment

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

I haven't looked through all these, but I assume you moved ones you thought necessary and deleted otherwise?

Copy link
Member Author

Choose a reason for hiding this comment

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

Yes, most of them became initializers calls to create syntax trees.

/// }
/// ```
///
/// The source file is hard-coded so this test case doesn't need to depend on the parser.
Copy link
Contributor

Choose a reason for hiding this comment

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

Is there some reason not to depend on the parser here?

Copy link
Member Author

Choose a reason for hiding this comment

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

It’s just that this test is in SwiftSyntaxTest and I don’t think we should be depending on the parser from SwiftSyntaxTest because it’s lower level. Also, it didn’t make any sense for me to move this file to SwiftParserTest because that’s not where it belongs, so I ended up with this solution, which I don’t find super terrible.

@ahoppen ahoppen force-pushed the ahoppen/remove-swiftsyntaxparser branch from 8e204f6 to 0484816 Compare April 14, 2023 22:13
@ahoppen ahoppen changed the title Remove SwiftSyntaxParser module Remove SwiftSyntaxParser module 🚥 #1541 Apr 15, 2023
@ahoppen ahoppen force-pushed the ahoppen/remove-swiftsyntaxparser branch from 0484816 to 3ac1944 Compare April 15, 2023 20:59
@ahoppen ahoppen changed the title Remove SwiftSyntaxParser module 🚥 #1541 Remove SwiftSyntaxParser module Apr 15, 2023
This removes the `SwiftSyntaxParser` module, which was just a wrapper around `SwiftParser` to ease the transition for clients that previously used `SwiftSyntaxParser`. Everybody should be using `SwiftParser` by now, so remove the deprecated module.

This migrates all test cases that still provide value to other modules.
@ahoppen ahoppen force-pushed the ahoppen/remove-swiftsyntaxparser branch from 3ac1944 to b79e559 Compare April 15, 2023 21:02
@ahoppen
Copy link
Member Author

ahoppen commented Apr 15, 2023

@swift-ci Please test

@ahoppen
Copy link
Member Author

ahoppen commented Apr 15, 2023

@swift-ci Please test Windows

2 similar comments
@ahoppen
Copy link
Member Author

ahoppen commented Apr 15, 2023

@swift-ci Please test Windows

@ahoppen
Copy link
Member Author

ahoppen commented Apr 16, 2023

@swift-ci Please test Windows

@ahoppen ahoppen merged commit 94120e0 into swiftlang:main Apr 17, 2023
@ahoppen ahoppen deleted the ahoppen/remove-swiftsyntaxparser branch April 17, 2023 23:47
ahoppen added a commit to ahoppen/swift-syntax that referenced this pull request Apr 18, 2023
…ntaxparser

Remove SwiftSyntaxParser module
ahoppen added a commit to ahoppen/swift-syntax that referenced this pull request Apr 18, 2023
…ntaxparser

Remove SwiftSyntaxParser module
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.

3 participants