[Bug] `abort_code = <...>` overwrites the module name with the package name when they're the same.

[kklas]

🐛 Bug

I have a package called amm and in it a module also called amm. When I do #[expected_failure(abort_code = amm::EFooBar)] on a test it will error Unbound module '(amm=0x0)::EFooBar' because (I presume) amm in amm::EFooBar is being replaced with package name instead of module name (which are both amm).

Because of this I have to write either 0x0::amm::EFooBar or amm::amm::EFooBar.

I would have expected for amm::EFooBar to work because there is an import statement in the test module use 0x0::amm.

System information

sui 0.18.0

[tnowacki]

I have a package called amm and in it a module also called amm.

I expected people to do this a lot less often than is probably done in practice :p

The cause of this bug is actually due to an ambiguity in attributes, since with this change (to have constants in attributes), I also added location = <module identifier> but this is now the first place where module names can appear in the same syntactic location as expressions. So when compiling the value for the attribute, we do not know ahead of time whether it is an expression or a module identifier.

In short, I knowingly added this bug because I wanted to get it out the door :P Though I should have filed a follow up issue to fix it! And thanks @kklas for filing one!

[CocDap]

I have the same issue when i following this tutorial https://github.com/move-language/move/blob/main/language/documentation/tutorial/step_5/BasicCoin/sources/BasicCoin.move

Basically, I write unit test

Bug context:

    #[test(account = @0x1)]
    #[expected_failure(abort_code = 2)]
    fun publish_balance_already_exists(account:signer) {
        publish_balance(&account);
        publish_balance(&account);
    }
    // this should be fail
    #[test]
    #[expected_failure]
    fun balance_of_dne() acquires Balance {
        balance_of(@0x1);

    }
   // this should be fail
    #[test]
    #[expected_failure]
    fun withdraw_dne() acquires Balance {
        Coin {value:_} = withdraw(@0x1, 0);

    }

Bug Context Result: Unit test balance_of_dne and withdraw_dne didn't trigger

Normal Context: When I remove abort_code

    #[test(account = @0x1)]
    #[expected_failure]
    fun publish_balance_already_exists(account:signer) {
        publish_balance(&account);
        publish_balance(&account);
    }

    #[test]
    #[expected_failure]
    fun balance_of_dne() acquires Balance {
        balance_of(@0x1);

    }

    #[test]
    #[expected_failure]
    fun withdraw_dne() acquires Balance {
        Coin {value:_} = withdraw(@0x1, 0);

    }

Normal Context Result:

[sblackshear]

I think @tnowacki's addition of named abort codes for expected_failure has now fixed this.

[tnowacki]

I think @kklas's original issue is slightly different than the bug here posted below.

The original report was for addressing the ambiguity in the grammar for location and in annotations as a whole.

Using the original example, #[expected_failure(abort_code = amm::EFooBar)] The compiler does not know whether amm is an address alias (with EFooBar being a module) or whether amm is a module alias (with EFooBar being a member). This did not occur previously anywhere in the syntax as module identifiers could not exist in the same position as expressions. So we need some way of resolving this for annotations