Shared code analysis annotations

0 views
Skip to first unread message

Steve Dunn

unread,
Nov 12, 2009, 6:19:35 AM11/12/09
to Gendarme
Hi,
Love Gendarme, great work!
I've found that it's giving me false positives, epecially for the rule
'CheckParametersNullityInVisibleMethodsRule'.
To be fair, Gendarme is correct in what it's reporting as I wrap 'if
(arg==null) throw .... in a wrapper. e.g.

public XmlWrapper(XNode xml)
{
Guard.ArgumentNotNull( @"xml", xml );
...

Guard.ArgumentNotNull looke like (in it's plain form, I'll explain in
a bit!):
public static void ArgumentNotNull( string argName, object
argValue)
{
if ( argValue == null )
{
throw new ArgumentNullException( argName );
}
}

Now, it'd be very difficult for Gendarme (and other code analysis
tools) to figure out what's intended here (throw an exception if the
parameter is null).

Here's the crux of my post:
I use ReSharer a lot, and that has a set of attributes that give hints
as to what you intend, so for the method above, with attributes, it
looks like:
[AssertionMethod]
public static void ArgumentNotNull(
[InvokerParameterName] string argName,
[AssertionCondition( AssertionConditionType.IS_NOT_NULL)]
object argValue)
{
if ( argValue == null )
{
throw new ArgumentNullException( argName );
}
}

[AssertionMethod] means this method asserts something
[AssertionCondition] is the actual condition
[InvokerParameterName] is not related to this issue (it helps R#
ensure that the string passed representing a parameter name is an
actual parameter)

It would be great if there were a set of common code analysis
annotations that could be shared between all the code analysis tools
(R#, Gendarme, FxCop etc.)

As well as the R# annotations, I've seen others (like Lokad) extend
these with attributes for types that should be treated as immutable).

What do you think?

Cheers,

Steve
http://dunnhq.com

Sebastien Pouliot

unread,
Nov 15, 2009, 11:31:00 AM11/15/09
to Gendarme
Hello Steve,

On Nov 12, 6:19 am, Steve Dunn <steve.j.d...@gmail.com> wrote:
> Hi,
> Love Gendarme, great work!

Thanks :)

> I've found that it's giving me false positives, epecially for the rule
> 'CheckParametersNullityInVisibleMethodsRule'.
> To be fair, Gendarme is correct in what it's reporting as I wrap 'if
> (arg==null) throw .... in a wrapper.  e.g.
>
>         public XmlWrapper(XNode xml)
>         {
>             Guard.ArgumentNotNull( @"xml", xml );
> ...
>
> Guard.ArgumentNotNull looke like (in it's plain form, I'll explain in
> a bit!):
>         public static void ArgumentNotNull( string argName, object
> argValue)
>         {
>             if ( argValue == null )
>             {
>                 throw new ArgumentNullException( argName );
>             }
>         }
>
> Now, it'd be very difficult for Gendarme (and other code analysis
> tools) to figure out what's intended here (throw an exception if the
> parameter is null).

Right. The current code only deals with a "method-by-method" basis.

> Here's the crux of my post:
> I use ReSharer a lot, and that has a set of attributes that give hints
> as to what you intend, so for the method above, with attributes, it
> looks like:
>         [AssertionMethod]
>         public static void ArgumentNotNull(
>             [InvokerParameterName] string argName,
>             [AssertionCondition( AssertionConditionType.IS_NOT_NULL)]
> object argValue)
>         {
>             if ( argValue == null )
>             {
>                 throw new ArgumentNullException( argName );
>             }
>         }
>
> [AssertionMethod] means this method asserts something
> [AssertionCondition] is the actual condition
> [InvokerParameterName] is not related to this issue (it helps R#
> ensure that the string passed representing a parameter name is an
> actual parameter)
>
> It would be great if there were a set of common code analysis
> annotations that could be shared between all the code analysis tools
> (R#, Gendarme, FxCop etc.)

A common set would be nice. However I'm putting my (personal ;-) faith
in code contracts.

> As well as the R# annotations, I've seen others (like Lokad) extend
> these with attributes for types that should be treated as immutable).
>
> What do you think?

I'm not a Resharper user, actually I don't know if any Gendarme hacker
that use it (maybe JB?) or how many Gendarme users, so it's a bit hard
to comment (i.e. take this with a grain of salt ;-).

The general rule is to keep rules as simple as possible, i.e. avoiding
2x complexity for a 5% gain. In this case I feel it's not worth
enhancing the _existing_ rules to support R#.

OTOH I'm 100% for (a) reusing existing rules and (b) having
specialized rules. So if anyone is interested in having "R#" enhanced
rules then they would be very welcome into Gendarme (e.g.
Gendarme.Rules.Resharper).

Back to code contract it's not clear (yet) how we'll be using them
into Gendarme (inside the default rules or specialized rules).
Hopefully we'll be able to play a bit with the feature before Fx4.0
gets released.

Thanks,
Sebastien

p.s. it would be nice if you could send some links to R# documentation
(Lokad too) to this list. That could give us some (other) ideas and a
better idea of the complexity versus gain that could be achieved.
Reply all
Reply to author
Forward
0 new messages