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

Consider adding R# annotations #207

Closed
GoogleCodeExporter opened this Issue Mar 15, 2015 · 12 comments

Comments

Projects
None yet
2 participants
@GoogleCodeExporter

GoogleCodeExporter commented Mar 15, 2015

ReSharper is very widely used, and has some annotations which we could apply 
within Noda Time to indicate nullity, purity etc:

http://www.jetbrains.com/resharper/webhelp/Code_Analysis__Annotations_in_Source_
Code.html

We definitely wouldn't want to add a binary dependency, and we may well want to 
move the annotations to another namespace, but that would still be useful:

- We'd get the benefits ourselves when developing Noda Time
- Other developers could configure R# to notice the Noda Time copy of the 
annotations too, at which point it would be able to help them use Noda Time 
more effectively.

It may be worth trying to find other examples of open source projects doing 
this, to see how they've found it.

Original issue reported on code.google.com by jonathan.skeet on 4 Apr 2013 at 8:31

@GoogleCodeExporter

This comment has been minimized.

GoogleCodeExporter commented Mar 15, 2015

Any thoughts about the feasibility/desirability of doing this for 1.2.0?

Original comment by malcolm.rowe on 26 Jul 2013 at 10:07

@GoogleCodeExporter

This comment has been minimized.

GoogleCodeExporter commented Mar 15, 2015

Haven't put any more thought into it; we're close enough to the 1.2.0 release 
that I don't really want to *start* thinking about it properly - let's punt to 
1.3.

Original comment by jonathan.skeet on 26 Jul 2013 at 10:22

  • Added labels: Milestone-1.3-consider
  • Removed labels: Milestone-1.2-consider
@GoogleCodeExporter

This comment has been minimized.

GoogleCodeExporter commented Mar 15, 2015

A cleaner solution to this, which wouldn't embedded any tool-specific noise 
into the code, might be to use Resharper's XML External Annotations feature.
All that would be needed would be to include a file called 
NodaTime.ExternalAnnotations.xml alongside the DLL - just like the docs XML 
file is at present.  People without Resharper could just ignore it, and people 
with Resharper would get the benefit of the annotations automatically.

I've attached a sample (only contains four methods) - put this into the nuget 
package installation directory (i.e. alongside the .DLL), and then 
unload/reload the solution in VS.

I'm not highly experienced with Resharper external annotations, but it seems 
from experimentation that methods which take parameters need to have them 
listed explicitly in brackets, and methods which take nothing must not have 
empty brackets.



Original comment by elmcroft on 28 Oct 2013 at 2:37

Attachments:

@GoogleCodeExporter

This comment has been minimized.

GoogleCodeExporter commented Mar 15, 2015

Hmm. It's a nice idea, but I don't think we'd want to maintain it *as a 
separate XML file*. If we could generate it conditionally having put the 
attributes on the members as normal, that would be great. (So have one 
configuration where the attributes were actually applied, and others where 
they're not.) I don't know if the R# attributes *are* conditional at all - and 
I've never even tried applying attributes conditionally. Will look into it.

Original comment by jonathan.skeet on 29 Oct 2013 at 6:54

@GoogleCodeExporter

This comment has been minimized.

GoogleCodeExporter commented Mar 15, 2015

I'm talking with JetBrains about the best way to move forward on this. There 
are various options.

Original comment by jonathan.skeet on 29 Oct 2013 at 8:05

@GoogleCodeExporter

This comment has been minimized.

GoogleCodeExporter commented Mar 15, 2015

Okay, I now have a bunch of annotations applied, and a copy of subset of the 
JetBrains attributes included in the Noda Time source repository, changed to be 
internal - but still in the same namespace. We also have a NodaTime.Annotations 
namespace for annotations that we find useful that aren't part of R#.

We may consider changing the R# annotations namespace once R# has a 
self-discovery mechanism.

Original comment by jonathan.skeet on 8 Nov 2013 at 10:35

  • Changed state: Fixed
@GoogleCodeExporter

This comment has been minimized.

GoogleCodeExporter commented Mar 15, 2015

Original comment by malcolm.rowe on 22 Jun 2014 at 12:15

  • Added labels: Milestone-1.3.0
  • Removed labels: Milestone-1.3-consider
@GoogleCodeExporter

This comment has been minimized.

GoogleCodeExporter commented Mar 15, 2015

Hi,

I am using NodaTime 1.3, and I am using ReSharper, but I am not using R# 
annotations.  However, since I added NodaTime throughout my solution, R# has 
found some classes in the JetBrains.Annotations namespace which are actually 
inside the NodaTime assembly, and is adding NotNull attributes on my arguments. 
This is *really* annoying, because I have to manually remove NotNull attributes 
every time I add a null check. Is there any way to revisit this to work in a 
less invasive way?


Original comment by appleche...@gmail.com on 8 Jul 2014 at 11:30

@GoogleCodeExporter

This comment has been minimized.

GoogleCodeExporter commented Mar 15, 2015

Can you say exactly what you mean by "every time I add a null check"? A null 
check to what? Perhaps these are things which shouldn't require a null check at 
all...

Also, I suspect you can edit the R# options to determine how it uses the 
annotations.

Original comment by jonathan.skeet on 8 Jul 2014 at 11:51

@GoogleCodeExporter

This comment has been minimized.

GoogleCodeExporter commented Mar 15, 2015

I've tried to reproduce this problem myself, and I can't make R# introduce the 
annotations automatically. Could you open a new issue giving the details of 
exactly 
what you're doing, what version of R# you're using etc? If you could include a 
small program and explain "When I add this method, it does [...]" that would be 
really useful. If you're getting a sucky experience, I definitely want to help 
with that - but I need to be able to reproduce it first.

Original comment by jonathan.skeet on 8 Jul 2014 at 3:51

@GoogleCodeExporter

This comment has been minimized.

GoogleCodeExporter commented Mar 15, 2015

Sure, I will try to get a small project together to reproduce this :)

Original comment by appleche...@gmail.com on 9 Jul 2014 at 11:47

@GoogleCodeExporter

This comment has been minimized.

GoogleCodeExporter commented Mar 15, 2015

I've raised issue 300 to cover this, now that I've had more information via a 
Stack Overflow question. I think what I'd missed was the way you were adding 
the non-null check - with a context action, right?

Original comment by jonathan.skeet on 19 Jul 2014 at 9:12

@malcolmr malcolmr modified the milestone: 1.3.0 Mar 15, 2015

Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment