Skip to content
New issue

Have a question about this project? Sign up for a free GitHub account to open an issue and contact its maintainers and the community.

By clicking “Sign up for GitHub”, you agree to our terms of service and privacy statement. We’ll occasionally send you account related emails.

Already on GitHub? Sign in to your account

Jump to bottom

Three Gendarme rules about fields #73

Closed
GoogleCodeExporter opened this issue Jun 3, 2015 · 16 comments
Closed

Three Gendarme rules about fields #73

GoogleCodeExporter opened this issue Jun 3, 2015 · 16 comments
Labels
auto-migrated ClaimedBy-andreas.noever Code DueDate-2008-01-27 QualityAssurance

Comments

@GoogleCodeExporter
Copy link 

This task is to create three (3) rules for Gendarme [1] to check field for
some conditions that be confusing (or easily forgotten) when using fields.

1. Gendarme.Rules.Security.ArrayFieldsShouldNotBeReadOnlyRule

This rule should warn about read-only arrays because elements inside the 
array are NOT read-only.

2. Gendarme.Rules.Security.NativeFieldsShouldNotBeVisibleRule

This rule is very similar to AvoidPublicInstanceFieldsRule, but someone
turning off the *design* rule should still be warned about the security
aspects of exposing native fields (for reasons similar to
TypeExposeFieldsRule).

3. Gendarme.Rules.Concurrency.NonConstantStaticFieldsShouldNotBeVisible 

This rule should warn for all static fields that are not constant because
their access should be synchronized in multi-threaded applications.


So three easy rules for Gendarme. Ideal if you want to learn Gendarme or Cecil.

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 13 Jan 2008 at 3:00
@GoogleCodeExporter
Copy link
Author 

I want to claim this task when #79 is closed (it's all posted and tested so hope
there'll be no problems).

Original comment by dan.abra...@gmail.com on 20 Jan 2008 at 11:47

@GoogleCodeExporter
Copy link
Author 

Hi,

I'm very sorry but I already implemented the rules while waiting for #81 to be
closed. Probably should have put up a notice here... I hope you didn't already 
start
working on this one?

Andreas Noever

Original comment by andreas....@gmail.com on 21 Jan 2008 at 6:04

Attachments:

@GoogleCodeExporter
Copy link
Author 

I've implemented them too but didn't post because of #79..
Right, you were first then.

Original comment by dan.abra...@gmail.com on 21 Jan 2008 at 7:55

@GoogleCodeExporter
Copy link
Author 

Here're mines, just in case you're wondering :-p

Alas..

Original comment by dan.abra...@gmail.com on 21 Jan 2008 at 8:00

Attachments:

@GoogleCodeExporter
Copy link
Author 

My docs =)

Original comment by dan.abra...@gmail.com on 21 Jan 2008 at 8:01

Attachments:

@GoogleCodeExporter
Copy link
Author 

Hey guys, the rules are to *claim* a task when a previous one is complete, then 
start
your work. Here we have claims, without a closed tasks, then source(s) without
claims... :(

I'll be back in a bit (in about one hour on IRC) - I need another network cable 
for
my temporary setup :|

Original comment by sebastie...@gmail.com on 21 Jan 2008 at 12:41

@GoogleCodeExporter
Copy link
Author 

task to Andreas, because he has no other claimed task at the moment

note: but please, don't start to work on a task before status is changed to 
'''Claimed'''

Original comment by sebastie...@gmail.com on 21 Jan 2008 at 4:08

  • Changed state: Claimed
  • Added labels: ClaimedBy-andreas.noever, DueDate-2008-01-27

@GoogleCodeExporter
Copy link
Author 

ArrayFieldsShouldNotBeReadOnlyRule
* nice trick to skip interfaces, enums should be skipped too (delegates too but 
it's
not worth the extra time checking time)
* rule should also apply to protected arrays (with unit tests)

Original comment by sebastie...@gmail.com on 21 Jan 2008 at 4:37

@GoogleCodeExporter
Copy link
Author 

NativeFieldsShouldNotBeVisibleRule
* same for enums
* same for protected
* read-only IntPtr and UIntPtr are ok (since they can't be modified)
* HandleRef are fine since the pointer is read-only, so I guess this breaks the
IsNative usage (IsPointer ? with IsNative being IsPointer + HandleRef ?)

Original comment by sebastie...@gmail.com on 21 Jan 2008 at 4:45

@GoogleCodeExporter
Copy link
Author 

NonConstantStaticFieldsShouldNotBeVisibleRule
* same enum
* same protected

Make sure to update the unit tests too!
Thanks
Sebastien

Original comment by sebastie...@gmail.com on 21 Jan 2008 at 4:58

@GoogleCodeExporter
Copy link
Author 

[deleted comment]

@GoogleCodeExporter
Copy link
Author 

* nice trick to skip interfaces, enums should be skipped too (delegates too but 
it's
not worth the extra time checking time)
* rule should also apply to protected arrays (with unit tests)
changed in all rules.

* read-only IntPtr and UIntPtr are ok (since they can't be modified)
* HandleRef are fine since the pointer is read-only, so I guess this breaks the
IsNative usage (IsPointer ? with IsNative being IsPointer + HandleRef ?)
IsNative() can still be used, but IsInitOnly needs to be checked too.

Documentation updated to reflect the changes. Added "what could 
happen"-Examples for
NonConstantStaticFieldsShouldNotBeVisibleRule.

Original comment by andreas....@gmail.com on 21 Jan 2008 at 6:29

Attachments:

@GoogleCodeExporter
Copy link
Author 

ArrayFieldsShouldNotBeReadOnlyRule
* the check isn't correct, and this probably applies to all other rules too, 
since
internal gets reported (please add test cases for that too)
* also, sorry I missed it in the tests earlier, a field can't be visible if 
it's type
isn't visible (i.e. all test have non-public classes, please keep a private 
test case
too ;-).

Original comment by sebastie...@gmail.com on 21 Jan 2008 at 6:43

@GoogleCodeExporter
Copy link
Author 

The two other rules are similar (type visibility and handling internal too).
Once fixed, with tests, this task can be closed.
Thanks!
Sebastien

Original comment by sebastie...@gmail.com on 21 Jan 2008 at 6:56

@GoogleCodeExporter
Copy link
Author 

Fixed the internal issue.

A IsVisible() rock is provided for MethodDefinition, FieldDefinition and
TypeDefinition + UnitTests.

Andreas Noever

Original comment by andreas....@gmail.com on 21 Jan 2008 at 8:29

Attachments:

@GoogleCodeExporter
Copy link
Author 

Everything looks fine! Will commit all this tonight.
Thanks again :)
Sebastien

Original comment by sebastie...@gmail.com on 21 Jan 2008 at 8:49

  • Changed state: Closed

Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Assignees
No one assigned
Labels
auto-migrated ClaimedBy-andreas.noever Code DueDate-2008-01-27 QualityAssurance
Projects
None yet
Milestone
No milestone
Development

No branches or pull requests
1 participant
@GoogleCodeExporter