Skip to content
Open
Show file tree
Hide file tree
Changes from all commits
Commits
File filter

Filter by extension

Filter by extension

Conversations
Failed to load comments.
Loading
Jump to
Jump to file
Failed to load files.
Loading
Diff view
Diff view
1 change: 1 addition & 0 deletions Directory.Build.props
Original file line number Diff line number Diff line change
Expand Up @@ -29,6 +29,7 @@
<DebugSymbols>true</DebugSymbols>
<DebugType>portable</DebugType>
<LangVersion>13.0</LangVersion>
<Nullable>enable</Nullable>
Copy link

Copilot AI Feb 4, 2026

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Enabling nullable reference types globally in Directory.Build.props will affect all projects in the solution, including existing handwritten code that may not be nullable-aware. This is a significant breaking change that could introduce numerous compiler warnings throughout the codebase. Consider:

  1. Verifying that all projects build without warnings or document the expected warnings
  2. Providing a migration guide for consumers of the library
  3. Considering a phased approach where nullable is enabled only for generated code initially using #nullable enable directives in generated files
  4. If global nullable enablement is intended, ensure all existing code has been reviewed for nullable compliance

This is a major architectural decision that could impact maintainability and should be carefully validated before merging.

Copilot uses AI. Check for mistakes.
</PropertyGroup>

<PropertyGroup Condition="'$(GITHUB_ACTIONS)' == 'true'">
Expand Down
43 changes: 22 additions & 21 deletions src/LibKubernetesGenerator/TypeHelper.cs
Original file line number Diff line number Diff line change
Expand Up @@ -23,7 +23,7 @@
scriptObject.Import(nameof(IfType), new Func<JsonSchemaProperty, string, bool>(IfType));
}

private string GetDotNetType(JsonObjectType jsonType, string name, bool required, string format)
private string GetDotNetType(JsonObjectType jsonType, string name, bool required, string? format)
{
if (name == "pretty" && !required)
{
Expand Down Expand Up @@ -81,7 +81,7 @@
switch (format)
{
case "byte":
return "byte[]";
return required ? "byte[]" : "byte[]?";
case "date-time":

// eventTime is required but should be optional, see https://github.com/kubernetes-client/csharp/issues/1197
Expand All @@ -100,58 +100,58 @@
}
}

return "string";
return required ? "string" : "string?";
case JsonObjectType.Object:
return "object";
return required ? "object" : "object?";
default:
throw new NotSupportedException();
}
}

private string GetDotNetType(JsonSchema schema, JsonSchemaProperty parent)
private string GetDotNetType(JsonSchema? schema, JsonSchemaProperty parent, bool isCollectionItem = false)
{
if (schema != null)
{
if (schema.IsArray)
if (schema.IsArray && schema.Item != null)
{
return $"IList<{GetDotNetType(schema.Item, parent)}>";
return $"IList<{GetDotNetType(schema.Item, parent, isCollectionItem: true)}>?";
}

if (schema.IsDictionary && schema.AdditionalPropertiesSchema != null)
{
return $"IDictionary<string, {GetDotNetType(schema.AdditionalPropertiesSchema, parent)}>";
return $"IDictionary<string, {GetDotNetType(schema.AdditionalPropertiesSchema, parent, isCollectionItem: true)}>?";
}

if (schema?.Reference != null)
if (schema.Reference != null)
{
return classNameHelper.GetClassNameForSchemaDefinition(schema.Reference);
var typeName = classNameHelper.GetClassNameForSchemaDefinition(schema.Reference);
// Collection items are always non-nullable, unless we're at the root level
return (isCollectionItem || parent.IsRequired) ? typeName : typeName + "?";
}

if (schema != null)
{
return GetDotNetType(schema.Type, parent.Name, parent.IsRequired, schema.Format);
}
return GetDotNetType(schema.Type, parent.Name, isCollectionItem || parent.IsRequired, schema.Format);
}

return GetDotNetType(parent.Type, parent.Name, parent.IsRequired, parent.Format);
return GetDotNetType(parent.Type, parent.Name, isCollectionItem || parent.IsRequired, parent.Format);
}

public string GetDotNetType(JsonSchemaProperty p)
{
if (p.Reference != null)
{
return classNameHelper.GetClassNameForSchemaDefinition(p.Reference);
var typeName = classNameHelper.GetClassNameForSchemaDefinition(p.Reference);
return p.IsRequired ? typeName : typeName + "?";
}

if (p.IsArray)
if (p.IsArray && p.Item != null)
{
// getType
return $"IList<{GetDotNetType(p.Item, p)}>";
// getType - items in arrays are non-nullable
return $"IList<{GetDotNetType(p.Item, p, isCollectionItem: true)}>?";
}

if (p.IsDictionary && p.AdditionalPropertiesSchema != null)
{
return $"IDictionary<string, {GetDotNetType(p.AdditionalPropertiesSchema, p)}>";
return $"IDictionary<string, {GetDotNetType(p.AdditionalPropertiesSchema, p, isCollectionItem: true)}>?";
Comment on lines +117 to +154
Copy link

Copilot AI Feb 4, 2026

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

The current implementation always makes collections nullable (lines 117, 122, 149, 154) regardless of whether they are top-level properties or items within another collection. This means nested collections like IList of IList of string would be typed as IList<IList?>? where the inner lists are nullable.

However, the design goal stated in the PR is that collection items should be non-nullable. For consistency, nested collections that are collection items should probably be non-nullable. Consider modifying the logic to:

  • Make collections nullable only when they are top-level properties (not collection items)
  • For example, line 117 could be: return isCollectionItem ? $"IList&lt;{{...}}&gt;" : $"IList<{{...}}>?";

This would ensure IList<IList?> where the inner collection is non-nullable (cannot have null inner lists), while the items in those inner lists could still be nullable if needed.

Copilot uses AI. Check for mistakes.
}

return GetDotNetType(p.Type, p.Name, p.IsRequired, p.Format);
Expand All @@ -161,7 +161,8 @@
{
if (parameter.Schema?.Reference != null)
{
return classNameHelper.GetClassNameForSchemaDefinition(parameter.Schema.Reference);
var typeName = classNameHelper.GetClassNameForSchemaDefinition(parameter.Schema.Reference);
return parameter.IsRequired ? typeName : typeName + "?";
}
else if (parameter.Schema != null)
{
Expand Down Expand Up @@ -298,7 +299,7 @@
var listSchema = response?.Schema?.Reference;
if (listSchema?.Properties?.TryGetValue("items", out var itemsProperty) != true)
{
return null;

Check warning on line 302 in src/LibKubernetesGenerator/TypeHelper.cs

View workflow job for this annotation

GitHub Actions / Dotnet build (ubuntu-latest)

Possible null reference return.

Check warning on line 302 in src/LibKubernetesGenerator/TypeHelper.cs

View workflow job for this annotation

GitHub Actions / Analyze (csharp)

Possible null reference return.

Check warning on line 302 in src/LibKubernetesGenerator/TypeHelper.cs

View workflow job for this annotation

GitHub Actions / e2e

Possible null reference return.

Check warning on line 302 in src/LibKubernetesGenerator/TypeHelper.cs

View workflow job for this annotation

GitHub Actions / Dotnet build (macOS-latest)

Possible null reference return.
}

if (itemsProperty.Reference != null)
Expand All @@ -311,6 +312,6 @@
return classNameHelper.GetClassNameForSchemaDefinition(itemsProperty.Item.Reference);
}

return null;

Check warning on line 315 in src/LibKubernetesGenerator/TypeHelper.cs

View workflow job for this annotation

GitHub Actions / Analyze (csharp)

Possible null reference return.

Check warning on line 315 in src/LibKubernetesGenerator/TypeHelper.cs

View workflow job for this annotation

GitHub Actions / e2e

Possible null reference return.

Check warning on line 315 in src/LibKubernetesGenerator/TypeHelper.cs

View workflow job for this annotation

GitHub Actions / Dotnet build (macOS-latest)

Possible null reference return.
}
}
Expand Down
95 changes: 95 additions & 0 deletions tests/KubernetesClient.Tests/NullableReferenceTypesTests.cs
Original file line number Diff line number Diff line change
@@ -0,0 +1,95 @@
using System.Collections.Generic;
using k8s.Models;
using Xunit;

namespace k8s.Tests
{
public class NullableReferenceTypesTests
{
[Fact]
public void ContainerVolumeMountsIsNullableProperty()
{
// Arrange & Act
var container = new V1Container();

// Assert
// VolumeMounts should be null by default (nullable property)
Assert.Null(container.VolumeMounts);

// This should not throw NullReferenceException anymore - users should check for null
// container.VolumeMounts.Add(new V1VolumeMount()); // This would throw

// Proper usage: Initialize the list first
container.VolumeMounts = new List<V1VolumeMount>
{
new V1VolumeMount(),
};

Assert.NotNull(container.VolumeMounts);
Assert.Single(container.VolumeMounts);
}

[Fact]
public void ContainerNameIsRequiredProperty()
{
// Arrange & Act
var container = new V1Container
{
Name = "test-container",
};

// Assert
// Name is a required property (non-nullable string)
Assert.NotNull(container.Name);
Assert.Equal("test-container", container.Name);
}

[Fact]
public void ContainerImageIsOptionalProperty()
{
// Arrange & Act
var container = new V1Container();
Copy link

Copilot AI Feb 4, 2026

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

If the Name property of V1Container is marked as required in the Kubernetes schema (which seems likely based on test ContainerNameIsRequiredProperty), this test will fail to compile because required properties must be initialized in object initializers. Consider either:

  1. Initializing Name in the object initializer: var container = new V1Container { Name = "test-container" };
  2. Using the SetsRequiredMembers attribute if this is intentional for testing purposes
  3. Verifying whether Name is actually required in the Kubernetes schema

The same issue may affect line 65 in the ContainerLifecycleIsOptionalComplexProperty test.

Copilot uses AI. Check for mistakes.

// Assert
// Image is optional (nullable string)
Assert.Null(container.Image);

container.Image = "nginx:latest";
Assert.Equal("nginx:latest", container.Image);
}

[Fact]
public void ContainerLifecycleIsOptionalComplexProperty()
{
// Arrange & Act
var container = new V1Container();

// Assert
// Lifecycle is optional (nullable reference type)
Assert.Null(container.Lifecycle);

container.Lifecycle = new V1Lifecycle();
Assert.NotNull(container.Lifecycle);
}

[Fact]
public void ContainerCollectionItemsAreNonNullable()
{
// Arrange
var container = new V1Container
{
// Initialize the list - the list itself can be null, but items cannot be null
VolumeMounts = new List<V1VolumeMount>
{
new V1VolumeMount { Name = "vol1", MountPath = "/data", },
new V1VolumeMount { Name = "vol2", MountPath = "/config", },
},
};

// Act & Assert
Assert.NotNull(container.VolumeMounts);
Assert.Equal(2, container.VolumeMounts.Count);
Assert.All(container.VolumeMounts, vm => Assert.NotNull(vm));
}
}
}
Loading