aspire Check List: Validate arguments of public methods

Is your feature request related to a problem? Please describe the problem.

Check List to #2649 Source CA1062

Cause An externally visible method dereferences one of its reference arguments without verifying whether that argument is null (Nothing in Visual Basic).

You can configure this rule to exclude certain types and parameters from analysis. You can also indicate null-check validation methods.

Rule description All reference arguments that are passed to externally visible methods should be checked against null. If appropriate, throw an ArgumentNullException when the argument is null.

If a method can be called from an unknown assembly because it is declared public or protected, you should validate all parameters of the method. If the method is designed to be called only by known assemblies, mark the method internal and apply the InternalsVisibleToAttribute attribute to the assembly that contains the method.

Describe the solution you'd like

I suggest covering this with additional tests. Check every reference argument in the public API ArgumentNullException.ThrowIfNull(source).

Using the example of Aspire.Hosting.Redis RedisBuilderExtensions.cs

public static IResourceBuilder<RedisResource> AddRedis(this IDistributedApplicationBuilder builder, string name, int? port = null)
{
    ArgumentNullException.ThrowIfNull(builder);

    var redis = new RedisResource(name);
    return builder.AddResource(redis)
                  .WithEndpoint(port: port, targetPort: 6379, name: RedisResource.PrimaryEndpointName)
                  .WithImage(RedisContainerImageTags.Image, RedisContainerImageTags.Tag)
                  .WithImageRegistry(RedisContainerImageTags.Registry);
}

[Fact]
public void AddRedisContainerShouldThrowsWhenBuilderIsNull()
{
    IDistributedApplicationBuilder builder = null!;
    const string name = "Redis";

    var action = () => builder.AddRedis(name);

    Assert.Multiple(() =>
    {
        var exception = Assert.Throws<ArgumentNullException>(action);
        Assert.Equal(nameof(builder), exception.ParamName);
    });
}

Attached as an full example CA1062#Aspire.Hosting.Redis

Additional context

When checking, we get a NullReferenceException

Components.*

[ ] - Adding public API test coverage for Aspire.Azure.AI.OpenAI (assigned to @Zombach)
[ ] - Adding public API test coverage for Aspire.Azure.Data.Tables (assigned to @Zombach)
[ ] - Adding public API test coverage for Aspire.Azure.Messaging.EventHubs
[ ] - Adding public API test coverage for Aspire.Azure.Messaging.ServiceBus
[ ] - Adding public API test coverage for Aspire.Azure.Messaging.WebPubSub
[ ] - Adding public API test coverage for Aspire.Azure.Search.Documents
[ ] - Adding public API test coverage for Aspire.Azure.Security.KeyVault
[ ] - Adding public API test coverage for Aspire.Azure.Storage.Blobs
[ ] - Adding public API test coverage for Aspire.Azure.Storage.Queues
[ ] - Adding public API test coverage for Aspire.Confluent.Kafka
[x] - Adding public API test coverage for Aspire.Elastic.Clients.Elasticsearch (assigned to @Alirexaa)
[ ] - Adding public API test coverage for Aspire.Keycloak.Authentication (assigned to @Zombach)
[ ] - Adding public API test coverage for Aspire.Microsoft.Azure.Cosmos (assigned to @Zombach)
[ ] - Adding public API test coverage for Aspire.Microsoft.Data.SqlClient
[ ] - Adding public API test coverage for Aspire.Microsoft.EntityFrameworkCore.Cosmos
[ ] - Adding public API test coverage for Aspire.Microsoft.EntityFrameworkCore.SqlServer
[x] - Adding public API test coverage for Aspire.Milvus.Client (assigned to @Alirexaa)
[x] - Adding public API test coverage for Aspire.MongoDB.Driver (assigned to @Alirexaa)
[ ] - Adding public API test coverage for Aspire.MySqlConnector
[ ] - Adding public API test coverage for Aspire.NATS.Net (assigned to @Alirexaa)
[ ] - Adding public API test coverage for Aspire.Npgsql
[ ] - Adding public API test coverage for Aspire.Npgsql.EntityFrameworkCore.PostgreSQL
[ ] - Adding public API test coverage for Aspire.Oracle.EntityFrameworkCore
[ ] - Adding public API test coverage for Aspire.Pomelo.EntityFrameworkCore.MySql
[ ] - Adding public API test coverage for Aspire.Qdrant.Client
[ ] - Adding public API test coverage for Aspire.RabbitMQ.Client
[ ] - Adding public API test coverage for Aspire.Seq
[ ] - Adding public API test coverage for Aspire.StackExchange.Redis
[ ] - Adding public API test coverage for Aspire.StackExchange.Redis.DistributedCaching
[ ] - Adding public API test coverage for Aspire.StackExchange.Redis.OutputCaching

Hosting.*

[ ] - Adding public API test coverage for Aspire.Hosting.AppHost
[ ] - Adding public API test coverage for Aspire.Hosting.AWS
[ ] - Adding public API test coverage for Aspire.Hosting.Dapr
[ ] - Adding public API test coverage for Aspire.Hosting.Elasticsearch (assigned to @Alirexaa)
[ ] - Adding public API test coverage for Aspire.Hosting.Garnet (assigned to @Zombach)
[x] - Adding public API test coverage for Aspire.Hosting.Kafka (assigned to @Alirexaa)
[x] - Adding public API test coverage for Aspire.Hosting.Keycloak (assigned to @Zombach)
[x] - Adding public API test coverage for Aspire.Hosting.Milvus (assigned to @Alirexaa)
[ ] - Adding public API test coverage for Aspire.Hosting.MongoDB (assigned to @Alirexaa)
[x] - Adding public API test coverage for Aspire.Hosting.MySql (assigned to @Alirexaa)
[ ] - Adding public API test coverage for Aspire.Hosting.Nats (assigned to @Zombach)
[ ] - Adding public API test coverage for Aspire.Hosting.NodeJs (assigned to @Alirexaa)
[ ] - Adding public API test coverage for Aspire.Hosting.Oracle
[ ] - Adding public API test coverage for Aspire.Hosting.Orleans
[ ] - Adding public API test coverage for Aspire.Hosting.PostgreSQL (assigned to @Zombach)
[ ] - Adding public API test coverage for Aspire.Hosting.Python (assigned to @Zombach)
[x] - Adding public API test coverage for Aspire.Hosting.Qdrant (assigned to @Zombach)
[x] - Adding public API test coverage for Aspire.Hosting.RabbitMQ (assigned to @Alirexaa)
[x] - Adding public API test coverage for Aspire.Hosting.Redis (assigned to @Zombach)
[ ] - Adding public API test coverage for Aspire.Hosting.Seq
[x] - Adding public API test coverage for Aspire.Hosting.SqlServer (assigned to @Alirexaa)
[ ] - Adding public API test coverage for Aspire.Hosting.Valkey (assigned to @bell-nat)

ServiceDiscovery.*

[ ] - Adding public API test coverage for Microsoft.Extensions.ServiceDiscovery (assigned to @Zombach)
[ ] - Adding public API test coverage for Microsoft.Extensions.ServiceDiscovery.Abstractions (assigned to @Zombach)
[x] - Adding public API test coverage for Microsoft.Extensions.ServiceDiscovery.Dns (assigned to @Zombach)
[ ] - Adding public API test coverage for Microsoft.Extensions.ServiceDiscovery.Yarp (assigned to @Zombach)

Jul 24 '24 08:07 Zombach

@davidfowl @eerhardt Helloy. If you don't mind, could you familiarize yourself with this task?

Jul 24 '24 09:07 Zombach

It seems to duplicate of #2649

Jul 24 '24 12:07 Alirexaa

It seems to duplicate of #2649

Really. Then you need to understand whether this solution option is suitable and make adjustments if necessary.

Jul 24 '24 12:07 Zombach

@DamianEdwards Good afternoon. Because you are the original creator of this issue #2649. Could you please rate this topic? I would like to resolve this issue and track progress status using a checklist.

But this requires a go-ahead, And also successful Merge CA1062#Aspire.Hosting.Redis To have a working example taking into account the necessary edits

Jul 26 '24 08:07 Zombach

I will work on Aspire.Hosting.Elasticsearch.

Jul 29 '24 21:07 Alirexaa

I will work on Aspire.Hosting.Nats

Jul 31 '24 19:07 Zombach

I will work on Aspire.Hosting.Keycloak

Aug 01 '24 21:08 Zombach

I will work on Aspire.Hosting.Milvus

Aug 01 '24 21:08 Alirexaa

I will work on Aspire.Hosting.PosgreSQL

Aug 02 '24 06:08 Zombach

I will work on Aspire.Hosting.RabbitMQ

Aug 02 '24 12:08 Alirexaa

I will work on Aspire.Hosting.MySql

Aug 02 '24 12:08 Alirexaa

I will work on Adding public API test coverage for Aspire.Hosting.Kafka

Aug 02 '24 16:08 Alirexaa

I will work on Adding public API test coverage for Aspire.Hosting.Garnet

Aug 02 '24 16:08 Zombach

I will work in Aspire.Hosting.NodeJs

Aug 02 '24 16:08 Alirexaa

I will work in Microsoft.Extensions.ServiceDiscovery.Yarp

Aug 04 '24 11:08 Zombach

Add groups Components.* and ServiceDiscovery.*

Aug 04 '24 11:08 Zombach

Add groups Components.* and ServiceDiscovery.*

hmm, a lot of work :) Thanks.

Aug 04 '24 12:08 Alirexaa

I will work on Aspire.Elastic.Clients.Elasticsearch

Aug 04 '24 17:08 Alirexaa

I will work on Aspire.Milvus.Client

Aug 04 '24 17:08 Alirexaa

I will work on Aspire.MongoDB.Driver

Aug 04 '24 17:08 Alirexaa

I will work on Aspire.Hosting.MongoDB

Aug 06 '24 19:08 Alirexaa

I will work on Microsoft.Extensions.ServiceDiscovery.Dns

Aug 08 '24 06:08 Zombach

I will work in Aspire.NATS.Net

Aug 09 '24 13:08 Alirexaa

I will work on Aspire.Hosting.SqlServer

Aug 09 '24 13:08 Alirexaa

@Alirexaa @Zombach Could you make these in batches, it might help reduce the noise and we are on agreement with how these should be done so the back-and-forth should be minimal. Thanks. Mayube one for clients and one for hosting, and if one of you owns it in the fork then they could invite the other so they can work on the same branch if necessary (I can already as a contributor of this repos). Then when I merge I will keep both names as contributors for internet points.

Aug 09 '24 22:08 sebastienros

@Alirexaa @Zombach Could you make these in batches, it might help reduce the noise and we are on agreement with how these should be done so the back-and-forth should be minimal. Thanks. Mayube one for clients and one for hosting, and if one of you owns it in the fork then they could invite the other so they can work on the same branch if necessary (I can already as a contributor of this repos). Then when I merge I will keep both names as contributors for internet points.

Yes, okay. We will do this in blocks.

Aug 09 '24 22:08 Zombach

I'm working with @Alirexaa on #5250 Microsoft.Extensions.ServiceDiscovery.Abstractions Microsoft.Extensions.ServiceDiscovery Aspire.Keycloak.Authentication Aspire.Microsoft.Azure.Cosmos Aspire.Azure.AI.OpenAI Aspire.Azure.Data.Tables Aspire.Hosting.AWS Aspire.Confluent.Kafka Aspire.Azure.Search.Documents Aspire.Azure.Security.KeyVault Aspire.Azure.Storage.Queues Aspire.Hosting.Dapr

Aug 09 '24 23:08 Zombach

I'm working on https://github.com/dotnet/aspire/pull/5402 Aspire.Azure.Messaging.EventHubs Aspire.Azure.Messaging.ServiceBus Aspire.Azure.Messaging.WebPubSub Aspire.Azure.Storage.Blobs Aspire.Microsoft.Data.SqlClient Aspire.Microsoft.EntityFrameworkCore.Cosmos Aspire.Microsoft.EntityFrameworkCore.SqlServer Aspire.MySqlConnector Aspire.Npgsql Aspire.Npgsql.EntityFrameworkCore.PostgreSQL Aspire.Oracle.EntityFrameworkCore

Aug 23 '24 17:08 Zombach

I'm working on https://github.com/dotnet/aspire/pull/5407 Aspire.Pomelo.EntityFrameworkCore.MySql Aspire.Qdrant.Client Aspire.RabbitMQ.Client Aspire.Seq Aspire.StackExchange.Redis Aspire.StackExchange.Redis.DistributedCaching Aspire.StackExchange.Redis.OutputCaching

Aug 23 '24 21:08 Zombach

These projects are frozen for now, since they have not yet moved the test projects into a separate project.

Aspire.Hosting.AppHost Aspire.Hosting.Orleans Aspire.Hosting.Seq

Aug 25 '24 07:08 Zombach

aspire aspire copied to clipboard

Check List: Validate arguments of public methods

Is your feature request related to a problem? Please describe the problem.

Describe the solution you'd like

Additional context

Components.*

Hosting.*

ServiceDiscovery.*

aspire
aspire copied to clipboard