I've written another diagnostic for VSDiagnostics that adds compile-time security for your formatting specified in a string.Format call. The exact scenario it guards against is when you have something like this:
string s = string.Format("Hello {0}, I'm {1}!", "John");
This will throw a runtime exception because it cannot find an argument for the second placeholder. What my analyzer does is evaluate the format, looks at the eligible placeholders and compares that with the amount of arguments that have been passed in. When it determines that the format is invalid for the given arguments, it will underline the format and report an error-level diagnostic.
Note that the purpose of this analyzer should be the above scenario. Other things like placeholders being in a non-lexical order or unused placeholders are handled separately.
There are a few restrictions in place:
- The specified format has to be a literal -- it cannot be retrieved from a field, method call or anything else
- Interpolated strings as a format specified are not (yet) supported
- If the arguments are passed in using an explicit array, only inline initialized arrays are accepted. Arrays defined elsewhere are not supported.
I am mostly interested in finding a way to make the logic of selecting the format a little more cleanly. Additionally (perhaps more importantly): can you find a scenario that I haven't accounted for? All verified scenarios so far can be found here.
Analyzer Github
[DiagnosticAnalyzer(LanguageNames.CSharp)]
public class StringDotFormatWithDifferentAmountOfArgumentsAnalyzer : DiagnosticAnalyzer
{
private const DiagnosticSeverity Severity = DiagnosticSeverity.Warning;
private static readonly string Category = VSDiagnosticsResources.StringsCategory;
private static readonly string Message = VSDiagnosticsResources.StringDotFormatWithDifferentAmountOfArgumentsMessage;
private static readonly string Title = VSDiagnosticsResources.StringDotFormatWithDifferentAmountOfArgumentsTitle;
internal static DiagnosticDescriptor Rule =>
new DiagnosticDescriptor(DiagnosticId.StringDotFormatWithDifferentAmountOfArguments, Title, Message, Category, Severity, true);
public override ImmutableArray<DiagnosticDescriptor> SupportedDiagnostics => ImmutableArray.Create(Rule);
public override void Initialize(AnalysisContext context)
{
context.RegisterSyntaxNodeAction(AnalyzeNode, SyntaxKind.InvocationExpression);
}
private void AnalyzeNode(SyntaxNodeAnalysisContext context)
{
var invocation = context.Node as InvocationExpressionSyntax;
if (invocation == null)
{
return;
}
// Verify we're dealing with a string.Format() call
if (!invocation.IsAnInvocationOf(typeof(string), nameof(string.Format), context.SemanticModel))
{
return;
}
if (invocation.ArgumentList == null)
{
return;
}
// Verify the format is a literal expression and not a method invocation or an identifier
// The overloads are in the form string.Format(string, object[]) or string.Format(CultureInfo, string, object[])
var allArguments = invocation.ArgumentList.Arguments;
var firstArgument = allArguments.ElementAtOrDefault(0, null);
var secondArgument = allArguments.ElementAtOrDefault(1, null);
if (firstArgument == null)
{
return;
}
var firstArgumentIsLiteral = firstArgument.Expression is LiteralExpressionSyntax;
var secondArgumentIsLiteral = secondArgument?.Expression is LiteralExpressionSyntax;
if (!firstArgumentIsLiteral && !secondArgumentIsLiteral)
{
return;
}
// We ignore interpolated strings for now (workitem tracked in https://github.com/Vannevelj/VSDiagnostics/issues/313)
if (firstArgument.Expression is InterpolatedStringExpressionSyntax)
{
return;
}
// If we got here, it means that the either the first or the second argument is a literal.
// If the first is a literal then that is our format
var formatString = firstArgumentIsLiteral
? ((LiteralExpressionSyntax) firstArgument.Expression).GetText().ToString()
: ((LiteralExpressionSyntax) secondArgument.Expression).GetText().ToString();
// Get the total amount of arguments passed in for the format
// If the first one is the literal (aka: the format specified) then every other argument is an argument to the format
// If not, it means the first one is the CultureInfo, the second is the format and all others are format arguments
// We also have to check whether or not the arguments are passed in through an explicit array or whether they use the params syntax
var formatArguments = firstArgumentIsLiteral
? allArguments.Skip(1).ToArray()
: allArguments.Skip(2).ToArray();
var amountOfFormatArguments = formatArguments.Length;
if (formatArguments.Length == 1)
{
// Inline array creation à la string.Format("{0}", new object[] { "test" })
var arrayCreation = formatArguments[0].Expression as ArrayCreationExpressionSyntax;
if (arrayCreation?.Initializer?.Expressions != null)
{
amountOfFormatArguments = arrayCreation.Initializer.Expressions.Count;
}
// We don't handle method calls
var invocationExpression = formatArguments[0].Expression as InvocationExpressionSyntax;
if (invocationExpression != null)
{
return;
}
// If it's an identifier, we don't handle those that provide an array as a single argument
// Other types are fine though -- think about string.Format("{0}", name);
var referencedIdentifier = formatArguments[0].Expression as IdentifierNameSyntax;
if (referencedIdentifier != null)
{
// This is also hit by any other kind of identifier so we have to differentiate
var referencedType = context.SemanticModel.GetTypeInfo(referencedIdentifier);
if (referencedType.Type == null || referencedType.Type is IErrorTypeSymbol)
{
return;
}
if (referencedType.Type.TypeKind.HasFlag(SyntaxKind.ArrayType))
{
// If we got here it means the arguments are passed in through an identifier which resolves to an array
// aka: calling a method that returns an array or referencing a variable/field that is of type array
// We cannot reliably get the amount of arguments if it's a method
// We could get them when it's a field/variable/property but that takes some more work and thinking about it
// This is tracked in workitem https://github.com/Vannevelj/VSDiagnostics/issues/330
return;
}
}
}
// Get the placeholders we use, stripped off their format specifier, get the highest value
// and verify that this value + 1 (to account for 0-based indexing) is not greater than the amount of placeholder arguments
var placeholders = PlaceholderHelpers.GetPlaceholders(formatString)
.Cast<Match>()
.Select(x => x.Value)
.Select(PlaceholderHelpers.GetPlaceholderIndex)
.Select(int.Parse)
.ToList();
if (!placeholders.Any())
{
return;
}
var highestPlaceholder = placeholders.Max();
if (highestPlaceholder + 1 > amountOfFormatArguments)
{
context.ReportDiagnostic(Diagnostic.Create(Rule, firstArgumentIsLiteral
? firstArgument.GetLocation()
: secondArgument.GetLocation()));
}
}
}
Extensions Github
public static class Extensions
{
// TODO: tests
// NOTE: string.Format() vs Format() (current/external type)
public static bool IsAnInvocationOf(this InvocationExpressionSyntax invocation, Type type, string method, SemanticModel semanticModel)
{
var memberAccessExpression = invocation?.Expression as MemberAccessExpressionSyntax;
if (memberAccessExpression == null)
{
return false;
}
var invokedType = semanticModel.GetSymbolInfo(memberAccessExpression.Expression);
var invokedMethod = semanticModel.GetSymbolInfo(memberAccessExpression.Name);
if (invokedType.Symbol == null || invokedMethod.Symbol == null)
{
return false;
}
return invokedType.Symbol.MetadataName == type.Name &&
invokedMethod.Symbol.MetadataName == method;
}
// TODO: tests
public static T ElementAtOrDefault<T>(this IEnumerable<T> list, int index, T @default)
{
return index >= 0 && index < list.Count() ? list.ElementAt(index) : @default;
}
}
PlaceholderHelpers Github
internal static class PlaceholderHelpers
{
/// <summary>
/// Removes all curly braces and formatting definitions from the placeholder
/// </summary>
/// <param name="input">The placeholder entry to parse.</param>
/// <returns>Returns the placeholder index.</returns>
internal static string GetPlaceholderIndex(string input)
{
var temp = input.Trim('{', '}');
var colonIndex = temp.IndexOf(':');
if (colonIndex > 0)
{
return temp.Remove(colonIndex);
}
return temp;
}
/// <summary>
/// Get all elements in a string that are enclosed by an uneven amount of curly brackets (to account for escaped
/// brackets).
/// The result will be elements that are either plain integers or integers with a format appended to it, delimited by a
/// colon.
/// </summary>
/// <param name="input">The format string with placeholders.</param>
/// <returns>Returns a collection of matches according to the regex.</returns>
internal static MatchCollection GetPlaceholders(string input)
{
// This regex uses a named group so we can easily access the actual value
return Regex.Matches(input, @"(?<!\{)\{(?:\{\{)*((?<index>\d+)(?::.*?)?)\}(?:\}\})*(?!\})");
}
/// <summary>
/// Returns all elements from the input, split on the placeholders.
/// This method is useful if you want to make use of the rest of the string as well.
/// </summary>
internal static string[] GetPlaceholdersSplit(string input)
{
return Regex.Split(input, @"(?<!\{)\{(?:\{\{)*(\d+(?::.*?)?)\}(?:\}\})*(?!\})");
}
}
Unit tests Github
Tests have been omitted for brevity. You can take a look at the Github link to see which scenarios I have accounted for.
Amountshould beNumber. \$\endgroup\$