Skip to content

Commit fb0aedf

Browse files
committed
fix: refuse type parameters a resource reaches through a proto record
Before planning, a proto-backed record is named by its proto type, so the walk for a resource's type parameters stopped there. nexgen then wrote generic models for a resource class that isn't generic, and the code didn't compile. The checks of the resource and of the operations that its methods bind both walk through protos.
1 parent bca32e1 commit fb0aedf

7 files changed

Lines changed: 140 additions & 30 deletions

File tree

Lines changed: 6 additions & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -0,0 +1,6 @@
1+
inputs = ["input.wit", "../../../inputs/deps"]
2+
descriptors = "temporal"
3+
4+
[error]
5+
kind = "invalid-wit"
6+
contains = ["resource-bound operation `UpdateProvider` cannot use generic type parameters"]
Lines changed: 28 additions & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -0,0 +1,28 @@
1+
package temporal:nexus@1.0.0;
2+
3+
world system { export compute-service; }
4+
5+
// Only the operation that the method binds reaches the type parameter, through a proto record.
6+
/// @nexus.endpoint "compute"
7+
interface compute-service {
8+
use nexus:temporal-types/model@1.0.0.{placeholder};
9+
10+
/// @nexus.type-parameter
11+
type output-t = placeholder;
12+
13+
/// @nexus.proto "temporal.api.compute.v1.ComputeProvider"
14+
record provider {
15+
%type: string,
16+
details: option<output-t>,
17+
nexus-endpoint: string,
18+
}
19+
20+
resource holder {
21+
constructor(%type: string, nexus-endpoint: string);
22+
23+
/// @nexus.operation name="UpdateProvider"
24+
update: func();
25+
}
26+
27+
update-provider: func(request: provider);
28+
}
Lines changed: 6 additions & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -0,0 +1,6 @@
1+
inputs = ["input.wit", "../../../inputs/deps"]
2+
descriptors = "temporal"
3+
4+
[error]
5+
kind = "invalid-wit"
6+
contains = ["resource `holder`", "cannot contain generic type parameters"]
Lines changed: 27 additions & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -0,0 +1,27 @@
1+
package temporal:nexus@1.0.0;
2+
3+
world system { export compute-service; }
4+
5+
// The resource reaches the type parameter only through a proto-backed record.
6+
/// @nexus.endpoint "compute"
7+
interface compute-service {
8+
use nexus:temporal-types/model@1.0.0.{placeholder};
9+
10+
/// @nexus.type-parameter
11+
type output-t = placeholder;
12+
13+
/// @nexus.proto "temporal.api.compute.v1.ComputeProvider"
14+
record provider {
15+
/// @nexus.omit
16+
%type: placeholder,
17+
details: output-t,
18+
/// @nexus.omit
19+
nexus-endpoint: placeholder,
20+
}
21+
22+
resource holder {
23+
constructor(id: string);
24+
25+
update: func(provider: provider);
26+
}
27+
}
Lines changed: 1 addition & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -0,0 +1 @@
1+
A resource that reaches a type parameter through a proto-backed record is refused, as one that reaches it through a plain record is.

‎src/planning/selected_validation.rs‎

Lines changed: 9 additions & 11 deletions
Original file line numberDiff line numberDiff line change
@@ -106,18 +106,20 @@ fn validate_generic_model_semantics(spec: &ApiSpec<SelectedFamily>, path: &Path)
106106
}
107107
}
108108
for resource in &service.resources {
109+
let is_generic = |value| !spec.type_parameters_through_protos(value).is_empty();
109110
let resource_is_generic = resource
110111
.fields
111112
.iter()
112-
.any(|field| !spec.type_parameters(&field.field_type).is_empty())
113+
.any(|field| is_generic(&field.field_type))
113114
|| resource.methods.iter().any(|method| {
114115
method
115116
.params
116117
.iter()
117-
.any(|param| !spec.type_parameters(&param.field_type).is_empty())
118-
|| method.result.as_ref().is_some_and(|result| {
119-
!spec.type_parameters(&result.result_type).is_empty()
120-
})
118+
.any(|param| is_generic(&param.field_type))
119+
|| method
120+
.result
121+
.as_ref()
122+
.is_some_and(|result| is_generic(&result.result_type))
121123
});
122124
if resource_is_generic {
123125
return Err(invalid(format!(
@@ -133,12 +135,8 @@ fn validate_generic_model_semantics(spec: &ApiSpec<SelectedFamily>, path: &Path)
133135
let Some(operation) = service.operation(operation_name) else {
134136
continue;
135137
};
136-
let operation_is_generic = operation
137-
.input_type()
138-
.is_some_and(|input| !spec.type_parameters(input).is_empty())
139-
|| operation
140-
.output_type()
141-
.is_some_and(|output| !spec.type_parameters(output).is_empty());
138+
let operation_is_generic = operation.input_type().is_some_and(is_generic)
139+
|| operation.output_type().is_some_and(is_generic);
142140
if operation_is_generic {
143141
return Err(invalid(format!(
144142
"resource-bound operation `{operation_name}` cannot use generic type parameters"

‎src/spec.rs‎

Lines changed: 63 additions & 19 deletions
Original file line numberDiff line numberDiff line change
@@ -1712,7 +1712,7 @@ where
17121712
pub(crate) fn record_type_parameters(&self, record_name: &str) -> Vec<TypeParameterUsage> {
17131713
let mut parameters = IndexMap::<String, TypeParameterUsage>::new();
17141714
let mut visiting = std::collections::BTreeSet::new();
1715-
self.collect_record_type_parameters(record_name, &mut visiting, &mut parameters);
1715+
self.collect_record_type_parameters(record_name, &mut visiting, &mut parameters, &|_| None);
17161716
parameters.into_values().collect()
17171717
}
17181718

@@ -1721,14 +1721,16 @@ where
17211721
pub(crate) fn variant_type_parameters(&self, variant_name: &str) -> Vec<TypeParameterUsage> {
17221722
let mut parameters = IndexMap::<String, TypeParameterUsage>::new();
17231723
let mut visiting = std::collections::BTreeSet::new();
1724-
self.collect_variant_type_parameters(variant_name, &mut visiting, &mut parameters);
1724+
self.collect_variant_type_parameters(variant_name, &mut visiting, &mut parameters, &|_| {
1725+
None
1726+
});
17251727
parameters.into_values().collect()
17261728
}
17271729

17281730
pub(crate) fn type_parameters(&self, value: &TypeSpec<F>) -> Vec<TypeParameterUsage> {
17291731
let mut parameters = IndexMap::<String, TypeParameterUsage>::new();
17301732
let mut visiting = std::collections::BTreeSet::new();
1731-
self.collect_type_parameters(value, &mut visiting, &mut parameters);
1733+
self.collect_type_parameters(value, &mut visiting, &mut parameters, &|_| None);
17321734
parameters.into_values().collect()
17331735
}
17341736

@@ -1737,6 +1739,7 @@ where
17371739
record_name: &str,
17381740
visiting: &mut std::collections::BTreeSet<String>,
17391741
parameters: &mut IndexMap<String, TypeParameterUsage>,
1742+
proto_record: &dyn Fn(&F::Proto) -> Option<String>,
17401743
) {
17411744
if !visiting.insert(record_name.to_string()) {
17421745
return;
@@ -1751,7 +1754,7 @@ where
17511754
{
17521755
continue;
17531756
}
1754-
self.collect_type_parameters(&field.field_type, visiting, parameters);
1757+
self.collect_type_parameters(&field.field_type, visiting, parameters, proto_record);
17551758
}
17561759
}
17571760
visiting.remove(record_name);
@@ -1762,14 +1765,15 @@ where
17621765
variant_name: &str,
17631766
visiting: &mut std::collections::BTreeSet<String>,
17641767
parameters: &mut IndexMap<String, TypeParameterUsage>,
1768+
proto_record: &dyn Fn(&F::Proto) -> Option<String>,
17651769
) {
17661770
if !visiting.insert(variant_name.to_string()) {
17671771
return;
17681772
}
17691773
if let Some(variant) = self.variant(variant_name) {
17701774
for case in &variant.cases {
17711775
if let Some(payload) = &case.payload {
1772-
self.collect_type_parameters(payload, visiting, parameters);
1776+
self.collect_type_parameters(payload, visiting, parameters, proto_record);
17731777
}
17741778
}
17751779
}
@@ -1781,6 +1785,7 @@ where
17811785
value: &TypeSpec<F>,
17821786
visiting: &mut std::collections::BTreeSet<String>,
17831787
parameters: &mut IndexMap<String, TypeParameterUsage>,
1788+
proto_record: &dyn Fn(&F::Proto) -> Option<String>,
17841789
) {
17851790
match value {
17861791
TypeSpec::TypeParameter(parameter) => {
@@ -1794,33 +1799,49 @@ where
17941799
}
17951800
}
17961801
TypeSpec::Option(inner) | TypeSpec::List(inner) => {
1797-
self.collect_type_parameters(inner, visiting, parameters)
1802+
self.collect_type_parameters(inner, visiting, parameters, proto_record)
17981803
}
17991804
TypeSpec::Tuple(items) => {
18001805
for item in items {
1801-
self.collect_type_parameters(item, visiting, parameters);
1806+
self.collect_type_parameters(item, visiting, parameters, proto_record);
18021807
}
18031808
}
18041809
TypeSpec::Map(key, value) => {
1805-
self.collect_type_parameters(key, visiting, parameters);
1806-
self.collect_type_parameters(value, visiting, parameters);
1810+
self.collect_type_parameters(key, visiting, parameters, proto_record);
1811+
self.collect_type_parameters(value, visiting, parameters, proto_record);
18071812
}
18081813
TypeSpec::Result { ok, err } => {
18091814
if let Some(ok) = ok {
1810-
self.collect_type_parameters(ok, visiting, parameters);
1815+
self.collect_type_parameters(ok, visiting, parameters, proto_record);
18111816
}
18121817
if let Some(err) = err {
1813-
self.collect_type_parameters(err, visiting, parameters);
1818+
self.collect_type_parameters(err, visiting, parameters, proto_record);
18141819
}
18151820
}
1816-
TypeSpec::Record(record) => {
1817-
self.collect_record_type_parameters(record.as_ref(), visiting, parameters)
1818-
}
1819-
TypeSpec::Variant(variant) => {
1820-
self.collect_variant_type_parameters(variant.as_ref(), visiting, parameters)
1821-
}
1821+
TypeSpec::Record(record) => self.collect_record_type_parameters(
1822+
record.as_ref(),
1823+
visiting,
1824+
parameters,
1825+
proto_record,
1826+
),
1827+
TypeSpec::Variant(variant) => self.collect_variant_type_parameters(
1828+
variant.as_ref(),
1829+
visiting,
1830+
parameters,
1831+
proto_record,
1832+
),
18221833
TypeSpec::External(ExternalTypeSpec::Alias(alias)) => {
1823-
self.collect_type_parameters(&alias.target, visiting, parameters)
1834+
self.collect_type_parameters(&alias.target, visiting, parameters, proto_record)
1835+
}
1836+
TypeSpec::External(ExternalTypeSpec::Proto(proto)) => {
1837+
if let Some(record_name) = proto_record(proto) {
1838+
self.collect_record_type_parameters(
1839+
&record_name,
1840+
visiting,
1841+
parameters,
1842+
proto_record,
1843+
);
1844+
}
18241845
}
18251846
TypeSpec::Bool
18261847
| TypeSpec::Int(_)
@@ -1830,12 +1851,35 @@ where
18301851
| TypeSpec::Enum(_)
18311852
| TypeSpec::Flags(_)
18321853
| TypeSpec::Resource(_)
1833-
| TypeSpec::External(ExternalTypeSpec::Proto(_))
18341854
| TypeSpec::External(ExternalTypeSpec::Json(_)) => {}
18351855
}
18361856
}
18371857
}
18381858

1859+
impl<F> ApiSpec<F>
1860+
where
1861+
F: TypeFamily,
1862+
F::Proto: AsRef<str>,
1863+
F::Record: AsRef<str>,
1864+
F::Variant: AsRef<str>,
1865+
{
1866+
/// Like `type_parameters`, but also walks the record behind each proto type. Before planning, a
1867+
/// proto-backed record is named by its proto type, so the plain walk stops there.
1868+
pub(crate) fn type_parameters_through_protos(
1869+
&self,
1870+
value: &TypeSpec<F>,
1871+
) -> Vec<TypeParameterUsage> {
1872+
let mut parameters = IndexMap::<String, TypeParameterUsage>::new();
1873+
let mut visiting = std::collections::BTreeSet::new();
1874+
let proto_record = |proto: &F::Proto| {
1875+
self.record_for_proto(proto.as_ref())
1876+
.map(|record| record.full_name.clone())
1877+
};
1878+
self.collect_type_parameters(value, &mut visiting, &mut parameters, &proto_record);
1879+
parameters.into_values().collect()
1880+
}
1881+
}
1882+
18391883
#[derive(Debug, Clone, PartialEq)]
18401884
pub struct FunctionFieldSpec<F: TypeFamily = AuthoredFamily> {
18411885
pub primary: bool,

0 commit comments

Comments
 (0)