Work In Progress Rust
10 points by dureuill
10 points by dureuill
To be honest, when I see the rationale for wip, the impression I get is "author doesn't understand how the tool is meant to be used, so they reinvented Clippy".
The toolchain devs draw a distinction between what gets to be a rustc warning and what gets to be a Clippy lint and wip seems to do a fair bit of reinventing existing functionality under new names just to force those lints across the divide from cargo clippy into cargo check and cargo build where you have to see them during prototyping when they'll flood out warnings that you may want to address during prototyping.
For example:
wip!'s desciption is #![warn(clippy::todo)] forced to go off outside cargo clippy at the cost of adding a new dependency and new non-standard construct.unwrap_wip is #![warn(clippy::unwrap_used)].For me, .unwrap() is for prototyping and I set #![warn(clippy::unwrap_used)] while .expect("description of invariant") is for production cases where you'd use .unwrap() instead of .unwrap_wip() and I do not set #![warn(clippy::expect_used)]. (eg. .expect("parse regex from const"))
I'm not sure about clone_wip and fixme! directly. I've never thought on those use-cases enough to have a ready answer for how the devs intended those cases to be handled.
Hey, that may be honest, but that's not super nice :-) I've been using this stuff for a decade, of course that doesn't mean I know everything, but perhaps leave me the benefit of the doubt?
You are right though, that the article is probably missing a discussion about existing linters like clippy. The omission might be tied to my personal use of the tool: I used to have clippy set to run on save, but the tool was so slow on our project that I had to stop that, otherwise I was facing minutes between saves... Now it's running in CI + as a pre-push hook.
Pulling the warnings to rustc is much, much faster and allows to display in-line warnings while iterating, which is the point of the workflow.
Clippy is also a pain to configure, at least last time I checked, it didn't have a way to specify a lint across all crates of a workspace in code, so we had to resort to adding an explicit -D in our CI for every lint we wanted to deny in the workspace, or the global attributes to all crates all the time.
To respond to your points directly:
#![warn(clippy::todo) is needed for all crates, so that's not a huge win for us over cargo add/rm a crate just during development. I really think todo! should warn by default either in rustc or in clippy, and then I'd agree that'd make wip less useful. I'd be glad if the crate could move the needle on that front.unwrap_wip is #![warn(clippy::unwrap_used)] on at least two fronts:
unwrap to non production and expect to prod is a common standpoint but far from universal.expects, but then we'd have to use a placeholder message everywhere, or spend way too much time re-materializing the precondition, which, a lot of the time is going to be "yes this mutex is not poisoned" or "yes I want to propagate the failure of this task to the caller".fixme! is super useful. I'd love its inclusion in std or something, it's actually what I've been using the most in the crate.wip_iter that lets you use a todo like macro in a function with a Return-Position-Impl-Trait for iterators, and same with wip_future. Happened a lot of time to me that I had to "make up a type" because return type inference + todo doesn't work. Someone also suggested throwing a compile error by default in release mode, that I will explore along with some other ideas for improvements I have.the cost of adding a new dependency and new non-standard construct.
To balance a bit this, this is a single-file dep that you'll only use during development and should typically have removed by the time you finish
(Not a full response, but some good news!)
last time I checked, it didn't have a way to specify a lint across all crates of a workspace in code
Now in the top-level Cargo.toml you can assign lint levels under [workspace.lints.clippy] like lint_name = "allow"|"warn"|"deny" as long as each crate's Cargo.toml sets lints.workspace = true.
A problem of prototyping with unwrap is when you want to introduce errors correctly later it can change the shape of your return types and that has knock on effects, especially if those functions are used in a functional way.
I would encourage using result with a box dyn error or string or anyhow as the error type. You can always change it to return a “real” error enum later. But it’s harder to refactor code that looks infaillible to return a result later.
And, also be careful if you use LLMs. They are pattern matching machines and don’t distinguish between “intentionally did something meh I want to improve later” and "this is how this project SHOULD handle errors. I.e. it will multiply your "just for now" decisions over time. A comment or two on main sometimes help guide them for where/when it's okay to use a lazier error.
Usually I add the wip dependency to crates in the workspace during development, and then remove it when rewriting history before review.
I wonder if there's a way to smooth this out a bit with extra tooling...I'm just a bit wary of having to amend the middle of a PR w/ lock file rewrites, since if the dependency tree was manipulated elsewhere inside then you end up with extraneous conflicts. Perhaps I can abuse jj fix... Regardless, very nice project idea, I'm sure I'll end up using it somewhere.
I saw at least once someone being wrong on the Internet about
todo!(): they would state that it was meant to communicate to users of a program that some features were not yet implemented.Rust actually has a dedicated macro for this use case, and its name is
unimplemented!().
I and the docs disagree with your interpretation (though the following paragraph which I omitted balances your position out a little):
The difference between
unimplemented!andtodo!is that whiletodo!conveys an intent of implementing the functionality later and the message is “not yet implemented”,unimplemented!makes no such claims. Its message is “not implemented”.
When todo! was added in 1.40.0, it was purely a shorter name:
There is no difference between
unimplemented!andtodo!apart from the name.
Only in 1.42.0 was the word “yet” removed from unimplemented’s message. And deciding on these semantics for both was mildly controversial, as I remember it; as had been shipping todo! as a shorter spelling of unimplemented!.
These days, unimplemented!() is suggested for places like trait implementations where for technical reasons a method body must be supplied, but which should never be called. And again, that’s what the docs show as an example.
Some interesting ideas that hopefully motivate some changes in rustc. todo!() should really have a warning by default.
It would also be nice if these kinds of warnings were somehow turned off when the developer was in hacky mode. Too many warnings is overwhelming, I'm usually running cargo in a 19 line console at the bottom of my IDE. All those dev warnings drown out real errors and more important warnings. Some discipline in always sorting the errors last could ease this problem too.
Another thing I would point out is that the names of .unwrap_wip() and .clone_wip() are too long, longer than the originals. My recommendation would be to name them .du() and .dc() for "dev unwrap" and "dev clone". That would encourage their use in quick programming.