3

I have the following piece of code with warning Expression is always true:

public class MyClass
{
    private readonly IHubContext<NotificationHub> _hubContext;

    public MyClass(
        IHubContext<NotificationHub> hubContext)
    {
        _logHandler = logHandler;
        _hubContext = hubContext;
    }
            
    foreach (string group in groups)        
    {
        IClientProxy connection = _hubContext.Clients.User(group);
        if (connection != null) // Expression is always true
        {
        }
    }
}

I know that Resharper is very smart, however I cannot understand why it says Expression is always true as IClientProxy is reference type. IClientProxy resides in Microsoft.AspNetCore.SignalR namespace. Do you know why Resharper says Expression is always true?

This is how User is declared in public interface IHubClients<T>:

public interface IHubClients<T> 
{
    /// <summary>
    /// Gets a <typeparamref name="T" /> that can be used to invoke 
    /// methods on all connections associated with the specified user.
    /// </summary>
    /// <param name="userId">The user ID.</param>
    /// <returns>A client caller.</returns>
    T User(string userId);
}

UPDATE:

If I add ? sign:

IClientProxy? connection = _hubContext.Clients.User(group);

Then I will get the following warning:

The annotation for nullable reference types should only be used in code within a '#nullable' annotations context.

UPDATE 1

Even if I use var:

var IClientProxy? connection = _hubContext.Clients.User(group);
if (connection != null)

then warning is arised Expression is always true:

enter image description here

Learner
  • 417
  • 6
  • 24
  • 1
    Presumably this is using NRT annotations to declare that `User(group)` always returns a non-null value? i.e. the difference between `IClientProxy? Foo()` and `IClientProxy Foo()` when `#nullable enable` or the equivalent in the csproj is used – Marc Gravell Jan 09 '23 at 13:47
  • @MarcGravell I am sorry but what is NRT? – Learner Jan 09 '23 at 13:48
  • 1
    "nullable reference types" - compiler assisted decoration of when a reference is expected to possibly vs never contain null values; https://learn.microsoft.com/en-us/dotnet/csharp/nullable-references – Marc Gravell Jan 09 '23 at 13:49
  • 2
    Because you declared it as `IClientProxy connection` and not `IClientProxy? connection`, you are telling the compiler that you think that `connection` is not null. Thus, the compiler complains when you go on to test if the thing you said can't be null, is actually null. – Matthew Watson Jan 09 '23 at 13:50
  • @MatthewWatson I've tried to add `?` sign and I've got the following warning `The annotation for nullable reference types should only be used in code within a '#nullable' annotations context.` – Learner Jan 09 '23 at 13:59
  • @MarcGravell Do you mean that if `User` property declared as nullable like this `T? User(string userId);`, then warning would vanish? I've added a piece of code where `T User(string userId);` is declared in interface. Please, see please my question – Learner Jan 09 '23 at 14:04
  • @Learner well, you'd need to use `IClientProxy?` too in the variable declaration - or just `var` (and it will be implied from the target method) – Marc Gravell Jan 09 '23 at 14:56
  • @MarcGravell I've tried, but it continue to show this warning `Expression is always true`. I've attached an image to my question. – Learner Jan 09 '23 at 15:15
  • 2
    So it seems that [`IHubClients.Client(string)`](https://learn.microsoft.com/en-us/dotnet/api/microsoft.aspnetcore.signalr.ihubclients-1.client?view=aspnetcore-7.0) is declared to not return null, so perhaps that's what causes it. Look at [the implementation here](https://source.dot.net/#Microsoft.AspNetCore.SignalR.Core/Internal/HubClients%2560T.cs) - it never returns null, as per the interface definition, so you shouldn't be checking it for null. – Matthew Watson Jan 09 '23 at 15:30
  • You should enable `enable` in your .csproj PropertyGroup – theemee Jan 09 '23 at 15:31
  • @MatthewWatson could you please explain how you figured out that **it never returns null**? Could you please explain in plain English? Feel free to make an answer! Thanks! – Learner Jan 09 '23 at 16:59
  • @theemee could you please explain or give a link to read more about `enable` – Learner Jan 10 '23 at 06:26
  • 1
    @Learner You can see that it never returns null in two ways: (1) The interface is declared as `public T Client (string connectionId)` rather than `public T? Client (string connectionId)` (i.e. it doesn't have the `?` to indicate that it can be nullable. (2) Inspecting the implementation confirms that the code always returns non-null. – Matthew Watson Jan 10 '23 at 08:50
  • 1
    @Learner https://stackoverflow.com/a/56267555 – theemee Jan 10 '23 at 10:03

1 Answers1

2

Firstly, we should note that the warning is from Resharper, and not the actual compiler. If you disable Resharper, that particular warning will go away.

That said, it is a valid warning. To understand why, consider the following interface definition:

public interface IDemo<T>
{
    public T Func();
}

That definition states that Func() will not return null. If it could return null, it would be defined using T? thus: public T? Func();

A simple implementation of that interface could look like this:

public sealed class Demo<T>: IDemo<T> where T: new()
{
    public T Func()
    {
        return new T();
    }
}

This compiles OK. However, if you change the implementation of Func() to:

public T Func()
{
    return null;
}

you will get a real (not Resharper) compile error:

error CS0403: Cannot convert null to type parameter 'T' because it could be a non-nullable value type. Consider using 'default(T)' instead.
warning CS8603: Possible null reference return.

Now consider this code (where I chose an arbitrary type with a default constructor for illustration, in this case Random):

IDemo<Random> test = new Demo<Random>();

if (test.Func() != null)
{
    Console.WriteLine("Not null");
}

This provokes the Resharper warning that you're seeing, the reason being that IDemo<Random>.Func() has been specified to return non-null, as explained above.

The method you are using is declared as public T IHubClients<T>.Client(String) - i.e. it returns T and not T? - which means it can never (or should never) be null.

Thus when you check it for null, Resharper is warning you because you should not need to do that.

Matthew Watson
  • 104,400
  • 10
  • 158
  • 276