Skip to content

Commit

Permalink
.Net: Fix nullable reference type warnings (#3510)
Browse files Browse the repository at this point in the history
### Motivation and Context

I temporarily retargeted all of the projects to net8.0 and fixed all of
the NRT warnings, some of which were noise, and some of which were
legit.

### Contribution Checklist

<!-- Before submitting this PR, please make sure: -->

- [x] The code builds clean without any errors or warnings
- [x] The PR follows the [SK Contribution
Guidelines](https://github.com/microsoft/semantic-kernel/blob/main/CONTRIBUTING.md)
and the [pre-submission formatting
script](https://github.com/microsoft/semantic-kernel/blob/main/CONTRIBUTING.md#development-scripts)
raises no violations
- [x] All unit tests pass, and I have added new tests where possible
- [x] I didn't break anyone 😄

---------

Co-authored-by: Roger Barreto <19890735+RogerBarreto@users.noreply.github.com>
  • Loading branch information
stephentoub and RogerBarreto committed Nov 15, 2023
1 parent 47b8794 commit 904f592
Show file tree
Hide file tree
Showing 35 changed files with 106 additions and 101 deletions.
Expand Up @@ -70,17 +70,16 @@ public HuggingFaceTextEmbeddingGeneration(string model, HttpClient httpClient, s
{
Verify.NotNullOrWhiteSpace(model);
Verify.NotNull(httpClient);

this._model = model;
this._endpoint = endpoint;
this._httpClient = httpClient;
this._attributes.Add(IAIServiceExtensions.ModelIdKey, this._model);
this._attributes.Add(IAIServiceExtensions.EndpointKey, this._endpoint ?? this._httpClient.BaseAddress.ToString());

if (httpClient.BaseAddress == null && string.IsNullOrEmpty(endpoint))
{
throw new SKException("The HttpClient BaseAddress and endpoint are both null or empty. Please ensure at least one is provided.");
}

this._model = model;
this._endpoint = endpoint;
this._httpClient = httpClient;
this._attributes.Add(IAIServiceExtensions.ModelIdKey, model);
this._attributes.Add(IAIServiceExtensions.EndpointKey, endpoint ?? httpClient.BaseAddress!.ToString());
}

/// <inheritdoc/>
Expand Down
Expand Up @@ -61,7 +61,7 @@ private protected ClientBase(ILoggerFactory? loggerFactory = null)
/// <summary>
/// Instance of <see cref="Meter"/> for metrics.
/// </summary>
private static readonly Meter s_meter = new(typeof(ClientBase).Assembly.GetName().Name);
private static readonly Meter s_meter = new(typeof(ClientBase).Assembly.GetName().Name!);

/// <summary>
/// Instance of <see cref="Counter{T}"/> to keep track of the number of prompt tokens used.
Expand Down
Expand Up @@ -34,7 +34,9 @@ public Task<ChatMessageBase> GetChatMessageAsync(CancellationToken cancellationT
var message = this._choice.Messages
.FirstOrDefault(message => message.Role.Equals(AuthorRole.Assistant.Label, StringComparison.Ordinal));

return Task.FromResult<ChatMessageBase>(new SKChatMessage(message.Role, message.Content));
return message is not null ?
Task.FromResult<ChatMessageBase>(new SKChatMessage(message.Role, message.Content)) :
Task.FromException<ChatMessageBase>(new SKException("No message found"));
}

public async Task<string> GetCompletionAsync(CancellationToken cancellationToken = default)
Expand Down
Expand Up @@ -399,7 +399,7 @@ private string NormalizeIndexName(string indexName)
private SearchClient GetSearchClient(string indexName)
{
// Search an available client from the local cache
if (!this._clientsByIndex.TryGetValue(indexName, out SearchClient client))
if (!this._clientsByIndex.TryGetValue(indexName, out SearchClient? client))
{
client = this._adminClient.GetSearchClient(indexName);
this._clientsByIndex[indexName] = client;
Expand Down
Expand Up @@ -31,6 +31,8 @@ public class ChromaClient : IChromaClient
/// <param name="loggerFactory">The <see cref="ILoggerFactory"/> to use for logging. If null, no logging will be performed.</param>
public ChromaClient(string endpoint, ILoggerFactory? loggerFactory = null)
{
Verify.NotNull(endpoint);

this._httpClient = new HttpClient(NonDisposableHttpClientHandler.Instance, disposeHandler: false);
this._endpoint = endpoint;
this._logger = loggerFactory is not null ? loggerFactory.CreateLogger(typeof(ChromaClient)) : NullLogger.Instance;
Expand Down Expand Up @@ -166,10 +168,10 @@ public async Task<ChromaQueryResultModel> QueryEmbeddingsAsync(string collection
HttpRequestMessage request,
CancellationToken cancellationToken = default)
{
string endpoint = this._endpoint ?? this._httpClient.BaseAddress.ToString();
string endpoint = this._endpoint ?? this._httpClient.BaseAddress!.ToString();
endpoint = this.SanitizeEndpoint(endpoint);

string operationName = request.RequestUri.ToString();
string operationName = request.RequestUri!.ToString();

request.RequestUri = new Uri(new Uri(endpoint), operationName);

Expand Down
Expand Up @@ -108,7 +108,7 @@ public override PodType Read(ref Utf8JsonReader reader, Type typeToConvert, Json
object? enumValue = Enum
.GetValues(typeToConvert)
.Cast<object?>()
.FirstOrDefault(value => value != null && typeToConvert.GetMember(value.ToString())[0]
.FirstOrDefault(value => value != null && typeToConvert.GetMember(value.ToString()!)[0]
.GetCustomAttribute(typeof(EnumMemberAttribute)) is EnumMemberAttribute enumMemberAttr && enumMemberAttr.Value == stringValue);

if (enumValue != null)
Expand Down
Expand Up @@ -551,7 +551,7 @@ private string GetIndexOperationsApiBasePath()

private async Task<string> GetIndexHostAsync(string indexName, CancellationToken cancellationToken = default)
{
if (this._indexHostMapping.TryGetValue(indexName, out string indexHost))
if (this._indexHostMapping.TryGetValue(indexName, out string? indexHost))
{
return indexHost;
}
Expand Down
Expand Up @@ -86,7 +86,7 @@ await foreach (PineconeDocument document in documents)
continue;
}

if (!document.Metadata.TryGetValue("text", out object value))
if (!document.Metadata.TryGetValue("text", out object? value))
{
yield return document;

Expand Down Expand Up @@ -159,7 +159,7 @@ await foreach (PineconeDocument record in data)
currentBatch = new List<PineconeDocument>(batchSize);
}

if (currentBatch.Count <= 0)
if (currentBatch.Count == 0)
{
yield break;
}
Expand All @@ -177,13 +177,6 @@ private static int GetMetadataSize(Dictionary<string, object> metadata)
return (int)stream.Length;
}

private static int GetEntrySize(KeyValuePair<string, object> entry)
{
Dictionary<string, object> temp = new() { { entry.Key, entry.Value } };

return GetMetadataSize(temp);
}

/// <summary>
/// Utility method to convert a dictionary of filters to the format expected by Pinecone.
/// </summary>
Expand Down
Expand Up @@ -81,8 +81,7 @@ static bool IsReservedIpAddress(string host)
throw new ArgumentException($"The {name} `{url}` is incomplete, enter a valid URL starting with 'https://", name);
}

bool result = Uri.TryCreate(url, UriKind.Absolute, out var uri);
if (!result || string.IsNullOrEmpty(uri.Host))
if (!Uri.TryCreate(url, UriKind.Absolute, out var uri) || string.IsNullOrEmpty(uri.Host))
{
throw new ArgumentException($"The {name} `{url}` is not valid", name);
}
Expand Down
Expand Up @@ -480,7 +480,7 @@ private static Uri SanitizeEndpoint(string endpoint, int? port = null)
//Apply endpoint override if it's specified.
if (this._endpointOverride != null)
{
request.RequestUri = new Uri(this._endpointOverride, request.RequestUri);
request.RequestUri = new Uri(this._endpointOverride, request.RequestUri!);
}

HttpResponseMessage response = await this._httpClient.SendWithSuccessCheckAsync(request, cancellationToken).ConfigureAwait(false);
Expand Down
Expand Up @@ -321,12 +321,12 @@ public async Task<string> UpsertAsync(string collectionName, MemoryRecord record

DateTimeOffset? timestamp = weaviateObject.Properties == null
? null
: weaviateObject.Properties.TryGetValue("sk_timestamp", out object value)
: weaviateObject.Properties.TryGetValue("sk_timestamp", out object? value)
? Convert.ToDateTime(value.ToString(), CultureInfo.InvariantCulture)
: null;

MemoryRecord record = new(
key: weaviateObject.Id!,
key: weaviateObject.Id,
timestamp: timestamp,
embedding: weaviateObject.Vector,
metadata: ToMetadata(weaviateObject));
Expand Down Expand Up @@ -502,7 +502,7 @@ private static MemoryRecord DeserializeToMemoryRecord(JsonNode? json)
// Get a class description, useful for checking name collisions
private static string ToWeaviateFriendlyClassDescription(string collectionName)
{
return $"{"Semantic Kernel memory store for collection:"} {collectionName}";
return $"Semantic Kernel memory store for collection: {collectionName}";
}

// Convert a collectionName to a valid Weaviate class name
Expand All @@ -528,7 +528,7 @@ private static string ToWeaviateFriendlyClassName(string collectionName)
var apiVersion = !string.IsNullOrWhiteSpace(this._apiVersion) ? this._apiVersion : DefaultApiVersion;
var baseAddress = this._endpoint ?? this._httpClient.BaseAddress;

request.RequestUri = new Uri(baseAddress, $"{apiVersion}/{request.RequestUri}");
request.RequestUri = new Uri(baseAddress!, $"{apiVersion}/{request.RequestUri}");

if (!string.IsNullOrEmpty(this._apiKey))
{
Expand Down Expand Up @@ -564,10 +564,10 @@ private static MemoryRecordMetadata ToMetadata(WeaviateObject weaviateObject)
return new(
false,
string.Empty,
weaviateObject.Properties["sk_id"].ToString(),
weaviateObject.Properties["sk_description"].ToString(),
weaviateObject.Properties["sk_text"].ToString(),
weaviateObject.Properties["sk_additional_metadata"].ToString()
weaviateObject.Properties["sk_id"].ToString() ?? string.Empty,
weaviateObject.Properties["sk_description"].ToString() ?? string.Empty,
weaviateObject.Properties["sk_text"].ToString() ?? string.Empty,
weaviateObject.Properties["sk_additional_metadata"].ToString() ?? string.Empty
);
}
}
4 changes: 2 additions & 2 deletions dotnet/src/Functions/Functions.Grpc/GrpcOperationRunner.cs
Expand Up @@ -219,12 +219,12 @@ private static TypeInfo BuildGrpcOperationDataContractType(GrpcOperationDataCont
propertyBuilder.SetSetMethod(setterBuilder);

//Add ProtoMember attribute to the data contract with tag/number
var dataMemberAttributeBuilder = new CustomAttributeBuilder(typeof(ProtoMemberAttribute).GetConstructor(new[] { typeof(int) }), new object[] { field.Number });
var dataMemberAttributeBuilder = new CustomAttributeBuilder(typeof(ProtoMemberAttribute).GetConstructor(new[] { typeof(int) })!, new object[] { field.Number });
propertyBuilder.SetCustomAttribute(dataMemberAttributeBuilder);
}

//Add ProtoContract attribute to the data contract
var dataContractAttributeBuilder = new CustomAttributeBuilder(typeof(ProtoContractAttribute).GetConstructor(Type.EmptyTypes), Array.Empty<object>());
var dataContractAttributeBuilder = new CustomAttributeBuilder(typeof(ProtoContractAttribute).GetConstructor(Type.EmptyTypes)!, Array.Empty<object>());
typeBuilder.SetCustomAttribute(dataContractAttributeBuilder);

var type = typeBuilder.CreateTypeInfo();
Expand Down
Expand Up @@ -126,15 +126,16 @@ private List<GrpcOperationDataContractTypeFiled> GetDataContractFields(List<Fiel
private static string GetProtobufDataTypeName(FieldDescriptorProto.Type type)
{
var fieldInfo = typeof(FieldDescriptorProto.Type).GetField(type.ToString());

//Get protobuf type name from enum attribute - [global::ProtoBuf.ProtoEnum(Name = @"TYPE_DOUBLE")]
var attribute = (ProtoEnumAttribute)Attribute.GetCustomAttribute(fieldInfo, typeof(ProtoEnumAttribute));

if (attribute == null)
if (fieldInfo != null)
{
throw new SKException($"Impossible to find protobuf type name corresponding to '{type}' type.");
//Get protobuf type name from enum attribute - [global::ProtoBuf.ProtoEnum(Name = @"TYPE_DOUBLE")]
var attribute = (ProtoEnumAttribute?)Attribute.GetCustomAttribute(fieldInfo, typeof(ProtoEnumAttribute));
if (attribute != null)
{
return attribute.Name;
}
}

return attribute.Name;
throw new SKException($"Impossible to find protobuf type name corresponding to '{type}' type.");
}
}
Expand Up @@ -38,7 +38,7 @@ public static string SerializeArrayAsSeparateParameters(string name, JsonArray a
/// <returns>A string containing the serialized parameter.</returns>
public static string SerializeArrayAsDelimitedValues(JsonArray array, string delimiter)
{
var values = new List<string>();
var values = new List<string?>();

foreach (var item in array)
{
Expand Down
Expand Up @@ -12,13 +12,13 @@ namespace Microsoft.SemanticKernel.Functions.OpenAPI.Model;
public class RestApiOperationResponseConverter : TypeConverter
{
/// <inheritdoc/>
public override bool CanConvertTo(ITypeDescriptorContext context, Type destinationType)
public override bool CanConvertTo(ITypeDescriptorContext? context, Type? destinationType)
{
return destinationType == typeof(string) || base.CanConvertTo(context, destinationType);
}

/// <inheritdoc/>
public override object ConvertTo(ITypeDescriptorContext context, CultureInfo culture, object value, Type destinationType)
public override object? ConvertTo(ITypeDescriptorContext? context, CultureInfo? culture, object? value, Type destinationType)
{
// Convert object content to a string based on the type of the `Content` property.
// More granular conversion logic can be built based on the value of the `ContentType` property, if needed.
Expand Down
Expand Up @@ -231,8 +231,8 @@ private static List<RestApiOperationParameter> CreateRestApiOperationParameters(
parameter.Schema.Type,
parameter.Required,
parameter.Explode,
(RestApiOperationParameterLocation)Enum.Parse(typeof(RestApiOperationParameterLocation), parameter.In.ToString()),
(RestApiOperationParameterStyle)Enum.Parse(typeof(RestApiOperationParameterStyle), parameter.Style.ToString()),
(RestApiOperationParameterLocation)Enum.Parse(typeof(RestApiOperationParameterLocation), parameter.In.ToString()!),
(RestApiOperationParameterStyle)Enum.Parse(typeof(RestApiOperationParameterStyle), parameter.Style.ToString()!),
parameter.Schema.Items?.Type,
GetParameterValue(parameter.Name, parameter.Schema.Default),
parameter.Description,
Expand Down
Expand Up @@ -183,7 +183,11 @@ private static async Task<RestApiOperationResponse> SerializeResponseContentAsyn
{
var contentType = content.Headers.ContentType;

var mediaType = contentType.MediaType;
var mediaType = contentType?.MediaType;
if (mediaType is null)
{
throw new SKException("No media type available.");
}

// Obtain the content serializer by media type (e.g., text/plain, application/json, image/jpg)
if (!s_serializerByContentType.TryGetValue(mediaType, out var serializer))
Expand All @@ -207,7 +211,7 @@ private static async Task<RestApiOperationResponse> SerializeResponseContentAsyn
// Serialize response content and return it
var serializedContent = await serializer.Invoke(content).ConfigureAwait(false);

return new RestApiOperationResponse(serializedContent, contentType.ToString());
return new RestApiOperationResponse(serializedContent, contentType!.ToString());
}

/// <summary>
Expand Down
33 changes: 17 additions & 16 deletions dotnet/src/Planners/Planners.Core/Action/ActionPlanner.cs
Expand Up @@ -134,9 +134,9 @@ public async Task<Plan> CreatePlanAsync(string goal, CancellationToken cancellat
{
foreach (KeyValuePair<string, object> p in planData.Plan.Parameters)
{
if (p.Value != null)
if (p.Value?.ToString() is string value)
{
plan.Steps[0].Parameters[p.Key] = p.Value.ToString();
plan.Steps[0].Parameters[p.Key] = value;
}
}
}
Expand Down Expand Up @@ -263,24 +263,25 @@ No parameters.
/// <returns>Instance of <see cref="ActionPlanResponse"/> object deserialized from extracted JSON.</returns>
private ActionPlanResponse? ParsePlannerResult(FunctionResult plannerResult)
{
Match match = s_planRegex.Match(plannerResult.GetValue<string>());

if (match.Success && match.Groups["Close"].Length > 0)
if (plannerResult.GetValue<string>() is string result)
{
string planJson = $"{{{match.Groups["Close"]}}}";
try
{
return JsonSerializer.Deserialize<ActionPlanResponse?>(planJson, s_actionPlayResponseOptions);
}
catch (Exception e)
Match match = s_planRegex.Match(result);

if (match.Success && match.Groups["Close"] is { Length: > 0 } close)
{
throw new SKException("Plan parsing error, invalid JSON", e);
string planJson = $"{{{close}}}";
try
{
return JsonSerializer.Deserialize<ActionPlanResponse?>(planJson, s_actionPlayResponseOptions);
}
catch (Exception e)
{
throw new SKException("Plan parsing error, invalid JSON", e);
}
}
}
else
{
throw new SKException($"Failed to extract valid json string from planner result: '{plannerResult}'");
}

throw new SKException($"Failed to extract valid json string from planner result: '{plannerResult}'");
}

private void PopulateList(StringBuilder list, IEnumerable<FunctionView> functions)
Expand Down
Expand Up @@ -87,12 +87,12 @@ public async Task<Plan> CreatePlanAsync(string goal, CancellationToken cancellat
/// <summary>
/// Instance of <see cref="ActivitySource"/> for planner-related activities.
/// </summary>
private static readonly ActivitySource s_activitySource = new(typeof(InstrumentedActionPlanner).FullName);
private static readonly ActivitySource s_activitySource = new(typeof(InstrumentedActionPlanner).FullName!);

/// <summary>
/// Instance of <see cref="Meter"/> for planner-related metrics.
/// </summary>
private static readonly Meter s_meter = new(typeof(InstrumentedActionPlanner).FullName);
private static readonly Meter s_meter = new(typeof(InstrumentedActionPlanner).FullName!);

/// <summary>
/// Instance of <see cref="Histogram{T}"/> to record plan creation execution time.
Expand Down
Expand Up @@ -87,12 +87,12 @@ public async Task<Plan> CreatePlanAsync(string goal, CancellationToken cancellat
/// <summary>
/// Instance of <see cref="ActivitySource"/> for planner-related activities.
/// </summary>
private static readonly ActivitySource s_activitySource = new(typeof(InstrumentedSequentialPlanner).FullName);
private static readonly ActivitySource s_activitySource = new(typeof(InstrumentedSequentialPlanner).FullName!);

/// <summary>
/// Instance of <see cref="Meter"/> for planner-related metrics.
/// </summary>
private static readonly Meter s_meter = new(typeof(InstrumentedSequentialPlanner).FullName);
private static readonly Meter s_meter = new(typeof(InstrumentedSequentialPlanner).FullName!);

/// <summary>
/// Instance of <see cref="Histogram{T}"/> to record plan creation execution time.
Expand Down
Expand Up @@ -80,12 +80,12 @@ public Plan CreatePlan(string goal)
/// <summary>
/// Instance of <see cref="ActivitySource"/> for planner-related activities.
/// </summary>
private static readonly ActivitySource s_activitySource = new(typeof(InstrumentedStepwisePlanner).FullName);
private static readonly ActivitySource s_activitySource = new(typeof(InstrumentedStepwisePlanner).FullName!);

/// <summary>
/// Instance of <see cref="Meter"/> for planner-related metrics.
/// </summary>
private static readonly Meter s_meter = new(typeof(InstrumentedStepwisePlanner).FullName);
private static readonly Meter s_meter = new(typeof(InstrumentedStepwisePlanner).FullName!);

/// <summary>
/// Instance of <see cref="Histogram{T}"/> to record plan creation execution time.
Expand Down

0 comments on commit 904f592

Please sign in to comment.