From bd26ca462a9aacb697aecc700a8e48c3bf2ac7cb Mon Sep 17 00:00:00 2001 From: Sergio Benitez Date: Thu, 21 Mar 2024 20:24:53 -0700 Subject: [PATCH] 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. --- core/codegen/src/attribute/entry/launch.rs | 5 +- core/codegen/src/attribute/entry/main.rs | 7 +- core/codegen/src/attribute/entry/mod.rs | 4 + core/codegen/src/attribute/mod.rs | 1 + core/codegen/src/attribute/param/parse.rs | 7 +- core/codegen/src/attribute/route/mod.rs | 9 +- core/codegen/src/attribute/route/parse.rs | 8 +- core/codegen/src/attribute/suppress/lint.rs | 115 ++++++++++++++++++++ core/codegen/src/attribute/suppress/mod.rs | 20 ++++ core/codegen/src/http_codegen.rs | 9 +- core/codegen/src/lib.rs | 27 +++++ core/codegen/tests/route-format.rs | 8 +- 12 files changed, 201 insertions(+), 19 deletions(-) create mode 100644 core/codegen/src/attribute/suppress/lint.rs create mode 100644 core/codegen/src/attribute/suppress/mod.rs diff --git a/core/codegen/src/attribute/entry/launch.rs b/core/codegen/src/attribute/entry/launch.rs index 12289219..9d439814 100644 --- a/core/codegen/src/attribute/entry/launch.rs +++ b/core/codegen/src/attribute/entry/launch.rs @@ -3,6 +3,7 @@ use devise::ext::SpanDiagnosticExt; use proc_macro2::{TokenStream, Span}; use super::EntryAttr; +use crate::attribute::suppress::Lint; use crate::exports::mixed; /// `#[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()), }; - 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) { call.span() .warning("task is being spawned outside an async context") .span_help(f.sig.span(), "declare this function as `async fn` \ to require async execution") .span_note(Span::call_site(), "`#[launch]` call is here") + .note(lint.how_to_suppress()) .emit_as_expr_tokens(); } } diff --git a/core/codegen/src/attribute/entry/main.rs b/core/codegen/src/attribute/entry/main.rs index 66e7a1cb..88ad5d67 100644 --- a/core/codegen/src/attribute/entry/main.rs +++ b/core/codegen/src/attribute/entry/main.rs @@ -1,3 +1,5 @@ +use crate::attribute::suppress::Lint; + use super::EntryAttr; use devise::{Spanned, Result}; @@ -12,11 +14,12 @@ impl EntryAttr for Main { fn function(f: &mut syn::ItemFn) -> Result { let (attrs, vis, block, sig) = (&f.attrs, &f.vis, &f.block, &mut f.sig); - if sig.ident != "main" { - // FIXME(diag): warning! + let lint = Lint::ArbitraryMain; + if sig.ident != "main" && lint.enabled(sig.ident.span()) { Span::call_site() .warning("attribute is typically applied to `main` function") .span_note(sig.ident.span(), "this function is not `main`") + .note(lint.how_to_suppress()) .emit_as_item_tokens(); } diff --git a/core/codegen/src/attribute/entry/mod.rs b/core/codegen/src/attribute/entry/mod.rs index 9ed98bc0..7fec9121 100644 --- a/core/codegen/src/attribute/entry/mod.rs +++ b/core/codegen/src/attribute/entry/mod.rs @@ -6,6 +6,8 @@ use devise::{Diagnostic, Spanned, Result}; use devise::ext::SpanDiagnosticExt; use proc_macro2::{TokenStream, Span}; +use crate::attribute::suppress::Lint; + // Common trait implemented by `async` entry generating attributes. trait EntryAttr { /// Whether the attribute requires the attributed function to be `async`. @@ -23,6 +25,8 @@ fn _async_entry( .map_err(Diagnostic::from) .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() { return Err(Span::call_site() .error("attribute can only be applied to `async` functions") diff --git a/core/codegen/src/attribute/mod.rs b/core/codegen/src/attribute/mod.rs index c851bebc..8fbb3fa9 100644 --- a/core/codegen/src/attribute/mod.rs +++ b/core/codegen/src/attribute/mod.rs @@ -3,3 +3,4 @@ pub mod catch; pub mod route; pub mod param; pub mod async_bound; +pub mod suppress; diff --git a/core/codegen/src/attribute/param/parse.rs b/core/codegen/src/attribute/param/parse.rs index c334034e..5fc36be6 100644 --- a/core/codegen/src/attribute/param/parse.rs +++ b/core/codegen/src/attribute/param/parse.rs @@ -6,6 +6,7 @@ use crate::name::Name; use crate::proc_macro_ext::StringLit; use crate::attribute::param::{Parameter, Dynamic}; use crate::http::uri::fmt::{Part, Kind, Path}; +use crate::attribute::suppress::Lint; #[derive(Debug)] pub struct Error<'a> { @@ -47,6 +48,7 @@ impl Parameter { let mut trailing = false; // Check if this is a dynamic param. If so, check its well-formedness. + let lint = Lint::SegmentChars; if segment.starts_with('<') && segment.ends_with('>') { let mut name = &segment[1..(segment.len() - 1)]; if name.ends_with("..") { @@ -71,12 +73,13 @@ impl Parameter { } } else if segment.is_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); source_span.warning("`segment` starts with `<` but does not end with `>`") .help(format!("perhaps you meant the dynamic parameter `<{}>`?", candidate)) + .note(lint.how_to_suppress()) .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") .emit_as_item_tokens(); } diff --git a/core/codegen/src/attribute/route/mod.rs b/core/codegen/src/attribute/route/mod.rs index 6b9ffb1c..36fd9559 100644 --- a/core/codegen/src/attribute/route/mod.rs +++ b/core/codegen/src/attribute/route/mod.rs @@ -14,6 +14,8 @@ use crate::exports::mixed; use self::parse::{Route, Attribute, MethodAttribute}; +use super::suppress::Lint; + impl Route { pub fn guards(&self) -> impl Iterator { self.param_guards() @@ -399,17 +401,18 @@ fn incomplete_route( args: TokenStream, input: TokenStream ) -> Result { - let method_str = method.to_string().to_lowercase(); // 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()) .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) .map_err(Diagnostic::from) .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 method_attribute = MethodAttribute::from_meta(&syn::parse2(full_attr)?)?; diff --git a/core/codegen/src/attribute/route/parse.rs b/core/codegen/src/attribute/route/parse.rs index 3918114d..7c74637c 100644 --- a/core/codegen/src/attribute/route/parse.rs +++ b/core/codegen/src/attribute/route/parse.rs @@ -3,6 +3,7 @@ use devise::ext::{SpanDiagnosticExt, TypeExt}; use indexmap::{IndexSet, IndexMap}; use proc_macro2::Span; +use crate::attribute::suppress::Lint; use crate::proc_macro_ext::Diagnostics; use crate::http_codegen::{Method, MediaType}; 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. if let Some(ref data) = attr.data { - if !attr.method.0.supports_payload() { - let msg = format!("'{}' does not typically support payloads", attr.method.0); + let lint = Lint::DubiousPayload; + if !attr.method.0.supports_payload() && lint.enabled(handler.span()) { // FIXME(diag: warning) 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(); } } diff --git a/core/codegen/src/attribute/suppress/lint.rs b/core/codegen/src/attribute/suppress/lint.rs new file mode 100644 index 00000000..758c9c2a --- /dev/null +++ b/core/codegen/src/attribute/suppress/lint.rs @@ -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 { + $(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>>> = RefCell::default(); +} + +fn span_to_range(span: Span) -> Option> { + 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::::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 { + 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) + }) + } +} diff --git a/core/codegen/src/attribute/suppress/mod.rs b/core/codegen/src/attribute/suppress/mod.rs new file mode 100644 index 00000000..97255004 --- /dev/null +++ b/core/codegen/src/attribute/suppress/mod.rs @@ -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) + } + } +} diff --git a/core/codegen/src/http_codegen.rs b/core/codegen/src/http_codegen.rs index 35a5cb9a..896f1d6d 100644 --- a/core/codegen/src/http_codegen.rs +++ b/core/codegen/src/http_codegen.rs @@ -2,7 +2,7 @@ use quote::ToTokens; use devise::{FromMeta, MetaItem, Result, ext::{Split2, PathExt, SpanDiagnosticExt}}; use proc_macro2::{TokenStream, Span}; -use crate::http; +use crate::{http, attribute::suppress::Lint}; #[derive(Debug)] pub struct ContentType(pub http::ContentType); @@ -73,10 +73,11 @@ impl FromMeta for MediaType { let mt = http::MediaType::parse_flexible(&String::from_meta(meta)?) .ok_or_else(|| meta.value_span().error("invalid or unknown media type"))?; - if !mt.is_known() { - // FIXME(diag: warning) + let lint = Lint::UnknownFormat; + if !mt.is_known() && lint.enabled(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(); } diff --git a/core/codegen/src/lib.rs b/core/codegen/src/lib.rs index aab19f30..8dda9388 100644 --- a/core/codegen/src/lib.rs +++ b/core/codegen/src/lib.rs @@ -356,6 +356,33 @@ pub fn catch(args: TokenStream, input: TokenStream) -> TokenStream { 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. /// /// Simply decorate a test `async fn` with `#[async_test]` instead of `#[test]`: diff --git a/core/codegen/tests/route-format.rs b/core/codegen/tests/route-format.rs index be05eab3..221ce975 100644 --- a/core/codegen/tests/route-format.rs +++ b/core/codegen/tests/route-format.rs @@ -66,19 +66,19 @@ fn test_formats() { // Test custom formats. -// TODO: #[rocket(allow(unknown_format))] +#[suppress(unknown_format)] #[get("/", format = "application/foo")] fn get_foo() -> &'static str { "get_foo" } -// TODO: #[rocket(allow(unknown_format))] +#[suppress(unknown_format)] #[post("/", format = "application/foo")] fn post_foo() -> &'static str { "post_foo" } -// TODO: #[rocket(allow(unknown_format))] +#[suppress(unknown_format)] #[get("/", format = "bar/baz", rank = 2)] fn get_bar_baz() -> &'static str { "get_bar_baz" } -// TODO: #[rocket(allow(unknown_format))] +#[suppress(unknown_format)] #[put("/", format = "bar/baz")] fn put_bar_baz() -> &'static str { "put_bar_baz" }