Option to generate NotNull attributes for implicitly nullable items

[fschmied]

It would be great if NullGuard had an option of adding NotNull attributes (type to be configured by the user) to those items (parameters, method return values, property values) it guards against null values. This would enable library and framework authors to auto-generate nullability annotations (consistent with the actual null checks) to be interpreted by ReSharper and similar tools.

E.g., library source code:

public void Foo (string s1, [CanBeNull] string s2)
{
  // code relying on s1 not being null, whereas s2 can be null
}

Representation of woven library code:

public void Foo ([NotNull] string s1, [CanBeNull] string s2)
{
  if (s1 == null)
    throw new ArgumentException ("NullGuard...");

  // code relying on s1 not being null, whereas s2 can be null
}

Library usage:

Foo(null, "x"); // ReSharper warning: Possible 'null' assignment to entity marked with 'NotNull' attribute

[drauch]

Would love that as well :+1:

[ulrichb]

Yes, it would be great if consumers of a NullGuard-rewritten library which use ReSharper get these nullability annotations "for free".

Regarding the implementation, I see two possibilities to get references to the JetBrains annotation attributes during the weaving process.

1. Reference the attributes in a configuration assembly attribute.

[assembly: AnnotateNullChecksWith(typeof(JetBrains.Annotations.NotNullAttribute))]

• PRO: Would not depend on JetBrains annotations (arbitrary attributes could be injected).
• CON: Here the problem is that ReSharper expects the [NotNull] attribute for return values on the method instead of a return value attribute. This means that
  • a) we would have to implement this feature especially for ReSharper again, or
  • b) to be more generic, we could use a "InjectAttributeOnMethodInsteadOfReturnValue"-flag in the configuration assembly attribute, or
  • c) we could evaluate the AttributeTargets on the [NotNull] attribute (make it dependent on AttributeTargets.ReturnValue)

2. Use internalized JetBrains annotation attributes. (as described in this bog post)

E.g. inject an internal JetBrains.Annotations.NotNullAttribute class type into the rewritten assembly and use this to inject the [NotNull] attributes.

• PRO: The library-writers doesn't have to reference (or declare) JetBrains annotations if they doesn't use ReSharper by themselves. If the annotations would be injected by default (or opt-out) library-consumer, which use ReSharper, would get the annotations and therefore a stronger ReSharper analysis for free. (The library writer would just have to update NullGuard.)
• CON: Complexity. We would first have to check if in the rewritten assembly already contains a JetBrains.Annotations.NotNullAttribute and, if not, inject one.

I tend to option 2 with opt-out configuration because it shouldn't be that complicated to implement the check and create a type with one attribute and a default c'tor in Cecil and we would get this zero-configuration benefit for ReSharper users of NullGuard-rewritten assemblies.

@maintainers: Would you accept a PR?

[distantcam]

@ulrichb Sorry to get back to you so late.

Yes I would accept a PR for option 2. That seems the best approach instead of trying to add an external reference to a library.

[ulrichb]

Hi @distantcam,

OK. Many thanks!

Just to make it sure: In option 1, not NullGuard, but the developers of consuming projects would have to reference JetBrains.Annotations or copy the attribute source code into an own library (as described in the blog post linked above). Afterwards, they could use [assembly: AnnotateNullChecksWith(​typeof(​NotNullAttribute))] with the referenced NotNullAttribute.

[tpluscode]

@distantcam what if NullGuard pulled JetBrainsAnnotations.Fody as dependency to ensure that JetBrains.Annotations is referenced?

[SimonCropp]

Will close this for now. Pending a non wip pr being submitted