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.Analyzers/InternalUsageDiagnosticAnalyzer.cs

@@ -5,21 +5,23 @@
 using System.Collections.Immutable;
 using System.Linq;
 using Microsoft.CodeAnalysis;
-using Microsoft.CodeAnalysis.CSharp;
-using Microsoft.CodeAnalysis.CSharp.Syntax;
 using Microsoft.CodeAnalysis.Diagnostics;
+using Microsoft.CodeAnalysis.Operations;
+
+using CSharpSyntax = Microsoft.CodeAnalysis.CSharp.Syntax;
+using VBSyntax = Microsoft.CodeAnalysis.VisualBasic.Syntax;
 
 namespace Microsoft.EntityFrameworkCore
 {
-    [DiagnosticAnalyzer(LanguageNames.CSharp)]
+    [DiagnosticAnalyzer(LanguageNames.CSharp, LanguageNames.VisualBasic)]
     public class InternalUsageDiagnosticAnalyzer : DiagnosticAnalyzer
     {
         public const string Id = "EF1001";
 
         public const string MessageFormat
-            = "{0} is an internal API that supports the Entity Framework Core infrastructure and " +
-              "not subject to the same compatibility standards as public APIs. " +
-              "It may be changed or removed without notice in any release.";
+            = "{0} is an internal API that supports the Entity Framework Core infrastructure and "
+            + "not subject to the same compatibility standards as public APIs. "
+            + "It may be changed or removed without notice in any release.";
 
         protected const string DefaultTitle = "Internal EF Core API usage.";
         protected const string Category = "Usage";
@@ -40,84 +42,244 @@ private static readonly DiagnosticDescriptor _descriptor
         public override void Initialize(AnalysisContext context)
         {
             context.EnableConcurrentExecution();
-            context.RegisterSyntaxNodeAction(
-                AnalyzeNode,
-                SyntaxKind.SimpleMemberAccessExpression,
-                SyntaxKind.ObjectCreationExpression,
-                SyntaxKind.ClassDeclaration);
             context.ConfigureGeneratedCodeAnalysis(GeneratedCodeAnalysisFlags.None);
+
+            context.RegisterOperationAction(AnalyzeNode,
+                OperationKind.FieldReference,
+                OperationKind.PropertyReference,
+                OperationKind.MethodReference,
+                OperationKind.EventReference,
+                OperationKind.Invocation,
+                OperationKind.ObjectCreation,
+                OperationKind.VariableDeclaration,
+                OperationKind.TypeOf);
+
+            context.RegisterSymbolAction(AnalyzeSymbol, SymbolKind.NamedType, SymbolKind.Property, SymbolKind.Field, SymbolKind.Event);
         }
 
-        private void AnalyzeNode(SyntaxNodeAnalysisContext context)
+        private static void AnalyzeNode(OperationAnalysisContext context)
         {
-            switch (context.Node)
+            switch (context.Operation.Kind)
             {
-                case MemberAccessExpressionSyntax memberAccessSyntax:
-                {
-                    if (context.SemanticModel.GetSymbolInfo(context.Node, context.CancellationToken).Symbol is ISymbol symbol
-                        && !Equals(symbol.ContainingAssembly, context.Compilation.Assembly))
+                case OperationKind.FieldReference:
+                    AnalyzeMember(context, ((IFieldReferenceOperation)context.Operation).Field);
+                    break;
+                case OperationKind.PropertyReference:
+                    AnalyzeMember(context, ((IPropertyReferenceOperation)context.Operation).Property);
+                    break;
+                case OperationKind.EventReference:
+                    AnalyzeMember(context, ((IEventReferenceOperation)context.Operation).Event);
+                    break;
+                case OperationKind.MethodReference:
+                    AnalyzeMember(context, ((IMethodReferenceOperation)context.Operation).Method);
+                    break;
+                case OperationKind.ObjectCreation:
+                    AnalyzeMember(context, ((IObjectCreationOperation)context.Operation).Constructor);
+                    break;
+                case OperationKind.Invocation:
+                    AnalyzeInvocation(context, (IInvocationOperation)context.Operation);
+                    break;
+                case OperationKind.VariableDeclaration:
+                    AnalyzeVariableDeclaration(context, ((IVariableDeclarationOperation)context.Operation));
+                    break;
+                case OperationKind.TypeOf:
+                    AnalyzeTypeof(context, ((ITypeOfOperation)context.Operation));
+                    break;
+                default:
+                    throw new ArgumentException($"Unexpected {nameof(OperationKind)}: {context.Operation.Kind}");
+            }
+
+        }
+
+        private static void AnalyzeMember(OperationAnalysisContext context, ISymbol symbol)
+        {
+            if ((object)symbol.ContainingAssembly == context.Compilation.Assembly)
+            {
+                // Skip all methods inside the same assembly - internal access is fine
+                return;
+            }
+
+            var containingType = symbol.ContainingType;
+
+            switch (symbol)
+            {
+                case IMethodSymbol _:
+                case IFieldSymbol _:
+                case IPropertySymbol _:
+                case IEventSymbol _:
+                    if (HasInternalAttribute(symbol))
                     {
-                        var containingType = symbol.ContainingType;
-
-                        if (HasInternalAttribute(symbol))
-                        {
-                            context.ReportDiagnostic(
-                                Diagnostic.Create(_descriptor, memberAccessSyntax.Name.GetLocation(), $"{containingType}.{symbol.Name}"));
-                            return;
-                        }
-
-                        if (IsInInternalNamespace(containingType)
-                            || HasInternalAttribute(containingType))
-                        {
-                            context.ReportDiagnostic(Diagnostic.Create(_descriptor, memberAccessSyntax.Name.GetLocation(), containingType));
-                            return;
-                        }
+                        ReportDiagnostic(context, symbol.Name == ".ctor" ? (object)containingType : $"{containingType}.{symbol.Name}");
+                        return;
                     }
+                    break;
+            }
 
+            if (IsInternal(context, containingType))
+            {
+                ReportDiagnostic(context, containingType);
+            }
+        }
+
+        private static void AnalyzeInvocation(OperationAnalysisContext context, IInvocationOperation invocation)
+        {
+            // First check for any internal type parameters
+            foreach (var a in invocation.TargetMethod.TypeArguments)
+            {
+                if (IsInternal(context, a))
+                {
+                    context.ReportDiagnostic(Diagnostic.Create(_descriptor, context.Operation.Syntax.GetLocation(), a));
+                }
+            }
+
+            // Then check the method being invoked
+            AnalyzeMember(context, invocation.TargetMethod);
+        }
+
+        private static void AnalyzeVariableDeclaration(OperationAnalysisContext context, IVariableDeclarationOperation variableDeclaration)
+        {
+            foreach (var declarator in variableDeclaration.Declarators)
+            {
+                if (IsInternal(context, declarator.Symbol.Type))
+                {
+                    var syntax = context.Operation.Syntax switch
+                    {
+                        CSharpSyntax.VariableDeclarationSyntax s => s.Type,
+                        _ => context.Operation.Syntax
+                    };
+                    context.ReportDiagnostic(Diagnostic.Create(_descriptor, syntax.GetLocation(), declarator.Symbol.Type));
                     return;
                 }
+            }
+        }
+
+        private static void AnalyzeTypeof(OperationAnalysisContext context, ITypeOfOperation typeOf)
+        {
+            if (IsInternal(context, typeOf.TypeOperand))
+            {
+                ReportDiagnostic(context, typeOf.TypeOperand);
+            }
+        }
+
+        private static void AnalyzeSymbol(SymbolAnalysisContext context)
+        {
+            switch (context.Symbol)
+            {
+                case INamedTypeSymbol symbol:
+                    AnalyzeNamedTypeSymbol(context, symbol);
+                    break;
+
+                case IFieldSymbol symbol:
+                    AnalyzeMemberDeclarationTypeSymbol(context, symbol, symbol.Type);
+                    break;
+
+                case IPropertySymbol symbol:
+                    AnalyzeMemberDeclarationTypeSymbol(context, symbol, symbol.Type);
+                    break;
+
+                case IEventSymbol symbol:
+                    AnalyzeMemberDeclarationTypeSymbol(context, symbol, symbol.Type);
+                    break;
 
-                case ObjectCreationExpressionSyntax creationSyntax:
+                default:
+                    throw new ArgumentException($"Unexpected {nameof(ISymbol)}: {context.Symbol.GetType().Name}");
+            }
+        }
+
+        private static void AnalyzeNamedTypeSymbol(SymbolAnalysisContext context, INamedTypeSymbol symbol)
+        {
+            if (symbol.BaseType is ITypeSymbol baseSymbol
+                && IsInternal(context, baseSymbol))
+            {
+                foreach (var declaringSyntax in symbol.DeclaringSyntaxReferences)
                 {
-                    if (context.SemanticModel.GetSymbolInfo(context.Node, context.CancellationToken).Symbol is ISymbol symbol
-                        && !Equals(symbol.ContainingAssembly, context.Compilation.Assembly))
+                    var location = declaringSyntax.GetSyntax() switch
                     {
-                        var containingType = symbol.ContainingType;
-
-                        if (HasInternalAttribute(symbol))
-                        {
-                            context.ReportDiagnostic(Diagnostic.Create(_descriptor, creationSyntax.GetLocation(), containingType));
-                            return;
-                        }
-
-                        if (IsInInternalNamespace(containingType)
-                            || HasInternalAttribute(containingType))
-                        {
-                            context.ReportDiagnostic(Diagnostic.Create(_descriptor, creationSyntax.Type.GetLocation(), containingType));
-                            return;
-                        }
-                    }
+                        CSharpSyntax.ClassDeclarationSyntax s when s.BaseList?.Types.Count > 0
+                            => s.BaseList.Types[0].GetLocation(),
+                        { } otherSyntax => otherSyntax.GetLocation()
+                    };
 
-                    return;
+                    context.ReportDiagnostic(Diagnostic.Create(_descriptor, location, baseSymbol));
                 }
+            }
 
-                case ClassDeclarationSyntax declarationSyntax:
+            foreach (var iface in symbol.Interfaces.Where(i => IsInternal(context, i)))
+            {
+                foreach (var declaringSyntax in symbol.DeclaringSyntaxReferences)
                 {
-                    if (context.SemanticModel.GetDeclaredSymbol(declarationSyntax)?.BaseType is ISymbol symbol
-                        && !Equals(symbol.ContainingAssembly, context.Compilation.Assembly)
-                        && (IsInInternalNamespace(symbol) || HasInternalAttribute(symbol))
-                        && declarationSyntax.BaseList?.Types.Count > 0)
+                    var location = declaringSyntax.GetSyntax() switch
                     {
-                        context.ReportDiagnostic(Diagnostic.Create(_descriptor, declarationSyntax.BaseList.Types[0].GetLocation(), symbol));
-                    }
+                        CSharpSyntax.ClassDeclarationSyntax s => s.Identifier.GetLocation(),
+                        { } otherSyntax => otherSyntax.GetLocation()
+                    };
 
-                    return;
+                    context.ReportDiagnostic(Diagnostic.Create(_descriptor, location, iface));
+                }
+            }
+        }
+
+        private static void AnalyzeMemberDeclarationTypeSymbol(
+            SymbolAnalysisContext context,
+            ISymbol declarationSymbol,
+            ITypeSymbol typeSymbol)
+        {
+            if (IsInternal(context, typeSymbol))
+            {
+                foreach (var declaringSyntax in declarationSymbol.DeclaringSyntaxReferences)
+                {
+                    ReportDiagnostic(context, declaringSyntax.GetSyntax(), typeSymbol);
                 }
             }
         }
 
+        private static void ReportDiagnostic(OperationAnalysisContext context, object messageArg)
+            => context.ReportDiagnostic(Diagnostic.Create(_descriptor, NarrowDownSyntax(context.Operation.Syntax).GetLocation(), messageArg));
+
+        private static void ReportDiagnostic(SymbolAnalysisContext context, SyntaxNode syntax, object messageArg)
+            => context.ReportDiagnostic(Diagnostic.Create(_descriptor, NarrowDownSyntax(syntax).GetLocation(), messageArg));
+
+        /// <summary>
+        ///     Given a syntax node, pattern matches some known types and returns a narrowed-down node for the type syntax which
+        ///     should be reported in diagnostics.
+        /// </summary>
+        private static SyntaxNode NarrowDownSyntax(SyntaxNode syntax)
+            => syntax switch
+            {
+                CSharpSyntax.InvocationExpressionSyntax s
+                    when s.Expression is CSharpSyntax.MemberAccessExpressionSyntax memberAccessSyntax
+                    => memberAccessSyntax.Name,
+                CSharpSyntax.MemberAccessExpressionSyntax s => s.Name,
+                CSharpSyntax.ObjectCreationExpressionSyntax s => s.Type,
+                CSharpSyntax.PropertyDeclarationSyntax s => s.Type,
+                CSharpSyntax.VariableDeclaratorSyntax declarator
+                    => declarator.Parent is CSharpSyntax.VariableDeclarationSyntax declaration
+                        ? declaration.Type
+                        : (SyntaxNode)declarator,
+                CSharpSyntax.TypeOfExpressionSyntax s => s.Type,
+
+                VBSyntax.InvocationExpressionSyntax s
+                    when s.Expression is VBSyntax.MemberAccessExpressionSyntax memberAccessSyntax
+                    => memberAccessSyntax.Name,
+                VBSyntax.MemberAccessExpressionSyntax s => s.Name,
+                VBSyntax.ObjectCreationExpressionSyntax s => s.Type,
+                VBSyntax.TypeOfExpressionSyntax s => s.Type,
+
+                _ => syntax
+            };
+
+        private static bool IsInternal(SymbolAnalysisContext context, ITypeSymbol symbol)
+            => (object)symbol.ContainingAssembly != context.Compilation.Assembly
+                && (IsInInternalNamespace(symbol) || HasInternalAttribute(symbol));
+
+        private static bool IsInternal(OperationAnalysisContext context, ITypeSymbol symbol)
+            => (object)symbol.ContainingAssembly != context.Compilation.Assembly
+                && (IsInInternalNamespace(symbol) || HasInternalAttribute(symbol));
+
         private static bool HasInternalAttribute(ISymbol symbol)
-            => symbol != null && symbol.GetAttributes().Any(a => a.AttributeClass.Name == "EntityFrameworkInternalAttribute");
+            => symbol != null
+                && symbol.GetAttributes().Any(a =>
+                    a.AttributeClass.ToDisplayString() == "Microsoft.EntityFrameworkCore.Infrastructure.EntityFrameworkInternalAttribute");
 
         private static bool IsInInternalNamespace(ISymbol symbol)
         {

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(