WiseTechGlobal / WTG.Analyzers

Analyzers from WiseTech Global to enforce our styles, behaviours, and prevent common mistakes.
Other
16 stars 3 forks source link

EmitAnalyzer crashes in VS2022 / .NET 6 SDK #162

Closed yaakov-h closed 2 years ago

yaakov-h commented 2 years ago
Analyzer 'WTG.Analyzers.EmitAnalyzer' threw an exception of type 'System.ArgumentNullException' with message 'Value cannot be null.
Parameter name: syntax'.
Exception occurred with following context:
Compilation: WTG.Glow.Framework.Repository.EntityEmitter
SyntaxTree: C:\git\wtg\Glow\Glow\Framework\src\Repository\Common\Repository.EntityEmitter\EntityTypeEmitter.cs
SyntaxNode: generator.Emit(shortForm ? OpCodes ... [InvocationExpressionSyntax]@[22704..22771) (662,3)-(662,70)

System.ArgumentNullException: Value cannot be null.
Parameter name: syntax
   at Microsoft.CodeAnalysis.CSharp.CSharpSemanticModel.CheckSyntaxNode(CSharpSyntaxNode syntax)
   at Microsoft.CodeAnalysis.CSharp.CSharpSemanticModel.GetSymbolInfo(ExpressionSyntax expression, CancellationToken cancellationToken)
   at Microsoft.CodeAnalysis.CSharp.CSharpExtensions.GetSymbolInfo(SemanticModel semanticModel, ExpressionSyntax expression, CancellationToken cancellationToken)
   at WTG.Analyzers.EmitAnalyzer.TryGetOpCodeFieldFromEmitCall(SemanticModel model, InvocationExpressionSyntax emitCall, IFieldSymbol& opCodeSymbol, CancellationToken cancellation)
   at WTG.Analyzers.EmitAnalyzer.AnalyzeInvoke(SyntaxNodeAnalysisContext context)
   at Microsoft.CodeAnalysis.Diagnostics.AnalyzerExecutor.<>c__62`1.<ExecuteSyntaxNodeAction>b__62_0(ValueTuple`2 data)
   at Microsoft.CodeAnalysis.Diagnostics.AnalyzerExecutor.ExecuteAndCatchIfThrows_NoLock[TArg](DiagnosticAnalyzer analyzer, Action`1 analyze, TArg argument, Nullable`1 info)
brian-reichle commented 2 years ago

I can reproduce it with the following:

public void Method(ILGenerator g, bool shortForm, Label label) => g.Emit(shortForm ? OpCodes.Brtrue_S : OpCodes.Brtrue, label);

Seems were missing a null check here, for fieldIdentifier: https://github.com/WiseTechGlobal/WTG.Analyzers/blob/0c0e65d8d97a4f86dd5c1a0d0bb1950203172844/src/WTG.Analyzers/Analyzers/Emit/EmitAnalyzer.cs#L86-L87

On the one hand we could simply add that null check and effectively ignore any emit call where the opcode isn't provided by a simple reference to a static member on OpCodes. Or we could step into things like conditional expressions (and maybe switch expressions) to build up a list of possible opcodes, then enforce that the Emit overload is valid for all of them.