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

Two Gendarme rules to check for bad practices #74

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

Two Gendarme rules to check for bad practices #74

GoogleCodeExporter opened this issue Jun 3, 2015 · 6 comments

Comments

@GoogleCodeExporter
Copy link

This task is to create two (2) rules for Gendarme [1] to check field for
some common practices that potential hides issues.

1. Gendarme.Rules.BadPractice.GetEntryAssemblyMayReturnNullRule

The static method Assembly.GetEntryAssembly() returns null when it's not
called from the root (main) application domain. This may be a problem if a
library is being used, for example, in ASP.NET. Note: rule does not apply
to .EXE (i.e. only .DLL)

2. Gendarme.Rules.BadPractice.ConstructorShouldNotCallVirtualMethodsRule

The rule should warn if any constructor, from non-sealed classes, calls
virtual methods. This can cause problems because the exact method called
isn't know before runtime. Also the virtual method, from a base class, can
be run before any of it's type constructor. Note that some compilers  use
the callvirt instruction to call non-virtual methods - those shouldn't be
reported.


So two rules for Gendarme that requires you to scan the IL of the generated
methods.

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:26

@GoogleCodeExporter
Copy link
Author

I claim this and show some work already done.

ConstructorShouldNotCallVirtualMethodsRule

This rule warns the developer if any virtual methods are called in the 
constructor of
non-sealed type. The problem is exact method that will be executed is not known
before runtime. Also such virtual calls may be executed before the constructor 
of
deriving type is called, thus making possible mistakes.

Bad example:
class A {
    public A ()
    {
        this.DoSomething ();
    }
    protected virtual void DoSomething () { }
}

class B : A {
    private int x;

    public B ()
    {
        x = 10;
    }

    protected override void DoSomething ()
    {
        Console.WriteLine (x);
    }
}

B b = new B (); // outputs zero because B constructor hasn't been called yet

No good examples here, I think (any example avoiding this is good).

Original comment by dan.abra...@gmail.com on 14 Jan 2008 at 2:16

Attachments:

@GoogleCodeExporter
Copy link
Author

You got it (but I won't have time to review anything before tonight)

Original comment by sebastie...@gmail.com on 14 Jan 2008 at 2:21

  • Changed state: Claimed
  • Added labels: ClaimedBy-dan.abramov, DueDate-2008-01-22

@GoogleCodeExporter
Copy link
Author

GetEntryAssemblyMayReturnNullRule

Calling Assembly.GetEntryAssembly () not from the main application domain will 
always
result in null, and this may become a problem for ASP .NET applications.

Bad example:

// ASP .NET page
Response.WriteLine (Assembly.GetEntryAssembly ().CodeBase); // null reference 
exception

Original comment by dan.abra...@gmail.com on 14 Jan 2008 at 3:20

Attachments:

@GoogleCodeExporter
Copy link
Author

It all looks good. BTW nice tests on GetEntryAssemblyMayReturnNullTest to get 
both
case working :)

I'll only be able to test them tonight but, as I don't anticipate any issue, 
you can
go ahead with another task.

But before that please supply good examples for both, it's not hard and will 
make my
life easier when I update the web site documentation ;-)

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

@GoogleCodeExporter
Copy link
Author

Good example for ConstructorShouldNotCallVirtualMethodsRule:

class A {
    public A ()
    {
    }

    public void Run ()
    {
        // here we can call virtual methods
        this.DoSomething ();
    }   

    protected virtual void DoSomething () { }
}

class B : A {
    private int x;

    public B ()
    {
        x = 10;
    }

    protected override void DoSomething ()
    {
        Console.WriteLine (x);
    }
}

B b = new B ();
b.Run (); // outputs 10 as intended


Good example for GetEntryAssemblyMayReturnNullRule:

// executable
public class MainClass {
    static void Main ()
    {
        Console.WriteLine (Assembly.GetEntryAssembly ().CodeBase);
    }
}

Original comment by dan.abra...@gmail.com on 14 Jan 2008 at 4:07

@GoogleCodeExporter
Copy link
Author

aha! you were too quick and forgot to check for HasBody ;-)
I made the 2 liners fixes and committed to SVN.

Good work :)
Sebastien

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

  • Changed state: Closed

Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Projects
None yet
Development

No branches or pull requests

1 participant