Redo pubternalyzer with Roslyn Operations (#21996)

* Rewrite most of the analyzer using Operations, for much better perf and VB.NET support. * Support some extra language scenarios which weren't being detected. Fixes #19648 Fixes #20206
dotnet · Aug 16, 2020 · bc68329 · bc68329
1 parent 1dc1788
commit bc68329
Show file tree

Hide file tree

Showing 16 changed files with 470 additions and 145 deletions.
diff --git a/benchmark/EFCore.Benchmarks/EFCore.Benchmarks.csproj b/benchmark/EFCore.Benchmarks/EFCore.Benchmarks.csproj
@@ -12,7 +12,7 @@
 
   <ItemGroup Condition=" '$(Configuration)' == 'Release' Or '$(Configuration)' == 'Debug' ">
     <ProjectReference Include="..\..\src\EFCore.Relational\EFCore.Relational.csproj" />
-    <PackageReference Include="Microsoft.CodeAnalysis.CSharp.Workspaces" Version="3.3.1" />
+    <PackageReference Include="Microsoft.CodeAnalysis.CSharp.Workspaces" Version="$(MicrosoftCodeAnalysisVersion)" />
   </ItemGroup>
 
   <ItemGroup Condition=" '$(Configuration)' == 'Release22' Or '$(Configuration)' == 'Debug22' ">

diff --git a/eng/Versions.props b/eng/Versions.props
@@ -28,4 +28,7 @@
     <MicrosoftExtensionsHostFactoryResolverSourcesVersion>5.0.0-rc.1.20409.3</MicrosoftExtensionsHostFactoryResolverSourcesVersion>
     <MicrosoftExtensionsLoggingVersion>5.0.0-rc.1.20409.3</MicrosoftExtensionsLoggingVersion>
   </PropertyGroup>
+  <PropertyGroup Label="Other dependencies">
+    <MicrosoftCodeAnalysisVersion>3.7.0</MicrosoftCodeAnalysisVersion>
+  </PropertyGroup>
 </Project>
diff --git a/src/EFCore.Analyzers/AnalyzerReleases.Shipped.md b/src/EFCore.Analyzers/AnalyzerReleases.Shipped.md
@@ -0,0 +1,18 @@
+## Release 2.1.0
+
+### New Rules
+Rule ID | Category | Severity | Notes
+--------|----------|----------|-------
+EF1000  | Usage    | Warning  | RawSqlStringInjectionDiagnosticAnalyzer, [Documentation](https://docs.microsoft.com/ef/core/querying/raw-sql)
+
+## Release 3.0.0
+
+### New Rules
+Rule ID | Category | Severity | Notes
+--------|----------|----------|-------
+EF1001  | Usage    | Warning  | InternalUsageDiagnosticAnalyzer
+
+### Removed Rules
+Rule ID | Category | Severity | Notes
+--------|----------|----------|--------------------
+EF1000  | Security | Disabled | RawSqlStringInjectionDiagnosticAnalyzer, [Documentation](https://docs.microsoft.com/ef/core/querying/raw-sql)
diff --git a/src/EFCore.Analyzers/AnalyzerReleases.Unshipped.md b/src/EFCore.Analyzers/AnalyzerReleases.Unshipped.md
diff --git a/src/EFCore.Analyzers/EFCore.Analyzers.csproj b/src/EFCore.Analyzers/EFCore.Analyzers.csproj
@@ -15,7 +15,8 @@
 
   <ItemGroup>
     <!-- NB: Version affects the minimum required Visual Studio version -->
-    <PackageReference Include="Microsoft.CodeAnalysis.CSharp.Workspaces" Version="3.3.1" PrivateAssets="all" />
+    <PackageReference Include="Microsoft.CodeAnalysis.CSharp" Version="$(MicrosoftCodeAnalysisVersion)" PrivateAssets="all" />
+    <PackageReference Include="Microsoft.CodeAnalysis.VisualBasic" Version="$(MicrosoftCodeAnalysisVersion)" PrivateAssets="all" />
     <PackageReference Update="NETStandard.Library" PrivateAssets="all" />
   </ItemGroup>
 
@@ -32,4 +33,9 @@
     </ItemGroup>
   </Target>
 
+  <ItemGroup>
+    <AdditionalFiles Include="AnalyzerReleases.Shipped.md" />
+    <AdditionalFiles Include="AnalyzerReleases.Unshipped.md" />
+  </ItemGroup>
+
 </Project>
diff --git a/src/EFCore.Analyzers/InternalUsageDiagnosticAnalyzer.cs b/src/EFCore.Analyzers/InternalUsageDiagnosticAnalyzer.cs
diff --git a/...edQueryCompilingExpressionVisitor.CosmosProjectionBindingRemovingExpressionVisitorBase.cs b/...edQueryCompilingExpressionVisitor.CosmosProjectionBindingRemovingExpressionVisitorBase.cs
@@ -365,9 +365,12 @@ private void AddInclude(
                 var includeMethod = navigation.IsCollection ? _includeCollectionMethodInfo : _includeReferenceMethodInfo;
                 var includingClrType = navigation.DeclaringEntityType.ClrType;
                 var relatedEntityClrType = navigation.TargetEntityType.ClrType;
+#pragma warning disable EF1001 // Internal EF Core API usage.
                 var entityEntryVariable = _trackQueryResults
                     ? shaperBlock.Variables.Single(v => v.Type == typeof(InternalEntityEntry))
                     : (Expression)Expression.Constant(null, typeof(InternalEntityEntry));
+#pragma warning restore EF1001 // Internal EF Core API usage.
+
                 var concreteEntityTypeVariable = shaperBlock.Variables.Single(v => v.Type == typeof(IEntityType));
                 var inverseNavigation = navigation.Inverse;
                 var fixup = GenerateFixup(
@@ -394,7 +397,9 @@ private static readonly MethodInfo _includeReferenceMethodInfo
                     .GetDeclaredMethod(nameof(IncludeReference));
 
             private static void IncludeReference<TIncludingEntity, TIncludedEntity>(
+#pragma warning disable EF1001 // Internal EF Core API usage.
                 InternalEntityEntry entry,
+#pragma warning restore EF1001 // Internal EF Core API usage.
                 object entity,
                 IEntityType entityType,
                 TIncludedEntity relatedEntity,
@@ -437,7 +442,9 @@ private static readonly MethodInfo _includeCollectionMethodInfo
                     .GetDeclaredMethod(nameof(IncludeCollection));
 
             private static void IncludeCollection<TIncludingEntity, TIncludedEntity>(
+#pragma warning disable EF1001 // Internal EF Core API usage.
                 InternalEntityEntry entry,
+#pragma warning restore EF1001 // Internal EF Core API usage.
                 object entity,
                 IEntityType entityType,
                 IEnumerable<TIncludedEntity> relatedEntities,

diff --git a/.../Query/Internal/CosmosShapedQueryCompilingExpressionVisitor.ReadItemQueryingEnumerable.cs b/.../Query/Internal/CosmosShapedQueryCompilingExpressionVisitor.ReadItemQueryingEnumerable.cs
@@ -259,28 +259,28 @@ private bool TryGenerateIdFromKeys(IProperty idProperty, out object value)
                 {
                     var entityEntry = Activator.CreateInstance(_readItemExpression.EntityType.ClrType);
 
-#pragma warning disable EF1001
+#pragma warning disable EF1001 // Internal EF Core API usage.
                     var internalEntityEntry = new InternalEntityEntryFactory().Create(
                         _cosmosQueryContext.Context.GetDependencies().StateManager, _readItemExpression.EntityType, entityEntry);
-#pragma warning restore EF1001
+#pragma warning restore EF1001 // Internal EF Core API usage.
 
                     foreach (var keyProperty in _readItemExpression.EntityType.FindPrimaryKey().Properties)
                     {
                         var property = _readItemExpression.EntityType.FindProperty(keyProperty.Name);
 
                         if (TryGetParameterValue(property, out var parameterValue))
                         {
+#pragma warning disable EF1001 // Internal EF Core API usage.
                             internalEntityEntry[property] = parameterValue;
+#pragma warning restore EF1001 // Internal EF Core API usage.
                         }
                     }
 
 #pragma warning disable EF1001 // Internal EF Core API usage.
                     internalEntityEntry.SetEntityState(EntityState.Added);
-#pragma warning restore EF1001 // Internal EF Core API usage.
 
                     value = internalEntityEntry[idProperty];
 
-#pragma warning disable EF1001 // Internal EF Core API usage.
                     internalEntityEntry.SetEntityState(EntityState.Detached);
 #pragma warning restore EF1001 // Internal EF Core API usage.
 

diff --git a/src/EFCore.Relational/Migrations/Internal/MigrationsModelDiffer.cs b/src/EFCore.Relational/Migrations/Internal/MigrationsModelDiffer.cs
@@ -82,13 +82,17 @@ public class MigrationsModelDiffer : IMigrationsModelDiffer
         public MigrationsModelDiffer(
             [NotNull] IRelationalTypeMappingSource typeMappingSource,
             [NotNull] IMigrationsAnnotationProvider migrationsAnnotations,
+#pragma warning disable EF1001 // Internal EF Core API usage.
             [NotNull] IChangeDetector changeDetector,
+#pragma warning restore EF1001 // Internal EF Core API usage.
             [NotNull] IUpdateAdapterFactory updateAdapterFactory,
             [NotNull] CommandBatchPreparerDependencies commandBatchPreparerDependencies)
         {
             Check.NotNull(typeMappingSource, nameof(typeMappingSource));
             Check.NotNull(migrationsAnnotations, nameof(migrationsAnnotations));
+#pragma warning disable EF1001 // Internal EF Core API usage.
             Check.NotNull(changeDetector, nameof(changeDetector));
+#pragma warning restore EF1001 // Internal EF Core API usage.
             Check.NotNull(updateAdapterFactory, nameof(updateAdapterFactory));
             Check.NotNull(commandBatchPreparerDependencies, nameof(commandBatchPreparerDependencies));
 
@@ -137,7 +141,9 @@ public MigrationsModelDiffer(
         ///     any release. You should only use it directly in your code with extreme caution and knowing that
         ///     doing so can result in application failures when updating to a new Entity Framework Core release.
         /// </summary>
+#pragma warning disable EF1001 // Internal EF Core API usage.
         protected virtual IChangeDetector ChangeDetector { get; }
+#pragma warning restore EF1001 // Internal EF Core API usage.
 
         /// <summary>
         ///     This is an internal API that supports the Entity Framework Core infrastructure and not subject to

diff --git a/src/EFCore.SqlServer/Metadata/Internal/SqlServerAnnotationProvider.cs b/src/EFCore.SqlServer/Metadata/Internal/SqlServerAnnotationProvider.cs
@@ -31,7 +31,9 @@ public class SqlServerAnnotationProvider : RelationalAnnotationProvider
         ///     Initializes a new instance of this class.
         /// </summary>
         /// <param name="dependencies"> Parameter object containing dependencies for this service. </param>
+#pragma warning disable EF1001 // Internal EF Core API usage.
         public SqlServerAnnotationProvider([NotNull] RelationalAnnotationProviderDependencies dependencies)
+#pragma warning restore EF1001 // Internal EF Core API usage.
             : base(dependencies)
         {
         }

diff --git a/src/EFCore.SqlServer/Migrations/Internal/SqlServerMigrationsAnnotationProvider.cs b/src/EFCore.SqlServer/Migrations/Internal/SqlServerMigrationsAnnotationProvider.cs
@@ -31,7 +31,9 @@ public class SqlServerMigrationsAnnotationProvider : MigrationsAnnotationProvide
         ///     Initializes a new instance of this class.
         /// </summary>
         /// <param name="dependencies"> Parameter object containing dependencies for this service. </param>
+#pragma warning disable EF1001 // Internal EF Core API usage.
         public SqlServerMigrationsAnnotationProvider([NotNull] MigrationsAnnotationProviderDependencies dependencies)
+#pragma warning restore EF1001 // Internal EF Core API usage.
             : base(dependencies)
         {
         }

diff --git a/src/EFCore.Sqlite.Core/Metadata/Internal/SqliteAnnotationProvider.cs b/src/EFCore.Sqlite.Core/Metadata/Internal/SqliteAnnotationProvider.cs
@@ -33,7 +33,9 @@ public class SqliteAnnotationProvider : RelationalAnnotationProvider
         ///     any release. You should only use it directly in your code with extreme caution and knowing that
         ///     doing so can result in application failures when updating to a new Entity Framework Core release.
         /// </summary>
+#pragma warning disable EF1001 // Internal EF Core API usage.
         public SqliteAnnotationProvider([NotNull] RelationalAnnotationProviderDependencies dependencies)
+#pragma warning restore EF1001 // Internal EF Core API usage.
             : base(dependencies)
         {
         }

diff --git a/test/EFCore.Analyzers.Tests/EFCore.Analyzers.Test.csproj b/test/EFCore.Analyzers.Tests/EFCore.Analyzers.Test.csproj
@@ -11,7 +11,8 @@
     <ProjectReference Include="..\..\src\EFCore.Analyzers\EFCore.Analyzers.csproj" />
     <ProjectReference Include="..\..\src\EFCore.Relational\EFCore.Relational.csproj" />
     <ProjectReference Include="..\EFCore.Tests\EFCore.Tests.csproj" />
-    <PackageReference Include="Microsoft.CodeAnalysis.CSharp.Workspaces" Version="3.3.1" />
+    <PackageReference Include="Microsoft.CodeAnalysis.CSharp.Workspaces" Version="$(MicrosoftCodeAnalysisVersion)" />
+    <PackageReference Include="Microsoft.CodeAnalysis.VisualBasic" Version="$(MicrosoftCodeAnalysisVersion)" />
     <PackageReference Include="Microsoft.Extensions.DependencyModel" Version="$(MicrosoftExtensionsDependencyModelVersion)" />
   </ItemGroup>