shana / google-highly-open-participation-mono

Automatically exported from code.google.com/p/google-highly-open-participation-mono
0 stars 0 forks source link

Three Gendarme rules about naming #68

Closed GoogleCodeExporter closed 9 years ago

GoogleCodeExporter commented 9 years ago
This task is to create three (3) rule for Gendarme [1] to check the correct
naming of elements.

1. Gendarme.Rules.Naming.DoNotUseReservedInEnumValueNamesRule

This rule should warn of any enum's value name that contains the word
reserved. This is bad practice and not needed in .net

2. Gendarme.Rules.Naming.DoNotPrefixValuesWithEnumNameRule

This rule should warn of any enums that contains value's name that are
prefixed with the enum's name. A bad example would be

public enum Test {
   TestOk,
   TestBad
}

3. Gendarme.Rules.Naming.ParameterNamesShouldMatchOverridenMethodRule

This rule should warn of any overriden method (or interface implementation)
where the name of one, or many, parameter(s) don't match the base class (or
interface).

So 2 easy rules for Gendarme, which is a nice introduction if you don't yet
know about Cecil (or Gendarme), and one a (little bit, but not much) harder.

To complete the task it must include:
- the rules source code;
- unit tests to ensure the rules works correctly;
- documentation for the web site (see [2] for an example)

This tasks should take between 2-5 days (depending if you already know
Gendarme/Cecil or not).

[1] http://www.mono-project.com/Gendarme
[2] http://groups.google.com/group/gendarme

Original issue reported on code.google.com by sebastie...@gmail.com on 4 Jan 2008 at 1:39

GoogleCodeExporter commented 9 years ago
I claim this task.

Original comment by andreas....@gmail.com on 13 Jan 2008 at 3:08

GoogleCodeExporter commented 9 years ago
it's yours!

Original comment by sebastie...@gmail.com on 13 Jan 2008 at 3:13

GoogleCodeExporter commented 9 years ago
DoNotUseReservedInEnumValueNamesRule

It is considered bad practice to define reserved values in enums. This rule 
warns if
any value contains the word reserved.

Bad example:

enum Answer {
    Yes,
    No,
    Maybe,
    Reserved
}

Good example:

enum Answer {
    Yes,
    No,
    Maybe
}

DoNotPrefixValuesWithEnumNameRule

This rule checks for enum values that are prefixed with the enum's name.

Bad example:

enum Answer {
    AnswerYes,
    AnswerNo,
    AnswerMaybe,
}

Good example:

enum Answer {
    Yes,
    No,
    Maybe
}

ParameterNamesShouldMatchOverridenMethodRule

This rule warns if an overriden method's parameter names do not match those of 
the
base class (or those of the implemented interface).

Bad example:

class Base {
    public abstract void Write (string text);
}

class SubType : Base {
    public overwrite void Write (string output)
    {
        //...
    }
}
Good example:

class SubType : Base {
    public overwrite void Write (string text)
    {
        //...
    }
}

Original comment by andreas....@gmail.com on 13 Jan 2008 at 8:17

Attachments:

GoogleCodeExporter commented 9 years ago
The new zip format, compressing 9kb down to 0kb. Sadly it's not working...

Original comment by andreas....@gmail.com on 13 Jan 2008 at 8:19

Attachments:

GoogleCodeExporter commented 9 years ago
0kb seems like the holy grail - too bad it didn't work out ;-)

DoNotPrefixValuesWithEnumNameRule
* Nice trick on field.IsStatic to check for value__ (and like all nice hacks 
it's
worth a comment ;-)
* RuleSuccess == null, so you can return a null MessageCollection (the rules 
with
null checks are because we cannot return an empty MessageCollection, but it 
doesn't
apply here since you don't create one until you need it :-)
* I'm not sure about the FalsePositive case, where the enum value == enum 
name... it
seems bad naming to me. Could you add another test case while I think about it 
(and
ask around) ?
* Just to get 100% coverage (always nice even if it doesn't mean perfection) 
add a
test case for a non-enum. E.g. using the test fixture class itself is ok.

DoNotUseReservedInEnumValueNamesRule
* same comments (except for false positive ;-)

got to go now... will review ParameterNamesShouldMatchOverridenMethodRule later 
(or
tomorrow). Thanks!

Original comment by sebastie...@gmail.com on 13 Jan 2008 at 10:05

GoogleCodeExporter commented 9 years ago
> public overwrite void Write (string output)

Didn't know there are _overwritten_ methods in C# =P

Original comment by dan.abra...@gmail.com on 14 Jan 2008 at 9:32

GoogleCodeExporter commented 9 years ago
ParameterNamesShouldMatchOverridenMethodRule looks ok, expect for the extra line
TypeDefinition type = (TypeDefinition) method.DeclaringType;
which is unneeded (at least in the attached version).

I won't have time to test it until tonight but, as I don't expect any problem, 
so you
can go ahead and claim another task.

Original comment by sebastie...@gmail.com on 14 Jan 2008 at 1:54

GoogleCodeExporter commented 9 years ago
* Nice trick on field.IsStatic to check for value__ (and like all nice hacks 
it's
worth a comment ;-)
Thanks :)
* RuleSuccess == null, so you can return a null MessageCollection (the rules 
with
null checks are because we cannot return an empty MessageCollection, but it 
doesn't
apply here since you don't create one until you need it :-)
Removed it.
* I'm not sure about the FalsePositive case, where the enum value == enum 
name... it
seems bad naming to me. Could you add another test case while I think about it 
(and
ask around) ?
I have added a test case + code that also warns if enum value == enum name (old 
code
is commented out)
* Just to get 100% coverage (always nice even if it doesn't mean perfection) 
add a
test case for a non-enum. E.g. using the test fixture class itself is ok.
Done

Changed overwrite to override in ParameterNamesShouldMatchOverridenMethodRule's
warning message.

Original comment by andreas....@gmail.com on 14 Jan 2008 at 2:36

Attachments:

GoogleCodeExporter commented 9 years ago
It's all in SVN. Thanks!

p.s. "and like all nice hacks it's worth a comment ;-)" I meant a source code 
comment ;-)

Original comment by sebastie...@gmail.com on 15 Jan 2008 at 1:51