Skip to content

Commit f776ea3

Browse files
committed
feat: implement warnings on NeverClones that implement Copy
1 parent ba4157a commit f776ea3

File tree

2 files changed

+102
-1
lines changed

2 files changed

+102
-1
lines changed

libs/pavexc/src/compiler/analyses/cloning.rs

Lines changed: 92 additions & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -106,3 +106,95 @@ fn must_be_clonable(
106106
.build();
107107
diagnostics.push(diagnostic.into());
108108
}
109+
110+
/// Check all types whose cloning strategy is set to "NeverClone" are not Copy.
111+
#[tracing::instrument(
112+
name = "If cloning is not allowed, types should not be copyable",
113+
skip_all
114+
)]
115+
pub(crate) fn never_clones_should_not_be_copyable<'a>(
116+
component_db: &ComponentDb,
117+
computation_db: &ComputationDb,
118+
package_graph: &PackageGraph,
119+
krate_collection: &CrateCollection,
120+
diagnostics: &mut Vec<miette::Error>,
121+
) {
122+
let copy = process_framework_path("core::marker::Copy", krate_collection);
123+
let ResolvedType::ResolvedPath(copy) = copy else {
124+
unreachable!()
125+
};
126+
127+
for (id, _) in component_db.iter() {
128+
let HydratedComponent::Constructor(constructor) =
129+
component_db.hydrated_component(id, computation_db)
130+
else {
131+
continue;
132+
};
133+
if component_db.cloning_strategy(id) != CloningStrategy::NeverClone {
134+
continue;
135+
}
136+
let output_type = constructor.output_type();
137+
if let Ok(()) = assert_trait_is_implemented(krate_collection, output_type, &copy) {
138+
should_not_be_never_clone(
139+
output_type,
140+
id,
141+
package_graph,
142+
component_db,
143+
computation_db,
144+
diagnostics,
145+
);
146+
}
147+
}
148+
}
149+
150+
fn should_not_be_never_clone(
151+
type_: &ResolvedType,
152+
component_id: ComponentId,
153+
package_graph: &PackageGraph,
154+
component_db: &ComponentDb,
155+
computation_db: &ComputationDb,
156+
diagnostics: &mut Vec<miette::Error>,
157+
) {
158+
let component_id = component_db
159+
.derived_from(&component_id)
160+
.unwrap_or(component_id);
161+
let user_component_id = component_db.user_component_id(component_id).unwrap();
162+
let user_component_db = &component_db.user_component_db();
163+
let callable_type = user_component_db[user_component_id].callable_type();
164+
let location = user_component_db.get_location(user_component_id);
165+
let source = try_source!(location, package_graph, diagnostics);
166+
let label = source.as_ref().and_then(|source| {
167+
diagnostic::get_f_macro_invocation_span(source, location)
168+
.labeled(format!("The {callable_type} was registered here"))
169+
});
170+
let warning_msg = match callable_type {
171+
CallableType::Constructor => {
172+
let callable_path = &computation_db[user_component_id].path;
173+
format!(
174+
"A type should not be copyable if you set its cloning strategy to `NeverClone`.\n\
175+
The cloning strategy for `{callable_path}` is `NeverClone`, but `{}`, its output type, implements the `Copy` trait.",
176+
type_.display_for_error(),
177+
)
178+
}
179+
CallableType::PrebuiltType => {
180+
format!(
181+
"A type should not be copyable if you set its cloning strategy to `NeverClone`.\n\
182+
The cloning strategy for `{}`, a prebuilt type, is `NeverClone`, but it implements the `Copy` trait.",
183+
type_.display_for_error(),
184+
)
185+
}
186+
_ => unreachable!(),
187+
};
188+
let e = anyhow::anyhow!("copyable type is marked NeverClone").context(warning_msg);
189+
let help = format!(
190+
"Either set the cloning strategy to `CloneIfNecessary` or remove `Copy` for `{}`",
191+
type_.display_for_error()
192+
);
193+
let diagnostic = CompilerDiagnostic::builder(e)
194+
.severity(miette::Severity::Warning)
195+
.optional_source(source)
196+
.optional_label(label)
197+
.help(help)
198+
.build();
199+
diagnostics.push(diagnostic.into());
200+
}

libs/pavexc/src/compiler/app.rs

Lines changed: 10 additions & 1 deletion
Original file line numberDiff line numberDiff line change
@@ -13,7 +13,9 @@ use crate::compiler::analyses::application_state::ApplicationState;
1313
use crate::compiler::analyses::call_graph::{
1414
application_state_call_graph, ApplicationStateCallGraph,
1515
};
16-
use crate::compiler::analyses::cloning::clonables_can_be_cloned;
16+
use crate::compiler::analyses::cloning::{
17+
clonables_can_be_cloned, never_clones_should_not_be_copyable,
18+
};
1719
use crate::compiler::analyses::components::{ComponentDb, ComponentId};
1820
use crate::compiler::analyses::computations::ComputationDb;
1921
use crate::compiler::analyses::constructibles::ConstructibleDb;
@@ -123,6 +125,13 @@ impl App {
123125
&krate_collection,
124126
&mut diagnostics,
125127
);
128+
never_clones_should_not_be_copyable(
129+
&component_db,
130+
&computation_db,
131+
&package_graph,
132+
&krate_collection,
133+
&mut diagnostics,
134+
);
126135
exit_on_errors!(diagnostics);
127136
let handler_id2pipeline = {
128137
let handler_ids: BTreeSet<_> = router.handler_ids();

0 commit comments

Comments
 (0)