`clarinet check` does not catch using keywords as variable names

[jbencin]

Description

clarinet check does not catch the case when you use Clarity keywords for let or match bindings. For example, neither the VSCode extension or clarinet check will find any issues with this function:

(define-public (allowed-variable-names)
  (let ((ok u1)
        (err u2)
        (uint u4)
        (int u4)
        (bool u5)
        (principal u6)
        (optional u7)
        (result u8)
        (tx-sender u9)
        (if u10)
        (none u11)
        (some u12)
        ;;(block-height u13)
        (tenure-height u14)
        (contract-caller u15)
        (chain-id u16)
        (true u17)
        (false u18))
  (ok true)))

The only keyword I've found that's not allowed is block-height

Most of these variable declarations will cause NameAlreadyUsed errors at runtime:

Error calling contract function: Runtime error while interpreting ST1PQHQKV0RJXZFY1DGX8MNSNYVE3VGZJSRTPGZGM.ntt-manager-v1: Unchecked(NameAlreadyUsed("true"))

But not all of them. For example, you can have a variable named bool or uint

Related Issues

• #418
• stacks-network/clarity-wasm#248

[hugoclrd]

@obycode is this somehing we can still expect to be fixed in stacks-core (cf #418)? I guess we (as in "the clarinet") would rather try and fix it in the vm instead of patching it in Clairnet.

The only keyword I've found that's not allowed is block-height

I guess that's only because block-height has been removed in Clarity 3 (in favor of stacks-block-height and burn-block-height). So in theory, it not a reserved anymore 🫣

[jbencin]

So in theory, it not a reserved anymore

So it should allow me to use it as a variable name then? Oddly it's the only one that's not allowed

I guess we (as in "the clarinet") would rather try and fix it in the vm instead of patching it in Clairnet.

Perhaps this would be a good issue for the linter (#1792) instead of clarinet check, since it's technically deployable, but not a good idea?

[hugoclrd]

So in theory, it not a reserved anymore

So it should allow me to use it as a variable name then? Oddly it's the only one that's not allowed

No what I meant is that it's the only that could be used as a variable name. But I think that, for the sake of simplicity, it might have been fully baned from the language (otherwise the transition between Clarity 2 and 3 might have been chaotic).

this would be a good issue for the linter (https://github.com/hirosystems/clarinet/issues/1792) instead of clarinet check

Well technically, clarinet check does run "check checker" at the moment, so we might want it to run the lints as well? idk. But what i meant is that it could be fix in stacks_core/clarity directly, in the type checker