Skip to content

Commit

Permalink
Recognize C# 8 non-nullable reference types as [Required]
Browse files Browse the repository at this point in the history
  • Loading branch information
roji committed Apr 25, 2019
1 parent c7564b9 commit d7d899e
Show file tree
Hide file tree
Showing 11 changed files with 815 additions and 1 deletion.
29 changes: 29 additions & 0 deletions src/EFCore/Diagnostics/CoreEventId.cs
Original file line number Diff line number Diff line change
Expand Up @@ -84,7 +84,9 @@ private enum Id
RedundantIndexRemoved,
IncompatibleMatchingForeignKeyProperties,
RequiredAttributeOnDependent,
NonNullableOnDependent,
RequiredAttributeOnBothNavigations,
NonNullableReferenceOnBothNavigations,
ConflictingShadowForeignKeysWarning,
MultiplePrimaryKeyCandidates,
MultipleNavigationProperties,
Expand Down Expand Up @@ -443,6 +445,20 @@ public static readonly EventId PossibleUnintendedReferenceComparisonWarning
/// </summary>
public static readonly EventId RequiredAttributeOnDependent = MakeModelId(Id.RequiredAttributeOnDependent);

/// <summary>
/// <para>
/// The entity type with the navigation property that has non-nullability
/// was configured as the dependent side in the relationship.
/// </para>
/// <para>
/// This event is in the <see cref="DbLoggerCategory.Model" /> category.
/// </para>
/// <para>
/// This event uses the <see cref="NavigationEventData" /> payload when used with a <see cref="DiagnosticSource" />.
/// </para>
/// </summary>
public static readonly EventId NonNullableOnDependent = MakeModelId(Id.NonNullableOnDependent);

/// <summary>
/// <para>
/// Navigations separated into two relationships as <see cref="RequiredAttribute" /> was specified on both navigations.
Expand All @@ -456,6 +472,19 @@ public static readonly EventId PossibleUnintendedReferenceComparisonWarning
/// </summary>
public static readonly EventId RequiredAttributeOnBothNavigations = MakeModelId(Id.RequiredAttributeOnBothNavigations);

/// <summary>
/// <para>
/// Navigations separated into two relationships as non-nullability was specified on both navigations.
/// </para>
/// <para>
/// This event is in the <see cref="DbLoggerCategory.Model" /> category.
/// </para>
/// <para>
/// This event uses the <see cref="TwoPropertyBaseCollectionsEventData" /> payload when used with a <see cref="DiagnosticSource" />.
/// </para>
/// </summary>
public static readonly EventId NonNullableReferenceOnBothNavigations = MakeModelId(Id.NonNullableReferenceOnBothNavigations);

/// <summary>
/// <para>
/// The properties that best match the foreign key convention are already used by a different foreign key.
Expand Down
89 changes: 89 additions & 0 deletions src/EFCore/Diagnostics/CoreLoggerExtensions.cs
Original file line number Diff line number Diff line change
Expand Up @@ -1153,6 +1153,45 @@ private static string RequiredAttributeOnDependent(EventDefinitionBase definitio
return d.GenerateMessage(p.Navigation.Name, p.Navigation.DeclaringEntityType.DisplayName());
}

/// <summary>
/// This 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. 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>
public static void NonNullableOnDependent(
[NotNull] this IDiagnosticsLogger<DbLoggerCategory.Model> diagnostics,
[NotNull] INavigation navigation)
{
var definition = CoreResources.LogNonNullableOnDependent(diagnostics);

var warningBehavior = definition.GetLogBehavior(diagnostics);
if (warningBehavior != WarningBehavior.Ignore)
{
definition.Log(
diagnostics,
warningBehavior,
navigation.Name, navigation.DeclaringEntityType.DisplayName());
}

if (diagnostics.DiagnosticSource.IsEnabled(definition.EventId.Name))
{
diagnostics.DiagnosticSource.Write(
definition.EventId.Name,
new NavigationEventData(
definition,
NonNullableOnDependent,
navigation));
}
}

private static string NonNullableOnDependent(EventDefinitionBase definition, EventData payload)
{
var d = (EventDefinition<string, string>)definition;
var p = (NavigationEventData)payload;
return d.GenerateMessage(p.Navigation.Name, p.Navigation.DeclaringEntityType.DisplayName());
}

/// <summary>
/// This 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
Expand Down Expand Up @@ -1203,6 +1242,56 @@ private static string RequiredAttributeOnBothNavigations(EventDefinitionBase def
secondNavigation.Name);
}

/// <summary>
/// This 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. 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>
public static void NonNullableReferenceOnBothNavigations(
[NotNull] this IDiagnosticsLogger<DbLoggerCategory.Model> diagnostics,
[NotNull] INavigation firstNavigation,
[NotNull] INavigation secondNavigation)
{
var definition = CoreResources.LogNonNullableReferenceOnBothNavigations(diagnostics);

var warningBehavior = definition.GetLogBehavior(diagnostics);
if (warningBehavior != WarningBehavior.Ignore)
{
definition.Log(
diagnostics,
warningBehavior,
firstNavigation.DeclaringEntityType.DisplayName(),
firstNavigation.Name,
secondNavigation.DeclaringEntityType.DisplayName(),
secondNavigation.Name);
}

if (diagnostics.DiagnosticSource.IsEnabled(definition.EventId.Name))
{
diagnostics.DiagnosticSource.Write(
definition.EventId.Name,
new TwoPropertyBaseCollectionsEventData(
definition,
NonNullableReferenceOnBothNavigations,
new[] { firstNavigation },
new[] { secondNavigation }));
}
}

private static string NonNullableReferenceOnBothNavigations(EventDefinitionBase definition, EventData payload)
{
var d = (EventDefinition<string, string, string, string>)definition;
var p = (TwoPropertyBaseCollectionsEventData)payload;
var firstNavigation = p.FirstPropertyCollection[0];
var secondNavigation = p.SecondPropertyCollection[0];
return d.GenerateMessage(
firstNavigation.DeclaringType.DisplayName(),
firstNavigation.Name,
secondNavigation.DeclaringType.DisplayName(),
secondNavigation.Name);
}

/// <summary>
/// This 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
Expand Down
18 changes: 18 additions & 0 deletions src/EFCore/Diagnostics/LoggingDefinitions.cs
Original file line number Diff line number Diff line change
Expand Up @@ -457,6 +457,15 @@ public abstract class LoggingDefinitions
[EntityFrameworkInternal]
public EventDefinitionBase LogRequiredAttributeOnDependent;

/// <summary>
/// This 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. 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>
[EntityFrameworkInternal]
public EventDefinitionBase LogNonNullableOnDependent;

/// <summary>
/// This 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
Expand All @@ -466,6 +475,15 @@ public abstract class LoggingDefinitions
[EntityFrameworkInternal]
public EventDefinitionBase LogRequiredAttributeOnBothNavigations;

/// <summary>
/// This 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. 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>
[EntityFrameworkInternal]
public EventDefinitionBase LogNonNullableReferenceOnBothNavigations;

/// <summary>
/// This 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
Expand Down
Original file line number Diff line number Diff line change
Expand Up @@ -113,6 +113,7 @@ var servicePropertyDiscoveryConvention
var concurrencyCheckAttributeConvention = new ConcurrencyCheckAttributeConvention(logger);
var databaseGeneratedAttributeConvention = new DatabaseGeneratedAttributeConvention(logger);
var requiredPropertyAttributeConvention = new RequiredPropertyAttributeConvention(logger);
var nonNullableReferencePropertyConvention = new NonNullableReferencePropertyConvention(logger);
var maxLengthAttributeConvention = new MaxLengthAttributeConvention(logger);
var stringLengthAttributeConvention = new StringLengthAttributeConvention(logger);
var timestampAttributeConvention = new TimestampAttributeConvention(logger);
Expand All @@ -121,6 +122,7 @@ var servicePropertyDiscoveryConvention
conventionSet.PropertyAddedConventions.Add(concurrencyCheckAttributeConvention);
conventionSet.PropertyAddedConventions.Add(databaseGeneratedAttributeConvention);
conventionSet.PropertyAddedConventions.Add(requiredPropertyAttributeConvention);
conventionSet.PropertyAddedConventions.Add(nonNullableReferencePropertyConvention);
conventionSet.PropertyAddedConventions.Add(maxLengthAttributeConvention);
conventionSet.PropertyAddedConventions.Add(stringLengthAttributeConvention);
conventionSet.PropertyAddedConventions.Add(timestampAttributeConvention);
Expand Down Expand Up @@ -190,6 +192,7 @@ var servicePropertyDiscoveryConvention

conventionSet.NavigationAddedConventions.Add(backingFieldConvention);
conventionSet.NavigationAddedConventions.Add(new RequiredNavigationAttributeConvention(logger));
conventionSet.NavigationAddedConventions.Add(new NonNullableNavigationConvention(logger));
conventionSet.NavigationAddedConventions.Add(inversePropertyAttributeConvention);
conventionSet.NavigationAddedConventions.Add(foreignKeyPropertyDiscoveryConvention);
conventionSet.NavigationAddedConventions.Add(relationshipDiscoveryConvention);
Expand All @@ -212,6 +215,7 @@ var servicePropertyDiscoveryConvention
conventionSet.PropertyFieldChangedConventions.Add(concurrencyCheckAttributeConvention);
conventionSet.PropertyFieldChangedConventions.Add(databaseGeneratedAttributeConvention);
conventionSet.PropertyFieldChangedConventions.Add(requiredPropertyAttributeConvention);
conventionSet.PropertyFieldChangedConventions.Add(nonNullableReferencePropertyConvention);
conventionSet.PropertyFieldChangedConventions.Add(maxLengthAttributeConvention);
conventionSet.PropertyFieldChangedConventions.Add(stringLengthAttributeConvention);
conventionSet.PropertyFieldChangedConventions.Add(timestampAttributeConvention);
Expand Down
Original file line number Diff line number Diff line change
@@ -0,0 +1,130 @@
// Copyright (c) .NET Foundation. All rights reserved.
// Licensed under the Apache License, Version 2.0. See License.txt in the project root for license information.

using System;
using System.ComponentModel.DataAnnotations;
using System.Linq;
using System.Reflection;
using JetBrains.Annotations;
using Microsoft.EntityFrameworkCore.Diagnostics;
using Microsoft.EntityFrameworkCore.Internal;
using Microsoft.EntityFrameworkCore.Metadata.Internal;
using Microsoft.EntityFrameworkCore.Utilities;
using Microsoft.Extensions.Logging;

namespace Microsoft.EntityFrameworkCore.Metadata.Conventions.Internal
{
/// <summary>
/// This 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. 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>
public class NonNullableNavigationConvention : INavigationAddedConvention
{
private const string NullableAttributeFullName = "System.Runtime.CompilerServices.NullableAttribute";
private Type _nullableAttrType;
private FieldInfo _nullableFlagsFieldInfo;

/// <summary>
/// This 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. 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>
public NonNullableNavigationConvention([NotNull] IDiagnosticsLogger<DbLoggerCategory.Model> logger)
{
Logger = logger;
}

/// <summary>
/// This 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. 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>
protected virtual IDiagnosticsLogger<DbLoggerCategory.Model> Logger { get; }

/// <summary>
/// This 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. 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>
public virtual InternalRelationshipBuilder Apply(
InternalRelationshipBuilder relationshipBuilder,
Navigation navigation)
{
Check.NotNull(relationshipBuilder, nameof(relationshipBuilder));
Check.NotNull(navigation, nameof(navigation));

if (!IsNonNullable(navigation))
{
return relationshipBuilder;
}

if (!navigation.IsDependentToPrincipal())
{
var inverse = navigation.FindInverse();
if (inverse != null)
{
if (IsNonNullable(inverse))
{
Logger.NonNullableReferenceOnBothNavigations(navigation, inverse);
return relationshipBuilder;
}
}

if (!navigation.ForeignKey.IsUnique
|| relationshipBuilder.Metadata.GetPrincipalEndConfigurationSource() != null)
{
return relationshipBuilder;
}

var newRelationshipBuilder = relationshipBuilder.HasEntityTypes(
relationshipBuilder.Metadata.DeclaringEntityType,
relationshipBuilder.Metadata.PrincipalEntityType,
ConfigurationSource.Convention);

if (newRelationshipBuilder == null)
{
return relationshipBuilder;
}

Logger.NonNullableOnDependent(newRelationshipBuilder.Metadata.DependentToPrincipal);
relationshipBuilder = newRelationshipBuilder;
}

return relationshipBuilder.IsRequired(true, ConfigurationSource.Convention) ?? relationshipBuilder;
}

private bool IsNonNullable(Navigation navigation)
=> navigation.DeclaringEntityType.HasClrType()
&& navigation.DeclaringEntityType.GetRuntimeProperties().Find(navigation.Name) is PropertyInfo propertyInfo
&& IsNonNullable(propertyInfo);

private bool IsNonNullable(MemberInfo memberInfo)
{
if (Attribute.GetCustomAttributes(memberInfo, true) is Attribute[] attributes
&& attributes.FirstOrDefault(a => a.GetType().FullName == NullableAttributeFullName) is Attribute attribute)
{
if (attribute.GetType() != _nullableAttrType)
{
_nullableFlagsFieldInfo = attribute.GetType().GetField("NullableFlags");
_nullableAttrType = attribute.GetType();
}

// For the interpretation of NullableFlags, see
// https://github.com/dotnet/roslyn/blob/master/docs/features/nullable-reference-types.md#annotations
if (_nullableFlagsFieldInfo?.GetValue(attribute) is byte[] flags
&& flags.Length >= 0
&& flags[0] == 1)
{
return true;
}
}

return false;
}
}
}
Loading

0 comments on commit d7d899e

Please sign in to comment.