From 1a9beddc65a4322a6529de3dbf2c95eb22faa793 Mon Sep 17 00:00:00 2001 From: Youssef Victor Date: Sun, 14 Feb 2021 22:06:36 +0200 Subject: [PATCH 1/5] Add CS8602 suppressor --- eng/Versions.props | 1 + .../NullableDiagnosticSuppressor.cs | 51 +++++++++ .../Utilities/IOperationExtensions.cs | 55 +++++++++ .../EFCore.Analyzers.Tests.csproj | 4 +- .../NullableDiagnosticSuppressorTest.cs | 55 +++++++++ .../DiagnosticAnalyzerTestBase.cs | 4 +- .../Verifiers/AdditionalMetadataReferences.cs | 104 ++++++++++++++++++ .../Verifiers/CSharpCodeFixVerifier`2+Test.cs | 78 +++++++++++++ .../Verifiers/CSharpCodeFixVerifier`2.cs | 54 +++++++++ 9 files changed, 403 insertions(+), 3 deletions(-) create mode 100644 src/EFCore.Analyzers/NullableDiagnosticSuppressor.cs create mode 100644 src/EFCore.Analyzers/Utilities/IOperationExtensions.cs create mode 100644 test/EFCore.Analyzers.Tests/NullableDiagnosticSuppressorTest.cs create mode 100644 test/EFCore.Analyzers.Tests/TestUtilities/Verifiers/AdditionalMetadataReferences.cs create mode 100644 test/EFCore.Analyzers.Tests/TestUtilities/Verifiers/CSharpCodeFixVerifier`2+Test.cs create mode 100644 test/EFCore.Analyzers.Tests/TestUtilities/Verifiers/CSharpCodeFixVerifier`2.cs diff --git a/eng/Versions.props b/eng/Versions.props index ed42966124d..4c1f2bcca22 100644 --- a/eng/Versions.props +++ b/eng/Versions.props @@ -30,5 +30,6 @@ 3.7.0 + 1.0.1-beta1.20623.3 diff --git a/src/EFCore.Analyzers/NullableDiagnosticSuppressor.cs b/src/EFCore.Analyzers/NullableDiagnosticSuppressor.cs new file mode 100644 index 00000000000..dad11bb7c1c --- /dev/null +++ b/src/EFCore.Analyzers/NullableDiagnosticSuppressor.cs @@ -0,0 +1,51 @@ +// Copyright (c) .NET Foundation. All rights reserved. +// Licensed under the Apache License, Version 2.0. See License.txt in the project root for license information. + +using System.Collections.Immutable; +using Microsoft.CodeAnalysis; +using Microsoft.CodeAnalysis.Diagnostics; +using Microsoft.EntityFrameworkCore.Utilities; + +namespace Microsoft.EntityFrameworkCore +{ + [DiagnosticAnalyzer(LanguageNames.CSharp)] + public sealed class NullableDiagnosticSuppressor : DiagnosticSuppressor + { + private static readonly SuppressionDescriptor _descriptor = new( + id: "EFS0001", + suppressedDiagnosticId: "CS8602", + justification: "The dereference is safe inside this expression tree."); + + public override ImmutableArray SupportedSuppressions => ImmutableArray.Create(_descriptor); + + public override void ReportSuppressions(SuppressionAnalysisContext context) + { + context.ReportSuppression(Suppression.Create(_descriptor, context.ReportedDiagnostics[0])); + + + /*var linqExpressionType = context.Compilation.GetTypeByMetadataName("System.Linq.Expressions.Expression`1"); + if (linqExpressionType is null) + { + return; + } + + foreach (var diagnostic in context.ReportedDiagnostics) + { + if (diagnostic.Location.SourceTree is not SyntaxTree tree) + { + continue; + } + + var root = tree.GetRoot(context.CancellationToken); + var node = root.FindNode(diagnostic.Location.SourceSpan, getInnermostNodeForTie: true); + var model = context.GetSemanticModel(tree); + var operation = model.GetOperation(node, context.CancellationToken); + if (operation?.IsWithinExpressionTree(linqExpressionType) == true) + { + context.ReportSuppression(Suppression.Create(_descriptor, diagnostic)); + } + + }*/ + } + } +} diff --git a/src/EFCore.Analyzers/Utilities/IOperationExtensions.cs b/src/EFCore.Analyzers/Utilities/IOperationExtensions.cs new file mode 100644 index 00000000000..4b7a50e6088 --- /dev/null +++ b/src/EFCore.Analyzers/Utilities/IOperationExtensions.cs @@ -0,0 +1,55 @@ +// Copyright (c) .NET Foundation. All rights reserved. +// Licensed under the Apache License, Version 2.0. See License.txt in the project root for license information. + +using System; +using System.Collections.Immutable; +using Microsoft.CodeAnalysis; + +namespace Microsoft.EntityFrameworkCore.Utilities +{ + internal static class IOperationExtensions + { + private static readonly ImmutableArray s_LambdaAndLocalFunctionKinds = + ImmutableArray.Create(OperationKind.AnonymousFunction, OperationKind.LocalFunction); + + public static bool IsWithinExpressionTree(this IOperation operation, INamedTypeSymbol linqExpressionTreeType) + => linqExpressionTreeType != null + && operation.GetAncestor(s_LambdaAndLocalFunctionKinds)?.Parent?.Type?.OriginalDefinition is { } lambdaType + && linqExpressionTreeType.Equals(lambdaType, SymbolEqualityComparer.Default); + + /// + /// Gets the first ancestor of this operation with: + /// 1. Any OperationKind from the specified . + /// 2. If is non-null, it succeeds for the ancestor. + /// Returns null if there is no such ancestor. + /// + public static IOperation? GetAncestor(this IOperation root, ImmutableArray ancestorKinds, Func? predicate = null) + { + if (root == null) + { + throw new ArgumentNullException(nameof(root)); + } + + var ancestor = root; + do + { + ancestor = ancestor.Parent; + } while (ancestor != null && !ancestorKinds.Contains(ancestor.Kind)); + + if (ancestor != null) + { + if (predicate != null && !predicate(ancestor)) + { + return GetAncestor(ancestor, ancestorKinds, predicate); + } + return ancestor; + } + else + { + return default; + } + } + + + } +} diff --git a/test/EFCore.Analyzers.Tests/EFCore.Analyzers.Tests.csproj b/test/EFCore.Analyzers.Tests/EFCore.Analyzers.Tests.csproj index d68e8bdd40f..08e9a7b9e75 100644 --- a/test/EFCore.Analyzers.Tests/EFCore.Analyzers.Tests.csproj +++ b/test/EFCore.Analyzers.Tests/EFCore.Analyzers.Tests.csproj @@ -5,6 +5,7 @@ Microsoft.EntityFrameworkCore.Analyzers.Tests Microsoft.EntityFrameworkCore true + enable @@ -13,7 +14,8 @@ - + + diff --git a/test/EFCore.Analyzers.Tests/NullableDiagnosticSuppressorTest.cs b/test/EFCore.Analyzers.Tests/NullableDiagnosticSuppressorTest.cs new file mode 100644 index 00000000000..d400974e4b4 --- /dev/null +++ b/test/EFCore.Analyzers.Tests/NullableDiagnosticSuppressorTest.cs @@ -0,0 +1,55 @@ +using System.Threading.Tasks; +using Microsoft.CodeAnalysis.Testing; +using Xunit; + +using VerifyWithSuppressor = Microsoft.EntityFrameworkCore.TestUtilities.Verifiers.CSharpCodeFixVerifier< + Microsoft.EntityFrameworkCore.NullableDiagnosticSuppressor, + Microsoft.CodeAnalysis.Testing.EmptyCodeFixProvider>; + +using VerifyWithoutSuppressor = Microsoft.EntityFrameworkCore.TestUtilities.Verifiers.CSharpCodeFixVerifier< + Microsoft.CodeAnalysis.Testing.EmptyDiagnosticAnalyzer, + Microsoft.CodeAnalysis.Testing.EmptyCodeFixProvider>; + +namespace Microsoft.EntityFrameworkCore +{ + public class NullableDiagnosticSuppressorTest + { + /// + /// Assert that given code have warnings specified by markup that are suppressed by + /// + private static async Task AssertWarningIsSuppressedAsync(string code) + { + await VerifyWithoutSuppressor.VerifyAnalyzerAsync(code); + + await new VerifyWithSuppressor.Test() + { + TestState = { Sources = { code }, MarkupHandling = MarkupMode.Ignore } + }.RunAsync(); + } + + /// + /// Assert that given code have warnings specified by markup that are not suppressed by + /// + private static async Task AssertWarningIsNotSuppressedAsync(string code) + { + await VerifyWithSuppressor.VerifyAnalyzerAsync(code); + await VerifyWithoutSuppressor.VerifyAnalyzerAsync(code); + } + + [Fact] + public async Task TestCompilerWarningAsync() + { + var code = @" +#nullable enable + +class C +{ + public void M(C? c) + { + _ = {|CS8602:c|}.ToString(); + } +}"; + await AssertWarningIsSuppressedAsync(code); + } + } +} diff --git a/test/EFCore.Analyzers.Tests/TestUtilities/DiagnosticAnalyzerTestBase.cs b/test/EFCore.Analyzers.Tests/TestUtilities/DiagnosticAnalyzerTestBase.cs index b6e070ebd81..99cdfc90925 100644 --- a/test/EFCore.Analyzers.Tests/TestUtilities/DiagnosticAnalyzerTestBase.cs +++ b/test/EFCore.Analyzers.Tests/TestUtilities/DiagnosticAnalyzerTestBase.cs @@ -52,7 +52,7 @@ protected async Task AssertNoDiagnostics(string source, params string[] extraUsi protected async Task GetDiagnosticsFullSourceAsync(string source) { var compilation = await CreateProject(source).GetCompilationAsync(); - var errors = compilation.GetDiagnostics().Where(d => d.Severity == DiagnosticSeverity.Error); + var errors = compilation!.GetDiagnostics().Where(d => d.Severity == DiagnosticSeverity.Error); Assert.Empty(errors); @@ -92,7 +92,7 @@ var metadataReferences .AddMetadataReferences(projectId, metadataReferences) .AddDocument(documentId, fileName, SourceText.From(source)); - return solution.GetProject(projectId) + return solution.GetProject(projectId)! .WithCompilationOptions(new CSharpCompilationOptions(OutputKind.DynamicallyLinkedLibrary)); } } diff --git a/test/EFCore.Analyzers.Tests/TestUtilities/Verifiers/AdditionalMetadataReferences.cs b/test/EFCore.Analyzers.Tests/TestUtilities/Verifiers/AdditionalMetadataReferences.cs new file mode 100644 index 00000000000..4c45e5e3ebc --- /dev/null +++ b/test/EFCore.Analyzers.Tests/TestUtilities/Verifiers/AdditionalMetadataReferences.cs @@ -0,0 +1,104 @@ +using System.Collections.Immutable; +using Microsoft.CodeAnalysis; +using Microsoft.CodeAnalysis.CSharp; +using Microsoft.CodeAnalysis.Testing; + +namespace Microsoft.EntityFrameworkCore.TestUtilities.Verifiers +{ + public static class AdditionalMetadataReferences + { + public static ReferenceAssemblies Default { get; } = CreateDefaultReferenceAssemblies(); + + public static ReferenceAssemblies DefaultWithoutRoslynSymbols { get; } = ReferenceAssemblies.Default + .AddAssemblies(ImmutableArray.Create("System.Xml.Data")) + .AddPackages(ImmutableArray.Create(new PackageIdentity("Microsoft.CodeAnalysis.Workspaces.Common", "3.0.0"))); + + public static ReferenceAssemblies DefaultWithSystemWeb { get; } = ReferenceAssemblies.NetFramework.Net472.Default + .AddAssemblies(ImmutableArray.Create("System.Web", "System.Web.Extensions")); + + public static ReferenceAssemblies DefaultForTaintedDataAnalysis { get; } = ReferenceAssemblies.NetFramework.Net472.Default + .AddAssemblies(ImmutableArray.Create("PresentationFramework", "System.DirectoryServices", "System.Web", "System.Web.Extensions", "System.Xaml")) + .AddPackages(ImmutableArray.Create( + new PackageIdentity("AntiXSS", "4.3.0"), + new PackageIdentity("Microsoft.AspNetCore.Mvc", "2.2.0"), + new PackageIdentity("Microsoft.EntityFrameworkCore.Relational", "2.0.3"))); + + public static ReferenceAssemblies DefaultWithSerialization { get; } = ReferenceAssemblies.NetFramework.Net472.Default + .AddAssemblies(ImmutableArray.Create("System.Runtime.Serialization")); + + public static ReferenceAssemblies DefaultWithAzureStorage { get; } = ReferenceAssemblies.Default + .AddPackages(ImmutableArray.Create(new PackageIdentity("WindowsAzure.Storage", "9.0.0"))); + + public static ReferenceAssemblies DefaultWithNewtonsoftJson10 { get; } = Default + .AddPackages(ImmutableArray.Create(new PackageIdentity("Newtonsoft.Json", "10.0.1"))); + + public static ReferenceAssemblies DefaultWithNewtonsoftJson12 { get; } = Default + .AddPackages(ImmutableArray.Create(new PackageIdentity("Newtonsoft.Json", "12.0.1"))); + + public static ReferenceAssemblies DefaultWithWinForms { get; } = ReferenceAssemblies.NetFramework.Net472.WindowsForms; + + public static ReferenceAssemblies DefaultWithWinHttpHandler { get; } = ReferenceAssemblies.NetStandard.NetStandard20 + .AddPackages(ImmutableArray.Create(new PackageIdentity("System.Net.Http.WinHttpHandler", "4.7.0"))); + + public static ReferenceAssemblies DefaultWithAspNetCoreMvc { get; } = Default + .AddPackages(ImmutableArray.Create( + new PackageIdentity("Microsoft.AspNetCore", "1.1.7"), + new PackageIdentity("Microsoft.AspNetCore.Mvc", "1.1.8"), + new PackageIdentity("Microsoft.AspNetCore.Http", "1.1.2"))); + + public static ReferenceAssemblies DefaultWithNUnit { get; } = Default + .AddPackages(ImmutableArray.Create(new PackageIdentity("NUnit", "3.12.0"))); + + public static ReferenceAssemblies DefaultWithXUnit { get; } = Default + .AddPackages(ImmutableArray.Create(new PackageIdentity("xunit", "2.4.1"))); + + public static ReferenceAssemblies DefaultWithMSTest { get; } = Default + .AddPackages(ImmutableArray.Create(new PackageIdentity("MSTest.TestFramework", "2.1.0"))); + + public static ReferenceAssemblies DefaultWithAsyncInterfaces { get; } = Default + .AddPackages(ImmutableArray.Create(new PackageIdentity("Microsoft.Bcl.AsyncInterfaces", "1.1.0"))); + + public static MetadataReference SystemCollectionsImmutableReference { get; } = MetadataReference.CreateFromFile(typeof(ImmutableHashSet<>).Assembly.Location); + public static MetadataReference SystemComponentModelCompositionReference { get; } = MetadataReference.CreateFromFile(typeof(System.ComponentModel.Composition.ExportAttribute).Assembly.Location); + public static MetadataReference SystemCompositionReference { get; } = MetadataReference.CreateFromFile(typeof(System.Composition.ExportAttribute).Assembly.Location); + public static MetadataReference SystemXmlDataReference { get; } = MetadataReference.CreateFromFile(typeof(System.Data.Rule).Assembly.Location); + public static MetadataReference CodeAnalysisReference { get; } = MetadataReference.CreateFromFile(typeof(Compilation).Assembly.Location); + public static MetadataReference CSharpSymbolsReference { get; } = MetadataReference.CreateFromFile(typeof(CSharpCompilation).Assembly.Location); + public static MetadataReference WorkspacesReference { get; } = MetadataReference.CreateFromFile(typeof(Workspace).Assembly.Location); +#if !NETCOREAPP + public static MetadataReference SystemWebReference { get; } = MetadataReference.CreateFromFile(typeof(System.Web.HttpRequest).Assembly.Location); + public static MetadataReference SystemRuntimeSerialization { get; } = MetadataReference.CreateFromFile(typeof(System.Runtime.Serialization.NetDataContractSerializer).Assembly.Location); +#endif + +#if !NETCOREAPP + public static MetadataReference SystemXaml { get; } = MetadataReference.CreateFromFile(typeof(System.Xaml.XamlReader).Assembly.Location); + public static MetadataReference PresentationFramework { get; } = MetadataReference.CreateFromFile(typeof(System.Windows.Markup.XamlReader).Assembly.Location); + public static MetadataReference SystemWeb { get; } = MetadataReference.CreateFromFile(typeof(System.Web.HttpRequest).Assembly.Location); + public static MetadataReference SystemWebExtensions { get; } = MetadataReference.CreateFromFile(typeof(System.Web.Script.Serialization.JavaScriptSerializer).Assembly.Location); + public static MetadataReference SystemServiceModel { get; } = MetadataReference.CreateFromFile(typeof(System.ServiceModel.OperationContractAttribute).Assembly.Location); +#endif + + private static ReferenceAssemblies CreateDefaultReferenceAssemblies() + { + var referenceAssemblies = ReferenceAssemblies.Default; + +#if !NETCOREAPP + referenceAssemblies = referenceAssemblies.AddAssemblies(ImmutableArray.Create("System.Xml.Data")); +#endif + + referenceAssemblies = referenceAssemblies.AddPackages(ImmutableArray.Create(new PackageIdentity("Microsoft.CodeAnalysis", "3.0.0"))); + +#if NETCOREAPP + referenceAssemblies = referenceAssemblies.AddPackages(ImmutableArray.Create( + new PackageIdentity("System.Runtime.Serialization.Formatters", "4.3.0"), + new PackageIdentity("System.Configuration.ConfigurationManager", "4.7.0"), + new PackageIdentity("System.Security.Cryptography.Cng", "4.7.0"), + new PackageIdentity("System.Security.Permissions", "4.7.0"), + new PackageIdentity("Microsoft.VisualBasic", "10.3.0"))); +#endif + + return referenceAssemblies; + } + } + +} diff --git a/test/EFCore.Analyzers.Tests/TestUtilities/Verifiers/CSharpCodeFixVerifier`2+Test.cs b/test/EFCore.Analyzers.Tests/TestUtilities/Verifiers/CSharpCodeFixVerifier`2+Test.cs new file mode 100644 index 00000000000..328da7d96bb --- /dev/null +++ b/test/EFCore.Analyzers.Tests/TestUtilities/Verifiers/CSharpCodeFixVerifier`2+Test.cs @@ -0,0 +1,78 @@ +using System; +using System.Collections.Immutable; +using System.Net; +using Microsoft.CodeAnalysis; +using Microsoft.CodeAnalysis.CodeFixes; +using Microsoft.CodeAnalysis.CSharp; +using Microsoft.CodeAnalysis.CSharp.Testing; +using Microsoft.CodeAnalysis.Diagnostics; +using Microsoft.CodeAnalysis.Testing.Verifiers; +using Microsoft.CodeAnalysis.Text; + +namespace Microsoft.EntityFrameworkCore.TestUtilities.Verifiers +{ + public static partial class CSharpCodeFixVerifier + where TAnalyzer : DiagnosticAnalyzer, new() + where TCodeFix : CodeFixProvider, new() + { + public class Test : CSharpCodeFixTest + { + static Test() + { + // If we have outdated defaults from the host unit test application targeting an older .NET Framework, use more + // reasonable TLS protocol version for outgoing connections. +#pragma warning disable CA5364 // Do Not Use Deprecated Security Protocols +#pragma warning disable CS0618 // Type or member is obsolete + if (ServicePointManager.SecurityProtocol == (SecurityProtocolType.Ssl3 | SecurityProtocolType.Tls)) +#pragma warning restore CS0618 // Type or member is obsolete +#pragma warning restore CA5364 // Do Not Use Deprecated Security Protocols + { + ServicePointManager.SecurityProtocol = SecurityProtocolType.Tls12; + } + } + + internal static readonly ImmutableDictionary NullableWarnings = GetNullableWarningsFromCompiler(); + + public Test() + { + ReferenceAssemblies = AdditionalMetadataReferences.Default; + + SolutionTransforms.Add((solution, projectId) => + { + var project = solution.GetProject(projectId)!; + var parseOptions = (CSharpParseOptions)project.ParseOptions!; + solution = solution.WithProjectParseOptions(projectId, parseOptions.WithLanguageVersion(LanguageVersion)); + + var compilationOptions = project.CompilationOptions!; + compilationOptions = compilationOptions.WithSpecificDiagnosticOptions(compilationOptions.SpecificDiagnosticOptions.SetItems(NullableWarnings)); + + solution = solution.WithProjectCompilationOptions(projectId, compilationOptions); + + if (AnalyzerConfigDocument is not null) + { + solution = solution.AddAnalyzerConfigDocument( + DocumentId.CreateNewId(projectId, debugName: ".editorconfig"), + ".editorconfig", + SourceText.From($"is_global = true" + Environment.NewLine + AnalyzerConfigDocument), + filePath: @"z:\.editorconfig"); + } + + return solution; + }); + } + + private static ImmutableDictionary GetNullableWarningsFromCompiler() + { + string[] args = { "/warnaserror:nullable" }; + var commandLineArguments = CSharpCommandLineParser.Default.Parse(args, baseDirectory: Environment.CurrentDirectory, sdkDirectory: Environment.CurrentDirectory); + var nullableWarnings = commandLineArguments.CompilationOptions.SpecificDiagnosticOptions; + return nullableWarnings; + } + + public LanguageVersion LanguageVersion { get; set; } = LanguageVersion.CSharp9; + + public string? AnalyzerConfigDocument { get; set; } + } + } + +} diff --git a/test/EFCore.Analyzers.Tests/TestUtilities/Verifiers/CSharpCodeFixVerifier`2.cs b/test/EFCore.Analyzers.Tests/TestUtilities/Verifiers/CSharpCodeFixVerifier`2.cs new file mode 100644 index 00000000000..de30230fa79 --- /dev/null +++ b/test/EFCore.Analyzers.Tests/TestUtilities/Verifiers/CSharpCodeFixVerifier`2.cs @@ -0,0 +1,54 @@ +using System.Threading.Tasks; +using Microsoft.CodeAnalysis; +using Microsoft.CodeAnalysis.CodeFixes; +using Microsoft.CodeAnalysis.CSharp.Testing; +using Microsoft.CodeAnalysis.Diagnostics; +using Microsoft.CodeAnalysis.Testing; +using Microsoft.CodeAnalysis.Testing.Verifiers; + +namespace Microsoft.EntityFrameworkCore.TestUtilities.Verifiers +{ + public static partial class CSharpCodeFixVerifier + where TAnalyzer : DiagnosticAnalyzer, new() + where TCodeFix : CodeFixProvider, new() + { + public static DiagnosticResult Diagnostic() + => CSharpCodeFixVerifier.Diagnostic(); + + public static DiagnosticResult Diagnostic(string diagnosticId) + => CSharpCodeFixVerifier.Diagnostic(diagnosticId); + + public static DiagnosticResult Diagnostic(DiagnosticDescriptor descriptor) + => CSharpCodeFixVerifier.Diagnostic(descriptor); + + public static async Task VerifyAnalyzerAsync(string source, params DiagnosticResult[] expected) + { + var test = new Test + { + TestCode = source, + }; + + test.ExpectedDiagnostics.AddRange(expected); + await test.RunAsync(); + } + + public static async Task VerifyCodeFixAsync(string source, string fixedSource) + => await VerifyCodeFixAsync(source, DiagnosticResult.EmptyDiagnosticResults, fixedSource); + + public static async Task VerifyCodeFixAsync(string source, DiagnosticResult expected, string fixedSource) + => await VerifyCodeFixAsync(source, new[] { expected }, fixedSource); + + public static async Task VerifyCodeFixAsync(string source, DiagnosticResult[] expected, string fixedSource) + { + var test = new Test + { + TestCode = source, + FixedCode = fixedSource, + }; + + test.ExpectedDiagnostics.AddRange(expected); + await test.RunAsync(); + } + } + +} From 0d472d506d8f8fc461a4f8e47b2a22933b3f7218 Mon Sep 17 00:00:00 2001 From: Youssef Victor Date: Mon, 15 Feb 2021 00:05:27 +0200 Subject: [PATCH 2/5] Fix tests --- src/EFCore.Analyzers/NullableDiagnosticSuppressor.cs | 7 ++----- .../NullableDiagnosticSuppressorTest.cs | 2 +- .../Verifiers/CSharpCodeFixVerifier`2+Test.cs | 11 +++-------- 3 files changed, 6 insertions(+), 14 deletions(-) diff --git a/src/EFCore.Analyzers/NullableDiagnosticSuppressor.cs b/src/EFCore.Analyzers/NullableDiagnosticSuppressor.cs index dad11bb7c1c..d12285e047c 100644 --- a/src/EFCore.Analyzers/NullableDiagnosticSuppressor.cs +++ b/src/EFCore.Analyzers/NullableDiagnosticSuppressor.cs @@ -20,10 +20,7 @@ public sealed class NullableDiagnosticSuppressor : DiagnosticSuppressor public override void ReportSuppressions(SuppressionAnalysisContext context) { - context.ReportSuppression(Suppression.Create(_descriptor, context.ReportedDiagnostics[0])); - - - /*var linqExpressionType = context.Compilation.GetTypeByMetadataName("System.Linq.Expressions.Expression`1"); + var linqExpressionType = context.Compilation.GetTypeByMetadataName("System.Linq.Expressions.Expression`1"); if (linqExpressionType is null) { return; @@ -45,7 +42,7 @@ public override void ReportSuppressions(SuppressionAnalysisContext context) context.ReportSuppression(Suppression.Create(_descriptor, diagnostic)); } - }*/ + } } } } diff --git a/test/EFCore.Analyzers.Tests/NullableDiagnosticSuppressorTest.cs b/test/EFCore.Analyzers.Tests/NullableDiagnosticSuppressorTest.cs index d400974e4b4..686066515ab 100644 --- a/test/EFCore.Analyzers.Tests/NullableDiagnosticSuppressorTest.cs +++ b/test/EFCore.Analyzers.Tests/NullableDiagnosticSuppressorTest.cs @@ -49,7 +49,7 @@ public void M(C? c) _ = {|CS8602:c|}.ToString(); } }"; - await AssertWarningIsSuppressedAsync(code); + await AssertWarningIsNotSuppressedAsync(code); } } } diff --git a/test/EFCore.Analyzers.Tests/TestUtilities/Verifiers/CSharpCodeFixVerifier`2+Test.cs b/test/EFCore.Analyzers.Tests/TestUtilities/Verifiers/CSharpCodeFixVerifier`2+Test.cs index 328da7d96bb..27ed38b62e8 100644 --- a/test/EFCore.Analyzers.Tests/TestUtilities/Verifiers/CSharpCodeFixVerifier`2+Test.cs +++ b/test/EFCore.Analyzers.Tests/TestUtilities/Verifiers/CSharpCodeFixVerifier`2+Test.cs @@ -6,6 +6,7 @@ using Microsoft.CodeAnalysis.CSharp; using Microsoft.CodeAnalysis.CSharp.Testing; using Microsoft.CodeAnalysis.Diagnostics; +using Microsoft.CodeAnalysis.Testing; using Microsoft.CodeAnalysis.Testing.Verifiers; using Microsoft.CodeAnalysis.Text; @@ -31,8 +32,6 @@ static Test() } } - internal static readonly ImmutableDictionary NullableWarnings = GetNullableWarningsFromCompiler(); - public Test() { ReferenceAssemblies = AdditionalMetadataReferences.Default; @@ -44,7 +43,6 @@ public Test() solution = solution.WithProjectParseOptions(projectId, parseOptions.WithLanguageVersion(LanguageVersion)); var compilationOptions = project.CompilationOptions!; - compilationOptions = compilationOptions.WithSpecificDiagnosticOptions(compilationOptions.SpecificDiagnosticOptions.SetItems(NullableWarnings)); solution = solution.WithProjectCompilationOptions(projectId, compilationOptions); @@ -61,12 +59,9 @@ public Test() }); } - private static ImmutableDictionary GetNullableWarningsFromCompiler() + protected override bool IsCompilerDiagnosticIncluded(Diagnostic diagnostic, CompilerDiagnostics compilerDiagnostics) { - string[] args = { "/warnaserror:nullable" }; - var commandLineArguments = CSharpCommandLineParser.Default.Parse(args, baseDirectory: Environment.CurrentDirectory, sdkDirectory: Environment.CurrentDirectory); - var nullableWarnings = commandLineArguments.CompilationOptions.SpecificDiagnosticOptions; - return nullableWarnings; + return !diagnostic.IsSuppressed; } public LanguageVersion LanguageVersion { get; set; } = LanguageVersion.CSharp9; From dd5bcc2e2a067473796a9e8ec96ec782d3e43ab3 Mon Sep 17 00:00:00 2001 From: Youssef Victor Date: Mon, 15 Feb 2021 01:39:53 +0200 Subject: [PATCH 3/5] Handle methods from EFCore queryable extensions --- .../NullableDiagnosticSuppressor.cs | 18 +++- .../Utilities/IOperationExtensions.cs | 20 +++-- .../Utilities/NotNullWhenAttribute.cs | 20 +++++ .../NullableDiagnosticSuppressorTest.cs | 30 +++++++ .../Verifiers/AdditionalMetadataReferences.cs | 84 +------------------ 5 files changed, 84 insertions(+), 88 deletions(-) create mode 100644 src/EFCore.Analyzers/Utilities/NotNullWhenAttribute.cs diff --git a/src/EFCore.Analyzers/NullableDiagnosticSuppressor.cs b/src/EFCore.Analyzers/NullableDiagnosticSuppressor.cs index d12285e047c..654f2fc8613 100644 --- a/src/EFCore.Analyzers/NullableDiagnosticSuppressor.cs +++ b/src/EFCore.Analyzers/NullableDiagnosticSuppressor.cs @@ -4,6 +4,7 @@ using System.Collections.Immutable; using Microsoft.CodeAnalysis; using Microsoft.CodeAnalysis.Diagnostics; +using Microsoft.CodeAnalysis.Operations; using Microsoft.EntityFrameworkCore.Utilities; namespace Microsoft.EntityFrameworkCore @@ -11,6 +12,8 @@ namespace Microsoft.EntityFrameworkCore [DiagnosticAnalyzer(LanguageNames.CSharp)] public sealed class NullableDiagnosticSuppressor : DiagnosticSuppressor { + private static readonly ImmutableArray _invocationKind = ImmutableArray.Create(OperationKind.Invocation); + private static readonly SuppressionDescriptor _descriptor = new( id: "EFS0001", suppressedDiagnosticId: "CS8602", @@ -26,6 +29,12 @@ public override void ReportSuppressions(SuppressionAnalysisContext context) return; } + var efQueryableExtensionsType = context.Compilation.GetTypeByMetadataName("Microsoft.EntityFrameworkCore.EntityFrameworkQueryableExtensions"); + if (efQueryableExtensionsType is null) + { + return; + } + foreach (var diagnostic in context.ReportedDiagnostics) { if (diagnostic.Location.SourceTree is not SyntaxTree tree) @@ -37,7 +46,14 @@ public override void ReportSuppressions(SuppressionAnalysisContext context) var node = root.FindNode(diagnostic.Location.SourceSpan, getInnermostNodeForTie: true); var model = context.GetSemanticModel(tree); var operation = model.GetOperation(node, context.CancellationToken); - if (operation?.IsWithinExpressionTree(linqExpressionType) == true) + if (operation is null) + { + continue; + } + + if (operation.IsWithinExpressionTree(linqExpressionType, out var anonymousOrLocalFunctionOperation) && + anonymousOrLocalFunctionOperation.GetAncestor(_invocationKind) is IInvocationOperation invocation && + SymbolEqualityComparer.Default.Equals(invocation.TargetMethod.ContainingType, efQueryableExtensionsType)) { context.ReportSuppression(Suppression.Create(_descriptor, diagnostic)); } diff --git a/src/EFCore.Analyzers/Utilities/IOperationExtensions.cs b/src/EFCore.Analyzers/Utilities/IOperationExtensions.cs index 4b7a50e6088..e46da887a53 100644 --- a/src/EFCore.Analyzers/Utilities/IOperationExtensions.cs +++ b/src/EFCore.Analyzers/Utilities/IOperationExtensions.cs @@ -3,19 +3,29 @@ using System; using System.Collections.Immutable; +using System.Diagnostics.CodeAnalysis; using Microsoft.CodeAnalysis; namespace Microsoft.EntityFrameworkCore.Utilities { internal static class IOperationExtensions { - private static readonly ImmutableArray s_LambdaAndLocalFunctionKinds = + private static readonly ImmutableArray lambdaAndLocalFunctionKinds = ImmutableArray.Create(OperationKind.AnonymousFunction, OperationKind.LocalFunction); - public static bool IsWithinExpressionTree(this IOperation operation, INamedTypeSymbol linqExpressionTreeType) - => linqExpressionTreeType != null - && operation.GetAncestor(s_LambdaAndLocalFunctionKinds)?.Parent?.Type?.OriginalDefinition is { } lambdaType - && linqExpressionTreeType.Equals(lambdaType, SymbolEqualityComparer.Default); + public static bool IsWithinExpressionTree(this IOperation operation, INamedTypeSymbol linqExpressionTreeType, [NotNullWhen(true)] out IOperation? anonymousOrLocalFunctionOperation) + { + if (operation.GetAncestor(lambdaAndLocalFunctionKinds) is IOperation op && + op.Parent?.Type?.OriginalDefinition is { } lambdaType && + linqExpressionTreeType.Equals(lambdaType, SymbolEqualityComparer.Default)) + { + anonymousOrLocalFunctionOperation = op; + return true; + } + + anonymousOrLocalFunctionOperation = default; + return false; + } /// /// Gets the first ancestor of this operation with: diff --git a/src/EFCore.Analyzers/Utilities/NotNullWhenAttribute.cs b/src/EFCore.Analyzers/Utilities/NotNullWhenAttribute.cs new file mode 100644 index 00000000000..687ead6735a --- /dev/null +++ b/src/EFCore.Analyzers/Utilities/NotNullWhenAttribute.cs @@ -0,0 +1,20 @@ +// Copyright (c) .NET Foundation. All rights reserved. +// Licensed under the Apache License, Version 2.0. See License.txt in the project root for license information. + +namespace System.Diagnostics.CodeAnalysis +{ + /// Specifies that when a method returns , the parameter will not be null even if the corresponding type allows it. + [AttributeUsage(AttributeTargets.Parameter, Inherited = false)] + internal sealed class NotNullWhenAttribute : Attribute + { + /// Initializes the attribute with the specified return value condition. + /// + /// The return value condition. If the method returns this value, the associated parameter will not be null. + /// + public NotNullWhenAttribute(bool returnValue) => ReturnValue = returnValue; + + /// Gets the return value condition. + public bool ReturnValue { get; } + } + +} diff --git a/test/EFCore.Analyzers.Tests/NullableDiagnosticSuppressorTest.cs b/test/EFCore.Analyzers.Tests/NullableDiagnosticSuppressorTest.cs index 686066515ab..87fb21ac590 100644 --- a/test/EFCore.Analyzers.Tests/NullableDiagnosticSuppressorTest.cs +++ b/test/EFCore.Analyzers.Tests/NullableDiagnosticSuppressorTest.cs @@ -51,5 +51,35 @@ public void M(C? c) }"; await AssertWarningIsNotSuppressedAsync(code); } + + [Fact] + public async Task TestEFCoreIncludeIsSuppressed() + { + var code = @" +#nullable enable + +using System.Linq; +using Microsoft.EntityFrameworkCore; + +class SomeModel +{ + public NavigationModel? Navigation { get; set; } +} + +class NavigationModel +{ + public string DeeperNavigation { get; set; } = null!; +} + +static class C +{ + public static void M(IQueryable q) + { + _ = q.Include(m => m.Navigation).ThenInclude(n => {|CS8602:n|}.DeeperNavigation); + } +} +"; + await AssertWarningIsSuppressedAsync(code); + } } } diff --git a/test/EFCore.Analyzers.Tests/TestUtilities/Verifiers/AdditionalMetadataReferences.cs b/test/EFCore.Analyzers.Tests/TestUtilities/Verifiers/AdditionalMetadataReferences.cs index 4c45e5e3ebc..2798267fdc7 100644 --- a/test/EFCore.Analyzers.Tests/TestUtilities/Verifiers/AdditionalMetadataReferences.cs +++ b/test/EFCore.Analyzers.Tests/TestUtilities/Verifiers/AdditionalMetadataReferences.cs @@ -9,93 +9,13 @@ public static class AdditionalMetadataReferences { public static ReferenceAssemblies Default { get; } = CreateDefaultReferenceAssemblies(); - public static ReferenceAssemblies DefaultWithoutRoslynSymbols { get; } = ReferenceAssemblies.Default - .AddAssemblies(ImmutableArray.Create("System.Xml.Data")) - .AddPackages(ImmutableArray.Create(new PackageIdentity("Microsoft.CodeAnalysis.Workspaces.Common", "3.0.0"))); - - public static ReferenceAssemblies DefaultWithSystemWeb { get; } = ReferenceAssemblies.NetFramework.Net472.Default - .AddAssemblies(ImmutableArray.Create("System.Web", "System.Web.Extensions")); - - public static ReferenceAssemblies DefaultForTaintedDataAnalysis { get; } = ReferenceAssemblies.NetFramework.Net472.Default - .AddAssemblies(ImmutableArray.Create("PresentationFramework", "System.DirectoryServices", "System.Web", "System.Web.Extensions", "System.Xaml")) - .AddPackages(ImmutableArray.Create( - new PackageIdentity("AntiXSS", "4.3.0"), - new PackageIdentity("Microsoft.AspNetCore.Mvc", "2.2.0"), - new PackageIdentity("Microsoft.EntityFrameworkCore.Relational", "2.0.3"))); - - public static ReferenceAssemblies DefaultWithSerialization { get; } = ReferenceAssemblies.NetFramework.Net472.Default - .AddAssemblies(ImmutableArray.Create("System.Runtime.Serialization")); - - public static ReferenceAssemblies DefaultWithAzureStorage { get; } = ReferenceAssemblies.Default - .AddPackages(ImmutableArray.Create(new PackageIdentity("WindowsAzure.Storage", "9.0.0"))); - - public static ReferenceAssemblies DefaultWithNewtonsoftJson10 { get; } = Default - .AddPackages(ImmutableArray.Create(new PackageIdentity("Newtonsoft.Json", "10.0.1"))); - - public static ReferenceAssemblies DefaultWithNewtonsoftJson12 { get; } = Default - .AddPackages(ImmutableArray.Create(new PackageIdentity("Newtonsoft.Json", "12.0.1"))); - - public static ReferenceAssemblies DefaultWithWinForms { get; } = ReferenceAssemblies.NetFramework.Net472.WindowsForms; - - public static ReferenceAssemblies DefaultWithWinHttpHandler { get; } = ReferenceAssemblies.NetStandard.NetStandard20 - .AddPackages(ImmutableArray.Create(new PackageIdentity("System.Net.Http.WinHttpHandler", "4.7.0"))); - - public static ReferenceAssemblies DefaultWithAspNetCoreMvc { get; } = Default - .AddPackages(ImmutableArray.Create( - new PackageIdentity("Microsoft.AspNetCore", "1.1.7"), - new PackageIdentity("Microsoft.AspNetCore.Mvc", "1.1.8"), - new PackageIdentity("Microsoft.AspNetCore.Http", "1.1.2"))); - - public static ReferenceAssemblies DefaultWithNUnit { get; } = Default - .AddPackages(ImmutableArray.Create(new PackageIdentity("NUnit", "3.12.0"))); - - public static ReferenceAssemblies DefaultWithXUnit { get; } = Default - .AddPackages(ImmutableArray.Create(new PackageIdentity("xunit", "2.4.1"))); - - public static ReferenceAssemblies DefaultWithMSTest { get; } = Default - .AddPackages(ImmutableArray.Create(new PackageIdentity("MSTest.TestFramework", "2.1.0"))); - - public static ReferenceAssemblies DefaultWithAsyncInterfaces { get; } = Default - .AddPackages(ImmutableArray.Create(new PackageIdentity("Microsoft.Bcl.AsyncInterfaces", "1.1.0"))); - - public static MetadataReference SystemCollectionsImmutableReference { get; } = MetadataReference.CreateFromFile(typeof(ImmutableHashSet<>).Assembly.Location); - public static MetadataReference SystemComponentModelCompositionReference { get; } = MetadataReference.CreateFromFile(typeof(System.ComponentModel.Composition.ExportAttribute).Assembly.Location); - public static MetadataReference SystemCompositionReference { get; } = MetadataReference.CreateFromFile(typeof(System.Composition.ExportAttribute).Assembly.Location); - public static MetadataReference SystemXmlDataReference { get; } = MetadataReference.CreateFromFile(typeof(System.Data.Rule).Assembly.Location); - public static MetadataReference CodeAnalysisReference { get; } = MetadataReference.CreateFromFile(typeof(Compilation).Assembly.Location); - public static MetadataReference CSharpSymbolsReference { get; } = MetadataReference.CreateFromFile(typeof(CSharpCompilation).Assembly.Location); - public static MetadataReference WorkspacesReference { get; } = MetadataReference.CreateFromFile(typeof(Workspace).Assembly.Location); -#if !NETCOREAPP - public static MetadataReference SystemWebReference { get; } = MetadataReference.CreateFromFile(typeof(System.Web.HttpRequest).Assembly.Location); - public static MetadataReference SystemRuntimeSerialization { get; } = MetadataReference.CreateFromFile(typeof(System.Runtime.Serialization.NetDataContractSerializer).Assembly.Location); -#endif - -#if !NETCOREAPP - public static MetadataReference SystemXaml { get; } = MetadataReference.CreateFromFile(typeof(System.Xaml.XamlReader).Assembly.Location); - public static MetadataReference PresentationFramework { get; } = MetadataReference.CreateFromFile(typeof(System.Windows.Markup.XamlReader).Assembly.Location); - public static MetadataReference SystemWeb { get; } = MetadataReference.CreateFromFile(typeof(System.Web.HttpRequest).Assembly.Location); - public static MetadataReference SystemWebExtensions { get; } = MetadataReference.CreateFromFile(typeof(System.Web.Script.Serialization.JavaScriptSerializer).Assembly.Location); - public static MetadataReference SystemServiceModel { get; } = MetadataReference.CreateFromFile(typeof(System.ServiceModel.OperationContractAttribute).Assembly.Location); -#endif - private static ReferenceAssemblies CreateDefaultReferenceAssemblies() { var referenceAssemblies = ReferenceAssemblies.Default; -#if !NETCOREAPP - referenceAssemblies = referenceAssemblies.AddAssemblies(ImmutableArray.Create("System.Xml.Data")); -#endif - - referenceAssemblies = referenceAssemblies.AddPackages(ImmutableArray.Create(new PackageIdentity("Microsoft.CodeAnalysis", "3.0.0"))); - -#if NETCOREAPP referenceAssemblies = referenceAssemblies.AddPackages(ImmutableArray.Create( - new PackageIdentity("System.Runtime.Serialization.Formatters", "4.3.0"), - new PackageIdentity("System.Configuration.ConfigurationManager", "4.7.0"), - new PackageIdentity("System.Security.Cryptography.Cng", "4.7.0"), - new PackageIdentity("System.Security.Permissions", "4.7.0"), - new PackageIdentity("Microsoft.VisualBasic", "10.3.0"))); -#endif + new PackageIdentity("Microsoft.EntityFrameworkCore", "5.0.3") + )); return referenceAssemblies; } From 5159844073caf8cddb8604503293ea8df9b14b14 Mon Sep 17 00:00:00 2001 From: Youssef Victor Date: Fri, 23 Apr 2021 15:22:45 +0200 Subject: [PATCH 4/5] Apply suggestions from code review Co-authored-by: Shay Rojansky --- eng/Versions.props | 2 +- src/EFCore.Analyzers/NullableDiagnosticSuppressor.cs | 3 +-- src/EFCore.Analyzers/Utilities/IOperationExtensions.cs | 1 - test/EFCore.Analyzers.Tests/EFCore.Analyzers.Tests.csproj | 2 +- 4 files changed, 3 insertions(+), 5 deletions(-) diff --git a/eng/Versions.props b/eng/Versions.props index 4c1f2bcca22..1fd1a369ab0 100644 --- a/eng/Versions.props +++ b/eng/Versions.props @@ -30,6 +30,6 @@ 3.7.0 - 1.0.1-beta1.20623.3 + 1.0.1-beta1.20623.3 diff --git a/src/EFCore.Analyzers/NullableDiagnosticSuppressor.cs b/src/EFCore.Analyzers/NullableDiagnosticSuppressor.cs index 654f2fc8613..1119546fa0d 100644 --- a/src/EFCore.Analyzers/NullableDiagnosticSuppressor.cs +++ b/src/EFCore.Analyzers/NullableDiagnosticSuppressor.cs @@ -17,7 +17,7 @@ public sealed class NullableDiagnosticSuppressor : DiagnosticSuppressor private static readonly SuppressionDescriptor _descriptor = new( id: "EFS0001", suppressedDiagnosticId: "CS8602", - justification: "The dereference is safe inside this expression tree."); + justification: "EF Core provides null-safety in translated expression trees."); public override ImmutableArray SupportedSuppressions => ImmutableArray.Create(_descriptor); @@ -57,7 +57,6 @@ public override void ReportSuppressions(SuppressionAnalysisContext context) { context.ReportSuppression(Suppression.Create(_descriptor, diagnostic)); } - } } } diff --git a/src/EFCore.Analyzers/Utilities/IOperationExtensions.cs b/src/EFCore.Analyzers/Utilities/IOperationExtensions.cs index e46da887a53..3804fab133c 100644 --- a/src/EFCore.Analyzers/Utilities/IOperationExtensions.cs +++ b/src/EFCore.Analyzers/Utilities/IOperationExtensions.cs @@ -60,6 +60,5 @@ public static bool IsWithinExpressionTree(this IOperation operation, INamedTypeS } } - } } diff --git a/test/EFCore.Analyzers.Tests/EFCore.Analyzers.Tests.csproj b/test/EFCore.Analyzers.Tests/EFCore.Analyzers.Tests.csproj index 08e9a7b9e75..40012e45f49 100644 --- a/test/EFCore.Analyzers.Tests/EFCore.Analyzers.Tests.csproj +++ b/test/EFCore.Analyzers.Tests/EFCore.Analyzers.Tests.csproj @@ -14,7 +14,7 @@ - + From b7cf6390a54706dd3a74d09f04c3c15d4cf5498f Mon Sep 17 00:00:00 2001 From: Youssef Victor Date: Fri, 23 Apr 2021 15:25:58 +0200 Subject: [PATCH 5/5] one line check --- src/EFCore.Analyzers/NullableDiagnosticSuppressor.cs | 3 +-- 1 file changed, 1 insertion(+), 2 deletions(-) diff --git a/src/EFCore.Analyzers/NullableDiagnosticSuppressor.cs b/src/EFCore.Analyzers/NullableDiagnosticSuppressor.cs index 1119546fa0d..3a3d5a3d760 100644 --- a/src/EFCore.Analyzers/NullableDiagnosticSuppressor.cs +++ b/src/EFCore.Analyzers/NullableDiagnosticSuppressor.cs @@ -45,8 +45,7 @@ public override void ReportSuppressions(SuppressionAnalysisContext context) var root = tree.GetRoot(context.CancellationToken); var node = root.FindNode(diagnostic.Location.SourceSpan, getInnermostNodeForTie: true); var model = context.GetSemanticModel(tree); - var operation = model.GetOperation(node, context.CancellationToken); - if (operation is null) + if (model.GetOperation(node, context.CancellationToken) is not IOperation operation) { continue; }