Skip to content

Add support for semantic functionality in macro expansion reference documents #1634

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
Aug 21, 2024

Conversation

lokesh-tr
Copy link
Contributor

@lokesh-tr lokesh-tr commented Aug 19, 2024

Reimplementation of #1610 after #1631

This PR migrates several requests made to sourcekitd to support Semantic Functionality in Macro Expansion Reference Documents.

What has been migrated and works?

  1. cursorInfo
  2. documentDiagnosticReport
  3. openInterface
  4. semanticTokens
  5. relatedIdentifiers (used by documentSymbolHighlight)
  6. Nested Macro Expansions

What has been migrated but doesn’t work?

  • inlayHints (variableTypeInfo) probably needs a fix in sourcekitd

What won't be migrated since they don't apply for reference documents?

  • Rename (Symbol)
  • Code Completion Session
  • RefactoringResponse.refactoring (i.e. Semantic Refactor)

Note:

  • A fix for the percent encoding issue will be done in the sourcekit-lsp PR and this PR will only work in conjunction with that.
  • Test cases for semantic functionality will be available in a follow-up PR

Moreover, this PR also does the following:

  1. Updates outdated doc comments from Support expansion of nested macros #1631
  2. Fixes an issue where wrong file and position passed in PeekDocumentsRequest from Support expansion of nested macros #1631

Expansion of Swift Macros in Visual Studio Code - Google Summer Of Code 2024
@lokesh-tr @ahoppen @adam-fowler

@lokesh-tr
Copy link
Contributor Author

Since test cases are passing for nested macro expansion. I would like to assume that the issue with semantic functionality not working is due to vs code / extension.

@lokesh-tr
Copy link
Contributor Author

I will try to come up with test cases for semantic functionality.

@lokesh-tr lokesh-tr force-pushed the support-semantic-functionality branch 2 times, most recently from ead445c to 2254675 Compare August 19, 2024 14:56
@lokesh-tr
Copy link
Contributor Author

lokesh-tr commented Aug 19, 2024

@ahoppen This feature is complete now, I will work on the test cases.

It would be nice if you can merge this and I can work on the test cases in a separate PR.

Everything works now :)

@lokesh-tr lokesh-tr force-pushed the support-semantic-functionality branch from 2254675 to 3021bbc Compare August 19, 2024 18:41
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.

This looks great! I think that the changes to MacroExpansionReferenceURLData aren’t needed if #1636 ends up fixing the URL encoding issue with VS Code.

@lokesh-tr lokesh-tr force-pushed the support-semantic-functionality branch from 3021bbc to be824f5 Compare August 20, 2024 02:40
@lokesh-tr
Copy link
Contributor Author

@ahoppen ready for review

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.

Looks great!

@ahoppen
Copy link
Member

ahoppen commented Aug 20, 2024

@swift-ci Please test

@ahoppen ahoppen enabled auto-merge August 20, 2024 04:46
@ahoppen
Copy link
Member

ahoppen commented Aug 20, 2024

@swift-ci Please test Windows

@lokesh-tr
Copy link
Contributor Author

@ahoppen what exactly causes the CI to fail? Is it due to the .enableUpcomingFeature("BareSlashRegexLiterals") ? What to do to make the CI to succeed?

@ahoppen
Copy link
Member

ahoppen commented Aug 20, 2024

#1638 should fix the Swift 5.10 build by just not using bare slash regex literals. That way we don’t need to update the package manifest. Could you undo your package manifest change? The PR should be fine in CI without it because it has a Swift 6 compiler with bare slash regex literals enabled.

auto-merge was automatically disabled August 20, 2024 15:28

Head branch was pushed to by a user without write access

@lokesh-tr lokesh-tr force-pushed the support-semantic-functionality branch from be824f5 to 1792f59 Compare August 20, 2024 15:28
@lokesh-tr
Copy link
Contributor Author

@ahoppen I updated the PR as you said. Can you please trigger the CI once again?

@lokesh-tr lokesh-tr force-pushed the support-semantic-functionality branch from 71f3656 to 0784041 Compare August 21, 2024 09:31
@ahoppen
Copy link
Member

ahoppen commented Aug 21, 2024

@swift-ci Please test

@ahoppen
Copy link
Member

ahoppen commented Aug 21, 2024

@swift-ci Please test Windows

@ahoppen ahoppen enabled auto-merge August 21, 2024 16:28
@ahoppen ahoppen merged commit d11c101 into swiftlang:main Aug 21, 2024
3 checks passed
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