Generify CST/Cursor/Query

[AntonyBlakey]

No description provided.

[changeset-bot]

⚠️ No Changeset found

Latest commit: 64f8ab0

Merging this PR will not cause a version bump for any packages. If these changes should not result in a new version, you're good to go. If these changes should result in a version bump, you need to add a changeset.

This PR includes no changesets

When changesets are added to this PR, you'll see the packages that this PR includes changesets for and the associated semver types

Click here to learn what changesets are, and how to add one.

Click here if you're a maintainer who wants to add a changeset to this PR

[Xanewok]

Whenever we want to use types as markers, it's slightly better to use an uninhabitable enum pub enum KindTypes {} as this makes it impossible to construct the value (by mistake), but can still be used in the generic argument position.

[AntonyBlakey]

done

[Xanewok]

Almost all of the types can be easily inferred in the return position; specifying turbofish ::<T> here is, I feel, adding unnecessary visual noise that distract slightly from the actual logic.

[AntonyBlakey]

The issue is that 'almost all' comment. I prefer a more regular, obvious form that doesn't require the reader to be aware of just when the type can be inferred. WDYT?

[AntonyBlakey]

but done after f2f

[Xanewok]

ditto here and in all of the similar places

[AntonyBlakey]

as above, so below

[OmarTawfik]

Instead of relying on strum in our public API, and forcing users to use it, I wonder if it is possible to use the standard std::str::FromStr instead, and keep strum as an internal implementation detail.

[AntonyBlakey]

It has to be TryFrom because it is a partial function.

[OmarTawfik]

FromStr also returns a Result<>:
https://doc.rust-lang.org/std/str/trait.FromStr.html

pub trait FromStr: Sized {
    type Err;

    fn from_str(s: &str) -> Result<Self, Self::Err>;
}

[OmarTawfik]

and is also typically used for many other rust types via "value".parse().

For us, we can use #[derive(strum::EnumString)] which auto-derives std::str::FromStr on the enum, but that is an implementation detail.

[AntonyBlakey]

the problem is that the Err associated type in FromStr needs to implement std::fmt::Debug, but it isn't possible to constrain the associated type using stable Rust.

[AntonyBlakey]

Either you or @Xanewok may have a solution - the only one I have propagates the constraint on the associated type all through the codebase.

[OmarTawfik]

I see. What do you think about simply using String as the error type for FromStr? this also has the advantage of providing an actual error message, instead of the (only) Unit variant strum::ParseError::VariantNotFound. The API will then be idiomatic, without exposing/depending on the highly-specific strum.

Not blocking though if it is not feasible. Just a minor suggestion.

[AntonyBlakey]

I tried that, but no joy, because the specification of the Err type still causes difficulties. My preference is to use an associated type constraint, as soon as it is stabilised. It is a significant improvement to the type system.

[Xanewok]

I'll take quick look if we can simplify this.

[OmarTawfik]

Looking great. I will rebase this on the latest main and send you an update!
Thanks.