Skip to content
New issue

Have a question about this project? Sign up for a free GitHub account to open an issue and contact its maintainers and the community.

By clicking “Sign up for GitHub”, you agree to our terms of service and privacy statement. We’ll occasionally send you account related emails.

Already on GitHub? Sign in to your account

New nullability annotation to capture GetType(..., throwOnError: true) semantics #2835

Open
jnm2 opened this issue Sep 27, 2019 · 4 comments

Comments

@jnm2
Copy link
Contributor

jnm2 commented Sep 27, 2019

When you call Assembly.GetType("Foo", throwOnError: true), there is a guarantee that it will either throw (not return) or return a non-null value.

It would be nice for throwOnError: true to not require ! whenever it is used, but the current nullability attributes don't provide a way of saying, "Not null if the named parameter is true."

Does something along these lines have merit?

[return: NotNullWhenEqual(nameof(throwOnError), true)]
public virtual Type? GetType(string name, bool throwOnError) => ...
namespace System.Diagnostics.CodeAnalysis
{
    [AttributeUsage(AttributeTargets.Field | AttributeTargets.Parameter
        | AttributeTargets.Property | AttributeTargets.ReturnValue, Inherited = false)]
    public sealed class NotNullAttribute : Attribute
    {
        public NotNullAttribute();
    }
    
    [AttributeUsage(AttributeTargets.Parameter, Inherited = false)]
    public sealed class NotNullWhenAttribute : Attribute
    {
        public NotNullWhenAttribute(bool returnValue);

        public bool ReturnValue { get; }
    }

+   [AttributeUsage(AttributeTargets.Field | AttributeTargets.Parameter
+       | AttributeTargets.Property | AttributeTargets.ReturnValue, Inherited = false)]
+   public sealed class NotNullWhenEqualAttribute : Attribute
+   {
+       public NotNullWhenEqualAttribute(string parameterName, object parameterValue);
+
+       public string ParameterName { get; }
+       
+       public object ParameterValue { get; }
+   }
}
@danmoseley
Copy link
Member

@jaredpar should this go in the dotnet/csharplang repo?

@jaredpar jaredpar transferred this issue from dotnet/corefx Sep 27, 2019
@stephentoub
Copy link
Member

FWIW, we discussed exactly such an attribute for exactly this (and other similar) cases in corelib when designing the initial set of attributes, and it was deemed too niche / too limited of an impact to be worth the extra complexity / cost.

@HaloFour
Copy link
Contributor

Perhaps new methods, like Assembly.GetTypeOrThrow() would be a better addition to the BCL.

@theunrepentantgeek
Copy link

When you call Assembly.GetType("Foo", throwOnError: true), there is a guarantee that it will either throw (not return) or return a non-null value.

Arguably, the problem here is the API design choice to use throwOnError as a parameter instead of providing two distinct methods.

FWIW, a team I once worked on (developing in Delphi) used the convention that GetType() would throw if not found, but FindType() might return nil.

Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
None yet
Projects
None yet
Development

No branches or pull requests

5 participants