proc-macros: rename parameter names #1365

deflator-hacker · 2024-05-06T14:26:43Z

Decoupling rust parameter names from (de)serialized json is essential for me and useful in general.

Some usecases include:

supporting parameter names that are reserved keywords in rust, such as type (currently impossible)
talking to case sensitive RPC servers only accepting non-snake-case parameters (currently impossible without clippy complaining)

Those are both usecases I personally have.

The PR implements this by introducing a new attribute for the rpc macro called argument.

documentation has been updated
tests have been written

I previously responded to an outdated similar PR here.

Minimal working example:

#[rpc(client, server)]
pub trait MyRpc {
	#[method(name = "foo")]
	async fn foo(
		&self,
		param_a: u8,
		#[argument(rename = "type")]
		r#type: String
	) -> RpcResult<u16>;
}

I divided the changes into multiple commits because they are a bit lengthy.
This is my first open source contribution, I am eager to learn, let me know what can be improved.

…nt attribute

niklasad1 · 2024-05-06T15:01:05Z

Looks overall good, nice job

I'm not following why you need to rename parameters when those passed as Array (these are omitted by the client)? In such scenarios then variable doesn't correspond to the actual param name, so why is it needed?

The contrary applies to JSON Objects where the key is the same name passed in the call with some glue to convert snake_case to camelCase. Then I follow why these need to be renamed to support reserved rust keywords etc....

I'll take a closer look tomorrow.

deflator-hacker · 2024-05-06T15:08:35Z

I'm not following why you need to rename parameters when those passed as Array (these are omitted by the client)? In such scenarios then variable doesn't correspond to the actual param name, so why is it needed?

You are absolutely right, there is no point (actually no way) to rename parameters in arrays.
I included a check for array params in proc_macros/tests/ui/correct/rename_fn_params.rs just because I wanted to assert that I did not break anything deserialization-related by accident.

proc-macros/src/render_server.rs

proc-macros/src/rpc_macro.rs

proc-macros/tests/ui/correct/basic.rs

proc-macros/src/rpc_macro.rs

proc-macros/src/render_server.rs

Co-authored-by: Niklas Adolfsson <niklasadolfsson1@gmail.com>

… namespace

- expose potentially renamed name as String insead of Option in RpcFnArg::name() - make RpcFnArg::rename_to private to migrate to new RpcFnArg::name() - adapt render_client.rs and render_server.rs to changes

niklasad1 · 2024-05-07T17:29:44Z

Looks great, I'll can push a fix to unblock the UI tests tomorrow it's probably the latest rustc release that broke them.

proc-macros/tests/ui/correct/basic.rs

- remove unnecessary param_kind: map - add array params test to prevent accidental breakage through param renaming

niklasad1 · 2024-05-08T08:37:43Z

proc-macros/src/render_server.rs

+				alias_vals
+					.push_str(&format!(r#"alias = "{}""#, heck::ToLowerCamelCase::to_lower_camel_case(name.as_str())));
+
+				let serde_rename = quote!(#[serde(rename = #name)]);


This is needed because arg_pat below can't be reserved rust names such as type

proc-macros/src/rpc_macro.rs

niklasad1

Nice work looks good

proc-macros/src/lib.rs

proc-macros/tests/ui/correct/basic.rs

jsdw

LGTM, just a couple of nits!

Co-authored-by: James Wilson <james@jsdw.me>

deflator-hacker added 5 commits May 6, 2024 03:29

proc-macros: add RpcFnArg to parse function argument attribute

d7f748f

proc-macros: integate RpcFnArg into rpc macro

47adf11

proc-macros: update rpc macro documentation example to display argume…

e891b0b

…nt attribute

proc-macros: add tests to verify correct argument rename logic

635d297

proc-macros: add more documentation on rpc macro for argument attribute

d1fa9ab

deflator-hacker requested a review from a team as a code owner May 6, 2024 14:26

deflator-hacker added 2 commits May 6, 2024 16:52

proc-macros: fix tabs in doc comments

02dc410

proc-macros: fix lints generated by CI

ff3dab0

niklasad1 mentioned this pull request May 7, 2024

[RFC] proc-macros: Support custom parameter names #1015

Closed

7 tasks

niklasad1 reviewed May 7, 2024

View reviewed changes

proc-macros/src/render_server.rs Show resolved Hide resolved

niklasad1 reviewed May 7, 2024

View reviewed changes

proc-macros/src/rpc_macro.rs Outdated Show resolved Hide resolved

niklasad1 reviewed May 7, 2024

View reviewed changes

proc-macros/src/rpc_macro.rs Outdated Show resolved Hide resolved

niklasad1 reviewed May 7, 2024

View reviewed changes

proc-macros/tests/ui/correct/basic.rs Show resolved Hide resolved

niklasad1 reviewed May 7, 2024

View reviewed changes

proc-macros/src/rpc_macro.rs Outdated Show resolved Hide resolved

niklasad1 reviewed May 7, 2024

View reviewed changes

proc-macros/src/render_server.rs Outdated Show resolved Hide resolved

deflator-hacker and others added 5 commits May 7, 2024 16:27

proc-macros: update internal documentation on RpcFnArg

ec1812f

Co-authored-by: Niklas Adolfsson <niklasadolfsson1@gmail.com>

proc-macros: use ObjectParams to test deserialization

fe2982d

proc-macros: remove rename_fn_params test

e23a00a

proc-macros: fix basic tests by importing ObjectParams and respecting…

0fbc84b

… namespace

proc-macros: refactor RpcFnArg

ce01049

- expose potentially renamed name as String insead of Option in RpcFnArg::name() - make RpcFnArg::rename_to private to migrate to new RpcFnArg::name() - adapt render_client.rs and render_server.rs to changes

niklasad1 reviewed May 7, 2024

View reviewed changes

proc-macros/tests/ui/correct/basic.rs Outdated Show resolved Hide resolved

niklasad1 reviewed May 7, 2024

View reviewed changes

proc-macros/tests/ui/correct/basic.rs Outdated Show resolved Hide resolved

proc-macros: update basic ui test

51ebbd2

- remove unnecessary param_kind: map - add array params test to prevent accidental breakage through param renaming

niklasad1 reviewed May 8, 2024

View reviewed changes

proc-macros/src/rpc_macro.rs Outdated Show resolved Hide resolved

Update proc-macros/src/rpc_macro.rs

f66352e

niklasad1 reviewed May 8, 2024

View reviewed changes

proc-macros/src/rpc_macro.rs Outdated Show resolved Hide resolved

Update proc-macros/src/rpc_macro.rs

f1092bb

niklasad1 approved these changes May 8, 2024

View reviewed changes

niklasad1 requested a review from jsdw May 8, 2024 08:54

Merge branch 'paritytech:master' into master

b54cd34

jsdw reviewed May 14, 2024

View reviewed changes

proc-macros/src/lib.rs Outdated Show resolved Hide resolved

jsdw reviewed May 14, 2024

View reviewed changes

proc-macros/tests/ui/correct/basic.rs Show resolved Hide resolved

jsdw reviewed May 14, 2024

View reviewed changes

proc-macros/tests/ui/correct/basic.rs Outdated Show resolved Hide resolved

jsdw approved these changes May 14, 2024

View reviewed changes

deflator-hacker and others added 2 commits May 14, 2024 19:16

proc-macros: remove unnecessary docs

0927d94

Co-authored-by: James Wilson <james@jsdw.me>

minor clarification in tests

48655c1

niklasad1 changed the title ~~[RFC] proc-macros: Rename parameter names~~ proc-macros: rename parameter names May 15, 2024

niklasad1 merged commit d2c27c5 into paritytech:master May 15, 2024
10 checks passed

niklasad1 mentioned this pull request May 28, 2024

Overriding name of parameter in param_kind=map for proc macro #1007

Closed

Provide feedback

Saved searches

Use saved searches to filter your results more quickly

proc-macros: rename parameter names #1365

proc-macros: rename parameter names #1365

deflator-hacker commented May 6, 2024

niklasad1 commented May 6, 2024 •

edited

deflator-hacker commented May 6, 2024

niklasad1 commented May 7, 2024

niklasad1 May 8, 2024 •

edited

niklasad1 left a comment

jsdw left a comment

proc-macros: rename parameter names #1365

proc-macros: rename parameter names #1365

Conversation

deflator-hacker commented May 6, 2024

niklasad1 commented May 6, 2024 • edited

deflator-hacker commented May 6, 2024

niklasad1 commented May 7, 2024

niklasad1 May 8, 2024 • edited

Choose a reason for hiding this comment

niklasad1 left a comment

Choose a reason for hiding this comment

jsdw left a comment

Choose a reason for hiding this comment

niklasad1 commented May 6, 2024 •

edited

niklasad1 May 8, 2024 •

edited