Skip to content

Add Uri StringSyntaxAttribute syntaxes #44570

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 6 commits into from
Oct 17, 2022

Conversation

blouflashdb
Copy link
Contributor

Add Uri StringSyntaxAttribute syntaxes

  • You've read the Contributor Guide and Code of Conduct.
  • You've included unit or integration tests for your change, where applicable.
  • You've included inline docs for your change, where applicable.
  • There's an open issue for the PR that you are making. If you'd like to propose a new feature or change, please open an issue to discuss the change or find an existing issue.

Description

Adds uri StringSyntaxAttribute syntaxes to all public APIs where an uri is expected.

Closes Uri Task in #44535

@ghost ghost added the community-contribution Indicates that the PR has been added by a community member label Oct 15, 2022
@ghost
Copy link

ghost commented Oct 15, 2022

Thanks for your PR, @blouflashdb. Someone from the team will get assigned to your PR shortly and we'll get it reviewed.

@blouflashdb
Copy link
Contributor Author

Well somehow I can't use it in HubConnectionBuilderHttpExtensions.cs for some reason.
Check failure on line 26 in src/SignalR/clients/csharp/Client/src/HubConnectionBuilderHttpExtensions.cs

Azure Pipelines
/ aspnetcore-ci (Build Build: Linux ARM)
src/SignalR/clients/csharp/Client/src/HubConnectionBuilderHttpExtensions.cs#L26
src/SignalR/clients/csharp/Client/src/HubConnectionBuilderHttpExtensions.cs(26,131): error CS0122: (NETCORE_ENGINEERING_TELEMETRY=Build) 'StringSyntaxAttribute' is inaccessible due to its protection level

@JamesNK
Copy link
Member

JamesNK commented Oct 16, 2022

The SignalR client also targets .NET Framework, which doesn't have that attribute.

To work around this:

  1. Copy the attribute's source code from StringSyntaxAttribute.cs
  2. Add an internal polyfill type in the Shared/CodeAnalysis directory - https://github.com/dotnet/aspnetcore/tree/1b53b22a866eb4bc58f2d5552db7f3ac08f1a952/src/Shared/CodeAnalysis. The trimming types already in that directory are a good example of this technique.
  3. Finally, add a link to the shared source file from the SignalR client project.

It doesn't matter that the attribute is internal and in a different assembly. As long as the type name and namespace match, tooling and other apps can use it.

@blouflashdb
Copy link
Contributor Author

Thanks, I think I got it.

Copy link
Member

@JamesNK JamesNK left a comment

Choose a reason for hiding this comment

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

RelativeOrAbsolute would be the default assumption so there is no need to explicitly specify it.

Otherwise, LGTM

@JamesNK JamesNK merged commit fa2fd21 into dotnet:main Oct 17, 2022
@ghost ghost added this to the 8.0-preview1 milestone Oct 17, 2022
@blouflashdb blouflashdb deleted the addUriStringSyntaxAttribute branch October 17, 2022 14:20
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
community-contribution Indicates that the PR has been added by a community member
Projects
None yet
Development

Successfully merging this pull request may close these issues.

2 participants