Warning about unused function parameters and imports

[pcaversaccio]

Also related to #2431. Can we make sure that the compiler issues a warning for unused imports and function parameters? I think it would be a good idea in order to improve the quality and cleanliness of the code.

Example: Function Parameters

@external
@pure
def test(owner: uint256) -> uint256:
    return 1

For instance here, the compiler should issue a warning that owner is not used.

Example: Imports

# workhorse.vy
struct MyStruct:
    addr: address

interface UseStruct:
    def use_struct(x: MyStruct): nonpayable

@internal
def utility_function1(x: uint256, y: uint256):
    return x + y

@internal
def utility_function2(addr: UseStruct):
    addr.use_struct(MyStruct({x: self}))

import workhorse

@external
@pure
def foo() -> uint256:
    return 1

For instance here, the compiler should issue a warning that workhorse is not used.

[fubuloubu]

Also related to #2431. Can we make sure that the compiler issues a warning for unused imports and function parameters? I think it would be a good idea in order to improve the quality and cleanliness of the code.

Example: Function Parameters

@external
@pure
def test(owner: uint256) -> uint256:
    return 1

For instance here, the compiler should issue a warning that owner is not used.

I don't think I agree with this one as much, there are definitely times where you need to implement a specific interface, but a parameter isn't useful

[pcaversaccio]

I don't think I agree with this one as much, there are definitely times where you need to implement a specific interface, but a parameter isn't useful

right, but this could be solved using the following syntax:

@external
@pure
def test(uint256) -> uint256:
    return 1

No named parameter (in this case owner) for the interface parameter you have to implement. In that case, the compiler does not issue a warning. The above syntax would currently throw a NamespaceCollision error. So the question is whether we should allow for it within function declarations.

[fubuloubu]

That requires modifying a fair bit of the processing path for args, and also really isn't python syntax

A better way would be just prepend with _ to ignore the warning

[pcaversaccio]

Right makes sense to keep Python consistency. We could do both: using _ as the variable name or prepending _ to the variable name. In both cases the warning will be ignored.

[trocher]

Adding this as a note:
Would probably have to think about preventing the use of _ in other cases when this is implemented as currently, this compiles:

_: uint256
@external
def test() -> uint256:
    _:uint256 = 12
    return _
@external
def bar(_:uint256) -> (uint256,uint256):
    return (_,self._)

Might also be good to prevent it anyway now that I think about it.

[pcaversaccio]

Might also be good to prevent it anyway now that I think about it.

Yes good catch - _ itself should be treated similarly as a reserved keyword. In any case, the library modules will be released as 0.4.0 if I understood correctly, so I assume such a slight breaking change should be ok.

[tserg]

I think it is worth discussing whether these Quality-of-Life improvements are better handled by downstream tooling such as https://github.com/AlbertoCentonze/natrix. The rationale is that we do not want to saddle the compiler with maintaining these non-critical features.

[charles-cooper]

#4447 related -- i think this may be the best path forward since we don't need to decide whether making something a warning is worth it or not, we just issue the warning and the user also has the option to quash or promote the warning.