Skip to content

Commit f448f8a

Browse files
stephentoubstephentoub
committed
Remove HasXx methods and add TryGetXx methods
1 parent 7785207 commit f448f8a

15 files changed

Lines changed: 208 additions & 209 deletions

File tree

‎dotnet/src/SemanticKernel.Abstractions/Orchestration/SKContext.cs‎

Lines changed: 1 addition & 6 deletions
Original file line numberDiff line numberDiff line change
@@ -96,12 +96,7 @@ public ISKFunction Func(string skillName, string functionName)
9696
{
9797
Verify.NotNull(this.Skills, "The skill collection hasn't been set");
9898

99-
if (this.Skills.HasNativeFunction(skillName, functionName))
100-
{
101-
return this.Skills.GetNativeFunction(skillName, functionName);
102-
}
103-
104-
return this.Skills.GetSemanticFunction(skillName, functionName);
99+
return this.Skills.GetFunction(skillName, functionName);
105100
}
106101

107102
/// <summary>

‎dotnet/src/SemanticKernel.Abstractions/SkillDefinition/IReadOnlySkillCollection.cs‎

Lines changed: 69 additions & 50 deletions
Original file line numberDiff line numberDiff line change
@@ -1,98 +1,117 @@
11
// Copyright (c) Microsoft. All rights reserved.
22

3+
using System.Diagnostics.CodeAnalysis;
34
using Microsoft.SemanticKernel.Orchestration;
45

56
namespace Microsoft.SemanticKernel.SkillDefinition;
67

78
/// <summary>
89
/// Read-only skill collection interface.
910
/// </summary>
10-
[System.Diagnostics.CodeAnalysis.SuppressMessage("Naming", "CA1710:Identifiers should have correct suffix", Justification = "It is a collection")]
1111
[System.Diagnostics.CodeAnalysis.SuppressMessage("Naming", "CA1711:Identifiers should not have incorrect suffix", Justification = "It is a collection")]
1212
public interface IReadOnlySkillCollection
1313
{
1414
/// <summary>
15-
/// Check if the collection contains the specified function in the global skill, regardless of the function type
15+
/// Gets the function stored in the collection.
1616
/// </summary>
17-
/// <param name="skillName">Skill name</param>
18-
/// <param name="functionName">Function name</param>
19-
/// <returns>True if the function exists, false otherwise</returns>
20-
bool HasFunction(string skillName, string functionName);
17+
/// <param name="functionName">The name of the function to retrieve.</param>
18+
/// <returns>The function retrieved from the collection.</returns>
19+
/// <exception cref="KernelException">The specified function could not be found in the collection.</exception>
20+
ISKFunction GetFunction(string functionName);
2121

2222
/// <summary>
23-
/// Check if the collection contains the specified function, regardless of the function type
23+
/// Gets the function stored in the collection.
2424
/// </summary>
25-
/// <param name="functionName">Function name</param>
26-
/// <returns>True if the function exists, false otherwise</returns>
27-
bool HasFunction(string functionName);
25+
/// <param name="skillName">The name of the skill with which the function is associated.</param>
26+
/// <param name="functionName">The name of the function to retrieve.</param>
27+
/// <returns>The function retrieved from the collection.</returns>
28+
/// <exception cref="KernelException">The specified function could not be found in the collection.</exception>
29+
ISKFunction GetFunction(string skillName, string functionName);
2830

2931
/// <summary>
30-
/// Check if a semantic function is registered
32+
/// Gets the function stored in the collection.
3133
/// </summary>
32-
/// <param name="functionName">Function name</param>
33-
/// <param name="skillName">Skill name</param>
34-
/// <returns>True if the function exists</returns>
35-
bool HasSemanticFunction(string skillName, string functionName);
34+
/// <param name="functionName">The name of the function to retrieve.</param>
35+
/// <param name="functionInstance">When this method returns, the function that was retrieved if one with the specified name was found; otherwise, <see langword="null"/>.</param>
36+
/// <returns><see langword="true"/> if the function was found; otherwise, <see langword="false"/>.</returns>
37+
bool TryGetFunction(string functionName, [NotNullWhen(true)] out ISKFunction? functionInstance);
3638

3739
/// <summary>
38-
/// Check if a native function is registered
40+
/// Gets the function stored in the collection.
3941
/// </summary>
40-
/// <param name="functionName">Function name</param>
41-
/// <param name="skillName">Skill name</param>
42-
/// <returns>True if the function exists</returns>
43-
bool HasNativeFunction(string skillName, string functionName);
42+
/// <param name="skillName">The name of the skill with which the function is associated.</param>
43+
/// <param name="functionName">The name of the function to retrieve.</param>
44+
/// <param name="functionInstance">When this method returns, the function that was retrieved if one with the specified name was found; otherwise, <see langword="null"/>.</param>
45+
/// <returns><see langword="true"/> if the function was found; otherwise, <see langword="false"/>.</returns>
46+
bool TryGetFunction(string skillName, string functionName, [NotNullWhen(true)] out ISKFunction? functionInstance);
4447

4548
/// <summary>
46-
/// Check if a native function is registered in the global skill
49+
/// Gets the semantic function stored in the collection.
4750
/// </summary>
48-
/// <param name="functionName">Function name</param>
49-
/// <returns>True if the function exists</returns>
50-
bool HasNativeFunction(string functionName);
51+
/// <param name="functionName">The name of the function to retrieve.</param>
52+
/// <returns>The function retrieved from the collection.</returns>
53+
/// <exception cref="KernelException">The specified function could not be found in the collection.</exception>
54+
ISKFunction GetSemanticFunction(string functionName);
5155

5256
/// <summary>
53-
/// Return the function delegate stored in the collection, regardless of the function type
57+
/// Gets the semantic function stored in the collection.
5458
/// </summary>
55-
/// <param name="functionName">Function name</param>
56-
/// <returns>Function delegate</returns>
57-
ISKFunction GetFunction(string functionName);
59+
/// <param name="skillName">The name of the skill with which the function is associated.</param>
60+
/// <param name="functionName">The name of the function to retrieve.</param>
61+
/// <returns>The function retrieved from the collection.</returns>
62+
/// <exception cref="KernelException">The specified function could not be found in the collection.</exception>
63+
ISKFunction GetSemanticFunction(string skillName, string functionName);
5864

5965
/// <summary>
60-
/// Return the function delegate stored in the collection, regardless of the function type
66+
/// Gets the semantic function stored in the collection.
6167
/// </summary>
62-
/// <param name="functionName">Function name</param>
63-
/// <param name="skillName">Skill name</param>
64-
/// <returns>Function delegate</returns>
65-
ISKFunction GetFunction(string skillName, string functionName);
68+
/// <param name="functionName">The name of the function to retrieve.</param>
69+
/// <param name="functionInstance">When this method returns, the function that was retrieved if one with the specified name was found; otherwise, <see langword="null"/>.</param>
70+
/// <returns><see langword="true"/> if the function was found; otherwise, <see langword="false"/>.</returns>
71+
bool TryGetSemanticFunction(string functionName, [NotNullWhen(true)] out ISKFunction? functionInstance);
6672

6773
/// <summary>
68-
/// Return the semantic function delegate stored in the collection
74+
/// Gets the semantic function stored in the collection.
6975
/// </summary>
70-
/// <param name="functionName">Function name</param>
71-
/// <returns>Semantic function delegate</returns>
72-
ISKFunction GetSemanticFunction(string functionName);
76+
/// <param name="skillName">The name of the skill with which the function is associated.</param>
77+
/// <param name="functionName">The name of the function to retrieve.</param>
78+
/// <param name="functionInstance">When this method returns, the function that was retrieved if one with the specified name was found; otherwise, <see langword="null"/>.</param>
79+
/// <returns><see langword="true"/> if the function was found; otherwise, <see langword="false"/>.</returns>
80+
bool TryGetSemanticFunction(string skillName, string functionName, [NotNullWhen(true)] out ISKFunction? functionInstance);
7381

7482
/// <summary>
75-
/// Return the semantic function delegate stored in the collection
83+
/// Gets the native function stored in the collection.
7684
/// </summary>
77-
/// <param name="functionName">Function name</param>
78-
/// <param name="skillName">Skill name</param>
79-
/// <returns>Semantic function delegate</returns>
80-
ISKFunction GetSemanticFunction(string skillName, string functionName);
85+
/// <param name="functionName">The name of the function to retrieve.</param>
86+
/// <returns>The function retrieved from the collection.</returns>
87+
/// <exception cref="KernelException">The specified function could not be found in the collection.</exception>
88+
ISKFunction GetNativeFunction(string functionName);
8189

8290
/// <summary>
83-
/// Return the native function delegate stored in the collection
91+
/// Gets the native function stored in the collection.
8492
/// </summary>
85-
/// <param name="functionName">Function name</param>
86-
/// <param name="skillName">Skill name</param>
87-
/// <returns>Native function delegate</returns>
93+
/// <param name="skillName">The name of the skill with which the function is associated.</param>
94+
/// <param name="functionName">The name of the function to retrieve.</param>
95+
/// <returns>The function retrieved from the collection.</returns>
96+
/// <exception cref="KernelException">The specified function could not be found in the collection.</exception>
8897
ISKFunction GetNativeFunction(string skillName, string functionName);
8998

9099
/// <summary>
91-
/// Return the native function delegate stored in the collection
100+
/// Gets the native function stored in the collection.
92101
/// </summary>
93-
/// <param name="functionName">Function name</param>
94-
/// <returns>Native function delegate</returns>
95-
ISKFunction GetNativeFunction(string functionName);
102+
/// <param name="functionName">The name of the function to retrieve.</param>
103+
/// <param name="functionInstance">When this method returns, the function that was retrieved if one with the specified name was found; otherwise, <see langword="null"/>.</param>
104+
/// <returns><see langword="true"/> if the function was found; otherwise, <see langword="false"/>.</returns>
105+
bool TryGetNativeFunction(string functionName, [NotNullWhen(true)] out ISKFunction? functionInstance);
106+
107+
/// <summary>
108+
/// Gets the native function stored in the collection.
109+
/// </summary>
110+
/// <param name="skillName">The name of the skill with which the function is associated.</param>
111+
/// <param name="functionName">The name of the function to retrieve.</param>
112+
/// <param name="functionInstance">When this method returns, the function that was retrieved if one with the specified name was found; otherwise, <see langword="null"/>.</param>
113+
/// <returns><see langword="true"/> if the function was found; otherwise, <see langword="false"/>.</returns>
114+
bool TryGetNativeFunction(string skillName, string functionName, [NotNullWhen(true)] out ISKFunction? functionInstance);
96115

97116
/// <summary>
98117
/// Get all registered functions details, minus the delegates

‎dotnet/src/SemanticKernel.Abstractions/SkillDefinition/ISkillCollection.cs‎

Lines changed: 0 additions & 1 deletion
Original file line numberDiff line numberDiff line change
@@ -7,7 +7,6 @@ namespace Microsoft.SemanticKernel.SkillDefinition;
77
/// <summary>
88
/// Skill collection interface.
99
/// </summary>
10-
[System.Diagnostics.CodeAnalysis.SuppressMessage("Naming", "CA1710:Identifiers should have correct suffix", Justification = "It is a collection")]
1110
[System.Diagnostics.CodeAnalysis.SuppressMessage("Naming", "CA1711:Identifiers should not have incorrect suffix", Justification = "It is a collection")]
1211
public interface ISkillCollection : IReadOnlySkillCollection
1312
{

‎dotnet/src/SemanticKernel.UnitTests/KernelTests.cs‎

Lines changed: 2 additions & 1 deletion
Original file line numberDiff line numberDiff line change
@@ -132,7 +132,8 @@ public void ItAllowsToImportSkillsInTheGlobalNamespace()
132132

133133
// Assert
134134
Assert.Equal(3, skill.Count);
135-
Assert.True(kernel.Skills.HasNativeFunction("GetAnyValue"));
135+
Assert.True(kernel.Skills.TryGetNativeFunction("GetAnyValue", out ISKFunction? functionInstance));
136+
Assert.NotNull(functionInstance);
136137
}
137138

138139
[Fact]

‎dotnet/src/SemanticKernel.UnitTests/Planning/PlanningTests.cs‎

Lines changed: 4 additions & 2 deletions
Original file line numberDiff line numberDiff line change
@@ -57,13 +57,15 @@ public async Task ItCanCreatePlanAsync(string goal)
5757
{
5858
skills.Setup(x => x.GetSemanticFunction(It.Is<string>(s => s == skillName), It.Is<string>(s => s == name)))
5959
.Returns(mockFunction.Object);
60-
skills.Setup(x => x.HasSemanticFunction(It.Is<string>(s => s == skillName), It.Is<string>(s => s == name))).Returns(true);
60+
ISKFunction? outFunc = mockFunction.Object;
61+
skills.Setup(x => x.TryGetSemanticFunction(It.Is<string>(s => s == skillName), It.Is<string>(s => s == name), out outFunc)).Returns(true);
6162
}
6263
else
6364
{
6465
skills.Setup(x => x.GetNativeFunction(It.Is<string>(s => s == skillName), It.Is<string>(s => s == name)))
6566
.Returns(mockFunction.Object);
66-
skills.Setup(x => x.HasNativeFunction(It.Is<string>(s => s == skillName), It.Is<string>(s => s == name))).Returns(true);
67+
ISKFunction? outFunc = mockFunction.Object;
68+
skills.Setup(x => x.TryGetNativeFunction(It.Is<string>(s => s == skillName), It.Is<string>(s => s == name), out outFunc)).Returns(true);
6769
}
6870
}
6971

‎dotnet/src/SemanticKernel.UnitTests/Planning/SKContextExtensionsTests.cs‎

Lines changed: 6 additions & 6 deletions
Original file line numberDiff line numberDiff line change
@@ -81,9 +81,9 @@ public async Task CanCallGetAvailableFunctionsWithFunctionsAsync()
8181
x.SearchAsync(It.IsAny<string>(), It.IsAny<string>(), It.IsAny<int>(), It.IsAny<double>(), It.IsAny<bool>(), It.IsAny<CancellationToken>()))
8282
.Returns(asyncEnumerable);
8383

84-
skills.Setup(x => x.HasFunction(It.IsAny<string>(), It.IsAny<string>())).Returns(true);
85-
skills.Setup(x => x.HasSemanticFunction(It.IsAny<string>(), It.IsAny<string>())).Returns(true);
86-
skills.Setup(x => x.HasNativeFunction(It.IsAny<string>(), It.IsAny<string>())).Returns(true);
84+
skills.Setup(x => x.TryGetFunction(It.IsAny<string>(), It.IsAny<string>(), out It.Ref<ISKFunction?>.IsAny)).Returns(true);
85+
skills.Setup(x => x.TryGetSemanticFunction(It.IsAny<string>(), It.IsAny<string>(), out It.Ref<ISKFunction?>.IsAny)).Returns(true);
86+
skills.Setup(x => x.TryGetNativeFunction(It.IsAny<string>(), It.IsAny<string>(), out It.Ref<ISKFunction?>.IsAny)).Returns(true);
8787
skills.Setup(x => x.GetSemanticFunction(It.IsAny<string>(), It.IsAny<string>())).Returns(mockSemanticFunction.Object);
8888
skills.Setup(x => x.GetNativeFunction(It.IsAny<string>(), It.IsAny<string>())).Returns(mockNativeFunction.Object);
8989
skills.Setup(x => x.GetFunctionsView(It.IsAny<bool>(), It.IsAny<bool>())).Returns(functionsView);
@@ -145,9 +145,9 @@ public async Task CanCallGetAvailableFunctionsWithFunctionsWithRelevancyAsync()
145145
x.SearchAsync(It.IsAny<string>(), It.IsAny<string>(), It.IsAny<int>(), It.IsAny<double>(), It.IsAny<bool>(), It.IsAny<CancellationToken>()))
146146
.Returns(asyncEnumerable);
147147

148-
skills.Setup(x => x.HasFunction(It.IsAny<string>(), It.IsAny<string>())).Returns(true);
149-
skills.Setup(x => x.HasSemanticFunction(It.IsAny<string>(), It.IsAny<string>())).Returns(true);
150-
skills.Setup(x => x.HasNativeFunction(It.IsAny<string>(), It.IsAny<string>())).Returns(true);
148+
skills.Setup(x => x.TryGetFunction(It.IsAny<string>(), It.IsAny<string>(), out It.Ref<ISKFunction?>.IsAny)).Returns(true);
149+
skills.Setup(x => x.TryGetSemanticFunction(It.IsAny<string>(), It.IsAny<string>(), out It.Ref<ISKFunction?>.IsAny)).Returns(true);
150+
skills.Setup(x => x.TryGetNativeFunction(It.IsAny<string>(), It.IsAny<string>(), out It.Ref<ISKFunction?>.IsAny)).Returns(true);
151151
skills.Setup(x => x.GetSemanticFunction(It.IsAny<string>(), It.IsAny<string>())).Returns(mockSemanticFunction.Object);
152152
skills.Setup(x => x.GetNativeFunction(It.IsAny<string>(), It.IsAny<string>())).Returns(mockNativeFunction.Object);
153153
skills.Setup(x => x.GetFunctionsView(It.IsAny<bool>(), It.IsAny<bool>())).Returns(functionsView);

‎dotnet/src/SemanticKernel.UnitTests/Planning/SequentialPlanParserTests.cs‎

Lines changed: 4 additions & 2 deletions
Original file line numberDiff line numberDiff line change
@@ -93,13 +93,15 @@ private void CreateKernelAndFunctionCreateMocks(List<(string name, string skillN
9393
{
9494
skills.Setup(x => x.GetSemanticFunction(It.Is<string>(s => s == skillName), It.Is<string>(s => s == name)))
9595
.Returns(mockFunction.Object);
96-
skills.Setup(x => x.HasSemanticFunction(It.Is<string>(s => s == skillName), It.Is<string>(s => s == name))).Returns(true);
96+
ISKFunction? outFunc = mockFunction.Object;
97+
skills.Setup(x => x.TryGetSemanticFunction(It.Is<string>(s => s == skillName), It.Is<string>(s => s == name), out outFunc)).Returns(true);
9798
}
9899
else
99100
{
100101
skills.Setup(x => x.GetNativeFunction(It.Is<string>(s => s == skillName), It.Is<string>(s => s == name)))
101102
.Returns(mockFunction.Object);
102-
skills.Setup(x => x.HasNativeFunction(It.Is<string>(s => s == skillName), It.Is<string>(s => s == name))).Returns(true);
103+
ISKFunction? outFunc = mockFunction.Object;
104+
skills.Setup(x => x.TryGetNativeFunction(It.Is<string>(s => s == skillName), It.Is<string>(s => s == name), out outFunc)).Returns(true);
103105
}
104106
}
105107
}

‎dotnet/src/SemanticKernel.UnitTests/TemplateEngine/Blocks/CodeBlockTests.cs‎

Lines changed: 9 additions & 5 deletions
Original file line numberDiff line numberDiff line change
@@ -33,7 +33,7 @@ public async Task ItThrowsIfAFunctionDoesntExistAsync()
3333
{
3434
// Arrange
3535
var context = new SKContext(new ContextVariables(), NullMemory.Instance, this._skills.Object, this._log.Object);
36-
this._skills.Setup(x => x.HasFunction("functionName")).Returns(false);
36+
this._skills.Setup(x => x.TryGetFunction("functionName", out It.Ref<ISKFunction?>.IsAny)).Returns(false);
3737
var target = new CodeBlock("functionName", this._log.Object);
3838

3939
// Act
@@ -52,7 +52,8 @@ public async Task ItThrowsIfAFunctionCallThrowsAsync()
5252
function
5353
.Setup(x => x.InvokeAsync(It.IsAny<SKContext?>(), It.IsAny<CompleteRequestSettings?>(), It.IsAny<ILogger?>(), It.IsAny<CancellationToken?>()))
5454
.Throws(new RuntimeWrappedException("error"));
55-
this._skills.Setup(x => x.HasFunction("functionName")).Returns(true);
55+
ISKFunction? outFunc = function.Object;
56+
this._skills.Setup(x => x.TryGetFunction("functionName", out outFunc)).Returns(true);
5657
this._skills.Setup(x => x.GetFunction("functionName")).Returns(function.Object);
5758
var target = new CodeBlock("functionName", this._log.Object);
5859

@@ -209,7 +210,8 @@ public async Task ItInvokesFunctionCloningAllVariablesAsync()
209210
ctx["var2"] = "overridden";
210211
});
211212

212-
this._skills.Setup(x => x.HasFunction(FUNC)).Returns(true);
213+
ISKFunction? outFunc = function.Object;
214+
this._skills.Setup(x => x.TryGetFunction(FUNC, out outFunc)).Returns(true);
213215
this._skills.Setup(x => x.GetFunction(FUNC)).Returns(function.Object);
214216

215217
// Act
@@ -249,7 +251,8 @@ public async Task ItInvokesFunctionWithCustomVariableAsync()
249251
canary = ctx!["input"];
250252
});
251253

252-
this._skills.Setup(x => x.HasFunction(FUNC)).Returns(true);
254+
ISKFunction? outFunc = function.Object;
255+
this._skills.Setup(x => x.TryGetFunction(FUNC, out outFunc)).Returns(true);
253256
this._skills.Setup(x => x.GetFunction(FUNC)).Returns(function.Object);
254257

255258
// Act
@@ -281,7 +284,8 @@ public async Task ItInvokesFunctionWithCustomValueAsync()
281284
canary = ctx!["input"];
282285
});
283286

284-
this._skills.Setup(x => x.HasFunction(FUNC)).Returns(true);
287+
ISKFunction? outFunc = function.Object;
288+
this._skills.Setup(x => x.TryGetFunction(FUNC, out outFunc)).Returns(true);
285289
this._skills.Setup(x => x.GetFunction(FUNC)).Returns(function.Object);
286290

287291
// Act

0 commit comments

Comments
 (0)