Skip to content
New issue

Have a question about this project? Sign up for a free GitHub account to open an issue and contact its maintainers and the community.

By clicking “Sign up for GitHub”, you agree to our terms of service and privacy statement. We’ll occasionally send you account related emails.

Already on GitHub? Sign in to your account

Add code generation for Dart #5966

Open
wants to merge 216 commits into
base: main
Choose a base branch
from

Conversation

ricardoboss
Copy link
Contributor

@ricardoboss ricardoboss commented Jan 7, 2025

Adds code generation for the Dart programming language.

This PR was a joint effort and contains lots of good work by @joanne-ter-maat and @Kees-Schotanus and is the result of lots of iterations and review over at kiota-community#1 by @baywet and @andreaTP.

Fixes #3745.

ricardoboss and others added 30 commits January 27, 2024 23:09
This code is not nearly finished,
but was pushed to work simultaneously on this code base.
Using a DartRefiner that is created from scratch.
In the end this will be our actual refiner,
and we will update ILanguage to reflect this.
Also added imprts for properties and methods.
Tjose imports are not correct yet.
Imports in other files are still incorrect.
No idea if it actually works, but at least it compiles.
Added more Dart syntax to creating methods.
createFromDiscriminatorValue also looks correct.
@ricardoboss ricardoboss marked this pull request as ready for review January 7, 2025 13:26
@ricardoboss ricardoboss requested a review from a team as a code owner January 7, 2025 13:26
baywet
baywet previously approved these changes Jan 7, 2025
Copy link
Member

@baywet baywet left a comment

Choose a reason for hiding this comment

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

Thank you all for the great work! And special thanks to @ricardoboss for ensuring we cross the finish line on this project!
@andrueastman @Ndiritu I'm going to ask that both of you review this PR. I've already reviewed it on the fork, and I was coaching the team as well. So getting additional pair of eyes will be paramount here.
The idea is to try and make it for the minor release on the 9th, no rush though.

Copy link
Member

@andrueastman andrueastman left a comment

Choose a reason for hiding this comment

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

Are the current failures in the integration tests expected for now and to be resolved in a separate PR?

writer.StartBlock($"class {codeElement.Name}{derivation}{implements} {{");
}

private String getAlias(string alias)
Copy link
Member

Choose a reason for hiding this comment

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

Suggested change
private String getAlias(string alias)
private static string getAlias(string alias)

{
if (parentClass.IsOfKind(CodeClassKind.Model) && codeElement.IsOfKind(CodeMethodKind.Constructor) && !parentClass.IsErrorDefinition)
{
return !parentClass.Properties.Where(prop => !string.IsNullOrEmpty(prop.DefaultValue)).Any();
Copy link
Member

Choose a reason for hiding this comment

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

Suggested change
return !parentClass.Properties.Where(prop => !string.IsNullOrEmpty(prop.DefaultValue)).Any();
return parentClass.Properties.All(prop => string.IsNullOrEmpty(prop.DefaultValue));

{
if (property.Type is CodeType propertyType)
{
var typeName = conventions.GetTypeString(propertyType, codeElement, true, false);
Copy link
Member

Choose a reason for hiding this comment

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

unused var

Suggested change
var typeName = conventions.GetTypeString(propertyType, codeElement, true, false);

}
}

private string DefaultDeserializerReturnType => $"Map<String, void Function({conventions.ParseNodeInterfaceName})>";
Copy link
Member

Choose a reason for hiding this comment

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

unused property

Suggested change
private string DefaultDeserializerReturnType => $"Map<String, void Function({conventions.ParseNodeInterfaceName})>";

}
}

protected virtual void WriteCommandBuilderBody(CodeMethod codeElement, CodeClass parentClass, RequestParams requestParams, bool isVoid, string returnType, LanguageWriter writer)
Copy link
Member

Choose a reason for hiding this comment

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

We should probably remove this function. Then throw exception in the caller directly in the switch case on line 123(and remove the irrelevant code there.)

{
writer.WriteLine("@override");
}
var genericTypePrefix = isVoid ? string.Empty : "<";
Copy link
Member

Choose a reason for hiding this comment

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

unused var

Suggested change
var genericTypePrefix = isVoid ? string.Empty : "<";

Comment on lines 170 to 178
private static IEnumerable<CodeNamespace> GetAllNamespaces(CodeNamespace ns)
{
foreach (var childNs in ns.Namespaces)
{
yield return childNs;
foreach (var childNsSegment in GetAllNamespaces(childNs))
yield return childNsSegment;
}
}
Copy link
Member

Choose a reason for hiding this comment

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

unused function

Suggested change
private static IEnumerable<CodeNamespace> GetAllNamespaces(CodeNamespace ns)
{
foreach (var childNs in ns.Namespaces)
{
yield return childNs;
foreach (var childNsSegment in GetAllNamespaces(childNs))
yield return childNsSegment;
}
}

Comment on lines 230 to 236
var parentElementsHash = targetElement.Parent is CodeClass parentClass ?
parentClass.Methods.Select(static x => x.Name)
.Union(parentClass.Properties.Select(static x => x.Name))
.Distinct(StringComparer.OrdinalIgnoreCase)
.ToHashSet(StringComparer.OrdinalIgnoreCase) :
new HashSet<string>(0, StringComparer.OrdinalIgnoreCase);

Copy link
Member

Choose a reason for hiding this comment

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

Unused function. (We can then also remove the second parameter of the function)

Suggested change
var parentElementsHash = targetElement.Parent is CodeClass parentClass ?
parentClass.Methods.Select(static x => x.Name)
.Union(parentClass.Properties.Select(static x => x.Name))
.Distinct(StringComparer.OrdinalIgnoreCase)
.ToHashSet(StringComparer.OrdinalIgnoreCase) :
new HashSet<string>(0, StringComparer.OrdinalIgnoreCase);

@ricardoboss
Copy link
Contributor Author

ricardoboss commented Jan 8, 2025

Are the current failures in the integration tests expected for now and to be resolved in a separate PR?

Not really, they are a side-effect of recent changes in the Dart packages. I've created a separate PR to fix this: kiota-community#4

Additionally, I discovered a bug that also has a separate PR to fix it: kiota-community#3

The name used for the URL template needs to match the serialized name of the custom parameter.
baywet
baywet previously approved these changes Jan 8, 2025
baywet
baywet previously approved these changes Jan 8, 2025
@ricardoboss
Copy link
Contributor Author

@andrueastman I applied all your suggestions. Thanks for reviewing!
@baywet There are lots of diagnostics I want to tackle in the future, but the current state should be good for a preview release :)

Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
None yet
Projects
None yet
Development

Successfully merging this pull request may close these issues.

Dart support
5 participants