[deleted comment]

Hi there!
What exactly do you expect in this task? Is it also about changing the 
interface?

Wrong URL, it should be: http://www.mono-project.com/Gendarme
Another useful one is: http://groups.google.com/group/gendarme

I claim this task.

Can you please give me some additional time for that task?
Unfortunately it happened so it's only now that I have time to begin working on 
it.
And, by the way, what is expected amount of work?
I'm now working on implementing some of those naming rules:
http://www.gotdotnet.com/Team/FxCop/Docs/Rules/Naming.html

The expected delivery is one naming rule, for a week long work. To be accepted, 
a
rule has to come with its corresponding tests, and the patch ready to be 
integrated.
Also, you should check that Gendarme doesn't already have a similar rule.

Huh, it's okay then, as I have already implemented four :-)

Somewhy I couldn't manage to get a working svn diff so I just attach "new"
Gendarme.Rules.Naming directory.
To make it work, checkout the latest sources, delete _everything_ in
rules/Gendarme.Rules.Naming directory and extract tarball's contents into it.
Hope that'll work.

Will anyone check that out?

Of course we will.

Thing is that we sometimes tend to be busy you know?

I can't review the code if it's no in unified diff (svn diff).
Moreover, the code as it is mixes spaces and tabs, so it's:

1) hard to review
2) unmergable with gendarme.

Hello Dan,
Could you explain the problem you had to produce a diff using SVN ?
That would ease the review quite a bit.
Thanks!

Thank you very much for your comments.

Tabs and spaces mix must be my MonoDevelop settings problem. I'll be more 
careful
next time. However isn't this just a matter of replacing all '        ' with 
'\t'?
Anyways, I'll fix it this evening.

And then I will try svn diff it once again.
Thanks!

Dan: Tabs-to-spaces can be a lossy conversion. For example, my preferred style 
is to
use tabs for indenting scopes, and spaces for aligning things past the 
indentation
level of the scope. That way you can change the displayed tab width without 
visually
messing up code alignment. This is the style that MonoDevelop's "smart" 
indednter
uses right now. Hitting the tab key will reindent the current line how the 
indenter
thinks it should be.

Of course, if a project uses only spaces or only tabs, the conversion isn't a
problem. I believe Mono's policy in general is pure tabs, though my mixed style 
fits
in fine. Note that "8-space tabs" means actual \t tabs, but displayed with a 
width of
8 spaces (this is the default tab width in pretty much every terminal, so I 
don't
know why code editors tend to default to 4).

I attach the diff, however still I couldn't manage to patch a fresh local copy 
with
it (I tried `patch -p0 < path/to/my/diff`).

P.S. Just build the Gendarme.Rules.Naming and its test project, as other ones 
don't
want to compile against fresher Mono.Cecil.

I won't have time to test it until this weekend but the diff itself looks great 
:-)

One thing you may want to do is update your Mono.Cecil (the one used by the 
current
source code is newer, not older, than your own). Also, at this moment, the main 
build
mechanism are the Makefile (if you want to try your hands at them ;-). This may
explains why you had some problems compiling inside MonoDevelop (which, last 
time I
used it, was still using an old Mono.Cecil.dll).

JB or Sebastien, could you review and close this? Students can only do one task 
at a
time, so we're preventing Dan from claiming something else.

Dan, you can claim another issue if you want while we review this one.

Dan,

UseCorrectPrefixRule and DoNotUseIncorrectPrefixRule should be merged into a 
single
rule (with a specific message in each case). Can you attach the merged files 
(rule
and test) in an archive (not a patch, since I'm having bug trouble with the 
existing
one and those will be new file).

Except for this merging in Gendarme is mostly done (the string changes are 
already in
SVN) and all I'll start to test (outside the unit tests) the rules.

Thanks (and sorry for the delay).

Ok, self-test is turning a lot of false-positive.

The naming rules needs to check for compiler generated code and other stuff out 
of
"user" control.

e.g. the check for pascal-cased [type|method] names must ignore things like
<Module> type
.ctor and .cctor methods (constructor and static constructors)
get_* and set_* methods (properties getters and setters)

[deleted comment]

Will be fixed soon.
Here's merged UseCorrectPrefix stuff and updated .xml.in file.

Great :) this is now merged (in my local tree)

self-check (gendarme running on gendarme) reports

222 defects for UseCorrectCasingRule: 175 defects, mostly (or all) false 
positives;
3 defects for UseCorrectPrefixRule: all valid
1 defects for UseCorrectSuffixRule: valid
0 defects for UsePreferredTermsRule

Other existing rules executed on yours results in [1]
UseCorrectPrefixRule [2]
    1x MethodCanBeMadeStaticRule [3] (for HasPrefix)
UseCorrectCasingRule [4]
    4x MethodCanBeMadeStaticRule [3] (for IsPascalCase, PascalCase, IsCamelCase,
CamelCase)

So good job!

[1] excluding UseCorrectPrefix results
[2] I'll fix this one before committing (since it's ready for commit in my tree)
[3] it's a new rule and we have a lot of defect wrt it.
[4] since you have to pending fix in this one, you can "static-ify" the methods

p.s. in case you're interested a new Gendarme task has been added to the 
current list
(see #59).

Well, I've done some work so

[*] Now it all compiles against Mono.Cecil 0.6
[*] I hope now no false positives

However, it seems to me that there are some 'false negatives'. I had been 
running
gendarme over the same assembly and 've been having lots of _true_ violations - 
and
then something happened and now I have _lot_ less of ones. However I could not 
find a
source of this bug (if it is a bug).
Hope you'll help me find out what's gone wrong (test are running okay, though).

P.S. I attach the whole folder so you could just copy everything to your folder 
and 
merge it. However I'd advice you to back your copy up because my code must have 
a
problem described above.

P.P.S. Sorry for my poor English and delays :-)

Dan, unless you tell me you fixed something specific in some rule, I'm not gonna
update everything (since it's all merged and tested in my tree). So I'll only 
update
(and test) your new UseCorrectCasingRule.

From a previous *quick* view of the 200-ish false positives in self-test I 
haven't
seen one valid instance. However I won't take chance and will review all of them
before updating UseCorrectCasingRule. Thanks for the heads up!

I check the previous results and didn't found any valid case (in self-test). 
The only
case I've seen are:
   <Module>
   <PrivateImplementationDetails>
   .ctor
   .cctor
   get_*
   set_*
   anonymous methods
All valid cases since they are out of control of the developer.

Now your archive seems to be missing the UseCorrectCasingRule.cs file, the unit 
tests
are still there and so is a new NamingUtils.cs file. Could you attach only the 
needed
file(s) ?

[deleted comment]

Hmm..
Do you have your working copy commited in any branch or smth? I need it as a 
base to
start applying new changes.

If not, please, create a Google Code project and commit there your local copy 
(I mean
one you mentioned in #24). I need it ready to compile so if your Mono.Cecil is
older/newer than 0.6, post it there too please.

Thanks in advance.

Oh - and here UseCorrectCasingRule.cs is:

Thanks for the missing file. 

It seems that you took the hard road (maybe I should have hinted you) since the 
same
problem was already fixed in existing rules.

In details
* NamingUtils methods shouldn't be checking for $ or < in strings. There are 
better,
and safer, ways to do this (look at AvoidUninstantiedInternalClassesRule);
* NamingUtils has a bit of duplication (e.g. checking for 
CompilerGeneratedAttribute)
and Gendarme will report this (the smells rules);
* UseCorrectCasing now totally ignores get_ and set_ while it should not. The 
rest of
the name still needs casing validation;
* The same applies for add_ remove_ and other special names (look at
Mono.Cecil.MethodSemanticsAttributes). The prefix should be skipped, but the 
rest of
the name should be validated;
* New unit tests should be added to catch/test those cases.

Thanks

[note: I'm only the reviewer here. I will provide a patch, in another comment, 
but I
won't create a new repository for your task]

Here the diff of my gendarme/rules/Gendarme.Rules.Naming directory

What I get from gendarme SVN does _not_ compile against the freshest Cecil 
(0.6, as
mentioned in mono docs), and before applying the diff I'd like it _to_ compile 
:-) .
I have maid some changes to code (e.g. replace deprecated 
TypeDefinition.IsPublic
calls etc) and posted them in #25 but, as everything seems to compile on your
machine, I ask you again: what version of Mono.Cecil do you use? Can you just 
post it
here or send me the file itself by e-mail?
Of course I cannot make changed to code unless it compiles correctly so hope 
you'll
help me to handle this.

Thanks for patience,
Dan.

Gendarme from SVN always compile against Cecil from SVN.

Depending on how you checked out Gendarme you probably already have the latest 
Cecil
sources around (or else it's easy to get it). Inside /cecil/lib you'll find a
standalone makefile (standalone.make iirc). You can build and install the most 
recent
Cecil using this makefile.

make -f standalone.make
make -f standalone.make install

Then you should be able compile the latest Gendarme without problem.

Another option, which I have not tried, is to use MonoDevelop to open the VS.NET
solution file that Cecil provides (I believe it's current).

As a last resort continue with your current Cecil(*) and concentrate on fixing 
the
UseCorrectCase rule (with tests). This is the single missing thing to close 
this task
(but obviously I'm willing to help you if you want to fix other issues related 
to
Gendarme/Cecil :-)

(*) I'll make the required (and small) changes before the commit to SVN

[deleted comment]

I've compiled everything as you said (at last).
Some rules in Performance assembly fail, by the way, as seen in the attached
screenshot, However that should not worry me, should it? :-)

Now I'm working on naming rules, hope I'll be able to finish it all by the 
evening
(about 19.00 UTC).

No it shouldn't worry you ;-)

MonoDevelop use an older version of Mono.Cecil, so some symbols are missing. 
Right
now the "supported" way to build/test Gendarme are the Makefiles (i.e. 
MonoDevelop
and VS solution/projects files are supplied but user-supported).

Note that this will probably change in the future, since we will
(a) want the MD addin to work properly;
(b) probably (pending tests) use MD to export the Makefile and VS.NET solutions;

Hola!

At last I managed to make it all work as intentioned!

:P

Note that I still check if type name contains some characters ($, < and >) 
because
unfortunately it doesn't work the other way (see lines commented out in 
NamingUtils.cs).

Kinda changelog:
[*] UseCorrectPrefixRule.cs
    Test/UseCorrectPrefixTest.cs
         -- now doesn't take 'CLS' or other abbreviations as rule violations (e.g.
'CFG' is not like 'CMyCLass' + corresponding test cases
[*] UseCorrectCasing.cs
    NamingUtils.cs
    Test/UseCorrectCasing.cs
        -- now handles props and special names right (at lease I hope so), though
with some workarounds (need help here) + new test cases
[*] UseCorrectSuffixRule.cs
        -- now it doesn't show error messages like 'type name should end with one of
following suffixes: Collection, Collection, Collection' - not being really an 
error,
that looked bad.

Ok, I cannot merge this again to test (it's too time consuming without patches) 
so
I'll only comment on (parts of) this version.

First I think you forgot to look at Mono.Cecil.MethodSemanticsAttributes ;-)
This code snippet...

string name = methodDefinition.Name;    
if (name.StartsWith ("get_") || name.StartsWith ("set_")) {
    name = name.Substring (4);          
    // we NEED to check properties though they're generated by compiler
} else if (NamingUtils.IsNameGeneratedByCompiler (methodDefinition)) {
    return runner.RuleSuccess;          
}

will fail if I
* define methods that are named get_ or set_ (with or without a suffix);
* the getter/setter was generated by the compiler (I don't know any C# case but 
it's
a possibility, e.g. another compiler), i.e. the check should be before

it also doesn't handle the case for add_ and remove_ (MethodSemanticsAttributes
again) which aren't related to operator overloading (good test case btw).

[As a "rule" always check for rule applicability before starting the rule 
logic. This
helps to keep the rules readable (and this will become mandatory in the next 
version
of Gendarme)]

Also

* can you tell me why you added RemoveLeadingUnderlines ? the current code, if
needed, will potentially (and needlessly) allocate a lot of strings.

* what are the false positives for which you commented code inside NamingUtils ?

* there's still code duplication inside NamingUtils to check for attributes.

> First I think you forgot to look at Mono.Cecil.MethodSemanticsAttributes ;-)
OK.

> can you tell me why you added RemoveLeadingUnderlines ? the current code, if
needed, will potentially (and needlessly) allocate a lot of strings.
Because otherwise I get error messages like "replace '_blabla' identifier with
'_blabla'" but not the intended "replace '_blabla' identifier with 'Blabla'".

> what are the false positives for which you commented code inside NamingUtils ?
If you build with that code, unit tests will fail for CorrectCasing (good
pascal-cased class) and something else (don't remember what exactly).

> there's still code duplication inside NamingUtils to check for attributes.
OK.

Perhaps, I should better use StringBuilder instead (speaking of underlines in
NamingUtils).

If the check is really needed(*) then it would be better to loop to find the 
first
non _ char then do a single String.Substring call

(*) there's already a rule to report _ in names

p.s. I'll try your unit tests with your newer code too see the CorrectCasing 
failure

I get 66 (all) success using

            // triggers false positives -- why?
            if (type.IsNested)
                return IsGeneratedByCompiler (type) || IsGeneratedByCompiler (type.DeclaringType);
            // so as of now we use this ugly replacement
//          if (type.Name.Contains ("$") || type.Name.Contains ("<") || 
type.Name.Contains
(">"))
//              return true;            

This site is an horrible chat room. Is it possible for you to get into IRC ? If 
so
can you join #gendarme on GIMPnet, my nick is spouliot. Thanks

Hope it's last version :-)

It's now like SVN + patch in #16 + copy this, if I am right.

Rules and test have been committed to SVN (r91901/2).
You may want to take a look to be sure I didn't merge incorrectly.

Great job! Thanks and enjoy your vacations ;-)

note: switching owner and closing task.