-
-
Notifications
You must be signed in to change notification settings - Fork 1.6k
New issue
Have a question about this project? Sign up for a free GitHub account to open an issue and contact its maintainers and the community.
By clicking “Sign up for GitHub”, you agree to our terms of service and privacy statement. We’ll occasionally send you account related emails.
Already on GitHub? Sign in to your account
Linting #1114
Linting #1114
Conversation
- Use hyper's MakeService implementation with futures API - Use tokio runtime to serve HTTP backend
Minimum rustc bump required for rust-lang/rust#61775
This lets us keep support for keep-alive and remote address while doing other work on async, at the cost of TLS. Abstracting over the connection type will be done more thoroughly later.
This requires some awkward channel and spawning work because Body might contain borrowed data.
Adds body_string_wait and body_bytes_wait functions to LocalRequest for convenience.
This is required to be able to do anything useful with the body in the outgoing response. Request fairings do not appear to need to be async as everything on Data that returns a future moves self and on_request only gets &Data, but the same change in this commit should work for on_request if desired.
Like with response fairings, this is required to be able to do anything useful with the body.
Also update 'static_files' example.
…ely match the pre-async code.
This adds an unstable "async_test" function for use in tests, to ensure they stay working while refactoring other APIs.
… operations. Despite this change, using body_bytes_wait on (for example) a File will still fail due to tokio-rs/tokio#1356.
* body_string_wait and body_bytes_wait are removed; use `.await` instead * `dispatch()` is now an async fn and must be .await-ed * Add `#[rocket::async_test]` macro, similar in purpose to `tokio::test` * Tests in core use either `rocket::async_test(async { })` or `#[rocket::async_test]` in order to `.await` the futures returned from `dispatch()` and `body_{string,bytes}()` Broken: * Cloned dispatch and mut_dispatch() with a live previous response now both fail, due to a (partial) check for mutable aliasing in LocalRequest.
…mples. Some tests are still failing and need example-specific changes.
Additionally pin tokio to '=0.2.0-alpha.2'; before this change cargo selects '0.2.0-alpha.3' which hyper does not compile against.
Doing this is allowed in newer versions of the compiler.
Use `crate` over `pub`. This way, rustc can let us know if something is actually unused, rather than assuming it may be used publicly.
`.params()` already returns an iterator
This is likely left over from the conversion to 2018, where `extern crate` would have been converted to this.
In these situations, the relevant path is already imported into the current scope. As such, we can remove parts of the path without cost.
Using an ident in a match arm binds the value, rather than matching on type, which is what is wanted here. To match on type, we need to use the path.
The enum variants in question should still be in scope there, so that is concerning. Which lint says this? Which lints were all of these, in general? We may want to
Is it a newer version than we already require? I think it's the same version #964 bumped to ( I think I will want to hold off on some of the more disruptive changes until |
The non-clippy lints come from this list. I enabled them one at a time to avoid information overload; there's a few false positives. rustc lints
clippy lintsJust including the ones that have warnings.
Nope. I don't remember the minimum nightly, I know we require at least an August compiler because of async/await stabilization, and I have used this before then on my own projects. #964 sounds right.
Fair. I'm sure it'll already be quite interesting to merge the branch in.
Looking into this again, I believe I was mistaken. I recall experimentally removing the |
Per a brief discussion on IRC/Matrix, this will be held off on until after merging |
3ccc3b2
to
dee92be
Compare
Closing this out to clear the queue. Let's resume the discussion once |
Sounds good @SergioBenitez. Let me know when async gets merged and I'll create another PR. |
Most of these lints are either built into rustc (and are allowed by default) or are available via clippy. A couple I've done by hand by using ripgrep to find instances (an example of this is
use foo
declarations, which isn't available via tooling).Commit messages, which describe what was changed:
Remove unnecessary braces on import
Use shorter path where already imported
In these situations, the relevant path is already imported into the current scope. As such, we can remove parts of the path without cost.
Remove
use foo
, which is a noopThis is likely left over from the conversion to 2018, where
extern crate
would have been converted to this.Use struct field shorthand where possible
Remove identity function
.params()
already returns an iteratorRestrict visibility where not externally visible
Use
crate
overpub
. This way, rustc can let us know if something is actually unused, rather than assuming it may be used publicly.Remove
self
from path where possibleDoing this is allowed in newer versions of the compiler.