diff --git a/CHANGELOG.md b/CHANGELOG.md index 0d497e8c00..97249d51b3 100644 --- a/CHANGELOG.md +++ b/CHANGELOG.md @@ -18,6 +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 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 472659e39a..d7d6d57e8c 100644 --- a/it/config.json +++ b/it/config.json @@ -146,10 +146,6 @@ }, "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..f82e15d04e 100644 --- a/it/ruby/.rubocop.yml +++ b/it/ruby/.rubocop.yml @@ -131,6 +131,11 @@ Naming/VariableNumber: Metrics/CyclomaticComplexity: Enabled: false +# 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 factory methods may just delegate to super Lint/UselessMethodDefinition: Enabled: false diff --git a/it/ruby/Gemfile b/it/ruby/Gemfile index a2a044d4fd..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.15.1" +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.10.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.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/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/src/Kiota.Builder/Writers/Ruby/CodeMethodWriter.cs b/src/Kiota.Builder/Writers/Ruby/CodeMethodWriter.cs index e90a9f4a1c..0817ed624c 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,106 @@ 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) + { + // 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"; + } + // 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 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"); + 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}"); + } + // 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(factoryBranchesChain + ? $"{elseIfPrefix}if !{GetIntersectionValueVarName(property)}.nil?" + : $"unless {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 +382,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 +409,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 +530,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 +552,68 @@ 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(); + // 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) + { + 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(); + if (nonComplexProperties.Length == 1 && complexProperties.Length == 0) + { + WriteComposedTypeGuardedSerialization(nonComplexProperties[0], writer); + return; + } + 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"); + // 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($"composed_values = [{complexPropNames}].compact"); + writer.WriteLine("writer.write_object_value(nil, *composed_values) unless composed_values.empty?"); + 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..8bf8e4f0c8 100644 --- a/src/kiota/appsettings.json +++ b/src/kiota/appsettings.json @@ -318,22 +318,22 @@ "Dependencies": [ { "Name": "microsoft_kiota_abstractions", - "Version": "0.15.1", + "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.10.0", + "Version": "0.19.0", "Type": "Serialization" }, { "Name": "microsoft_kiota_authentication_oauth", - "Version": "0.9.0", + "Version": "0.19.0", "Type": "Authentication" } ], diff --git a/tests/Kiota.Builder.Tests/Refiners/RubyLanguageRefinerTests.cs b/tests/Kiota.Builder.Tests/Refiners/RubyLanguageRefinerTests.cs index 213d8fbb1e..b59a00b62d 100644 --- a/tests/Kiota.Builder.Tests/Refiners/RubyLanguageRefinerTests.cs +++ b/tests/Kiota.Builder.Tests/Refiners/RubyLanguageRefinerTests.cs @@ -123,6 +123,34 @@ public async Task EscapesReservedKeywordsAsync() Assert.Contains("escaped", model.Name); } [Fact] + public async Task EscapesInitializeAsync() + { + // 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); + } + [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; diff --git a/tests/Kiota.Builder.Tests/Writers/Ruby/CodeMethodWriterTests.cs b/tests/Kiota.Builder.Tests/Writers/Ruby/CodeMethodWriterTests.cs index 67e67033a2..bd4c0019c5 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,316 @@ 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 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(); + 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 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(); + 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("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); + } + [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("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); + } + /// + /// 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)); + } }