search

Fody / NullGuard

Adds null argument checks to an assembly
MIT License
686 stars 55 forks source link

Make the attributes configurable #4

Closed haacked closed 11 years ago

haacked commented 11 years ago

I like that you moved the AllowNullAttribute into the ReferenceAssembly. Someone filed an issue against NullGuard to rename it to CanBeNullAttribute so that R# understands it.

I was planning to simply make it configurable. You can tell it which attribute is your "can be null" attribute and it'll look for that one.

Also, when this is done, I will probably submodule it into my NullGuard project. Great stuff!

distantcam commented 11 years ago

What's the benefit of making the attribute configurable? It's just an extra option that puts mental weight on someone wanting to use this package.

SimonCropp commented 11 years ago

I think the point @Haacked is making that certain tools like resharper have a feature that allows code time suggestions for argument validation. Resharep calls this "Annotations" http://www.jetbrains.com/resharper/webhelp/Code_Analysis__Annotations_in_Source_Code.html

So if we make our attribute names configurable people can tweak them to get extra value.

Is that correct @Haacked ?

@Haacked the problem with this is that the current approach of this fody addin is to remove the reference at compile time. So we trim all the attributes from the metadata. This allows people to deploy their assembly with no extra baggage. This would effectively render the r# integration moot.

An alternative approach would be to merge the attribute into the target assembly

Another approach is to hack the target xml doc file and use external annotations http://www.jetbrains.com/resharper/webhelp/Code_Analysis__External_Annotations.html This would be more compatible with the "no file to deploy" approach

With this approach i think making attribute configurable is less important.

Thoughts?

haacked commented 11 years ago

Ah, I didn't realize the reference is removed. So in the final compiled code, there's no attributes. In that case, you can probably close this.

SimonCropp commented 11 years ago

just to being clear the "reference being removed" is a design decision of each addin. Fody does not enforce this. this addin can decide not remove the reference. but the result is that the ref assembly needs to be shipped,

haacked commented 11 years ago

Ah, I see. So in the case of NullGuard.Fody, you decided to remove the reference so the rewritten code that's shipped is pretty much exactly what you would have hand-written. Right? Thats' what I was hoping for. :)

SimonCropp commented 11 years ago

Yeah people have like this approach in the past. Means fody addins can have no runtime dependencies.

But as mentioned r# also supports annotations in xml. the addin could hack the XML. this would still allow "no runtime dependencies" and get the code time intellisense.

haacked commented 11 years ago

Very cool! How far along is Fody.NullGuard? Ready for prime time?

SimonCropp commented 11 years ago

well @distantcam found one issue https://github.com/SimonCropp/NullGuard.Fody/issues/5 we should also throw some async/await scenarios at it

I will throw it at some large codebases and then peverify them. will try with nservicebus and ravendb.

as "Ready for prime time?" i would not stick a "1.0" version on it until it gets a little more testing