Incorrect translation of float data type

[softworkz]

This is in the OpenApi v3 doc:

And this is what is getting generated:

And that's what the OpenApi spec says:

[baywet]

Hi @softworkz
Thanks for your interest in Kiota and for reaching out.

The mapping of primitive types in Kiota is here

kiota/src/Kiota.Builder/KiotaBuilder.cs

Line 594 in b12abea

("number", "double" or "float" or "decimal") => format.ToLowerInvariant(),

And the unit tests for it there

kiota/tests/Kiota.Builder.Tests/KiotaBuilderTests.cs

Line 1618 in b12abea

public void MapsPrimitiveFormats(string type, string format, string expected){

I suspect what's throwing kiota off balance is the fact that the number is also marked as nullable. Kiota always defaults scalar properties to their nullable equivalent (as you can see with max height in your screenshot), this is a design decision that was made because properties can be selected or not (get, with $select in odata for example) or rely on service side defined defaults (post). In effect having a nullable scalar in the description doesn't "add" any value for kiota.

Just for the purpose of the investigation, can you edit your description to remove nullable, and see what results you get please?

[softworkz]

Kiota always defaults scalar properties to their nullable equivalent (as you can see with max height in your screenshot), this is a design decision that was made because properties can be selected or not

For a similar reason, we have most types defined as nullable.

Kiota always defaults scalar properties to their nullable equivalent (as you can see with max height in your screenshot)

MaxHeight is nullable already in the definition:

so it probably can't be an issue with nullable.

[baywet]

Ah I missed the fact we were talking about query parameters.
It seems for whatever historic reason the query parameters type mapping is not using the type mapping method.

kiota/src/Kiota.Builder/KiotaBuilder.cs

Line 1184 in b12abea

Type = new CodeType

This should be an easy fix:

1. replace that section to call the GetPrimitiveType method instead
2. below, handle the collection/array aspect (move the collection king assignment
3. below, if the type name is null/empty, default it to string

Is this something you'd be willing to submit a pull request for?

[softworkz]

below, handle the collection/array aspect (move the collection king assignment

Why isn't there a single function that does this?
It seems bug-prone to have code for this at multiple places..

[baywet]

below, handle the collection/array aspect (move the collection king assignment

Why isn't there a single function that does this? It seems bug-prone to have code for this at multiple places..

1. because of organic growth in the code and no refactoring on that specific aspect.
2. because depending on the call we might, want an array, a complex collection (List in dotnet), or no collection at all (composed types)

I'd be open to introducing the following method

private static void SetCollectionInformation(CodeTypeBase target, OpenApiSchema schema, CodeTypeBase.CodeTypeCollectionKind collectionKind = CodeTypeBase.CodeTypeCollectionKind.Array) {
    target.CollectionKind = schema.IsArray() ? collectionKind : default;
}

I didn't suggest that originally because I didn't want the refactoring burden to fall on you.

[softworkz]

Is this something you'd be willing to submit a pull request for?

I'm afraid, but I can't do that because I figured that there are too many things that I don't understand or don't make sense to me.
Looking at GetPrimitiveType() alone, I see multiple things that seem to be wrong and/or questionable.

        var typeNames = new List<string>{typeSchema?.Items?.Type, childType, typeSchema?.Type};
        if(typeSchema?.AnyOf?.Any() ?? false)
            typeNames.AddRange(typeSchema.AnyOf.Select(x => x.Type)); // double is sometimes an anyof string, number and enum
        // first value that's not null, and not "object" for primitive collections, the items type matters
        var typeName = typeNames.FirstOrDefault(x => !string.IsNullOrEmpty(x) && !typeNamesToSkip.Contains(x));

This looks like we just want to get "something", but how can it be so totally irrelevant from where the typeName is coming actually? Isn't that important?

        if (typeSchema?.Items?.Enum?.Any() ?? false)
            typeName = childType;
        else {

What is the above meaning to do? When a schema for Items exists and that schema has one or more Enum entries, then use that value as type name which is supplied as function parameter? What when it's empty? How can the calling function know that it must specify that child-type parameter?

            var format = typeSchema?.Format ?? typeSchema?.Items?.Format;

Above, we're already taking the type value from "somewhere'". And now, we're taking the format value from "somewhere" again, And then, we're checking that combination of values in the switch, even though they might not even belong together.
This can't be right.

Also, the format value can have a meaning when it's empty (not specified). We just can't take it from elsewhere because it's empty.

[baywet]

no worries, I'll take it on when I have some time.

[softworkz]

Sure. Thank you very much!

[baywet]

We're going to keep this one open since it's an actual issue with the generator that eventually needs to be addressed.

[softworkz]

Yea, sorry, closing was a bit egoistic.

[baywet]

Submitted #1723 , which will close this issue when merged