Fix TempleDao repo issues

[alexroan]

Fixes #574

• Unreachable code in variables.rs defaults to Mutability:immutable
• get_node_sort_key uses name_location only if it doesn't contain -1. Otherwise it uses src
• Added Templegold to CI

[alexroan]

After fixing the unreachable code, I now get the same error as described in #574 .

The error occurs in the inconsistent-type-names detector. It's capturing something that has a source location of "-1:-1:-1".

[alexroan]

The troublesome node:

VariableDeclaration(
    VariableDeclaration {
        base_functions: None,
        constant: false,
        documentation: None,
        function_selector: None,
        indexed: None,
        mutability: Some(
            Mutable,
        ),
        name: "",
        name_location: Some(
            "-1:-1:-1",
        ),
        overrides: None,
        scope: 1611,
        state_variable: false,
        storage_location: Default,
        type_descriptions: TypeDescriptions {
            type_identifier: Some(
                "t_uint256",
            ),
            type_string: Some(
                "uint256",
            ),
        },
        type_name: Some(
            ElementaryTypeName(
                ElementaryTypeName {
                    state_mutability: None,
                    name: "uint",
                    type_descriptions: TypeDescriptions {
                        type_identifier: Some(
                            "t_uint256",
                        ),
                        type_string: Some(
                            "uint256",
                        ),
                    },
                },
            ),
        ),
        value: None,
        visibility: Internal,
        src: "5399:4:3",
        id: 1522,
    },
)

I believe it's the return variable of the function.

It looks like aderyn pulled in the name_location and not the src?

[TilakMaddy]

Alex, this is not a correct fix for state mutability. Please dont push it. Here's why -

For older versions of solidity, state mutability can be none while constant and mutable properties carry the information of mutability. Therefore the helper function is required to abstract that away.

One potential fix is to return None instead of unreachable.

[TilakMaddy]

State mutability can be none when it doesn't apply .for example in interface function definitions

[alexroan]

Aligned. I'll revert these changes and come up with another solution. We should account for immutable

[alexroan]

@TilakMaddy can you please review this? :)

[TilakMaddy]

Question -

1. How do you know that it's immutable and not just None ? Solidity immutable keyword was added in 0.6 . Therefore this cannot be immutable imo . It has to be none

(We must change the return type of the helper function to be option)

2. what about name location then .. Looks like it's out of the picture now. (If thats true it will be a problem, why don't we keep the transformations, except that when "-1" is found in name location, you return source location. Otherwise, just return name location)... It's important for vscode extension to not highlight the whole contract for example, instead just highlight the name

[alexroan]

How do you know that it's immutable and not just None ? Solidity immutable keyword was added in 0.6 . Therefore this cannot be immutable imo . It has to be none

Good point. Changed to return Optional, and return None in the else case.

what about name location then .. Looks like it's out of the picture now. (If thats true it will be a problem, why don't we keep the transformations, except that when "-1" is found in name location, you return source location. Otherwise, just return name location)... It's important for vscode extension to not highlight the whole contract for example, instead just highlight the name

Let's think about this as a follow-up. I'd be interested to see how it affects report output.