Migrate ModelTypeResolver and ObjectModelFactory to IR types#210
Migrate ModelTypeResolver and ObjectModelFactory to IR types#210MadsBogeskov wants to merge 6 commits intomainfrom
Conversation
Co-Authored-By: Claude Sonnet 4.6 <noreply@anthropic.com>
Introduces a new `SwaggerSwiftIR` target containing spec-agnostic intermediate representation types that both Swagger 2.0 and future OpenAPI 3.x parsers will convert into. Adds `SwaggerToIRConverter` in `SwaggerSwiftML` to convert the existing `Swagger` struct into `IRDocument`, including full schema, parameter, request body, and response mapping. Co-Authored-By: Claude Sonnet 4.6 <noreply@anthropic.com>
- Add OpenAPI3Document and supporting model types (schema, parameter, operation, request body, response, components, server, OrRef) - Add OpenAPI3ToIRConverter converting OAS3 → IRDocument - Add SwaggerReader.readSpec() with version detection, returning APISpec (.swagger2 or .openapi3) — both convert to IRDocument via .toIR() - Add 33 tests covering parsing, IR conversion, and version detection - Add BasicOpenAPI3.yaml fixture Co-Authored-By: Claude Sonnet 4.6 <noreply@anthropic.com>
Replace all SwaggerSwiftML-specific DataFormat/Schema references in the resolver layer with spec-agnostic IRSchema types. Introduces IRStringFormat, IRIntegerFormat, and IRNumberFormat; adds Schema.toIR() / Node<Schema>.toIR() helpers for incremental call-site migration in APIRequestFactory and RequestParameterFactory. All 41 tests pass. Co-Authored-By: Claude Sonnet 4.6 <noreply@anthropic.com>
![cursor[bot]](https://avatars.githubusercontent.com/in/1210556?s=60&v=4){kind=small}
| case .int32: return .int32 | ||
| case .long: return .int64 | ||
| default: return nil | ||
| } |
There was a problem hiding this comment.
Integer format "int64" silently downgrades to Int
High Severity
The DataFormat.irIntegerFormat conversion only maps .int32 → .int32 and .long → .int64. The standard Swagger 2.0 format "int64" is parsed as DataFormat.unsupported("int64"), which falls into default: return nil. The old IntegerResolver explicitly handled .unsupported("int64") → Int64 and .unsupported("decimal") → Double. Now those formats map to nil, causing IntegerResolver to default to plain Int, silently losing 64-bit precision for a standard format.
Additional Locations (1)
- [
Sources/SwaggerSwiftCore/Model Type Resolver/Resolvers/IntegerResolver.swift#L3-L13](https://github.com/lunarway/SwaggerSwift/blob/6b6662e48a703e504f551ef33af68a17e21eb003/Sources/SwaggerSwiftCore/Model Type Resolver/Resolvers/IntegerResolver.swift#L3-L13)
Reviewed by Cursor Bugbot for commit 6b6662e. Configure here.
| allOf: irAllOf | ||
| ))) | ||
| } | ||
| } |
There was a problem hiding this comment.
Items.toIR() drops vendor extension custom fields
Medium Severity
Items.toIR() creates IRSchema instances without propagating Items.customFields. The IRSchema init defaults customFields to [:], so vendor extensions like x-override-name and x-internal on array item objects are silently dropped. The old code explicitly passed node.customFields to objectModelFactory.make when handling Items of type object.
Reviewed by Cursor Bugbot for commit 6b6662e. Configure here.
| typeName = typeName | ||
| .modelNamed | ||
| .split(separator: ".").map { String($0).uppercasingFirst }.joined() | ||
| let qualifiedName = "\(serviceName).\(typeName)" |
There was a problem hiding this comment.
Reference "Type" collision check compares raw name
Medium Severity
In resolveProperty, the condition refName == "Type" checks the raw reference name extracted from the IR, but the old code checked modelReference.typeName == "Type" which was already .modelNamed-transformed. A definition named "type" (lowercase) transforms to "Type" via .modelNamed, triggering collision handling in the old code but not in the new code, potentially producing ServiceName.Type which conflicts with Swift's metatype syntax.
Reviewed by Cursor Bugbot for commit 6b6662e. Configure here.
| // qualification — matching the historical codegen behaviour where top-level response | ||
| // type references are not prefixed with the service name. | ||
| if case .reference(let name) = irSchema.type { | ||
| return (.object(typeName: name), []) |
There was a problem hiding this comment.
Response $ref ignores actual type of referenced schema
Medium Severity
When a response or body schema contains a bare $ref, the new code unconditionally returns .object(typeName: name). The old code resolved the referenced schema and returned its actual type — e.g., .string(defaultValue: nil) for string typedefs, or .enumeration for enum types. This changes generated function signatures for any spec where a response or body $ref points to a non-object definition.
Additional Locations (1)
- [
Sources/SwaggerSwiftCore/API Request Factory/RequestParameterFactory.swift#L362-L372](https://github.com/lunarway/SwaggerSwift/blob/6b6662e48a703e504f551ef33af68a17e21eb003/Sources/SwaggerSwiftCore/API Request Factory/RequestParameterFactory.swift#L362-L372)
Reviewed by Cursor Bugbot for commit 6b6662e. Configure here.
Co-Authored-By: Claude Sonnet 4.6 <noreply@anthropic.com>
There was a problem hiding this comment.
Cursor Bugbot has reviewed your changes and found 2 potential issues.
There are 6 total unresolved issues (including 4 from previous reviews).
❌ Bugbot Autofix is OFF. To automatically fix reported issues with cloud agents, have a team admin enable autofix in the Cursor dashboard.
Reviewed by Cursor Bugbot for commit 8ad1c98. Configure here.
|
|
||
| mutating func run() async throws { | ||
| guard let token = gitHubToken ?? ProcessInfo.processInfo.environment["GITHUB_TOKEN"] else { | ||
| guard let token = githubToken ?? ProcessInfo.processInfo.environment["GITHUB_TOKEN"] else { |
There was a problem hiding this comment.
Error message references old CLI flag name
Medium Severity
The property was renamed from gitHubToken to githubToken, which changes the swift-argument-parser derived CLI flag from --git-hub-token to --github-token. The ValidationError message still references the old --git-hub-token flag, so users following the error guidance will use a flag that doesn't exist.
Reviewed by Cursor Bugbot for commit 8ad1c98. Configure here.
| return (.object(typeName: "[String: AdditionalProperty]"), []) | ||
| case .schema(let valueSchema): | ||
| if case .reference(let name) = valueSchema.type { | ||
| return (.object(typeName: "[String: \(serviceName).\(name)]"), []) |
There was a problem hiding this comment.
Dictionary reference values gain unexpected namespace prefix
Medium Severity
When a dictionary schema has a $ref value type, the new code produces [String: ServiceName.TypeName] (with serviceName prefix), whereas the old ModelTypeResolver produced [String: TypeName] (unqualified). This changes the generated Swift type name for dictionary-typed properties with reference values.
Reviewed by Cursor Bugbot for commit 8ad1c98. Configure here.
Co-Authored-By: Claude Sonnet 4.6 <noreply@anthropic.com>
1 participant
Summary
ModelTypeResolver,ObjectModelFactory,StringResolver,IntegerResolver, andNumberResolverto acceptIRSchema/ IR format enums instead ofSwaggerSwiftML.Schema/DataFormatSchema.toIR()andNode<Schema>.toIR()extension helpers inSwaggerSwiftMLso call sites inSwaggerSwiftCorecan convert incrementally without needing a full document contextcustomFields: [String: String]toIRSchemaso vendor extensions (x-override-name,x-internal,x-nullable) propagate through the IR layerAPIRequestFactoryandRequestParameterFactoryto convert at the call boundary; preserves the historical behaviour where top-level response/body$refs are not namespace-qualifiedTest plan
swift test --parallel— all 41 tests pass (including golden file snapshot test)TestService.swiftis byte-for-byte identical to before the migration🤖 Generated with Claude Code
Note
Medium Risk
Refactors core codegen type-resolution logic and introduces a new IR layer plus OpenAPI 3 parsing, which could subtly change generated types (namespacing, allOf/additionalProperties handling) despite attempts to preserve existing output.
Overview
Adds a new
SwaggerSwiftIRtarget with a spec-agnostic intermediate representation (IRDocument,IRSchema, parameters/operations/responses) and converts both Swagger 2.0 and new OpenAPI 3.x documents into this IR (SwaggerToIRConverter,OpenAPI3ToIRConverter).Migrates
ModelTypeResolver/ObjectModelFactory(andString/Integer/Numberresolvers) to operate onIRSchemainstead ofSwaggerSwiftMLschema types, updatingAPIFactory,APIRequestFactory, andRequestParameterFactoryto convert schema nodes viatoIR()while preserving the previous non-namespaced behavior for top-level body/response$refs.Updates the CLI/CI token flag to
--github-token, adds OpenAPI3 parsing fixtures/tests, and adjusts resolver unit tests to use IR format enums.Reviewed by Cursor Bugbot for commit 80867ee. Bugbot is set up for automated code reviews on this repo. Configure here.