From 29313b41b1e682ee3b726ba28d93f51c4009ee2d Mon Sep 17 00:00:00 2001 From: andreatp Date: Tue, 25 Aug 2026 12:02:42 +0100 Subject: [PATCH 1/5] fix(ruby): add composed type support and disable inner classes for wrappers Implements union and intersection composed types for the Ruby generator: factory, serializer and deserializer bodies for each, mirroring Python's approach adapted to Ruby idioms. Composed type wrappers are no longer emitted as inner classes. Ruby renders the wrapper's accessors outside the inner class body, producing invalid code; passing supportInnerClasses: false places wrappers as namespace-level siblings, matching what Python already does. Each non-complex property in an intersection factory parses into its own val_ variable. A shared variable would be reassigned inside the previous branch of the if/elsif chain, so every property after the first was silently never deserialized. Block writers close their if/elsif chains with CloseBlock("end", false) since the loop already restored the indent; closing again shifted the `end` and every later `end` in the file one level left. AssertBalancedBlocks now tracks opener columns rather than only counting, so a misaligned `end` fails the test rather than passing silently. --- CHANGELOG.md | 1 + it/config.json | 24 +- it/ruby/.rubocop.yml | 31 +++ it/ruby/Gemfile | 4 +- src/Kiota.Builder/Refiners/RubyRefiner.cs | 3 +- .../Writers/Ruby/CodeMethodWriter.cs | 195 +++++++++++++- src/kiota/appsettings.json | 4 +- .../Writers/Ruby/CodeMethodWriterTests.cs | 251 ++++++++++++++++++ 8 files changed, 490 insertions(+), 23 deletions(-) diff --git a/CHANGELOG.md b/CHANGELOG.md index 9988c18b2e..0860fc9240 100644 --- a/CHANGELOG.md +++ b/CHANGELOG.md @@ -18,6 +18,7 @@ and this project adheres to [Semantic Versioning](https://semver.org/spec/v2.0.0 ### Changed - Fixed defaults primitive default-value handling during code generation for all languages +- Ruby: added composed type (union/intersection) support for factory methods, serializers, and deserializers. Disabled inner classes for composed type wrappers. Un-suppresses the Twitter integration test. Requires `microsoft_kiota_abstractions` 0.16.0 and `microsoft_kiota_serialization_json` 0.11.0. [kiota-abstractions-ruby#73](https://github.com/microsoft/kiota-abstractions-ruby/issues/73) [#1816](https://github.com/microsoft/kiota/issues/1816) - Fixed plugin manifest generation to omit unsafe `oauth_card_path` file references that could resolve outside the plugin package. - Bumped `Microsoft.OpenApi` and `Microsoft.OpenApi.YamlReader` to 3.10.0. - Numeric scalar unions with a format (e.g. `type: ["integer", "string"]` with `format: int32`, as emitted by ASP.NET Core's OpenAPI 3.1 generator under System.Text.Json's default `JsonNumberHandling.AllowReadingFromString`) now map to the numeric type instead of degrading to `UntypedNode`. [#6541](https://github.com/microsoft/kiota/issues/6541) diff --git a/it/config.json b/it/config.json index 472659e39a..2b9e6351d8 100644 --- a/it/config.json +++ b/it/config.json @@ -16,7 +16,7 @@ "Suppressions": [ { "Language": "ruby", - "Rationale": "https://github.com/microsoft/kiota-abstractions-ruby/issues/73" + "Rationale": "Generated client emits a require_relative to a models file that is never generated (e.g. models/with_path), causing a LoadError. Pre-existing naming/require bug, unrelated to composed types." }, { "Language": "dart", @@ -62,8 +62,7 @@ } ] }, - "https://raw.githubusercontent.com/googlemaps/openapi-specification/refs/tags/v1.22.5/dist/google-maps-platform-openapi3.yml": { - }, + "https://raw.githubusercontent.com/googlemaps/openapi-specification/refs/tags/v1.22.5/dist/google-maps-platform-openapi3.yml": {}, "https://developers.pipedrive.com/docs/api/v1/openapi.yaml": { "ExcludePatterns": [ { @@ -95,12 +94,12 @@ "https://raw.githubusercontent.com/stripe/openapi/refs/heads/master/latest/openapi.spec3.json": { "Suppressions": [ { - "Language": "java", - "Rationale": "https://github.com/microsoft/kiota/issues/2842" + "Language": "ruby", + "Rationale": "Schema names containing dots produce invalid Ruby class names (e.g. 'class S.v2.coreAccount...'), causing SyntaxErrors. Pre-existing naming bug, unrelated to composed types." }, { - "Language": "ruby", - "Rationale": "https://github.com/microsoft/kiota/issues/1816" + "Language": "java", + "Rationale": "https://github.com/microsoft/kiota/issues/2842" }, { "Language": "php", @@ -111,10 +110,6 @@ { "Language": "java", "Rationale": "https://github.com/microsoft/kiota/issues/2842" - }, - { - "Language": "ruby", - "Rationale": "https://github.com/microsoft/kiota/issues/1816" } ] }, @@ -142,14 +137,9 @@ } ] }, - "https://raw.githubusercontent.com/docusign/OpenAPI-Specifications/refs/heads/master/esignature.rest.swagger-v2.1.json": { - }, + "https://raw.githubusercontent.com/docusign/OpenAPI-Specifications/refs/heads/master/esignature.rest.swagger-v2.1.json": {}, "https://api.twitter.com/2/openapi.json": { "Suppressions": [ - { - "Language": "ruby", - "Rationale": "https://github.com/microsoft/kiota-abstractions-ruby/issues/73" - }, { "Language": "dart", "Rationale": "dart analyze fails on unnecessary_null_comparison for primitive binary union media models - https://github.com/microsoft/kiota/issues/7997" diff --git a/it/ruby/.rubocop.yml b/it/ruby/.rubocop.yml index dd8feca659..19c218dbde 100644 --- a/it/ruby/.rubocop.yml +++ b/it/ruby/.rubocop.yml @@ -134,3 +134,34 @@ Metrics/CyclomaticComplexity: # Generated factory methods may just delegate to super Lint/UselessMethodDefinition: Enabled: false + +# Generated class declarations may have an empty line after the class keyword +Layout/EmptyLinesAroundClassBody: + Enabled: false + +# Generated composed type methods can have high perceived complexity +Metrics/PerceivedComplexity: + Enabled: false + + +# Generated code may duplicate method definitions in certain request builder patterns +Lint/DuplicateMethods: + Enabled: false + + + +# Generated constructors may return values or skip super calls +Lint/ReturnInVoidContext: + Enabled: false + +Lint/MissingSuper: + Enabled: false + + + +# Generated composed types use if !x.nil? for elsif compatibility +Style/NegatedIf: + Enabled: false + +Style/GuardClause: + Enabled: false diff --git a/it/ruby/Gemfile b/it/ruby/Gemfile index a2a044d4fd..997771ac8f 100644 --- a/it/ruby/Gemfile +++ b/it/ruby/Gemfile @@ -11,10 +11,10 @@ gem "rspec", "~> 3.0" gem "rubocop", "~> 1.21" -gem "microsoft_kiota_abstractions", "~> 0.15.1" +gem "microsoft_kiota_abstractions", "~> 0.16.0" gem "microsoft_kiota_faraday", "~> 0.16.0" -gem "microsoft_kiota_serialization_json", "~> 0.10.0" +gem "microsoft_kiota_serialization_json", "~> 0.11.0" gem "microsoft_kiota_authentication_oauth", "~> 0.9.0" diff --git a/src/Kiota.Builder/Refiners/RubyRefiner.cs b/src/Kiota.Builder/Refiners/RubyRefiner.cs index b2165d289a..929c9fb5b8 100644 --- a/src/Kiota.Builder/Refiners/RubyRefiner.cs +++ b/src/Kiota.Builder/Refiners/RubyRefiner.cs @@ -43,7 +43,8 @@ public override Task RefineAsync(CodeNamespace generatedCode, CancellationToken UpdateReferencesToDisambiguatedClasses(generatedCode, classesToDisambiguate, suffix); ConvertUnionTypesToWrapper(generatedCode, _configuration.UsesBackingStore, - static s => s + static s => s, + false ); var reservedNamesProvider = new RubyReservedNamesProvider(); CorrectNames(generatedCode, s => diff --git a/src/Kiota.Builder/Writers/Ruby/CodeMethodWriter.cs b/src/Kiota.Builder/Writers/Ruby/CodeMethodWriter.cs index e90a9f4a1c..6e2d9c122b 100644 --- a/src/Kiota.Builder/Writers/Ruby/CodeMethodWriter.cs +++ b/src/Kiota.Builder/Writers/Ruby/CodeMethodWriter.cs @@ -72,6 +72,8 @@ public override void WriteCodeElement(CodeMethod codeElement, LanguageWriter wri break; case CodeMethodKind.RequestBuilderBackwardCompatibility: throw new InvalidOperationException("RequestBuilderBackwardCompatibility is not supported as the request builders are implemented by properties."); + case CodeMethodKind.ComposedTypeMarker: + throw new InvalidOperationException("ComposedTypeMarker is not required as the wrapper is implemented directly."); default: writer.WriteLine("return nil;"); break; @@ -86,9 +88,18 @@ private void WriteRawUrlBuilderBody(CodeClass parentClass, CodeMethod codeElemen } private const string DiscriminatorMappingVarName = "mapping_value"; private const string NodeVarName = "mapping_value_node"; - private static void WriteFactoryMethodBody(CodeMethod codeElement, CodeClass parentClass, LanguageWriter writer) + private void WriteFactoryMethodBody(CodeMethod codeElement, CodeClass parentClass, LanguageWriter writer) { var parseNodeParameter = codeElement.Parameters.OfKind(CodeParameterKind.ParseNode) ?? throw new InvalidOperationException("Factory method should have a ParseNode parameter"); + if (parentClass.DiscriminatorInformation.ShouldWriteDiscriminatorForUnionType) + WriteFactoryMethodBodyForUnionModel(parseNodeParameter, parentClass, writer); + else if (parentClass.DiscriminatorInformation.ShouldWriteDiscriminatorForIntersectionType) + WriteFactoryMethodBodyForIntersectionModel(parseNodeParameter, parentClass, writer); + else + WriteFactoryMethodBodyForInheritedModel(parseNodeParameter, parentClass, writer); + } + private static void WriteFactoryMethodBodyForInheritedModel(CodeParameter parseNodeParameter, CodeClass parentClass, LanguageWriter writer) + { var writeDiscriminatorValueRead = parentClass.DiscriminatorInformation.ShouldWriteParseNodeCheck && !parentClass.DiscriminatorInformation.ShouldWriteDiscriminatorForIntersectionType; var discriminatorMappings = parentClass.DiscriminatorInformation.DiscriminatorMappings.OrderBy(static x => x.Key).ToArray(); if (writeDiscriminatorValueRead && discriminatorMappings.Length > 0) @@ -108,6 +119,93 @@ private static void WriteFactoryMethodBody(CodeMethod codeElement, CodeClass par } writer.WriteLine($"return {parentClass.Name.ToFirstCharacterUpperCase()}.new"); } + private void WriteFactoryMethodBodyForUnionModel(CodeParameter parseNodeParameter, CodeClass parentClass, LanguageWriter writer) + { + writer.WriteLine($"result = {parentClass.Name.ToFirstCharacterUpperCase()}.new"); + var parseNodeParameterName = parseNodeParameter.Name.ToSnakeCase(); + var customProperties = parentClass.GetPropertiesOfKind(CodePropertyKind.Custom) + .OrderBy(static x => x, new CodePropertyTypeComparer()) + .ThenBy(static x => x.Name, StringComparer.OrdinalIgnoreCase) + .ToArray(); + var complexPropertiesWithMappings = customProperties + .Where(static x => x.Type is CodeType propType && propType.TypeDefinition is CodeClass && propType.CollectionKind == CodeTypeBase.CodeTypeCollectionKind.None) + .Select(p => (property: p, mappedKey: parentClass.DiscriminatorInformation.DiscriminatorMappings + .FirstOrDefault(x => x.Value.Name.Equals(p.Type.Name, StringComparison.OrdinalIgnoreCase)).Key)) + .Where(static x => !string.IsNullOrEmpty(x.mappedKey)) + .ToArray(); + if (complexPropertiesWithMappings.Length > 0) + { + writer.WriteLine($"{NodeVarName} = {parseNodeParameterName}.get_child_node(\"{RubyConventionService.SanitizeRubyDoubleQuoteLiteral(parentClass.DiscriminatorInformation.DiscriminatorPropertyName)}\")"); + writer.StartBlock($"unless {NodeVarName}.nil?"); + writer.WriteLine($"{DiscriminatorMappingVarName} = {NodeVarName}.get_string_value"); + var elseIfPrefix = string.Empty; + foreach (var (property, mappedKey) in complexPropertiesWithMappings) + { + writer.StartBlock($"{elseIfPrefix}if {DiscriminatorMappingVarName}.downcase == \"{RubyConventionService.SanitizeRubyDoubleQuoteLiteral(mappedKey)}\".downcase"); + writer.WriteLine($"result.{property.Name.ToSnakeCase()} = {property.Type.Name.ToFirstCharacterUpperCase()}.new"); + writer.DecreaseIndent(); + elseIfPrefix = "els"; + } + // the loop already restored the indent, so the chain's `end` must not decrease it again + writer.CloseBlock("end", false); + writer.CloseBlock("end"); + } + foreach (var property in customProperties.Where(static x => x.Type is not CodeType propType || propType.TypeDefinition is not CodeClass || propType.CollectionKind != CodeTypeBase.CodeTypeCollectionKind.None)) + { + var methodName = GetDeserializationMethodName(property.Type); + writer.WriteLine($"val = {parseNodeParameterName}.{methodName}"); + writer.StartBlock("unless val.nil?"); + writer.WriteLine($"result.{property.Name.ToSnakeCase()} = val"); + writer.CloseBlock("end"); + } + writer.WriteLine("return result"); + } + private static string GetIntersectionValueVarName(CodeProperty property) => $"val_{property.Name.ToSnakeCase()}"; + private void WriteFactoryMethodBodyForIntersectionModel(CodeParameter parseNodeParameter, CodeClass parentClass, LanguageWriter writer) + { + writer.WriteLine($"result = {parentClass.Name.ToFirstCharacterUpperCase()}.new"); + var parseNodeParameterName = parseNodeParameter.Name.ToSnakeCase(); + var customProperties = parentClass.GetPropertiesOfKind(CodePropertyKind.Custom) + .OrderBy(static x => x, new CodePropertyTypeComparer(orderByDesc: true)) + .ThenBy(static x => x.Name, StringComparer.OrdinalIgnoreCase) + .ToArray(); + var nonComplexProperties = customProperties.Where(static x => x.Type is not CodeType propType || propType.TypeDefinition is not CodeClass || propType.CollectionKind != CodeTypeBase.CodeTypeCollectionKind.None).ToArray(); + var complexProperties = customProperties.Where(static x => x.Type is CodeType propType && propType.TypeDefinition is CodeClass && propType.CollectionKind == CodeTypeBase.CodeTypeCollectionKind.None).ToArray(); + // each property needs its own variable: a shared one would be reassigned inside the + // previous branch of the if/elsif chain, so only the first property would ever be read + foreach (var property in nonComplexProperties) + { + var methodName = GetDeserializationMethodName(property.Type); + writer.WriteLine($"{GetIntersectionValueVarName(property)} = {parseNodeParameterName}.{methodName}"); + } + var elseIfPrefix = string.Empty; + foreach (var property in nonComplexProperties) + { + writer.StartBlock($"{elseIfPrefix}if !{GetIntersectionValueVarName(property)}.nil?"); + writer.WriteLine($"result.{property.Name.ToSnakeCase()} = {GetIntersectionValueVarName(property)}"); + writer.DecreaseIndent(); + elseIfPrefix = "els"; + } + if (complexProperties.Length > 0 && nonComplexProperties.Length > 0) + { + writer.StartBlock("else"); + foreach (var property in complexProperties) + { + writer.WriteLine($"result.{property.Name.ToSnakeCase()} = {property.Type.Name.ToFirstCharacterUpperCase()}.new"); + } + writer.DecreaseIndent(); + } + else if (complexProperties.Length > 0) + { + foreach (var property in complexProperties) + { + writer.WriteLine($"result.{property.Name.ToSnakeCase()} = {property.Type.Name.ToFirstCharacterUpperCase()}.new"); + } + } + if (nonComplexProperties.Length > 0) + writer.CloseBlock("end", false); + writer.WriteLine("return result"); + } private static void AddNullChecks(CodeMethod codeElement, LanguageWriter writer) { if (!codeElement.IsOverload) @@ -271,6 +369,15 @@ private void WriteIndexerBody(CodeMethod codeElement, CodeClass parentClass, Lan conventions.AddRequestBuilderBody(parentClass, returnType, writer, conventions.TempDictionaryVarName, $"return {prefix}"); } private void WriteDeserializerBody(CodeClass parentClass, LanguageWriter writer) + { + if (parentClass.DiscriminatorInformation.ShouldWriteDiscriminatorForUnionType) + WriteDeserializerBodyForUnionModel(parentClass, writer); + else if (parentClass.DiscriminatorInformation.ShouldWriteDiscriminatorForIntersectionType) + WriteDeserializerBodyForIntersectionModel(parentClass, writer); + else + WriteDeserializerBodyForInheritedModel(parentClass, writer); + } + private void WriteDeserializerBodyForInheritedModel(CodeClass parentClass, LanguageWriter writer) { if (parentClass.StartBlock.Inherits != null) writer.WriteLine("return super.merge({"); @@ -289,6 +396,36 @@ private void WriteDeserializerBody(CodeClass parentClass, LanguageWriter writer) else writer.WriteLine("}"); } + private static void WriteDeserializerBodyForUnionModel(CodeClass parentClass, LanguageWriter writer) + { + var complexProperties = parentClass.GetPropertiesOfKind(CodePropertyKind.Custom) + .Where(static x => x.Type is CodeType propType && propType.TypeDefinition is CodeClass && propType.CollectionKind == CodeTypeBase.CodeTypeCollectionKind.None) + .OrderBy(static x => x.Name, StringComparer.OrdinalIgnoreCase) + .ToArray(); + foreach (var property in complexProperties) + { + writer.StartBlock($"unless @{property.Name.ToSnakeCase()}.nil?"); + writer.WriteLine($"return @{property.Name.ToSnakeCase()}.get_field_deserializers()"); + writer.CloseBlock("end"); + } + writer.WriteLine("return {}"); + } + private static void WriteDeserializerBodyForIntersectionModel(CodeClass parentClass, LanguageWriter writer) + { + var complexProperties = parentClass.GetPropertiesOfKind(CodePropertyKind.Custom) + .Where(static x => x.Type is CodeType propType && propType.TypeDefinition is CodeClass && propType.CollectionKind == CodeTypeBase.CodeTypeCollectionKind.None) + .OrderBy(static x => x.Name, StringComparer.OrdinalIgnoreCase) + .ToArray(); + if (complexProperties.Length > 0) + { + var condition = string.Join(" || ", complexProperties.Select(x => $"@{x.Name.ToSnakeCase()}")); + writer.StartBlock($"if {condition}"); + var propNames = string.Join(", ", complexProperties.Select(x => $"@{x.Name.ToSnakeCase()}")); + writer.WriteLine($"return MicrosoftKiotaAbstractions::ParseNodeHelper.merge_deserializers_for_intersection_wrapper({propNames})"); + writer.CloseBlock("end"); + } + writer.WriteLine("return {}"); + } private void WriteRequestExecutorBody(CodeMethod codeElement, RequestParams requestParams, CodeClass parentClass, string returnType, LanguageWriter writer) { if (returnType.Equals("void", StringComparison.OrdinalIgnoreCase)) @@ -380,6 +517,15 @@ private void WriteRequestGeneratorBody(CodeMethod codeElement, RequestParams req } private static string GetPropertyCall(CodeProperty property, string defaultValue) => property == null ? defaultValue : $"@{property.NamePrefix}{property.Name.ToSnakeCase()}"; private void WriteSerializerBody(CodeClass parentClass, LanguageWriter writer) + { + if (parentClass.DiscriminatorInformation.ShouldWriteDiscriminatorForUnionType) + WriteSerializerBodyForUnionModel(parentClass, writer); + else if (parentClass.DiscriminatorInformation.ShouldWriteDiscriminatorForIntersectionType) + WriteSerializerBodyForIntersectionModel(parentClass, writer); + else + WriteSerializerBodyForInheritedModel(parentClass, writer); + } + private void WriteSerializerBodyForInheritedModel(CodeClass parentClass, LanguageWriter writer) { var additionalDataProperty = parentClass.GetPropertyOfKind(CodePropertyKind.AdditionalData); if (parentClass.StartBlock.Inherits != null) @@ -393,6 +539,53 @@ private void WriteSerializerBody(CodeClass parentClass, LanguageWriter writer) if (additionalDataProperty != null) writer.WriteLine($"writer.write_additional_data(@{additionalDataProperty.NamePrefix}{additionalDataProperty.Name.ToSnakeCase()})"); } + private void WriteSerializerBodyForUnionModel(CodeClass parentClass, LanguageWriter writer) + { + var customProperties = parentClass.GetPropertiesOfKind(CodePropertyKind.Custom) + .OrderBy(static x => x, new CodePropertyTypeComparer()) + .ThenBy(static x => x.Name, StringComparer.OrdinalIgnoreCase) + .ToArray(); + var elseIfPrefix = string.Empty; + foreach (var property in customProperties) + { + writer.StartBlock($"{elseIfPrefix}if !@{property.Name.ToSnakeCase()}.nil?"); + writer.WriteLine($"writer.{GetSerializationMethodName(property.Type)}(nil, @{property.Name.ToSnakeCase()})"); + writer.DecreaseIndent(); + elseIfPrefix = "els"; + } + // the loop already restored the indent, so the chain's `end` must not decrease it again + if (customProperties.Length > 0) + writer.CloseBlock("end", false); + } + private void WriteSerializerBodyForIntersectionModel(CodeClass parentClass, LanguageWriter writer) + { + var customProperties = parentClass.GetPropertiesOfKind(CodePropertyKind.Custom) + .OrderBy(static x => x, new CodePropertyTypeComparer(orderByDesc: true)) + .ThenBy(static x => x.Name, StringComparer.OrdinalIgnoreCase) + .ToArray(); + var nonComplexProperties = customProperties.Where(static x => x.Type is not CodeType propType || propType.TypeDefinition is not CodeClass || propType.CollectionKind != CodeTypeBase.CodeTypeCollectionKind.None).ToArray(); + var complexProperties = customProperties.Where(static x => x.Type is CodeType propType && propType.TypeDefinition is CodeClass && propType.CollectionKind == CodeTypeBase.CodeTypeCollectionKind.None).ToArray(); + var elseIfPrefix = string.Empty; + foreach (var property in nonComplexProperties) + { + writer.StartBlock($"{elseIfPrefix}if !@{property.Name.ToSnakeCase()}.nil?"); + writer.WriteLine($"writer.{GetSerializationMethodName(property.Type)}(nil, @{property.Name.ToSnakeCase()})"); + writer.DecreaseIndent(); + elseIfPrefix = "els"; + } + if (complexProperties.Length > 0) + { + if (nonComplexProperties.Length > 0) + writer.StartBlock("else"); + var complexPropNames = string.Join(", ", complexProperties.Select(x => $"@{x.Name.ToSnakeCase()}")); + writer.WriteLine($"writer.write_object_value(nil, {complexPropNames})"); + if (nonComplexProperties.Length > 0) + writer.DecreaseIndent(); + } + // the branches above already restored the indent, so the chain's `end` must not decrease it again + if (nonComplexProperties.Length > 0) + writer.CloseBlock("end", false); + } private static readonly BaseCodeParameterOrderComparer parameterOrderComparer = new(); private void WriteMethodPrototype(CodeMethod code, LanguageWriter writer) { diff --git a/src/kiota/appsettings.json b/src/kiota/appsettings.json index 4109cdd911..12b9745d0e 100644 --- a/src/kiota/appsettings.json +++ b/src/kiota/appsettings.json @@ -318,7 +318,7 @@ "Dependencies": [ { "Name": "microsoft_kiota_abstractions", - "Version": "0.15.1", + "Version": "0.16.0", "Type": "Abstractions" }, { @@ -328,7 +328,7 @@ }, { "Name": "microsoft_kiota_serialization_json", - "Version": "0.10.0", + "Version": "0.11.0", "Type": "Serialization" }, { diff --git a/tests/Kiota.Builder.Tests/Writers/Ruby/CodeMethodWriterTests.cs b/tests/Kiota.Builder.Tests/Writers/Ruby/CodeMethodWriterTests.cs index 67e67033a2..23ef6f987b 100644 --- a/tests/Kiota.Builder.Tests/Writers/Ruby/CodeMethodWriterTests.cs +++ b/tests/Kiota.Builder.Tests/Writers/Ruby/CodeMethodWriterTests.cs @@ -1,4 +1,5 @@ using System; +using System.Collections.Generic; using System.IO; using System.Linq; @@ -1498,4 +1499,254 @@ public void WritesRequestGeneratorContentTypeQuotes() var result = tw.ToString(); Assert.Contains("'application/json; profile=\\'CamelCase\\''", result); } + private void AddUnionTypeWrapper() + { + setup(); + var complexType1 = root.AddClass(new CodeClass { Name = "ComplexType1", Kind = CodeClassKind.Model }).First(); + var complexType2 = root.AddClass(new CodeClass { Name = "ComplexType2", Kind = CodeClassKind.Model }).First(); + parentClass.OriginalComposedType = new CodeUnionType { Name = "UnionType" }; + parentClass.DiscriminatorInformation.DiscriminatorPropertyName = "@odata.type"; + parentClass.DiscriminatorInformation.AddDiscriminatorMapping("#kiota.complexType1", new CodeType { Name = "ComplexType1", TypeDefinition = complexType1 }); + parentClass.DiscriminatorInformation.AddDiscriminatorMapping("#kiota.complexType2", new CodeType { Name = "ComplexType2", TypeDefinition = complexType2, CollectionKind = CodeTypeBase.CodeTypeCollectionKind.Complex }); + parentClass.AddProperty(new CodeProperty { Name = "complexType1Value", Kind = CodePropertyKind.Custom, Type = new CodeType { Name = "ComplexType1", TypeDefinition = complexType1 } }); + parentClass.AddProperty(new CodeProperty { Name = "complexType2Value", Kind = CodePropertyKind.Custom, Type = new CodeType { Name = "ComplexType2", TypeDefinition = complexType2, CollectionKind = CodeTypeBase.CodeTypeCollectionKind.Complex } }); + parentClass.AddProperty(new CodeProperty { Name = "stringValue", Kind = CodePropertyKind.Custom, Type = new CodeType { Name = "string" } }); + } + private void AddIntersectionTypeWrapper() + { + setup(); + var complexType1 = root.AddClass(new CodeClass { Name = "ComplexType1", Kind = CodeClassKind.Model }).First(); + var complexType2 = root.AddClass(new CodeClass { Name = "ComplexType2", Kind = CodeClassKind.Model }).First(); + var complexType3 = root.AddClass(new CodeClass { Name = "ComplexType3", Kind = CodeClassKind.Model }).First(); + parentClass.OriginalComposedType = new CodeIntersectionType { Name = "IntersectionType" }; + parentClass.DiscriminatorInformation.DiscriminatorPropertyName = "@odata.type"; + parentClass.DiscriminatorInformation.AddDiscriminatorMapping("#kiota.complexType1", new CodeType { Name = "ComplexType1", TypeDefinition = complexType1 }); + parentClass.DiscriminatorInformation.AddDiscriminatorMapping("#kiota.complexType2", new CodeType { Name = "ComplexType2", TypeDefinition = complexType2, CollectionKind = CodeTypeBase.CodeTypeCollectionKind.Complex }); + parentClass.DiscriminatorInformation.AddDiscriminatorMapping("#kiota.complexType3", new CodeType { Name = "ComplexType3", TypeDefinition = complexType3 }); + parentClass.AddProperty(new CodeProperty { Name = "complexType1Value", Kind = CodePropertyKind.Custom, Type = new CodeType { Name = "ComplexType1", TypeDefinition = complexType1 } }); + parentClass.AddProperty(new CodeProperty { Name = "complexType2Value", Kind = CodePropertyKind.Custom, Type = new CodeType { Name = "ComplexType2", TypeDefinition = complexType2, CollectionKind = CodeTypeBase.CodeTypeCollectionKind.Complex } }); + parentClass.AddProperty(new CodeProperty { Name = "complexType3Value", Kind = CodePropertyKind.Custom, Type = new CodeType { Name = "ComplexType3", TypeDefinition = complexType3 } }); + parentClass.AddProperty(new CodeProperty { Name = "stringValue", Kind = CodePropertyKind.Custom, Type = new CodeType { Name = "string" } }); + } + [Fact] + public void WritesIntersectionFactoryBodyGivesEachPropertyItsOwnVariable() + { + // regression: a shared `val` is reassigned inside the previous branch of the + // if/elsif chain, so every property after the first is silently never deserialized + setup(); + var complexType1 = root.AddClass(new CodeClass { Name = "ComplexType1", Kind = CodeClassKind.Model }).First(); + parentClass.OriginalComposedType = new CodeIntersectionType { Name = "IntersectionType" }; + parentClass.AddProperty(new CodeProperty { Name = "complexType1Value", Kind = CodePropertyKind.Custom, Type = new CodeType { Name = "ComplexType1", TypeDefinition = complexType1 } }); + parentClass.AddProperty(new CodeProperty { Name = "stringValue", Kind = CodePropertyKind.Custom, Type = new CodeType { Name = "string" } }); + parentClass.AddProperty(new CodeProperty { Name = "numberValue", Kind = CodePropertyKind.Custom, Type = new CodeType { Name = "integer" } }); + method.Kind = CodeMethodKind.Factory; + method.ReturnType = new CodeType { Name = "ParentClass", TypeDefinition = parentClass }; + method.AddParameter(new CodeParameter { Kind = CodeParameterKind.ParseNode, Name = "parseNode", Type = new CodeType { Name = "ParseNode" } }); + writer.Write(method); + var result = tw.ToString(); + + // every parse happens up-front, so no assignment can be swallowed by a branch + Assert.Contains("val_string_value = parse_node.", result); + Assert.Contains("val_number_value = parse_node.", result); + // and each branch tests its own variable rather than repeating one condition + Assert.Contains("val_string_value.nil?", result); + Assert.Contains("val_number_value.nil?", result); + Assert.Contains("elsif !val_", result); + Assert.DoesNotContain("if !val.nil?", result); + + // the parses must precede the chain, never sit inside it + var firstBranch = result.IndexOf("if !val_", StringComparison.Ordinal); + Assert.True(result.IndexOf("val_string_value = parse_node.", StringComparison.Ordinal) < firstBranch); + Assert.True(result.IndexOf("val_number_value = parse_node.", StringComparison.Ordinal) < firstBranch); + + AssertBalancedBlocks(result); + } + [Fact] + public void WritesUnionFactoryBody() + { + AddUnionTypeWrapper(); + method.Kind = CodeMethodKind.Factory; + method.ReturnType = new CodeType { Name = "ParentClass", TypeDefinition = parentClass }; + method.AddParameter(new CodeParameter { Kind = CodeParameterKind.ParseNode, Name = "parseNode", Type = new CodeType { Name = "ParseNode" } }); + writer.Write(method); + var result = tw.ToString(); + Assert.Contains("result = ParentClass.new", result); + Assert.Contains("mapping_value_node", result); + Assert.Contains("ComplexType1.new", result); + Assert.Contains("return result", result); + Assert.DoesNotContain("ComplexType2.new", result); + AssertBalancedBlocks(result); + } + [Fact] + public void WritesUnionFactoryBodySkipsEmptyMappings() + { + setup(); + var complexType1 = root.AddClass(new CodeClass { Name = "ComplexType1", Kind = CodeClassKind.Model }).First(); + parentClass.OriginalComposedType = new CodeUnionType { Name = "UnionType" }; + parentClass.DiscriminatorInformation.DiscriminatorPropertyName = ""; + parentClass.AddProperty(new CodeProperty { Name = "complexType1Value", Kind = CodePropertyKind.Custom, Type = new CodeType { Name = "ComplexType1", TypeDefinition = complexType1 } }); + method.Kind = CodeMethodKind.Factory; + method.ReturnType = new CodeType { Name = "ParentClass", TypeDefinition = parentClass }; + method.AddParameter(new CodeParameter { Kind = CodeParameterKind.ParseNode, Name = "parseNode", Type = new CodeType { Name = "ParseNode" } }); + writer.Write(method); + var result = tw.ToString(); + Assert.Contains("result = ParentClass.new", result); + Assert.DoesNotContain("mapping_value_node", result); + Assert.DoesNotContain("unless", result); + Assert.Contains("return result", result); + } + [Fact] + public void WritesIntersectionFactoryBody() + { + AddIntersectionTypeWrapper(); + method.Kind = CodeMethodKind.Factory; + method.ReturnType = new CodeType { Name = "ParentClass", TypeDefinition = parentClass }; + method.AddParameter(new CodeParameter { Kind = CodeParameterKind.ParseNode, Name = "parseNode", Type = new CodeType { Name = "ParseNode" } }); + writer.Write(method); + var result = tw.ToString(); + Assert.Contains("result = ParentClass.new", result); + Assert.DoesNotContain("mapping_value_node", result); + Assert.Contains("ComplexType1.new", result); + Assert.Contains("ComplexType3.new", result); + Assert.Contains("else", result); + Assert.Contains("return result", result); + } + [Fact] + public void WritesIntersectionFactoryBodyOnlyComplex() + { + setup(); + var complexType1 = root.AddClass(new CodeClass { Name = "ComplexType1", Kind = CodeClassKind.Model }).First(); + var complexType2 = root.AddClass(new CodeClass { Name = "ComplexType2", Kind = CodeClassKind.Model }).First(); + parentClass.OriginalComposedType = new CodeIntersectionType { Name = "IntersectionType" }; + parentClass.AddProperty(new CodeProperty { Name = "complexType1Value", Kind = CodePropertyKind.Custom, Type = new CodeType { Name = "ComplexType1", TypeDefinition = complexType1 } }); + parentClass.AddProperty(new CodeProperty { Name = "complexType2Value", Kind = CodePropertyKind.Custom, Type = new CodeType { Name = "ComplexType2", TypeDefinition = complexType2 } }); + method.Kind = CodeMethodKind.Factory; + method.ReturnType = new CodeType { Name = "ParentClass", TypeDefinition = parentClass }; + method.AddParameter(new CodeParameter { Kind = CodeParameterKind.ParseNode, Name = "parseNode", Type = new CodeType { Name = "ParseNode" } }); + writer.Write(method); + var result = tw.ToString(); + Assert.Contains("ComplexType1.new", result); + Assert.Contains("ComplexType2.new", result); + Assert.DoesNotContain("else", result); + Assert.DoesNotContain("if !", result); + } + [Fact] + public void WritesUnionSerializerBody() + { + AddUnionTypeWrapper(); + method.Kind = CodeMethodKind.Serializer; + method.AddParameter(new CodeParameter { Kind = CodeParameterKind.Serializer, Name = "writer", Type = new CodeType { Name = "SerializationWriter" } }); + writer.Write(method); + var result = tw.ToString(); + Assert.DoesNotContain("super", result); + Assert.Contains("complex_type1_value", result); + Assert.Contains("write_object_value", result); + Assert.Contains("write_string_value", result); + Assert.Contains("elsif", result); + AssertBalancedBlocks(result); + } + [Fact] + public void WritesIntersectionSerializerBody() + { + AddIntersectionTypeWrapper(); + method.Kind = CodeMethodKind.Serializer; + method.AddParameter(new CodeParameter { Kind = CodeParameterKind.Serializer, Name = "writer", Type = new CodeType { Name = "SerializationWriter" } }); + writer.Write(method); + var result = tw.ToString(); + Assert.DoesNotContain("super", result); + Assert.Contains("write_object_value(nil, @complex_type1_value, @complex_type3_value)", result); + Assert.Contains("write_string_value", result); + Assert.Contains("else", result); + AssertBalancedBlocks(result); + } + [Fact] + public void WritesIntersectionSerializerBodyOnlyComplex() + { + setup(); + var complexType1 = root.AddClass(new CodeClass { Name = "ComplexType1", Kind = CodeClassKind.Model }).First(); + var complexType2 = root.AddClass(new CodeClass { Name = "ComplexType2", Kind = CodeClassKind.Model }).First(); + parentClass.OriginalComposedType = new CodeIntersectionType { Name = "IntersectionType" }; + parentClass.AddProperty(new CodeProperty { Name = "complexType1Value", Kind = CodePropertyKind.Custom, Type = new CodeType { Name = "ComplexType1", TypeDefinition = complexType1 } }); + parentClass.AddProperty(new CodeProperty { Name = "complexType2Value", Kind = CodePropertyKind.Custom, Type = new CodeType { Name = "ComplexType2", TypeDefinition = complexType2 } }); + method.Kind = CodeMethodKind.Serializer; + method.AddParameter(new CodeParameter { Kind = CodeParameterKind.Serializer, Name = "writer", Type = new CodeType { Name = "SerializationWriter" } }); + writer.Write(method); + var result = tw.ToString(); + Assert.Contains("write_object_value(nil, @complex_type1_value, @complex_type2_value)", result); + Assert.DoesNotContain("if !", result); + Assert.DoesNotContain("else", result); + AssertBalancedBlocks(result); + } + /// + /// Asserts every block opener has a matching `end` AND that the `end` is written at the + /// same column as its opener. Counting alone is not enough: a stray DecreaseIndent() + /// before CloseBlock("end") still balances the count while shifting the `end` (and every + /// later one in the file) a level to the left. + /// + private static void AssertBalancedBlocks(string generatedRubyBody) + { + static int IndentOf(string raw) => raw.Length - raw.TrimStart().Length; + var openBlocks = new Stack<(string Line, int Indent)>(); + foreach (var rawLine in generatedRubyBody.Split('\n')) + { + var line = rawLine.Trim(); + if (line.Length == 0 || line.StartsWith("#", StringComparison.Ordinal)) continue; + if (line.Equals("end", StringComparison.Ordinal)) + { + Assert.True(openBlocks.Count > 0, $"unmatched `end` in:\n{generatedRubyBody}"); + var opener = openBlocks.Pop(); + Assert.True(opener.Indent == IndentOf(rawLine), + $"`end` at column {IndentOf(rawLine)} does not align with `{opener.Line}` at column {opener.Indent} in:\n{generatedRubyBody}"); + continue; + } + // elsif/else continue the current block rather than opening a new one + if (line.StartsWith("elsif ", StringComparison.Ordinal) || line.Equals("else", StringComparison.Ordinal)) + { + Assert.True(openBlocks.Count > 0, $"`{line}` outside any block in:\n{generatedRubyBody}"); + Assert.True(openBlocks.Peek().Indent == IndentOf(rawLine), + $"`{line}` at column {IndentOf(rawLine)} does not align with its opener at column {openBlocks.Peek().Indent} in:\n{generatedRubyBody}"); + continue; + } + if (line.StartsWith("def ", StringComparison.Ordinal) || + line.StartsWith("if ", StringComparison.Ordinal) || + line.StartsWith("unless ", StringComparison.Ordinal) || + line.StartsWith("case ", StringComparison.Ordinal)) + openBlocks.Push((line, IndentOf(rawLine))); + } + Assert.True(openBlocks.Count == 0, + $"unclosed block `{(openBlocks.Count > 0 ? openBlocks.Peek().Line : string.Empty)}` in:\n{generatedRubyBody}"); + } + [Fact] + public void WritesUnionDeserializerBody() + { + AddUnionTypeWrapper(); + method.Kind = CodeMethodKind.Deserializer; + writer.Write(method); + var result = tw.ToString(); + Assert.DoesNotContain("super", result); + Assert.Contains("@complex_type1_value.get_field_deserializers()", result); + Assert.DoesNotContain("complex_type2_value", result); + Assert.Contains("return {}", result); + AssertBalancedBlocks(result); + } + [Fact] + public void WritesIntersectionDeserializerBody() + { + AddIntersectionTypeWrapper(); + method.Kind = CodeMethodKind.Deserializer; + writer.Write(method); + var result = tw.ToString(); + Assert.DoesNotContain("super", result); + Assert.Contains("merge_deserializers_for_intersection_wrapper(@complex_type1_value, @complex_type3_value)", result); + Assert.DoesNotContain("complex_type2_value", result); + Assert.Contains("return {}", result); + AssertBalancedBlocks(result); + } + [Fact] + public void ThrowsOnComposedTypeMarker() + { + setup(); + method.Kind = CodeMethodKind.ComposedTypeMarker; + Assert.Throws(() => writer.Write(method)); + } } From 3a0f109ddf132ea0ba865e642784c7f3b4ddec32 Mon Sep 17 00:00:00 2001 From: andreatp Date: Mon, 24 Aug 2026 17:47:33 +0100 Subject: [PATCH 2/5] fix(ruby): drop blank mixin line and reserve `initialize` Two independent sources of invalid or misleading generated Ruby: - CodeClassDeclarationWriter wrote the mixin line unconditionally, so a class with no Implements got `GetIndent() + ""` as the first line of its body -- a whitespace-only line (and trailing whitespace) in every request builder, query-parameter class and composed-type wrapper. - `initialize` was not a reserved name, so an API member of that name (e.g. Twitter's /2/media/upload/initialize) became a request-builder property and emitted a second `def initialize` in a class that already had one, returning a value and skipping super. Re-enable the cops these were masking: Layout/EmptyLinesAroundClassBody, Lint/DuplicateMethods, Lint/ReturnInVoidContext, Lint/MissingSuper. --- it/ruby/.rubocop.yml | 11 ----------- .../Refiners/RubyReservedNamesProvider.cs | 2 ++ .../Writers/Ruby/CodeClassDeclarationWriter.cs | 5 +++-- .../Refiners/RubyLanguageRefinerTests.cs | 14 ++++++++++++++ .../Ruby/CodeClassDeclarationWriterTests.cs | 11 +++++++++++ 5 files changed, 30 insertions(+), 13 deletions(-) diff --git a/it/ruby/.rubocop.yml b/it/ruby/.rubocop.yml index 19c218dbde..bd7d6fd0a1 100644 --- a/it/ruby/.rubocop.yml +++ b/it/ruby/.rubocop.yml @@ -135,27 +135,16 @@ Metrics/CyclomaticComplexity: Lint/UselessMethodDefinition: Enabled: false -# Generated class declarations may have an empty line after the class keyword -Layout/EmptyLinesAroundClassBody: - Enabled: false # Generated composed type methods can have high perceived complexity Metrics/PerceivedComplexity: Enabled: false -# Generated code may duplicate method definitions in certain request builder patterns -Lint/DuplicateMethods: - Enabled: false -# Generated constructors may return values or skip super calls -Lint/ReturnInVoidContext: - Enabled: false -Lint/MissingSuper: - Enabled: false diff --git a/src/Kiota.Builder/Refiners/RubyReservedNamesProvider.cs b/src/Kiota.Builder/Refiners/RubyReservedNamesProvider.cs index a692cefc63..72cddb7d47 100644 --- a/src/Kiota.Builder/Refiners/RubyReservedNamesProvider.cs +++ b/src/Kiota.Builder/Refiners/RubyReservedNamesProvider.cs @@ -45,6 +45,8 @@ public class RubyReservedNamesProvider : IReservedNamesProvider "BaseRequestBuilder", "ObjectId", "object_id", + // Object protocol: a generated member of this name would redefine the constructor + "initialize", }); public HashSet ReservedNames => _reservedNames.Value; } diff --git a/src/Kiota.Builder/Writers/Ruby/CodeClassDeclarationWriter.cs b/src/Kiota.Builder/Writers/Ruby/CodeClassDeclarationWriter.cs index 8027cf3ab1..d13bd86c9d 100644 --- a/src/Kiota.Builder/Writers/Ruby/CodeClassDeclarationWriter.cs +++ b/src/Kiota.Builder/Writers/Ruby/CodeClassDeclarationWriter.cs @@ -57,7 +57,8 @@ public override void WriteCodeElement(ClassDeclaration codeElement, LanguageWrit if (codeElement.Parent is CodeClass parentClass) conventions.WriteShortDescription(parentClass, writer); writer.StartBlock($"class {codeElement.Name.ToFirstCharacterUpperCase()}{derivation}"); - var mixins = !codeElement.Implements.Any() ? string.Empty : $"include {codeElement.Implements.Select(static x => x.Name).Aggregate(static (x, y) => x + ", " + y)}"; - writer.WriteLine($"{mixins}"); + // writing an empty mixin line would leave a blank (indent-only) first line in the class body + if (codeElement.Implements.Any()) + writer.WriteLine($"include {codeElement.Implements.Select(static x => x.Name).Aggregate(static (x, y) => x + ", " + y)}"); } } diff --git a/tests/Kiota.Builder.Tests/Refiners/RubyLanguageRefinerTests.cs b/tests/Kiota.Builder.Tests/Refiners/RubyLanguageRefinerTests.cs index 213d8fbb1e..9064b892e8 100644 --- a/tests/Kiota.Builder.Tests/Refiners/RubyLanguageRefinerTests.cs +++ b/tests/Kiota.Builder.Tests/Refiners/RubyLanguageRefinerTests.cs @@ -123,6 +123,20 @@ public async Task EscapesReservedKeywordsAsync() Assert.Contains("escaped", model.Name); } [Fact] + public async Task EscapesInitializeAsync() + { + // an API member named `initialize` would redefine the Ruby constructor, + // yielding a duplicate `def initialize` that returns a value and skips super + var model = root.AddClass(new CodeClass + { + Name = "initialize", + Kind = CodeClassKind.Model + }).First(); + await ILanguageRefiner.RefineAsync(new GenerationConfiguration { Language = GenerationLanguage.Ruby, ClientNamespaceName = graphNS.Name }, root, cancellationToken: TestContext.Current.CancellationToken); + Assert.NotEqual("initialize", model.Name); + Assert.Contains("escaped", model.Name); + } + [Fact] public async Task ConvertEnumsToPascalCaseAsync() { var model = root.AddEnum(new CodeEnum diff --git a/tests/Kiota.Builder.Tests/Writers/Ruby/CodeClassDeclarationWriterTests.cs b/tests/Kiota.Builder.Tests/Writers/Ruby/CodeClassDeclarationWriterTests.cs index e1d04678ab..0acd957036 100644 --- a/tests/Kiota.Builder.Tests/Writers/Ruby/CodeClassDeclarationWriterTests.cs +++ b/tests/Kiota.Builder.Tests/Writers/Ruby/CodeClassDeclarationWriterTests.cs @@ -46,6 +46,17 @@ public void WritesSimpleDeclaration() Assert.Contains("class", result); } [Fact] + public void DoesNotWriteBlankLineWhenThereAreNoMixins() + { + // an empty mixin line leaves an indent-only first line in the class body + // (RuboCop Layout/EmptyLinesAroundClassBody) + codeElementWriter.WriteCodeElement(parentClass.StartBlock, writer); + var result = tw.ToString(); + // the bug emitted GetIndent() + "" -> a line of pure whitespace + Assert.DoesNotContain(result.Split(Environment.NewLine), + static l => l.Length > 0 && l.Trim().Length == 0); + } + [Fact] public void WritesImplementation() { var declaration = parentClass.StartBlock; From df8e006d026ae0b5f5d6e9d338255ee5dd91d13a Mon Sep 17 00:00:00 2001 From: andreatp Date: Tue, 25 Aug 2026 11:57:02 +0100 Subject: [PATCH 3/5] chore(ruby): move runtime gems to 0.19.0 --- CHANGELOG.md | 5 ++++- it/config.json | 20 +++++++++++++------- it/ruby/.rubocop.yml | 9 --------- it/ruby/Gemfile | 8 ++++---- src/kiota/appsettings.json | 8 ++++---- 5 files changed, 25 insertions(+), 25 deletions(-) diff --git a/CHANGELOG.md b/CHANGELOG.md index 0860fc9240..a3a71a88dd 100644 --- a/CHANGELOG.md +++ b/CHANGELOG.md @@ -18,7 +18,10 @@ and this project adheres to [Semantic Versioning](https://semver.org/spec/v2.0.0 ### Changed - Fixed defaults primitive default-value handling during code generation for all languages -- Ruby: added composed type (union/intersection) support for factory methods, serializers, and deserializers. Disabled inner classes for composed type wrappers. Un-suppresses the Twitter integration test. Requires `microsoft_kiota_abstractions` 0.16.0 and `microsoft_kiota_serialization_json` 0.11.0. [kiota-abstractions-ruby#73](https://github.com/microsoft/kiota-abstractions-ruby/issues/73) [#1816](https://github.com/microsoft/kiota/issues/1816) +- Ruby: added composed type (union/intersection) support to the factory, serializer and deserializer bodies, and stopped emitting composed type wrappers as inner classes. Un-suppresses the Twitter integration test. [kiota-abstractions-ruby#73](https://github.com/microsoft/kiota-abstractions-ruby/issues/73) [#1816](https://github.com/microsoft/kiota/issues/1816) +- Ruby: `initialize` is now a reserved name. An API member called `initialize` previously generated a second `def initialize`, redefining the constructor; it is now escaped. +- Ruby: removed the blank line emitted at the start of every generated class body. +- Ruby: moved the runtime gem dependencies to 0.19.0, the first release able to serialize a primitive composed type member and to tell which member a payload holds. - Fixed plugin manifest generation to omit unsafe `oauth_card_path` file references that could resolve outside the plugin package. - Bumped `Microsoft.OpenApi` and `Microsoft.OpenApi.YamlReader` to 3.10.0. - Numeric scalar unions with a format (e.g. `type: ["integer", "string"]` with `format: int32`, as emitted by ASP.NET Core's OpenAPI 3.1 generator under System.Text.Json's default `JsonNumberHandling.AllowReadingFromString`) now map to the numeric type instead of degrading to `UntypedNode`. [#6541](https://github.com/microsoft/kiota/issues/6541) diff --git a/it/config.json b/it/config.json index 2b9e6351d8..d7d6d57e8c 100644 --- a/it/config.json +++ b/it/config.json @@ -16,7 +16,7 @@ "Suppressions": [ { "Language": "ruby", - "Rationale": "Generated client emits a require_relative to a models file that is never generated (e.g. models/with_path), causing a LoadError. Pre-existing naming/require bug, unrelated to composed types." + "Rationale": "https://github.com/microsoft/kiota-abstractions-ruby/issues/73" }, { "Language": "dart", @@ -62,7 +62,8 @@ } ] }, - "https://raw.githubusercontent.com/googlemaps/openapi-specification/refs/tags/v1.22.5/dist/google-maps-platform-openapi3.yml": {}, + "https://raw.githubusercontent.com/googlemaps/openapi-specification/refs/tags/v1.22.5/dist/google-maps-platform-openapi3.yml": { + }, "https://developers.pipedrive.com/docs/api/v1/openapi.yaml": { "ExcludePatterns": [ { @@ -93,14 +94,14 @@ }, "https://raw.githubusercontent.com/stripe/openapi/refs/heads/master/latest/openapi.spec3.json": { "Suppressions": [ - { - "Language": "ruby", - "Rationale": "Schema names containing dots produce invalid Ruby class names (e.g. 'class S.v2.coreAccount...'), causing SyntaxErrors. Pre-existing naming bug, unrelated to composed types." - }, { "Language": "java", "Rationale": "https://github.com/microsoft/kiota/issues/2842" }, + { + "Language": "ruby", + "Rationale": "https://github.com/microsoft/kiota/issues/1816" + }, { "Language": "php", "Rationale": "https://github.com/microsoft/kiota/issues/5354" @@ -110,6 +111,10 @@ { "Language": "java", "Rationale": "https://github.com/microsoft/kiota/issues/2842" + }, + { + "Language": "ruby", + "Rationale": "https://github.com/microsoft/kiota/issues/1816" } ] }, @@ -137,7 +142,8 @@ } ] }, - "https://raw.githubusercontent.com/docusign/OpenAPI-Specifications/refs/heads/master/esignature.rest.swagger-v2.1.json": {}, + "https://raw.githubusercontent.com/docusign/OpenAPI-Specifications/refs/heads/master/esignature.rest.swagger-v2.1.json": { + }, "https://api.twitter.com/2/openapi.json": { "Suppressions": [ { diff --git a/it/ruby/.rubocop.yml b/it/ruby/.rubocop.yml index bd7d6fd0a1..ff66ed93af 100644 --- a/it/ruby/.rubocop.yml +++ b/it/ruby/.rubocop.yml @@ -135,19 +135,10 @@ Metrics/CyclomaticComplexity: Lint/UselessMethodDefinition: Enabled: false - # Generated composed type methods can have high perceived complexity Metrics/PerceivedComplexity: Enabled: false - - - - - - - - # Generated composed types use if !x.nil? for elsif compatibility Style/NegatedIf: Enabled: false diff --git a/it/ruby/Gemfile b/it/ruby/Gemfile index 997771ac8f..ca025efd9d 100644 --- a/it/ruby/Gemfile +++ b/it/ruby/Gemfile @@ -11,10 +11,10 @@ gem "rspec", "~> 3.0" gem "rubocop", "~> 1.21" -gem "microsoft_kiota_abstractions", "~> 0.16.0" +gem "microsoft_kiota_abstractions", "~> 0.19.0" -gem "microsoft_kiota_faraday", "~> 0.16.0" +gem "microsoft_kiota_faraday", "~> 0.19.0" -gem "microsoft_kiota_serialization_json", "~> 0.11.0" +gem "microsoft_kiota_serialization_json", "~> 0.19.0" -gem "microsoft_kiota_authentication_oauth", "~> 0.9.0" +gem "microsoft_kiota_authentication_oauth", "~> 0.19.0" diff --git a/src/kiota/appsettings.json b/src/kiota/appsettings.json index 12b9745d0e..8bf8e4f0c8 100644 --- a/src/kiota/appsettings.json +++ b/src/kiota/appsettings.json @@ -318,22 +318,22 @@ "Dependencies": [ { "Name": "microsoft_kiota_abstractions", - "Version": "0.16.0", + "Version": "0.19.0", "Type": "Abstractions" }, { "Name": "microsoft_kiota_faraday", - "Version": "0.16.0", + "Version": "0.19.0", "Type": "Http" }, { "Name": "microsoft_kiota_serialization_json", - "Version": "0.11.0", + "Version": "0.19.0", "Type": "Serialization" }, { "Name": "microsoft_kiota_authentication_oauth", - "Version": "0.9.0", + "Version": "0.19.0", "Type": "Authentication" } ], From 90b4193368be4eac0191e8939adccb0bfe8442f9 Mon Sep 17 00:00:00 2001 From: andreatp Date: Wed, 26 Aug 2026 12:32:03 +0100 Subject: [PATCH 4/5] review: guard nil discriminator, fix write_object_value nil-drop, harden tests --- .../Writers/Ruby/CodeMethodWriter.cs | 9 ++++-- .../Refiners/RubyLanguageRefinerTests.cs | 18 +++++++++-- .../Writers/Ruby/CodeMethodWriterTests.cs | 30 +++++++++++++++++-- 3 files changed, 51 insertions(+), 6 deletions(-) diff --git a/src/Kiota.Builder/Writers/Ruby/CodeMethodWriter.cs b/src/Kiota.Builder/Writers/Ruby/CodeMethodWriter.cs index 6e2d9c122b..cfb8f1c229 100644 --- a/src/Kiota.Builder/Writers/Ruby/CodeMethodWriter.cs +++ b/src/Kiota.Builder/Writers/Ruby/CodeMethodWriter.cs @@ -141,7 +141,9 @@ private void WriteFactoryMethodBodyForUnionModel(CodeParameter parseNodeParamete var elseIfPrefix = string.Empty; foreach (var (property, mappedKey) in complexPropertiesWithMappings) { - writer.StartBlock($"{elseIfPrefix}if {DiscriminatorMappingVarName}.downcase == \"{RubyConventionService.SanitizeRubyDoubleQuoteLiteral(mappedKey)}\".downcase"); + // safe navigation: a ParseNode may yield a nil discriminator value, and the + // inherited factory's `case` path tolerates that, so this one must too + writer.StartBlock($"{elseIfPrefix}if {DiscriminatorMappingVarName}&.downcase == \"{RubyConventionService.SanitizeRubyDoubleQuoteLiteral(mappedKey)}\".downcase"); writer.WriteLine($"result.{property.Name.ToSnakeCase()} = {property.Type.Name.ToFirstCharacterUpperCase()}.new"); writer.DecreaseIndent(); elseIfPrefix = "els"; @@ -577,8 +579,11 @@ private void WriteSerializerBodyForIntersectionModel(CodeClass parentClass, Lang { if (nonComplexProperties.Length > 0) writer.StartBlock("else"); + // write_object_value returns early when its first argument is nil, which would drop + // every remaining member, so compact the list and skip the call when nothing is set var complexPropNames = string.Join(", ", complexProperties.Select(x => $"@{x.Name.ToSnakeCase()}")); - writer.WriteLine($"writer.write_object_value(nil, {complexPropNames})"); + writer.WriteLine($"composed_values = [{complexPropNames}].compact"); + writer.WriteLine("writer.write_object_value(nil, *composed_values) unless composed_values.empty?"); if (nonComplexProperties.Length > 0) writer.DecreaseIndent(); } diff --git a/tests/Kiota.Builder.Tests/Refiners/RubyLanguageRefinerTests.cs b/tests/Kiota.Builder.Tests/Refiners/RubyLanguageRefinerTests.cs index 9064b892e8..b59a00b62d 100644 --- a/tests/Kiota.Builder.Tests/Refiners/RubyLanguageRefinerTests.cs +++ b/tests/Kiota.Builder.Tests/Refiners/RubyLanguageRefinerTests.cs @@ -125,14 +125,28 @@ public async Task EscapesReservedKeywordsAsync() [Fact] public async Task EscapesInitializeAsync() { - // an API member named `initialize` would redefine the Ruby constructor, - // yielding a duplicate `def initialize` that returns a value and skips super + // the real-world case is an API path segment (e.g. /media/upload/initialize) becoming a + // request-builder property, which emitted a second `def initialize` in a class that + // already had one -- returning a value and skipping super + var requestBuilder = root.AddClass(new CodeClass + { + Name = "uploadRequestBuilder", + Kind = CodeClassKind.RequestBuilder + }).First(); + var navProperty = requestBuilder.AddProperty(new CodeProperty + { + Name = "initialize", + Kind = CodePropertyKind.RequestBuilder, + Type = new CodeType { Name = "initializeRequestBuilder" }, + }).First(); var model = root.AddClass(new CodeClass { Name = "initialize", Kind = CodeClassKind.Model }).First(); await ILanguageRefiner.RefineAsync(new GenerationConfiguration { Language = GenerationLanguage.Ruby, ClientNamespaceName = graphNS.Name }, root, cancellationToken: TestContext.Current.CancellationToken); + Assert.NotEqual("initialize", navProperty.Name); + Assert.Contains("escaped", navProperty.Name); Assert.NotEqual("initialize", model.Name); Assert.Contains("escaped", model.Name); } diff --git a/tests/Kiota.Builder.Tests/Writers/Ruby/CodeMethodWriterTests.cs b/tests/Kiota.Builder.Tests/Writers/Ruby/CodeMethodWriterTests.cs index 23ef6f987b..ebd90fc09f 100644 --- a/tests/Kiota.Builder.Tests/Writers/Ruby/CodeMethodWriterTests.cs +++ b/tests/Kiota.Builder.Tests/Writers/Ruby/CodeMethodWriterTests.cs @@ -1562,6 +1562,30 @@ public void WritesIntersectionFactoryBodyGivesEachPropertyItsOwnVariable() AssertBalancedBlocks(result); } [Fact] + public void EscapesUnionFactoryDiscriminatorLiterals() + { + // the union discriminator path interpolates schema-controlled strings into Ruby + // literals; regression guard against reintroducing raw injection + setup(); + var complexType1 = root.AddClass(new CodeClass { Name = "ComplexType1", Kind = CodeClassKind.Model }).First(); + parentClass.OriginalComposedType = new CodeUnionType { Name = "UnionType" }; + parentClass.DiscriminatorInformation.DiscriminatorPropertyName = "@odata.ty\"pe\nx"; + parentClass.DiscriminatorInformation.AddDiscriminatorMapping("ns.chi\"ld\nmodel#x", + new CodeType { Name = "ComplexType1", TypeDefinition = complexType1 }); + parentClass.AddProperty(new CodeProperty { Name = "complexType1Value", Kind = CodePropertyKind.Custom, Type = new CodeType { Name = "ComplexType1", TypeDefinition = complexType1 } }); + method.Kind = CodeMethodKind.Factory; + method.ReturnType = new CodeType { Name = "ParentClass", TypeDefinition = parentClass }; + method.AddParameter(new CodeParameter { Kind = CodeParameterKind.ParseNode, Name = "parseNode", Type = new CodeType { Name = "ParseNode" } }); + writer.Write(method); + var result = tw.ToString(); + // quotes, newlines and Ruby interpolation markers must all arrive escaped + Assert.Contains("\\\"", result); + Assert.Contains("\\n", result); + Assert.Contains("\\#", result); + Assert.DoesNotContain("ns.chi\"ld", result); + AssertBalancedBlocks(result); + } + [Fact] public void WritesUnionFactoryBody() { AddUnionTypeWrapper(); @@ -1654,7 +1678,8 @@ public void WritesIntersectionSerializerBody() writer.Write(method); var result = tw.ToString(); Assert.DoesNotContain("super", result); - Assert.Contains("write_object_value(nil, @complex_type1_value, @complex_type3_value)", result); + Assert.Contains("composed_values = [@complex_type1_value, @complex_type3_value].compact", result); + Assert.Contains("writer.write_object_value(nil, *composed_values) unless composed_values.empty?", result); Assert.Contains("write_string_value", result); Assert.Contains("else", result); AssertBalancedBlocks(result); @@ -1672,7 +1697,8 @@ public void WritesIntersectionSerializerBodyOnlyComplex() method.AddParameter(new CodeParameter { Kind = CodeParameterKind.Serializer, Name = "writer", Type = new CodeType { Name = "SerializationWriter" } }); writer.Write(method); var result = tw.ToString(); - Assert.Contains("write_object_value(nil, @complex_type1_value, @complex_type2_value)", result); + Assert.Contains("composed_values = [@complex_type1_value, @complex_type2_value].compact", result); + Assert.Contains("writer.write_object_value(nil, *composed_values) unless composed_values.empty?", result); Assert.DoesNotContain("if !", result); Assert.DoesNotContain("else", result); AssertBalancedBlocks(result); From da1156af3a7e6a4c50acd9b55ac3b7f5f62d73e8 Mon Sep 17 00:00:00 2001 From: andreatp Date: Thu, 27 Aug 2026 17:47:06 +0100 Subject: [PATCH 5/5] fix(ruby): stop emitting a negated if for a single-member composed type - a lone branch has no elsif to chain to, so it reads as `unless` or a guard clause - drops the Style/NegatedIf and Style/GuardClause exclusions this made unnecessary --- it/ruby/.rubocop.yml | 14 +++----- .../Writers/Ruby/CodeMethodWriter.cs | 25 ++++++++++++- .../Writers/Ruby/CodeMethodWriterTests.cs | 36 +++++++++++++++++++ 3 files changed, 64 insertions(+), 11 deletions(-) diff --git a/it/ruby/.rubocop.yml b/it/ruby/.rubocop.yml index ff66ed93af..f82e15d04e 100644 --- a/it/ruby/.rubocop.yml +++ b/it/ruby/.rubocop.yml @@ -131,17 +131,11 @@ Naming/VariableNumber: Metrics/CyclomaticComplexity: Enabled: false -# Generated factory methods may just delegate to super -Lint/UselessMethodDefinition: - Enabled: false - -# Generated composed type methods can have high perceived complexity +# Composed type factories, serializers and deserializers branch once per union member, +# so their complexity tracks the arity of the union rather than any avoidable nesting Metrics/PerceivedComplexity: Enabled: false -# Generated composed types use if !x.nil? for elsif compatibility -Style/NegatedIf: - Enabled: false - -Style/GuardClause: +# Generated factory methods may just delegate to super +Lint/UselessMethodDefinition: Enabled: false diff --git a/src/Kiota.Builder/Writers/Ruby/CodeMethodWriter.cs b/src/Kiota.Builder/Writers/Ruby/CodeMethodWriter.cs index cfb8f1c229..0817ed624c 100644 --- a/src/Kiota.Builder/Writers/Ruby/CodeMethodWriter.cs +++ b/src/Kiota.Builder/Writers/Ruby/CodeMethodWriter.cs @@ -163,6 +163,12 @@ private void WriteFactoryMethodBodyForUnionModel(CodeParameter parseNodeParamete writer.WriteLine("return result"); } private static string GetIntersectionValueVarName(CodeProperty property) => $"val_{property.Name.ToSnakeCase()}"; + private void WriteComposedTypeGuardedSerialization(CodeProperty property, LanguageWriter writer) + { + var propertyName = property.Name.ToSnakeCase(); + writer.WriteLine($"return if @{propertyName}.nil?"); + writer.WriteLine($"writer.{GetSerializationMethodName(property.Type)}(nil, @{propertyName})"); + } private void WriteFactoryMethodBodyForIntersectionModel(CodeParameter parseNodeParameter, CodeClass parentClass, LanguageWriter writer) { writer.WriteLine($"result = {parentClass.Name.ToFirstCharacterUpperCase()}.new"); @@ -180,10 +186,15 @@ private void WriteFactoryMethodBodyForIntersectionModel(CodeParameter parseNodeP var methodName = GetDeserializationMethodName(property.Type); writer.WriteLine($"{GetIntersectionValueVarName(property)} = {parseNodeParameterName}.{methodName}"); } + // Ruby has no `elsunless`, so a chain has to open with `if !x.nil?`; a lone branch with no + // else reads as `unless x.nil?` instead, which is also what RuboCop's Style/NegatedIf wants + var factoryBranchesChain = nonComplexProperties.Length > 1 || complexProperties.Length > 0; var elseIfPrefix = string.Empty; foreach (var property in nonComplexProperties) { - writer.StartBlock($"{elseIfPrefix}if !{GetIntersectionValueVarName(property)}.nil?"); + writer.StartBlock(factoryBranchesChain + ? $"{elseIfPrefix}if !{GetIntersectionValueVarName(property)}.nil?" + : $"unless {GetIntersectionValueVarName(property)}.nil?"); writer.WriteLine($"result.{property.Name.ToSnakeCase()} = {GetIntersectionValueVarName(property)}"); writer.DecreaseIndent(); elseIfPrefix = "els"; @@ -547,6 +558,13 @@ private void WriteSerializerBodyForUnionModel(CodeClass parentClass, LanguageWri .OrderBy(static x => x, new CodePropertyTypeComparer()) .ThenBy(static x => x.Name, StringComparer.OrdinalIgnoreCase) .ToArray(); + // a lone member is a guard clause rather than a conditional wrapping the whole body, which + // is what RuboCop's Style/GuardClause and Style/NegatedIf both ask for + if (customProperties.Length == 1) + { + WriteComposedTypeGuardedSerialization(customProperties[0], writer); + return; + } var elseIfPrefix = string.Empty; foreach (var property in customProperties) { @@ -567,6 +585,11 @@ private void WriteSerializerBodyForIntersectionModel(CodeClass parentClass, Lang .ToArray(); var nonComplexProperties = customProperties.Where(static x => x.Type is not CodeType propType || propType.TypeDefinition is not CodeClass || propType.CollectionKind != CodeTypeBase.CodeTypeCollectionKind.None).ToArray(); var complexProperties = customProperties.Where(static x => x.Type is CodeType propType && propType.TypeDefinition is CodeClass && propType.CollectionKind == CodeTypeBase.CodeTypeCollectionKind.None).ToArray(); + if (nonComplexProperties.Length == 1 && complexProperties.Length == 0) + { + WriteComposedTypeGuardedSerialization(nonComplexProperties[0], writer); + return; + } var elseIfPrefix = string.Empty; foreach (var property in nonComplexProperties) { diff --git a/tests/Kiota.Builder.Tests/Writers/Ruby/CodeMethodWriterTests.cs b/tests/Kiota.Builder.Tests/Writers/Ruby/CodeMethodWriterTests.cs index ebd90fc09f..bd4c0019c5 100644 --- a/tests/Kiota.Builder.Tests/Writers/Ruby/CodeMethodWriterTests.cs +++ b/tests/Kiota.Builder.Tests/Writers/Ruby/CodeMethodWriterTests.cs @@ -1670,6 +1670,42 @@ public void WritesUnionSerializerBody() AssertBalancedBlocks(result); } [Fact] + public void WritesUnionSerializerBodyForSingleMemberAsGuardClause() + { + // a single member has no elsif to chain to, so `if !x.nil?` wrapping the whole body trips + // both Style/NegatedIf and Style/GuardClause -- this reached CI via NoUnderscoresInModel + setup(); + parentClass.OriginalComposedType = new CodeUnionType { Name = "UnionType" }; + parentClass.AddProperty(new CodeProperty { Name = "stringValue", Kind = CodePropertyKind.Custom, Type = new CodeType { Name = "string" } }); + method.Kind = CodeMethodKind.Serializer; + method.AddParameter(new CodeParameter { Kind = CodeParameterKind.Serializer, Name = "writer", Type = new CodeType { Name = "SerializationWriter" } }); + writer.Write(method); + var result = tw.ToString(); + Assert.Contains("return if @string_value.nil?", result); + Assert.Contains("writer.write_string_value(nil, @string_value)", result); + Assert.DoesNotContain("if !", result); + Assert.DoesNotContain("elsif", result); + AssertBalancedBlocks(result); + } + [Fact] + public void WritesIntersectionFactoryBodyForSingleMemberWithUnless() + { + // Ruby has no `elsunless`, so a chain has to open with `if !x.nil?`, but a lone branch + // with no else must not: RuboCop's Style/NegatedIf wants `unless` there + setup(); + parentClass.OriginalComposedType = new CodeIntersectionType { Name = "IntersectionType" }; + parentClass.AddProperty(new CodeProperty { Name = "stringValue", Kind = CodePropertyKind.Custom, Type = new CodeType { Name = "string" } }); + method.Kind = CodeMethodKind.Factory; + method.ReturnType = new CodeType { Name = "ParentClass", TypeDefinition = parentClass }; + method.AddParameter(new CodeParameter { Kind = CodeParameterKind.ParseNode, Name = "parseNode", Type = new CodeType { Name = "ParseNode" } }); + writer.Write(method); + var result = tw.ToString(); + Assert.Contains("unless val_string_value.nil?", result); + Assert.DoesNotContain("if !", result); + Assert.DoesNotContain("elsif", result); + AssertBalancedBlocks(result); + } + [Fact] public void WritesIntersectionSerializerBody() { AddIntersectionTypeWrapper();