move-language / move

Apache License 2.0
2.25k stars 684 forks source link

[Bug] move-analyzer - named addresses - vscode #957

Open EmilRejman opened 1 year ago

EmilRejman commented 1 year ago

🐛 Bug

While using named_addresses:

[addresses]
hello_blockchain = "_"

The move-analyzer VSCode pacage is throwing error:

Unresolved addresses found: [ Named address 'hello_blockchain' in package 'Resources' ] To fix this, add an entry for each unresolved address to the [addresses] section of /workspace/services/move_learning/apps/move-examples/1_resources/Move.toml: e.g., [addresses] Std = "0x1" Alternatively, you can also define [dev-addresses] and call with the -d flag

and not showing any other code errors in VSCode.

There is no other way to make named addresses working in VSCode.

To reproduce

use any named-address with VSCode move-analyzer package.

Stack trace/error message

Unresolved addresses found: [ Named address 'hello_blockchain' in package 'Resources' ] To fix this, add an entry for each unresolved address to the [addresses] section of /workspace/services/move_learning/apps/move-examples/1_resources/Move.toml: e.g., [addresses] Std = "0x1" Alternatively, you can also define [dev-addresses] and call with the -d flag

Expected Behavior

It handles named-address like any other address, so we can use --named-addresses with cli, eg:

aptos move compile --named-addresses hello_blockchain=default 

System information

Docker: aptoslabs/tools:mainnet

Additional context

none

honehone12 commented 1 year ago

How about hello_blockchain = "0x1234" ? Or is this not what you want ?

EmilRejman commented 1 year ago

It doesnt work, at least for aptos: Command:

aptos move compile --named-addresses hello_blockchain=default

Result:

Compiling, may take a little while to download git dependencies...
{
  "Error": "Move compilation failed: Unable to resolve packages for package 'Resources': Unable to resolve named address 'hello_blockchain' in package 'Resources' when resolving dependencies: Attempted to assign a different value '0x1234' to an a already-assigned named address '0xf854(...)'"
}

BTW my move-analyzer module ofter "freezes", eg. I make some changes in the code and there is completly no different output from move-analyzer package/rust-module, it is the same error when for example I deleted whole line. Tehre are no options to configure move-analyzer or aquire logs in easy way to see why is it freezing

honehone12 commented 1 year ago

I see. How about hello_blockchain = "0xf854...YOUR DEFAULT ADDRESS"

EmilRejman commented 1 year ago

What is the point of named-addresses then?

Yeah it works that way, but I just wanted to report the bug, which is names-addresses is not working correctly with move-analyzer.

honehone12 commented 1 year ago

Ah, I think I understand now. While it is still not sure this is the "bug of move-analyzer", maybe you want to say is right. Don't you think that "enhancement for integration with aptos cli" sounds better? Because this is nothing to do with other frameworks, right?

honehone12 commented 1 year ago

I changed move/language/tools/move-package/src/source_package/manifest_parser.rs from line:226 into

if entry_str == EMPTY_ADDR_STR {
    // original codes
    // if addresses.insert(ident, None).is_some() {
    //     bail!("Duplicate address name '{}' found.", ident);
    // }
    // changed codes
    if addresses.insert(
        ident,
        Some(parse_address_literal("0x00").context(
            "Error with parsing stubbed address 0x00"
        )?),
    )
    .is_some() {
        bail!("Duplicate address name '{}' found.", ident);
    }
} else ...

and move-analyzer looks working for now. Please test if you like.

delvindonn commented 1 year ago

is the fix be merged ?

honehone12 commented 1 year ago

no.