Skip to content
Merged
Show file tree
Hide file tree
Changes from all commits
Commits
File filter

Filter by extension

Filter by extension

Conversations
Failed to load comments.
Loading
Jump to
Jump to file
Failed to load files.
Loading
Diff view
Diff view
2 changes: 1 addition & 1 deletion README.md
Original file line number Diff line number Diff line change
Expand Up @@ -197,7 +197,7 @@ If you are already using other analyzers, you can check [which rules are duplica
|[MA0180](https://github.com/meziantou/Meziantou.Analyzer/blob/main/docs/Rules/MA0180.md)|Design|ILogger type parameter should match containing type|⚠️|❌|✔️|
|[MA0181](https://github.com/meziantou/Meziantou.Analyzer/blob/main/docs/Rules/MA0181.md)|Style|Do not use cast|ℹ️|❌|❌|
|[MA0182](https://github.com/meziantou/Meziantou.Analyzer/blob/main/docs/Rules/MA0182.md)|Design|Avoid unused internal types|ℹ️|✔️|✔️|
|[MA0183](https://github.com/meziantou/Meziantou.Analyzer/blob/main/docs/Rules/MA0183.md)|Usage|string.Format should use a format string with placeholders|⚠️|✔️|❌|
|[MA0183](https://github.com/meziantou/Meziantou.Analyzer/blob/main/docs/Rules/MA0183.md)|Usage|The format string should use placeholders|⚠️|✔️|❌|
|[MA0184](https://github.com/meziantou/Meziantou.Analyzer/blob/main/docs/Rules/MA0184.md)|Style|Do not use interpolated string without parameters|👻|✔️|✔️|
|[MA0185](https://github.com/meziantou/Meziantou.Analyzer/blob/main/docs/Rules/MA0185.md)|Performance|Simplify string.Create when all parameters are culture invariant|ℹ️|✔️|✔️|
|[MA0186](https://github.com/meziantou/Meziantou.Analyzer/blob/main/docs/Rules/MA0186.md)|Design|Equals method should use \[NotNullWhen(true)\] on the parameter|ℹ️|❌|❌|
Expand Down
6 changes: 3 additions & 3 deletions docs/README.md
Original file line number Diff line number Diff line change
Expand Up @@ -181,7 +181,7 @@
|[MA0180](https://github.com/meziantou/Meziantou.Analyzer/blob/main/docs/Rules/MA0180.md)|Design|ILogger type parameter should match containing type|<span title='Warning'>⚠️</span>|❌|✔️|
|[MA0181](https://github.com/meziantou/Meziantou.Analyzer/blob/main/docs/Rules/MA0181.md)|Style|Do not use cast|<span title='Info'>ℹ️</span>|❌|❌|
|[MA0182](https://github.com/meziantou/Meziantou.Analyzer/blob/main/docs/Rules/MA0182.md)|Design|Avoid unused internal types|<span title='Info'>ℹ️</span>|✔️|✔️|
|[MA0183](https://github.com/meziantou/Meziantou.Analyzer/blob/main/docs/Rules/MA0183.md)|Usage|string.Format should use a format string with placeholders|<span title='Warning'>⚠️</span>|✔️|❌|
|[MA0183](https://github.com/meziantou/Meziantou.Analyzer/blob/main/docs/Rules/MA0183.md)|Usage|The format string should use placeholders|<span title='Warning'>⚠️</span>|✔️|❌|
|[MA0184](https://github.com/meziantou/Meziantou.Analyzer/blob/main/docs/Rules/MA0184.md)|Style|Do not use interpolated string without parameters|<span title='Hidden'>👻</span>|✔️|✔️|
|[MA0185](https://github.com/meziantou/Meziantou.Analyzer/blob/main/docs/Rules/MA0185.md)|Performance|Simplify string.Create when all parameters are culture invariant|<span title='Info'>ℹ️</span>|✔️|✔️|
|[MA0186](https://github.com/meziantou/Meziantou.Analyzer/blob/main/docs/Rules/MA0186.md)|Design|Equals method should use \[NotNullWhen(true)\] on the parameter|<span title='Info'>ℹ️</span>|❌|❌|
Expand Down Expand Up @@ -744,7 +744,7 @@ dotnet_diagnostic.MA0181.severity = none
# MA0182: Avoid unused internal types
dotnet_diagnostic.MA0182.severity = suggestion

# MA0183: string.Format should use a format string with placeholders
# MA0183: The format string should use placeholders
dotnet_diagnostic.MA0183.severity = warning

# MA0184: Do not use interpolated string without parameters
Expand Down Expand Up @@ -1300,7 +1300,7 @@ dotnet_diagnostic.MA0181.severity = none
# MA0182: Avoid unused internal types
dotnet_diagnostic.MA0182.severity = none

# MA0183: string.Format should use a format string with placeholders
# MA0183: The format string should use placeholders
dotnet_diagnostic.MA0183.severity = none

# MA0184: Do not use interpolated string without parameters
Expand Down
11 changes: 10 additions & 1 deletion docs/Rules/MA0183.md
Original file line number Diff line number Diff line change
@@ -1,4 +1,13 @@
# MA0183 - string.Format should use a format string with placeholders
# MA0183 - The format string should use placeholders
<!-- sources -->
Source: [StringFormatShouldBeConstantAnalyzer.cs](https://github.com/meziantou/Meziantou.Analyzer/blob/main/src/Meziantou.Analyzer/Rules/StringFormatShouldBeConstantAnalyzer.cs)
<!-- sources -->

This rule detects calls to formatting methods where the format string has no placeholders but formatting arguments are provided, or where a formatting method is called with no arguments at all (making the format call redundant).

The following methods are checked:

- `string.Format`
- `Console.Write`
- `Console.WriteLine`
- `System.Text.StringBuilder.AppendFormat`
Original file line number Diff line number Diff line change
Expand Up @@ -542,7 +542,7 @@ dotnet_diagnostic.MA0181.severity = none
# MA0182: Avoid unused internal types
dotnet_diagnostic.MA0182.severity = suggestion

# MA0183: string.Format should use a format string with placeholders
# MA0183: The format string should use placeholders
dotnet_diagnostic.MA0183.severity = warning

# MA0184: Do not use interpolated string without parameters
Expand Down
Original file line number Diff line number Diff line change
Expand Up @@ -542,7 +542,7 @@ dotnet_diagnostic.MA0181.severity = none
# MA0182: Avoid unused internal types
dotnet_diagnostic.MA0182.severity = none

# MA0183: string.Format should use a format string with placeholders
# MA0183: The format string should use placeholders
dotnet_diagnostic.MA0183.severity = none

# MA0184: Do not use interpolated string without parameters
Expand Down
133 changes: 79 additions & 54 deletions src/Meziantou.Analyzer/Rules/StringFormatShouldBeConstantAnalyzer.cs
Original file line number Diff line number Diff line change
Expand Up @@ -11,8 +11,8 @@ public sealed class StringFormatShouldBeConstantAnalyzer : DiagnosticAnalyzer
{
private static readonly DiagnosticDescriptor Rule = new(
RuleIdentifiers.StringFormatShouldBeConstant,
title: "string.Format should use a format string with placeholders",
messageFormat: "Use string literal instead of string.Format when the format string has no placeholders",
title: "The format string should use placeholders",
messageFormat: "Use a string literal instead of a format method when the format string has no placeholders",
RuleCategories.Usage,
DiagnosticSeverity.Warning,
isEnabledByDefault: true,
Expand All @@ -25,54 +25,99 @@ public override void Initialize(AnalysisContext context)
{
context.EnableConcurrentExecution();
context.ConfigureGeneratedCodeAnalysis(GeneratedCodeAnalysisFlags.None);
context.RegisterOperationAction(Analyze, OperationKind.Invocation);
context.RegisterCompilationStartAction(compilationStartContext =>
{
var formatProviderType = compilationStartContext.Compilation.GetBestTypeByMetadataName("System.IFormatProvider");
var consoleType = compilationStartContext.Compilation.GetBestTypeByMetadataName("System.Console");
var stringBuilderType = compilationStartContext.Compilation.GetBestTypeByMetadataName("System.Text.StringBuilder");

compilationStartContext.RegisterOperationAction(
context => Analyze(context, formatProviderType, consoleType, stringBuilderType),
OperationKind.Invocation);
});
}

private static void Analyze(OperationAnalysisContext context)
private static void Analyze(OperationAnalysisContext context, ITypeSymbol? formatProviderType, ITypeSymbol? consoleType, ITypeSymbol? stringBuilderType)
{
var operation = (IInvocationOperation)context.Operation;
var method = operation.TargetMethod;

// Check if it's a string.Format call
if (operation.TargetMethod.Name != "Format")
if (operation.Arguments.Length == 0)
return;

if (operation.TargetMethod.ContainingType.SpecialType != SpecialType.System_String)
// Determine if this is a known format method, what the format arg index is,
// and whether to report when there are no formatting arguments.
int formatArgumentIndex;
bool reportWhenNoFormattingArgs;

if (method.ContainingType.SpecialType == SpecialType.System_String && method.Name == "Format")
{
// string.Format - report even when called with no args (e.g. string.Format("abc"))
reportWhenNoFormattingArgs = true;
formatArgumentIndex = GetFormatArgIndex(operation, formatProviderType);
}
else if (consoleType is not null && method.ContainingType.IsEqualTo(consoleType) &&
(method.Name == "Write" || method.Name == "WriteLine"))
{
// Console.Write / Console.WriteLine - only report when formatting arguments are supplied
// (Console.Write("abc") is a valid non-format call)
if (method.Parameters.Length <= 1)
return;

reportWhenNoFormattingArgs = false;
formatArgumentIndex = 0;
}
else if (stringBuilderType is not null && method.ContainingType.IsEqualTo(stringBuilderType) &&
method.Name == "AppendFormat")
{
// StringBuilder.AppendFormat - report even when called with no args
reportWhenNoFormattingArgs = true;
formatArgumentIndex = GetFormatArgIndex(operation, formatProviderType);
}
else
{
return;
}

if (operation.Arguments.Length == 0)
if (formatArgumentIndex < 0 || formatArgumentIndex >= operation.Arguments.Length)
return;

var formatProviderType = context.Compilation.GetTypeByMetadataName("System.IFormatProvider");
var formatArgument = operation.Arguments[formatArgumentIndex];

// Find the format string argument (either first or second parameter depending on overload)
IArgumentOperation? formatArgument = null;
var formatArgumentIndex = -1;
// Check if there are any formatting arguments after the format string
var hasFormattingArguments = HasFormattingArguments(operation, formatArgumentIndex);

if (operation.Arguments.Length > 0)
// Case 1: No formatting arguments at all
if (!hasFormattingArguments)
{
var firstArg = operation.Arguments[0];
if (firstArg.Parameter?.Type.IsEqualTo(formatProviderType) == true)
if (reportWhenNoFormattingArgs)
{
// First argument is IFormatProvider, so format string is the second argument
if (operation.Arguments.Length > 1)
{
formatArgument = operation.Arguments[1];
formatArgumentIndex = 1;
}
context.ReportDiagnostic(Rule, operation);
}
else

return;
}

// Case 2: Has formatting arguments but constant format string has no placeholders
if (formatArgument.Value.ConstantValue.HasValue && formatArgument.Value.ConstantValue.Value is string formatString)
{
if (!HasPlaceholders(formatString))
{
// First argument is the format string
formatArgument = firstArg;
formatArgumentIndex = 0;
context.ReportDiagnostic(Rule, operation);
}
}
}

if (formatArgument is null)
return;
private static int GetFormatArgIndex(IInvocationOperation operation, ITypeSymbol? formatProviderType)
{
if (operation.Arguments.Length > 0 && operation.Arguments[0].Parameter?.Type.IsEqualTo(formatProviderType) == true)
return 1;

// Check if there are any formatting arguments after the format string
var hasFormattingArguments = false;
return 0;
}

private static bool HasFormattingArguments(IInvocationOperation operation, int formatArgumentIndex)
{
for (var i = formatArgumentIndex + 1; i < operation.Arguments.Length; i++)
{
var arg = operation.Arguments[i];
Expand All @@ -82,10 +127,8 @@ private static void Analyze(OperationAnalysisContext context)
{
// Check if the array has any elements
if (arrayCreation.Initializer is not null && arrayCreation.Initializer.ElementValues.Length > 0)
{
hasFormattingArguments = true;
break;
}
return true;

// Skip empty params arrays
continue;
}
Expand All @@ -94,10 +137,7 @@ private static void Analyze(OperationAnalysisContext context)
if (arg.ArgumentKind is ArgumentKind.ParamCollection && arg.Value is ICollectionExpressionOperation collectionExpression)
{
if (collectionExpression.Elements.Length > 0)
{
hasFormattingArguments = true;
break;
}
return true;

continue;
}
Expand All @@ -107,25 +147,10 @@ private static void Analyze(OperationAnalysisContext context)
if (arg.IsImplicit)
continue;

hasFormattingArguments = true;
break;
}

// Case 1: No formatting arguments at all - report regardless of format string
if (!hasFormattingArguments)
{
context.ReportDiagnostic(Rule, operation);
return;
return true;
}

// Case 2: Has formatting arguments but constant format string has no placeholders
if (formatArgument.Value.ConstantValue.HasValue && formatArgument.Value.ConstantValue.Value is string formatString)
{
if (!HasPlaceholders(formatString))
{
context.ReportDiagnostic(Rule, operation);
}
}
return false;
}

private static bool HasPlaceholders(string formatString)
Expand Down
Loading