bkoelman / ResharperCodeContractNullability

Reports diagnostics, helping you to annotate your source tree with (Item)NotNull / (Item)CanBeNull attributes.
https://www.jetbrains.com/resharper/help/Code_Analysis__Code_Annotations.html
Apache License 2.0
26 stars 4 forks source link

Add ability to hide warnings for private fields #15

Closed imanushin closed 8 years ago

imanushin commented 8 years ago

In the some projects, private read-only fields are not null (because they were set from ctor), so it is better to have configurable parameter to ignore such statements. In the current moment, all of them should have NotNull attribute, which increase code size without the any benefits.

bkoelman commented 8 years ago

Hi @imanushin, can you provide an example?

I just discovered that Resharper v9.2 only gives null warnings when the NotNull/CanBeNull attributes are defined in the JetBrains.Annotations namespace. This changed since I started this project.

Perhaps the bug here is that ResharperCodeContractNullability allows the attributes in any namespace, which is no longer correct.

bkoelman commented 8 years ago

Having the attributes defined in the JetBrains.Annotations namespace does trigger the Resharper warning "Expression is always false" in the example below. It disappears after removing the [NotNull] on _some, so it looks to me putting the attribute there is helpful.

image

bkoelman commented 8 years ago

Perhaps the bug here is that ResharperCodeContractNullability allows the attributes in any namespace, which is no longer correct.

As it turns out, Resharper (today and earlier versions) searches for attributes only in namespaces that are set in Resharper > Options > Code Annotations > Annotations namespace to use. Because ResharperCodeContractNullability does not parse the Resharper solution/project/machine settings hierarchy, it just searches all namespaces (and caches the result).

So, although the mechanism is not entirely the same, it is usually sufficiently the same.

To get back to your scenario: can you verify that the correct namespace is set in Resharper options?

imanushin commented 8 years ago

Steps to reproduce:

  1. Download project
  2. Open file
  3. Go to line 10, to field "GameModel _initialModel"

Expected result: It is not marked as "RNUL"

Actual result: It is market as "RNUL"

I'll try to fix it, by dividing warnings to the separate warnings: for fields, for properties, etc.

bkoelman commented 8 years ago

Thanks for providing an example. From what I understand, the GameModel type is a class, so a reference type. Which is a candidate for annotation.

I understand your concern that putting the attribute on the field as well as on the ctor parameter is redundant. But the thing is: Resharper is not so smart to see that. I'm not near a compiler the next days, but from my testing in the past it does make Resharper apply nullability rules when you add the attribute to the field.

Try removing it or adding it on the field, then see a Resharper warning appear/disappear when you type in another method:

var test = _initialModel.Data;

If you can provide an example that shows the attribute on the field is redundant, I'll investigate why. Until then, I believe the best way forward is to submit a bug at JetBrains to make Resharper smarter. Once that's in place, I'll update this analyzer accordingly.

Hope that helps.