Skip to content

Commit 3f0deb5

Browse files
Copilotmeziantou
Copilot
and
meziantou
authored
MA0183: Extend detection to Console.Write, Console.WriteLine, and StringBuilder.AppendFormat (#1042)
* Initial plan * MA0183: Extend rule to detect Console.Write, Console.WriteLine, and StringBuilder.AppendFormat Co-authored-by: meziantou <509220+meziantou@users.noreply.github.com> --------- Co-authored-by: copilot-swe-agent[bot] <198982749+Copilot@users.noreply.github.com> Co-authored-by: meziantou <509220+meziantou@users.noreply.github.com>
1 parent 9d2958f commit 3f0deb5

File tree

7 files changed

+356
-61
lines changed

7 files changed

+356
-61
lines changed

‎README.md‎

Lines changed: 1 addition & 1 deletion
Original file line numberDiff line numberDiff line change
@@ -197,7 +197,7 @@ If you are already using other analyzers, you can check [which rules are duplica
197197
|[MA0180](https://github.com/meziantou/Meziantou.Analyzer/blob/main/docs/Rules/MA0180.md)|Design|ILogger type parameter should match containing type|⚠️|❌|✔️|
198198
|[MA0181](https://github.com/meziantou/Meziantou.Analyzer/blob/main/docs/Rules/MA0181.md)|Style|Do not use cast|ℹ️|❌|❌|
199199
|[MA0182](https://github.com/meziantou/Meziantou.Analyzer/blob/main/docs/Rules/MA0182.md)|Design|Avoid unused internal types|ℹ️|✔️|✔️|
200-
|[MA0183](https://github.com/meziantou/Meziantou.Analyzer/blob/main/docs/Rules/MA0183.md)|Usage|string.Format should use a format string with placeholders|⚠️|✔️|❌|
200+
|[MA0183](https://github.com/meziantou/Meziantou.Analyzer/blob/main/docs/Rules/MA0183.md)|Usage|The format string should use placeholders|⚠️|✔️|❌|
201201
|[MA0184](https://github.com/meziantou/Meziantou.Analyzer/blob/main/docs/Rules/MA0184.md)|Style|Do not use interpolated string without parameters|👻|✔️|✔️|
202202
|[MA0185](https://github.com/meziantou/Meziantou.Analyzer/blob/main/docs/Rules/MA0185.md)|Performance|Simplify string.Create when all parameters are culture invariant|ℹ️|✔️|✔️|
203203
|[MA0186](https://github.com/meziantou/Meziantou.Analyzer/blob/main/docs/Rules/MA0186.md)|Design|Equals method should use \[NotNullWhen(true)\] on the parameter|ℹ️|❌|❌|

‎docs/README.md‎

Lines changed: 3 additions & 3 deletions
Original file line numberDiff line numberDiff line change
@@ -181,7 +181,7 @@
181181
|[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>|❌|✔️|
182182
|[MA0181](https://github.com/meziantou/Meziantou.Analyzer/blob/main/docs/Rules/MA0181.md)|Style|Do not use cast|<span title='Info'>ℹ️</span>|❌|❌|
183183
|[MA0182](https://github.com/meziantou/Meziantou.Analyzer/blob/main/docs/Rules/MA0182.md)|Design|Avoid unused internal types|<span title='Info'>ℹ️</span>|✔️|✔️|
184-
|[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>|✔️|❌|
184+
|[MA0183](https://github.com/meziantou/Meziantou.Analyzer/blob/main/docs/Rules/MA0183.md)|Usage|The format string should use placeholders|<span title='Warning'>⚠️</span>|✔️|❌|
185185
|[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>|✔️|✔️|
186186
|[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>|✔️|✔️|
187187
|[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>|❌|❌|
@@ -744,7 +744,7 @@ dotnet_diagnostic.MA0181.severity = none
744744
# MA0182: Avoid unused internal types
745745
dotnet_diagnostic.MA0182.severity = suggestion
746746
747-
# MA0183: string.Format should use a format string with placeholders
747+
# MA0183: The format string should use placeholders
748748
dotnet_diagnostic.MA0183.severity = warning
749749
750750
# MA0184: Do not use interpolated string without parameters
@@ -1300,7 +1300,7 @@ dotnet_diagnostic.MA0181.severity = none
13001300
# MA0182: Avoid unused internal types
13011301
dotnet_diagnostic.MA0182.severity = none
13021302
1303-
# MA0183: string.Format should use a format string with placeholders
1303+
# MA0183: The format string should use placeholders
13041304
dotnet_diagnostic.MA0183.severity = none
13051305
13061306
# MA0184: Do not use interpolated string without parameters

‎docs/Rules/MA0183.md‎

Lines changed: 10 additions & 1 deletion
Original file line numberDiff line numberDiff line change
@@ -1,4 +1,13 @@
1-
# MA0183 - string.Format should use a format string with placeholders
1+
# MA0183 - The format string should use placeholders
22
<!-- sources -->
33
Source: [StringFormatShouldBeConstantAnalyzer.cs](https://github.com/meziantou/Meziantou.Analyzer/blob/main/src/Meziantou.Analyzer/Rules/StringFormatShouldBeConstantAnalyzer.cs)
44
<!-- sources -->
5+
6+
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).
7+
8+
The following methods are checked:
9+
10+
- `string.Format`
11+
- `Console.Write`
12+
- `Console.WriteLine`
13+
- `System.Text.StringBuilder.AppendFormat`

‎src/Meziantou.Analyzer.Pack/configuration/default.editorconfig‎

Lines changed: 1 addition & 1 deletion
Original file line numberDiff line numberDiff line change
@@ -542,7 +542,7 @@ dotnet_diagnostic.MA0181.severity = none
542542
# MA0182: Avoid unused internal types
543543
dotnet_diagnostic.MA0182.severity = suggestion
544544

545-
# MA0183: string.Format should use a format string with placeholders
545+
# MA0183: The format string should use placeholders
546546
dotnet_diagnostic.MA0183.severity = warning
547547

548548
# MA0184: Do not use interpolated string without parameters

‎src/Meziantou.Analyzer.Pack/configuration/none.editorconfig‎

Lines changed: 1 addition & 1 deletion
Original file line numberDiff line numberDiff line change
@@ -542,7 +542,7 @@ dotnet_diagnostic.MA0181.severity = none
542542
# MA0182: Avoid unused internal types
543543
dotnet_diagnostic.MA0182.severity = none
544544

545-
# MA0183: string.Format should use a format string with placeholders
545+
# MA0183: The format string should use placeholders
546546
dotnet_diagnostic.MA0183.severity = none
547547

548548
# MA0184: Do not use interpolated string without parameters

‎src/Meziantou.Analyzer/Rules/StringFormatShouldBeConstantAnalyzer.cs‎

Lines changed: 79 additions & 54 deletions
Original file line numberDiff line numberDiff line change
@@ -11,8 +11,8 @@ public sealed class StringFormatShouldBeConstantAnalyzer : DiagnosticAnalyzer
1111
{
1212
private static readonly DiagnosticDescriptor Rule = new(
1313
RuleIdentifiers.StringFormatShouldBeConstant,
14-
title: "string.Format should use a format string with placeholders",
15-
messageFormat: "Use string literal instead of string.Format when the format string has no placeholders",
14+
title: "The format string should use placeholders",
15+
messageFormat: "Use a string literal instead of a format method when the format string has no placeholders",
1616
RuleCategories.Usage,
1717
DiagnosticSeverity.Warning,
1818
isEnabledByDefault: true,
@@ -25,54 +25,99 @@ public override void Initialize(AnalysisContext context)
2525
{
2626
context.EnableConcurrentExecution();
2727
context.ConfigureGeneratedCodeAnalysis(GeneratedCodeAnalysisFlags.None);
28-
context.RegisterOperationAction(Analyze, OperationKind.Invocation);
28+
context.RegisterCompilationStartAction(compilationStartContext =>
29+
{
30+
var formatProviderType = compilationStartContext.Compilation.GetBestTypeByMetadataName("System.IFormatProvider");
31+
var consoleType = compilationStartContext.Compilation.GetBestTypeByMetadataName("System.Console");
32+
var stringBuilderType = compilationStartContext.Compilation.GetBestTypeByMetadataName("System.Text.StringBuilder");
33+
34+
compilationStartContext.RegisterOperationAction(
35+
context => Analyze(context, formatProviderType, consoleType, stringBuilderType),
36+
OperationKind.Invocation);
37+
});
2938
}
3039

31-
private static void Analyze(OperationAnalysisContext context)
40+
private static void Analyze(OperationAnalysisContext context, ITypeSymbol? formatProviderType, ITypeSymbol? consoleType, ITypeSymbol? stringBuilderType)
3241
{
3342
var operation = (IInvocationOperation)context.Operation;
43+
var method = operation.TargetMethod;
3444

35-
// Check if it's a string.Format call
36-
if (operation.TargetMethod.Name != "Format")
45+
if (operation.Arguments.Length == 0)
3746
return;
3847

39-
if (operation.TargetMethod.ContainingType.SpecialType != SpecialType.System_String)
48+
// Determine if this is a known format method, what the format arg index is,
49+
// and whether to report when there are no formatting arguments.
50+
int formatArgumentIndex;
51+
bool reportWhenNoFormattingArgs;
52+
53+
if (method.ContainingType.SpecialType == SpecialType.System_String && method.Name == "Format")
54+
{
55+
// string.Format - report even when called with no args (e.g. string.Format("abc"))
56+
reportWhenNoFormattingArgs = true;
57+
formatArgumentIndex = GetFormatArgIndex(operation, formatProviderType);
58+
}
59+
else if (consoleType is not null && method.ContainingType.IsEqualTo(consoleType) &&
60+
(method.Name == "Write" || method.Name == "WriteLine"))
61+
{
62+
// Console.Write / Console.WriteLine - only report when formatting arguments are supplied
63+
// (Console.Write("abc") is a valid non-format call)
64+
if (method.Parameters.Length <= 1)
65+
return;
66+
67+
reportWhenNoFormattingArgs = false;
68+
formatArgumentIndex = 0;
69+
}
70+
else if (stringBuilderType is not null && method.ContainingType.IsEqualTo(stringBuilderType) &&
71+
method.Name == "AppendFormat")
72+
{
73+
// StringBuilder.AppendFormat - report even when called with no args
74+
reportWhenNoFormattingArgs = true;
75+
formatArgumentIndex = GetFormatArgIndex(operation, formatProviderType);
76+
}
77+
else
78+
{
4079
return;
80+
}
4181

42-
if (operation.Arguments.Length == 0)
82+
if (formatArgumentIndex < 0 || formatArgumentIndex >= operation.Arguments.Length)
4383
return;
4484

45-
var formatProviderType = context.Compilation.GetTypeByMetadataName("System.IFormatProvider");
85+
var formatArgument = operation.Arguments[formatArgumentIndex];
4686

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

51-
if (operation.Arguments.Length > 0)
90+
// Case 1: No formatting arguments at all
91+
if (!hasFormattingArguments)
5292
{
53-
var firstArg = operation.Arguments[0];
54-
if (firstArg.Parameter?.Type.IsEqualTo(formatProviderType) == true)
93+
if (reportWhenNoFormattingArgs)
5594
{
56-
// First argument is IFormatProvider, so format string is the second argument
57-
if (operation.Arguments.Length > 1)
58-
{
59-
formatArgument = operation.Arguments[1];
60-
formatArgumentIndex = 1;
61-
}
95+
context.ReportDiagnostic(Rule, operation);
6296
}
63-
else
97+
98+
return;
99+
}
100+
101+
// Case 2: Has formatting arguments but constant format string has no placeholders
102+
if (formatArgument.Value.ConstantValue.HasValue && formatArgument.Value.ConstantValue.Value is string formatString)
103+
{
104+
if (!HasPlaceholders(formatString))
64105
{
65-
// First argument is the format string
66-
formatArgument = firstArg;
67-
formatArgumentIndex = 0;
106+
context.ReportDiagnostic(Rule, operation);
68107
}
69108
}
109+
}
70110

71-
if (formatArgument is null)
72-
return;
111+
private static int GetFormatArgIndex(IInvocationOperation operation, ITypeSymbol? formatProviderType)
112+
{
113+
if (operation.Arguments.Length > 0 && operation.Arguments[0].Parameter?.Type.IsEqualTo(formatProviderType) == true)
114+
return 1;
73115

74-
// Check if there are any formatting arguments after the format string
75-
var hasFormattingArguments = false;
116+
return 0;
117+
}
118+
119+
private static bool HasFormattingArguments(IInvocationOperation operation, int formatArgumentIndex)
120+
{
76121
for (var i = formatArgumentIndex + 1; i < operation.Arguments.Length; i++)
77122
{
78123
var arg = operation.Arguments[i];
@@ -82,10 +127,8 @@ private static void Analyze(OperationAnalysisContext context)
82127
{
83128
// Check if the array has any elements
84129
if (arrayCreation.Initializer is not null && arrayCreation.Initializer.ElementValues.Length > 0)
85-
{
86-
hasFormattingArguments = true;
87-
break;
88-
}
130+
return true;
131+
89132
// Skip empty params arrays
90133
continue;
91134
}
@@ -94,10 +137,7 @@ private static void Analyze(OperationAnalysisContext context)
94137
if (arg.ArgumentKind is ArgumentKind.ParamCollection && arg.Value is ICollectionExpressionOperation collectionExpression)
95138
{
96139
if (collectionExpression.Elements.Length > 0)
97-
{
98-
hasFormattingArguments = true;
99-
break;
100-
}
140+
return true;
101141

102142
continue;
103143
}
@@ -107,25 +147,10 @@ private static void Analyze(OperationAnalysisContext context)
107147
if (arg.IsImplicit)
108148
continue;
109149

110-
hasFormattingArguments = true;
111-
break;
112-
}
113-
114-
// Case 1: No formatting arguments at all - report regardless of format string
115-
if (!hasFormattingArguments)
116-
{
117-
context.ReportDiagnostic(Rule, operation);
118-
return;
150+
return true;
119151
}
120152

121-
// Case 2: Has formatting arguments but constant format string has no placeholders
122-
if (formatArgument.Value.ConstantValue.HasValue && formatArgument.Value.ConstantValue.Value is string formatString)
123-
{
124-
if (!HasPlaceholders(formatString))
125-
{
126-
context.ReportDiagnostic(Rule, operation);
127-
}
128-
}
153+
return false;
129154
}
130155

131156
private static bool HasPlaceholders(string formatString)

0 commit comments

Comments
 (0)