HiveBrain v1.2.0
Get Started
← Back to all entries
patterncsharpMinor

Analyzing naming conventions

Submitted by: @import:stackexchange-codereview··
0
Viewed 0 times
conventionsanalyzingnaming

Problem

I just created a naming convention analyzer + code fix based on the Roslyn platform. The implemented naming conventions are the following:

If you're interested in seeing all the test scenarios that have been covered, you can take a look here.

Some notes about the implementation:

-
Analyzers and code fixes cannot share data between each other, which is a major pain. That's why I have to go find out which convention to use in my analyzer and do that again in my code fix. Though now that I think about it: maybe I can extract this common behaviour to a helper method. It's still double work but at least I won't have to write it double. Feel free to provide a proposal implementation.

-
Verbatim identifiers (@class) and Unicode-littered identifiers (cl\u0061ss) are not applicable for a renaming.

-
If the identifier contains special characters, I don't touch it either.

-
I just realized I didn't account for members without explicit modifiers. You can ignore that aspect and pretend I don't do them.

You can view the PR related to this post here.

Analyzer

```
using System.Collections.Immutable;
using System.Linq;
using Microsoft.CodeAnalysis;
using Microsoft.CodeAnalysis.CSharp;
using Microsoft.CodeAnalysis.CSharp.Syntax;
using Microsoft.CodeAnalysis.Diagnostics;
using VSDiagnostics.Utilities;

namespace VSDiagnostics.Diagnostics.General.NamingConventions
{
[DiagnosticAnalyzer(LanguageNames.CSharp)]
public class NamingConventionsAnalyzer : DiagnosticAnalyzer
{
public const string DiagnosticId = nameof(NamingConventionsAnalyzer);
internal const string Title = "A member does not follow naming conventions.";
internal const string Message = "The {0} {1} does not follow naming conventions. Should be {2}.";
internal const string Category = "General";
internal const DiagnosticSeverity Severity = DiagnosticSeverity.Warning;
internal static DiagnosticDescriptor Rule = new DiagnosticDescriptor(DiagnosticId, Tit

Solution

if (nodeAsField.Modifiers.Any(x => new[] { "internal", "protected", "public" }.Contains(x.Text)))
{
    // ...
}
else if (nodeAsField.Modifiers.Any(x => x.Text == "private"))
{
    // ...
}


I find the following alternative a little easier to read, and less prone to typos:

var modifiers = nodeAsField.Modifiers;
if (modifiers.Any(SyntaxKind.InternalKeyword) ||
    modifiers.Any(SyntaxKind.ProtectedKeyword) ||
    modifiers.Any(SyntaxKind.PublicKeyword))
{
    // ...
}
else if (modifiers.Any(SyntaxKind.PrivateKeyword))
{
    // ...
}


It might also be more performant since we avoid string comparisons and array allocations in the lambda (though of course you should measure this yourself).

Heap Allocations Viewer also warns that in the original code nodeAsField.Modifiers is boxed.

I noticed that structs are missing from both the naming conventions table and the code.

Code Snippets

if (nodeAsField.Modifiers.Any(x => new[] { "internal", "protected", "public" }.Contains(x.Text)))
{
    // ...
}
else if (nodeAsField.Modifiers.Any(x => x.Text == "private"))
{
    // ...
}
var modifiers = nodeAsField.Modifiers;
if (modifiers.Any(SyntaxKind.InternalKeyword) ||
    modifiers.Any(SyntaxKind.ProtectedKeyword) ||
    modifiers.Any(SyntaxKind.PublicKeyword))
{
    // ...
}
else if (modifiers.Any(SyntaxKind.PrivateKeyword))
{
    // ...
}

Context

StackExchange Code Review Q#92506, answer score: 5

Revisions (0)

No revisions yet.