Introduce #[suppress]: suppress Rocket lints.

Adding `#[suppress(lint)]` for a known `lint` suppresses the
corresponding warning generated by Rocket's codegen. The lints are:

  * `unknown_format`
  * `dubious_payload`
  * `segment_chars`
  * `arbitrary_main`
  * `sync_spawn`

For example, the following works as expected to suppress the warning
generated by the unknown media type "application/foo":

```rust
fn post_foo() -> &'static str { "post_foo" }
```

Resolves #1188.
This commit is contained in:
Sergio Benitez 2024-03-21 20:24:53 -07:00
parent 35a1cf12b6
commit bd26ca462a
12 changed files with 201 additions and 19 deletions

View File

@ -3,6 +3,7 @@ use devise::ext::SpanDiagnosticExt;
use proc_macro2::{TokenStream, Span}; use proc_macro2::{TokenStream, Span};
use super::EntryAttr; use super::EntryAttr;
use crate::attribute::suppress::Lint;
use crate::exports::mixed; use crate::exports::mixed;
/// `#[rocket::launch]`: generates a `main` function that calls the attributed /// `#[rocket::launch]`: generates a `main` function that calls the attributed
@ -90,13 +91,15 @@ impl EntryAttr for Launch {
None => quote_spanned!(ty.span() => #rocket.launch()), None => quote_spanned!(ty.span() => #rocket.launch()),
}; };
if f.sig.asyncness.is_none() { let lint = Lint::SyncSpawn;
if f.sig.asyncness.is_none() && lint.enabled(f.sig.asyncness.span()) {
if let Some(call) = likely_spawns(f) { if let Some(call) = likely_spawns(f) {
call.span() call.span()
.warning("task is being spawned outside an async context") .warning("task is being spawned outside an async context")
.span_help(f.sig.span(), "declare this function as `async fn` \ .span_help(f.sig.span(), "declare this function as `async fn` \
to require async execution") to require async execution")
.span_note(Span::call_site(), "`#[launch]` call is here") .span_note(Span::call_site(), "`#[launch]` call is here")
.note(lint.how_to_suppress())
.emit_as_expr_tokens(); .emit_as_expr_tokens();
} }
} }

View File

@ -1,3 +1,5 @@
use crate::attribute::suppress::Lint;
use super::EntryAttr; use super::EntryAttr;
use devise::{Spanned, Result}; use devise::{Spanned, Result};
@ -12,11 +14,12 @@ impl EntryAttr for Main {
fn function(f: &mut syn::ItemFn) -> Result<TokenStream> { fn function(f: &mut syn::ItemFn) -> Result<TokenStream> {
let (attrs, vis, block, sig) = (&f.attrs, &f.vis, &f.block, &mut f.sig); let (attrs, vis, block, sig) = (&f.attrs, &f.vis, &f.block, &mut f.sig);
if sig.ident != "main" { let lint = Lint::ArbitraryMain;
// FIXME(diag): warning! if sig.ident != "main" && lint.enabled(sig.ident.span()) {
Span::call_site() Span::call_site()
.warning("attribute is typically applied to `main` function") .warning("attribute is typically applied to `main` function")
.span_note(sig.ident.span(), "this function is not `main`") .span_note(sig.ident.span(), "this function is not `main`")
.note(lint.how_to_suppress())
.emit_as_item_tokens(); .emit_as_item_tokens();
} }

View File

@ -6,6 +6,8 @@ use devise::{Diagnostic, Spanned, Result};
use devise::ext::SpanDiagnosticExt; use devise::ext::SpanDiagnosticExt;
use proc_macro2::{TokenStream, Span}; use proc_macro2::{TokenStream, Span};
use crate::attribute::suppress::Lint;
// Common trait implemented by `async` entry generating attributes. // Common trait implemented by `async` entry generating attributes.
trait EntryAttr { trait EntryAttr {
/// Whether the attribute requires the attributed function to be `async`. /// Whether the attribute requires the attributed function to be `async`.
@ -23,6 +25,8 @@ fn _async_entry<A: EntryAttr>(
.map_err(Diagnostic::from) .map_err(Diagnostic::from)
.map_err(|d| d.help("attribute can only be applied to functions"))?; .map_err(|d| d.help("attribute can only be applied to functions"))?;
Lint::suppress_attrs(&function.attrs, function.span());
if A::REQUIRES_ASYNC && function.sig.asyncness.is_none() { if A::REQUIRES_ASYNC && function.sig.asyncness.is_none() {
return Err(Span::call_site() return Err(Span::call_site()
.error("attribute can only be applied to `async` functions") .error("attribute can only be applied to `async` functions")

View File

@ -3,3 +3,4 @@ pub mod catch;
pub mod route; pub mod route;
pub mod param; pub mod param;
pub mod async_bound; pub mod async_bound;
pub mod suppress;

View File

@ -6,6 +6,7 @@ use crate::name::Name;
use crate::proc_macro_ext::StringLit; use crate::proc_macro_ext::StringLit;
use crate::attribute::param::{Parameter, Dynamic}; use crate::attribute::param::{Parameter, Dynamic};
use crate::http::uri::fmt::{Part, Kind, Path}; use crate::http::uri::fmt::{Part, Kind, Path};
use crate::attribute::suppress::Lint;
#[derive(Debug)] #[derive(Debug)]
pub struct Error<'a> { pub struct Error<'a> {
@ -47,6 +48,7 @@ impl Parameter {
let mut trailing = false; let mut trailing = false;
// Check if this is a dynamic param. If so, check its well-formedness. // Check if this is a dynamic param. If so, check its well-formedness.
let lint = Lint::SegmentChars;
if segment.starts_with('<') && segment.ends_with('>') { if segment.starts_with('<') && segment.ends_with('>') {
let mut name = &segment[1..(segment.len() - 1)]; let mut name = &segment[1..(segment.len() - 1)];
if name.ends_with("..") { if name.ends_with("..") {
@ -71,12 +73,13 @@ impl Parameter {
} }
} else if segment.is_empty() { } else if segment.is_empty() {
return Err(Error::new(segment, source_span, ErrorKind::Empty)); return Err(Error::new(segment, source_span, ErrorKind::Empty));
} else if segment.starts_with('<') { } else if segment.starts_with('<') && lint.enabled(source_span) {
let candidate = candidate_from_malformed(segment); let candidate = candidate_from_malformed(segment);
source_span.warning("`segment` starts with `<` but does not end with `>`") source_span.warning("`segment` starts with `<` but does not end with `>`")
.help(format!("perhaps you meant the dynamic parameter `<{}>`?", candidate)) .help(format!("perhaps you meant the dynamic parameter `<{}>`?", candidate))
.note(lint.how_to_suppress())
.emit_as_item_tokens(); .emit_as_item_tokens();
} else if segment.contains('>') || segment.contains('<') { } else if (segment.contains('>') || segment.contains('<')) && lint.enabled(source_span) {
source_span.warning("`segment` contains `<` or `>` but is not a dynamic parameter") source_span.warning("`segment` contains `<` or `>` but is not a dynamic parameter")
.emit_as_item_tokens(); .emit_as_item_tokens();
} }

View File

@ -14,6 +14,8 @@ use crate::exports::mixed;
use self::parse::{Route, Attribute, MethodAttribute}; use self::parse::{Route, Attribute, MethodAttribute};
use super::suppress::Lint;
impl Route { impl Route {
pub fn guards(&self) -> impl Iterator<Item = &Guard> { pub fn guards(&self) -> impl Iterator<Item = &Guard> {
self.param_guards() self.param_guards()
@ -399,17 +401,18 @@ fn incomplete_route(
args: TokenStream, args: TokenStream,
input: TokenStream input: TokenStream
) -> Result<TokenStream> { ) -> Result<TokenStream> {
let method_str = method.to_string().to_lowercase();
// FIXME(proc_macro): there should be a way to get this `Span`. // FIXME(proc_macro): there should be a way to get this `Span`.
let method_str = method.to_string().to_lowercase();
let method_span = StringLit::new(format!("#[{}]", method), Span::call_site()) let method_span = StringLit::new(format!("#[{}]", method), Span::call_site())
.subspan(2..2 + method_str.len()); .subspan(2..2 + method_str.len());
let method_ident = syn::Ident::new(&method_str, method_span); let full_span = args.span().join(input.span()).unwrap_or(input.span());
let function: syn::ItemFn = syn::parse2(input) let function: syn::ItemFn = syn::parse2(input)
.map_err(Diagnostic::from) .map_err(Diagnostic::from)
.map_err(|d| d.help(format!("#[{}] can only be used on functions", method_str)))?; .map_err(|d| d.help(format!("#[{}] can only be used on functions", method_str)))?;
Lint::suppress_attrs(&function.attrs, full_span);
let method_ident = syn::Ident::new(&method_str, method_span);
let full_attr = quote!(#method_ident(#args)); let full_attr = quote!(#method_ident(#args));
let method_attribute = MethodAttribute::from_meta(&syn::parse2(full_attr)?)?; let method_attribute = MethodAttribute::from_meta(&syn::parse2(full_attr)?)?;

View File

@ -3,6 +3,7 @@ use devise::ext::{SpanDiagnosticExt, TypeExt};
use indexmap::{IndexSet, IndexMap}; use indexmap::{IndexSet, IndexMap};
use proc_macro2::Span; use proc_macro2::Span;
use crate::attribute::suppress::Lint;
use crate::proc_macro_ext::Diagnostics; use crate::proc_macro_ext::Diagnostics;
use crate::http_codegen::{Method, MediaType}; use crate::http_codegen::{Method, MediaType};
use crate::attribute::param::{Parameter, Dynamic, Guard}; use crate::attribute::param::{Parameter, Dynamic, Guard};
@ -127,11 +128,12 @@ impl Route {
// Emit a warning if a `data` param was supplied for non-payload methods. // Emit a warning if a `data` param was supplied for non-payload methods.
if let Some(ref data) = attr.data { if let Some(ref data) = attr.data {
if !attr.method.0.supports_payload() { let lint = Lint::DubiousPayload;
let msg = format!("'{}' does not typically support payloads", attr.method.0); if !attr.method.0.supports_payload() && lint.enabled(handler.span()) {
// FIXME(diag: warning) // FIXME(diag: warning)
data.full_span.warning("`data` used with non-payload-supporting method") data.full_span.warning("`data` used with non-payload-supporting method")
.span_note(attr.method.span, msg) .note(format!("'{}' does not typically support payloads", attr.method.0))
.note(lint.how_to_suppress())
.emit_as_item_tokens(); .emit_as_item_tokens();
} }
} }

View File

@ -0,0 +1,115 @@
use std::cell::RefCell;
use std::collections::{HashMap, HashSet};
use std::ops::Range;
use devise::ext::PathExt;
use proc_macro2::{Span, TokenStream};
use syn::{parse::Parser, punctuated::Punctuated};
macro_rules! declare_lints {
($($name:ident ( $string:literal) ),* $(,)?) => (
#[derive(Debug, Copy, Clone, PartialEq, Eq, Hash, PartialOrd, Ord)]
pub enum Lint {
$($name),*
}
impl Lint {
fn from_str(string: &str) -> Option<Self> {
$(if string.eq_ignore_ascii_case($string) {
return Some(Lint::$name);
})*
None
}
fn as_str(&self) -> &'static str {
match self {
$(Lint::$name => $string),*
}
}
fn lints() -> &'static str {
concat!("[" $(,$string,)", "* "]")
}
}
)
}
declare_lints! {
UnknownFormat("unknown_format"),
DubiousPayload("dubious_payload"),
SegmentChars("segment_chars"),
ArbitraryMain("arbitrary_main"),
SyncSpawn("sync_spawn"),
}
thread_local! {
static SUPPRESSIONS: RefCell<HashMap<Lint, HashSet<Range<usize>>>> = RefCell::default();
}
fn span_to_range(span: Span) -> Option<Range<usize>> {
let string = format!("{span:?}");
let i = string.find('(')?;
let j = string[i..].find(')')?;
let (start, end) = string[(i + 1)..(i + j)].split_once("..")?;
Some(Range { start: start.parse().ok()?, end: end.parse().ok()? })
}
impl Lint {
pub fn suppress_attrs(attrs: &[syn::Attribute], ctxt: Span) {
let _ = attrs.iter().try_for_each(|attr| Lint::suppress_attr(attr, ctxt));
}
pub fn suppress_attr(attr: &syn::Attribute, ctxt: Span) -> Result<(), syn::Error> {
let syn::Meta::List(list) = &attr.meta else {
return Ok(());
};
if !list.path.last_ident().map_or(false, |i| i == "suppress") {
return Ok(());
}
Self::suppress_tokens(list.tokens.clone(), ctxt)
}
pub fn suppress_tokens(attr_tokens: TokenStream, ctxt: Span) -> Result<(), syn::Error> {
let lints = Punctuated::<Lint, syn::Token![,]>::parse_terminated.parse2(attr_tokens)?;
lints.iter().for_each(|lint| lint.suppress(ctxt));
Ok(())
}
pub fn suppress(self, ctxt: Span) {
SUPPRESSIONS.with_borrow_mut(|s| {
let range = span_to_range(ctxt).unwrap_or_default();
s.entry(self).or_default().insert(range);
})
}
pub fn is_suppressed(self, ctxt: Span) -> bool {
SUPPRESSIONS.with_borrow(|s| {
let this = span_to_range(ctxt).unwrap_or_default();
s.get(&self).map_or(false, |set| {
set.iter().any(|r| this.start >= r.start && this.end <= r.end)
})
})
}
pub fn enabled(self, ctxt: Span) -> bool {
!self.is_suppressed(ctxt)
}
pub fn how_to_suppress(self) -> String {
format!("apply `#[suppress({})]` before the item to suppress this lint", self.as_str())
}
}
impl syn::parse::Parse for Lint {
fn parse(input: syn::parse::ParseStream<'_>) -> syn::Result<Self> {
let ident: syn::Ident = input.parse()?;
let name = ident.to_string();
Lint::from_str(&name).ok_or_else(|| {
let msg = format!("invalid lint `{name}` (known lints: {})", Lint::lints());
syn::Error::new(ident.span(), msg)
})
}
}

View File

@ -0,0 +1,20 @@
use proc_macro2::TokenStream;
use devise::Spanned;
mod lint;
pub use lint::Lint;
pub fn suppress_attribute(
args: proc_macro::TokenStream,
input: proc_macro::TokenStream
) -> TokenStream {
let input: TokenStream = input.into();
match Lint::suppress_tokens(args.into(), input.span()) {
Ok(_) => input,
Err(e) => {
let error: TokenStream = e.to_compile_error().into();
quote!(#error #input)
}
}
}

View File

@ -2,7 +2,7 @@ use quote::ToTokens;
use devise::{FromMeta, MetaItem, Result, ext::{Split2, PathExt, SpanDiagnosticExt}}; use devise::{FromMeta, MetaItem, Result, ext::{Split2, PathExt, SpanDiagnosticExt}};
use proc_macro2::{TokenStream, Span}; use proc_macro2::{TokenStream, Span};
use crate::http; use crate::{http, attribute::suppress::Lint};
#[derive(Debug)] #[derive(Debug)]
pub struct ContentType(pub http::ContentType); pub struct ContentType(pub http::ContentType);
@ -73,10 +73,11 @@ impl FromMeta for MediaType {
let mt = http::MediaType::parse_flexible(&String::from_meta(meta)?) let mt = http::MediaType::parse_flexible(&String::from_meta(meta)?)
.ok_or_else(|| meta.value_span().error("invalid or unknown media type"))?; .ok_or_else(|| meta.value_span().error("invalid or unknown media type"))?;
if !mt.is_known() { let lint = Lint::UnknownFormat;
// FIXME(diag: warning) if !mt.is_known() && lint.enabled(meta.value_span()) {
meta.value_span() meta.value_span()
.warning(format!("'{}' is not a known media type", mt)) .warning(format!("'{}' is not a known format or media type", mt))
.note(lint.how_to_suppress())
.emit_as_item_tokens(); .emit_as_item_tokens();
} }

View File

@ -356,6 +356,33 @@ pub fn catch(args: TokenStream, input: TokenStream) -> TokenStream {
emit!(attribute::catch::catch_attribute(args, input)) emit!(attribute::catch::catch_attribute(args, input))
} }
/// Suppress a warning generated by a Rocket lint.
///
/// Lints:
/// * `unknown_format`
/// * `dubious_payload`
/// * `segment_chars`
/// * `arbitrary_main`
/// * `sync_spawn`
///
/// # Example
///
/// ```rust
/// # #[macro_use] extern crate rocket;
///
/// #[suppress(dubious_payload)]
/// #[get("/", data = "<_a>")]
/// fn _f(_a: String) { }
///
/// #[get("/", data = "<_a>", format = "foo/bar")]
/// #[suppress(dubious_payload, unknown_format)]
/// fn _g(_a: String) { }
/// ```
#[proc_macro_attribute]
pub fn suppress(args: TokenStream, input: TokenStream) -> TokenStream {
emit!(attribute::suppress::suppress_attribute(args, input))
}
/// Retrofits supports for `async fn` in unit tests. /// Retrofits supports for `async fn` in unit tests.
/// ///
/// Simply decorate a test `async fn` with `#[async_test]` instead of `#[test]`: /// Simply decorate a test `async fn` with `#[async_test]` instead of `#[test]`:

View File

@ -66,19 +66,19 @@ fn test_formats() {
// Test custom formats. // Test custom formats.
// TODO: #[rocket(allow(unknown_format))] #[suppress(unknown_format)]
#[get("/", format = "application/foo")] #[get("/", format = "application/foo")]
fn get_foo() -> &'static str { "get_foo" } fn get_foo() -> &'static str { "get_foo" }
// TODO: #[rocket(allow(unknown_format))] #[suppress(unknown_format)]
#[post("/", format = "application/foo")] #[post("/", format = "application/foo")]
fn post_foo() -> &'static str { "post_foo" } fn post_foo() -> &'static str { "post_foo" }
// TODO: #[rocket(allow(unknown_format))] #[suppress(unknown_format)]
#[get("/", format = "bar/baz", rank = 2)] #[get("/", format = "bar/baz", rank = 2)]
fn get_bar_baz() -> &'static str { "get_bar_baz" } fn get_bar_baz() -> &'static str { "get_bar_baz" }
// TODO: #[rocket(allow(unknown_format))] #[suppress(unknown_format)]
#[put("/", format = "bar/baz")] #[put("/", format = "bar/baz")]
fn put_bar_baz() -> &'static str { "put_bar_baz" } fn put_bar_baz() -> &'static str { "put_bar_baz" }