[CodeGenLib] Add validation step for input #1753
Merged
Conversation
This file contains hidden or bidirectional Unicode text that may be interpreted or compiled differently than what appears below. To review, open the file in an editor that reveals hidden Unicode characters.
Learn more about bidirectional Unicode characters
…nslator Motivation: The IDLToStructuredSwiftTranslator should discover errors in the CodeGenerationRequest object, before the specialized translators start their work. This way, if the input is not correct no translator will be started. Modifications: Moved the validation functions into a IDLToStructuredSwiftTranslator extension and the associated tests in a separate test file. Result: The input validation is not done by each individual specialzed translator, but by the main translator, before transformations take place.
stefanadranca
changed the title
gjcairo
approved these changes
Jan 4, 2024
Collaborator
gjcairo
left a comment
gjcairo
left a comment
There was a problem hiding this comment.
LGTM, just left a very small comment
Comment on lines
71
to
83
| let noNamespaceServices = servicesByNamespace["", default: []] | ||
| let namespaces = servicesByNamespace.keys | ||
| for service in noNamespaceServices { | ||
| if namespaces.contains(service.name) { | ||
| throw CodeGenError( | ||
| code: .nonUniqueServiceName, | ||
| message: """ | ||
| Services with no namespace must not have the same names as the namespaces. \ | ||
| \(service.name) is used as a name for a service with no namespace and a namespace. | ||
| """ | ||
| ) | ||
| } | ||
| } |
Collaborator
There was a problem hiding this comment.
Very minor nit, but you can skip the , default: []] subscript and instead make this whole block conditional to it being an empty key:
Suggested change
| let noNamespaceServices = servicesByNamespace["", default: []] | |
| let namespaces = servicesByNamespace.keys | |
| for service in noNamespaceServices { | |
| if namespaces.contains(service.name) { | |
| throw CodeGenError( | |
| code: .nonUniqueServiceName, | |
| message: """ | |
| Services with no namespace must not have the same names as the namespaces. \ | |
| \(service.name) is used as a name for a service with no namespace and a namespace. | |
| """ | |
| ) | |
| } | |
| } | |
| if let noNamespaceServices = servicesByNamespace[""] { | |
| let namespaces = servicesByNamespace.keys | |
| for service in noNamespaceServices { | |
| if namespaces.contains(service.name) { | |
| throw CodeGenError( | |
| code: .nonUniqueServiceName, | |
| message: """ | |
| Services with no namespace must not have the same names as the namespaces. \ | |
| \(service.name) is used as a name for a service with no namespace and a namespace. | |
| """ | |
| ) | |
| } | |
| } | |
| } |
glbrntt
approved these changes
Jan 8, 2024
| grouping: codeGenerationRequest.services, | ||
| by: { $0.namespace } | ||
| ) | ||
| try checkServiceNamesAreUnique(for: servicesByNamespace) |
Collaborator
There was a problem hiding this comment.
Suggested change
| try checkServiceNamesAreUnique(for: servicesByNamespace) | |
| try self.checkServiceNamesAreUnique(for: servicesByNamespace) |
| ) | ||
| try checkServiceNamesAreUnique(for: servicesByNamespace) | ||
| for service in codeGenerationRequest.services { | ||
| try checkMethodNamesAreUnique(in: service) |
Collaborator
There was a problem hiding this comment.
Suggested change
| try checkMethodNamesAreUnique(in: service) | |
| try self.checkMethodNamesAreUnique(in: service) |
glbrntt
pushed a commit
to glbrntt/grpc-swift
that referenced
this pull request
Feb 5, 2024
Add validation step for input in IDLToStructuredSwiftTranslator Motivation: The IDLToStructuredSwiftTranslator should discover errors in the CodeGenerationRequest object, before the specialized translators start their work. This way, if the input is not correct no translator will be started. Modifications: Moved the validation functions into a IDLToStructuredSwiftTranslator extension and the associated tests in a separate test file. Result: The input validation is not done by each individual specialzed translator, but by the main translator, before transformations take place.
Sign up for free
to join this conversation on GitHub.
Already have an account?
Sign in to comment
3 participants
Add this suggestion to a batch that can be applied as a single commit.
This suggestion is invalid because no changes were made to the code.
Suggestions cannot be applied while the pull request is closed.
Suggestions cannot be applied while viewing a subset of changes.
Only one suggestion per line can be applied in a batch.
Add this suggestion to a batch that can be applied as a single commit.
Applying suggestions on deleted lines is not supported.
You must change the existing code in this line in order to create a valid suggestion.
Outdated suggestions cannot be applied.
This suggestion has been applied or marked resolved.
Suggestions cannot be applied from pending reviews.
Suggestions cannot be applied on multi-line comments.
Suggestions cannot be applied while the pull request is queued to merge.
Suggestion cannot be applied right now. Please check back later.
Add validation step for input in IDLToStructuredSwiftTranslator
Motivation:
The IDLToStructuredSwiftTranslator should discover errors in the CodeGenerationRequest object, before the specialized translators start their work. This way, if the input is not correct no translator will be started.
Modifications:
Moved the validation functions into a IDLToStructuredSwiftTranslator extension and the associated tests in a separate test file.
Result:
The input validation is not done by each individual specialzed translator, but by the main translator, before transformations take place.