Skip to content

Commit

Permalink
Add nullable attributes/parameter checks for query
Browse files Browse the repository at this point in the history
Fixes #16705

Attempted to add null checks when the API is a public-surface API, but not for purely internal APIs.

There may be some mistakes here; I plan to do a bit more static analysis after the initial merge.

Also fixed the ApiConsistencyTest to check pubternal types again (not sure why this was removed) and to check protected constructors.
  • Loading branch information
ajcvickers committed Dec 1, 2019
1 parent ced15e9 commit d47ab37
Show file tree
Hide file tree
Showing 309 changed files with 4,003 additions and 1,163 deletions.
Original file line number Diff line number Diff line change
Expand Up @@ -32,7 +32,7 @@ public static class CosmosLoggerExtensions
/// </summary>
public static void ExecutingSqlQuery(
[NotNull] this IDiagnosticsLogger<DbLoggerCategory.Database.Command> diagnosticsLogger,
CosmosSqlQuery cosmosSqlQuery)
[NotNull] CosmosSqlQuery cosmosSqlQuery)
{
var definition = new EventDefinition<string, string, string>(
diagnosticsLogger.Options,
Expand Down
Original file line number Diff line number Diff line change
Expand Up @@ -2,6 +2,7 @@
// Licensed under the Apache License, Version 2.0. See License.txt in the project root for license information.

using System.Linq.Expressions;
using JetBrains.Annotations;
using Microsoft.EntityFrameworkCore.Cosmos.Query.Internal;

// ReSharper disable once CheckNamespace
Expand All @@ -21,7 +22,7 @@ public static class CosmosExpressionExtensions
/// 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 bool IsLogicalNot(this SqlUnaryExpression sqlUnaryExpression)
public static bool IsLogicalNot([NotNull] this SqlUnaryExpression sqlUnaryExpression)
=> sqlUnaryExpression.OperatorType == ExpressionType.Not
&& (sqlUnaryExpression.Type == typeof(bool)
|| sqlUnaryExpression.Type == typeof(bool?));
Expand Down
Original file line number Diff line number Diff line change
Expand Up @@ -46,7 +46,7 @@ public CosmosOptionsExtension()
/// 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 CosmosOptionsExtension(CosmosOptionsExtension copyFrom)
protected CosmosOptionsExtension([NotNull] CosmosOptionsExtension copyFrom)
{
_accountEndpoint = copyFrom._accountEndpoint;
_accountKey = copyFrom._accountKey;
Expand Down Expand Up @@ -79,7 +79,7 @@ public virtual DbContextOptionsExtensionInfo Info
/// 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 CosmosOptionsExtension WithAccountEndpoint(string accountEndpoint)
public virtual CosmosOptionsExtension WithAccountEndpoint([NotNull] string accountEndpoint)
{
var clone = Clone();

Expand All @@ -102,7 +102,7 @@ public virtual CosmosOptionsExtension WithAccountEndpoint(string accountEndpoint
/// 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 CosmosOptionsExtension WithAccountKey(string accountKey)
public virtual CosmosOptionsExtension WithAccountKey([NotNull] string accountKey)
{
var clone = Clone();

Expand All @@ -125,7 +125,7 @@ public virtual CosmosOptionsExtension WithAccountKey(string accountKey)
/// 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 CosmosOptionsExtension WithDatabaseName(string database)
public virtual CosmosOptionsExtension WithDatabaseName([NotNull] string database)
{
var clone = Clone();

Expand All @@ -148,7 +148,7 @@ public virtual CosmosOptionsExtension WithDatabaseName(string database)
/// 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 CosmosOptionsExtension WithRegion(string region)
public virtual CosmosOptionsExtension WithRegion([NotNull] string region)
{
var clone = Clone();

Expand Down
Original file line number Diff line number Diff line change
@@ -1,6 +1,7 @@
// 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 JetBrains.Annotations;
using Microsoft.EntityFrameworkCore.Metadata;

namespace Microsoft.EntityFrameworkCore.Cosmos.Metadata.Internal
Expand All @@ -19,7 +20,7 @@ public static class CosmosEntityTypeExtensions
/// 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 bool IsDocumentRoot(this IEntityType entityType)
public static bool IsDocumentRoot([NotNull] this IEntityType entityType)
=> entityType.BaseType?.IsDocumentRoot()
?? (!entityType.IsOwned()
|| entityType[CosmosAnnotationNames.ContainerName] != null);
Expand Down
Original file line number Diff line number Diff line change
@@ -1,6 +1,7 @@
// 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 JetBrains.Annotations;
using Microsoft.EntityFrameworkCore.Metadata;

namespace Microsoft.EntityFrameworkCore.Cosmos.Metadata.Internal
Expand All @@ -19,7 +20,7 @@ public static class CosmosNavigationExtensions
/// 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 bool IsEmbedded(this INavigation navigation)
public static bool IsEmbedded([NotNull] this INavigation navigation)
=> !navigation.IsDependentToPrincipal()
&& !navigation.ForeignKey.DeclaringEntityType.IsDocumentRoot();
}
Expand Down
7 changes: 6 additions & 1 deletion src/EFCore.Cosmos/Query/Internal/ContainsTranslator.cs
Original file line number Diff line number Diff line change
Expand Up @@ -5,6 +5,8 @@
using System.Collections.Generic;
using System.Linq;
using System.Reflection;
using JetBrains.Annotations;
using Microsoft.EntityFrameworkCore.Utilities;

namespace Microsoft.EntityFrameworkCore.Cosmos.Query.Internal
{
Expand All @@ -24,7 +26,7 @@ public class ContainsTranslator : IMethodCallTranslator
/// 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 ContainsTranslator(ISqlExpressionFactory sqlExpressionFactory)
public ContainsTranslator([NotNull] ISqlExpressionFactory sqlExpressionFactory)
{
_sqlExpressionFactory = sqlExpressionFactory;
}
Expand All @@ -37,6 +39,9 @@ public ContainsTranslator(ISqlExpressionFactory sqlExpressionFactory)
/// </summary>
public virtual SqlExpression Translate(SqlExpression instance, MethodInfo method, IReadOnlyList<SqlExpression> arguments)
{
Check.NotNull(method, nameof(method));
Check.NotNull(arguments, nameof(arguments));

if (method.IsGenericMethod
&& method.GetGenericMethodDefinition().Equals(EnumerableMethods.Contains))
{
Expand Down
Original file line number Diff line number Diff line change
Expand Up @@ -5,6 +5,8 @@
using System.Collections.Generic;
using System.Linq;
using System.Reflection;
using JetBrains.Annotations;
using Microsoft.EntityFrameworkCore.Utilities;

namespace Microsoft.EntityFrameworkCore.Cosmos.Query.Internal
{
Expand All @@ -26,8 +28,8 @@ public class CosmosMemberTranslatorProvider : IMemberTranslatorProvider
/// doing so can result in application failures when updating to a new Entity Framework Core release.
/// </summary>
public CosmosMemberTranslatorProvider(
ISqlExpressionFactory sqlExpressionFactory,
IEnumerable<IMemberTranslatorPlugin> plugins)
[NotNull] ISqlExpressionFactory sqlExpressionFactory,
[NotNull] IEnumerable<IMemberTranslatorPlugin> plugins)
{
_plugins.AddRange(plugins.SelectMany(p => p.Translators));
//_translators
Expand All @@ -46,6 +48,10 @@ public CosmosMemberTranslatorProvider(
/// </summary>
public virtual SqlExpression Translate(SqlExpression instance, MemberInfo member, Type returnType)
{
Check.NotNull(instance, nameof(instance));
Check.NotNull(member, nameof(member));
Check.NotNull(returnType, nameof(returnType));

return _plugins.Concat(_translators)
.Select(t => t.Translate(instance, member, returnType)).FirstOrDefault(t => t != null);
}
Expand All @@ -56,7 +62,7 @@ public virtual SqlExpression Translate(SqlExpression instance, MemberInfo member
/// 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 void AddTranslators(IEnumerable<IMemberTranslator> translators)
protected virtual void AddTranslators([NotNull] IEnumerable<IMemberTranslator> translators)
=> _translators.InsertRange(0, translators);
}
}
Original file line number Diff line number Diff line change
Expand Up @@ -4,7 +4,9 @@
using System.Collections.Generic;
using System.Linq;
using System.Reflection;
using JetBrains.Annotations;
using Microsoft.EntityFrameworkCore.Metadata;
using Microsoft.EntityFrameworkCore.Utilities;

namespace Microsoft.EntityFrameworkCore.Cosmos.Query.Internal
{
Expand All @@ -26,8 +28,8 @@ public class CosmosMethodCallTranslatorProvider : IMethodCallTranslatorProvider
/// doing so can result in application failures when updating to a new Entity Framework Core release.
/// </summary>
public CosmosMethodCallTranslatorProvider(
ISqlExpressionFactory sqlExpressionFactory,
IEnumerable<IMethodCallTranslatorPlugin> plugins)
[NotNull] ISqlExpressionFactory sqlExpressionFactory,
[NotNull] IEnumerable<IMethodCallTranslatorPlugin> plugins)
{
_plugins.AddRange(plugins.SelectMany(p => p.Translators));

Expand All @@ -53,18 +55,9 @@ public CosmosMethodCallTranslatorProvider(
public virtual SqlExpression Translate(
IModel model, SqlExpression instance, MethodInfo method, IReadOnlyList<SqlExpression> arguments)
{
// TODO: UDF support. See issue#15338
//var dbFunction = model.FindDbFunction(method);
//if (dbFunction != null)
//{
// return dbFunction.Translation?.Invoke(
// arguments.Select(e => _sqlExpressionFactory.ApplyDefaultTypeMapping(e)).ToList())
// ?? _sqlExpressionFactory.Function(
// dbFunction.Schema,
// dbFunction.Name,
// arguments,
// method.ReturnType);
//}
Check.NotNull(model, nameof(model));
Check.NotNull(method, nameof(method));
Check.NotNull(arguments, nameof(arguments));

return _plugins.Concat(_translators)
.Select(t => t.Translate(instance, method, arguments))
Expand All @@ -77,7 +70,7 @@ public virtual SqlExpression Translate(
/// 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 void AddTranslators(IEnumerable<IMethodCallTranslator> translators)
protected virtual void AddTranslators([NotNull] IEnumerable<IMethodCallTranslator> translators)
=> _translators.InsertRange(0, translators);
}
}
Original file line number Diff line number Diff line change
Expand Up @@ -6,12 +6,14 @@
using System.Linq;
using System.Linq.Expressions;
using System.Reflection;
using JetBrains.Annotations;
using Microsoft.EntityFrameworkCore.Cosmos.Metadata.Internal;
using Microsoft.EntityFrameworkCore.Diagnostics;
using Microsoft.EntityFrameworkCore.Infrastructure;
using Microsoft.EntityFrameworkCore.Metadata;
using Microsoft.EntityFrameworkCore.Query;
using Microsoft.EntityFrameworkCore.Storage;
using Microsoft.EntityFrameworkCore.Utilities;

namespace Microsoft.EntityFrameworkCore.Cosmos.Query.Internal
{
Expand Down Expand Up @@ -44,7 +46,7 @@ private readonly Stack<INavigation> _includedNavigations
/// 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 CosmosProjectionBindingExpressionVisitor(CosmosSqlTranslatingExpressionVisitor sqlTranslator)
public CosmosProjectionBindingExpressionVisitor([NotNull] CosmosSqlTranslatingExpressionVisitor sqlTranslator)
{
_sqlTranslator = sqlTranslator;
}
Expand All @@ -55,7 +57,7 @@ public CosmosProjectionBindingExpressionVisitor(CosmosSqlTranslatingExpressionVi
/// 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 Expression Translate(SelectExpression selectExpression, Expression expression)
public virtual Expression Translate([NotNull] SelectExpression selectExpression, [NotNull] Expression expression)
{
_selectExpression = selectExpression;
_clientEval = false;
Expand Down Expand Up @@ -170,6 +172,8 @@ private static T GetParameterValue<T>(QueryContext queryContext, string paramete
/// </summary>
protected override Expression VisitMember(MemberExpression memberExpression)
{
Check.NotNull(memberExpression, nameof(memberExpression));

if (!_clientEval)
{
return null;
Expand Down Expand Up @@ -254,6 +258,8 @@ protected override Expression VisitMember(MemberExpression memberExpression)

protected override Expression VisitMethodCall(MethodCallExpression methodCallExpression)
{
Check.NotNull(methodCallExpression, nameof(methodCallExpression));

if (methodCallExpression.TryGetEFPropertyArguments(out var source, out var memberName))
{
if (!_clientEval)
Expand Down Expand Up @@ -403,6 +409,8 @@ protected override Expression VisitMethodCall(MethodCallExpression methodCallExp
/// </summary>
protected override Expression VisitExtension(Expression extensionExpression)
{
Check.NotNull(extensionExpression, nameof(extensionExpression));

switch (extensionExpression)
{
case EntityShaperExpression entityShaperExpression:
Expand Down Expand Up @@ -465,6 +473,8 @@ protected override Expression VisitExtension(Expression extensionExpression)
/// </summary>
protected override Expression VisitNew(NewExpression newExpression)
{
Check.NotNull(newExpression, nameof(newExpression));

if (newExpression.Arguments.Count == 0)
{
return newExpression;
Expand Down Expand Up @@ -508,6 +518,8 @@ protected override Expression VisitNew(NewExpression newExpression)
/// </summary>
protected override Expression VisitMemberInit(MemberInitExpression memberInitExpression)
{
Check.NotNull(memberInitExpression, nameof(memberInitExpression));

var newExpression = Visit(memberInitExpression.NewExpression);
if (newExpression == null)
{
Expand Down
Loading

0 comments on commit d47ab37

Please sign in to comment.