Skip to content

Correctly find the node to fix in regex generator codefix#72079

Merged
joperezr merged 3 commits into
dotnet:mainfrom
Youssef1313:most-inner-for-tie-regex-codefix
Jul 19, 2022
Merged

Correctly find the node to fix in regex generator codefix#72079
joperezr merged 3 commits into
dotnet:mainfrom
Youssef1313:most-inner-for-tie-regex-codefix

Conversation

@Youssef1313

Copy link
Copy Markdown
Member

Fixes #72047

@ghost ghost added area-System.Text.RegularExpressions community-contribution Indicates that the PR has been added by a community member labels Jul 13, 2022
@ghost

ghost commented Jul 13, 2022

Copy link
Copy Markdown

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

Issue Details

Fixes #72047

Author: Youssef1313
Assignees: -
Labels:

area-System.Text.RegularExpressions, community-contribution

Milestone: -

Comment on lines -89 to -96
// Find the node that corresponding to the diagnostic which we will then fix.
SyntaxNode? root = await document.GetSyntaxRootAsync(cancellationToken).ConfigureAwait(false);
if (root is null)
{
return document;
}

SyntaxNode nodeToFix = root.FindNode(diagnostic.Location.SourceSpan, getInnermostNodeForTie: false);

Copy link
Copy Markdown
Member Author

Choose a reason for hiding this comment

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

No need to re-calculate these. They were already calculated and can be passed along. I think the result cannot have changed in-between the calls, basically because Document is immutable and we're using the same document to re-calculate.

@Youssef1313

Copy link
Copy Markdown
Member Author

Tagging @joperezr @stephentoub for review.

@joperezr
joperezr requested a review from CyrusNajmabadi July 13, 2022 19:11

@joperezr joperezr left a comment

Copy link
Copy Markdown
Member

Choose a reason for hiding this comment

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

Thanks @Youssef1313.

@CyrusNajmabadi

Copy link
Copy Markdown
Contributor

Thanks @Youssef1313. You're awesome!

@joperezr

Copy link
Copy Markdown
Member

Looks like my latest PR caused a merge conflict on the tests. I took the liberty of fixing the merge conflict and pushing to your PR, I hope that is ok. Will merge this as soon as CI is green.

@Youssef1313

Copy link
Copy Markdown
Member Author

@joperezr CI failures are unrelated.

@joperezr
joperezr merged commit e0f614d into dotnet:main Jul 19, 2022
@ghost ghost locked as resolved and limited conversation to collaborators Aug 19, 2022
Sign up for free to subscribe to this conversation on GitHub. Already have an account? Sign in.

Labels

area-System.Text.RegularExpressions 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.

UpgradeToRegexGeneratorCodeFixer crashes with InvalidCastException

3 participants