kcl-libs/derive-docs breakage with upcoming Rust 1.77

[nnethercote]

If you compile kcl-libs with the Rust nightly that will be coming out in the next day or two, you will compile errors like this:

error: custom attribute panicked
   --> src/std/sketch.rs:787:1
    |
787 | / #[stdlib {
788 | |     name = "startSketchOn",
789 | | }]
    | |__^
    |
    = help: message: `"Plane >"` is not a valid identifier

The problem is in the stdlib attribute proc macro in derive-docs, which has this code in src/lib.rs:

    let ret_ty = ast.sig.output.clone();
    let ret_ty_string = ret_ty
        .into_token_stream()
        .to_string()
        .replace("-> ", "")
        .replace("Result < ", "")
        .replace(", KclError >", "");
    let return_type = if !ret_ty_string.is_empty() {
        let ret_ty_string = if ret_ty_string.starts_with("Box <") {
            ret_ty_string
                .trim_start_matches("Box <")
                .trim_end_matches('>')
                .trim()
                .to_string()
        } else {
            ret_ty_string.trim().to_string()
        };

The problem is this uses to_string on a token stream and then does substring matching on the result, which assumes that the output of to_string has a particular form. But the only guarantee on to_string is that the result should be parseable Rust. The whitespace within the output may change.

In the upcoming Nightly, the return type changes from this:

-> Result < Box < ExtrudeGroup >, KclError > {}

to this:

-> Result < Box < ExtrudeGroup > , KclError > {}

And the space inserted between the > and , breaks the substring matching, because trim_end_matches('>') fails due to the trailing space.

This should be fixable with minor changes to the substring matching. But substring matching really shouldn't be used in general because it's fragile in the face of these kinds of changes. Looking at individual TokenTrees within the TokenStream is a more reliable way of doing things, either directly (e.g. see this PR) or possibly using syn.

[nnethercote]

Has this been fixed? I don't see any change that would have fixed it, but maybe I'm overlooking something.

[jessfraz]

Oh shit sorry I just assumed it was working because im on latest rust? did they not release?

[jessfraz]

I guess 1.77 is not out yet

[nnethercote]

That's right, 1.76 was just released, 1.77 is about six weeks away.

[jessfraz]

I think this fixed it #1487

[nnethercote]

This should be fixable with minor changes to the substring matching. But substring matching really shouldn't be used in general because it's fragile in the face of these kinds of changes. Looking at individual TokenTrees within the TokenStream is a more reliable way of doing things, either directly (e.g. see this PR) or possibly using syn.

#1487 fixes the current problem. It's still something of a bandaid fix, as the quoted paragraph describes, but it's better than nothing.

[jessfraz]

TIL there’s another way to do this without string matching wow such sadness for everywhere I have use this.. okay I’ll in the future I’ll be better

[jessfraz]

fixed half of the string matching while fixing another bug, I promise we will get there :) https://github.com/KittyCAD/modeling-app/pull/1701/files#diff-58bfb285fa12f8cddd416ae2f26a3a5eed6085073660924c44dd554030e74270