Skip to content

Improve usability of "enable nullable reference types" refactoring#58316

Merged
sharwell merged 3 commits intodotnet:mainfrom
sharwell:nullable-refactoring
Dec 15, 2021
Merged

Improve usability of "enable nullable reference types" refactoring#58316
sharwell merged 3 commits intodotnet:mainfrom
sharwell:nullable-refactoring

Conversation

@sharwell
Copy link
Copy Markdown
Contributor

See #57476

@sharwell sharwell requested a review from a team as a code owner December 14, 2021 00:51
@ghost ghost added the Area-IDE label Dec 14, 2021

// Only the input solution contains '#nullable enable'
if (!document.GetTextSynchronously(CancellationToken.None).ToString().Contains("#nullable enable"))
if (!Regex.IsMatch(document.GetTextSynchronously(CancellationToken.None).ToString(), "#nullable ?enable"))
Copy link
Copy Markdown
Contributor

Choose a reason for hiding this comment

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

❓My regex fu is limited. Is this matching 0 or 1 spaces before enable? Or is the ? applied to enable somehow?

Copy link
Copy Markdown
Contributor

Choose a reason for hiding this comment

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

yeah, i don't quite get the need for this. afaict, this is allowing for either one or two spaces after #nullable. Is that really needed?

Copy link
Copy Markdown
Contributor

Choose a reason for hiding this comment

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

maybe + or \s+ if you want to match any amount of whitespace?

Copy link
Copy Markdown
Member

@Youssef1313 Youssef1313 Dec 14, 2021

Choose a reason for hiding this comment

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

Spaces between # and nullable are allowed by the compiler (it's a test code anyway, so probably doesn't matter).

Copy link
Copy Markdown
Contributor Author

@sharwell sharwell Dec 14, 2021

Choose a reason for hiding this comment

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

afaict, this is allowing for either one or two spaces after #nullable. Is that really needed?

Yes. All of the tests have one space except for one test that has two spaces. I'll update the comment.

Copy link
Copy Markdown
Contributor Author

Choose a reason for hiding this comment

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

➡️ Comment is now updated

Copy link
Copy Markdown
Contributor Author

Choose a reason for hiding this comment

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

This whole method is a workaround for the fact that the test library can't cleanly represent a code action that changes compilation options. Considering a few options and this will be updated after.

@sharwell sharwell force-pushed the nullable-refactoring branch from 92787bd to b885da9 Compare December 14, 2021 16:26
@sharwell sharwell merged commit cdd0aca into dotnet:main Dec 15, 2021
@sharwell sharwell deleted the nullable-refactoring branch December 15, 2021 00:20
@ghost ghost added this to the Next milestone Dec 15, 2021
@Cosifne Cosifne modified the milestones: Next, 17.1.P3 Jan 5, 2022
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment

Labels

Projects

None yet

Development

Successfully merging this pull request may close these issues.

5 participants