Skip to content

Address my own review comments to #1179 #1237

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 4 commits into from
May 9, 2024

Conversation

ahoppen
Copy link
Member

@ahoppen ahoppen commented May 7, 2024

Addresses a few minor comments and the following major ones:

  • Add test cases for the syntax refactorings
  • Don’t report code actions for refactorings that don’t actually modify the source
  • Instead of just looking at the parent of the token of the selected range, walk up the syntax tree to find the syntax node to refactor. This makes the refactorings available in a lot more locations.

@ahoppen ahoppen requested review from DougGregor and bnbarham May 7, 2024 23:22
@ahoppen ahoppen requested a review from benlangmuir as a code owner May 7, 2024 23:22
@ahoppen
Copy link
Member Author

ahoppen commented May 7, 2024

@swift-ci Please test

func testAddDocumentationRefactorSingleParameter() throws {
try assertRefactor(
"""
1️⃣func 2️⃣refactor(syntax: DeclSyntax) { }
Copy link
Contributor

Choose a reason for hiding this comment

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

Is on the label valid? Parameter?

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 doesn't work in the assertRefactor tests because those invoke the refactoring directly from the syntax node. I did add a new code action test case that tests invoking documentation adding from the parameters (because that does the new logic of walking up parents to find the node on which to invoke the refactoring action).

@ahoppen
Copy link
Member Author

ahoppen commented May 8, 2024

Fixed a few more bugs:

  • Documentation was added at the start of a declaration’s trivia. This meant that if there were two newlines before the declaration, the documentation would be separated to the declaration by one newline and if the declaration was at the start of a line, the declaration would be on the same line as the doc comment, effectively making the documentation part of a comment.
  • Don’t show Add documentation if the declaration is not on a new line, like var x = 1; var y = 2
  • Add tests to check that the code action request also works when selecting ranges

@ahoppen
Copy link
Member Author

ahoppen commented May 8, 2024

@swift-ci Please test

@ahoppen
Copy link
Member Author

ahoppen commented May 8, 2024

@swift-ci Please test Windows

Copy link
Member

@DougGregor DougGregor left a comment

Choose a reason for hiding this comment

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

Thank you!

ahoppen added 4 commits May 8, 2024 14:56
Addresses a few minor comments and the following major ones:
- Add test cases for the syntax refactorings
- Don’t report code actions for refactorings that don’t actually modify the source
- Instead of just looking at the parent of the token of the selected range, walk up the syntax tree to find the syntax node to refactor. This makes the refactorings available in a lot more locations.
…eclaration’s trivia

This meant that if there were two newlines before the declaration, the documentation would be separated to the declaration by one newline and if the declaration was at the start of a line, the declaration would be on the same line as the doc comment, effectively making the documentation part of a comment.
@ahoppen ahoppen force-pushed the refactoring-review-comments branch from a45a3cf to f1d6a08 Compare May 8, 2024 22:05
@ahoppen
Copy link
Member Author

ahoppen commented May 8, 2024

@swift-ci Please test

@ahoppen
Copy link
Member Author

ahoppen commented May 9, 2024

@swift-ci Please test Windows

@ahoppen ahoppen merged commit 80694a3 into swiftlang:main May 9, 2024
3 checks passed
@ahoppen ahoppen deleted the refactoring-review-comments branch May 9, 2024 19:02
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