From db211093526678b3140ce4e9d12bd144c22a9af0 Mon Sep 17 00:00:00 2001 From: Sebastien Lebreton Date: Wed, 29 Jun 2022 10:27:07 +0200 Subject: [PATCH] Add UNT0029, Pattern matching with null on Unity objects (#229) * Add UNT0029, Pattern matching with null on Unity objects * Use matching instead * Fix all nullability warnings after Roslyn 3.7.0 bump (#230) --- doc/UNT0029.md | 39 ++++++++ doc/index.md | 1 + .../UnityObjectNullHandlingTests.cs | 97 ++++++++++++++++++- .../ContextMenuSuppressor.cs | 13 ++- .../CreateInstance.cs | 2 +- .../EmptyUnityMessage.cs | 8 +- .../ImplicitUsageAttributeSuppressor.cs | 8 +- .../ImproperMenuItemMethod.cs | 4 +- .../ImproperSerializeField.cs | 11 ++- .../IndirectionMessage.cs | 2 +- .../InitializeOnLoadStaticCtor.cs | 6 +- src/Microsoft.Unity.Analyzers/InputGetKey.cs | 2 +- .../LoadAttributeMethod.cs | 4 +- .../LoadAttributeMethodSuppressor.cs | 6 +- .../MessageSignature.cs | 11 ++- .../MessageSuppressor.cs | 6 +- .../MethodInvocation.cs | 15 ++- .../Microsoft.Unity.Analyzers.csproj | 2 +- .../NonGenericGetComponent.cs | 22 ++++- .../NullableReferenceTypesSuppressor.cs | 15 +-- .../PhysicsAllocMethodUsage.cs | 1 - .../PropertyDrawerOnGUI.cs | 7 +- .../ProtectedUnityMessage.cs | 8 +- src/Microsoft.Unity.Analyzers/Reflection.cs | 5 + .../Resources/Strings.Designer.cs | 36 +++++++ .../Resources/Strings.resx | 12 +++ .../SerializeFieldSuppressor.cs | 8 +- .../SetPositionAndRotation.cs | 5 + .../SuppressionAnalysisContextExtensions.cs | 2 +- .../TagComparison.cs | 7 +- .../ThrowExpressionSuppressor.cs | 6 +- .../TryGetComponent.cs | 5 +- .../UnityObjectNullHandling.cs | 64 ++++++++++-- .../UnusedCoroutineReturnValue.cs | 2 +- .../UnusedMethodSuppressor.cs | 9 +- .../UpdateDeltaTime.cs | 7 +- src/Microsoft.Unity.Analyzers/VectorMath.cs | 8 +- 37 files changed, 399 insertions(+), 67 deletions(-) create mode 100644 doc/UNT0029.md diff --git a/doc/UNT0029.md b/doc/UNT0029.md new file mode 100644 index 00000000..5ff4d59a --- /dev/null +++ b/doc/UNT0029.md @@ -0,0 +1,39 @@ +# UNT0029 Pattern matching with null on Unity objects + +Unity overrides the null comparison operator for Unity objects, which is incompatible with pattern matching with null. + +## Examples of patterns that are flagged by this analyzer + +```csharp +using UnityEngine; + +class Camera : MonoBehaviour +{ + public Transform a = null; + + public void Update() + { + if (a is not null) { } + } +} +``` + +## Solution + +Use null comparison: + +```csharp +using UnityEngine; + +class Camera : MonoBehaviour +{ + public Transform a = null; + + public void Update() + { + if (a != null) { } + } +} +``` + +A code fix is offered for this diagnostic to automatically apply this change. diff --git a/doc/index.md b/doc/index.md index 748d97c1..ff64b880 100644 --- a/doc/index.md +++ b/doc/index.md @@ -30,6 +30,7 @@ ID | Title | Category [UNT0026](UNT0026.md) | GetComponent always allocates | Performance [UNT0027](UNT0027.md) | Do not call PropertyDrawer.OnGUI() | Correctness [UNT0028](UNT0028.md) | Use non-allocating physics APIs | Performance +[UNT0029](UNT0029.md) | Pattern matching with null on Unity objects | Correctness # Diagnostic Suppressors diff --git a/src/Microsoft.Unity.Analyzers.Tests/UnityObjectNullHandlingTests.cs b/src/Microsoft.Unity.Analyzers.Tests/UnityObjectNullHandlingTests.cs index 6c8ba5ae..14b34d1b 100644 --- a/src/Microsoft.Unity.Analyzers.Tests/UnityObjectNullHandlingTests.cs +++ b/src/Microsoft.Unity.Analyzers.Tests/UnityObjectNullHandlingTests.cs @@ -344,7 +344,6 @@ public Transform NP() await VerifyCSharpFixAsync(test, fixedTest); } - [Fact] public async Task CoalescingAssignmentForRegularObjects() { @@ -361,6 +360,102 @@ public System.Object NP() return a ??= b; } } +"; + + await VerifyCSharpDiagnosticAsync(test); + } + + [Fact] + public async Task IsNullPatternExpression() + { + const string test = @" +using UnityEngine; + +class Camera : MonoBehaviour +{ + public Transform a = null; + + public void Update() + { + if (a is null) { } + } +} +"; + + var diagnostic = ExpectDiagnostic(UnityObjectNullHandlingAnalyzer.IsPatternRule) + .WithLocation(10, 13); + + await VerifyCSharpDiagnosticAsync(test, diagnostic); + + const string fixedTest = @" +using UnityEngine; + +class Camera : MonoBehaviour +{ + public Transform a = null; + + public void Update() + { + if (a == null) { } + } +} +"; + await VerifyCSharpFixAsync(test, fixedTest); + } + + [Fact] + public async Task IsNotNullPatternExpression() + { + const string test = @" +using UnityEngine; + +class Camera : MonoBehaviour +{ + public Transform a = null; + + public void Update() + { + if (a is not null) { } + } +} +"; + + var diagnostic = ExpectDiagnostic(UnityObjectNullHandlingAnalyzer.IsPatternRule) + .WithLocation(10, 13); + + await VerifyCSharpDiagnosticAsync(test, diagnostic); + + const string fixedTest = @" +using UnityEngine; + +class Camera : MonoBehaviour +{ + public Transform a = null; + + public void Update() + { + if (a != null) { } + } +} +"; + await VerifyCSharpFixAsync(test, fixedTest); + } + + [Fact] + public async Task IsRegularPatternExpression() + { + const string test = @" +using UnityEngine; + +class Camera : MonoBehaviour +{ + public object a = null; + + public void Update() + { + if (a is not null) { } + } +} "; await VerifyCSharpDiagnosticAsync(test); diff --git a/src/Microsoft.Unity.Analyzers/ContextMenuSuppressor.cs b/src/Microsoft.Unity.Analyzers/ContextMenuSuppressor.cs index 96770c6e..ad9b9ad2 100644 --- a/src/Microsoft.Unity.Analyzers/ContextMenuSuppressor.cs +++ b/src/Microsoft.Unity.Analyzers/ContextMenuSuppressor.cs @@ -43,8 +43,11 @@ public override void ReportSuppressions(SuppressionAnalysisContext context) private void AnalyzeDiagnostic(Diagnostic diagnostic, SuppressionAnalysisContext context) { - var location = diagnostic.Location; - var model = context.GetSemanticModel(location.SourceTree); + var syntaxTree = diagnostic.Location.SourceTree; + if (syntaxTree == null) + return; + + var model = context.GetSemanticModel(syntaxTree); var node = context.GetSuppressibleNode(diagnostic, n => n is MethodDeclarationSyntax or VariableDeclaratorSyntax); switch (node) @@ -69,13 +72,13 @@ private static bool IsReportable(ISymbol symbol) switch (symbol) { case IMethodSymbol methodSymbol: - if (methodSymbol.GetAttributes().Any(a => a.AttributeClass.Matches(typeof(UnityEngine.ContextMenu)) || a.AttributeClass.Matches(typeof(UnityEditor.MenuItem)))) + if (methodSymbol.GetAttributes().Any(a => a.AttributeClass != null && (a.AttributeClass.Matches(typeof(UnityEngine.ContextMenu)) || a.AttributeClass.Matches(typeof(UnityEditor.MenuItem))))) return true; if (IsReferencedByContextMenuItem(methodSymbol, containingType)) return true; break; case IFieldSymbol fieldSymbol: - if (fieldSymbol.GetAttributes().Any(a => a.AttributeClass.Matches(typeof(UnityEngine.ContextMenuItemAttribute)))) + if (fieldSymbol.GetAttributes().Any(a => a.AttributeClass != null && a.AttributeClass.Matches(typeof(UnityEngine.ContextMenuItemAttribute)))) return true; break; } @@ -92,7 +95,7 @@ private static bool IsReferencedByContextMenuItem(IMethodSymbol symbol, INamedTy var attributes = fieldSymbol .GetAttributes() - .Where(a => a.AttributeClass.Matches(typeof(UnityEngine.ContextMenuItemAttribute))) + .Where(a => a.AttributeClass != null && a.AttributeClass.Matches(typeof(UnityEngine.ContextMenuItemAttribute))) .ToArray(); if (!attributes.Any()) diff --git a/src/Microsoft.Unity.Analyzers/CreateInstance.cs b/src/Microsoft.Unity.Analyzers/CreateInstance.cs index 643ee253..ecaf3ea1 100644 --- a/src/Microsoft.Unity.Analyzers/CreateInstance.cs +++ b/src/Microsoft.Unity.Analyzers/CreateInstance.cs @@ -152,7 +152,7 @@ private static async Task ReplaceWithInvocationAsync(Document document SingletonSeparatedList( IdentifierName(typeInfo.Type.Name)))))); - var newRoot = root.ReplaceNode(creation, invocation); + var newRoot = root?.ReplaceNode(creation, invocation); if (newRoot == null) return document; diff --git a/src/Microsoft.Unity.Analyzers/EmptyUnityMessage.cs b/src/Microsoft.Unity.Analyzers/EmptyUnityMessage.cs index 387217da..dc9366e4 100644 --- a/src/Microsoft.Unity.Analyzers/EmptyUnityMessage.cs +++ b/src/Microsoft.Unity.Analyzers/EmptyUnityMessage.cs @@ -54,11 +54,17 @@ private static void AnalyzeMethodDeclaration(SyntaxNodeAnalysisContext context) return; var typeSymbol = context.SemanticModel.GetDeclaredSymbol(classDeclaration); + if (typeSymbol == null) + return; + var scriptInfo = new ScriptInfo(typeSymbol); if (!scriptInfo.HasMessages) return; var symbol = context.SemanticModel.GetDeclaredSymbol(method); + if (symbol == null) + return; + if (!scriptInfo.IsMessage(symbol)) return; @@ -106,7 +112,7 @@ public sealed override async Task RegisterCodeFixesAsync(CodeFixContext context) private static async Task DeleteEmptyMessageAsync(Document document, MethodDeclarationSyntax declaration, CancellationToken cancellationToken) { var root = await document.GetSyntaxRootAsync(cancellationToken).ConfigureAwait(false); - var newRoot = root.RemoveNode(declaration, SyntaxRemoveOptions.KeepNoTrivia); + var newRoot = root?.RemoveNode(declaration, SyntaxRemoveOptions.KeepNoTrivia); if (newRoot == null) return document; diff --git a/src/Microsoft.Unity.Analyzers/ImplicitUsageAttributeSuppressor.cs b/src/Microsoft.Unity.Analyzers/ImplicitUsageAttributeSuppressor.cs index 934da7df..8bccd6e5 100644 --- a/src/Microsoft.Unity.Analyzers/ImplicitUsageAttributeSuppressor.cs +++ b/src/Microsoft.Unity.Analyzers/ImplicitUsageAttributeSuppressor.cs @@ -36,7 +36,11 @@ private void AnalyzeDiagnostic(Diagnostic diagnostic, SuppressionAnalysisContext if (methodDeclarationSyntax == null) return; - var model = context.GetSemanticModel(diagnostic.Location.SourceTree); + var syntaxTree = diagnostic.Location.SourceTree; + if (syntaxTree == null) + return; + + var model = context.GetSemanticModel(syntaxTree); if (model.GetDeclaredSymbol(methodDeclarationSyntax) is not IMethodSymbol methodSymbol) return; @@ -49,6 +53,6 @@ private void AnalyzeDiagnostic(Diagnostic diagnostic, SuppressionAnalysisContext private bool IsSuppressable(IMethodSymbol methodSymbol) { // The Unity code stripper will consider any attribute with the exact name "PreserveAttribute", regardless of the namespace or assembly - return methodSymbol.GetAttributes().Any(a => a.AttributeClass.Matches(typeof(JetBrains.Annotations.UsedImplicitlyAttribute)) || a.AttributeClass.Name == nameof(UnityEngine.Scripting.PreserveAttribute)); + return methodSymbol.GetAttributes().Any(a => a.AttributeClass != null && (a.AttributeClass.Matches(typeof(JetBrains.Annotations.UsedImplicitlyAttribute)) || a.AttributeClass.Name == nameof(UnityEngine.Scripting.PreserveAttribute))); } } diff --git a/src/Microsoft.Unity.Analyzers/ImproperMenuItemMethod.cs b/src/Microsoft.Unity.Analyzers/ImproperMenuItemMethod.cs index 37f2995b..fe948a69 100644 --- a/src/Microsoft.Unity.Analyzers/ImproperMenuItemMethod.cs +++ b/src/Microsoft.Unity.Analyzers/ImproperMenuItemMethod.cs @@ -46,7 +46,7 @@ private static void AnalyzeMethodDeclaration(SyntaxNodeAnalysisContext context) if (context.SemanticModel.GetDeclaredSymbol(method) is not { } methodSymbol) return; - if (!methodSymbol.GetAttributes().Any(a => a.AttributeClass.Matches(typeof(UnityEditor.MenuItem)))) + if (!methodSymbol.GetAttributes().Any(a => a.AttributeClass != null && a.AttributeClass.Matches(typeof(UnityEditor.MenuItem)))) return; if (methodSymbol.IsStatic) @@ -89,7 +89,7 @@ private static async Task AddStaticDeclarationAsync(Document document, newMethodDeclaration = newMethodDeclaration.AddModifiers(SyntaxFactory.Token(SyntaxKind.StaticKeyword)); } - var newRoot = root.ReplaceNode(methodDeclaration, newMethodDeclaration); + var newRoot = root?.ReplaceNode(methodDeclaration, newMethodDeclaration); if (newRoot == null) return document; diff --git a/src/Microsoft.Unity.Analyzers/ImproperSerializeField.cs b/src/Microsoft.Unity.Analyzers/ImproperSerializeField.cs index 86bfa07d..14124f71 100644 --- a/src/Microsoft.Unity.Analyzers/ImproperSerializeField.cs +++ b/src/Microsoft.Unity.Analyzers/ImproperSerializeField.cs @@ -38,7 +38,7 @@ public override void Initialize(AnalysisContext context) private static void AnalyzeMemberDeclaration(SyntaxNodeAnalysisContext context) { var model = context.SemanticModel; - ISymbol symbol; + ISymbol? symbol; switch (context.Node) { @@ -58,6 +58,9 @@ private static void AnalyzeMemberDeclaration(SyntaxNodeAnalysisContext context) return; } + if (symbol == null) + return; + if (!IsReportable(symbol)) return; @@ -69,7 +72,7 @@ private static bool IsReportable(ISymbol symbol) if (!symbol.ContainingType.Extends(typeof(UnityEngine.Object))) return false; - if (!symbol.GetAttributes().Any(a => a.AttributeClass.Matches(typeof(UnityEngine.SerializeField)))) + if (!symbol.GetAttributes().Any(a => a.AttributeClass != null && a.AttributeClass.Matches(typeof(UnityEngine.SerializeField)))) return false; return symbol switch @@ -126,7 +129,7 @@ private static async Task RemoveSerializeFieldAttributeAsync(Document if (nodes.Any()) { var newAttributes = attributeList.RemoveNodes(nodes, SyntaxRemoveOptions.KeepNoTrivia); - if (newAttributes.Attributes.Any()) + if (newAttributes != null && newAttributes.Attributes.Any()) attributes = attributes.Add(newAttributes); } else @@ -137,7 +140,7 @@ private static async Task RemoveSerializeFieldAttributeAsync(Document .WithAttributeLists(attributes) .WithLeadingTrivia(declaration.GetLeadingTrivia()); - var newRoot = root.ReplaceNode(declaration, newDeclaration); + var newRoot = root?.ReplaceNode(declaration, newDeclaration); if (newRoot == null) return document; diff --git a/src/Microsoft.Unity.Analyzers/IndirectionMessage.cs b/src/Microsoft.Unity.Analyzers/IndirectionMessage.cs index 380814c9..91864b0f 100644 --- a/src/Microsoft.Unity.Analyzers/IndirectionMessage.cs +++ b/src/Microsoft.Unity.Analyzers/IndirectionMessage.cs @@ -87,7 +87,7 @@ private static async Task DeleteIndirectionAsync(Document document, Me { var root = await document.GetSyntaxRootAsync(cancellationToken).ConfigureAwait(false); var newExpression = access.Expression; - var newRoot = root.ReplaceNode(access, newExpression); + var newRoot = root?.ReplaceNode(access, newExpression); if (newRoot == null) return document; diff --git a/src/Microsoft.Unity.Analyzers/InitializeOnLoadStaticCtor.cs b/src/Microsoft.Unity.Analyzers/InitializeOnLoadStaticCtor.cs index 6a023f4d..ce966a88 100644 --- a/src/Microsoft.Unity.Analyzers/InitializeOnLoadStaticCtor.cs +++ b/src/Microsoft.Unity.Analyzers/InitializeOnLoadStaticCtor.cs @@ -44,10 +44,12 @@ private static void AnalyzeClassDeclaration(SyntaxNodeAnalysisContext context) return; var typeSymbol = context.SemanticModel.GetDeclaredSymbol(classDeclaration); + if (typeSymbol == null) + return; var isInitOnLoad = typeSymbol .GetAttributes() - .Any(a => a.AttributeClass.Matches(typeof(UnityEditor.InitializeOnLoadAttribute))); + .Any(a => a.AttributeClass != null && a.AttributeClass.Matches(typeof(UnityEditor.InitializeOnLoadAttribute))); if (!isInitOnLoad) return; @@ -94,7 +96,7 @@ private static async Task CreateStaticCtorAsync(Document document, Cla var newClassDeclaration = classDeclaration .WithMembers(classDeclaration.Members.Insert(0, emptyStaticConstructor)); - var newRoot = root.ReplaceNode(classDeclaration, newClassDeclaration); + var newRoot = root?.ReplaceNode(classDeclaration, newClassDeclaration); if (newRoot == null) return document; diff --git a/src/Microsoft.Unity.Analyzers/InputGetKey.cs b/src/Microsoft.Unity.Analyzers/InputGetKey.cs index 066ebada..3c9b9eb1 100644 --- a/src/Microsoft.Unity.Analyzers/InputGetKey.cs +++ b/src/Microsoft.Unity.Analyzers/InputGetKey.cs @@ -162,7 +162,7 @@ private async Task UseKeyCodeMemberAsArgumentAsync(Document document, .ReplaceNode(argument, GetKeyCodeArgument(les) .WithTrailingTrivia(argument.GetTrailingTrivia()))); - var newRoot = root.ReplaceNode(invocation, newInvocation); + var newRoot = root?.ReplaceNode(invocation, newInvocation); if (newRoot == null) return document; diff --git a/src/Microsoft.Unity.Analyzers/LoadAttributeMethod.cs b/src/Microsoft.Unity.Analyzers/LoadAttributeMethod.cs index ffce294e..3b29ce4d 100644 --- a/src/Microsoft.Unity.Analyzers/LoadAttributeMethod.cs +++ b/src/Microsoft.Unity.Analyzers/LoadAttributeMethod.cs @@ -63,7 +63,7 @@ private static bool IsDecorated(ISymbol symbol) { return symbol .GetAttributes() - .Any(a => IsLoadAttributeType(a.AttributeClass)); + .Any(a => a.AttributeClass != null && IsLoadAttributeType(a.AttributeClass)); } private static bool IsLoadAttributeType(ITypeSymbol type) @@ -124,7 +124,7 @@ private static async Task FixMethodAsync(Document document, MethodDecl .AddModifiers(SyntaxFactory.Token(SyntaxKind.StaticKeyword)); } - var newRoot = root.ReplaceNode(methodDeclaration, newMethodDeclaration); + var newRoot = root?.ReplaceNode(methodDeclaration, newMethodDeclaration); if (newRoot == null) return document; diff --git a/src/Microsoft.Unity.Analyzers/LoadAttributeMethodSuppressor.cs b/src/Microsoft.Unity.Analyzers/LoadAttributeMethodSuppressor.cs index df87107b..1d5e04d9 100644 --- a/src/Microsoft.Unity.Analyzers/LoadAttributeMethodSuppressor.cs +++ b/src/Microsoft.Unity.Analyzers/LoadAttributeMethodSuppressor.cs @@ -29,7 +29,11 @@ public override void ReportSuppressions(SuppressionAnalysisContext context) private static void AnalyzeDiagnostic(Diagnostic diagnostic, SuppressionAnalysisContext context) { - var model = context.GetSemanticModel(diagnostic.Location.SourceTree); + var syntaxTree = diagnostic.Location.SourceTree; + if (syntaxTree == null) + return; + + var model = context.GetSemanticModel(syntaxTree); var methodDeclarationSyntax = context.GetSuppressibleNode(diagnostic); // Reuse the same detection logic regarding decorated methods with *InitializeOnLoadMethodAttribute or DidReloadScripts diff --git a/src/Microsoft.Unity.Analyzers/MessageSignature.cs b/src/Microsoft.Unity.Analyzers/MessageSignature.cs index 054614ee..ac21b129 100644 --- a/src/Microsoft.Unity.Analyzers/MessageSignature.cs +++ b/src/Microsoft.Unity.Analyzers/MessageSignature.cs @@ -46,6 +46,9 @@ private static void AnalyzeClassDeclaration(SyntaxNodeAnalysisContext context) return; var typeSymbol = context.SemanticModel.GetDeclaredSymbol(classDeclaration); + if (typeSymbol == null) + return; + var scriptInfo = new ScriptInfo(typeSymbol); if (!scriptInfo.HasMessages) return; @@ -68,6 +71,9 @@ private static void AnalyzeClassDeclaration(SyntaxNodeAnalysisContext context) continue; var methodSymbol = context.SemanticModel.GetDeclaredSymbol(method); + if (methodSymbol == null) + continue; + // A message is detected, so the signature is correct if (scriptInfo.IsMessage(methodSymbol)) continue; @@ -114,6 +120,9 @@ private static async Task FixMethodDeclarationSignatureAsync(Document .ConfigureAwait(false); var methodSymbol = semanticModel.GetDeclaredSymbol(methodDeclaration); + if (methodSymbol == null) + return document; + var typeSymbol = methodSymbol.ContainingType; var scriptInfo = new ScriptInfo(typeSymbol); @@ -132,7 +141,7 @@ private static async Task FixMethodDeclarationSignatureAsync(Document var newMethodDeclaration = methodDeclaration .WithParameterList(CreateParameterList(builder, message)); - var newRoot = root.ReplaceNode(methodDeclaration, newMethodDeclaration); + var newRoot = root?.ReplaceNode(methodDeclaration, newMethodDeclaration); if (newRoot == null) return document; diff --git a/src/Microsoft.Unity.Analyzers/MessageSuppressor.cs b/src/Microsoft.Unity.Analyzers/MessageSuppressor.cs index 36831454..6366281c 100644 --- a/src/Microsoft.Unity.Analyzers/MessageSuppressor.cs +++ b/src/Microsoft.Unity.Analyzers/MessageSuppressor.cs @@ -61,7 +61,11 @@ private void AnalyzeDiagnostic(Diagnostic diagnostic, SuppressionAnalysisContext if (node == null) return; - var model = context.GetSemanticModel(diagnostic.Location.SourceTree); + var syntaxTree = diagnostic.Location.SourceTree; + if (syntaxTree == null) + return; + + var model = context.GetSemanticModel(syntaxTree); if (model.GetDeclaredSymbol(node) is not IMethodSymbol methodSymbol) return; diff --git a/src/Microsoft.Unity.Analyzers/MethodInvocation.cs b/src/Microsoft.Unity.Analyzers/MethodInvocation.cs index debe0497..7f96abca 100644 --- a/src/Microsoft.Unity.Analyzers/MethodInvocation.cs +++ b/src/Microsoft.Unity.Analyzers/MethodInvocation.cs @@ -87,7 +87,7 @@ private static void AnalyzeInvocation(SyntaxNodeAnalysisContext context) if (context.Node is not InvocationExpressionSyntax invocation) return; - var options = invocation.SyntaxTree?.Options as CSharpParseOptions; + var options = invocation.SyntaxTree.Options as CSharpParseOptions; if (options == null || options.LanguageVersion < LanguageVersion.CSharp6) // we want nameof support return; @@ -140,11 +140,20 @@ protected virtual async Task IsRegistrableAsync(CodeFixContext context, In { // for now we do not offer codefixes for mixed types var model = await context.Document.GetSemanticModelAsync(context.CancellationToken).ConfigureAwait(false); + if (model == null) + return false; if (invocation.Expression is not MemberAccessExpressionSyntax maes) return true; - if (model.GetTypeInfo(maes.ChildNodes().FirstOrDefault()).Type is not INamedTypeSymbol typeInvocationContext) + var node = maes + .ChildNodes() + .FirstOrDefault(); + + if (node == null) + return false; + + if (model.GetTypeInfo(node).Type is not INamedTypeSymbol typeInvocationContext) return false; var mdec = invocation @@ -178,7 +187,7 @@ private async Task ChangeArgumentAsync(Document document, InvocationEx .ReplaceNode(argument, GetArgument(name) .WithTrailingTrivia(argument.GetTrailingTrivia()))); - var newRoot = root.ReplaceNode(invocation, newInvocation); + var newRoot = root?.ReplaceNode(invocation, newInvocation); if (newRoot == null) return document; diff --git a/src/Microsoft.Unity.Analyzers/Microsoft.Unity.Analyzers.csproj b/src/Microsoft.Unity.Analyzers/Microsoft.Unity.Analyzers.csproj index 6f5f9632..75b3c4af 100644 --- a/src/Microsoft.Unity.Analyzers/Microsoft.Unity.Analyzers.csproj +++ b/src/Microsoft.Unity.Analyzers/Microsoft.Unity.Analyzers.csproj @@ -8,7 +8,7 @@ - + diff --git a/src/Microsoft.Unity.Analyzers/NonGenericGetComponent.cs b/src/Microsoft.Unity.Analyzers/NonGenericGetComponent.cs index ca994e9e..704b72b8 100644 --- a/src/Microsoft.Unity.Analyzers/NonGenericGetComponent.cs +++ b/src/Microsoft.Unity.Analyzers/NonGenericGetComponent.cs @@ -110,24 +110,36 @@ private static async Task UseGenericGetComponentAsync(Document documen { var root = await document.GetSyntaxRootAsync(cancellationToken).ConfigureAwait(false); - var typeOf = (TypeOfExpressionSyntax)invocation.ArgumentList.Arguments[0].Expression; + var invocationArgumentList = invocation.ArgumentList; + var syntaxList = invocationArgumentList.Arguments; + + var argumentSyntax = syntaxList.FirstOrDefault(); + if (argumentSyntax == null) + return document; + + var typeOf = (TypeOfExpressionSyntax)argumentSyntax.Expression; var identifierSyntax = (IdentifierNameSyntax)invocation.Expression; + var newArgumentList = invocationArgumentList.RemoveNode(argumentSyntax, SyntaxRemoveOptions.KeepNoTrivia); + if (newArgumentList == null) + return document; + var newInvocation = invocation .WithExpression(GenericName( identifierSyntax.Identifier, TypeArgumentList( SeparatedList(new[] {typeOf.Type})))) - .WithArgumentList(invocation.ArgumentList.Arguments.Count == 0 - ? ArgumentList() - : invocation.ArgumentList.RemoveNode(invocation.ArgumentList.Arguments[0], SyntaxRemoveOptions.KeepNoTrivia)); + .WithArgumentList(newArgumentList); // If we're casting the GetComponent result, remove the cast as the returned value is now type safe var target = IsParentCastingResult(invocation) ? invocation.Parent : invocation; - var newRoot = root.ReplaceNode(target, newInvocation.WithAdditionalAnnotations(Formatter.Annotation)); + if (target == null) + return document; + + var newRoot = root?.ReplaceNode(target, newInvocation.WithAdditionalAnnotations(Formatter.Annotation)); if (newRoot == null) return document; diff --git a/src/Microsoft.Unity.Analyzers/NullableReferenceTypesSuppressor.cs b/src/Microsoft.Unity.Analyzers/NullableReferenceTypesSuppressor.cs index ab428c3d..ae86473d 100644 --- a/src/Microsoft.Unity.Analyzers/NullableReferenceTypesSuppressor.cs +++ b/src/Microsoft.Unity.Analyzers/NullableReferenceTypesSuppressor.cs @@ -28,20 +28,19 @@ public override void ReportSuppressions(SuppressionAnalysisContext context) { foreach (var diagnostic in context.ReportedDiagnostics) { - var root = diagnostic.Location.SourceTree.GetRoot(); - if (root == null) + var syntaxTree = diagnostic.Location.SourceTree; + if (syntaxTree == null) continue; - + + var root = syntaxTree.GetRoot(); var node = root.FindNode(diagnostic.Location.SourceSpan); - if (node == null) - continue; var classDeclaration = node.FirstAncestorOrSelf(); if (classDeclaration is null) continue; - var model = context.GetSemanticModel(diagnostic.Location.SourceTree); - var symbol = model?.GetDeclaredSymbol(classDeclaration); + var model = context.GetSemanticModel(syntaxTree); + var symbol = model.GetDeclaredSymbol(classDeclaration); if (symbol is null) continue; @@ -70,6 +69,8 @@ public override void ReportSuppressions(SuppressionAnalysisContext context) private static void AnalyzeFields(VariableDeclaratorSyntax declarator, Diagnostic diagnostic, SuppressionAnalysisContext context, SyntaxNode root) { var declarationSyntax = declarator.FirstAncestorOrSelf(); + if (declarationSyntax == null) + return; //suppress for fields that are not private and not static => statics cannot be set in editor and are not shown in the inspector and cannot be set there if (!declarationSyntax.Modifiers.Any(modifier => modifier.IsKind(SyntaxKind.PrivateKeyword) || modifier.IsKind(SyntaxKind.StaticKeyword)) diff --git a/src/Microsoft.Unity.Analyzers/PhysicsAllocMethodUsage.cs b/src/Microsoft.Unity.Analyzers/PhysicsAllocMethodUsage.cs index 9923b6b4..b794f427 100644 --- a/src/Microsoft.Unity.Analyzers/PhysicsAllocMethodUsage.cs +++ b/src/Microsoft.Unity.Analyzers/PhysicsAllocMethodUsage.cs @@ -5,7 +5,6 @@ using System.Collections.Generic; using System.Collections.Immutable; -using System.Linq; using System.Reflection; using Microsoft.CodeAnalysis; using Microsoft.CodeAnalysis.CSharp.Syntax; diff --git a/src/Microsoft.Unity.Analyzers/PropertyDrawerOnGUI.cs b/src/Microsoft.Unity.Analyzers/PropertyDrawerOnGUI.cs index 1900ae3b..3f4e6e53 100644 --- a/src/Microsoft.Unity.Analyzers/PropertyDrawerOnGUI.cs +++ b/src/Microsoft.Unity.Analyzers/PropertyDrawerOnGUI.cs @@ -81,7 +81,12 @@ public sealed override async Task RegisterCodeFixesAsync(CodeFixContext context) private static async Task RemoveInvocationAsync(Document document, InvocationExpressionSyntax invocation, CancellationToken cancellationToken) { var root = await document.GetSyntaxRootAsync(cancellationToken).ConfigureAwait(false); - var newRoot = root.RemoveNode(invocation.Parent, SyntaxRemoveOptions.KeepNoTrivia); + + var parent = invocation.Parent; + if (parent == null) + return document; + + var newRoot = root?.RemoveNode(parent, SyntaxRemoveOptions.KeepNoTrivia); if (newRoot == null) return document; diff --git a/src/Microsoft.Unity.Analyzers/ProtectedUnityMessage.cs b/src/Microsoft.Unity.Analyzers/ProtectedUnityMessage.cs index 07b824b8..4aa3dd25 100644 --- a/src/Microsoft.Unity.Analyzers/ProtectedUnityMessage.cs +++ b/src/Microsoft.Unity.Analyzers/ProtectedUnityMessage.cs @@ -51,11 +51,17 @@ private static void AnalyzeMethodDeclaration(SyntaxNodeAnalysisContext context) return; var typeSymbol = context.SemanticModel.GetDeclaredSymbol(classDeclaration); + if (typeSymbol == null) + return; + var scriptInfo = new ScriptInfo(typeSymbol); if (!scriptInfo.HasMessages) return; var symbol = context.SemanticModel.GetDeclaredSymbol(method); + if (symbol == null) + return; + if (!scriptInfo.IsMessage(symbol)) return; @@ -104,7 +110,7 @@ private static async Task MakeMessageProtectedAsync(Document document, newDeclaration = newDeclaration.AddModifiers(SyntaxFactory.Token(kind)); } - var newRoot = root.ReplaceNode(declaration, newDeclaration); + var newRoot = root?.ReplaceNode(declaration, newDeclaration); if (newRoot == null) return document; diff --git a/src/Microsoft.Unity.Analyzers/Reflection.cs b/src/Microsoft.Unity.Analyzers/Reflection.cs index df93f292..7540c7de 100644 --- a/src/Microsoft.Unity.Analyzers/Reflection.cs +++ b/src/Microsoft.Unity.Analyzers/Reflection.cs @@ -50,12 +50,17 @@ private static void AnalyzeMethodDeclaration(SyntaxNodeAnalysisContext context) private static bool IsCriticalMessage(SyntaxNodeAnalysisContext context, MethodDeclarationSyntax method) { var methodSymbol = context.SemanticModel.GetDeclaredSymbol(method); + if (methodSymbol == null) + return false; var classDeclaration = method.FirstAncestorOrSelf(); if (classDeclaration == null) return false; var typeSymbol = context.SemanticModel.GetDeclaredSymbol(classDeclaration); + if (typeSymbol == null) + return false; + var scriptInfo = new ScriptInfo(typeSymbol); if (!scriptInfo.HasMessages) return false; diff --git a/src/Microsoft.Unity.Analyzers/Resources/Strings.Designer.cs b/src/Microsoft.Unity.Analyzers/Resources/Strings.Designer.cs index 3fe48901..49487bcc 100644 --- a/src/Microsoft.Unity.Analyzers/Resources/Strings.Designer.cs +++ b/src/Microsoft.Unity.Analyzers/Resources/Strings.Designer.cs @@ -969,6 +969,42 @@ internal static string UnityObjectCoalescingAssignmentSuppressorJustification { } } + /// + /// Looks up a localized string similar to Use null comparison. + /// + internal static string UnityObjectIsPatternCodeFixTitle { + get { + return ResourceManager.GetString("UnityObjectIsPatternCodeFixTitle", resourceCulture); + } + } + + /// + /// Looks up a localized string similar to Unity overrides the null comparison operator for Unity objects, which is incompatible with pattern matching with null.. + /// + internal static string UnityObjectIsPatternDiagnosticDescription { + get { + return ResourceManager.GetString("UnityObjectIsPatternDiagnosticDescription", resourceCulture); + } + } + + /// + /// Looks up a localized string similar to Unity objects should not use pattern matching with null.. + /// + internal static string UnityObjectIsPatternDiagnosticMessageFormat { + get { + return ResourceManager.GetString("UnityObjectIsPatternDiagnosticMessageFormat", resourceCulture); + } + } + + /// + /// Looks up a localized string similar to Pattern matching with null on Unity objects. + /// + internal static string UnityObjectIsPatternDiagnosticTitle { + get { + return ResourceManager.GetString("UnityObjectIsPatternDiagnosticTitle", resourceCulture); + } + } + /// /// Looks up a localized string similar to Use null comparison. /// diff --git a/src/Microsoft.Unity.Analyzers/Resources/Strings.resx b/src/Microsoft.Unity.Analyzers/Resources/Strings.resx index 9e30a92c..b3fb7b4a 100644 --- a/src/Microsoft.Unity.Analyzers/Resources/Strings.resx +++ b/src/Microsoft.Unity.Analyzers/Resources/Strings.resx @@ -510,4 +510,16 @@ Use non-allocating physics APIs + + Use null comparison + + + Unity overrides the null comparison operator for Unity objects, which is incompatible with pattern matching with null. + + + Unity objects should not use pattern matching with null. + + + Pattern matching with null on Unity objects + \ No newline at end of file diff --git a/src/Microsoft.Unity.Analyzers/SerializeFieldSuppressor.cs b/src/Microsoft.Unity.Analyzers/SerializeFieldSuppressor.cs index 2cda87c5..0cf67fec 100644 --- a/src/Microsoft.Unity.Analyzers/SerializeFieldSuppressor.cs +++ b/src/Microsoft.Unity.Analyzers/SerializeFieldSuppressor.cs @@ -48,7 +48,7 @@ public override void ReportSuppressions(SuppressionAnalysisContext context) private static bool IsSuppressable(IFieldSymbol fieldSymbol) { - if (fieldSymbol.GetAttributes().Any(a => a.AttributeClass.Matches(typeof(SerializeField)) || a.AttributeClass.Matches(typeof(SerializeReference)))) + if (fieldSymbol.GetAttributes().Any(a => a.AttributeClass != null && (a.AttributeClass.Matches(typeof(SerializeField)) || a.AttributeClass.Matches(typeof(SerializeReference))))) return true; if (fieldSymbol.DeclaredAccessibility == Accessibility.Public && fieldSymbol.ContainingType.Extends(typeof(Object))) @@ -63,7 +63,11 @@ private void AnalyzeDiagnostic(Diagnostic diagnostic, SuppressionAnalysisContext if (fieldDeclarationSyntax == null) return; - var model = context.GetSemanticModel(diagnostic.Location.SourceTree); + var syntaxTree = diagnostic.Location.SourceTree; + if (syntaxTree == null) + return; + + var model = context.GetSemanticModel(syntaxTree); if (model.GetDeclaredSymbol(fieldDeclarationSyntax) is not IFieldSymbol fieldSymbol) return; diff --git a/src/Microsoft.Unity.Analyzers/SetPositionAndRotation.cs b/src/Microsoft.Unity.Analyzers/SetPositionAndRotation.cs index 764e58f7..6f5cd409 100644 --- a/src/Microsoft.Unity.Analyzers/SetPositionAndRotation.cs +++ b/src/Microsoft.Unity.Analyzers/SetPositionAndRotation.cs @@ -80,8 +80,13 @@ internal static bool GetNextAssignmentExpression(SemanticModel model, Assignment return false; var block = assignmentExpression.FirstAncestorOrSelf(); + if (block == null) + return false; + var siblingsAndSelf = block.ChildNodes().ToImmutableArray(); var expression = assignmentExpression.FirstAncestorOrSelf(); + if (expression == null) + return false; var lastIndexOf = siblingsAndSelf.LastIndexOf(expression); if (lastIndexOf == -1) diff --git a/src/Microsoft.Unity.Analyzers/SuppressionAnalysisContextExtensions.cs b/src/Microsoft.Unity.Analyzers/SuppressionAnalysisContextExtensions.cs index 40ebbbc1..c5dc5070 100644 --- a/src/Microsoft.Unity.Analyzers/SuppressionAnalysisContextExtensions.cs +++ b/src/Microsoft.Unity.Analyzers/SuppressionAnalysisContextExtensions.cs @@ -21,7 +21,7 @@ internal static class SuppressionAnalysisContextExtensions { var location = diagnostic.Location; var sourceTree = location.SourceTree; - var root = sourceTree.GetRoot(context.CancellationToken); + var root = sourceTree?.GetRoot(context.CancellationToken); return root? .FindNode(location.SourceSpan) diff --git a/src/Microsoft.Unity.Analyzers/TagComparison.cs b/src/Microsoft.Unity.Analyzers/TagComparison.cs index d8761768..48c01ecd 100644 --- a/src/Microsoft.Unity.Analyzers/TagComparison.cs +++ b/src/Microsoft.Unity.Analyzers/TagComparison.cs @@ -143,6 +143,9 @@ public sealed override async Task RegisterCodeFixesAsync(CodeFixContext context) _ => null }; + if (action == null) + return; + context.RegisterCodeFix( CodeAction.Create( Strings.TagComparisonCodeFixTitle, @@ -161,7 +164,7 @@ private static async Task ReplaceInvocationExpressionAsync(Document do SortExpressions(model, expr, out var tagExpression, out var otherExpression); var replacement = BuildReplacementNode(tagExpression, otherExpression); - var newRoot = root.ReplaceNode(expr, replacement); + var newRoot = root?.ReplaceNode(expr, replacement); if (newRoot == null) return document; @@ -182,7 +185,7 @@ private static async Task ReplaceBinaryExpressionAsync(Document docume if (expr.Kind() == SyntaxKind.NotEqualsExpression) replacement = SyntaxFactory.PrefixUnaryExpression(SyntaxKind.LogicalNotExpression, replacement); - var newRoot = root.ReplaceNode(expr, replacement); + var newRoot = root?.ReplaceNode(expr, replacement); if (newRoot == null) return document; diff --git a/src/Microsoft.Unity.Analyzers/ThrowExpressionSuppressor.cs b/src/Microsoft.Unity.Analyzers/ThrowExpressionSuppressor.cs index 7a87af07..dbde25a6 100644 --- a/src/Microsoft.Unity.Analyzers/ThrowExpressionSuppressor.cs +++ b/src/Microsoft.Unity.Analyzers/ThrowExpressionSuppressor.cs @@ -42,9 +42,11 @@ private static void AnalyzeDiagnostic(Diagnostic diagnostic, SuppressionAnalysis if (ifStatement?.Condition is not BinaryExpressionSyntax binaryExpression) return; - var model = context.GetSemanticModel(diagnostic.Location.SourceTree); - if (model == null) + var syntaxTree = diagnostic.Location.SourceTree; + if (syntaxTree == null) return; + + var model = context.GetSemanticModel(syntaxTree); if (ShouldReportSuppression(binaryExpression.Left, model) || ShouldReportSuppression(binaryExpression.Right, model)) context.ReportSuppression(Suppression.Create(Rule, diagnostic)); diff --git a/src/Microsoft.Unity.Analyzers/TryGetComponent.cs b/src/Microsoft.Unity.Analyzers/TryGetComponent.cs index 5945122b..dab25f73 100644 --- a/src/Microsoft.Unity.Analyzers/TryGetComponent.cs +++ b/src/Microsoft.Unity.Analyzers/TryGetComponent.cs @@ -58,7 +58,7 @@ private static void AnalyzeInvocation(SyntaxNodeAnalysisContext context) private static bool IsTryGetComponentSupported(SyntaxNodeAnalysisContext context) { // We need Unity 2019.2+ for proper support - var goType = context.Compilation.GetTypeByMetadataName(typeof(UnityEngine.GameObject).FullName); + var goType = context.Compilation?.GetTypeByMetadataName(typeof(UnityEngine.GameObject).FullName); return goType?.MemberNames.Contains(nameof(UnityEngine.Component.TryGetComponent)) ?? false; } } @@ -86,6 +86,9 @@ public TryGetComponentContext(string targetIdentifier, bool isVariableDeclaratio // We want an assignment or variable declaration with invocation as initializer var invocationParent = invocation.Parent; + if (invocationParent == null) + return null; + if (!TryGetTargetdentifier(model, invocationParent, out var targetIdentifier, out var isVariableDeclaration)) return null; diff --git a/src/Microsoft.Unity.Analyzers/UnityObjectNullHandling.cs b/src/Microsoft.Unity.Analyzers/UnityObjectNullHandling.cs index c305c318..0c5864f5 100644 --- a/src/Microsoft.Unity.Analyzers/UnityObjectNullHandling.cs +++ b/src/Microsoft.Unity.Analyzers/UnityObjectNullHandling.cs @@ -47,7 +47,21 @@ public class UnityObjectNullHandlingAnalyzer : DiagnosticAnalyzer isEnabledByDefault: true, description: Strings.UnityObjectCoalescingAssignmentDiagnosticDescription); - public override ImmutableArray SupportedDiagnostics => ImmutableArray.Create(NullCoalescingRule, NullPropagationRule, CoalescingAssignmentRule); + internal static readonly DiagnosticDescriptor IsPatternRule = new( + id: "UNT0029", + title: Strings.UnityObjectIsPatternDiagnosticTitle, + messageFormat: Strings.UnityObjectIsPatternDiagnosticMessageFormat, + category: DiagnosticCategory.Correctness, + defaultSeverity: DiagnosticSeverity.Info, + isEnabledByDefault: true, + description: Strings.UnityObjectIsPatternDiagnosticDescription); + + public override ImmutableArray SupportedDiagnostics => ImmutableArray.Create( + NullCoalescingRule, + NullPropagationRule, + CoalescingAssignmentRule, + IsPatternRule + ); public override void Initialize(AnalysisContext context) { @@ -56,8 +70,25 @@ public override void Initialize(AnalysisContext context) context.RegisterSyntaxNodeAction(AnalyzeCoalesceExpression, SyntaxKind.CoalesceExpression); context.RegisterSyntaxNodeAction(AnalyzeConditionalAccessExpression, SyntaxKind.ConditionalAccessExpression); context.RegisterSyntaxNodeAction(AnalyzeCoalesceAssignmentExpression, SyntaxKind.CoalesceAssignmentExpression); + context.RegisterSyntaxNodeAction(AnalyzeIsPatternExpression, SyntaxKind.IsPatternExpression); } + + private static void AnalyzeIsPatternExpression(SyntaxNodeAnalysisContext context) + { + var pattern = (IsPatternExpressionSyntax)context.Node; + switch (pattern.Pattern) + { + // obj is null + case ConstantPatternSyntax {Expression.RawKind: (int)SyntaxKind.NullLiteralExpression}: + + //obj is not null, we need roslyn 3.7.0 here for UnaryPatternSyntax type and SyntaxKind.NotPattern enum value + case UnaryPatternSyntax {RawKind: (int)SyntaxKind.NotPattern, Pattern: ConstantPatternSyntax {Expression.RawKind: (int)SyntaxKind.NullLiteralExpression} }: + AnalyzeExpression(pattern, pattern.Expression, context, IsPatternRule); + break; + } + } + private static void AnalyzeCoalesceAssignmentExpression(SyntaxNodeAnalysisContext context) { var assignment = (AssignmentExpressionSyntax)context.Node; @@ -92,13 +123,18 @@ private static void AnalyzeExpression(ExpressionSyntax originalExpression, Expre [ExportCodeFixProvider(LanguageNames.CSharp)] public class UnityObjectNullHandlingCodeFix : CodeFixProvider { - public sealed override ImmutableArray FixableDiagnosticIds => ImmutableArray.Create(UnityObjectNullHandlingAnalyzer.NullCoalescingRule.Id, UnityObjectNullHandlingAnalyzer.NullPropagationRule.Id, UnityObjectNullHandlingAnalyzer.CoalescingAssignmentRule.Id); + public sealed override ImmutableArray FixableDiagnosticIds => ImmutableArray.Create( + UnityObjectNullHandlingAnalyzer.NullCoalescingRule.Id, + UnityObjectNullHandlingAnalyzer.NullPropagationRule.Id, + UnityObjectNullHandlingAnalyzer.CoalescingAssignmentRule.Id, + UnityObjectNullHandlingAnalyzer.IsPatternRule.Id + ); public sealed override FixAllProvider GetFixAllProvider() => WellKnownFixAllProviders.BatchFixer; public sealed override async Task RegisterCodeFixesAsync(CodeFixContext context) { - var expression = await context.GetFixableNodeAsync(c => c is BinaryExpressionSyntax || c is AssignmentExpressionSyntax || c is ConditionalAccessExpressionSyntax); + var expression = await context.GetFixableNodeAsync(c => c is BinaryExpressionSyntax or AssignmentExpressionSyntax or ConditionalAccessExpressionSyntax or IsPatternExpressionSyntax); if (expression == null) return; @@ -128,6 +164,15 @@ public sealed override async Task RegisterCodeFixesAsync(CodeFixContext context) action = CodeAction.Create(Strings.UnityObjectCoalescingAssignmentCodeFixTitle, ct => ReplaceCoalescingAssignmentAsync(context.Document, aes, ct), aes.ToFullString()); break; + + // Pattern expression + case IsPatternExpressionSyntax pes: + if (HasSideEffect(pes.Expression)) + return; + + action = CodeAction.Create(Strings.UnityObjectIsPatternCodeFixTitle, ct => ReplacePatternExpressionAsync(context.Document, pes, ct), pes.ToFullString()); + break; + default: return; } @@ -149,7 +194,7 @@ private static bool HasSideEffect(ExpressionSyntax expression) private static async Task ReplaceWithAsync(Document document, SyntaxNode source, SyntaxNode replacement, CancellationToken cancellationToken) { var root = await document.GetSyntaxRootAsync(cancellationToken).ConfigureAwait(false); - var newRoot = root.ReplaceNode(source, replacement); + var newRoot = root?.ReplaceNode(source, replacement); return newRoot == null ? document : document.WithSyntaxRoot(newRoot); } @@ -189,6 +234,15 @@ private static async Task ReplaceNullPropagationAsync(Document documen return await ReplaceWithAsync(document, access, conditional, cancellationToken); } + + private static async Task ReplacePatternExpressionAsync(Document document, IsPatternExpressionSyntax pattern, CancellationToken cancellationToken) + { + // obj is null => obj == null, obj is not null => obj != null + var kind = pattern.Pattern is ConstantPatternSyntax ? SyntaxKind.EqualsExpression : SyntaxKind.NotEqualsExpression; + var binary = SyntaxFactory.BinaryExpression(kind, pattern.Expression, SyntaxFactory.LiteralExpression(SyntaxKind.NullLiteralExpression)); + + return await ReplaceWithAsync(document, pattern, binary, cancellationToken); + } } [DiagnosticAnalyzer(LanguageNames.CSharp)] @@ -246,8 +300,6 @@ private void AnalyzeBinaryExpression(Diagnostic diagnostic, SuppressionAnalysisC } var model = context.GetSemanticModel(binaryExpression.SyntaxTree); - if (model == null) - return; var type = model.GetTypeInfo(binaryExpression.Left); if (type.Type == null) diff --git a/src/Microsoft.Unity.Analyzers/UnusedCoroutineReturnValue.cs b/src/Microsoft.Unity.Analyzers/UnusedCoroutineReturnValue.cs index ea8490b5..2f27a48a 100644 --- a/src/Microsoft.Unity.Analyzers/UnusedCoroutineReturnValue.cs +++ b/src/Microsoft.Unity.Analyzers/UnusedCoroutineReturnValue.cs @@ -107,7 +107,7 @@ private static async Task WrapWithStartCoroutineAsync(Document documen SingletonSeparatedList( Argument(invocation))))); - var newRoot = root.ReplaceNode(parent, newExpressionStatement); + var newRoot = root?.ReplaceNode(parent, newExpressionStatement); if (newRoot == null) return document; diff --git a/src/Microsoft.Unity.Analyzers/UnusedMethodSuppressor.cs b/src/Microsoft.Unity.Analyzers/UnusedMethodSuppressor.cs index 3d38dc26..c1cb988b 100644 --- a/src/Microsoft.Unity.Analyzers/UnusedMethodSuppressor.cs +++ b/src/Microsoft.Unity.Analyzers/UnusedMethodSuppressor.cs @@ -32,14 +32,17 @@ public override void ReportSuppressions(SuppressionAnalysisContext context) private static void AnalyzeDiagnostic(Diagnostic diagnostic, SuppressionAnalysisContext context) { - var sourceTree = diagnostic.Location.SourceTree; - var root = sourceTree.GetRoot(context.CancellationToken); + var syntaxTree = diagnostic.Location.SourceTree; + if (syntaxTree == null) + return; + + var root = syntaxTree.GetRoot(context.CancellationToken); var methodDeclarationSyntax = context.GetSuppressibleNode(diagnostic); if (methodDeclarationSyntax == null) return; - var model = context.GetSemanticModel(diagnostic.Location.SourceTree); + var model = context.GetSemanticModel(syntaxTree); if (model.GetDeclaredSymbol(methodDeclarationSyntax) is not IMethodSymbol methodSymbol) return; diff --git a/src/Microsoft.Unity.Analyzers/UpdateDeltaTime.cs b/src/Microsoft.Unity.Analyzers/UpdateDeltaTime.cs index 0ac24f30..2d1b7b03 100644 --- a/src/Microsoft.Unity.Analyzers/UpdateDeltaTime.cs +++ b/src/Microsoft.Unity.Analyzers/UpdateDeltaTime.cs @@ -59,8 +59,13 @@ private static bool IsMessage(SyntaxNodeAnalysisContext context, MethodDeclarati return false; var methodSymbol = context.SemanticModel.GetDeclaredSymbol(method); + if (methodSymbol == null) + return false; var typeSymbol = context.SemanticModel.GetDeclaredSymbol(classDeclaration); + if (typeSymbol == null) + return false; + var scriptInfo = new ScriptInfo(typeSymbol); if (!scriptInfo.HasMessages) return false; @@ -143,7 +148,7 @@ private static async Task ReplaceDeltaTimeIdentifierAsync(Document doc .WithIdentifier(SyntaxFactory.Identifier(name)) .WithTriviaFrom(identifierName); - var newRoot = root.ReplaceNode(identifierName, newIdentifierName); + var newRoot = root?.ReplaceNode(identifierName, newIdentifierName); if (newRoot == null) return document; diff --git a/src/Microsoft.Unity.Analyzers/VectorMath.cs b/src/Microsoft.Unity.Analyzers/VectorMath.cs index b8dbb361..cebe5db8 100644 --- a/src/Microsoft.Unity.Analyzers/VectorMath.cs +++ b/src/Microsoft.Unity.Analyzers/VectorMath.cs @@ -63,8 +63,11 @@ private void AnalyzeMultiplyExpression(SyntaxNodeAnalysisContext context) } } - private static bool IsReportableExpression(SyntaxNodeAnalysisContext context, SyntaxNode node, Type vectorType) + private static bool IsReportableExpression(SyntaxNodeAnalysisContext context, SyntaxNode? node, Type vectorType) { + if (node == null) + return false; + return IsSupportedExpression(context, node, vectorType) && NeedsOrdering(context, (ExpressionSyntax)node); } @@ -195,9 +198,6 @@ private static async Task FixOperandOrderAsync(Document document, Bina .WithTriviaFrom(node); var newRoot = root.ReplaceNode(node, newNode); - if (newRoot == null) - return document; - return document.WithSyntaxRoot(newRoot); }