Subscribe to The Madman Scribblings        RSS Feed
-----

Roslyn Diagnostic & Code Fix for Mis-Assignment in Constructor

Icon 1 Comments
Roslyn Diagnostic & Code Fix for Mis-Assignment in Constructor

Used: VS2014ctp4

Background

This Rosyln based diagnostic and code-fix for both VB.net and C# stems from a issue / feature request on the Roslyn project site. (Better Code Fix support for bad constructor assignments)


Quote

Frequently in constructors we do something like this

public foo(string bar) 
{ 
_bar = bar; 
this.bar = bar' // which one depends on our habits/guidelines 
}


If the user inadvertently says

bar = bar


the current code fix just offers to disable the warning - not what the user wants.

Please add a code fix that says "yo dude, you meant to assign to a field" (alternate wording anticipated).

These two patterns are so common, that explicitly fixing for them seems desirable.

And a refactor to just make all those assignments would be pretty nifty too.

Kathleen





Problem Analysis

target = source

Is the target of the assignment a parameter of the constructor? Then this is likely a mistake.

If so, offer to rewrite the assignment to,
  • _target = source

  • Me.target = source (VB.net)

  • this.target = source (C#)


The VB.net and C# diagnostic and codefix are implemented using VB.net. The shows you can implement a diagnostic for a different target language.




VB.net

Diagnostic

Imports System.Collections.Immutable
Imports Microsoft.CodeAnalysis.Diagnostics

<DiagnosticAnalyzer(LanguageNames.VisualBasic)>
Public Class DiagnosticAnalyzer
  Implements ISyntaxNodeAnalyzer(Of SyntaxKind)

  Friend Const DiagnosticId = "BadConstructorAssignment"
  Friend Const Description = "Did you mean to assign to a parameter arguement?"
  Friend Const MessageFormat = "Did you mean to assign to a parameter arguement? (Parameter name '{0}') "
  Friend Const Category = "Naming"

  Friend Shared Rule As New DiagnosticDescriptor(DiagnosticId, Description, MessageFormat, Category, DiagnosticSeverity.Warning, True)

  Private ReadOnly Property SyntaxKindsOfInterest As ImmutableArray(Of SyntaxKind) Implements ISyntaxNodeAnalyzer(Of SyntaxKind).SyntaxKindsOfInterest
    Get
      Return ImmutableArray.Create(SyntaxKind.SubNewStatement)
    End Get
  End Property



This says run out diagnostic when the compiler encounters a SubNewStatement (VB.net's constructor).


  Public ReadOnly Property SupportedDiagnostics As ImmutableArray(Of DiagnosticDescriptor) Implements IDiagnosticAnalyzer.SupportedDiagnostics
    Get
      Return ImmutableArray.Create(Rule)
    End Get
  End Property



  Public Sub AnalyzeNode(node As SyntaxNode, semanticModel As SemanticModel, addDiagnostic As Action(Of Diagnostic), options As AnalyzerOptions, cancellationToken As CancellationToken) Implements ISyntaxNodeAnalyzer(Of SyntaxKind).AnalyzeNode
    Dim constructor = TryCast(node, Syntax.SubNewStatementSyntax)
    If constructor Is Nothing Then Exit Sub
    Dim st = constructor.SyntaxTree
    Dim names = constructor.ParameterList.Parameters.Select(Function(p) p.Identifier.Identifier.GetIdentifierText)
    For Each statement In DirectCast(constructor.Parent, ConstructorBlockSyntax).Statements.OfType(Of AssignmentStatementSyntax).Where(Function(a) TypeOf a.Left Is SimpleNameSyntax)
      Dim lhs_Id = CType(statement.Left, SimpleNameSyntax).Identifier
      If names.Contains(lhs_Id.GetIdentifierText()) Then addDiagnostic(Diagnostic.Create(Rule, Location.Create(st, statement.Left.Span), lhs_Id))
    Next   
  End Sub
End Class



This checks to see if the method is a constructor method.
Gets the list of parameter names used be the constructor.
For each of the statements (lines of code) within the constructor's body, that are an assignment statement and the target (a.Left) is a simple name identifier.
Then checks to see the name of the target identifier is one of the parameter names, if so add a diagnostic on that node.

OK know we have identified the syntax node that have an issue (mis-assignment)
Let's write a couple of codefixes to rewrite the sourcecode.

CodeFix

<ExportCodeFixProvider(DiagnosticAnalyzer.DiagnosticId, LanguageNames.VisualBasic)>
Friend Class CodeFixProvider
  Implements ICodeFixProvider

  Public Function GetFixableDiagnosticIds() As IEnumerable(Of String) Implements ICodeFixProvider.GetFixableDiagnosticIds
    Return {DiagnosticAnalyzer.DiagnosticId}
  End Function

  Public Async Function GetFixesAsync(document As Document, span As TextSpan, diagnostics As IEnumerable(Of Diagnostic), cancellationToken As CancellationToken) As Task(Of IEnumerable(Of CodeAction)) Implements ICodeFixProvider.GetFixesAsync
    Dim root = Await document.GetSyntaxRootAsync(cancellationToken).ConfigureAwait(False)
    Dim diagnosticSpan = diagnostics.First().Location.SourceSpan
    Dim declarations = root.FindToken(diagnosticSpan.Start).Parent.AncestorsAndSelf().OfType(Of SimpleNameSyntax)()
    If declarations.Any Then
      Dim ID_Node = declarations.First
      Dim Assignment_Statement = TryCast(ID_Node.Parent, AssignmentStatementSyntax)
      If Assignment_Statement Is Nothing Then Return {}
      Return {CodeAction.Create("Prefix with underscore", Function(c) Prefix_WithUnderscore(document, Assignment_Statement, c)),
               CodeAction.Create("Prefix with Me.", Function(c) Prefix_WithMeDot(document, Assignment_Statement, c))}
    End If
    Return {}
  End Function



  Private Async Function Prefix_WithUnderscore(document As Document, assignment_statement As AssignmentStatementSyntax, cancellationToken As CancellationToken) As Task(Of Solution)
    Dim LHS = DirectCast(assignment_statement.Left, SimpleNameSyntax)
    Dim _LHS = LHS.WithIdentifier(SyntaxFactory.Identifier("_" & LHS.Identifier.Text))
    Dim semanticModel = Await document.GetSemanticModelAsync(cancellationToken)
    Dim root = Await semanticModel.SyntaxTree.GetRootAsync
    Dim tree = VisualBasicSyntaxTree.Create(root.ReplaceNode(LHS, _LHS))
    Dim newSolution = document.WithSyntaxRoot(Await tree.GetRootAsync)
    Return newSolution.Project.Solution
  End Function



  Private Async Function Prefix_WithMeDot(document As Document, assignment_statement As AssignmentStatementSyntax, cancellationToken As CancellationToken) As Task(Of Solution)
    Dim LHS = DirectCast(assignment_statement.Left, SimpleNameSyntax)
    Dim _LHS = SyntaxFactory.ExpressionStatement(SyntaxFactory.InvocationExpression(SyntaxFactory.SimpleMemberAccessExpression(SyntaxFactory.MeExpression, SyntaxFactory.IdentifierName(LHS.Identifier.Text))))
    Dim ass = SyntaxFactory.SimpleAssignmentStatement(_LHS.Expression, assignment_statement.Right)
    Dim semanticModel = Await document.GetSemanticModelAsync(cancellationToken)
    Dim root = Await semanticModel.SyntaxTree.GetRootAsync
    Dim tree = VisualBasicSyntaxTree.Create(root.ReplaceNode(Of SyntaxNode)(assignment_statement, ass))
    Dim newSolution = document.WithSyntaxRoot(Await tree.GetRootAsync)
    Return newSolution.Project.Solution
  End Function
End Class






CSharp

Diagnostic

Imports System.Collections.Immutable
Imports Microsoft.CodeAnalysis.Diagnostics
Imports Microsoft.CodeAnalysis.CSharp.CSharpExtensions
Imports Microsoft.CodeAnalysis.CSharp.Syntax
Imports Microsoft.CodeAnalysis.CSharp.SyntaxExtensions


<DiagnosticAnalyzer(LanguageNames.CSharp)>
Public Class DiagnosticAnalyzer
  Implements ISyntaxNodeAnalyzer(Of SyntaxKind)

  Friend Const DiagnosticId = "BadConstructorAssignment"
  Friend Const Description = "Did you mean to assign to a parameter arguement?"
  Friend Const MessageFormat = "Did you mean to assign to a parameter arguement? (Parameter name '{0}') "
  Friend Const Category = "Naming"

  Friend Shared Rule As New DiagnosticDescriptor(DiagnosticId, Description, MessageFormat, Category, DiagnosticSeverity.Warning, True)

  Private ReadOnly Property SyntaxKindsOfInterest As ImmutableArray(Of SyntaxKind) Implements ISyntaxNodeAnalyzer(Of SyntaxKind).SyntaxKindsOfInterest
    Get
      Return ImmutableArray.Create(SyntaxKind.ConstructorDeclaration )
    End Get
  End Property

  Public ReadOnly Property SupportedDiagnostics As ImmutableArray(Of DiagnosticDescriptor) Implements IDiagnosticAnalyzer.SupportedDiagnostics
    Get
      Return ImmutableArray.Create(Rule)
    End Get
  End Property



You'd think it be easy to find mis-assignment nodes in C#
  Public Sub AnalyzeNode(node As SyntaxNode, semanticModel As SemanticModel, addDiagnostic As Action(Of Diagnostic), options As AnalyzerOptions, cancellationToken As CancellationToken) Implements ISyntaxNodeAnalyzer(Of SyntaxKind).AnalyzeNode
    Dim constructor = TryCast(node, Syntax.ConstructorDeclarationSyntax)
    If constructor Is Nothing Then Exit Sub
    Dim st = constructor.SyntaxTree
    Dim names = constructor.ParameterList.Parameters.Select(Function(p) p.Identifier.Text)
    Dim statements = constructor.Body.Statements.OfType(Of ExpressionStatementSyntax).Select(Function(s) s.Expression).OfType(of BinaryExpressionSyntax).Where(Function(s) s.IsKind(SyntaxKind.SimpleAssignmentExpression)) 
    For Each statement In statements
      Dim lhs_Id = CType(statement.Left, SimpleNameSyntax).Identifier
      If names.Contains(lhs_Id.Text) Then addDiagnostic(Diagnostic.Create(Rule, Location.Create(st, statement.Left.Span), lhs_Id))
    Next
  End Sub
End Class


Nope, it a little long winded.
  • Is it an ExpressionStatement?
  • Is it a BinaryExpression?
  • Is it a SimpleAssignment?


Whereas in VB.net you can do simple type check on the node AssignmentStatementSyntax.

Now we've finally gotten the issue nodes. Let's implement the codefixes.

CodeFix

Imports Microsoft.CodeAnalysis.Rename
Imports System.Collections.Immutable
Imports Microsoft.CodeAnalysis.Diagnostics
Imports Microsoft.CodeAnalysis.CSharp.CSharpExtensions
Imports Microsoft.CodeAnalysis.CSharp.Syntax
Imports Microsoft.CodeAnalysis.CSharp.SyntaxExtensions

<ExportCodeFixProvider(DiagnosticAnalyzer.DiagnosticId, LanguageNames.CSharp)>
Friend Class CodeFixProvider
  Implements ICodeFixProvider

  Public Function GetFixableDiagnosticIds() As IEnumerable(Of String) Implements ICodeFixProvider.GetFixableDiagnosticIds
    Return {DiagnosticAnalyzer.DiagnosticId}
  End Function

  Public Async Function GetFixesAsync(document As Document, span As TextSpan, diagnostics As IEnumerable(Of Diagnostic), cancellationToken As CancellationToken) As Task(Of IEnumerable(Of CodeAction)) Implements ICodeFixProvider.GetFixesAsync
    Dim root = Await document.GetSyntaxRootAsync(cancellationToken).ConfigureAwait(False)
    Dim diagnosticSpan = diagnostics.First().Location.SourceSpan
    Dim declarations = root.FindToken(diagnosticSpan.Start).Parent.AncestorsAndSelf().OfType(Of SimpleNameSyntax)()
    If declarations.Any Then
      Dim ID_Node = declarations.First
      Dim Assignment_Statement = TryCast(ID_Node.Parent, BinaryExpressionSyntax)
      If Assignment_Statement Is Nothing Then Return {}
      Return {CodeAction.Create("Prefix with underscore", Function(c) Prefix_WithUnderscore(document, Assignment_Statement, c)),
              CodeAction.Create("Prefix with this.", Function(c) Prefix_WithThisDot(document, Assignment_Statement, c))}
    End If
    Return {}
  End Function



  Private Async Function Prefix_WithUnderscore(document As Document, assignment_statement As BinaryExpressionSyntax, cancellationToken As CancellationToken) As Task(Of Solution)
    Dim LHS = DirectCast(assignment_statement.Left, SimpleNameSyntax)
    Dim _LHS = LHS.WithIdentifier(SyntaxFactory.Identifier("_" & LHS.Identifier.Text))
    Dim semanticModel = Await document.GetSemanticModelAsync(cancellationToken)
    Dim root = Await semanticModel.SyntaxTree.GetRootAsync
    Dim tree = CSharpSyntaxTree.Create(CType(root.ReplaceNode(LHS, _LHS), CSharpSyntaxNode))
    Dim newSolution = document.WithSyntaxRoot(Await tree.GetRootAsync)
    Return newSolution.Project.Solution
  End Function


Very similar to the VB.net implementation.


Now the this. variation.
  Private Async Function Prefix_WithThisDot(document As Document, assignment_statement As BinaryExpressionSyntax, cancellationToken As CancellationToken) As Task(Of Solution)
    Dim LHS = DirectCast(assignment_statement.Left, SimpleNameSyntax)
    Dim _LHS = SyntaxFactory.ExpressionStatement(
             SyntaxFactory.MemberAccessExpression(SyntaxKind.SimpleMemberAccessExpression, SyntaxFactory.ThisExpression, SyntaxFactory.IdentifierName(LHS.Identifier.Text))
               )
    Dim ass = SyntaxFactory.BinaryExpression(SyntaxKind.SimpleAssignmentExpression, _LHS.Expression, assignment_statement.Right)
    Dim semanticModel = Await document.GetSemanticModelAsync(cancellationToken)
    Dim root = Await semanticModel.SyntaxTree.GetRootAsync
    Dim tree = CSharpSyntaxTree.Create(CType(root.ReplaceNode(Of SyntaxNode)(assignment_statement, ass), CSharpSyntaxNode))
    Dim newSolution = document.WithSyntaxRoot(Await tree.GetRootAsync)
    Return newSolution.Project.Solution
  End Function
End Class






Fairly easy to implement both diagnostic and codefix. The complexity comes from the usage of syntax nodes to find and create code.

1 Comments On This Entry

Page 1 of 1

lucky3 Icon

13 November 2014 - 02:03 PM
I've actually read this post. Great!
0
Page 1 of 1

Search My Blog

Recent Entries

Recent Comments