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

Wrote my first Roslyn Diagnostic and Code Fix today.

Icon 4 Comments
Wrote my first Roslyn Diagnostic and Code Fix today.

The idea of it is diagnose to one of the code "smells" from vb6 style of coding in a vb.net program.

If ... Then ... End If

If cond Then
 statement
End If


which can be simplified to a single line if statement.
If cond Then statement





Show me the code! Show me the Code!

If you think, "I'll just chuck a Regex at that and I'm done." then you're sadly mistaken and a liability.
Code is much more hard to analyse.

I have to admit the code which follows may not be the correct way or even efficient to do the job, but hey ho.
Since I'm just starting to learn and understand a rather complex beasts of a dragon. Not only the syntax and grammar of the Roslyn compiler api. but the grammar and syntax rules of the programming languages I'm writing a code fix / analyser for. :shuriken: :magic:

DiagnosticAnalyzer

The diagnostic analyzer is the part that the IDE to display the green squiggle under the specific section of code.

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

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

  Friend Const DiagnosticId = "CodeSmell1"
  Friend Const Description = "If (condtion) Then ...  End If."
  Friend Const MessageFormat = "Replace with single line If (condition) Then ... statement."
  Friend Const Category = "Syntax"

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

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

  Public ReadOnly Property SyntaxKindsOfInterest As ImmutableArray(Of SyntaxKind) Implements ISyntaxNodeAnalyzer(Of SyntaxKind).SyntaxKindsOfInterest
    Get
      Return ImmutableArray.Create(VisualBasic.SyntaxKind.MultiLineIfBlock)
    End Get
  End Property

  Public Sub AnalyzeNode(node As SyntaxNode,
                semanticModel As SemanticModel,
                addDiagnostic As Action(Of Diagnostic),
            cancellationToken As CancellationToken) Implements ISyntaxNodeAnalyzer(Of SyntaxKind).AnalyzeNode
    Dim thisNode = CType(node, MultiLineIfBlockSyntax)
    If thisNode Is Nothing Then Return
    If thisNode.ChildNodesAndTokens.Any(Function(n) n.IsKind(SyntaxKind.ElseIfKeyword) OrElse
                                                      n.IsKind(SyntaxKind.ElseKeyword) OrElse
                                                      n.GetTrailingTrivia.Any(Function(m) m.IsKind(SyntaxKind.CommentTrivia))) Then Return
    If thisNode.IfPart.Statements.Count = 1 Then
      Dim diag = Diagnostic.Create(Rule, Location.Create(node.SyntaxTree, TextSpan.FromBounds(thisNode.FullSpan.Start, thisNode.FullSpan.End)), node)
      addDiagnostic(diag)
    End If
  End Sub
End Class



CodeFix Provider

This part generates the fixed up code.

Option Strict On
Imports Microsoft.CodeAnalysis.Rename
Imports Microsoft.CodeAnalysis.VisualBasic
Imports Microsoft.CodeAnalysis


<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 declaration = root.FindToken(diagnosticSpan.Start).Parent
    Return {CodeAction.Create("Make Single Line IF Statement", Function(c) MakeSingleLineIfStatementAsync(document, declaration, c))}
  End Function

  Private Async Function MakeSingleLineIfStatementAsync(doc As Document, typeStmt As SyntaxNode, cancellationToken As CancellationToken) As Task(Of Document)
    Dim MutlilineIfEndIfStatement = CType(typeStmt.Parent.Parent, MultiLineIfBlockSyntax)
    If MutlilineIfEndIfStatement Is Nothing Then Return Nothing
    Dim statements = MutlilineIfEndIfStatement.IfPart.Statements
    Dim SingleLineIfStatement = SyntaxFactory.ParseCompilationUnit(String.Format("If {0} Then {1}", MutlilineIfEndIfStatement.IfPart.Begin.Condition.ToFullString, statements.ToString)).Members(0)
    Dim root = Await doc.GetSyntaxRootAsync(cancellationToken)
    Dim newRoot = root.ReplaceNode(Of SyntaxNode)(MutlilineIfEndIfStatement, SingleLineIfStatement).NormalizeWhitespace
    Dim newDoc = doc.WithSyntaxRoot(newRoot)
    Return newDoc
  End Function

End Class



Screenshots

Before
Attached Image


During
Attached Image


After
Attached Image




Conclusion

Writing code alter and modify the source code of a programming language isn't simple or easy. It takes a lot of time. What you thought was true sometimes isn't.
I think it's kind of cool and fun. As well as educational.

Next Step

Fix the analyser to recognise when there a comment directly after the Then keyword, as at the moment, any comment is consumed, eaten and destroyed when the code fix is applied.

4 Comments On This Entry

Page 1 of 1

cfoley Icon

07 May 2014 - 06:29 AM
Is that really a code smell? Leaving it as three lines makes the code easier to modify:

* easier to add and remove lines in the body of the if
* easier to show a comment applies to the body
* easier to have commented out alternatives during the process of editing
* easier to split the body into multiple lines
* easier to add a breakpoint to the body
* easier to add assertions, debug output or logging
* easier to add Else or Else If

Combining them to a single line sometimes makes it nicer to read. It's a great option to have but it's not always the best way to go.
0

AdamSpeight2008 Icon

07 May 2014 - 01:31 PM
I consider it a code smell, especially when it only contains one statement. Plus there are multiple occurance in the same method.
An example from the Roslyn source code the Visual Basic Compiler.

  ''' <summary>
  ''' Insert one or more tokens in the list at the specified index.
  ''' </summary>
  ''' <returns>A new list with the tokens inserted.</returns>
  <Extension>
  Public Function Insert(list As SyntaxTokenList, index As Integer, ParamArray items As SyntaxToken()) As SyntaxTokenList
    If index < 0 OrElse index > list.Count Then
       Throw New ArgumentOutOfRangeException("index")
    End If

    If items Is Nothing Then
       Throw New ArgumentNullException("items")
    End If

    If list.Count = 0 Then
       Return SyntaxFactory.TokenList(items)
    Else
      Dim builder = New Syntax.SyntaxTokenListBuilder(list.Count + items.Length)
      If index > 0 Then
         builder.Add(list, 0, index)
      End If

      builder.Add(items)

      If index < list.Count Then
         builder.Add(list, index, list.Count - index)
      End If

      Return builder.ToList()
    End If
  End Function



The same code rewritten to use a single line If-Statements.
        ''' <summary>
        ''' Insert one or more tokens in the list at the specified index.
        ''' </summary>
        ''' <returns>A new list with the tokens inserted.</returns>
        <Extension>
        Public Function Insert(list As SyntaxTokenList, index As Integer, ParamArray items As SyntaxToken()) As SyntaxTokenList
            If (index < 0) OrElse (index > list.Count) Then Throw New ArgumentOutOfRangeException("index")
            If items Is Nothing                        Then Throw New ArgumentNullException("items")
            If list.Count = 0                          Then Return SyntaxFactory.TokenList(items)
            Dim builder = New Syntax.SyntaxTokenListBuilder(list.Count + items.Length)
            If index > 0 Then  builder.Add(list, 0, index)
            builder.Add(items)
            If index < list.Count Then builder.Add(list, index, list.Count - index)
            Return builder.ToList()
        End Function



It's half the size and much easier to scan.
0

cfoley Icon

07 May 2014 - 04:32 PM
Half the length, twice the line width, no blank lines for grouping sections and if you want to add logging or more lines of code you have to revert to the original anyway.

What is more important? Ease of reading or ease of modification. Ease of reading is subjective (I like the shape of the first one, you like the shortness of the second) but the first one is certainly more easily modified.
0

cfoley Icon

12 May 2014 - 04:01 AM
Here is a very interesting article that explores code readability. It uses single vs multiline conditionals as one of its examples and discusses when each might be appropriate.

http://blog.8thlight...oding-slow.html
0
Page 1 of 1

Search My Blog

Recent Comments