Skip to content

Commit

Permalink
Suppress warnings on uninitialized DbSet properties (#26795)
Browse files Browse the repository at this point in the history
Closes #21608
  • Loading branch information
roji authored Nov 26, 2021
1 parent b44b8b6 commit e41f912
Show file tree
Hide file tree
Showing 8 changed files with 372 additions and 29 deletions.
13 changes: 13 additions & 0 deletions src/EFCore.Analyzers/EFCore.Analyzers.csproj
Original file line number Diff line number Diff line change
Expand Up @@ -19,6 +19,11 @@

<ItemGroup>
<Compile Include="..\Shared\CodeAnnotations.cs" />
<Compile Update="Properties\AnalyzerStrings.Designer.cs">
<DesignTime>True</DesignTime>
<AutoGen>True</AutoGen>
<DependentUpon>AnalyzerStrings.resx</DependentUpon>
</Compile>
</ItemGroup>

<ItemGroup>
Expand All @@ -45,4 +50,12 @@
<AdditionalFiles Include="AnalyzerReleases.Unshipped.md" />
</ItemGroup>

<ItemGroup>
<EmbeddedResource Update="Properties\AnalyzerStrings.resx">
<Generator>PublicResXFileCodeGenerator</Generator>
<LastGenOutput>AnalyzerStrings.Designer.cs</LastGenOutput>
<CustomToolNamespace>Microsoft.EntityFrameworkCore</CustomToolNamespace>
</EmbeddedResource>
</ItemGroup>

</Project>
17 changes: 4 additions & 13 deletions src/EFCore.Analyzers/InternalUsageDiagnosticAnalyzer.cs
Original file line number Diff line number Diff line change
Expand Up @@ -10,26 +10,17 @@
namespace Microsoft.EntityFrameworkCore;

[DiagnosticAnalyzer(LanguageNames.CSharp)]
public class InternalUsageDiagnosticAnalyzer : DiagnosticAnalyzer
public sealed 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.";

protected const string DefaultTitle = "Internal EF Core API usage.";
protected const string Category = "Usage";

private static readonly int EFLen = "EntityFrameworkCore".Length;

private static readonly DiagnosticDescriptor _descriptor
= new(
Id,
title: DefaultTitle,
messageFormat: MessageFormat,
category: Category,
title: AnalyzerStrings.InternalUsageTitle,
messageFormat: AnalyzerStrings.InternalUsageMessageFormat,
category: "Usage",
defaultSeverity: DiagnosticSeverity.Warning,
isEnabledByDefault: true);

Expand Down
66 changes: 66 additions & 0 deletions src/EFCore.Analyzers/Properties/AnalyzerStrings.Designer.cs

Some generated files are not rendered by default. Learn more about how customized files appear on GitHub.

30 changes: 30 additions & 0 deletions src/EFCore.Analyzers/Properties/AnalyzerStrings.resx
Original file line number Diff line number Diff line change
@@ -0,0 +1,30 @@
<?xml version="1.0" encoding="utf-8"?>

<root>
<xsd:schema id="root" xmlns="" xmlns:xsd="http://www.w3.org/2001/XMLSchema" xmlns:msdata="urn:schemas-microsoft-com:xml-msdata">
<xsd:element name="root" msdata:IsDataSet="true">

</xsd:element>
</xsd:schema>
<resheader name="resmimetype">
<value>text/microsoft-resx</value>
</resheader>
<resheader name="version">
<value>1.3</value>
</resheader>
<resheader name="reader">
<value>System.Resources.ResXResourceReader, System.Windows.Forms, Version=2.0.0.0, Culture=neutral, PublicKeyToken=b77a5c561934e089</value>
</resheader>
<resheader name="writer">
<value>System.Resources.ResXResourceWriter, System.Windows.Forms, Version=2.0.0.0, Culture=neutral, PublicKeyToken=b77a5c561934e089</value>
</resheader>
<data name="UninitializedDbSetWarningSuppressionJustification" xml:space="preserve">
<value>DbSet properties on DbContext subclasses are automatically populated by the DbContext constructor.</value>
</data>
<data name="InternalUsageMessageFormat" xml:space="preserve">
<value>{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.</value>
</data>
<data name="InternalUsageTitle" xml:space="preserve">
<value>Internal EF Core API usage.</value>
</data>
</root>
84 changes: 84 additions & 0 deletions src/EFCore.Analyzers/UninitializedDbSetDiagnosticSuppressor.cs
Original file line number Diff line number Diff line change
@@ -0,0 +1,84 @@
// Licensed to the .NET Foundation under one or more agreements.
// The .NET Foundation licenses this file to you under the MIT license.

using System.Collections.Immutable;
using Microsoft.CodeAnalysis;
using Microsoft.CodeAnalysis.CSharp.Syntax;
using Microsoft.CodeAnalysis.Diagnostics;

namespace Microsoft.EntityFrameworkCore;

[DiagnosticAnalyzer(LanguageNames.CSharp)]
public sealed class UninitializedDbSetDiagnosticSuppressor : DiagnosticSuppressor
{
private static readonly SuppressionDescriptor _suppressUninitializedDbSetRule = new(
id: "SPR1001",
suppressedDiagnosticId: "CS8618",
justification: AnalyzerStrings.UninitializedDbSetWarningSuppressionJustification);

/// <inheritdoc />
public override ImmutableArray<SuppressionDescriptor> SupportedSuppressions { get; }
= ImmutableArray.Create(_suppressUninitializedDbSetRule);

/// <inheritdoc />
public override void ReportSuppressions(SuppressionAnalysisContext context)
{
INamedTypeSymbol? dbSetTypeSymbol = null;
INamedTypeSymbol? dbContextTypeSymbol = null;

foreach (var diagnostic in context.ReportedDiagnostics)
{
// We have an warning about an uninitialized non-nullable property.
// Get the node, and make sure it's a property who's type syntactically contains DbSet (fast check before getting the
// semantic model, which is heavier).
if (diagnostic.Location.SourceTree is not { } sourceTree
|| sourceTree.GetRoot().FindNode(diagnostic.Location.SourceSpan) is not PropertyDeclarationSyntax propertyDeclarationSyntax
|| !propertyDeclarationSyntax.Type.ToString().Contains("DbSet"))
{
continue;
}

// Get the semantic symbol and do some basic checks
if (context.GetSemanticModel(sourceTree).GetDeclaredSymbol(propertyDeclarationSyntax) is not IPropertySymbol propertySymbol
|| propertySymbol.IsStatic
|| propertySymbol.IsReadOnly)
{
continue;
}

if (dbSetTypeSymbol is null || dbContextTypeSymbol is null)
{
dbSetTypeSymbol = context.Compilation.GetTypeByMetadataName("Microsoft.EntityFrameworkCore.DbSet`1");
dbContextTypeSymbol = context.Compilation.GetTypeByMetadataName("Microsoft.EntityFrameworkCore.DbContext");

if (dbSetTypeSymbol is null || dbContextTypeSymbol is null)
{
return;
}
}

// Check that the property is actually a DbSet<T>, and that its containing type inherits from DbContext
if (propertySymbol.Type.OriginalDefinition.Equals(dbSetTypeSymbol, SymbolEqualityComparer.Default)
&& InheritsFrom(propertySymbol.ContainingType, dbContextTypeSymbol))
{
context.ReportSuppression(Suppression.Create(SupportedSuppressions[0], diagnostic));
}

static bool InheritsFrom(ITypeSymbol typeSymbol, ITypeSymbol baseTypeSymbol)
{
var baseType = typeSymbol.BaseType;
while (baseType is not null)
{
if (baseType.Equals(baseTypeSymbol, SymbolEqualityComparer.Default))
{
return true;
}

baseType = baseType.BaseType;
}

return false;
}
}
}
}
18 changes: 7 additions & 11 deletions test/EFCore.Analyzers.Tests/InternalUsageDiagnosticAnalyzerTest.cs
Original file line number Diff line number Diff line change
Expand Up @@ -8,9 +8,6 @@ namespace Microsoft.EntityFrameworkCore;

public class InternalUsageDiagnosticAnalyzerTest : DiagnosticAnalyzerTestBase
{
protected override DiagnosticAnalyzer CreateDiagnosticAnalyzer()
=> new InternalUsageDiagnosticAnalyzer();

[ConditionalFact]
public Task Invocation_on_type_in_internal_namespace()
=> Test(
Expand Down Expand Up @@ -43,7 +40,7 @@ class MyClass : Microsoft.EntityFrameworkCore.Storage.Internal.RawRelationalPara
Assert.Equal(DiagnosticSeverity.Warning, diagnostic.Severity);
Assert.Equal(
string.Format(
InternalUsageDiagnosticAnalyzer.MessageFormat,
AnalyzerStrings.InternalUsageMessageFormat,
"Microsoft.EntityFrameworkCore.Storage.Internal.RawRelationalParameter"),
diagnostic.GetMessage());
Expand All @@ -58,7 +55,7 @@ class MyClass : Microsoft.EntityFrameworkCore.Storage.Internal.RawRelationalPara
Assert.Equal(DiagnosticSeverity.Warning, diagnostic.Severity);
Assert.Equal(
string.Format(
InternalUsageDiagnosticAnalyzer.MessageFormat,
AnalyzerStrings.InternalUsageMessageFormat,
"Microsoft.EntityFrameworkCore.Storage.Internal.RawRelationalParameter"),
diagnostic.GetMessage());
Expand Down Expand Up @@ -204,9 +201,7 @@ private async Task Test(

Assert.Equal(InternalUsageDiagnosticAnalyzer.Id, diagnostic.Id);
Assert.Equal(DiagnosticSeverity.Warning, diagnostic.Severity);
Assert.Equal(
string.Format(InternalUsageDiagnosticAnalyzer.MessageFormat, expectedInternalApi),
diagnostic.GetMessage());
Assert.Equal(string.Format(AnalyzerStrings.InternalUsageMessageFormat, expectedInternalApi), diagnostic.GetMessage());

var span = diagnostic.Location.SourceSpan;
Assert.Equal(expectedDiagnosticSpan, fullSource[span.Start..span.End]);
Expand All @@ -222,14 +217,15 @@ private async Task TestFullSource(

Assert.Equal(InternalUsageDiagnosticAnalyzer.Id, diagnostic.Id);
Assert.Equal(DiagnosticSeverity.Warning, diagnostic.Severity);
Assert.Equal(
string.Format(InternalUsageDiagnosticAnalyzer.MessageFormat, expectedInternalApi),
diagnostic.GetMessage());
Assert.Equal(string.Format(AnalyzerStrings.InternalUsageMessageFormat, expectedInternalApi), diagnostic.GetMessage());

var span = diagnostic.Location.SourceSpan;
Assert.Equal(expectedDiagnosticSpan, fullSource[span.Start..span.End]);
}

protected override Task<(Diagnostic[], string)> GetDiagnosticsAsync(string source, params string[] extraUsings)
=> base.GetDiagnosticsAsync(source, extraUsings.Concat(new[] { "Microsoft.EntityFrameworkCore.Internal" }).ToArray());

protected override DiagnosticAnalyzer CreateDiagnosticAnalyzer()
=> new InternalUsageDiagnosticAnalyzer();
}
Original file line number Diff line number Diff line change
Expand Up @@ -22,7 +22,11 @@ protected async Task AssertNoDiagnostics(string source, params string[] extraUsi
Assert.Empty(diagnostics);
}

protected virtual async Task<(Diagnostic[], string)> GetDiagnosticsAsync(string source, params string[] extraUsings)
protected virtual Task<(Diagnostic[], string)> GetDiagnosticsAsync(string source, params string[] extraUsings)
=> GetDiagnosticsAsync(source, analyzerDiagnosticsOnly: true, extraUsings);

protected virtual async Task<(Diagnostic[], string)> GetDiagnosticsAsync(
string source, bool analyzerDiagnosticsOnly, params string[] extraUsings)
{
var sb = new StringBuilder();
foreach (var @using in _usings.Concat(extraUsings))
Expand All @@ -42,10 +46,10 @@ protected async Task AssertNoDiagnostics(string source, params string[] extraUsi
.AppendLine("}");

var fullSource = sb.ToString();
return (await GetDiagnosticsFullSourceAsync(fullSource), fullSource);
return (await GetDiagnosticsFullSourceAsync(fullSource, analyzerDiagnosticsOnly), fullSource);
}

protected async Task<Diagnostic[]> GetDiagnosticsFullSourceAsync(string source)
protected async Task<Diagnostic[]> GetDiagnosticsFullSourceAsync(string source, bool analyzerDiagnosticsOnly = true)
{
var compilation = await CreateProject(source).GetCompilationAsync();
var errors = compilation.GetDiagnostics().Where(d => d.Severity == DiagnosticSeverity.Error);
Expand All @@ -60,7 +64,9 @@ var compilationWithAnalyzers
analyzer.SupportedDiagnostics.ToDictionary(d => d.Id, d => ReportDiagnostic.Default)))
.WithAnalyzers(ImmutableArray.Create(analyzer));

var diagnostics = await compilationWithAnalyzers.GetAnalyzerDiagnosticsAsync();
var diagnostics = analyzerDiagnosticsOnly
? await compilationWithAnalyzers.GetAnalyzerDiagnosticsAsync()
: await compilationWithAnalyzers.GetAllDiagnosticsAsync();

return diagnostics.OrderBy(d => d.Location.SourceSpan.Start).ToArray();
}
Expand Down Expand Up @@ -89,6 +95,9 @@ var metadataReferences
.AddDocument(documentId, fileName, SourceText.From(source));

return solution.GetProject(projectId)
.WithCompilationOptions(new CSharpCompilationOptions(OutputKind.DynamicallyLinkedLibrary));
.WithCompilationOptions(
new CSharpCompilationOptions(
OutputKind.DynamicallyLinkedLibrary,
nullableContextOptions: NullableContextOptions.Enable));
}
}
Loading

0 comments on commit e41f912

Please sign in to comment.