Skip to content

Commit

Permalink
Clean up codebase: fix reasonable clippy warnings.
Browse files Browse the repository at this point in the history
This commit is a codebase-wide cleanup driven by clippy warnings. In
addition to fixing every reasonable warning, the following new
functionality was introduced:

  * `Accept::new()` now takes any `T: Into<QMediaType>` iterator.
  * `TempFile::is_empty()` was added.
  * `HeaderMap` now implements `IntoIterator`.

This commit makes the following breaking changes:

  * The `IntoCollection` trait is gone. Generics previously bound by the
    trait are now bound by `IntoIterator`. This affects:
    - `Accept::new()`
    - `ContentType::with_params()`
    - `Permission::{allow, allowed}()`
  * `MediaType`, `QMediaType`, and `Allow` implement `IntoIterator`,
    enabling most existing code to continue working without change.
  * The inherent `HeaderMap::into_iter()` method was removed.
  * The `Ok` variant in ErrorKind::Liftoff` is now `Box<Rocket<Orbit>>`.
  • Loading branch information
SergioBenitez committed Mar 20, 2024
1 parent d9249db commit 02011a1
Show file tree
Hide file tree
Showing 98 changed files with 453 additions and 393 deletions.
4 changes: 4 additions & 0 deletions contrib/dyn_templates/Cargo.toml
Original file line number Diff line number Diff line change
Expand Up @@ -12,6 +12,10 @@ license = "MIT OR Apache-2.0"
edition = "2021"
rust-version = "1.75"

[lints.clippy]
type_complexity = "allow"
multiple_bound_locations = "allow"

[features]
tera = ["tera_"]
handlebars = ["handlebars_"]
Expand Down
6 changes: 3 additions & 3 deletions contrib/dyn_templates/src/context.rs
Original file line number Diff line number Diff line change
Expand Up @@ -84,7 +84,7 @@ impl Context {
if !templates.contains_key(name) {
let data_type = Path::new(name).extension()
.and_then(|osstr| osstr.to_str())
.and_then(|ext| ContentType::from_extension(ext))
.and_then(ContentType::from_extension)
.unwrap_or(ContentType::Text);

let info = TemplateInfo { path: None, engine_ext, data_type };
Expand Down Expand Up @@ -183,7 +183,7 @@ mod manager {
if let Some(true) = templates_changes {
info_!("Change detected: reloading templates.");
let root = self.context().root.clone();
if let Some(new_ctxt) = Context::initialize(&root, &callback) {
if let Some(new_ctxt) = Context::initialize(&root, callback) {
*self.context_mut() = new_ctxt;
} else {
warn_!("An error occurred while reloading templates.");
Expand Down Expand Up @@ -217,7 +217,7 @@ fn split_path(root: &Path, path: &Path) -> (String, Option<String>) {

// Ensure template name consistency on Windows systems
if cfg!(windows) {
name = name.replace("\\", "/");
name = name.replace('\\', "/");
}

(name, data_type.map(|d| d.to_string_lossy().into_owned()))
Expand Down
2 changes: 1 addition & 1 deletion contrib/dyn_templates/src/handlebars_templates.rs
Original file line number Diff line number Diff line change
Expand Up @@ -20,7 +20,7 @@ impl Engine for Handlebars<'static> {
}
}

ok.then(|| hb)
ok.then_some(hb)
}

fn render<C: Serialize>(&self, name: &str, context: C) -> Option<String> {
Expand Down
2 changes: 1 addition & 1 deletion contrib/dyn_templates/src/lib.rs
Original file line number Diff line number Diff line change
Expand Up @@ -412,7 +412,7 @@ impl Template {
Status::InternalServerError
})?;

let string = ctxt.engines.render(name, &info, value).ok_or_else(|| {
let string = ctxt.engines.render(name, info, value).ok_or_else(|| {
error_!("Template '{}' failed to render.", name);
Status::InternalServerError
})?;
Expand Down
4 changes: 2 additions & 2 deletions contrib/ws/src/websocket.rs
Original file line number Diff line number Diff line change
Expand Up @@ -109,8 +109,8 @@ impl WebSocket {
/// }))
/// }
/// ```
pub fn channel<'r, F: Send + 'r>(self, handler: F) -> Channel<'r>
where F: FnOnce(DuplexStream) -> BoxFuture<'r, Result<()>> + 'r
pub fn channel<'r, F>(self, handler: F) -> Channel<'r>
where F: FnOnce(DuplexStream) -> BoxFuture<'r, Result<()>> + Send + 'r
{
Channel { ws: self, handler: Box::new(handler), }
}
Expand Down
4 changes: 4 additions & 0 deletions core/codegen/Cargo.toml
Original file line number Diff line number Diff line change
Expand Up @@ -15,6 +15,10 @@ rust-version = "1.75"
[lib]
proc-macro = true

[lints.clippy]
manual_range_contains = "allow"
large_enum_variant = "allow"

[dependencies]
indexmap = "2"
quote = "1.0"
Expand Down
2 changes: 1 addition & 1 deletion core/codegen/src/attribute/async_bound/mod.rs
Original file line number Diff line number Diff line change
Expand Up @@ -16,7 +16,7 @@ fn _async_bound(

let mut func: TraitItemFn = syn::parse(input)?;
let original: TraitItemFn = func.clone();
if !func.sig.asyncness.is_some() {
if func.sig.asyncness.is_none() {
let diag = Span::call_site()
.error("attribute can only be applied to async fns")
.span_help(func.sig.span(), "this fn declaration must be `async`");
Expand Down
2 changes: 1 addition & 1 deletion core/codegen/src/attribute/param/parse.rs
Original file line number Diff line number Diff line change
Expand Up @@ -31,7 +31,7 @@ impl Dynamic {
segment: &str,
span: Span,
) -> Result<Self, Error<'_>> {
match Parameter::parse::<P>(&segment, span)? {
match Parameter::parse::<P>(segment, span)? {
Parameter::Dynamic(d) | Parameter::Ignored(d) => Ok(d),
Parameter::Guard(g) => Ok(g.source),
Parameter::Static(_) => Err(Error::new(segment, span, ErrorKind::Static)),
Expand Down
2 changes: 1 addition & 1 deletion core/codegen/src/attribute/route/mod.rs
Original file line number Diff line number Diff line change
Expand Up @@ -293,7 +293,7 @@ fn sentinels_expr(route: &Route) -> TokenStream {

let eligible_types = route.guards()
.map(|guard| &guard.ty)
.chain(ret_ty.as_ref().into_iter())
.chain(ret_ty.as_ref())
.flat_map(|ty| ty.unfold_with_ty_macros(TY_MACS, ty_mac_mapper))
.filter(|ty| ty.is_concrete(&generic_idents))
.map(|child| (child.parent, child.ty));
Expand Down
4 changes: 2 additions & 2 deletions core/codegen/src/attribute/route/parse.rs
Original file line number Diff line number Diff line change
Expand Up @@ -180,7 +180,7 @@ impl Route {
// Collect all of the declared dynamic route parameters.
let all_dyn_params = path_params.iter().filter_map(|p| p.dynamic())
.chain(query_params.iter().filter_map(|p| p.dynamic()))
.chain(data_guard.as_ref().map(|g| &g.source).into_iter());
.chain(data_guard.as_ref().map(|g| &g.source));

// Check for any duplicates in the dynamic route parameters.
let mut dyn_params: IndexSet<&Dynamic> = IndexSet::new();
Expand All @@ -196,7 +196,7 @@ impl Route {
.filter(|(name, _)| {
let mut all_other_guards = path_params.iter().filter_map(|p| p.guard())
.chain(query_params.iter().filter_map(|p| p.guard()))
.chain(data_guard.as_ref().into_iter());
.chain(data_guard.as_ref());

all_other_guards.all(|g| &g.name != *name)
})
Expand Down
6 changes: 3 additions & 3 deletions core/codegen/src/bang/mod.rs
Original file line number Diff line number Diff line change
Expand Up @@ -47,19 +47,19 @@ pub fn catchers_macro(input: proc_macro::TokenStream) -> TokenStream {
pub fn uri_macro(input: proc_macro::TokenStream) -> TokenStream {
uri::_uri_macro(input.into())
.unwrap_or_else(|diag| diag.emit_as_expr_tokens_or(quote! {
rocket::http::uri::Origin::ROOT
rocket::http::uri::Origin::root()
}))
}

pub fn uri_internal_macro(input: proc_macro::TokenStream) -> TokenStream {
// TODO: Ideally we would generate a perfect `Origin::ROOT` so that we don't
// TODO: Ideally we would generate a perfect `Origin::root()` so that we don't
// assist in propagating further errors. Alas, we can't set the span to the
// invocation of `uri!` without access to `span.parent()`, and
// `Span::call_site()` here points to the `#[route]`, immediate caller,
// generating a rather confusing error message when there's a type-mismatch.
uri::_uri_internal_macro(input.into())
.unwrap_or_else(|diag| diag.emit_as_expr_tokens_or(quote! {
rocket::http::uri::Origin::ROOT
rocket::http::uri::Origin::root()
}))
}

Expand Down
6 changes: 3 additions & 3 deletions core/codegen/src/bang/test_guide.rs
Original file line number Diff line number Diff line change
Expand Up @@ -15,10 +15,10 @@ pub fn _macro(input: proc_macro::TokenStream) -> devise::Result<TokenStream> {

fn entry_to_tests(root_glob: &LitStr) -> Result<Vec<TokenStream>, Box<dyn Error>> {
let manifest_dir = std::env::var("CARGO_MANIFEST_DIR").expect("MANIFEST_DIR");
let full_glob = Path::new(&manifest_dir).join(&root_glob.value()).display().to_string();
let full_glob = Path::new(&manifest_dir).join(root_glob.value());

let mut tests = vec![];
for path in glob::glob(&full_glob).map_err(Box::new)? {
for path in glob::glob(&full_glob.to_string_lossy()).map_err(Box::new)? {
let path = path.map_err(Box::new)?;
let name = path.file_name()
.and_then(|f| f.to_str())
Expand All @@ -38,7 +38,7 @@ fn entry_to_tests(root_glob: &LitStr) -> Result<Vec<TokenStream>, Box<dyn Error>
}

if tests.is_empty() {
return Err(format!("glob '{}' evaluates to 0 files", full_glob).into());
return Err(format!("glob '{}' evaluates to 0 files", full_glob.display()).into());
}

Ok(tests)
Expand Down
4 changes: 2 additions & 2 deletions core/codegen/src/bang/uri.rs
Original file line number Diff line number Diff line change
Expand Up @@ -151,7 +151,7 @@ fn explode_path<'a>(
Parameter::Dynamic(_) | Parameter::Guard(_) => {
let (ident, ty) = args.next().expect("ident/ty for non-ignored");
let expr = exprs.next().expect("one expr per dynamic arg");
add_binding::<fmt::Path>(bindings, &ident, &ty, &expr);
add_binding::<fmt::Path>(bindings, ident, ty, expr);
quote_spanned!(expr.span() => &#ident as &dyn #uri_display)
}
Parameter::Ignored(_) => {
Expand Down Expand Up @@ -207,7 +207,7 @@ fn explode_query<'a>(
};

let name = &dynamic.name;
add_binding::<fmt::Query>(bindings, &ident, &ty, &expr);
add_binding::<fmt::Query>(bindings, ident, ty, expr);
Some(match dynamic.trailing {
false => quote_spanned! { expr.span() =>
#query_arg::NameValue(#name, &#ident as &dyn #uri_display)
Expand Down
7 changes: 2 additions & 5 deletions core/codegen/src/bang/uri_parsing.rs
Original file line number Diff line number Diff line change
Expand Up @@ -421,10 +421,7 @@ impl RoutedUri {

impl Arg {
fn is_named(&self) -> bool {
match *self {
Arg::Named(..) => true,
_ => false
}
matches!(self, Arg::Named(..))
}

fn unnamed(&self) -> &ArgExpr {
Expand Down Expand Up @@ -484,7 +481,7 @@ impl UriExpr {
let lit = input.parse::<StringLit>()?;
let uri = Uri::parse::<Origin<'_>>(&lit)
.or_else(|e| Uri::parse::<Absolute<'_>>(&lit).map_err(|e2| (e, e2)))
.map_err(|(e1, e2)| lit.starts_with('/').then(|| e1).unwrap_or(e2))
.map_err(|(e1, e2)| if lit.starts_with('/') { e1 } else { e2 })
.or_else(|e| uri_err(&lit, e))?;

if matches!(&uri, Uri::Origin(o) if o.query().is_some())
Expand Down
10 changes: 6 additions & 4 deletions core/codegen/src/derive/form_field.rs
Original file line number Diff line number Diff line change
Expand Up @@ -327,15 +327,15 @@ impl VisitMut for ValidationMutator<'_> {

*i = expr;
}
} else if let Some(expr) = inner_field(&i) {
} else if let Some(expr) = inner_field(i) {
*i = expr;
}

syn::visit_mut::visit_expr_mut(self, i);
}
}

pub fn validators<'v>(field: Field<'v>) -> Result<impl Iterator<Item = syn::Expr> + 'v> {
pub fn validators(field: Field<'_>) -> Result<impl Iterator<Item = syn::Expr> + '_> {
Ok(FieldAttr::from_attrs(FieldAttr::NAME, &field.attrs)?
.into_iter()
.chain(FieldAttr::from_attrs(FieldAttr::NAME, field.parent.attrs())?)
Expand Down Expand Up @@ -396,7 +396,7 @@ fn default_expr(expr: &syn::Expr) -> TokenStream {
}
}

pub fn default<'v>(field: Field<'v>) -> Result<Option<TokenStream>> {
pub fn default(field: Field<'_>) -> Result<Option<TokenStream>> {
let field_attrs = FieldAttr::from_attrs(FieldAttr::NAME, &field.attrs)?;
let parent_attrs = FieldAttr::from_attrs(FieldAttr::NAME, field.parent.attrs())?;

Expand Down Expand Up @@ -446,10 +446,12 @@ pub fn default<'v>(field: Field<'v>) -> Result<Option<TokenStream>> {
}
}

type Dup = (usize, Span, Span);

pub fn first_duplicate<K: Spanned, V: PartialEq + Spanned>(
keys: impl Iterator<Item = K> + Clone,
values: impl Fn(&K) -> Result<Vec<V>>,
) -> Result<Option<((usize, Span, Span), (usize, Span, Span))>> {
) -> Result<Option<(Dup, Dup)>> {
let (mut all_values, mut key_map) = (vec![], vec![]);
for key in keys {
all_values.append(&mut values(&key)?);
Expand Down
2 changes: 1 addition & 1 deletion core/codegen/src/derive/responder.rs
Original file line number Diff line number Diff line change
Expand Up @@ -79,7 +79,7 @@ pub fn derive_responder(input: proc_macro::TokenStream) -> TokenStream {

let responder = fields.iter().next().map(|f| {
let (accessor, ty) = (f.accessor(), f.ty.with_stripped_lifetimes());
quote_spanned! { f.span().into() =>
quote_spanned! { f.span() =>
let mut __res = <#ty as #_response::Responder>::respond_to(
#accessor, __req
)?;
Expand Down
1 change: 0 additions & 1 deletion core/codegen/src/exports.rs
Original file line number Diff line number Diff line change
Expand Up @@ -99,7 +99,6 @@ define_exported_paths! {
StaticCatcherInfo => ::rocket::StaticCatcherInfo,
Route => ::rocket::Route,
Catcher => ::rocket::Catcher,
SmallVec => ::rocket::http::private::SmallVec,
Status => ::rocket::http::Status,
}

Expand Down
4 changes: 2 additions & 2 deletions core/codegen/tests/route-format.rs
Original file line number Diff line number Diff line change
Expand Up @@ -89,10 +89,10 @@ fn test_custom_formats() {

let client = Client::debug(rocket).unwrap();

let foo_a = Accept::new([MediaType::new("application", "foo").into()]);
let foo_a = Accept::new([MediaType::new("application", "foo")]);
let foo_ct = ContentType::new("application", "foo");
let bar_baz_ct = ContentType::new("bar", "baz");
let bar_baz_a = Accept::new([MediaType::new("bar", "baz").into()]);
let bar_baz_a = Accept::new([MediaType::new("bar", "baz")]);

let response = client.get("/").header(foo_a).dispatch();
assert_eq!(response.into_string().unwrap(), "get_foo");
Expand Down
9 changes: 7 additions & 2 deletions core/http/Cargo.toml
Original file line number Diff line number Diff line change
Expand Up @@ -20,15 +20,20 @@ default = []
serde = ["uncased/with-serde-alloc", "serde_"]
uuid = ["uuid_"]

[lints.clippy]
module_inception = "allow"
multiple_bound_locations = "allow"
manual_range_contains = "allow"

[dependencies]
smallvec = { version = "1.11", features = ["const_generics", "const_new"] }
tinyvec = { version = "1.6", features = ["std", "rustc_1_57"] }
percent-encoding = "2"
time = { version = "0.3", features = ["formatting", "macros"] }
indexmap = "2"
ref-cast = "1.0"
uncased = "0.9.10"
either = "1"
pear = "0.2.8"
pear = "0.2.9"
memchr = "2"
stable-pattern = "0.1"
cookie = { version = "0.18", features = ["percent-encode"] }
Expand Down
70 changes: 2 additions & 68 deletions core/http/src/ext.rs
Original file line number Diff line number Diff line change
@@ -1,75 +1,9 @@
//! Extension traits implemented by several HTTP types.

use smallvec::{Array, SmallVec};
use state::InitCell;

// TODO: It would be nice if we could somehow have one trait that could give us
// either SmallVec or Vec.
/// Trait implemented by types that can be converted into a collection.
pub trait IntoCollection<T>: Sized {
/// Converts `self` into a collection.
fn into_collection<A: Array<Item=T>>(self) -> SmallVec<A>;

#[doc(hidden)]
fn mapped<U, F: FnMut(T) -> U, A: Array<Item=U>>(self, f: F) -> SmallVec<A>;
}

impl<T> IntoCollection<T> for T {
#[inline]
fn into_collection<A: Array<Item=T>>(self) -> SmallVec<A> {
let mut vec = SmallVec::new();
vec.push(self);
vec
}

#[inline(always)]
fn mapped<U, F: FnMut(T) -> U, A: Array<Item=U>>(self, mut f: F) -> SmallVec<A> {
f(self).into_collection()
}
}

impl<T> IntoCollection<T> for Vec<T> {
#[inline(always)]
fn into_collection<A: Array<Item=T>>(self) -> SmallVec<A> {
SmallVec::from_vec(self)
}

#[inline]
fn mapped<U, F: FnMut(T) -> U, A: Array<Item=U>>(self, f: F) -> SmallVec<A> {
self.into_iter().map(f).collect()
}
}

impl<T: Clone> IntoCollection<T> for &[T] {
#[inline(always)]
fn into_collection<A: Array<Item=T>>(self) -> SmallVec<A> {
self.iter().cloned().collect()
}

#[inline]
fn mapped<U, F, A: Array<Item=U>>(self, f: F) -> SmallVec<A>
where F: FnMut(T) -> U
{
self.iter().cloned().map(f).collect()
}
}

impl<T, const N: usize> IntoCollection<T> for [T; N] {
#[inline(always)]
fn into_collection<A: Array<Item=T>>(self) -> SmallVec<A> {
self.into_iter().collect()
}

#[inline]
fn mapped<U, F, A: Array<Item=U>>(self, f: F) -> SmallVec<A>
where F: FnMut(T) -> U
{
self.into_iter().map(f).collect()
}
}

use std::borrow::Cow;

use state::InitCell;

/// Trait implemented by types that can be converted into owned versions of
/// themselves.
pub trait IntoOwned {
Expand Down
Loading

0 comments on commit 02011a1

Please sign in to comment.