Accept a closure for with in lieu of a path for fields

[TedDriggs]

Fixes #309

This has some issues, so it's not yet ready to merge:

1. It doesn't work for all the places where with is used.
2. I'm not thrilled with the introduction of new locals to support this, and am wondering if there's a way to write it that better encapsulates the desired behavior.

[Veetaha]

If you'd like to avoid using a local variable there are two ways to do that:

The naive way is to just cast the expression with as: #closure as fn(...). Although the as operator is a bit more powerful for this task, because it's meant for potential type coersions, while in this case we just need a type hint.

Another way to do that is with ::core::convert::identity::<fn(...) -> _>(#closure) which provides a hint for the expression via the identity function parameter.

I think there doesn't have to be any special case for closure/non-closure expressions. If you pass a function path to the identity function, it'll compile just fine, so there shoudln't be a special case.

[TedDriggs]

Using identity seems to work well. It also solves one of the main barriers to allowing arbitrary expressions in this position, though I'm still hesitant to allow arbitrary expressions until I can test the isolation and the quality of the error messages when the user does something wrong.

[TedDriggs]

@Veetaha I've updated this using core::convert::identity, added a compile-fail test to show what happens if you try to access locals, and renamed darling_core::codegen::Field::with_path to be the more descriptive with_callable.

[Veetaha]

Thanks! LGTM, except docs should need a small update

[TedDriggs]

@Veetaha ugh, one more question for you: Any ideas on how to fix the output of the compiletest so it will pass both locally and on CI? It seems the issue is the path on the last line of the error, potentially due to the use of $RUST vs a specified path?

[Veetaha]

I haven't used compiletest. I assume it's an older equivalent of trybuild, which I've been using for my tests. Maybe it will work out of the box with trybuild?

The version of trybuild, that supports 1.56 MSRV is 1.0.89 (can be found in the table here).

Try adding this test, maybe it'll work:

#[test]
fn ui() {
    let t = trybuild::TestCases::new();
    t.compile_fail("tests/compile-fail/*.rs");
}

[TedDriggs]

I'm using trybuild - the compiletests is just the test directory name, since I didn't want to confuse it by recycling the name.

It appears that locally updating trybuild to the latest version triggers this error, so my theory about it being a missing environment variable may have been wrong. Added a commit bumping trybuild to latest version.

[TedDriggs]

Okay, it looks like trybuild@1.0.89 is the sweet spot where builds on 1.56.0 still work, though why CI is ever trying to build with trybuild on 1.56.0 is unclear to me, as that dependency is only present for cfg(compiletests) and that's only used on 1.77.0.

[Veetaha]

why CI is ever trying to build with trybuild on 1.56.0 is unclear to me, as that dependency is only present for cfg(compiletests) and that's only used on 1.77.0

Do you mean this error?:

error: failed to select a version for the requirement `toml = "^0.8"`
candidate versions found which didn't match: 0.5.11, 0.5.10, 0.5.9, ...
location searched: crates.io index
required by package `trybuild v1.0.99`

If yes, then that's expected because this happens not at the building stage, but when updating the crates.io index. cargo needs to make sure the dependency exists (even if it's under a cfg), and create a single Cargo.lock file which lists the pinned dependency versions assuming all dependencies are used.

Although, I must admit, I don't understand why cargo 1.56 can't find the toml ^0.8 crate versions. There clearly are entries for these versions in the index

[TedDriggs]

Yes, that's the error I meant. Thanks for the explanation. For now, it seems that leaving the dependency at 1.0.89 is sufficient. If it ever breaks, that'll probably be a sign it's time to bump darling's MSRV for its next release.

I've updated examples, tests, and CHANGELOG for this feature; I didn't see any places in the actual docs where a change was needed. I do want to look at expanding this to cover the other with scenarios, though I don't think that needs to be part of this PR.