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

Changed IDE0059 codefix title in pattern matching #60822

Merged
merged 3 commits into from
May 3, 2022

Conversation

DoctorKrolic
Copy link
Contributor

Fixes: #40002

@DoctorKrolic DoctorKrolic requested a review from a team as a code owner April 18, 2022 21:32
@ghost ghost added the Community The pull request was submitted by a contributor who is not a Microsoft employee. label Apr 18, 2022
@CyrusNajmabadi
Copy link
Member

@DoctorKrolic Answered on Discord. Thanks! :)

@DoctorKrolic
Copy link
Contributor Author

@CyrusNajmabadi please review

Copy link
Member

@CyrusNajmabadi CyrusNajmabadi left a comment

Choose a reason for hiding this comment

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

Thanks!

auto-merge was automatically disabled April 29, 2022 15:24

Head branch was pushed to by a user without write access

@DoctorKrolic
Copy link
Contributor Author

@CyrusNajmabadi I checked failing integration tests and found out, that IsUnusedLocalDiagnostic will be reported for all unused local nodes, e.g.:

class Program
{
  public static void Main()
  {
    var x = new Program();
  }
}

x will have IsUnusedLocalDiagnostic tag, but codefix will produce this code:

class Program
{
  public static void Main()
  {
    var _ = new Program();
  }
}

So the decision of whether should node be removed or changed to discard is fully implemented in codefixe's code. This method replaces all declaration patterns with type declarations:

protected override SyntaxNode TryUpdateParentOfUpdatedNode(SyntaxNode parent, SyntaxNode newNameNode, SyntaxEditor editor, ISyntaxFacts syntaxFacts)
{
if (newNameNode.IsKind(SyntaxKind.DiscardDesignation)
&& parent.IsKind(SyntaxKind.DeclarationPattern, out DeclarationPatternSyntax declarationPattern)
&& parent.SyntaxTree.Options.LanguageVersion() >= LanguageVersion.CSharp9)
{
var trailingTrivia = declarationPattern.Type.GetTrailingTrivia()
.AddRange(newNameNode.GetLeadingTrivia())
.AddRange(newNameNode.GetTrailingTrivia());
return SyntaxFactory.TypePattern(declarationPattern.Type).WithTrailingTrivia(trailingTrivia);
}
return null;
}

So in cases like

var a = "str";
if (a is object obj)
{
}

declaration pattern object obj will be changed to just type declaration object

Considering this fact, I reverted logic of PR back to just checking for declaration pattern when deciding what codefix title to show

@DoctorKrolic
Copy link
Contributor Author

@CyrusNajmabadi PTAL

@CyrusNajmabadi
Copy link
Member

Thanks :)

@CyrusNajmabadi CyrusNajmabadi merged commit c4be259 into dotnet:main May 3, 2022
@ghost ghost added this to the Next milestone May 3, 2022
@DoctorKrolic DoctorKrolic deleted the fix-codefix-title branch May 3, 2022 21:11
@Cosifne Cosifne modified the milestones: Next, 17.3 P2 May 31, 2022
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
Area-IDE Community The pull request was submitted by a contributor who is not a Microsoft employee.
Projects
None yet
Development

Successfully merging this pull request may close these issues.

Description of code fix for "unnecessary assignment" is misleading when the variable is removed
3 participants