Skip to content

Commit

Permalink
Merge pull request #3568 from microsoft/fix/update-php-promise-doc-in…
Browse files Browse the repository at this point in the history
…clude-generic-type

[FIX] Include the type which a promise resolves to.
  • Loading branch information
SilasKenneth authored Oct 31, 2023
2 parents 146453f + eda6bcb commit 462a1b5
Show file tree
Hide file tree
Showing 5 changed files with 26 additions and 23 deletions.
3 changes: 2 additions & 1 deletion CHANGELOG.md
Original file line number Diff line number Diff line change
Expand Up @@ -19,9 +19,10 @@ and this project adheres to [Semantic Versioning](https://semver.org/spec/v2.0.0

### Changed

- Include the type which a promise resolves to in PHPDocs for PHP. [#3542](https://github.com/microsoft/kiota/issues/3542)
- Added null check for null content type instances when getting deprecation information [#3588](https://github.com/microsoft/kiota/issues/3588)
- Aligns header management in Python with other languages. [#3430](https://github.com/microsoft/kiota/issues/3430)
- Fixed parameters that are in camelcase to snakecase in Python. [#3525](https://github.com/microsoft/kiota/issues/3525
- Fixed parameters that are in camelcase to snakecase in Python. [#3525](https://github.com/microsoft/kiota/issues/3525)
- Fixed missing imports for method parameters that are query parameters.
- Replaces the use of "new" by "override" and "virtual" in CSharp models.
- Fixed a bug in validation rules where form data would wrongfully warn for arrays. [#3569](https://github.com/microsoft/kiota/issues/3569)
Expand Down
2 changes: 1 addition & 1 deletion src/Kiota.Builder/Refiners/PhpRefiner.cs
Original file line number Diff line number Diff line change
Expand Up @@ -192,7 +192,7 @@ public override Task Refine(CodeNamespace generatedCode, CancellationToken cance
"Microsoft\\Kiota\\Abstractions\\Store", "BackingStoreFactory", "BackingStoreFactorySingleton"),
new (x => x is CodeProperty prop && prop.IsOfKind(CodePropertyKind.BackingStore),
"Microsoft\\Kiota\\Abstractions\\Store", "BackingStore", "BackedModel", "BackingStoreFactorySingleton" ),
new (x => x is CodeMethod method && method.IsOfKind(CodeMethodKind.RequestExecutor), "Http\\Promise", "Promise", "RejectedPromise"),
new (x => x is CodeMethod method && method.IsOfKind(CodeMethodKind.RequestExecutor), "Http\\Promise", "Promise"),
new (x => x is CodeMethod method && method.IsOfKind(CodeMethodKind.RequestExecutor), "", "Exception"),
new (x => x is CodeEnum, "Microsoft\\Kiota\\Abstractions\\", "Enum"),
new(static x => x is CodeProperty {Type.Name: {}} property && property.Type.Name.Equals("DateTime", StringComparison.OrdinalIgnoreCase), "", "\\DateTime"),
Expand Down
29 changes: 16 additions & 13 deletions src/Kiota.Builder/Writers/Php/CodeMethodWriter.cs
Original file line number Diff line number Diff line change
Expand Up @@ -259,7 +259,7 @@ private static void AssignPropertyFromParameter(CodeClass parentClass, CodeMetho
private void WriteMethodPhpDocs(CodeMethod codeMethod, LanguageWriter writer)
{
var methodDescription = codeMethod.Documentation.Description;

var methodThrows = codeMethod.IsOfKind(CodeMethodKind.RequestExecutor);
var hasMethodDescription = !string.IsNullOrEmpty(methodDescription.Trim());
if (!hasMethodDescription && !codeMethod.Parameters.Any())
{
Expand All @@ -276,13 +276,15 @@ private void WriteMethodPhpDocs(CodeMethod codeMethod, LanguageWriter writer)
{
var nullableSuffix = (codeMethod.ReturnType.IsNullable ? "|null" : "");
returnDocString = (codeMethod.Kind == CodeMethodKind.RequestExecutor)
? "@return Promise"
? $"@return Promise<{returnDocString}|null>"
: $"@return {returnDocString}{nullableSuffix}";
}
else returnDocString = String.Empty;

var throwsArray = methodThrows ? new[] { "@throws Exception" } : Array.Empty<string>();
conventions.WriteLongDescription(codeMethod.Documentation,
writer,
parametersWithOrWithoutDescription.Union(new[] { returnDocString })
parametersWithOrWithoutDescription.Union(new[] { returnDocString }).Union(throwsArray)
);

}
Expand All @@ -294,6 +296,7 @@ private string GetDocCommentReturnType(CodeMethod codeMethod)
CodeMethodKind.Deserializer => "array<string, callable(ParseNode): void>",
CodeMethodKind.Getter when codeMethod.AccessedProperty?.IsOfKind(CodePropertyKind.AdditionalData) ?? false => "array<string, mixed>",
CodeMethodKind.Getter when codeMethod.AccessedProperty?.Type.IsCollection ?? false => $"array<{conventions.TranslateType(codeMethod.AccessedProperty.Type)}>",
CodeMethodKind.RequestExecutor when codeMethod.ReturnType.IsCollection => $"array<{conventions.TranslateType(codeMethod.ReturnType)}>",
_ => conventions.GetTypeString(codeMethod.ReturnType, codeMethod)
};
}
Expand Down Expand Up @@ -735,8 +738,6 @@ private void WriteRequestExecutorBody(CodeMethod codeElement, CodeClass parentCl

var returnTypeName = conventions.GetTypeString(codeElement.ReturnType, codeElement, false);
writer.WriteLine($"$requestInfo = $this->{generatorMethodName}({joinedParams});");
writer.WriteLine("try {");
writer.IncreaseIndent();
var errorMappings = codeElement.ErrorMappings;
var hasErrorMappings = false;
var errorMappingsVarName = "$errorMappings";
Expand Down Expand Up @@ -772,15 +773,17 @@ private void WriteRequestExecutorBody(CodeMethod codeElement, CodeClass parentCl
? $", '{returnTypeName}'"
: returnEnumType;
var requestAdapterProperty = parentClass.GetPropertyOfKind(CodePropertyKind.RequestAdapter) ?? throw new InvalidOperationException("Request adapter property not found");
writer.WriteLine(
$"return {GetPropertyCall(requestAdapterProperty, string.Empty)}->{methodName}({RequestInfoVarName}{finalReturn}, {(hasErrorMappings ? $"{errorMappingsVarName}" : "null")});");

writer.DecreaseIndent();
writer.WriteLine("} catch(Exception $ex) {");
writer.IncreaseIndent();
writer.WriteLine("return new RejectedPromise($ex);");
writer.DecreaseIndent();
writer.WriteLine("}");
var methodCall = $"{GetPropertyCall(requestAdapterProperty, string.Empty)}->{methodName}({RequestInfoVarName}{finalReturn}, {(hasErrorMappings ? $"{errorMappingsVarName}" : "null")});";
if (methodName.Contains("sendPrimitive", StringComparison.OrdinalIgnoreCase))
{
writer.WriteLines($"/** @var Promise<{GetDocCommentReturnType(codeElement)}|null> $result */",
$"$result = {methodCall}",
"return $result;"
);
}
else writer.WriteLines(
$"return {methodCall}");
}

private static void WriteApiConstructorBody(CodeClass parentClass, CodeMethod codeMethod, LanguageWriter writer)
Expand Down
Original file line number Diff line number Diff line change
Expand Up @@ -134,8 +134,7 @@ public async Task ImportRequiredClassesWhenContainsRequestExecutor()
codeElementWriter.WriteCodeElement(dec, writer);
var result = tw.ToString();

Assert.Contains("use Http\\Promise\\Promise;", result);
Assert.Contains("use Http\\Promise\\RejectedPromise;", result);
Assert.Contains(@"use Http\Promise\Promise;", result);
Assert.Contains("use Exception;", result);
}

Expand Down
12 changes: 6 additions & 6 deletions tests/Kiota.Builder.Tests/Writers/Php/CodeMethodWriterTests.cs
Original file line number Diff line number Diff line change
Expand Up @@ -287,11 +287,10 @@ public async void WriteRequestExecutor()

Assert.Contains("public function post(): Promise", result);
Assert.Contains("$requestInfo = $this->createPostRequestInformation();", result);
Assert.Contains("RejectedPromise", result);
Assert.Contains("@link https://learn.microsoft.com/ Learning", result);
Assert.Contains("catch(Exception $ex)", result);
Assert.Contains("'401' => [Error401::class, 'createFromDiscriminatorValue']", result);
Assert.Contains("return $this->requestAdapter->sendPrimitiveAsync($requestInfo, StreamInterface::class, $errorMappings);", result);
Assert.Contains("$result = $this->requestAdapter->sendPrimitiveAsync($requestInfo, StreamInterface::class, $errorMappings);", result);
Assert.Contains("return $result;", result);
}

[Fact]
Expand Down Expand Up @@ -380,13 +379,14 @@ public async void WritesRequestExecutorForEnumTypes()
_codeMethodWriter.WriteCodeElement(codeMethod, languageWriter);
var result = stringWriter.ToString();

Assert.Contains("@throws Exception", result);
Assert.Contains("public function post(): Promise", result);
Assert.Contains("$requestInfo = $this->createPostRequestInformation();", result);
Assert.Contains("RejectedPromise", result);
Assert.Contains("@link https://learn.microsoft.com/ Learning", result);
Assert.Contains("catch(Exception $ex)", result);
Assert.Contains("'401' => [Error401::class, 'createFromDiscriminatorValue']", result);
Assert.Contains("return $this->requestAdapter->sendPrimitiveAsync($requestInfo, PhoneNumberPrefix::class, $errorMappings);", result);
Assert.Contains("/** @var Promise<PhoneNumberPrefix|null> $result", result);
Assert.Contains("$result = $this->requestAdapter->sendPrimitiveAsync($requestInfo, PhoneNumberPrefix::class, $errorMappings);", result);
Assert.Contains("return $result;", result);
}

public static IEnumerable<object[]> SerializerProperties => new List<object[]>
Expand Down

0 comments on commit 462a1b5

Please sign in to comment.