fixed State variables shadowed by a local variable are incorrectly set to the local variable

[Tiko7454]

if variables was not seen before, search for it in the state variables

[Tiko7454]

this pull request fixes #2058

[0xalpharush]

// SPDX-License-Identifier: UNLICENSED

pragma solidity 0.8.7;

contract B {
    uint public x = 21;
    function a()  public {
        uint x;
        uint u = 2 * x;
    }
}

Won't this now resolve x to the state variable instead of local?

[Tiko7454]

No, it works properly

>>> from slither import Slither
>>> a = Slither('a.sol').contracts[0].functions[0]
>>> a.local_variables
[<slither.core.variables.local_variable.LocalVariable object at 0x7ff15a5ac3d0>, <slither.core.variables.local_variable.LocalVariable object at 0x7ff15af1eb50>]
>>> a.nodes[2].variables_read
[<slither.core.variables.local_variable.LocalVariable object at 0x7ff15af1eb50>]
>>> a.nodes[2].variables_read[0]
<slither.core.variables.local_variable.LocalVariable object at 0x7ff15af1eb50>
>>> print(a.nodes[2].variables_read[0])
x

[0xalpharush]

It works for the compact AST because we have the referenced declaration. I'm not sure how to solve this for legacy AST without reworking name resolution quite a bit

You can try it by removing the pragma:
solc-select use 0.7.6 --always-installl && slither example.sol --print declaration --solc-force-legacy-json
u references the state variable

# Contracts
# B
        - Declaration: example.sol#5 (10 - 12)
        - Implementation: example.sol#5-11 (1 - 2)
        - References: []

        ## Function
                - B.a()
                        - Declaration: example.sol#7 (14 - 16)
                        - Implementation: example.sol#7-10 (5 - 6)
                        - References: []
                - B.slitherConstructorVariables()
                        - Declaration: example.sol#5-6 (1 - 16)
                        - Implementation: example.sol#5-11 (1 - 2)
                        - References: []

        ## State variables
                - x
                        - Declaration: example.sol#6 (17 - 19)
                        - Implementation: example.sol#6 (5 - 23)
                        - References: ['example.sol#9 (22 - 23)']

[Tiko7454]

When is legacy json considered as default option?

[0xalpharush]

It is enabled by default for solc 0.4.0 until 0.4.12. Then it must be enabled manually. It was deprecated in 0.8.0

[montyly]

I am fine if we don't consider the legacy AST for new fixes moving forward.

In the long term we need to rewrite the solc parsing without the legacy AST to reduce complexity, and keep the current parser as "best effort" for the legacy format

[0xalpharush]

#2121 fixes both

[0xalpharush]

@Tiko7454 We merged #2121 as fix. Thanks for reporting and purposing a fix this issue!