Skip to content
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

Avoid global:: qualifying all names in the Regex source generator #63512

Closed
stephentoub opened this issue Jan 7, 2022 · 6 comments · Fixed by #66432
Closed

Avoid global:: qualifying all names in the Regex source generator #63512

stephentoub opened this issue Jan 7, 2022 · 6 comments · Fixed by #66432
Labels
area-System.Text.RegularExpressions enhancement Product code improvement that does NOT require public API changes/additions
Milestone

Comments

@stephentoub
Copy link
Member

Blocked on dotnet/csharplang#5529

In the Regex source generator, we currently fully-qualify with global:: every name to something in the core libraries, e.g. global::System.MemoryExtensions.IndexOf(span, value) rather than span.IndexOf(value). This is done to avoid accidentally referencing something of the same name in the user's code, but it also leads to generating code that's much harder to read (and the generator itself is more verbose).

Once we have a solution to dotnet/csharplang#5529, we can spit the Regex-derived type into its own namespace, and put usings inside that namespace for all of the System.* namespaces we consume. At that point, we will no longer need to fully-qualify and can go back to using simple names, using extension methods as extensions, etc.

@ghost
Copy link

ghost commented Jan 7, 2022

Tagging subscribers to this area: @dotnet/area-system-text-regularexpressions
See info in area-owners.md if you want to be subscribed.

Issue Details

Blocked on dotnet/csharplang#5529

In the Regex source generator, we currently fully-qualify with global:: every name to something in the core libraries, e.g. global::System.MemoryExtensions.IndexOf(span, value) rather than span.IndexOf(value). This is done to avoid accidentally referencing something of the same name in the user's code, but it also leads to generating code that's much harder to read (and the generator itself is more verbose).

Once we have a solution to dotnet/csharplang#5529, we can spit the Regex-derived type into its own namespace, and put usings inside that namespace for all of the System.* namespaces we consume. At that point, we will no longer need to fully-qualify and can go back to using simple names, using extension methods as extensions, etc.

Author: stephentoub
Assignees: -
Labels:

area-System.Text.RegularExpressions

Milestone: 7.0.0

@dotnet-issue-labeler dotnet-issue-labeler bot added the untriaged New issue has not been triaged by the area owner label Jan 7, 2022
@stephentoub
Copy link
Member Author

cc: @jaredpar

@ShreyasJejurkar
Copy link
Contributor

Wouldn't aliasing work here in this case!?

using SystemStringBuilder = System.Text.StringBuilder

@stephentoub
Copy link
Member Author

Wouldn't aliasing work here in this case!?
using SystemStringBuilder = System.Text.StringBuilder

No. Beyond that resulting in similarly less readable code, it doesn't actually solve the problem.
https://sharplab.io/#v2:EYLgtghglgdgNAFxBAzmAPjCYCmKAOEAxjgAQCqKOATgHQAiOAZrDgCYCwAUAN7ekDSAAQBMwgIwBlBNVgBzAEIBXKABs2NUj1IBfbnq7csuAsTKUaDZq069+goQAYJ02TEUr1mgLwTaAFRwADwRaV3llNQ1qAG57AVgEGixVUioIVXZhMQBheK18wWEAZgkANmEAFlIAWQAKAEpCwT4uIvbBGBwAdxcZCM9oxoCAe3D3Rri2jt18gx0gA==

@gfoidl
Copy link
Member

gfoidl commented Jan 9, 2022

This should / could also be done for e.g. the LoggerMessages generated by the source generator.

@stephentoub
Copy link
Member Author

Yes, I'm using Regex as a case study for all of these as it's new in 7, but we should adopt similar strategies for our other generators, including the DllImport generator that's also new in 7.

@jeffschwMSFT jeffschwMSFT removed the untriaged New issue has not been triaged by the area owner label Jan 11, 2022
@joperezr joperezr added the enhancement Product code improvement that does NOT require public API changes/additions label Jan 19, 2022
@ghost ghost added the in-pr There is an active PR which will close this issue when it is merged label Mar 10, 2022
@ghost ghost removed the in-pr There is an active PR which will close this issue when it is merged label Mar 10, 2022
@ghost ghost locked as resolved and limited conversation to collaborators Apr 10, 2022
Sign up for free to subscribe to this conversation on GitHub. Already have an account? Sign in.
Labels
area-System.Text.RegularExpressions enhancement Product code improvement that does NOT require public API changes/additions
Projects
None yet
Development

Successfully merging a pull request may close this issue.

5 participants