Warn about shadowing state variables

[Denton-L]

Unsolved questions:

• in which situations should warnings be emitted?

We have to keep in mind that we want to have identifiers at global level at some point, but I think it is still fine because we have qualified import statements.

---

The following is valid Solidity code:

contract Example {
    uint n = 2;

    function test() constant returns (uint) {
        uint n = 1;
        return n;// Will return 1
    }
}

I propose that the compiler should not allow function-level variables to be named identically to contract-level variables.

In other languages, such as Java, this would be valid code because you can differentiate between fields and local variables using the this keyword. In Solidity, however, this is an issue because this has a different purpose and accessing this.* will spawn an external call, which would probably not be expected behaviour.

In addition to this, this behaviour is confusing for people who are starting out with the language.

[redsquirrel]

👍🏼

[ethers]

this behaviour is confusing for people who are starting out with the language.

+100

Another example using a "named return" related to #718

contract Example {
    uint n = 2;

    function test() constant returns (uint n) {
        return n; // Will return 0
    }
}

And another:

contract Example {
    uint n = 2;

    function test() constant returns (uint n) {
        n = 1;
        return n; // Will return 1
    }
}

One more:

contract Example {
    uint n = 2;
    uint x = 3;

    function test() constant returns (uint x) {
        uint n = 4;
        return n+x; // Will return 4
    }
}

Fortunately this doesn't compile:

contract Example {
    uint n = 2;

    function test() constant returns (uint n) {
        uint n = 3;
        return n; // does not compile
    }
}

[ghost]

Got burned by this. Lost 20 minutes or so. Added a global variable to a contract I'd not been working on for a few days and forgot that that variable name was already used inside a function.

[axic]

I think we should add a warning for shadowing "globals".

[chriseth]

This is implemented now.