Skip to content

Commit

Permalink
Suppress warnings on uninitialized DbSet properties
Browse files Browse the repository at this point in the history
  • Loading branch information
roji committed Nov 22, 2021
1 parent 197479e commit bcffe52
Show file tree
Hide file tree
Showing 4 changed files with 261 additions and 8 deletions.
90 changes: 90 additions & 0 deletions src/EFCore.Analyzers/UninitializedDbSetDiagnosticSuppressor.cs
Original file line number Diff line number Diff line change
@@ -0,0 +1,90 @@
// Licensed to the .NET Foundation under one or more agreements.
// The .NET Foundation licenses this file to you under the MIT license.

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

namespace Microsoft.EntityFrameworkCore;

[DiagnosticAnalyzer(LanguageNames.CSharp)]
public class UninitializedDbSetDiagnosticSuppressor : DiagnosticSuppressor
{
private static readonly SuppressionDescriptor _suppressUninitializedDbSetRule = new(
id: "SPR1001",
suppressedDiagnosticId: "CS8618",
justification: "DbSet properties on DbContext subclasses are automatically populated by the DbContext constructor");

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

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

foreach (var diagnostic in context.ReportedDiagnostics)
{
if (diagnostic.Id != "CS8618" || !diagnostic.Location.IsInSource)
{
continue;
}

// 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).
var sourceTree = diagnostic.Location.SourceTree;
var node = sourceTree?.GetRoot().FindNode(diagnostic.Location.SourceSpan);
if (node is not PropertyDeclarationSyntax propertyDeclarationSyntax
|| !propertyDeclarationSyntax.Type.ToString().Contains("DbSet"))
{
continue;
}

// Get the semantic symbol and do some basic checks
if (context.GetSemanticModel(sourceTree!).GetDeclaredSymbol(node) 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)
&& InheritsFromDbContext(propertySymbol.ContainingType, dbContextTypeSymbol))
{
context.ReportSuppression(Suppression.Create(SupportedSuppressions[0], diagnostic));
}

static bool InheritsFromDbContext(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;
}
}
}
}
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 @@ -232,4 +229,7 @@ private async Task TestFullSource(

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));
}
}
Original file line number Diff line number Diff line change
@@ -0,0 +1,154 @@
// Licensed to the .NET Foundation under one or more agreements.
// The .NET Foundation licenses this file to you under the MIT license.

using System.Threading.Tasks;
using Microsoft.CodeAnalysis;
using Microsoft.CodeAnalysis.Diagnostics;
using Microsoft.EntityFrameworkCore.TestUtilities;
using Xunit;

namespace Microsoft.EntityFrameworkCore;

public class UninitializedDbSetDiagnosticSuppressorTest : DiagnosticAnalyzerTestBase
{
[ConditionalFact]
public async Task DbSet_property_on_DbContext_is_suppressed()
{
var source = @"
public class MyDbContext : Microsoft.EntityFrameworkCore.DbContext
{
public Microsoft.EntityFrameworkCore.DbSet<Blog> Blogs { get; set; }
}
public class Blog
{
public int Id { get; set; }
}";

var diagnostic = Assert.Single(await GetDiagnosticsFullSourceAsync(source));

Assert.Equal("CS8618", diagnostic.Id);
Assert.True(diagnostic.IsSuppressed);
}

[ConditionalFact]
public async Task Non_public_DbSet_property_on_DbContext_is_suppressed()
{
var source = @"
public class MyDbContext : Microsoft.EntityFrameworkCore.DbContext
{
private Microsoft.EntityFrameworkCore.DbSet<Blog> Blogs { get; set; }
}
public class Blog
{
public int Id { get; set; }
}";

var diagnostic = Assert.Single(await GetDiagnosticsFullSourceAsync(source));

Assert.Equal("CS8618", diagnostic.Id);
Assert.True(diagnostic.IsSuppressed);
}

[ConditionalFact]
public async Task DbSet_property_with_non_public_setter_on_DbContext_is_suppressed()
{
var source = @"
public class MyDbContext : Microsoft.EntityFrameworkCore.DbContext
{
public Microsoft.EntityFrameworkCore.DbSet<Blog> Blogs { get; private set; }
}
public class Blog
{
public int Id { get; set; }
}";

var diagnostic = Assert.Single(await GetDiagnosticsFullSourceAsync(source));

Assert.Equal("CS8618", diagnostic.Id);
Assert.True(diagnostic.IsSuppressed);
}

[ConditionalFact]
public async Task DbSet_property_without_setter_on_DbContext_is_not_suppressed()
{
var source = @"
public class MyDbContext : Microsoft.EntityFrameworkCore.DbContext
{
public Microsoft.EntityFrameworkCore.DbSet<Blog> Blogs { get; }
}
public class Blog
{
public int Id { get; set; }
}";

var diagnostic = Assert.Single(await GetDiagnosticsFullSourceAsync(source));

Assert.Equal("CS8618", diagnostic.Id);
Assert.False(diagnostic.IsSuppressed);
}

[ConditionalFact]
public async Task Static_DbSet_property_on_DbContext_is_not_suppressed()
{
var source = @"
public class MyDbContext : Microsoft.EntityFrameworkCore.DbContext
{
public static Microsoft.EntityFrameworkCore.DbSet<Blog> Blogs { get; set; }
}
public class Blog
{
public int Id { get; set; }
}";

var diagnostic = Assert.Single(await GetDiagnosticsFullSourceAsync(source));

Assert.Equal("CS8618", diagnostic.Id);
Assert.False(diagnostic.IsSuppressed);
}

[ConditionalFact]
public async Task Non_DbSet_property_on_DbContext_is_not_suppressed()
{
var source = @"
public class MyDbContext : Microsoft.EntityFrameworkCore.DbContext
{
public string Name { get; set; }
}";

var diagnostic = Assert.Single(await GetDiagnosticsFullSourceAsync(source));

Assert.Equal("CS8618", diagnostic.Id);
Assert.False(diagnostic.IsSuppressed);
}

[ConditionalFact]
public async Task DbSet_property_on_non_DbContext_is_not_suppressed()
{
var source = @"
public class Foo
{
public Microsoft.EntityFrameworkCore.DbSet<Blog> Blogs { get; set; }
}
public class Blog
{
public int Id { get; set; }
}";

var diagnostic = Assert.Single(await GetDiagnosticsFullSourceAsync(source));

Assert.Equal("CS8618", diagnostic.Id);
Assert.False(diagnostic.IsSuppressed);
}

protected Task<Diagnostic[]> GetDiagnosticsFullSourceAsync(string source)
=> base.GetDiagnosticsFullSourceAsync(source, analyzerDiagnosticsOnly: false);

protected override DiagnosticAnalyzer CreateDiagnosticAnalyzer()
=> new UninitializedDbSetDiagnosticSuppressor();
}

0 comments on commit bcffe52

Please sign in to comment.