Last week a colleague of mine showed us how he used the new C# String Interpolation feature in his project.
var requestUri = $"api/v1/Colors/{id}/Variants";
which is equivalent to
var requestUri = string.Format("api/v1/Colors/{0}/Variants",id);
I responded that I like the new syntax, but the downside is that it uses the CurrentCulture of the executing thread as the FormatProvider. This makes the behaviour of this code dependant on the thread context which may lead to unexpected results.
This is the same problem that can occur with String.Format() or any other .Net method that uses Formatting. Therefore it is a common best practice to always provide a format provider explicitly to any method that has an overload that supports it. There is also a FxCop / Code Analysis rule that checks for this: CA1305: Specify IFormatProvider.
Fortunately there is also a way to specify the format provider when using string interpolation. The trick is to assign the interpolated string to a FormattableString instead if a normal String. The FormattabelString class then has a ToString(IFormatProvider) overload which allows you to pass the FormatProvider. I found a good explanation on this blog so I will not repeat this here in detail. There is also a nice static helper method on FormattableString that makes it real easy to use the Invariant Culture for your Interpolated strings
using static System.FormattableString;
// ...
var requestUri = Invariant($"api/v1/Colors/{id}/Variants")
(Note how new language features complement each other nicely :-)
So does this mean that we should never use string interpolation without using a helper method like this to set the FormatProvider? Consider the following:
string SayHello(string name)
{
return $"Hello {name}!";
}
In this case we have one interpolation; {name} which has an expression of type string. When a string is placed inside a FormatString it does not mater which FormatProvider is used, it will just be injected in the placeholder as is. In fact, a FormatProvider will only be used for types that implement IFormattable.
So the best practice I propose is:
If an interpolated string has any interpolations that implement IFormattable, the interpolated string SHOULD be assigned to a FormattableString. This way we are sure the developer has made an explicit choice on how the formattable interpolations should be formatted.
The downside of this rule is that it implies that the developer knows which types implement IFormattable in order to be sure. This is where Code Analysers come in. The new .Net Compiler Platform (FKA Roslyn) makes it very easy to create an analyzer that check your code for bad practices, so that is what I did.
I started with the Analyser With Code Fix (C#) template which you can download here. Inside the generated solution there is a class derived from DiagnosticAnalyzer.
[DiagnosticAnalyzer(LanguageNames.CSharp)]
public class StringInterpolationAnalyzer : DiagnosticAnalyzer
{
}
The beginning of this class contains some Localizable strings which are read from a resource file. I updated the resources file with some strings that made sense for this analyzer. The Initialize method is where we tell the runtime which kind of items we want to analyse. In this case this is the InterpolatedStringExpression.
public override void Initialize(AnalysisContext context)
{
context.RegisterSyntaxNodeAction(AnalyzeInterpolatedStringExpression,
SyntaxKind.InterpolatedStringExpression);
}
Now whenever someone types an Interpolated String in the editor the method AnalyzeInterpolatedStringExpression() gets called.
private static void AnalyzeInterpolatedStringExpression(SyntaxNodeAnalysisContext context)
{
// If an interpolated string has any interpolations that implement IFormattable,
// the interpolated string SHOULD be assigned to a FormattableString.
// This way we are sure the developer has made an explicit choice on how the
// formattable interpolations should be formatted.
var interpolatedStringExpression = (InterpolatedStringExpressionSyntax)context.Node;
var typeInfo = context.SemanticModel.GetTypeInfo(interpolatedStringExpression);
// If the Interpolated String Expression is assigned to FormattableString
// we trust it will be formatted correctly
if (typeInfo.ConvertedType.Name == "FormattableString") return;
// Find all InterpolationExpressions that have a type which implements IFormattable
var formattableInterpolations = interpolatedStringExpression.Contents
.OfType<InterpolationSyntax>()
.Where(i => IsFormattable(i.Expression,
context.SemanticModel));
// For each formatable interpolation report a separate Diagnostic
foreach (var interpolation in formattableInterpolations)
{
context.ReportDiagnostic(Diagnostic.Create(Rule,
interpolation.Expression.GetLocation(),
interpolation));
}
}
private static bool IsFormattable(ExpressionSyntax expression, SemanticModel semanticModel)
{
return semanticModel.GetTypeInfo(expression).ConvertedType?.AllInterfaces
.Any(i => i.Name == typeof(IFormattable).Name) ?? false;
}
This is where we can check if the Interpolated string conforms to our new rule. The object model is not very complicated. I drill down the context that is passed in as the parameter to find the InterpolatedStringExpression. The SemanticModel helps to get type information about the nodes in the syntax tree, so we can check if the expression is assigned to a FormattableString.
Next I look at the contents of the interpolatedString. This consists of the literal text tokens and the interpolations. I filter out just the interpolations and check to see if any of them implements IFormattable. If any is found I Report a diagnostic which results in Visual Studio Showing a Squiggle.
I am working on the Code Fix that goes with this analyzer, when it is done I'll post an update with the complete source code.
Let me know what you think of this best practice and code analyzer or any other suggestions or remarks you might have.
No comments:
Post a Comment