rust-analyzer / rowan

Apache License 2.0
677 stars 57 forks source link

Add try_to_node which does not panic #148

Closed azdavis closed 1 year ago

azdavis commented 1 year ago

https://github.com/rust-analyzer/rowan/issues/144 hasn't seen any movement and I'm not entirely sure how to fix it myself. So in the meantime it'd be nice to not panic and just fail more gracefully. It's also nice in general as a library to expose the possibility of handling errors, without panicking, to callers.

azdavis commented 1 year ago

@Veykril could this have a look?

azdavis commented 1 year ago

More discussion in the linked issue #144 but the original reason why i opened this pr has been fixed. If we don't think this is useful in general we can close, although it might be nice simply because it lets the caller choose whether they want to panic or not.

As I understand, generally libraries should only panic when:

I'm not sure trying and failing to resolve a syntax node pointer fits into either case.

azdavis commented 1 year ago

Could we make a decision either way on this?

azdavis commented 1 year ago

r? @Veykril