Redo pubternalyzer with Roslyn Operations

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' ">

eng/Versions.props

@@ -28,4 +28,7 @@
     <MicrosoftExtensionsHostFactoryResolverSourcesVersion>5.0.0-rc.1.20402.3</MicrosoftExtensionsHostFactoryResolverSourcesVersion>
     <MicrosoftExtensionsLoggingVersion>5.0.0-rc.1.20402.3</MicrosoftExtensionsLoggingVersion>
   </PropertyGroup>
+  <PropertyGroup Label="Other dependencies">
+    <MicrosoftCodeAnalysisVersion>3.7.0</MicrosoftCodeAnalysisVersion>
+  </PropertyGroup>
 </Project>

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)

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>

src/EFCore.Cosmos/Query/Internal/CosmosShapedQueryCompilingExpressionVisitor.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(

src/EFCore.Cosmos/Query/Internal/CosmosShapedQueryCompilingExpressionVisitor.ReadItemQueryingEnumerable.cs

@@ -254,28 +254,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.
 

src/EFCore.Relational/Migrations/Internal/MigrationsModelDiffer.cs

@@ -88,7 +88,9 @@ public MigrationsModelDiffer(
         {
             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 +139,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

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>
 

test/EFCore.Analyzers.Tests/InternalUsageDiagnosticAnalyzerTest.cs

@@ -15,74 +15,142 @@ public class InternalUsageDiagnosticAnalyzerTest : DiagnosticAnalyzerTestBase
         protected override DiagnosticAnalyzer CreateDiagnosticAnalyzer() => new InternalUsageDiagnosticAnalyzer();
 
         [ConditionalFact]
-        public async Task No_warning_on_ef_non_internal()
-            => await AssertNoDiagnostics(
-                @"
-var a = new Microsoft.EntityFrameworkCore.Infrastructure.Annotatable();
-var x = a.GetAnnotations();
-");
+        public Task Invocation_on_type_in_internal_namespace()
+            => Test(
+                "var x = typeof(object).GetMethod(nameof(object.ToString), Type.EmptyTypes).DisplayName();",
+                "Microsoft.EntityFrameworkCore.Internal.MethodInfoExtensions",
+                "DisplayName");
 
-        #region Namespace
+        [ConditionalFact]
+        public Task Instantiation_on_type_in_internal_namespace()
+            => Test(
+                "new CoreSingletonOptions();",
+                "Microsoft.EntityFrameworkCore.Internal.CoreSingletonOptions",
+                "CoreSingletonOptions");
 
         [ConditionalFact]
-        public async Task Warning_on_ef_internal_namespace_invocation()
+        public async Task Base_type()
         {
-            var (diagnostics, source) = await GetDiagnosticsAsync(
-                @"var x = typeof(object).GetMethod(nameof(object.ToString), Type.EmptyTypes).DisplayName();");
-            var diagnostic = diagnostics.Single();
+            var source = @"
+class MyClass : Microsoft.EntityFrameworkCore.Storage.Internal.RawRelationalParameter {
+    MyClass() : base(null, null) {}
+}";
 
-            Assert.Equal(InternalUsageDiagnosticAnalyzer.Id, diagnostic.Id);
-            Assert.Equal(DiagnosticSeverity.Warning, diagnostic.Severity);
-            Assert.Equal(
-                string.Format(InternalUsageDiagnosticAnalyzer.MessageFormat, "Microsoft.EntityFrameworkCore.Internal.MethodInfoExtensions"),
-                diagnostic.GetMessage());
+            var diagnostics = await GetDiagnosticsFullSourceAsync(source);
 
-            var span = diagnostic.Location.SourceSpan;
-            Assert.Equal("DisplayName", source[span.Start..span.End]);
+            Assert.Collection(diagnostics,
+                diagnostic =>
+                {
+                    Assert.Equal(InternalUsageDiagnosticAnalyzer.Id, diagnostic.Id);
+                    Assert.Equal(DiagnosticSeverity.Warning, diagnostic.Severity);
+                    Assert.Equal(
+                        string.Format(
+                            InternalUsageDiagnosticAnalyzer.MessageFormat, "Microsoft.EntityFrameworkCore.Storage.Internal.RawRelationalParameter"),
+                        diagnostic.GetMessage());
+
+                    var span = diagnostic.Location.SourceSpan;
+                    Assert.Equal(
+                        "Microsoft.EntityFrameworkCore.Storage.Internal.RawRelationalParameter",
+                        source[span.Start..span.End]);
+                },
+                diagnostic =>
+                {
+                    Assert.Equal(InternalUsageDiagnosticAnalyzer.Id, diagnostic.Id);
+                    Assert.Equal(DiagnosticSeverity.Warning, diagnostic.Severity);
+                    Assert.Equal(
+                        string.Format(
+                            InternalUsageDiagnosticAnalyzer.MessageFormat, "Microsoft.EntityFrameworkCore.Storage.Internal.RawRelationalParameter"),
+                        diagnostic.GetMessage());
+
+                    var span = diagnostic.Location.SourceSpan;
+                    Assert.Equal(": base(null, null)", source[span.Start..span.End]);
+                });
         }
 
         [ConditionalFact]
-        public async Task Warning_on_ef_internal_namespace_instantiation()
-        {
-            var (diagnostics, source) = await GetDiagnosticsAsync(@"new CoreSingletonOptions();");
-            var diagnostic = diagnostics.Single();
+        public Task Implemented_interface()
+            => TestFullSource(
+                @"
+using System.Threading;
+using System.Threading.Tasks;
+using Microsoft.EntityFrameworkCore.Internal;
 
-            Assert.Equal(InternalUsageDiagnosticAnalyzer.Id, diagnostic.Id);
-            Assert.Equal(DiagnosticSeverity.Warning, diagnostic.Severity);
-            Assert.Equal(
-                string.Format(InternalUsageDiagnosticAnalyzer.MessageFormat, "Microsoft.EntityFrameworkCore.Internal.CoreSingletonOptions"),
-                diagnostic.GetMessage());
+class MyClass : IDbContextPool {
+    public IDbContextPoolable Rent() => default;
+    public void Return(IDbContextPoolable context) {}
+    public ValueTask ReturnAsync(IDbContextPoolable context, CancellationToken cancellationToken = default) => default;
+}",
+                "Microsoft.EntityFrameworkCore.Internal.IDbContextPool",
+                "MyClass");
 
-            var span = diagnostic.Location.SourceSpan;
-            Assert.Equal("CoreSingletonOptions", source[span.Start..span.End]);
-        }
+        [ConditionalFact]
+        public Task Access_property_with_internal_attribute()
+            => Test(
+                "var x = Microsoft.EntityFrameworkCore.Infrastructure.EntityFrameworkRelationalServicesBuilder.RelationalServices.Count;",
+                "Microsoft.EntityFrameworkCore.Infrastructure.EntityFrameworkRelationalServicesBuilder.RelationalServices",
+                "RelationalServices");
 
         [ConditionalFact]
-        public async Task Warning_on_ef_internal_namespace_subclass()
-        {
-            var source = @"
-class MyClass : Microsoft.EntityFrameworkCore.Storage.Internal.RawRelationalParameter {
-    MyClass() : base (null, null) {}
-}";
+        public Task Instantiation_with_ctor_with_internal_attribute()
+            => Test(
+                "new Microsoft.EntityFrameworkCore.Update.UpdateSqlGeneratorDependencies(null, null);",
+                "Microsoft.EntityFrameworkCore.Update.UpdateSqlGeneratorDependencies",
+                "Microsoft.EntityFrameworkCore.Update.UpdateSqlGeneratorDependencies");
 
-            var diagnostics = await GetDiagnosticsFullSourceAsync(source);
-            var diagnostic = diagnostics.Single();
+        [ConditionalFact]
+        public Task Local_variable_declaration()
+            => Test(
+                "Microsoft.EntityFrameworkCore.ChangeTracking.Internal.IStateManager state = null;",
+                "Microsoft.EntityFrameworkCore.ChangeTracking.Internal.IStateManager",
+                "Microsoft.EntityFrameworkCore.ChangeTracking.Internal.IStateManager");
 
-            Assert.Equal(InternalUsageDiagnosticAnalyzer.Id, diagnostic.Id);
-            Assert.Equal(DiagnosticSeverity.Warning, diagnostic.Severity);
-            Assert.Equal(
-                string.Format(
-                    InternalUsageDiagnosticAnalyzer.MessageFormat, "Microsoft.EntityFrameworkCore.Storage.Internal.RawRelationalParameter"),
-                diagnostic.GetMessage());
+        [ConditionalFact]
+        public Task Generic_type_parameter_in_method_call()
+            => Test(
+                @"
+void SomeGenericMethod<T>() {}
 
-            var span = diagnostic.Location.SourceSpan;
-            Assert.Equal(
-                "Microsoft.EntityFrameworkCore.Storage.Internal.RawRelationalParameter",
-                source[span.Start..span.End]);
-        }
+SomeGenericMethod<Microsoft.EntityFrameworkCore.ChangeTracking.Internal.IStateManager>();",
+                "Microsoft.EntityFrameworkCore.ChangeTracking.Internal.IStateManager",
+                "SomeGenericMethod<Microsoft.EntityFrameworkCore.ChangeTracking.Internal.IStateManager>()");
+
+        [ConditionalFact]
+        public Task Typeof()
+            => Test(
+                "var t = typeof(Microsoft.EntityFrameworkCore.ChangeTracking.Internal.IStateManager);",
+                "Microsoft.EntityFrameworkCore.ChangeTracking.Internal.IStateManager",
+                "Microsoft.EntityFrameworkCore.ChangeTracking.Internal.IStateManager");
+
+        [ConditionalFact]
+        public Task Field_declaration()
+            => TestFullSource(
+                @"
+class MyClass {
+    private readonly Microsoft.EntityFrameworkCore.ChangeTracking.Internal.IStateManager StateManager;
+}",
+                "Microsoft.EntityFrameworkCore.ChangeTracking.Internal.IStateManager",
+                "Microsoft.EntityFrameworkCore.ChangeTracking.Internal.IStateManager");
+
+        [ConditionalFact]
+        public Task Property_declaration()
+            => TestFullSource(
+                @"
+class MyClass {
+    private Microsoft.EntityFrameworkCore.ChangeTracking.Internal.IStateManager StateManager { get; set; }
+}",
+                "Microsoft.EntityFrameworkCore.ChangeTracking.Internal.IStateManager",
+                "Microsoft.EntityFrameworkCore.ChangeTracking.Internal.IStateManager");
 
         [ConditionalFact]
-        public async Task No_warning_on_ef_internal_namespace_in_same_assembly()
+        public async Task No_warning_on_non_internal()
+            => await AssertNoDiagnostics(
+                @"
+var a = new Microsoft.EntityFrameworkCore.Infrastructure.Annotatable();
+var x = a.GetAnnotations();
+");
+
+        [ConditionalFact]
+        public async Task No_warning_in_same_assembly()
         {
             var diagnostics = await GetDiagnosticsFullSourceAsync(
                 @"
@@ -107,51 +175,42 @@ public void Main(string[] args) {
             Assert.Empty(diagnostics);
         }
 
-        #endregion Namespace
-
-        #region Attribute
-
-        [ConditionalFact]
-        public async Task Warning_on_ef_internal_attribute_property_access()
+        private async Task Test(
+            string source,
+            string expectedInternalApi,
+            string expectedDiagnosticSpan)
         {
-            var (diagnostics, source) = await GetDiagnosticsAsync(
-                @"var x = Microsoft.EntityFrameworkCore.Infrastructure.EntityFrameworkRelationalServicesBuilder.RelationalServices.Count;");
-            //throw new Exception("FOO: " + string.Join(", ", diagnostics.Select(d => d.ToString())));
-            var diagnostic = diagnostics.Single();
+            var (diagnostics, fullSource) = await GetDiagnosticsAsync(source);
+            var diagnostic = Assert.Single(diagnostics);
 
             Assert.Equal(InternalUsageDiagnosticAnalyzer.Id, diagnostic.Id);
             Assert.Equal(DiagnosticSeverity.Warning, diagnostic.Severity);
             Assert.Equal(
-                string.Format(
-                    InternalUsageDiagnosticAnalyzer.MessageFormat,
-                    "Microsoft.EntityFrameworkCore.Infrastructure.EntityFrameworkRelationalServicesBuilder.RelationalServices"),
+                string.Format(InternalUsageDiagnosticAnalyzer.MessageFormat, expectedInternalApi),
                 diagnostic.GetMessage());
 
             var span = diagnostic.Location.SourceSpan;
-            Assert.Equal("RelationalServices", source[span.Start..span.End]);
+            Assert.Equal(expectedDiagnosticSpan, fullSource[span.Start..span.End]);
         }
 
-        [ConditionalFact]
-        public async Task Warning_on_ef_internal_name_instantiation()
+        private async Task TestFullSource(
+            string fullSource,
+            string expectedInternalApi,
+            string expectedDiagnosticSpan)
         {
-            var (diagnostics, source) =
-                await GetDiagnosticsAsync(@"new Microsoft.EntityFrameworkCore.Update.UpdateSqlGeneratorDependencies(null, null);");
-            var diagnostic = diagnostics.Single();
+            var diagnostics = await GetDiagnosticsFullSourceAsync(fullSource);
+            var diagnostic = Assert.Single(diagnostics);
 
             Assert.Equal(InternalUsageDiagnosticAnalyzer.Id, diagnostic.Id);
             Assert.Equal(DiagnosticSeverity.Warning, diagnostic.Severity);
             Assert.Equal(
-                string.Format(
-                    InternalUsageDiagnosticAnalyzer.MessageFormat, "Microsoft.EntityFrameworkCore.Update.UpdateSqlGeneratorDependencies"),
+                string.Format(InternalUsageDiagnosticAnalyzer.MessageFormat, expectedInternalApi),
                 diagnostic.GetMessage());
 
             var span = diagnostic.Location.SourceSpan;
-            Assert.Equal(
-                "new Microsoft.EntityFrameworkCore.Update.UpdateSqlGeneratorDependencies(null, null)", source[span.Start..span.End]);
+            Assert.Equal(expectedDiagnosticSpan, fullSource[span.Start..span.End]);
         }
 
-        #endregion Attribute
-
         protected override Task<(Diagnostic[], string)> GetDiagnosticsAsync(string source, params string[] extraUsings)
             => base.GetDiagnosticsAsync(source, extraUsings.Concat(new[] { "Microsoft.EntityFrameworkCore.Internal" }).ToArray());
     }