Skip to content

Remove unused tokens in compilation test#951

Merged
austinabell merged 1 commit intonear:masterfrom
mooori:rm-unused-attrs
Oct 28, 2022
Merged

Remove unused tokens in compilation test#951
austinabell merged 1 commit intonear:masterfrom
mooori:rm-unused-attrs

Conversation

@mooori
Copy link
Contributor

@mooori mooori commented Oct 28, 2022

The #[near_bindgen] macro currently ignores tokens after the attribute name, for example #[near_bindgen(this_is_ignored)]. This is due to _attr (which contains these tokens) being ignored:

#[proc_macro_attribute]
pub fn near_bindgen(_attr: TokenStream, item: TokenStream) -> TokenStream {

There's a contract used in compilation test which has tokens after the attribute name: #[near_bindgen(init => new)]. They are removed in this PR.

Context

For near-plugins we're experimenting with passing extra tokens like #[near_bindgen(some_config(value_a, value_b))]. When adding logic to parse these tokens via darling the compilation of the contract with #[near_bindgen(init => new)] fails.

@mooori mooori marked this pull request as ready for review October 28, 2022 08:45
@austinabell
Copy link
Contributor

Sorry if I'm missing the motivation here, but what is the purpose of changing this within our compilation tests? This shouldn't affect what you're building?

Are you suggesting that something within our macros could be changed to allow it to be extended in external libs?

@mooori
Copy link
Contributor Author

mooori commented Oct 28, 2022

The feature proposed in #952 uses tokens passed after the name of #[near_bindgen]. This comment tries to extend on the motivation and highlights the code that breaks the compilation of cond_compilation.rs.

@mooori
Copy link
Contributor Author

mooori commented Oct 28, 2022

Assuming init => new is ignored, it might make sense to remove it even if #952 will be rejected. To avoid that someone stumbles upon this in the future.

@austinabell
Copy link
Contributor

Assuming init => new is ignored, it might make sense to remove it even if #952 will be rejected. To avoid that someone stumbles upon this in the future.

Yeah, that makes sense, I'm just making it clear we may use the attributes in the future. #934 is one that proposes this kind of change

@austinabell austinabell merged commit 9cc1f57 into near:master Oct 28, 2022
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 this pull request may close these issues.

2 participants

Comments