Skip to content

Commit

Permalink
Deny lints pre-ArgAbi codegen used to trigger (#1744)
Browse files Browse the repository at this point in the history
Lifts instances of crate-wide lint control into Cargo.toml and denies
some of them for pgrx-tests so that pgrx's macro expansions no longer
emit needless `allow`s in quite as many places. This also trims away a
few now-needless allowances.
  • Loading branch information
workingjubilee authored Jun 29, 2024
1 parent b0b6ca6 commit 43e68d2
Show file tree
Hide file tree
Showing 13 changed files with 30 additions and 46 deletions.
3 changes: 3 additions & 0 deletions cargo-pgrx/Cargo.toml
Original file line number Diff line number Diff line change
Expand Up @@ -70,3 +70,6 @@ rustls = [
"ureq/tls",
"ureq/native-certs" # induces rustls to use the OS-level root of trust
]

[lints.clippy]
or-fun-call = "allow" # kinda sus lint imo
3 changes: 0 additions & 3 deletions cargo-pgrx/src/main.rs
Original file line number Diff line number Diff line change
Expand Up @@ -7,9 +7,6 @@
//LICENSE All rights reserved.
//LICENSE
//LICENSE Use of this source code is governed by the MIT license that can be found in the LICENSE file.
#![deny(clippy::perf)] // our compile times are awful
#![allow(clippy::or_fun_call)] // often false positives

mod command;
mod manifest;
mod metadata;
Expand Down
11 changes: 1 addition & 10 deletions pgrx-macros/src/rewriter.rs
Original file line number Diff line number Diff line change
Expand Up @@ -36,7 +36,7 @@ pub fn item_fn_without_rewrite(mut func: ItemFn) -> syn::Result<proc_macro2::Tok
let input_func_name = func.sig.ident.to_string();
let sig = func.sig.clone();
let vis = func.vis.clone();
let mut attrs = mem::take(&mut func.attrs);
let attrs = mem::take(&mut func.attrs);
let generics = func.sig.generics.clone();

if attrs.iter().any(|attr| attr.path().is_ident("no_mangle"))
Expand All @@ -58,14 +58,6 @@ pub fn item_fn_without_rewrite(mut func: ItemFn) -> syn::Result<proc_macro2::Tok

func.sig.ident = format_ident!("{}_inner", func.sig.ident);

// the wrapper_inner function declaration may contain lifetimes that are not used, since our input type is `FunctionCallInfo` mainly and return type is `Datum`
let unused_lifetimes = match generics.lifetimes().next() {
Some(_) => quote! {
#[allow(unused_lifetimes, clippy::extra_unused_lifetimes)]
},
None => quote! {},
};

let arg_list = build_arg_list(&sig, false)?;
let func_name = func.sig.ident.clone();

Expand Down Expand Up @@ -103,7 +95,6 @@ pub fn item_fn_without_rewrite(mut func: ItemFn) -> syn::Result<proc_macro2::Tok
#(#attrs)*
#vis #sig {
#[allow(non_snake_case)]
#unused_lifetimes
#func

#[allow(unused_unsafe)]
Expand Down
10 changes: 10 additions & 0 deletions pgrx-pg-sys/Cargo.toml
Original file line number Diff line number Diff line change
Expand Up @@ -63,3 +63,13 @@ bindgen = { version = "0.69", default-features = false, features = ["runtime"] }
clang-sys = { version = "1", features = ["clang_6_0", "runtime"] }
quote = "1.0.33"
shlex = "1.3" # shell lexing, also used by many of our deps

[lints]
# we allow improper_ctypes just to eliminate these warnings:
# = note: `#[warn(improper_ctypes)]` on by default
# = note: 128-bit integers don't currently have a known stable ABI
rust.non_camel_case_types = "allow"
rust.non_snake_case = "allow"
rust.dead_code = "allow"
rust.non_upper_case_globals = "allow"
rust.improper_ctypes = "allow"
11 changes: 0 additions & 11 deletions pgrx-pg-sys/src/lib.rs
Original file line number Diff line number Diff line change
Expand Up @@ -7,17 +7,6 @@
//LICENSE All rights reserved.
//LICENSE
//LICENSE Use of this source code is governed by the MIT license that can be found in the LICENSE file.
//
// we allow improper_ctypes just to eliminate these warnings:
// = note: `#[warn(improper_ctypes)]` on by default
// = note: 128-bit integers don't currently have a known stable ABI
#![allow(non_camel_case_types)]
#![allow(non_snake_case)]
#![allow(dead_code)]
#![allow(non_upper_case_globals)]
#![allow(improper_ctypes)]
#![allow(clippy::unneeded_field_pattern)]

#[cfg(
// no features at all will cause problems
not(any(feature = "pg12", feature = "pg13", feature = "pg14", feature = "pg15", feature = "pg16")),
Expand Down
1 change: 1 addition & 0 deletions pgrx-sql-entity-graph/Cargo.toml
Original file line number Diff line number Diff line change
Expand Up @@ -42,3 +42,4 @@ syntect = { version = "5.1.0", default-features = false, features = ["default-fa

[lints.clippy]
assigning-clones = "allow" # wrong diagnosis and wrong suggestions
too-many-arguments = "allow" # I argue with myself all the time
12 changes: 1 addition & 11 deletions pgrx-sql-entity-graph/src/finfo.rs
Original file line number Diff line number Diff line change
Expand Up @@ -26,21 +26,11 @@ pub fn finfo_v1_extern_c(
) -> syn::Result<ItemFn> {
let original_name = &original.sig.ident;
let wrapper_symbol = format_ident!("{}_wrapper", original_name);
let lifetimes = &original.sig.generics;
// the wrapper function declaration may contain lifetimes that are not used, since
// our input type is FunctionCallInfo and our return type is Datum
let unused_lifetimes = match lifetimes.lifetimes().next() {
Some(_) => quote! {
#[allow(unused_lifetimes, clippy::extra_unused_lifetimes)]
},
None => quote! {},
};

let tokens = quote_spanned! { original.sig.span() =>
#[no_mangle]
#[doc(hidden)]
#unused_lifetimes
pub unsafe extern "C" fn #wrapper_symbol #lifetimes(#fcinfo: ::pgrx::pg_sys::FunctionCallInfo) -> ::pgrx::pg_sys::Datum {
pub unsafe extern "C" fn #wrapper_symbol(#fcinfo: ::pgrx::pg_sys::FunctionCallInfo) -> ::pgrx::pg_sys::Datum {
#contents
}
};
Expand Down
2 changes: 0 additions & 2 deletions pgrx-sql-entity-graph/src/lib.rs
Original file line number Diff line number Diff line change
Expand Up @@ -15,8 +15,6 @@ Rust to SQL mapping support.
to the `pgrx` framework and very subject to change between versions. While you may use this, please do it with caution.
*/
#![allow(clippy::too_many_arguments)]
#![allow(clippy::redundant_pattern_matching)]
pub use aggregate::entity::{AggregateTypeEntity, PgAggregateEntity};
pub use aggregate::{
AggregateType, AggregateTypeList, FinalizeModify, ParallelOption, PgAggregate,
Expand Down
2 changes: 1 addition & 1 deletion pgrx-sql-entity-graph/src/pgrx_attribute.rs
Original file line number Diff line number Diff line change
Expand Up @@ -65,7 +65,7 @@ impl Parse for PgrxArg {
#[track_caller]
fn parse(input: ParseStream<'_>) -> syn::Result<Self> {
let path = input.parse::<syn::Path>()?;
if let Ok(_) = input.parse::<Token![=]>() {
if input.parse::<Token![=]>().is_ok() {
Ok(Self::NameValue(NameValueArg { path, value: input.parse()? }))
} else {
Err(input.error("unsupported argument to #[pgrx] in this context"))
Expand Down
4 changes: 4 additions & 0 deletions pgrx-tests/Cargo.toml
Original file line number Diff line number Diff line change
Expand Up @@ -76,3 +76,7 @@ version = "=0.12.0-alpha.1"
[dev-dependencies]
eyre.workspace = true # testing functions that return `eyre::Result`
trybuild = "1"

[lints]
rust.unused-lifetimes = "deny"
clippy.used-underscore-binding = "deny"
1 change: 1 addition & 0 deletions pgrx-tests/src/tests/pg_guard_tests.rs
Original file line number Diff line number Diff line change
Expand Up @@ -34,6 +34,7 @@ extern "C" fn extern_func_impl_1() -> bool {
// and [no_mangle]
#[pg_guard]
#[no_mangle]
#[allow(unused_lifetimes)]
extern "C" fn extern_func_impl_2<'a>() -> bool {
true
}
Expand Down
8 changes: 8 additions & 0 deletions pgrx/Cargo.toml
Original file line number Diff line number Diff line change
Expand Up @@ -66,3 +66,11 @@ seahash = "4.1.0" # derive(PostgresHash)
serde.workspace = true # impls on pub types
serde_cbor = "0.11.2" # derive(PostgresType)
serde_json.workspace = true # everything JSON

[lints]
clippy.cast_ptr_alignment = "allow"
clippy.len_without_is_empty = "allow"
clippy.missing_safety_doc = "allow"
clippy.too_many_arguments = "allow"
clippy.type_complexity = "allow"
clippy.unnecessary_cast = "allow"
8 changes: 0 additions & 8 deletions pgrx/src/lib.rs
Original file line number Diff line number Diff line change
Expand Up @@ -21,14 +21,6 @@
//! }
//!
//! ```
#![allow(
clippy::cast_ptr_alignment,
clippy::len_without_is_empty,
clippy::missing_safety_doc,
clippy::too_many_arguments,
clippy::type_complexity,
clippy::unnecessary_cast
)]
#![cfg_attr(feature = "nightly", feature(allocator_api))]

#[macro_use]
Expand Down

0 comments on commit 43e68d2

Please sign in to comment.