[Mono-dev] Next Gendarme release(s)

[Sebastien Pouliot]

Hello Christian,

On Fri, 2006-09-29 at 12:54 +0200, Christian Birkl wrote:
> Hi Sebastien,
> 
> 2006/9/13, Sebastien Pouliot sebastien at ximian.com: 
>         
>         Last note: I plan on releasing another Gendarme version at, or
>         after,
>         the meeting - possibly including some participants feedback.
>         Let me know 
>         if you have bigger changes ready before mid-October.
>  
> I'd like to see the following changes in the next gendarme release
> (well, I guess not all will make it ;-) - some are cosmetic, some make
> rule writing more easier imho, ...)
>  
> 1) new ctor overloads for Location
>  
> - Location (AssemblyDefinition)
> - Location (ModuleDefinition)
> - Location (TypeReference)
> - Location (MethodDefinition)
> - Location (MethodDefinition, Instruction ins)
>  
> Currently nearly every rule builds the location strings by themselves
> (new Location(method.DeclaringType.FullName, method.Name,
> insn.Offset)). Imho those new overloads make the code more readable by
> writing code like new Location (method, ins) or new Location (type).

It makes sense to have helper ctors. Please provide a patch and it
should go quickly into SVN ;-)

> 2) message texts should be stored in <assembly>.xml
>  
> An example: UseStringEmptyRule.cs: Message msg = new Message
> ("instance of an empty string has been found.", loc,
> MessageType.Warning). But on the other side rule problem and solution
> are stored in the rule information xml file. This is just a "to be
> consistent" change, but perhaps will help you later to bring i8n to
> gendarme. A proposal:
>  
> <rule>
>   <problem>...</problem>
>   <solution>...</solution>
>   <messages>
>      <message Id="instance-found">instance of an empty string has been
> found.</message>
>   </messages>
> </rule>
>  
> Code: new Message ("instance-found", new Location (method, ins));

Using the XML files for string was my original idea. However I'm not
sure about that anymore. It's definitively something I want to talk with
some folks at the mono meeting. Ideally we would find a way to make it
easy (or at least identical) to i18n the rules, framework and all
(present and future) runners.

Any comments are welcome on the subject (and feel free to email me a
patch to the gendarme TODO file so we don't forget about this).

> 3) Remove the full qualified assembly name inside rule information
> file.
>  
> Currently each rule stores its assembly name via a Makefile
> preconditioned attribute
> (Type="Gendarme.Rules.Performance.EmptyDestructorRule,
> Gendarme.Rules.Performance, Version=@VERSION@, Culture=neutral,
> PublicKeyToken=null"). As proposed before this may not be necessary if
> you don't want to load your rules from the GAC (which imho will never
> happen). By just specifying Type and/or AssemblyName you could drop
> the Makefile dependency and let other developers easier work with
> these files. It's not very "sexy" to modify those files manually if
> you are using a IDE (like Visual Studio or #Develop).

You're probably right. Still it's a change I prefer to wait implementing
until we have a (or many ;-) GUI runners (so we don't end up changing
this many times over many versions).

(please feel free to email me a patch to the gendarme TODO file).

> 4) Redefine "Rule Failure" and "Rule Success"
>  
> With the current framework it is possible to create a violation by
> just returning an empty collection. IMHO each violation should have a
> location to make tracking down the source easier. My proposal is that
> rule "fails" if one or more messages are returned by the CheckXXX
> methods. 

Yes, it make sense. Actually the current implementation is mixed between
this (new rules) and a boolean (old rules)...

> On a side node, I don't like the "static RuleFailure" and
> "RuleSuccess" properties which imho can be dropped if the above change
> may be accepted.

That part was added to ease old (boolean) rules migration with the API
change - i.e. it can be removed once all rules are updated to the latest
API. Patches are welcome to complete this migration (and remove the
helpers).

> As always, RFC :-)

Thanks!

> Christian
> _______________________________________________
> Mono-devel-list mailing list
> Mono-devel-list at lists.ximian.com
> http://lists.ximian.com/mailman/listinfo/mono-devel-list
-- 
Sebastien Pouliot  <sebastien at ximian.com>
Blog: http://pages.infinit.net/ctech/