Skip to content

Commit cbc7f68

Browse files
silkfiresilkfire
committed
Add support to analyze implicit conversion when using constant values for [Params] and [Arguments] attribute values
1 parent c5b0e12 commit cbc7f68

14 files changed

+696
-191
lines changed

‎src/BenchmarkDotNet.Analyzers/AnalyzerHelper.cs‎

Lines changed: 79 additions & 22 deletions
Original file line numberDiff line numberDiff line change
@@ -5,6 +5,7 @@
55
using Microsoft.CodeAnalysis.CSharp.Syntax;
66

77
using System.Collections.Immutable;
8+
using System.Globalization;
89
using System.Linq;
910

1011
internal static class AnalyzerHelper
@@ -105,35 +106,68 @@ public static string NormalizeTypeName(INamedTypeSymbol namedTypeSymbol)
105106
return typeName;
106107
}
107108

108-
public static bool IsAssignableToField(Compilation compilation, ITypeSymbol targetType, string valueExpression)
109+
public static bool IsAssignableToField(Compilation compilation, ITypeSymbol targetType, string valueExpression, Optional<object?> constantValue, string? valueType)
109110
{
110-
var code = $$"""
111-
file static class Internal {
112-
static readonly {{targetType}} x = {{valueExpression}};
113-
}
114-
""";
111+
const string codeTemplate1 = """
112+
file static class Internal {{
113+
static readonly {0} x = {1};
114+
}}
115+
""";
116+
117+
const string codeTemplate2 = """
118+
file static class Internal {{
119+
static readonly {0} x = ({1}){2};
120+
}}
121+
""";
122+
123+
return IsAssignableTo(codeTemplate1, codeTemplate2, compilation, targetType, valueExpression, constantValue, valueType);
124+
}
115125

116-
var syntaxTree = CSharpSyntaxTree.ParseText(code);
126+
public static bool IsAssignableToLocal(Compilation compilation, ITypeSymbol targetType, string valueExpression, Optional<object?> constantValue, string? valueType)
127+
{
128+
const string codeTemplate1 = """
129+
file static class Internal {{
130+
static void Method() {{
131+
{0} x = {1};
132+
}}
133+
}}
134+
""";
135+
136+
const string codeTemplate2 = """
137+
file static class Internal {{
138+
static void Method() {{
139+
{0} x = ({1}){2};
140+
}}
141+
}}
142+
""";
143+
144+
return IsAssignableTo(codeTemplate1, codeTemplate2, compilation, targetType, valueExpression, constantValue, valueType);
145+
}
117146

118-
var compilationErrors = compilation.AddSyntaxTrees(syntaxTree)
119-
.GetSemanticModel(syntaxTree)
120-
.GetMethodBodyDiagnostics()
121-
.Where(d => d.DefaultSeverity == DiagnosticSeverity.Error)
122-
.ToList();
147+
private static bool IsAssignableTo(string codeTemplate1, string codeTemplate2, Compilation compilation, ITypeSymbol targetType, string valueExpression, Optional<object?> constantValue, string? valueType)
148+
{
149+
var hasNoCompilationErrors = HasNoCompilationErrors(string.Format(codeTemplate1, targetType, valueExpression), compilation);
150+
if (hasNoCompilationErrors)
151+
{
152+
return true;
153+
}
123154

124-
return compilationErrors.Count == 0;
155+
if (!constantValue.HasValue || valueType == null)
156+
{
157+
return false;
158+
}
159+
160+
var constantLiteral = FormatLiteral(constantValue.Value);
161+
if (constantLiteral == null)
162+
{
163+
return false;
164+
}
165+
166+
return HasNoCompilationErrors(string.Format(codeTemplate2, targetType, valueType, constantLiteral), compilation);
125167
}
126168

127-
public static bool IsAssignableToLocal(Compilation compilation, ITypeSymbol targetType, string valueExpression)
169+
private static bool HasNoCompilationErrors(string code, Compilation compilation)
128170
{
129-
var code = $$"""
130-
file static class Internal {
131-
static Internal() {
132-
{{targetType}} x = {{valueExpression}};
133-
}
134-
}
135-
""";
136-
137171
var syntaxTree = CSharpSyntaxTree.ParseText(code);
138172

139173
var compilationErrors = compilation.AddSyntaxTrees(syntaxTree)
@@ -144,5 +178,28 @@ static Internal() {
144178

145179
return compilationErrors.Count == 0;
146180
}
181+
182+
private static string? FormatLiteral(object? value)
183+
{
184+
return value switch
185+
{
186+
byte b => b.ToString(),
187+
sbyte sb => sb.ToString(),
188+
short s => s.ToString(),
189+
ushort us => us.ToString(),
190+
int i => i.ToString(),
191+
uint ui => $"{ui}U",
192+
long l => $"{l}L",
193+
ulong ul => $"{ul}UL",
194+
float f => $"{f.ToString(CultureInfo.InvariantCulture)}F",
195+
double d => $"{d.ToString(CultureInfo.InvariantCulture)}D",
196+
decimal m => $"{m.ToString(CultureInfo.InvariantCulture)}M",
197+
char c => $"'{c}'",
198+
bool b => b ? "true" : "false",
199+
string s => $"\"{s}\"",
200+
null => "null",
201+
_ => null
202+
};
203+
}
147204
}
148205
}

‎src/BenchmarkDotNet.Analyzers/AnalyzerReleases.Unshipped.md‎

Lines changed: 1 addition & 1 deletion
Original file line numberDiff line numberDiff line change
@@ -26,7 +26,7 @@ BDN1205 | Usage | Error | BDN1205_Attributes_GeneralParameterAttributes_N
2626
BDN1206 | Usage | Error | BDN1206_Attributes_GeneralParameterAttributes_PropertyCannotBeInitOnly
2727
BDN1207 | Usage | Error | BDN1207_Attributes_GeneralParameterAttributes_PropertyMustHavePublicSetter
2828
BDN1300 | Usage | Error | BDN1300_Attributes_ParamsAttribute_MustHaveValues
29-
BDN1301 | Usage | Error | BDN1301_Attributes_ParamsAttribute_UnexpectedValueType
29+
BDN1301 | Usage | Error | BDN1301_Attributes_ParamsAttribute_MustHaveMatchingValueType
3030
BDN1302 | Usage | Warning | BDN1302_Attributes_ParamsAttribute_UnnecessarySingleValuePassedToAttribute
3131
BDN1303 | Usage | Error | BDN1303_Attributes_ParamsAllValuesAttribute_NotAllowedOnFlagsEnumPropertyOrFieldType
3232
BDN1304 | Usage | Error | BDN1304_Attributes_ParamsAllValues_PropertyOrFieldTypeMustBeEnumOrBool

‎src/BenchmarkDotNet.Analyzers/Attributes/ArgumentsAttributeAnalyzer.cs‎

Lines changed: 24 additions & 7 deletions
Original file line numberDiff line numberDiff line change
@@ -89,7 +89,7 @@ private static void AnalyzeMethodDeclaration(SyntaxNodeAnalysisContext context)
8989
{
9090
if (hasBenchmarkAttribute && methodDeclarationSyntax.ParameterList.Parameters.Count > 0)
9191
{
92-
context.ReportDiagnostic(Diagnostic.Create(MethodWithoutAttributeMustHaveNoParametersRule, Location.Create(context.FilterTree, methodDeclarationSyntax.ParameterList.Parameters.Span)));
92+
context.ReportDiagnostic(Diagnostic.Create(MethodWithoutAttributeMustHaveNoParametersRule, Location.Create(context.FilterTree, methodDeclarationSyntax.ParameterList.Parameters.Span), methodDeclarationSyntax.Identifier.ToString()));
9393
}
9494

9595
return;
@@ -276,10 +276,16 @@ void ReportIfNotImplicitlyConvertibleValueTypeDiagnostic(Func<int, ExpressionSyn
276276

277277
var valueExpressionString = valueExpressionSyntax.ToString();
278278

279+
var constantValue = context.SemanticModel.GetConstantValue(valueExpressionSyntax);
280+
279281
var actualValueTypeSymbol = context.SemanticModel.GetTypeInfo(valueExpressionSyntax).Type;
280282
if (actualValueTypeSymbol != null && actualValueTypeSymbol.TypeKind != TypeKind.Error)
281283
{
282-
if (!AnalyzerHelper.IsAssignableToLocal(context.Compilation, methodParameterTypeSymbol, valueExpressionString))
284+
if (!AnalyzerHelper.IsAssignableToLocal(context.Compilation,
285+
methodParameterTypeSymbol,
286+
valueExpressionString,
287+
constantValue,
288+
actualValueTypeSymbol.ToString()))
283289
{
284290
ReportValueTypeMustBeImplicitlyConvertibleDiagnostic(valueExpressionSyntax.GetLocation(),
285291
valueExpressionSyntax.ToString(),
@@ -289,21 +295,32 @@ void ReportIfNotImplicitlyConvertibleValueTypeDiagnostic(Func<int, ExpressionSyn
289295
}
290296
else
291297
{
292-
ReportValueTypeMustBeImplicitlyConvertibleDiagnostic(valueExpressionSyntax.GetLocation(),
293-
valueExpressionSyntax.ToString(),
294-
methodParameterTypeSymbol.ToString());
298+
if (constantValue is { HasValue: true, Value: null })
299+
{
300+
if (!AnalyzerHelper.IsAssignableToLocal(context.Compilation,
301+
methodParameterTypeSymbol,
302+
valueExpressionString,
303+
constantValue,
304+
null))
305+
{
306+
ReportValueTypeMustBeImplicitlyConvertibleDiagnostic(valueExpressionSyntax.GetLocation(),
307+
valueExpressionString,
308+
methodParameterTypeSymbol.ToString(),
309+
"null");
310+
}
311+
}
295312
}
296313
}
297314

298315
return;
299316

300-
void ReportValueTypeMustBeImplicitlyConvertibleDiagnostic(Location diagnosticLocation, string value, string expectedType, string? actualType = null)
317+
void ReportValueTypeMustBeImplicitlyConvertibleDiagnostic(Location diagnosticLocation, string value, string expectedType, string actualType)
301318
{
302319
context.ReportDiagnostic(Diagnostic.Create(MustHaveMatchingValueTypeRule,
303320
diagnosticLocation,
304321
value,
305322
expectedType,
306-
actualType ?? "<unknown>"));
323+
actualType));
307324
}
308325
}
309326
}

‎src/BenchmarkDotNet.Analyzers/Attributes/GeneralParameterAttributesAnalyzer.cs‎

Lines changed: 28 additions & 24 deletions
Original file line numberDiff line numberDiff line change
@@ -75,16 +75,18 @@ public class GeneralParameterAttributesAnalyzer : DiagnosticAnalyzer
7575
isEnabledByDefault: true,
7676
description: AnalyzerHelper.GetResourceString(nameof(BenchmarkDotNetAnalyzerResources.Attributes_GeneralParameterAttributes_PropertyCannotBeInitOnly_Description)));
7777

78-
public override ImmutableArray<DiagnosticDescriptor> SupportedDiagnostics => ImmutableArray.Create(
79-
MutuallyExclusiveOnFieldRule,
80-
MutuallyExclusiveOnPropertyRule,
81-
FieldMustBePublic,
82-
PropertyMustBePublic,
83-
NotValidOnReadonlyFieldRule,
84-
NotValidOnConstantFieldRule,
85-
PropertyCannotBeInitOnlyRule,
86-
PropertyMustHavePublicSetterRule
87-
);
78+
public override ImmutableArray<DiagnosticDescriptor> SupportedDiagnostics =>
79+
[
80+
MutuallyExclusiveOnFieldRule,
81+
MutuallyExclusiveOnPropertyRule,
82+
FieldMustBePublic,
83+
PropertyMustBePublic,
84+
NotValidOnReadonlyFieldRule,
85+
NotValidOnConstantFieldRule,
86+
PropertyCannotBeInitOnlyRule,
87+
PropertyMustHavePublicSetterRule
88+
];
89+
8890
public override void Initialize(AnalysisContext analysisContext)
8991
{
9092
analysisContext.EnableConcurrentExecution();
@@ -106,7 +108,7 @@ public override void Initialize(AnalysisContext analysisContext)
106108

107109
private static void Analyze(SyntaxNodeAnalysisContext context)
108110
{
109-
if (!(context.Node is AttributeSyntax attributeSyntax))
111+
if (context.Node is not AttributeSyntax attributeSyntax)
110112
{
111113
return;
112114
}
@@ -118,14 +120,16 @@ private static void Analyze(SyntaxNodeAnalysisContext context)
118120

119121
var attributeSyntaxTypeSymbol = context.SemanticModel.GetTypeInfo(attributeSyntax).Type;
120122
if ( attributeSyntaxTypeSymbol == null
121-
|| !attributeSyntaxTypeSymbol.Equals(paramsAttributeTypeSymbol, SymbolEqualityComparer.Default)
122-
&& !attributeSyntaxTypeSymbol.Equals(paramsSourceAttributeTypeSymbol, SymbolEqualityComparer.Default)
123-
&& !attributeSyntaxTypeSymbol.Equals(paramsAllValuesAttributeTypeSymbol, SymbolEqualityComparer.Default))
123+
|| attributeSyntaxTypeSymbol.TypeKind == TypeKind.Error
124+
||
125+
(!attributeSyntaxTypeSymbol.Equals(paramsAttributeTypeSymbol, SymbolEqualityComparer.Default)
126+
&& !attributeSyntaxTypeSymbol.Equals(paramsSourceAttributeTypeSymbol, SymbolEqualityComparer.Default)
127+
&& !attributeSyntaxTypeSymbol.Equals(paramsAllValuesAttributeTypeSymbol, SymbolEqualityComparer.Default)))
124128
{
125129
return;
126130
}
127131

128-
var attributeTarget = attributeSyntax.FirstAncestorOrSelf<SyntaxNode>(n => n is FieldDeclarationSyntax || n is PropertyDeclarationSyntax);
132+
var attributeTarget = attributeSyntax.FirstAncestorOrSelf<SyntaxNode>(n => n is FieldDeclarationSyntax or PropertyDeclarationSyntax);
129133
if (attributeTarget == null)
130134
{
131135
return;
@@ -197,15 +201,15 @@ private static void Analyze(SyntaxNodeAnalysisContext context)
197201
}
198202

199203
private static void AnalyzeFieldOrPropertySymbol(SyntaxNodeAnalysisContext context,
200-
INamedTypeSymbol paramsAttributeTypeSymbol,
201-
INamedTypeSymbol paramsSourceAttributeTypeSymbol,
202-
INamedTypeSymbol paramsAllValuesAttributeTypeSymbol,
204+
INamedTypeSymbol? paramsAttributeTypeSymbol,
205+
INamedTypeSymbol? paramsSourceAttributeTypeSymbol,
206+
INamedTypeSymbol? paramsAllValuesAttributeTypeSymbol,
203207
ImmutableArray<AttributeSyntax> declaredAttributes,
204208
bool fieldOrPropertyIsPublic,
205-
Location fieldConstModifierLocation,
206-
Location fieldReadonlyModifierLocation,
209+
Location? fieldConstModifierLocation,
210+
Location? fieldReadonlyModifierLocation,
207211
string fieldOrPropertyIdentifier,
208-
Location propertyInitAccessorKeywordLocation,
212+
Location? propertyInitAccessorKeywordLocation,
209213
bool propertyIsMissingAssignableSetter,
210214
Location fieldOrPropertyIdentifierLocation,
211215
DiagnosticDescriptor fieldOrPropertyCannotHaveMoreThanOneParameterAttributeAppliedDiagnosticRule,
@@ -296,9 +300,9 @@ private static void AnalyzeFieldOrPropertySymbol(SyntaxNodeAnalysisContext conte
296300
}
297301

298302
private static bool AllAttributeTypeSymbolsExist(in SyntaxNodeAnalysisContext context,
299-
out INamedTypeSymbol paramsAttributeTypeSymbol,
300-
out INamedTypeSymbol paramsSourceAttributeTypeSymbol,
301-
out INamedTypeSymbol paramsAllValuesAttributeTypeSymbol)
303+
out INamedTypeSymbol? paramsAttributeTypeSymbol,
304+
out INamedTypeSymbol? paramsSourceAttributeTypeSymbol,
305+
out INamedTypeSymbol? paramsAllValuesAttributeTypeSymbol)
302306
{
303307
paramsAttributeTypeSymbol = context.Compilation.GetTypeByMetadataName("BenchmarkDotNet.Attributes.ParamsAttribute");
304308
if (paramsAttributeTypeSymbol == null)

0 commit comments

Comments
 (0)