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 optional static typing to functions and structs #904

Merged
merged 10 commits into from
Jan 28, 2025

Conversation

TheNachoBIT
Copy link
Contributor

This tiny PR adds the option for the parser to detect "-> Type" at the end of the function if its detected.

@TheNachoBIT
Copy link
Contributor Author

Update: I've discovered that structs do not have static typing aswell (again, this is in the parser, not in the VM).

So i've made a little update that adds optional static typing to the struct's fields :D

@TheNachoBIT TheNachoBIT changed the title (Parser-only) Add optional static typing to functions. (Parser-only) Add optional static typing to functions and structs. Jan 26, 2025
Copy link
Collaborator

@udoprog udoprog left a comment

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Cheers!

Could you also add the code which rejects the relevant items during indexing in case type information is specified? Otherwise the compiler will silently accept this even though it's subject to change which could accidentally allow breaking changes to sneak through.

That is currently checked in places like these:

pub(crate) fn item_fn(idx: &mut Indexer<'_, '_>, mut ast: ast::ItemFn) -> compile::Result<()> {

crates/rune/src/ast/item_struct.rs Outdated Show resolved Hide resolved
@udoprog udoprog added the enhancement New feature or request label Jan 28, 2025
Copy link
Collaborator

@udoprog udoprog left a comment

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

I think the last required component is indexing rejection.

@TheNachoBIT
Copy link
Contributor Author

There, i fixed it and i think its now ready to run the tests :D

@udoprog
Copy link
Collaborator

udoprog commented Jan 28, 2025

Great, now if the type parameters are set, they still need to be rejected during indexing:
Here:

pub(crate) fn item_fn(idx: &mut Indexer<'_, '_>, mut ast: ast::ItemFn) -> compile::Result<()> {

And here:
fn item_struct(idx: &mut Indexer<'_, '_>, mut ast: ast::ItemStruct) -> compile::Result<()> {

You can see other types of errors raised in there. Please also add the relevant test cases to make sure type annotations are rejected. You can see for example the attribute rejection test here to see how it can be done:

fn deny_enum_attributes() {

@TheNachoBIT
Copy link
Contributor Author

Sure sure! I'll try that :D

@TheNachoBIT
Copy link
Contributor Author

TheNachoBIT commented Jan 28, 2025

There, i think i've added indexing rejection successfully c:

@udoprog udoprog changed the title (Parser-only) Add optional static typing to functions and structs. Add optional static typing to functions and structs Jan 28, 2025
Copy link
Collaborator

@udoprog udoprog left a comment

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

There are a few more places you could add if you wanted to, declarations and function arguments come to mind. But looks good for now!

@TheNachoBIT
Copy link
Contributor Author

TheNachoBIT commented Jan 28, 2025

Yay! Niiice! Thank you so much! :D

There's probably a lot that i'll add more features to the Parser the more features i start adding to Vune. Thank u for approving my PR and i'm glad this is useful for Rune :D

@udoprog udoprog merged commit 92e6230 into rune-rs:main Jan 28, 2025
25 checks passed
@udoprog
Copy link
Collaborator

udoprog commented Jan 28, 2025

Yeah, of course! Thank you!

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

Successfully merging this pull request may close these issues.

2 participants