Remove TypeContext
#3890
Remove TypeContext
#3890
Conversation
tools/slicec-cs/src/builders.rs
Outdated
| if is_dispatch { | ||
| self.add_parameter( | ||
| "IceRpc.Features.IFeatureCollection?", | ||
| &escape_parameter_name(¶meters, "features"), | ||
| Some("null"), | ||
| Some("The invocation features.".to_owned()), | ||
| ); | ||
| } else { | ||
| self.add_parameter( | ||
| "IceRpc.Features.IFeatureCollection", | ||
| &escape_parameter_name(¶meters, "features"), | ||
| None, | ||
| Some("The dispatch features.".to_owned()), | ||
| ); |
There was a problem hiding this comment.
IMO using a bool makes this ever error prone. What about using a two discriminant enum?
There was a problem hiding this comment.
Yes, I picked a better name for this bool. I was too mechanistic in my translation.
| @@ -406,7 +406,7 @@ fn proxy_base_operation_impl(operation: &Operation, namespace: &str) -> CodeBloc | |||
| let mut builder = FunctionBuilder::new("public", &return_task, &async_name, FunctionType::ExpressionBody); | |||
| builder.set_inherit_doc(true); | |||
| builder.add_obsolete_attribute(operation); | |||
| builder.add_operation_parameters(operation, TypeContext::OutgoingParam); | |||
| builder.add_operation_parameters(operation, true); | |||
There was a problem hiding this comment.
If it's outgoing, and the bool is is_dispatch, this seems backwards.
|
I prefer to keep an enum for this distinction between incoming and outgoing, using a bool is just too confusing when reading the code. |
| fn incoming_type_string(&self, namespace: &str, ignore_optional: bool) -> String; | ||
| fn outgoing_type_string(&self, namespace: &str, ignore_optional: bool) -> String; |
There was a problem hiding this comment.
I would rename these functions incoming_parameter_type_string / outgoing_parameter_type_string
There was a problem hiding this comment.
I would add:
incoming_stream_parameter_type_string / outgoing_stream_parameter_string, where the normal functions are "no stream".
There was a problem hiding this comment.
I would replace ignore_optional by a function that removes ? at the end of the type string, and is called when needed.
|
|
||
| /// Returns the message of the `@param` tag corresponding to this parameter from the operation it's part of. | ||
| /// If the operation has no doc comment, or a matching `@param` tag, this returns `None`. | ||
| fn formatted_param_doc_comment(&self) -> Option<String>; | ||
| } | ||
|
|
||
| impl ParameterExt for Parameter { | ||
| fn cs_type_string(&self, namespace: &str, context: TypeContext, ignore_optional: bool) -> String { | ||
| let type_str = self.data_type().cs_type_string(namespace, context, ignore_optional); | ||
| fn cs_type_string(&self, namespace: &str, ignore_optional: bool, is_outgoing: bool) -> String { |
There was a problem hiding this comment.
I would replace this function by a pair of TypeRefExt functions.
|
Closed in favor of #3893 |
This PR Removes the need for ‘TypeContext’ from
slicec-cs. It’s purely refactoring; there should be no changes to the logic. We used this enum for 2 purposes:Purpose 1
cs_type_stringwould change the mapping of sequences and dictionaries based on this field.Now, instead of 1 function which matches 3 possible TypeContexts, we have 3 separate functions:
cs_type_string(… TypeContext::Field) -> field_type_stringcs_type_string(… TypeContext::IncomingParam) -> incoming_type_stringcs_type_string(… TypeContext::OutgoingParam) -> outgoing_type_stringincoming_type_stringandoutgoing_type_stringjust delegate tofield_type_string, for types other than seuqence or dictionary.Purpose 2
Various parameter-code used it to tell whether we were on the dispatch side or not.
Now, we just pass a bool named
is_dispatchwhich we already had in some functions anyways.IncomingParam -> false,OutgoingParam -> trueUsually these matches matched
TypeContext::Fieldtounreachable!.This change lets us remove all these dead branches.
Additionally, in
encoding.rswe usedTypeContextto tell whether we generated for fields or parameters.Instead of TypeContext, these functions now also take a bool:
is_outgoing_paramOutgoingParam -> trueandField -> false.Because this is all encoding code, we never encounter
IncomingParam.