Ensure services with no operations generate valid Rust #2351
Conversation
| @@ -216,15 +218,14 @@ class ServerServiceGeneratorV2( | |||
| pub fn build(self) -> Result<$serviceName<#{SmithyHttpServer}::routing::Route<$builderBodyGenericTypeName>>, MissingOperationsError> | |||
| { | |||
| let router = { | |||
| use #{SmithyHttpServer}::operation::OperationShape; | |||
| ##[allow(unused_mut)] | |||
There was a problem hiding this comment.
Can we add this conditionally on the fact that we know there are no operations?
There was a problem hiding this comment.
I don't think this is something we generally do (perhaps it should be). Will do it in this case.
There was a problem hiding this comment.
Loosening lints in all cases can lead to bugs going undetected (because a legitimate lint doesn't trigger). If it's easy enough to be precise, I think we should be.
There was a problem hiding this comment.
If it's easy enough to be precise, I think we should be.
+1 to this. I don't think we should bend over backwards to have lints be as tight as possible, but if it's easy I'd go for it.
| val expectMessageVariableName = "unexpected_error_msg" | ||
| val expectMessage = "this should never panic since we are supposed to check beforehand that a handler has been registered for this operation; please file a bug report under https://github.com/awslabs/smithy-rs/issues" |
There was a problem hiding this comment.
We used a variable there to avoid having multiple copies of the same string in the final binary artefact 🤔
There was a problem hiding this comment.
I see, I'd be surprised if it didn't deduplicate them. But I'll put a conditional #[allow(unused_variables)] instead.
|
A new generated diff is ready to view.
A new doc preview is ready to view. |
|
A new generated diff is ready to view.
A new doc preview is ready to view. |
| """ | ||
| if self.$fieldName.is_none() { | ||
| use #{SmithyHttpServer}::operation::OperationShape; |
There was a problem hiding this comment.
The line below uses OperationShape::NAME. It was previously imported below which was unused in the no operation case.
| var missingOperationsUnusedMut = "" | ||
| var unusedExpected = "" | ||
| if (operations.isEmpty()) { | ||
| missingOperationsUnusedMut = "##[allow(unused_mut)]" | ||
| unusedExpected = "##[allow(unused_variables)]" | ||
| } | ||
|
|
There was a problem hiding this comment.
I think you can use Attribute.AllowUnusedMut/AllowUnusedVariables.render(writer) for these.
|
A new generated diff is ready to view.
A new doc preview is ready to view. |
|
A new generated diff is ready to view.
A new doc preview is ready to view. |
|
A new generated diff is ready to view.
A new doc preview is ready to view. |
|
A new generated diff is ready to view.
A new doc preview is ready to view. |
|
Following the decision made in #1369 we should NOT generate code for this Smithy class of models and instead throw an exception. |
Description