Skip to content
New issue

Have a question about this project? Sign up for a free GitHub account to open an issue and contact its maintainers and the community.

By clicking “Sign up for GitHub”, you agree to our terms of service and privacy statement. We’ll occasionally send you account related emails.

Already on GitHub? Sign in to your account

Add support for \ in function names #2715

Open
6 of 7 tasks
bdemann opened this issue Feb 18, 2025 · 4 comments · May be fixed by #2730
Open
6 of 7 tasks

Add support for \ in function names #2715

bdemann opened this issue Feb 18, 2025 · 4 comments · May be fixed by #2730
Assignees

Comments

@bdemann
Copy link
Member

bdemann commented Feb 18, 2025

In the process of resolving #1561 we discovered that function names can have \ in them. So we need to:

  • function names containing \ need to be in quotes
  • figure out how to escape name for initArgs and postUpgradeArgs
    • in the specific edge case of a function being passed in like this "canister-id".crazyFunctionName
  • Figure out a good way to consistently test this
    • Make sure arbs field and function names are completely crazy and wrapped in quotes if needed
    • Make sure property tests have arbitrary names for init, postUpgrade etc
      • This was already in place, for example:
@init([m9JD4O2405, IDL.Int, IDL.Nat64, _eMOH, MgmPl63qU9, ly218GC5yl])
    _w(
@bdemann bdemann added this to the Release Candidate milestone Feb 18, 2025
@bdemann bdemann self-assigned this Feb 18, 2025
@bdemann
Copy link
Member Author

bdemann commented Feb 18, 2025

Note: Since submitting #1561 deployArgs in cansiter_arb.ts has been split and renamed to initArgs and postUpgradeArgs.

@bdemann
Copy link
Member Author

bdemann commented Feb 18, 2025

It turns out that azle already supported method names with \ in them.

    @query([], undefined, { hidden: true })
    'quoted\\Query'(): void {
        console.log('This is a query function');
    }
dfx canister call query quoted\\Query

this works perfectly well. One small fly in the ointment is that didc does not support candid names with \ so during the post build step (that we have no control over), or if you run didc check .azle/query/query.did then you get the following error:

error: parser error
  ┌─ .azle/query/query.did:2:11
  │
2 │     quoted\Query: () -> () query;
  │           ^ Unknown token \

Error: Candid parser error: Unknown token \ at 27..28

Caused by:
    Unknown token \ at 27..28

So from here we need to figure out if the candid parser intends for that to break, or if it's a bug. And then we need to decide if we are going to support it or not.

@bdemann
Copy link
Member Author

bdemann commented Feb 18, 2025

Looking more closely at the candid spec it seems they do have a way to handle this; by putting the identifier in a string.

So this works

service: () -> {
    "quoted\\Query": () -> () query;
    simpleQuery: () -> (text) query;
}

while this is invalid

service: () -> {
    quoted\Query: () -> () query;
    simpleQuery: () -> (text) query;
}

So we have a problem with how we are outputting the candid

@bdemann bdemann linked a pull request Feb 19, 2025 that will close this issue
29 tasks
@bdemann
Copy link
Member Author

bdemann commented Feb 19, 2025

  • Make sure that the did visitor has some way to handle this situation:
const pM7 = IDL.Variant({
    '3U': _Pr67o214Q04
});

should result in

variant {
    '3U':nat32;
};

But instead we get

variant {
    3U:nat32; 
};

Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
None yet
Projects
None yet
Development

Successfully merging a pull request may close this issue.

1 participant