diff --git a/CHANGELOG.md b/CHANGELOG.md index 1beb8b4569..ca7b9ae03e 100644 --- a/CHANGELOG.md +++ b/CHANGELOG.md @@ -11,6 +11,8 @@ and this project adheres to [Semantic Versioning](https://semver.org/spec/v2.0.0 ### Added +- Added an opt-in `--make-required-properties-non-nullable` (`--mrpnn`) generation option that generates properties marked as required in the OpenAPI description (and not explicitly nullable) as non-nullable and non-optional. Currently supported for TypeScript, Python, and Java. Disabled by default to preserve existing output. [#7795](https://github.com/microsoft/kiota/pull/7795) + ### Changed - Removed support for specifying dependency install commands through the `x-ms-kiota-info` OpenAPI description extension. diff --git a/specs/cli/client-add.md b/specs/cli/client-add.md index 7bd2a84dd1..d900536d7e 100644 --- a/specs/cli/client-add.md +++ b/specs/cli/client-add.md @@ -24,6 +24,7 @@ Once the `workspace.json` file is generated and the OpenAPI description file is | `--namespace-name \| -n` | No | Contoso.GraphApp | The namespace of the client class. Defaults to `Microsoft.Graph`. | Yes, without its value | | `--backing-store \| -b` | No | | Defaults to `false` | Yes | | `--exclude-backward-compatible \| --ebc` | No | | Whether to exclude the code generated only for backward compatibility reasons or not. Defaults to `false`. | Yes | +| `--make-required-properties-non-nullable \| --mrpnn` | No | | When enabled, properties marked as required and not explicitly nullable are generated as non-nullable (and non-optional, for languages that distinguish the two). Currently supported for TypeScript, Python, and Java. Defaults to `false`. | Yes | | `--structured-media-types \| -m` | No | `application/json` | Any valid media type which will match a request body type or a response type in the OpenAPI description. Default are documented [here](https://learn.microsoft.com/en-us/openapi/kiota/using#--structured-mime-types--m). | Yes | | `--skip-generation \| --sg` | No | true | When specified, the generation would be skipped. Defaults to false. |Yes | | `--output \| -o` | No | ./generated/graph/csharp | The output directory or file path for the generated code files. This is relative to the location of `workspace.json`. Defaults to `./output`. | Yes, without its value | diff --git a/specs/cli/client-edit.md b/specs/cli/client-edit.md index 63ae827779..4bb2913b59 100644 --- a/specs/cli/client-edit.md +++ b/specs/cli/client-edit.md @@ -20,6 +20,7 @@ Once the `workspace.json` file and the API Manifest are updated, the code genera | `--namespace-name \| -n` | No | Contoso.GraphApp | The namespace of the client class. Defaults to `Microsoft.Graph`. | Yes, without its value | | `--backing-store \| -b` | No | | Defaults to `false` | Yes | | `--exclude-backward-compatible \| --ebc` | No | | Whether to exclude the code generated only for backward compatibility reasons or not. Defaults to `false`. | Yes | +| `--make-required-properties-non-nullable \| --mrpnn` | No | | When enabled, properties marked as required and not explicitly nullable are generated as non-nullable (and non-optional, for languages that distinguish the two). Currently supported for TypeScript, Python, and Java. Defaults to `false`. | Yes | | `--structured-media-types \| -m` | No | `application/json` |Any valid media type which will match a request body type or a response type in the OpenAPI description. Default are documented [here](https://learn.microsoft.com/en-us/openapi/kiota/using#--structured-mime-types--m). | Yes | | `--skip-generation \| --sg` | No | true | When specified, the generation would be skipped. Defaults to false. | Yes | | `--output \| -o` | No | ./generated/graph/csharp | The output directory or file path for the generated code files. Defaults to `./output`. | Yes, without its value | diff --git a/specs/schemas/workspace.json b/specs/schemas/workspace.json index 89b2bb089e..5554afdc4c 100644 --- a/specs/schemas/workspace.json +++ b/specs/schemas/workspace.json @@ -52,6 +52,9 @@ }, "includeAdditionalData": { "type": "boolean" + }, + "makeRequiredPropertiesNonNullable": { + "type": "boolean" } } } diff --git a/src/Kiota.Builder/CodeDOM/CodeProperty.cs b/src/Kiota.Builder/CodeDOM/CodeProperty.cs index c5f155828d..5d99b84607 100644 --- a/src/Kiota.Builder/CodeDOM/CodeProperty.cs +++ b/src/Kiota.Builder/CodeDOM/CodeProperty.cs @@ -122,6 +122,14 @@ public bool IsPrimaryErrorMessage { get; set; } + /// + /// Indicates that this property appeared in the parent schema's required array. + /// Set during Code DOM construction in KiotaBuilder; should not be modified by refiners. + /// + public bool IsRequired + { + get; set; + } public object Clone() { @@ -143,6 +151,7 @@ public object Clone() OriginalPropertyFromBaseType = OriginalPropertyFromBaseType?.Clone() as CodeProperty, Deprecation = Deprecation, IsPrimaryErrorMessage = IsPrimaryErrorMessage, + IsRequired = IsRequired, }; return property; } diff --git a/src/Kiota.Builder/Configuration/GenerationConfiguration.cs b/src/Kiota.Builder/Configuration/GenerationConfiguration.cs index e82f1079d2..7677d88645 100644 --- a/src/Kiota.Builder/Configuration/GenerationConfiguration.cs +++ b/src/Kiota.Builder/Configuration/GenerationConfiguration.cs @@ -65,6 +65,15 @@ public bool UsesBackingStore { get; set; } + /// + /// When enabled, properties marked as required in the OpenAPI description and not explicitly nullable + /// are generated as non-nullable (and non-optional, for languages that distinguish the two). + /// Defaults to false to preserve the historical all-nullable behavior for existing clients. + /// + public bool MakeRequiredPropertiesNonNullable + { + get; set; + } public bool ExcludeBackwardCompatible { get; set; @@ -184,6 +193,7 @@ public object Clone() DisableSSLValidation = DisableSSLValidation, ExportPublicApi = ExportPublicApi, PluginAuthInformation = PluginAuthInformation, + MakeRequiredPropertiesNonNullable = MakeRequiredPropertiesNonNullable, }; } private static readonly StringIEnumerableDeepComparer comparer = new(); diff --git a/src/Kiota.Builder/Extensions/OpenApiSchemaExtensions.cs b/src/Kiota.Builder/Extensions/OpenApiSchemaExtensions.cs index 543e40916e..c444ba2046 100644 --- a/src/Kiota.Builder/Extensions/OpenApiSchemaExtensions.cs +++ b/src/Kiota.Builder/Extensions/OpenApiSchemaExtensions.cs @@ -80,6 +80,21 @@ public static bool HasAnyProperty(this IOpenApiSchema? schema) { return schema?.Properties is { Count: > 0 }; } + /// + /// Indicates whether the schema explicitly permits null. + /// Covers OAS 3.0 nullable: true and OAS 3.1 type: [..., "null"], + /// as well as the OAS 3.1 anyOf: [{ type: null }] pattern. + /// + internal static bool IsExplicitlyNullable(this IOpenApiSchema? schema) + { + if (schema is null) return false; + // OAS 3.0 nullable: true or OAS 3.1 type includes null + if ((schema.Type & JsonSchemaType.Null) is JsonSchemaType.Null) return true; + // OAS 3.1 anyOf/oneOf [ { type: null }, ... ] patterns + static bool HasNullMember(IList? members) => members?.Any(static x => + (x.Type & JsonSchemaType.Null) is JsonSchemaType.Null && !x.HasAnyProperty()) ?? false; + return HasNullMember(schema.AnyOf) || HasNullMember(schema.OneOf); + } public static bool IsInclusiveUnion(this IOpenApiSchema? schema, uint exclusiveMinimumNumberOfEntries = 1) { return schema?.AnyOf?.Count(static x => IsSemanticallyMeaningful(x, true)) > exclusiveMinimumNumberOfEntries; diff --git a/src/Kiota.Builder/KiotaBuilder.cs b/src/Kiota.Builder/KiotaBuilder.cs index 6cac83b4fb..d017be06f1 100644 --- a/src/Kiota.Builder/KiotaBuilder.cs +++ b/src/Kiota.Builder/KiotaBuilder.cs @@ -661,7 +661,7 @@ public async Task ApplyLanguageRefinementAsync(GenerationConfiguration config, C public async Task CreateLanguageSourceFilesAsync(GenerationLanguage language, CodeNamespace generatedCode, CancellationToken cancellationToken) { - var languageWriter = LanguageWriter.GetLanguageWriter(language, config.OutputPath, config.ClientNamespaceName, config.UsesBackingStore, config.ExcludeBackwardCompatible); + var languageWriter = LanguageWriter.GetLanguageWriter(language, config.OutputPath, config.ClientNamespaceName, config.UsesBackingStore, config.ExcludeBackwardCompatible, config.MakeRequiredPropertiesNonNullable); var stopwatch = new Stopwatch(); stopwatch.Start(); var codeRenderer = CodeRenderer.GetCodeRender(config); @@ -1174,7 +1174,7 @@ private CodeIndexer[] CreateIndexer(string childIdentifier, string childType, Co } private static readonly StructuralPropertiesReservedNameProvider structuralPropertiesReservedNameProvider = new(); - private CodeProperty? CreateProperty(string childIdentifier, string childType, IOpenApiSchema? propertySchema = null, CodeTypeBase? existingType = null, CodePropertyKind kind = CodePropertyKind.Custom) + private CodeProperty? CreateProperty(string childIdentifier, string childType, IOpenApiSchema? propertySchema = null, CodeTypeBase? existingType = null, CodePropertyKind kind = CodePropertyKind.Custom, bool isRequired = false) { var propertyName = childIdentifier.CleanupSymbolName(); if (structuralPropertiesReservedNameProvider.ReservedNames.Contains(propertyName)) @@ -1196,6 +1196,7 @@ private CodeIndexer[] CreateIndexer(string childIdentifier, string childType, Co ReadOnly = propertySchema?.ReadOnly ?? false, Type = resultType, Deprecation = propertySchema?.GetDeprecationInformation(), + IsRequired = isRequired, IsPrimaryErrorMessage = kind == CodePropertyKind.Custom && propertySchema is { Extensions: not null } && propertySchema.Extensions.TryGetValue(OpenApiPrimaryErrorMessageExtension.Name, out var openApiExtension) && @@ -1221,6 +1222,22 @@ openApiExtension is OpenApiPrimaryErrorMessageExtension primaryErrorMessageExten prop.DefaultValue = stringDefaultJsonValue2.ToString(); } + // Required, non-explicitly-nullable properties are marked non-nullable so writers can drop the optional/nullable markers. + // Collections are excluded: their IsNullable also drives element nullability, which the serialization APIs rely on. + // Scoped to the languages whose writers honor this safely (TypeScript, Python, Java); other languages keep the + // historical nullable rendering so their output is unchanged until support is added and validated for them. + // Clone first to avoid mutating a shared/cached type reference. + var isCollection = existingType != null + ? existingType.CollectionKind != CodeTypeBase.CodeTypeCollectionKind.None + : propertySchema.IsArray(); + var languageSupportsNonNullableRequired = config.Language is GenerationLanguage.TypeScript or GenerationLanguage.Python or GenerationLanguage.Java; + if (config.MakeRequiredPropertiesNonNullable && languageSupportsNonNullableRequired && kind == CodePropertyKind.Custom && isRequired && !propertySchema.IsExplicitlyNullable() && !isCollection) + { + if (existingType != null) + prop.Type = (CodeTypeBase)existingType.Clone(); + prop.Type.IsNullable = false; + } + if (existingType == null) { prop.Type.CollectionKind = propertySchema.IsArray() ? CodeTypeBase.CodeTypeCollectionKind.Complex : default; @@ -2434,7 +2451,13 @@ private void CreatePropertiesForModelClass(OpenApiUrlTreeNode currentNode, IOpen LogOmittedPropertyInvalidSchema(x.Key, model.Name, currentNode.Path); return null; } - return CreateProperty(x.Key, definition.Name, propertySchema: propertySchema, existingType: definition); + // Required-ness is read from the (already allOf-merged) schema's own `required` array. Note that + // MergeIntersectionSchemaEntries merges member *properties* but not their `required` arrays, so a + // property declared required only inside an allOf member is treated as not-required here. This is a + // safe under-application for MakeRequiredPropertiesNonNullable (such a property stays nullable) and + // matches the pre-existing required handling; widening it would be a separate, cross-cutting change. + var isRequired = schema.Required?.Contains(x.Key) ?? false; + return CreateProperty(x.Key, definition.Name, propertySchema: propertySchema, existingType: definition, isRequired: isRequired); }) .OfType() .ToArray() ?? []; diff --git a/src/Kiota.Builder/WorkspaceManagement/ApiClientConfiguration.cs b/src/Kiota.Builder/WorkspaceManagement/ApiClientConfiguration.cs index 05fe7ed810..a336c61350 100644 --- a/src/Kiota.Builder/WorkspaceManagement/ApiClientConfiguration.cs +++ b/src/Kiota.Builder/WorkspaceManagement/ApiClientConfiguration.cs @@ -51,6 +51,13 @@ public bool ExcludeBackwardCompatible get; set; } /// + /// Whether required, non-explicitly-nullable properties were generated as non-nullable for this client. + /// + public bool MakeRequiredPropertiesNonNullable + { + get; set; + } + /// /// The OpenAPI validation rules to disable during the generation. /// public HashSet DisabledValidationRules { get; set; } = new(StringComparer.OrdinalIgnoreCase); @@ -73,6 +80,7 @@ public ApiClientConfiguration(GenerationConfiguration config) : base(config) ClientNamespaceName = config.ClientNamespaceName; UsesBackingStore = config.UsesBackingStore; ExcludeBackwardCompatible = config.ExcludeBackwardCompatible; + MakeRequiredPropertiesNonNullable = config.MakeRequiredPropertiesNonNullable; IncludeAdditionalData = config.IncludeAdditionalData; StructuredMimeTypes = config.StructuredMimeTypes.ToList(); DisabledValidationRules = config.DisabledValidationRules.ToHashSet(StringComparer.OrdinalIgnoreCase); @@ -94,6 +102,7 @@ public void UpdateGenerationConfigurationFromApiClientConfiguration(GenerationCo config.TypeAccessModifier = parsedTypeAccessModifier; config.UsesBackingStore = UsesBackingStore; config.ExcludeBackwardCompatible = ExcludeBackwardCompatible; + config.MakeRequiredPropertiesNonNullable = MakeRequiredPropertiesNonNullable; config.IncludeAdditionalData = IncludeAdditionalData; config.StructuredMimeTypes = new(StructuredMimeTypes); config.DisabledValidationRules = DisabledValidationRules.ToHashSet(StringComparer.OrdinalIgnoreCase); @@ -111,6 +120,7 @@ public object Clone() UsesBackingStore = UsesBackingStore, IncludeAdditionalData = IncludeAdditionalData, ExcludeBackwardCompatible = ExcludeBackwardCompatible, + MakeRequiredPropertiesNonNullable = MakeRequiredPropertiesNonNullable, DisabledValidationRules = new(DisabledValidationRules, StringComparer.OrdinalIgnoreCase), }; CloneBase(result); diff --git a/src/Kiota.Builder/Writers/LanguageWriter.cs b/src/Kiota.Builder/Writers/LanguageWriter.cs index 22313267dd..7aedc62a6c 100644 --- a/src/Kiota.Builder/Writers/LanguageWriter.cs +++ b/src/Kiota.Builder/Writers/LanguageWriter.cs @@ -178,13 +178,13 @@ protected void AddOrReplaceCodeElementWriter(ICodeElementWriter writer) wh Writers[typeof(T)] = writer; } private readonly Dictionary Writers = []; // we have to type as object because dotnet doesn't have type capture i.e eq for `? extends CodeElement` - public static LanguageWriter GetLanguageWriter(GenerationLanguage language, string outputPath, string clientNamespaceName, bool usesBackingStore = false, bool excludeBackwardCompatible = false) + public static LanguageWriter GetLanguageWriter(GenerationLanguage language, string outputPath, string clientNamespaceName, bool usesBackingStore = false, bool excludeBackwardCompatible = false, bool makeRequiredPropertiesNonNullable = false) { return language switch { GenerationLanguage.CSharp => new CSharpWriter(outputPath, clientNamespaceName), GenerationLanguage.Java => new JavaWriter(outputPath, clientNamespaceName), - GenerationLanguage.TypeScript => new TypeScriptWriter(outputPath, clientNamespaceName), + GenerationLanguage.TypeScript => new TypeScriptWriter(outputPath, clientNamespaceName, makeRequiredPropertiesNonNullable), GenerationLanguage.Ruby => new RubyWriter(outputPath, clientNamespaceName), GenerationLanguage.PHP => new PhpWriter(outputPath, clientNamespaceName, usesBackingStore), GenerationLanguage.Python => new PythonWriter(outputPath, clientNamespaceName, usesBackingStore), diff --git a/src/Kiota.Builder/Writers/Python/CodeMethodWriter.cs b/src/Kiota.Builder/Writers/Python/CodeMethodWriter.cs index 394a0384a0..6f4f196e75 100644 --- a/src/Kiota.Builder/Writers/Python/CodeMethodWriter.cs +++ b/src/Kiota.Builder/Writers/Python/CodeMethodWriter.cs @@ -454,6 +454,15 @@ private void WriteSetterAccessProperties(CodeClass parentClass, LanguageWriter w } } private const string NoneKeyword = "None"; + private static string GetNonNullableDefaultValue(string pythonType) => pythonType switch + { + "str" => "''", + "int" => "0", + "float" => "0.0", + "bool" => "False", + "bytes" => "b''", + _ => NoneKeyword, + }; private void WriteSetterAccessPropertiesWithoutDefaults(CodeClass parentClass, LanguageWriter writer) { foreach (var propWithoutDefault in parentClass.GetPropertiesOfKind(SetterAccessProperties) @@ -463,10 +472,17 @@ private void WriteSetterAccessPropertiesWithoutDefaults(CodeClass parentClass, L { var returnType = conventions.GetTypeString(propWithoutDefault.Type, propWithoutDefault, true, writer); conventions.WriteInLineDescription(propWithoutDefault, writer); + // A required, non-nullable property (set only when MakeRequiredPropertiesNonNullable is on) is rendered + // without Optional[...]. A dataclass still needs a default for no-arg construction, so use a type-correct + // zero-value for primitives (str -> '', int -> 0, ...) instead of None. For non-primitive types (objects, + // enums) no zero-value literal exists, so None is kept — an advisory non-null, matching Java's @Nonnull. + var defaultValue = !propWithoutDefault.Type.IsNullable && propWithoutDefault.IsRequired + ? GetNonNullableDefaultValue(returnType) + : NoneKeyword; if (parentClass.IsOfKind(CodeClassKind.Model)) - writer.WriteLine($"{propWithoutDefault.Name}: {(propWithoutDefault.Type.IsNullable ? "Optional[" : string.Empty)}{returnType}{(propWithoutDefault.Type.IsNullable ? "]" : string.Empty)} = {NoneKeyword}"); + writer.WriteLine($"{propWithoutDefault.Name}: {(propWithoutDefault.Type.IsNullable ? "Optional[" : string.Empty)}{returnType}{(propWithoutDefault.Type.IsNullable ? "]" : string.Empty)} = {defaultValue}"); else - writer.WriteLine($"self.{conventions.GetAccessModifier(propWithoutDefault.Access)}{propWithoutDefault.NamePrefix}{propWithoutDefault.Name}: {(propWithoutDefault.Type.IsNullable ? "Optional[" : string.Empty)}{returnType}{(propWithoutDefault.Type.IsNullable ? "]" : string.Empty)} = {NoneKeyword}"); + writer.WriteLine($"self.{conventions.GetAccessModifier(propWithoutDefault.Access)}{propWithoutDefault.NamePrefix}{propWithoutDefault.Name}: {(propWithoutDefault.Type.IsNullable ? "Optional[" : string.Empty)}{returnType}{(propWithoutDefault.Type.IsNullable ? "]" : string.Empty)} = {defaultValue}"); } } private static void WriteSetterBody(CodeMethod codeElement, LanguageWriter writer, CodeClass parentClass) diff --git a/src/Kiota.Builder/Writers/TypeScript/CodeFunctionWriter.cs b/src/Kiota.Builder/Writers/TypeScript/CodeFunctionWriter.cs index 1f95fcbac6..c42b8e2b41 100644 --- a/src/Kiota.Builder/Writers/TypeScript/CodeFunctionWriter.cs +++ b/src/Kiota.Builder/Writers/TypeScript/CodeFunctionWriter.cs @@ -98,6 +98,13 @@ public override void WriteCodeElement(CodeFunction codeElement, LanguageWriter w var composedType = GetOriginalComposedType(codeMethod.ReturnType); var isComposedOfPrimitives = composedType is not null && composedType.IsComposedOfPrimitives(IsPrimitiveType); + // A primitive-composed factory's body is `parseNode?.get*Value()`, i.e. always `T | undefined`, so its + // declared return type must stay nullable for the `| undefined` suffix to be emitted (read by + // CodeMethodWriter.WriteMethodPrototypeInternal). MakeRequiredPropertiesNonNullable can otherwise flip it + // non-nullable via a required scalar-alias reference; no-op when the flag is off (already nullable). + if (codeMethod.Kind is CodeMethodKind.Factory && isComposedOfPrimitives) + codeMethod.ReturnType.IsNullable = true; + var returnType = codeMethod.Kind is CodeMethodKind.Factory && !isComposedOfPrimitives ? FactoryMethodReturnType : GetTypescriptTypeString(codeMethod.ReturnType, codeElement, inlineComposedTypeString: true); diff --git a/src/Kiota.Builder/Writers/TypeScript/CodePropertyWriter.cs b/src/Kiota.Builder/Writers/TypeScript/CodePropertyWriter.cs index 77a70e3e9d..f4f1d873c2 100644 --- a/src/Kiota.Builder/Writers/TypeScript/CodePropertyWriter.cs +++ b/src/Kiota.Builder/Writers/TypeScript/CodePropertyWriter.cs @@ -28,24 +28,32 @@ public override void WriteCodeElement(CodeProperty codeElement, LanguageWriter w switch (codeElement.Parent) { case CodeInterface: - WriteCodePropertyForInterface(codeElement, writer, returnType, isFlagEnum); + WriteCodePropertyForInterface(codeElement, writer, returnType, isFlagEnum, conventions.MakeRequiredPropertiesNonNullable); break; case CodeClass: throw new InvalidOperationException($"All properties are defined on interfaces in TypeScript."); } } - private static void WriteCodePropertyForInterface(CodeProperty codeElement, LanguageWriter writer, string returnType, bool isFlagEnum) + private static void WriteCodePropertyForInterface(CodeProperty codeElement, LanguageWriter writer, string returnType, bool isFlagEnum, bool makeRequiredPropertiesNonNullable) { + var collectionSuffix = isFlagEnum ? "[]" : string.Empty; switch (codeElement.Kind) { case CodePropertyKind.RequestBuilder: writer.WriteLine($"get {codeElement.Name.ToFirstCharacterLowerCase()}(): {returnType};"); break; case CodePropertyKind.QueryParameter: - writer.WriteLine($"{codeElement.Name.ToFirstCharacterLowerCase()}?: {returnType}{(isFlagEnum ? "[]" : string.Empty)};"); + writer.WriteLine($"{codeElement.Name.ToFirstCharacterLowerCase()}?: {returnType}{collectionSuffix};"); break; default: - writer.WriteLine($"{codeElement.Name.ToFirstCharacterLowerCase()}?: {returnType}{(isFlagEnum ? "[]" : string.Empty)} | null;"); + // When enabled, a required property is non-optional (no `?`), and drops `| null` unless its + // schema is explicitly nullable. Otherwise the historical optional + nullable form is kept. + var suppressOptionalAndNull = makeRequiredPropertiesNonNullable && codeElement.IsRequired; + var optionalMarker = suppressOptionalAndNull ? string.Empty : "?"; + var nullSuffix = suppressOptionalAndNull + ? (codeElement.Type.IsNullable ? " | null" : string.Empty) + : " | null"; + writer.WriteLine($"{codeElement.Name.ToFirstCharacterLowerCase()}{optionalMarker}: {returnType}{collectionSuffix}{nullSuffix};"); break; } } diff --git a/src/Kiota.Builder/Writers/TypeScript/TypeScriptConventionService.cs b/src/Kiota.Builder/Writers/TypeScript/TypeScriptConventionService.cs index 1019b241e4..81e1e4f200 100644 --- a/src/Kiota.Builder/Writers/TypeScript/TypeScriptConventionService.cs +++ b/src/Kiota.Builder/Writers/TypeScript/TypeScriptConventionService.cs @@ -12,6 +12,13 @@ namespace Kiota.Builder.Writers.TypeScript; public class TypeScriptConventionService : CommonLanguageConventionService { + /// + /// When true, required and not-explicitly-nullable properties are rendered as non-optional + /// (no ?) and non-nullable (no | null). Defaults to false to preserve the + /// historical behavior where every model property is optional and nullable. + /// + public bool MakeRequiredPropertiesNonNullable { get; init; } + #pragma warning disable CA1707 // Remove the underscores public const string TYPE_INTEGER = "integer"; public const string TYPE_INT64 = "int64"; diff --git a/src/Kiota.Builder/Writers/TypeScript/TypeScriptWriter.cs b/src/Kiota.Builder/Writers/TypeScript/TypeScriptWriter.cs index 45f8ccbd72..d0e8dadafb 100644 --- a/src/Kiota.Builder/Writers/TypeScript/TypeScriptWriter.cs +++ b/src/Kiota.Builder/Writers/TypeScript/TypeScriptWriter.cs @@ -4,10 +4,13 @@ namespace Kiota.Builder.Writers.TypeScript; public class TypeScriptWriter : LanguageWriter { - public TypeScriptWriter(string rootPath, string clientNamespaceName) + public TypeScriptWriter(string rootPath, string clientNamespaceName, bool makeRequiredPropertiesNonNullable = false) { PathSegmenter = new TypeScriptPathSegmenter(rootPath, clientNamespaceName); - var conventionService = new TypeScriptConventionService(); + var conventionService = new TypeScriptConventionService + { + MakeRequiredPropertiesNonNullable = makeRequiredPropertiesNonNullable + }; AddOrReplaceCodeElementWriter(new CodeClassDeclarationWriter(conventionService, clientNamespaceName)); AddOrReplaceCodeElementWriter(new CodeBlockEndWriter(conventionService)); AddOrReplaceCodeElementWriter(new CodeEnumWriter(conventionService)); diff --git a/src/kiota/Handlers/Client/AddHandler.cs b/src/kiota/Handlers/Client/AddHandler.cs index eb6d76e98a..9c3f66c71e 100644 --- a/src/kiota/Handlers/Client/AddHandler.cs +++ b/src/kiota/Handlers/Client/AddHandler.cs @@ -66,6 +66,11 @@ public required Option ExcludeBackwardCompatibleOption get; set; } + public required Option MakeRequiredPropertiesNonNullableOption + { + get; + set; + } public required Option> IncludePatternsOption { get; init; @@ -91,6 +96,7 @@ public override async Task InvokeAsync(ParseResult parseResult, Cancellatio string? openapi = parseResult.GetValue(DescriptionOption); bool backingStore = parseResult.GetValue(BackingStoreOption); bool excludeBackwardCompatible = parseResult.GetValue(ExcludeBackwardCompatibleOption); + bool makeRequiredPropertiesNonNullable = parseResult.GetValue(MakeRequiredPropertiesNonNullableOption); bool includeAdditionalData = parseResult.GetValue(AdditionalDataOption); bool skipGeneration = parseResult.GetValue(SkipGenerationOption); string? className = parseResult.GetValue(ClassOption); @@ -126,6 +132,7 @@ public override async Task InvokeAsync(ParseResult parseResult, Cancellatio AssignIfNotNullOrEmpty(namespaceName, (c, s) => c.ClientNamespaceName = s); Configuration.Generation.UsesBackingStore = backingStore; Configuration.Generation.ExcludeBackwardCompatible = excludeBackwardCompatible; + Configuration.Generation.MakeRequiredPropertiesNonNullable = makeRequiredPropertiesNonNullable; Configuration.Generation.IncludeAdditionalData = includeAdditionalData; Configuration.Generation.Language = language; WarnUsingPreviewLanguage(language); diff --git a/src/kiota/Handlers/Client/EditHandler.cs b/src/kiota/Handlers/Client/EditHandler.cs index f5a6d018a5..91256bc162 100644 --- a/src/kiota/Handlers/Client/EditHandler.cs +++ b/src/kiota/Handlers/Client/EditHandler.cs @@ -66,6 +66,11 @@ public required Option ExcludeBackwardCompatibleOption get; set; } + public required Option MakeRequiredPropertiesNonNullableOption + { + get; + set; + } public required Option> IncludePatternsOption { get; init; @@ -91,6 +96,7 @@ public override async Task InvokeAsync(ParseResult parseResult, Cancellatio string? openapi0 = parseResult.GetValue(DescriptionOption); bool? backingStore = parseResult.GetValue(BackingStoreOption); bool? excludeBackwardCompatible = parseResult.GetValue(ExcludeBackwardCompatibleOption); + bool? makeRequiredPropertiesNonNullable = parseResult.GetValue(MakeRequiredPropertiesNonNullableOption); bool? includeAdditionalData = parseResult.GetValue(AdditionalDataOption); bool skipGeneration = parseResult.GetValue(SkipGenerationOption); string? className0 = parseResult.GetValue(ClassOption); @@ -152,6 +158,8 @@ public override async Task InvokeAsync(ParseResult parseResult, Cancellatio Configuration.Generation.UsesBackingStore = backingStore.Value; if (excludeBackwardCompatible.HasValue) Configuration.Generation.ExcludeBackwardCompatible = excludeBackwardCompatible.Value; + if (makeRequiredPropertiesNonNullable.HasValue) + Configuration.Generation.MakeRequiredPropertiesNonNullable = makeRequiredPropertiesNonNullable.Value; if (includeAdditionalData.HasValue) Configuration.Generation.IncludeAdditionalData = includeAdditionalData.Value; AssignIfNotNullOrEmpty(output, (c, s) => c.OutputPath = s); diff --git a/src/kiota/Handlers/KiotaGenerateCommandHandler.cs b/src/kiota/Handlers/KiotaGenerateCommandHandler.cs index 4d38b7edf8..4cccb4fc15 100644 --- a/src/kiota/Handlers/KiotaGenerateCommandHandler.cs +++ b/src/kiota/Handlers/KiotaGenerateCommandHandler.cs @@ -86,6 +86,7 @@ public override async Task InvokeAsync(ParseResult parseResult, Cancellatio bool excludeBackwardCompatible = parseResult.GetValue(ExcludeBackwardCompatibleOption); bool clearCache = parseResult.GetValue(ClearCacheOption); bool disableSSLValidation = parseResult.GetValue(DisableSSLValidationOption); + bool makeRequiredPropertiesNonNullable = parseResult.GetValue(MakeRequiredPropertiesNonNullableOption); bool includeAdditionalData = parseResult.GetValue(AdditionalDataOption); string? className = parseResult.GetValue(ClassOption); AccessModifier typeAccessModifier = parseResult.GetValue(TypeAccessModifierOption); @@ -101,7 +102,7 @@ public override async Task InvokeAsync(ParseResult parseResult, Cancellatio var instrumentation = ServiceProvider.GetService(); var activitySource = instrumentation?.ActivitySource; - CreateTelemetryTags(activitySource, language, backingStore, excludeBackwardCompatible, clearCache, disableSSLValidation, cleanOutput, output, + CreateTelemetryTags(activitySource, language, backingStore, excludeBackwardCompatible, makeRequiredPropertiesNonNullable, clearCache, disableSSLValidation, cleanOutput, output, namespaceName, includePatterns0, excludePatterns0, structuredMimeTypes0, logLevel, out var tags); // Start span using var invokeActivity = activitySource?.StartActivity(ActivityKind.Internal, name: TelemetryLabels.SpanGenerateClientCommand, @@ -152,6 +153,7 @@ public override async Task InvokeAsync(ParseResult parseResult, Cancellatio Configuration.Generation.CleanOutput = cleanOutput; Configuration.Generation.ClearCache = clearCache; Configuration.Generation.DisableSSLValidation = disableSSLValidation; + Configuration.Generation.MakeRequiredPropertiesNonNullable = makeRequiredPropertiesNonNullable; var (loggerFactory, logger) = GetLoggerAndFactory(parseResult, Configuration.Generation.OutputPath); using (loggerFactory) @@ -233,19 +235,24 @@ public required Option DisableSSLValidationOption { get; init; } + public required Option MakeRequiredPropertiesNonNullableOption + { + get; init; + } private static void CreateTelemetryTags(ActivitySource? activitySource, GenerationLanguage language, bool backingStore, - bool excludeBackwardCompatible, bool clearCache, bool disableSslValidation, bool cleanOutput, string? output, + bool excludeBackwardCompatible, bool makeRequiredPropertiesNonNullable, bool clearCache, bool disableSslValidation, bool cleanOutput, string? output, string? namespaceName, List? includePatterns, List? excludePatterns, List? structuredMimeTypes, LogLevel? logLevel, out List>? tags) { // set up telemetry tags - tags = activitySource?.HasListeners() == true ? new List>(13) + tags = activitySource?.HasListeners() == true ? new List>(14) { new(TelemetryLabels.TagCommandSource, TelemetryLabels.CommandSourceCliValue), new(TelemetryLabels.TagGeneratorLanguage, language.ToString("G")), new($"{TelemetryLabels.TagCommandParams}.backing_store", backingStore), new($"{TelemetryLabels.TagCommandParams}.exclude_backward_compatible", excludeBackwardCompatible), + new($"{TelemetryLabels.TagCommandParams}.make_required_properties_non_nullable", makeRequiredPropertiesNonNullable), new($"{TelemetryLabels.TagCommandParams}.clear_cache", clearCache), new($"{TelemetryLabels.TagCommandParams}.disable_ssl_validation", disableSslValidation), new($"{TelemetryLabels.TagCommandParams}.clean_output", cleanOutput), diff --git a/src/kiota/KiotaClientCommands.cs b/src/kiota/KiotaClientCommands.cs index 16e91050f5..dcd2541a2f 100644 --- a/src/kiota/KiotaClientCommands.cs +++ b/src/kiota/KiotaClientCommands.cs @@ -45,6 +45,7 @@ public static Command GetAddCommand(IServiceProvider serviceProvider) var logLevelOption = KiotaHost.GetLogLevelOption(); var backingStoreOption = KiotaHost.GetBackingStoreOption(defaultConfiguration.UsesBackingStore); var excludeBackwardCompatible = KiotaHost.GetExcludeBackwardCompatibleOption(defaultConfiguration.ExcludeBackwardCompatible); + var makeRequiredPropertiesNonNullableOption = KiotaHost.GetMakeRequiredPropertiesNonNullableOption(defaultConfiguration.MakeRequiredPropertiesNonNullable); var additionalDataOption = KiotaHost.GetAdditionalDataOption(defaultConfiguration.IncludeAdditionalData); var structuredMimeTypesOption = KiotaHost.GetStructuredMimeTypesOption([.. defaultConfiguration.StructuredMimeTypes]); var (includePatterns, excludePatterns) = KiotaHost.GetIncludeAndExcludeOptions(defaultConfiguration.IncludePatterns, defaultConfiguration.ExcludePatterns); @@ -62,6 +63,7 @@ public static Command GetAddCommand(IServiceProvider serviceProvider) logLevelOption, backingStoreOption, excludeBackwardCompatible, + makeRequiredPropertiesNonNullableOption, additionalDataOption, structuredMimeTypesOption, includePatterns, @@ -80,6 +82,7 @@ public static Command GetAddCommand(IServiceProvider serviceProvider) LogLevelOption = logLevelOption, BackingStoreOption = backingStoreOption, ExcludeBackwardCompatibleOption = excludeBackwardCompatible, + MakeRequiredPropertiesNonNullableOption = makeRequiredPropertiesNonNullableOption, AdditionalDataOption = additionalDataOption, StructuredMimeTypesOption = structuredMimeTypesOption, IncludePatternsOption = includePatterns, @@ -121,6 +124,7 @@ public static Command GetEditCommand(IServiceProvider serviceProvider) var logLevelOption = KiotaHost.GetLogLevelOption(); var backingStoreOption = KiotaHost.GetOptionalBackingStoreOption(); var excludeBackwardCompatible = KiotaHost.GetOptionalExcludeBackwardCompatibleOption(); + var makeRequiredPropertiesNonNullableOption = KiotaHost.GetOptionalMakeRequiredPropertiesNonNullableOption(); var additionalDataOption = KiotaHost.GetOptionalAdditionalDataOption(); var structuredMimeTypesOption = KiotaHost.GetStructuredMimeTypesOption([]); var (includePatterns, excludePatterns) = KiotaHost.GetIncludeAndExcludeOptions([], []); @@ -138,6 +142,7 @@ public static Command GetEditCommand(IServiceProvider serviceProvider) logLevelOption, backingStoreOption, excludeBackwardCompatible, + makeRequiredPropertiesNonNullableOption, additionalDataOption, structuredMimeTypesOption, includePatterns, @@ -156,6 +161,7 @@ public static Command GetEditCommand(IServiceProvider serviceProvider) LogLevelOption = logLevelOption, BackingStoreOption = backingStoreOption, ExcludeBackwardCompatibleOption = excludeBackwardCompatible, + MakeRequiredPropertiesNonNullableOption = makeRequiredPropertiesNonNullableOption, AdditionalDataOption = additionalDataOption, StructuredMimeTypesOption = structuredMimeTypesOption, IncludePatternsOption = includePatterns, diff --git a/src/kiota/KiotaHost.cs b/src/kiota/KiotaHost.cs index b5e17e2d9f..3af4bc941d 100644 --- a/src/kiota/KiotaHost.cs +++ b/src/kiota/KiotaHost.cs @@ -589,6 +589,8 @@ private static Command GetGenerateCommand(IServiceProvider serviceProvider) var disableSSLValidationOption = GetDisableSSLValidationOption(defaultConfiguration.DisableSSLValidation); + var makeRequiredPropertiesNonNullableOption = GetMakeRequiredPropertiesNonNullableOption(defaultConfiguration.MakeRequiredPropertiesNonNullable); + var command = new Command("generate", "Generates a REST HTTP API client from an OpenAPI description file.") { descriptionOption, manifestOption, @@ -610,6 +612,7 @@ private static Command GetGenerateCommand(IServiceProvider serviceProvider) dvrOption, clearCacheOption, disableSSLValidationOption, + makeRequiredPropertiesNonNullableOption, }; command.Action = new KiotaGenerateCommandHandler { @@ -633,6 +636,7 @@ private static Command GetGenerateCommand(IServiceProvider serviceProvider) DisabledValidationRulesOption = dvrOption, ClearCacheOption = clearCacheOption, DisableSSLValidationOption = disableSSLValidationOption, + MakeRequiredPropertiesNonNullableOption = makeRequiredPropertiesNonNullableOption, ServiceProvider = serviceProvider, }; return command; @@ -719,6 +723,27 @@ private static Option GetDisableSSLValidationOption(bool defaultValue) return disableSSLValidationOption; } + private const string MakeRequiredPropertiesNonNullableOptionDescription = "When enabled, properties marked as required in the OpenAPI description and not explicitly nullable are generated as non-nullable (and non-optional, for languages that distinguish the two). Currently supported for TypeScript, Python, and Java. Disabled by default to preserve the previous behavior where all properties are nullable."; + internal static Option GetMakeRequiredPropertiesNonNullableOption(bool defaultValue = false) + { + var option = new Option("--make-required-properties-non-nullable") + { + DefaultValueFactory = _ => defaultValue, + Description = MakeRequiredPropertiesNonNullableOptionDescription, + }; + option.Aliases.Add("--mrpnn"); + return option; + } + internal static Option GetOptionalMakeRequiredPropertiesNonNullableOption() + { + var option = new Option("--make-required-properties-non-nullable") + { + Description = MakeRequiredPropertiesNonNullableOptionDescription, + }; + option.Aliases.Add("--mrpnn"); + return option; + } + private static void AddStringRegexValidator(Option option, Regex validator, string parameterName, bool allowEmpty = false) { option.Validators.Add(input => diff --git a/tests/Kiota.Builder.Tests/Extensions/OpenApiSchemaExtensionsTests.cs b/tests/Kiota.Builder.Tests/Extensions/OpenApiSchemaExtensionsTests.cs index 48657c3b19..a4885ecdd8 100644 --- a/tests/Kiota.Builder.Tests/Extensions/OpenApiSchemaExtensionsTests.cs +++ b/tests/Kiota.Builder.Tests/Extensions/OpenApiSchemaExtensionsTests.cs @@ -74,6 +74,30 @@ public void IsExclusiveUnionMatchesTypeArrays() }.IsExclusiveUnion()); } [Fact] + public void IsExplicitlyNullableDetectsNullMarkers() + { + // OAS 3.1 type includes null + Assert.True(new OpenApiSchema { Type = JsonSchemaType.String | JsonSchemaType.Null }.IsExplicitlyNullable()); + // OAS 3.1 anyOf [ { type: null }, ... ] + Assert.True(new OpenApiSchema + { + AnyOf = [new OpenApiSchema { Type = JsonSchemaType.Null }, new OpenApiSchema { Type = JsonSchemaType.String }] + }.IsExplicitlyNullable()); + // OAS 3.1 oneOf [ { type: null }, ... ] + Assert.True(new OpenApiSchema + { + OneOf = [new OpenApiSchema { Type = JsonSchemaType.Null }, new OpenApiSchema { Type = JsonSchemaType.String }] + }.IsExplicitlyNullable()); + // Non-nullable scalar + Assert.False(new OpenApiSchema { Type = JsonSchemaType.String }.IsExplicitlyNullable()); + // anyOf without a null member + Assert.False(new OpenApiSchema + { + AnyOf = [new OpenApiSchema { Type = JsonSchemaType.String }, new OpenApiSchema { Type = JsonSchemaType.Number }] + }.IsExplicitlyNullable()); + Assert.False(((IOpenApiSchema)null).IsExplicitlyNullable()); + } + [Fact] public void ExternalReferencesAreSupported() { var mockSchema = new OpenApiSchemaReference("example.json#/path/to/component", null, "http://example.com/example.json"); diff --git a/tests/Kiota.Builder.Tests/KiotaBuilderTests.cs b/tests/Kiota.Builder.Tests/KiotaBuilderTests.cs index 2876ff7637..c809365647 100644 --- a/tests/Kiota.Builder.Tests/KiotaBuilderTests.cs +++ b/tests/Kiota.Builder.Tests/KiotaBuilderTests.cs @@ -12,6 +12,7 @@ using Kiota.Builder.CodeDOM; using Kiota.Builder.Configuration; using Kiota.Builder.Extensions; +using Kiota.Builder.Refiners; using Microsoft.Extensions.Logging; using Microsoft.Extensions.Logging.Abstractions; @@ -11402,4 +11403,231 @@ public async Task PerOperationUrlTemplateOverrideSetForOptionalQueryOutlierAgain Assert.NotNull(patchGenerator); Assert.False(patchGenerator.HasUrlTemplateOverride); } + + #region Issue-3911 — required/nullable OAS properties → IsRequired / IsNullable + + private async Task GetItemPropertyAsync(string schemasYaml, string propertyName, bool makeRequiredPropertiesNonNullable, string openApiVersion = "3.0.1", GenerationLanguage language = GenerationLanguage.TypeScript, bool applyLanguageRefinement = false) + { + var tempFilePath = Path.GetTempFileName(); + _tempFiles.Add(tempFilePath); + await using var fs = await GetDocumentStreamAsync(@"openapi: " + openApiVersion + @" +info: + title: Test + version: 1.0.0 +servers: + - url: https://example.com/v1.0 +paths: + /items: + get: + responses: + '200': + content: + application/json: + schema: + $ref: '#/components/schemas/Item' +components: + schemas: +" + schemasYaml); + var mockLogger = new Mock>(); + var configuration = new GenerationConfiguration { ClientClassName = "Graph", OpenAPIFilePath = tempFilePath, MakeRequiredPropertiesNonNullable = makeRequiredPropertiesNonNullable, Language = language }; + var builder = new KiotaBuilder(mockLogger.Object, configuration, _httpClient); + var document = await builder.CreateOpenApiDocumentAsync(fs, cancellationToken: TestContext.Current.CancellationToken); + var node = builder.CreateUriSpace(document); + var codeModel = builder.CreateSourceModel(node); + if (applyLanguageRefinement) + await ILanguageRefiner.RefineAsync(configuration, codeModel, cancellationToken: TestContext.Current.CancellationToken); + // Search the whole tree: language refiners (e.g. Go) reorganize namespaces, so the "ApiSdk.models" path is not stable post-refinement. + var item = codeModel.FindChildByName("Item", true); + Assert.NotNull(item); + var prop = item.Properties.FirstOrDefault(p => p.Name.Equals(propertyName, StringComparison.OrdinalIgnoreCase)); + Assert.NotNull(prop); + return prop; + } + + private const string RequiredNonNullableStringSchema = @" Item: + type: object + required: + - name + properties: + name: + type: string"; + + [Fact] + public async Task RequiredNonNullableString_FlagOn_IsNullableFalse_IsRequiredTrue() + { + var prop = await GetItemPropertyAsync(RequiredNonNullableStringSchema, "name", makeRequiredPropertiesNonNullable: true); + Assert.False(prop.Type.IsNullable); + Assert.True(prop.IsRequired); + } + + [Fact] + public async Task RequiredNonNullableString_FlagOff_IsNullableTrue_IsRequiredTrue() + { + // Opt-out: IsNullable stays true (historical behavior), but IsRequired is still set accurately. + var prop = await GetItemPropertyAsync(RequiredNonNullableStringSchema, "name", makeRequiredPropertiesNonNullable: false); + Assert.True(prop.Type.IsNullable); + Assert.True(prop.IsRequired); + } + + [Fact] + public async Task RequiredNullableString_FlagOn_IsNullableTrue_IsRequiredTrue() + { + var prop = await GetItemPropertyAsync(@" Item: + type: object + required: + - name + properties: + name: + type: string + nullable: true", "name", makeRequiredPropertiesNonNullable: true); + Assert.True(prop.Type.IsNullable); + Assert.True(prop.IsRequired); + } + + [Fact] + public async Task OptionalNonNullableString_FlagOn_IsNullableTrue_IsRequiredFalse() + { + var prop = await GetItemPropertyAsync(@" Item: + type: object + properties: + name: + type: string", "name", makeRequiredPropertiesNonNullable: true); + Assert.True(prop.Type.IsNullable); + Assert.False(prop.IsRequired); + } + + [Fact] + public async Task RequiredNonNullableInteger_FlagOn_IsNullableFalse_IsRequiredTrue() + { + var prop = await GetItemPropertyAsync(@" Item: + type: object + required: + - count + properties: + count: + type: integer", "count", makeRequiredPropertiesNonNullable: true); + Assert.False(prop.Type.IsNullable); + Assert.True(prop.IsRequired); + } + + [Fact] + public async Task RequiredNonNullableObjectRef_FlagOn_IsNullableFalse_IsRequiredTrue() + { + var prop = await GetItemPropertyAsync(@" Item: + type: object + required: + - owner + properties: + owner: + $ref: '#/components/schemas/Owner' + Owner: + type: object + properties: + id: + type: string", "owner", makeRequiredPropertiesNonNullable: true); + Assert.False(prop.Type.IsNullable); + Assert.True(prop.IsRequired); + } + + [Fact] + public async Task RequiredNonNullableEnum_FlagOn_IsNullableFalse_IsRequiredTrue() + { + var prop = await GetItemPropertyAsync(@" Item: + type: object + required: + - status + properties: + status: + $ref: '#/components/schemas/Status' + Status: + type: string + enum: + - active + - inactive", "status", makeRequiredPropertiesNonNullable: true); + Assert.False(prop.Type.IsNullable); + Assert.True(prop.IsRequired); + } + + [Fact] + public async Task RequiredCollection_FlagOn_IsNullableTrue_IsRequiredTrue() + { + // Collections keep IsNullable = true: the element nullability and serializer API depend on it. + // Only scalar/object/enum required properties are made non-nullable. + var prop = await GetItemPropertyAsync(@" Item: + type: object + required: + - tags + properties: + tags: + type: array + items: + type: string", "tags", makeRequiredPropertiesNonNullable: true); + Assert.True(prop.Type.IsNullable); + Assert.True(prop.IsRequired); + } + + [Fact] + public async Task RequiredNonNullableString_Oas31_FlagOn_IsNullableFalse_IsRequiredTrue() + { + // OAS 3.1 scalar type form: a required, non-nullable property is made non-nullable just like OAS 3.0. + var prop = await GetItemPropertyAsync(@" Item: + type: object + required: + - name + properties: + name: + type: string", "name", makeRequiredPropertiesNonNullable: true, openApiVersion: "3.1.0"); + Assert.False(prop.Type.IsNullable); + Assert.True(prop.IsRequired); + } + + [Fact] + public async Task RequiredExplicitlyNullableString_Oas31_FlagOn_IsNullableTrue_IsRequiredTrue() + { + // OAS 3.1 explicit-null form (`type: [string, "null"]`): the property is required but explicitly nullable, + // so it must stay nullable even with the flag on. + var prop = await GetItemPropertyAsync(@" Item: + type: object + required: + - name + properties: + name: + type: + - string + - 'null'", "name", makeRequiredPropertiesNonNullable: true, openApiVersion: "3.1.0"); + Assert.True(prop.Type.IsNullable); + Assert.True(prop.IsRequired); + } + + [Theory] + [InlineData(GenerationLanguage.Python)] + [InlineData(GenerationLanguage.Java)] + public async Task RequiredNonNullableString_FlagOn_SupportedLanguage_StaysNonNullableAfterRefinement(GenerationLanguage language) + { + // Supported language: the flip must survive the full pipeline. Unlike PHP/Go, these refiners do not re-nullify + // model properties, so a required non-nullable property stays non-nullable through refinement. + // (TypeScript is validated at the writer level instead — it lowers models to interfaces during refinement.) + var prop = await GetItemPropertyAsync(RequiredNonNullableStringSchema, "name", makeRequiredPropertiesNonNullable: true, language: language, applyLanguageRefinement: true); + Assert.False(prop.Type.IsNullable); + Assert.True(prop.IsRequired); + } + + [Theory] + [InlineData(GenerationLanguage.CSharp)] + [InlineData(GenerationLanguage.Go)] + [InlineData(GenerationLanguage.PHP)] + [InlineData(GenerationLanguage.Dart)] + public async Task RequiredNonNullableString_FlagOn_UnsupportedLanguage_StaysNullableAfterRefinement(GenerationLanguage language) + { + // The flag is only honored for the validated languages (TypeScript, Python, Java). For every other language the + // property must keep its historical nullable rendering so generated output is unchanged until support is + // added and validated for it. This runs the FULL pipeline (build + language refinement) to prove that + // neither the builder gate nor any language refiner (e.g. PHP/Go MakeModelPropertiesNullable) leaves the + // property non-nullable — i.e. enabling the flag is a safe no-op for these languages. + var prop = await GetItemPropertyAsync(RequiredNonNullableStringSchema, "name", makeRequiredPropertiesNonNullable: true, language: language, applyLanguageRefinement: true); + Assert.True(prop.Type.IsNullable); + Assert.True(prop.IsRequired); + } + + #endregion } diff --git a/tests/Kiota.Builder.Tests/WorkspaceManagement/ApiClientConfigurationTests.cs b/tests/Kiota.Builder.Tests/WorkspaceManagement/ApiClientConfigurationTests.cs index 130b407c8b..81ef60a1e2 100644 --- a/tests/Kiota.Builder.Tests/WorkspaceManagement/ApiClientConfigurationTests.cs +++ b/tests/Kiota.Builder.Tests/WorkspaceManagement/ApiClientConfigurationTests.cs @@ -16,6 +16,7 @@ public void Clones() ClientNamespaceName = "foo", DescriptionLocation = "bar", ExcludeBackwardCompatible = true, + MakeRequiredPropertiesNonNullable = true, ExcludePatterns = [ "exclude" ], @@ -35,6 +36,7 @@ public void Clones() Assert.Equal(clientConfig.ClientNamespaceName, cloned.ClientNamespaceName); Assert.Equal(clientConfig.DescriptionLocation, cloned.DescriptionLocation); Assert.Equal(clientConfig.ExcludeBackwardCompatible, cloned.ExcludeBackwardCompatible); + Assert.Equal(clientConfig.MakeRequiredPropertiesNonNullable, cloned.MakeRequiredPropertiesNonNullable); Assert.Equal(clientConfig.ExcludePatterns, cloned.ExcludePatterns); Assert.Equal(clientConfig.IncludeAdditionalData, cloned.IncludeAdditionalData); Assert.Equal(clientConfig.IncludePatterns, cloned.IncludePatterns); @@ -53,6 +55,7 @@ public void CreatesApiClientConfigurationFromGenerationConfiguration() ClientClassName = "client", ClientNamespaceName = "namespace", ExcludeBackwardCompatible = true, + MakeRequiredPropertiesNonNullable = true, ExcludePatterns = ["exclude"], IncludeAdditionalData = true, IncludePatterns = ["include"], @@ -67,6 +70,7 @@ public void CreatesApiClientConfigurationFromGenerationConfiguration() Assert.Equal(generationConfiguration.ClientNamespaceName, clientConfig.ClientNamespaceName); Assert.Equal(generationConfiguration.OpenAPIFilePath, clientConfig.DescriptionLocation); Assert.Equal(generationConfiguration.ExcludeBackwardCompatible, clientConfig.ExcludeBackwardCompatible); + Assert.Equal(generationConfiguration.MakeRequiredPropertiesNonNullable, clientConfig.MakeRequiredPropertiesNonNullable); Assert.Equal(generationConfiguration.ExcludePatterns, clientConfig.ExcludePatterns); Assert.Equal(generationConfiguration.IncludeAdditionalData, clientConfig.IncludeAdditionalData); Assert.Equal(generationConfiguration.IncludePatterns, clientConfig.IncludePatterns); @@ -83,6 +87,7 @@ public void UpdatesGenerationConfigurationFromApiClientConfiguration() ClientNamespaceName = "namespace", DescriptionLocation = "openapi", ExcludeBackwardCompatible = true, + MakeRequiredPropertiesNonNullable = true, ExcludePatterns = ["exclude"], IncludeAdditionalData = true, IncludePatterns = ["include"], @@ -108,6 +113,7 @@ public void UpdatesGenerationConfigurationFromApiClientConfiguration() Assert.Equal(GenerationLanguage.CSharp, generationConfiguration.Language); Assert.Equal(clientConfiguration.DescriptionLocation, generationConfiguration.OpenAPIFilePath); Assert.Equal(clientConfiguration.ExcludeBackwardCompatible, generationConfiguration.ExcludeBackwardCompatible); + Assert.Equal(clientConfiguration.MakeRequiredPropertiesNonNullable, generationConfiguration.MakeRequiredPropertiesNonNullable); Assert.Equal(clientConfiguration.ExcludePatterns, generationConfiguration.ExcludePatterns); Assert.Equal(clientConfiguration.IncludeAdditionalData, generationConfiguration.IncludeAdditionalData); Assert.Equal(clientConfiguration.IncludePatterns, generationConfiguration.IncludePatterns); diff --git a/tests/Kiota.Builder.Tests/Writers/Java/CodePropertyWriterTests.cs b/tests/Kiota.Builder.Tests/Writers/Java/CodePropertyWriterTests.cs index 21e75ec198..2c130ec500 100644 --- a/tests/Kiota.Builder.Tests/Writers/Java/CodePropertyWriterTests.cs +++ b/tests/Kiota.Builder.Tests/Writers/Java/CodePropertyWriterTests.cs @@ -106,6 +106,19 @@ public void WritesNonNull() Assert.Contains("@jakarta.annotation.Nonnull", result); } [Fact] + public void WritesRequiredNonNullableProperty_FlagOn_Nonnull() + { + // With MakeRequiredPropertiesNonNullable on, the builder sets IsNullable=false on a required property; the Java + // writer renders it @Nonnull (and never @Nullable). @Nonnull is an annotation, so an uninitialized field is valid Java. + property.Kind = CodePropertyKind.Custom; + property.IsRequired = true; + (property.Type as CodeType).IsNullable = false; + writer.Write(property); + var result = tw.ToString(); + Assert.Contains("@jakarta.annotation.Nonnull", result); + Assert.DoesNotContain("@jakarta.annotation.Nullable", result); + } + [Fact] public void WritesCollectionFlagEnumsAsOneDimensionalArray() { property.Kind = CodePropertyKind.Custom; diff --git a/tests/Kiota.Builder.Tests/Writers/Python/CodeMethodWriterTests.cs b/tests/Kiota.Builder.Tests/Writers/Python/CodeMethodWriterTests.cs index c05f3f317b..a4932d504a 100644 --- a/tests/Kiota.Builder.Tests/Writers/Python/CodeMethodWriterTests.cs +++ b/tests/Kiota.Builder.Tests/Writers/Python/CodeMethodWriterTests.cs @@ -1719,6 +1719,87 @@ public void WritesConstructor() Assert.Contains($"self.{propName}: Optional[str] = None", result); Assert.DoesNotContain("get_path_parameters(", result); } + [Fact] + public void WritesModelRequiredNonNullablePrimitive_NoOptional_TypeCorrectDefault() + { + // A required, non-nullable primitive renders as a non-Optional dataclass field with a type-correct zero-value + // default (str -> ''). The default is needed for no-arg construction (Model()) and valid field ordering; using + // '' instead of None keeps the field type-correct (str = None would be rejected by mypy/pyright). + setup(); + method.Kind = CodeMethodKind.Constructor; + method.IsAsync = false; + var propName = "product_id"; + parentClass.Kind = CodeClassKind.Model; + parentClass.AddProperty(new CodeProperty + { + Name = propName, + Kind = CodePropertyKind.Custom, + IsRequired = true, + Type = new CodeType + { + Name = "string", + IsNullable = false, + } + }); + writer.Write(method); + var result = tw.ToString(); + Assert.Contains($"{propName}: str = ''", result); + Assert.DoesNotContain("Optional[", result); + Assert.DoesNotContain($"{propName}: str = None", result); + } + + [Fact] + public void WritesModelRequiredNonNullableObject_NoOptional_NoneDefault() + { + // A required, non-nullable object/enum has no zero-value literal, so it renders non-Optional with a None + // default — an advisory non-null (the deserializer always overwrites it), matching Java's @Nonnull posture. + setup(); + method.Kind = CodeMethodKind.Constructor; + method.IsAsync = false; + var propName = "owner"; + parentClass.Kind = CodeClassKind.Model; + parentClass.AddProperty(new CodeProperty + { + Name = propName, + Kind = CodePropertyKind.Custom, + IsRequired = true, + Type = new CodeType + { + Name = "Owner", + IsNullable = false, + } + }); + writer.Write(method); + var result = tw.ToString(); + Assert.Contains($"{propName}: Owner = None", result); + Assert.DoesNotContain("Optional[", result); + } + + [Fact] + public void WritesModelNullableProperty_Optional_KeepsNoneDefault() + { + // Opt-out / nullable: a nullable model property keeps the historical Optional[...] = None form. + setup(); + method.Kind = CodeMethodKind.Constructor; + method.IsAsync = false; + var propName = "product_id"; + parentClass.Kind = CodeClassKind.Model; + parentClass.AddProperty(new CodeProperty + { + Name = propName, + Kind = CodePropertyKind.Custom, + IsRequired = true, + Type = new CodeType + { + Name = "string", + IsNullable = true, + } + }); + writer.Write(method); + var result = tw.ToString(); + Assert.Contains($"{propName}: Optional[str] = None", result); + } + [Fact] public void EscapesCommentCharactersInDescription() { diff --git a/tests/Kiota.Builder.Tests/Writers/TypeScript/CodeFunctionWriterTests.cs b/tests/Kiota.Builder.Tests/Writers/TypeScript/CodeFunctionWriterTests.cs index 58f53a961c..05c8c477af 100644 --- a/tests/Kiota.Builder.Tests/Writers/TypeScript/CodeFunctionWriterTests.cs +++ b/tests/Kiota.Builder.Tests/Writers/TypeScript/CodeFunctionWriterTests.cs @@ -1686,6 +1686,42 @@ export function createPrimitivesFromDiscriminatorValue(parseNode: ParseNode | un AssertExtensions.CurlyBracesAreClosed(result, 1); } + [Fact] + public async Task Writes_UnionOfPrimitiveValues_FactoryFunction_KeepsUndefinedWhenReturnTypeNonNullableAsync() + { + // Regression for MakeRequiredPropertiesNonNullable: a required scalar-alias reference can flip a + // primitive-composed factory's ReturnType non-nullable. Its body is always `parseNode?.get*Value()` + // (`T | undefined`), so the signature must keep `| undefined` or tsc fails — the writer forces these + // factory return types nullable regardless of the flag. + var generationConfiguration = new GenerationConfiguration { Language = GenerationLanguage.TypeScript }; + var tempFilePath = Path.GetTempFileName(); + await File.WriteAllTextAsync(tempFilePath, UnionOfPrimitiveValuesSample.Yaml, cancellationToken: TestContext.Current.CancellationToken); + var mockLogger = new Mock>(); + var builder = new KiotaBuilder(mockLogger.Object, new GenerationConfiguration { ClientClassName = "Primitives", Serializers = ["none"], Deserializers = ["none"] }, _httpClient); + await using var fs = new FileStream(tempFilePath, FileMode.Open); + var document = await builder.CreateOpenApiDocumentAsync(fs, cancellationToken: TestContext.Current.CancellationToken); + var node = builder.CreateUriSpace(document); + builder.SetApiRootUrl(); + var codeModel = builder.CreateSourceModel(node); + var rootNS = codeModel.FindNamespaceByName("ApiSdk"); + Assert.NotNull(rootNS); + await ILanguageRefiner.RefineAsync(generationConfiguration, rootNS, cancellationToken: TestContext.Current.CancellationToken); + var modelsNS = rootNS.FindNamespaceByName("ApiSdk.primitives"); + Assert.NotNull(modelsNS); + var modelCodeFile = modelsNS.FindChildByName("primitivesRequestBuilder", false); + Assert.NotNull(modelCodeFile); + var factoryFunction = modelCodeFile.GetChildElements().FirstOrDefault(x => x is CodeFunction function && GetOriginalComposedType(function.OriginalLocalMethod.ReturnType) is not null) as CodeFunction; + Assert.NotNull(factoryFunction); + // Simulate the MakeRequiredPropertiesNonNullable flag tightening the composed return type. + factoryFunction.OriginalLocalMethod.ReturnType.IsNullable = false; + writer.Write(factoryFunction); + var result = tw.ToString(); + // The body returns `T | undefined`, so the declared return type must still carry `| undefined`. + Assert.Contains("| undefined {", result); + Assert.Contains("return parseNode?.getNumberValue() ?? parseNode?.getStringValue();", result); + AssertExtensions.CurlyBracesAreClosed(result, 1); + } + [Fact] public async Task Writes_UnionOfObjects_FactoryMethodAsync() { diff --git a/tests/Kiota.Builder.Tests/Writers/TypeScript/CodePropertyWriterTests.cs b/tests/Kiota.Builder.Tests/Writers/TypeScript/CodePropertyWriterTests.cs index 383dc1eb0d..4c47072653 100644 --- a/tests/Kiota.Builder.Tests/Writers/TypeScript/CodePropertyWriterTests.cs +++ b/tests/Kiota.Builder.Tests/Writers/TypeScript/CodePropertyWriterTests.cs @@ -145,4 +145,63 @@ public void DoesNodeEmitAdditionalDataPropertyOnInterfaces() var result = tw.ToString(); Assert.Empty(result); } + + private (LanguageWriter, StringWriter) GetFlagOnWriter() + { + var flagOnWriter = LanguageWriter.GetLanguageWriter(GenerationLanguage.TypeScript, DefaultPath, DefaultName, makeRequiredPropertiesNonNullable: true); + var sw = new StringWriter(); + flagOnWriter.SetTextWriter(sw); + return (flagOnWriter, sw); + } + + [Fact] + public void WritesRequiredNonNullableProperty_FlagOn_NoOptionalNoNull() + { + var (flagOnWriter, sw) = GetFlagOnWriter(); + property.Kind = CodePropertyKind.Custom; + property.Type.IsNullable = false; + property.IsRequired = true; + flagOnWriter.Write(property); + var result = sw.ToString(); + Assert.Contains($"{PropertyName}: {TypeName};", result); + Assert.DoesNotContain($"{PropertyName}?:", result); + Assert.DoesNotContain("| null", result); + } + + [Fact] + public void WritesRequiredNullableProperty_FlagOn_NoOptionalKeepsNull() + { + var (flagOnWriter, sw) = GetFlagOnWriter(); + property.Kind = CodePropertyKind.Custom; + property.Type.IsNullable = true; + property.IsRequired = true; + flagOnWriter.Write(property); + var result = sw.ToString(); + Assert.Contains($"{PropertyName}: {TypeName} | null;", result); + Assert.DoesNotContain($"{PropertyName}?:", result); + } + + [Fact] + public void WritesOptionalProperty_FlagOn_OptionalAndNull() + { + var (flagOnWriter, sw) = GetFlagOnWriter(); + property.Kind = CodePropertyKind.Custom; + property.Type.IsNullable = false; + property.IsRequired = false; + flagOnWriter.Write(property); + var result = sw.ToString(); + Assert.Contains($"{PropertyName}?: {TypeName} | null;", result); + } + + [Fact] + public void WritesRequiredProperty_FlagOff_OptionalAndNull() + { + // Opt-out: with the flag off, a required non-nullable property keeps the historical optional+nullable form. + property.Kind = CodePropertyKind.Custom; + property.Type.IsNullable = false; + property.IsRequired = true; + writer.Write(property); // default writer has the flag off + var result = tw.ToString(); + Assert.Contains($"{PropertyName}?: {TypeName} | null;", result); + } }