CheckParametersNullityInVisibleMethodsRule fix leads to AvoidCodeDuplicatedInSameClassRule break

[Andrew Hanson]

I fixed a few methods in my class that had the "CheckParametersNullityInVisibleMethodsRule" issue.  I opted to do so like this:

If IsNothing(storeCd) Then
     Throw New ArgumentNullException("storeCd")
End If

Upon doing so, now I have AvoidCodeDuplicatedInSameClassRule problems throughout.  I assume it's because I have the same basic structure at the top of every public method, but I thought because they were constructed on unique parameters that wouldn't matter.  Any advice?  Am I handling this incorrectly?

[Sebastien Pouliot]

Hello Andrew,

That *should* be too small to be identified as a 'dupe'. However I don't
have much experience with VB.NET compiler IL generation. It is possible
that the generated code is larger than expected (and large enough to be
seen as a valid pattern).

1) Can you try compiling your code with /o ? and see if the defects are
still reported ?

That's the "optimize" option, not sure if the vb command line options
are identical to the c# compiler). Without /o CSC is generating much
larger (and weirder) IL code. Maybe the VB compiler is doing the same?

2) If the above did not fix the issue could you compile a small sample
for me (two times, one with /o and one without it). Just enough to have
the null check + some code that will trigger the
AvoidCodeDuplicatedInSameClassRule defects.

Next fill a bug in bugzilla.novell.com [1] and attach the files (ZIPped
or not) to the report. Please state which version of the compiler (e.g.
VS2008 or VS2010) you're using.

> Am I handling this incorrectly?

To handle the null parameters, yes. Can't say for sure if the duplicate
code is related to this (or not).

Thanks
Sebastien

p.s. I won't be able to test this myself before Feb 8th week.

[1]
https://bugzilla.novell.com/buglist.cgi?query_format=advanced&bug_status=NEW&bug_status=ASSIGNED&bug_status=NEEDINFO&bug_status=REOPENED&component=Gendarme&product=Mono%3A%20Tools

[Andrew Hanson]

Before I go too far down this road, let me just make sure I'm not screwing up something stupid.  I'm using VS2005, could that be an issue?  I honestly didn't check the levels of support on it, but just wanted to bring that up.

It's definitely possible for the VB compiler to be getting screwy.  I've actually had it flag a single line before as duplicate code.  Sounds impossible, but I commented out the line and it didn't flag it, then un-commented it and it came back.  Logically it made sense to move the line to another point in the method and that ended up fixing it.  Doubt I'll be able to recreate that little bug, but it might be indicative of something weird with the VB compiler.

I hacked together a small sample that is still generating the issue for me.  I have the two versions (one compiled with optimizations and one without optimizations)  so I'll get to uploading them in a little bit.  This isn't like a deal breaker for me I was just wondering if other people saw what I was seeing. 

I actually see a good amount of strange behavior when it comes to using this utility with VB.NET, and so far I've just been ignoring it and skipping over it.  Perhaps I should post a little more of those things over here.

[Sebastien Pouliot]

On Wed, 2011-01-26 at 12:19 -0800, Andrew Hanson wrote:
> Before I go too far down this road, let me just make sure I'm not
> screwing up something stupid. I'm using VS2005, could that be an
> issue?

No. Gendarme should be able to analyze code down to FX 1.0 (VS2002).

However you do need FX3.5 to be installed. If not then Gendarme won't
start (actually it should not even install). Since you're getting
results it means you meet the basic requirements.

Just to be clear: the only time VS2008 (or above) is required is when
you wish to compile Gendarme, from source code, yourself.

> I honestly didn't check the levels of support on it, but just wanted
> to bring that up.
>
> It's definitely possible for the VB compiler to be getting screwy.

Well each .NET compiler is a bit different. That's not an issue for most
rules but, in some cases, the generated IL can be quite weird (like CSC
is when compiled without optimization) - but it's still "legal" IL.

xMSC (mono C# compilers family) and CSC are likely the most commonly
used compilers - or at least the one I'm the most familiar with.

Maybe other people on the list can comment on other compilers ?

> I've actually had it flag a single line before as duplicate code.
> Sounds impossible, but I commented out the line and it didn't flag it,
> then un-commented it and it came back.

The rule works with "patterns" which generally correspond to a few lines
of code. Of course moving a single line can "break" the pattern and
avoid two methods to match.

> Logically it made sense to move the line to another point in the
> method and that ended up fixing it. Doubt I'll be able to recreate
> that little bug, but it might be indicative of something weird with
> the VB compiler.
>
> I hacked together a small sample that is still generating the issue
> for me. I have the two versions (one compiled with optimizations and
> one without optimizations) so I'll get to uploading them in a little
> bit. This isn't like a deal breaker for me I was just wondering if
> other people saw what I was seeing.

Thanks - this is much appreciated :-) It will allow me to investigate
the issue.

Most, except the "weirdest" cases, IL generation difference can often be
accommodated with relatively small fixes to the rule (as long as we know
about it ;-)

> I actually see a good amount of strange behavior when it comes to
> using this utility with VB.NET, and so far I've just been ignoring it
> and skipping over it. Perhaps I should post a little more of those
> things over here.

If you have time please do fill bug reports on them :-) If it helps
(saving time) it's possible to make attachments, or even the bug itself,
private to novell's employees.

Thanks!
Sebastien

[Andrew Hanson]

I entered the bug into bugzilla.

https://bugzilla.novell.com/show_bug.cgi?id=667418

[Andrew Hanson]

Just as sort of a weird follow up, I seem to be getting this with my almost exactly same fix to address the UseObjectDisposedExceptionRule violations I had.  If you want I could probably construct an example for this as well, but like I said I'm doing the exact same thing to fix this rule as I was doing for the previous rule (i.e. check field value, throw exception).

[Sebastien Pouliot]

Thanks, but I think I figured it out.

VB uses helpers, e.g. IsNothing becomes a call to Information.IsNothing. That makes the statement large enough to be considered as a pattern, in turn making similar patterns in each method.

Once 2.10 is released I'll have a deeper look at the rule and see how it can "discard" such patterns.

Thanks for the test cases!
Sebastien