STRLCPY/graphql-engine

use clippy settings in Cargo workspace (#441)

<!-- Thank you for submitting this PR! :) -->

## Description

Following the approach taken here:
https://github.com/hasura/ndc-postgres/pull/402

This moves the `clippy` settings into the Cargo workspace file instead
of passing them for each invocation.

We enable all pedantic settings, run `cargo clippy --fix` to auto fix a
few things, and then manually disable all other lints.

Plenty of them are worth enabling and fixing in future IMO.

---------

Co-authored-by: Samir Talwar <[email protected]>
V3_GIT_ORIGIN_REV_ID: aa0e6ccb8d72a7393e14b5c58b82077a67d9cb15

Daniel Harvey committed with hasura-bot 1 month ago

2a24b306

1 parent 209ec112

Total 23 files Show one by one

■ ■ ■ ■ ■ ■

v3/Cargo.toml

		skipped 8 lines
9	9		"crates/*"
10	10		]
11	11
	12	+	[workspace.lints.clippy]
	13	+	all = { level = "warn", priority = -1 }
	14	+	pedantic = { level = "warn", priority = -1 }
	15	+	# disable certain pedantic warnings
	16	+	doc_markdown = "allow"
	17	+	missing_errors_doc = "allow"
	18	+	missing_panics_doc = "allow"
	19	+	module_name_repetitions = "allow"
	20	+	must_use_candidate = "allow"
	21	+	wildcard_imports = "allow"
	22	+	# disable these for now, but we should probably fix them
	23	+	cast_precision_loss = "allow"
	24	+	cloned_instead_of_copied = "allow"
	25	+	default_trait_access = "allow"
	26	+	explicit_into_iter_loop = "allow"
	27	+	explicit_iter_loop = "allow"
	28	+	from_iter_instead_of_collect = "allow"
	29	+	if_not_else = "allow"
	30	+	ignored_unit_patterns = "allow"
	31	+	implicit_clone = "allow"
	32	+	implicit_hasher = "allow"
	33	+	inconsistent_struct_constructor = "allow"
	34	+	inefficient_to_string = "allow"
	35	+	inline_always = "allow"
	36	+	manual_is_variant_and = "allow"
	37	+	manual_let_else = "allow"
	38	+	manual_string_new = "allow"
	39	+	map_unwrap_or = "allow"
	40	+	match_same_arms = "allow"
	41	+	match_wildcard_for_single_variants = "allow"
	42	+	needless_pass_by_value = "allow"
	43	+	redundant_closure_for_method_calls = "allow"
	44	+	redundant_else = "allow"
	45	+	result_large_err = "allow"
	46	+	return_self_not_must_use = "allow"
	47	+	semicolon_if_nothing_returned = "allow"
	48	+	similar_names = "allow"
	49	+	single_match_else = "allow"
	50	+	struct_field_names = "allow"
	51	+	too_many_arguments = "allow"
	52	+	too_many_lines = "allow"
	53	+	trivially_copy_pass_by_ref = "allow"
	54	+	uninlined_format_args = "allow"
	55	+	unnecessary_box_returns = "allow"
	56	+	unnecessary_wraps = "allow"
	57	+	unnested_or_patterns = "allow"
	58	+	unreadable_literal = "allow"
	59	+	unused_async = "allow"
	60	+	used_underscore_binding = "allow"
	61	+
12	62		[profile.release]
13	63		debug = true
14	64
15	65		[profile.bench]
16	66		debug = true
17	67
	68	+
	69	+

■ ■ ■ ■ ■ ■

v3/crates/custom-connector/Cargo.toml

		skipped 19 lines
20	20		serde_json = "^1.0.92"
21	21		tokio = { version = "^1.26.0", features = ["macros", "parking_lot", "rt-multi-thread"] }
22	22
	23	+	[lints]
	24	+	workspace = true
	25	+

■ ■ ■ ■ ■ ■

v3/crates/custom-connector/src/collections/actors.rs

		skipped 60 lines
61	61		let names = actor_name.split(' ').collect::<Vec<_>>();
62	62		let actor_first_name = names
63	63		.first()
64		-	.map(\|str\| str.to_string())
65		-	.unwrap_or(String::new());
	64	+	.map_or(String::new(), std::string::ToString::to_string);
66	65		let actor_last_name = names.into_iter().skip(1).collect::<Vec<_>>().join(" ");
67	66
68	67		Ok((actor_first_name, actor_last_name, actor))
69	68		})
70	69		.filter_map(move \|result\| match result {
71	70		Ok((actor_first_name, actor_last_name, actor)) => {
72		-	if filter_first_name
73		-	.map(\|first_name\| first_name == actor_first_name)
74		-	.unwrap_or(true)
75		-	&& filter_last_name
76		-	.map(\|last_name\| last_name == actor_last_name)
77		-	.unwrap_or(true)
	71	+	if filter_first_name.map_or(true, \|first_name\| first_name == actor_first_name)
	72	+	&& filter_last_name.map_or(true, \|last_name\| last_name == actor_last_name)
78	73		{
79	74		Some(Ok(actor))
80	75		} else {
		skipped 7 lines

■ ■ ■ ■ ■ ■

v3/crates/custom-connector/src/functions/latest_actor_id.rs

		skipped 22 lines
23	23		let latest_id = state
24	24		.actors
25	25		.iter()
26		-	.filter_map(\|(_id, a)\| a.get("id").and_then(\|v\| v.as_i64()))
	26	+	.filter_map(\|(_id, a)\| a.get("id").and_then(serde_json::Value::as_i64))
27	27		.max();
28	28		let latest_id_value = serde_json::to_value(latest_id).map_err(\|_\| {
29	29		(
		skipped 13 lines

■ ■ ■ ■ ■ ■

v3/crates/custom-connector/src/main.rs

		skipped 44 lines
45	45		// block until either of the above happens
46	46		#[cfg(unix)]
47	47		tokio::select! {
48		-	_ = sigint => (),
	48	+	() = sigint => (),
49	49		_ = sigterm => (),
50	50		}
51	51		#[cfg(windows)]
		skipped 46 lines

■ ■ ■ ■ ■ ■

v3/crates/custom-connector/src/query.rs

		skipped 725 lines
726	726		root,
727	727		item,
728	728		)?;
729		-	Ok(vals.iter().any(\|val\| val.is_null()))
	729	+	Ok(vals.iter().any(serde_json::Value::is_null))
730	730		}
731	731		},
732	732
		skipped 396 lines

■ ■ ■ ■ ■ ■

v3/crates/engine/Cargo.toml

skipped 77 lines
78 78 "strum", # used by strum_macros
79 79 ]
80 80
81 + [lints]
82 + workspace = true
83 +

All occurrences

■ ■ ■ ■ ■ ■

v3/crates/engine/bin/engine/main.rs

		skipped 31 lines
32	32
33	33		const DEFAULT_PORT: u16 = 3000;
34	34
	35	+	const MB: usize = 1_048_576;
	36	+
35	37		#[derive(Parser)]
36	38		#[command(version = VERSION)]
37	39		struct ServerOptions {
		skipped 66 lines
104	106		let terminate = std::future::pending::<()>();
105	107
106	108		tokio::select! {
107		-	_ = ctrl_c => {},
108		-	_ = terminate => {},
	109	+	() = ctrl_c => {},
	110	+	() = terminate => {},
109	111		}
110	112		}
111	113
		skipped 62 lines
174	176		.with_state(state.clone());
175	177
176	178		let health_route = Router::new().route("/health", get(handle_health));
177		-
178		-	const MB: usize = 1_048_576;
179	179
180	180		let app = Router::new()
181	181		// serve graphiql at root
		skipped 237 lines

■ ■ ■ ■ ■ ■

v3/crates/hasura-authn-core/Cargo.toml

skipped 19 lines
20 20 [dev-dependencies]
21 21 pretty_assertions = "1.3.0"
22 22
23 + [lints]
24 + workspace = true
25 +

All occurrences
■ ■ ■ ■ ■ ■

v3/crates/hasura-authn-jwt/Cargo.toml

skipped 31 lines
32 32 openssl = "0.10.64"
33 33 tokio = { version = "1.29.0", features = ["macros"] }
34 34
35 + [lints]
36 + workspace = true
37 +

All occurrences
■ ■ ■ ■ ■ ■

v3/crates/hasura-authn-webhook/Cargo.toml

skipped 25 lines
26 26 rand = "0.8.5"
27 27 tokio = "1.29.0"
28 28
29 + [lints]
30 + workspace = true
31 +

All occurrences

■ ■ ■ ■ ■ ■

v3/crates/hasura-authn-webhook/src/webhook.rs

		skipped 97 lines
98	98		"Connection",
99	99		"DNT",
100	100		"Content-Type",
101		-	].map(\|s\| s.to_lowercase()));
	101	+	].map(str::to_lowercase));
102	102		}
103	103
104	104		fn serialize_url<S>(url: &Url, s: S) -> Result<S::Ok, S::Error>
		skipped 641 lines

■ ■ ■ ■ ■ ■

v3/crates/lang-graphql/Cargo.toml

skipped 51 lines
52 52 pretty_assertions = "1.3.0"
53 53 rmp-serde = "1.1.1"
54 54
55 + [lints]
56 + workspace = true
57 +

All occurrences

■ ■ ■ ■ ■ ■

v3/crates/lang-graphql/src/lexer.rs

		skipped 494 lines
495	495		let foo_name = mk_name!("foo");
496	496		assert_eq!(
497	497		Lexer::new(
498		-	r#"
	498	+	r"
499	499
500	500		foo
501	501
502		-	"#
	502	+	"
503	503		)
504	504		.read_next_token(),
505	505		Some(Ok(spanned_token(
		skipped 6 lines
512	512		);
513	513		assert_eq!(
514	514		Lexer::new(
515		-	r#"
	515	+	r"
516	516		#comment
517	517		foo#comment
518		-	"#
	518	+	"
519	519		)
520	520		.read_next_token(),
521	521		Some(Ok(spanned_token(
		skipped 152 lines

■ ■ ■ ■ ■ ■

v3/crates/open-dds/Cargo.toml

skipped 30 lines
31 31 "strum", # used by strum_macros
32 32 ]
33 33
34 + [lints]
35 + workspace = true
36 +

All occurrences

■ ■ ■ ■ ■ ■

v3/crates/open-dds/src/permissions.rs

		skipped 297 lines
298	298		impl NullableModelPredicate {
299	299		pub fn as_option_ref(&self) -> Option<&ModelPredicate> {
300	300		match self {
301		-	NullableModelPredicate::Null(_) => None,
	301	+	NullableModelPredicate::Null(()) => None,
302	302		NullableModelPredicate::NotNull(p) => Some(p),
303	303		}
304	304		}
		skipped 189 lines

■ ■ ■ ■ ■ ■

v3/crates/open-dds/src/types.rs

		skipped 131 lines
132	132		}
133	133		}
134	134
135		-	const TYPE_REFERENCE_DESCRIPTION: &str = r#"A reference to an Open DD type including nullable values and arrays.
	135	+	const TYPE_REFERENCE_DESCRIPTION: &str = r"A reference to an Open DD type including nullable values and arrays.
136	136		Suffix '!' to indicate a non-nullable reference, and wrap in '[]' to indicate an array.
137		-	Eg: '[String!]!' is a non-nullable array of non-nullable strings."#;
	137	+	Eg: '[String!]!' is a non-nullable array of non-nullable strings.";
138	138
139	139		impl JsonSchema for TypeReference {
140	140		fn schema_name() -> String {
		skipped 550 lines

■ ■ ■ ■ ■ ■

v3/crates/opendds-derive/Cargo.toml

skipped 13 lines
14 14 quote = "1.0"
15 15 syn = { version = "1.0", features = ["full"] }
16 16
17 + [lints]
18 + workspace = true
19 +

All occurrences

■ ■ ■ ■ ■ ■

v3/crates/opendds-derive/src/container.rs

		skipped 162 lines
163	163		let field_name = field.ident.as_ref().unwrap();
164	164		let renamed_field = field_opts
165	165		.rename
166		-	.as_ref()
167		-	.cloned()
	166	+	.clone()
168	167		.unwrap_or_else(\|\| field_name.to_string().to_case(Case::Camel));
169	168		let is_default = field_opts.default.unwrap_or(false);
170	169		let is_optional = is_option_type(&field.ty);
		skipped 123 lines

■ ■ ■ ■ ■ ■

v3/crates/opendds-derive/src/struct_derive.rs

1		-	use quote::{quote, ToTokens};
	1	+	use quote::quote;
2	2
3	3		use crate::container::*;
4	4		use crate::helpers;
		skipped 123 lines
128	128		let default_exp = field
129	129		.default_exp
130	130		.as_ref()
131		-	.map(\|exp\| exp.to_token_stream())
	131	+	.map(quote::ToTokens::to_token_stream)
132	132		.unwrap_or_else(\|\| {
133	133		quote! {
134	134		serde_json::json!(<#ty as Default>::default())
		skipped 70 lines

■ ■ ■ ■ ■ ■

v3/crates/recursion_limit_macro/Cargo.toml

skipped 9 lines
10 10 syn = { version = "1.0", features = ["full"] }
11 11 quote = "1.0"
12 12
13 + [lints]
14 + workspace = true
15 +

All occurrences
■ ■ ■ ■ ■ ■

v3/crates/tracing-util/Cargo.toml

skipped 24 lines
25 25 "axum", # used in doc examples
26 26 ]
27 27
28 + [lints]
29 + workspace = true
30 +

All occurrences

■ ■ ■ ■ ■ ■

v3/justfile

1	1		set positional-arguments
2	2
3		-	CLIPPY_ARGS := "-A clippy::result_large_err -D clippy::complexity -A clippy::too_many_arguments -D clippy::style"
4		-	CLIPPY_FIX_ARGS := "-A clippy::result_large_err -A clippy::too_many_arguments"
5		-
6	3		# This produces "-p engine -p lang-graphql -p ...".
7	4		# See https://github.com/rust-lang/cargo/issues/4463 for why we build each
8	5		# package explicitly instead of `cargo build --workspace --all-targets`.
		skipped 30 lines
39	36		# linting run by CI
40	37		ci-lint:
41	38		just docker_with_source_only sh -c \
42		-	"cargo clippy --no-deps -- {{ CLIPPY_ARGS }}"
	39	+	"RUSTFLAGS='-D warnings' cargo clippy --no-deps"
43	40
44	41		fix:
45		-	just docker_with_source_only sh -c "cargo clippy --no-deps --fix --allow-no-vcs -- {{ CLIPPY_FIX_ARGS }}; cargo fmt"
	42	+	just docker_with_source_only sh -c "cargo clippy --no-deps --fix --allow-no-vcs; cargo fmt"
46	43
47	44		format:
48	45		cargo fmt --check
49	46		alias fmt := format
50	47
51	48		fix-local:
52		-	cargo clippy --no-deps --fix --allow-no-vcs -- {{ CLIPPY_FIX_ARGS }}; cargo fmt
	49	+	cargo clippy --no-deps --fix --allow-no-vcs
	50	+	cargo fmt
53	51
54	52		# tests run by CI
55	53		ci-test: docker-refresh
		skipped 75 lines
131	129		RUST_LOG=DEBUG \
132	130		cargo watch \
133	131		-x test \
134		-	-x 'clippy --no-deps -- {{ CLIPPY_ARGS }}' \
	132	+	-x 'clippy --no-deps' \
135	133		-x 'run --bin engine -- \
136	134		--otlp-endpoint http://localhost:4317 \
137	135		--authn-config-path auth_config.json \
		skipped 1 lines
139	137
140	138		# check the code is fine
141	139		lint-local:
142		-	cargo clippy --no-deps -- {{ CLIPPY_ARGS }}
	140	+	cargo clippy --no-deps
143	141
144	142		# ensure we don't have unused dependencies:
145	143		machete:
		skipped 13 lines

		skipped 77 lines
78	78		"strum", # used by strum_macros
79	79		]
80	80
	81	+	[lints]
	82	+	workspace = true
	83	+

		skipped 19 lines
20	20		[dev-dependencies]
21	21		pretty_assertions = "1.3.0"
22	22
	23	+	[lints]
	24	+	workspace = true
	25	+

		skipped 31 lines
32	32		openssl = "0.10.64"
33	33		tokio = { version = "1.29.0", features = ["macros"] }
34	34
	35	+	[lints]
	36	+	workspace = true
	37	+

		skipped 25 lines
26	26		rand = "0.8.5"
27	27		tokio = "1.29.0"
28	28
	29	+	[lints]
	30	+	workspace = true
	31	+

		skipped 51 lines
52	52		pretty_assertions = "1.3.0"
53	53		rmp-serde = "1.1.1"
54	54
	55	+	[lints]
	56	+	workspace = true
	57	+

		skipped 13 lines
14	14		quote = "1.0"
15	15		syn = { version = "1.0", features = ["full"] }
16	16
	17	+	[lints]
	18	+	workspace = true
	19	+

		skipped 9 lines
10	10		syn = { version = "1.0", features = ["full"] }
11	11		quote = "1.0"
12	12
	13	+	[lints]
	14	+	workspace = true
	15	+

		skipped 24 lines
25	25		"axum", # used in doc examples
26	26		]
27	27
	28	+	[lints]
	29	+	workspace = true
	30	+