views:

187

answers:

5

What definable static code checking rule do you wish to see added to FxCop and/or Gendarme?

Why to do you wish to see the rule added, e.g what are the benefits etc?

How could your rule be implemented?

+1  A: 

I'd like to define and implement my own rules very quickly. I tried this once for FxCop, but I found the API not to be very clear - and there was not too much documentation around. I used FxCop 1.36, maybe things changed ...

So I'd like to see FxCop having a clear and easy to use interface ... that would be great :)

The rules I tried to implement were:

  • DocumentInternalMethods
  • DocumentInternalTypes
  • ...

Basically I wanted to enforce xml-comments on non-public members.

tanascius
Use StyleCop for this, it has a rule for that builtin. XML-comments are not compiled to the assembly anyway, so the FXCop cannot see if those are defined or not, since it only analyses the compiled assemblies.
BeowulfOF
I know about StyleCop, but at this time I wanted to implement it in FxCop - and I did that. You are right, you have to load the xml-files separately.
tanascius
+1  A: 

Personally, I would prefer to see not using IDisposable implementations in using statements.

So if you had code like this:

FileStream fs = new FileStream(...);

// Other code.

fs.Dispose();

It would tell you to use it in a using statement.

The benefit would be that it would alert you to cases you might not be aware of where objects that should be disposed are not being disposed in a timely manner.

However, there are enough times where it's a valid situation to NOT declare IDisposable implementations in a using statement for a rule like this to become a pain very quickly. Most often, this case is taking an IDisposable implementation as a parameter to a method.

What I do not mean is usages of classes where the implementation details remove the need for calling Disposable, (e.g. MemoryStream or DataContext); those implement IDisposable and should always have Dispose called on them, regardless of the implementation details, as it is always better to code against the contract exposed.

casperOne
@Ian Ringrose: Updated my response to clarify.
casperOne
+1  A: 

I'd really like the binary analysis to be smart enough to recognize the possibility of an interface.

If it could determine from approaching the defined types and their members, if there are commons that could be extrapolated into an interface.

Clearly, this should not be more than a warning, since it is sometimes whished to explicitly not use an interface.

BeowulfOF
A: 

Upon thinkin about this, I too would like to see the binary analysis to be smart enough to check possible downgrade of access modifiers.

It shouldn't be to hard to determine if a class, property or method could be more restricted.

BeowulfOF
+1  A: 

You can have a glance at NDepend and its Code Query language (CQL). CQL is dedicated to write code quality rules that can be verified live in Visual Studio, or that can be verified during build process and reported in a HTML report. Here are a few samples of CQL rules (designed to be highly customizable):

Code refactored recently should be 100% covered by test:

WARN IF Count > 0 IN SELECT METHODS WHERE CodeWasChanged AND PercentageCoverage < 100

Complex methods should be commented:

WARN IF Count > 0 IN SELECT METHODS WHERE CyclomaticComplexity > 15 AND PercentageComment < 10

I don’t want that my User Interface layer to depend directly on the DataBase layer:

WARN IF Count > 0 IN SELECT NAMESPACES WHERE IsDirectlyUsing "DataLayer" AND NameIs "UILayer"

Static fields should not be named m_XXX (Custom naming conventions):

WARN IF Count > 0 IN SELECT FIELDS WHERE NameLike "^m_" AND IsStatic

Methods out of MyAssembly and MyAssembly2 should not have more than 30 lines of code:

WARN IF Count > 0 IN SELECT METHODS OUT OF ASSEMBLIES "MyAssembly1", "MyAssembly2" WHERE NbLinesOfCode > 30

Public methods should not be removed to avoid API breaking changes:

WARN IF Count > 0 IN SELECT METHODS WHERE IsPublic AND IsInOlderBuild AND WasRemoved

Types tagged with the attribute MyNamespace.FullCoveredAttribute must be thoroughly covered by tests:

WARN IF Count > 0 IN SELECT TYPES WHERE HasAttribute "MyNamespace.FullCoveredAttribute" AND PercentageCoverage < 100

Patrick Smacchia - NDepend dev
@Patrick: Can you please disclose your affiliation with your product in each post where you promote it. I've looked through some of your answers and I think they're quite useful and *not spammy at all*, it's just a general requirement that we're asking everyone to follow to avoid the appearance of [Astroturfing](http://en.wikipedia.org/wiki/Astroturfing). See the [FAQ](http://stackoverflow.com/faq) for the official policy statement. Thanks.
Bill the Lizard
Sure, I updated my stackoverflow name to: Patrick Smacchia - NDepend dev
Patrick Smacchia - NDepend dev