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

Fixes name collisions in TS refiner between models and interfaces. #4667

Merged
merged 3 commits into from
May 15, 2024
Merged
Show file tree
Hide file tree
Changes from all commits
Commits
File filter

Filter by extension

Filter by extension

Conversations
Failed to load comments.
Loading
Jump to
Jump to file
Failed to load files.
Loading
Diff view
Diff view
1 change: 1 addition & 0 deletions CHANGELOG.md
Original file line number Diff line number Diff line change
Expand Up @@ -34,6 +34,7 @@ and this project adheres to [Semantic Versioning](https://semver.org/spec/v2.0.0
- Fixed a bug where schema with multiple allOf entries would incorrectly get merged to inherit from the first entry [#4428] (https://github.com/microsoft/kiota/issues/4428)
- Fixes constructor generation for nullable properties that are initialized as null in C#,Java and PHP
- Fixed a bug where the hash alias in typescript wasn't being generated uniformly for similar interfaces [#84](https://github.com/microsoftgraph/msgraph-beta-sdk-typescript/issues/84)
- Fixes a bug where name collisions would occur in the Typescript refiner if model name also exists with the `Interface` suffix [#4382](https://github.com/microsoft/kiota/issues/4382)

## [1.14.0] - 2024-05-02

Expand Down
4 changes: 2 additions & 2 deletions src/Kiota.Builder/CodeDOM/CodeInterface.cs
Original file line number Diff line number Diff line change
Expand Up @@ -13,9 +13,9 @@

public class CodeInterface : ProprietableBlock<CodeInterfaceKind, InterfaceDeclaration>, ITypeDefinition, IDeprecableElement
{
public CodeClass? OriginalClass
public required CodeClass OriginalClass
{
get; set;
get; init;
}
public DeprecationInformation? Deprecation
{
Expand Down Expand Up @@ -62,6 +62,6 @@
return result;
}
}
public class InterfaceDeclaration : ProprietableBlockDeclaration

Check warning on line 65 in src/Kiota.Builder/CodeDOM/CodeInterface.cs

View workflow job for this annotation

GitHub Actions / Build

Remove this empty class, write its code or make it an "interface". (https://rules.sonarsource.com/csharp/RSPEC-2094)

Check warning on line 65 in src/Kiota.Builder/CodeDOM/CodeInterface.cs

View workflow job for this annotation

GitHub Actions / Build

Remove this empty class, write its code or make it an "interface". (https://rules.sonarsource.com/csharp/RSPEC-2094)
{
}
28 changes: 18 additions & 10 deletions src/Kiota.Builder/Refiners/TypeScriptRefiner.cs
Original file line number Diff line number Diff line change
Expand Up @@ -659,6 +659,7 @@ codeClass.Parent is CodeClass parentClass &&
Kind = CodeInterfaceKind.QueryParameters,
Documentation = codeClass.Documentation,
Deprecation = codeClass.Deprecation,
OriginalClass = codeClass
};
parentClass.RemoveChildElement(codeClass);
var codeInterface = targetNS.AddInterface(insertValue).First();
Expand All @@ -674,7 +675,7 @@ codeClass.Parent is CodeClass parentClass &&
}
CrawlTree(currentElement, ReplaceRequestQueryParamsWithInterfaces);
}
private const string TemporaryInterfaceNameSuffix = "Interface";

private const string ModelSerializerPrefix = "serialize";
private const string ModelDeserializerPrefix = "deserializeInto";

Expand Down Expand Up @@ -737,9 +738,9 @@ private static void AddInterfaceParamToSerializer(CodeInterface modelInterface,

method.AddParameter(new CodeParameter
{
Name = ReturnFinalInterfaceName(modelInterface.Name), // remove the interface suffix
Name = ReturnFinalInterfaceName(modelInterface),
DefaultValue = "{}",
Type = new CodeType { Name = ReturnFinalInterfaceName(modelInterface.Name), TypeDefinition = modelInterface },
Type = new CodeType { Name = ReturnFinalInterfaceName(modelInterface), TypeDefinition = modelInterface },
Kind = CodeParameterKind.DeserializationTarget,
});

Expand All @@ -750,7 +751,7 @@ private static void AddInterfaceParamToSerializer(CodeInterface modelInterface,
Name = modelInterface.Parent.Name,
Declaration = new CodeType
{
Name = ReturnFinalInterfaceName(modelInterface.Name),
Name = ReturnFinalInterfaceName(modelInterface),
TypeDefinition = modelInterface
}
});
Expand Down Expand Up @@ -781,7 +782,7 @@ private static void RenameModelInterfacesAndRemoveClasses(CodeElement currentEle
{
if (currentElement is CodeInterface modelInterface && modelInterface.IsOfKind(CodeInterfaceKind.Model) && modelInterface.Parent is CodeNamespace parentNS)
{
var finalName = ReturnFinalInterfaceName(modelInterface.Name);
var finalName = ReturnFinalInterfaceName(modelInterface);
if (!finalName.Equals(modelInterface.Name, StringComparison.Ordinal))
{
if (parentNS.FindChildByName<CodeClass>(finalName, false) is CodeClass existingClassToRemove)
Expand All @@ -801,13 +802,13 @@ private static void RenameCodeInterfaceParamsInSerializers(CodeFunction codeFunc
{
if (codeFunction.OriginalLocalMethod.Parameters.FirstOrDefault(static x => x.Type is CodeType codeType && codeType.TypeDefinition is CodeInterface) is CodeParameter param && param.Type is CodeType codeType && codeType.TypeDefinition is CodeInterface paramInterface)
{
param.Name = ReturnFinalInterfaceName(paramInterface.Name);
param.Name = ReturnFinalInterfaceName(paramInterface);
}
}

private static string ReturnFinalInterfaceName(string interfaceName)
private static string ReturnFinalInterfaceName(CodeInterface codeInterface)
{
return interfaceName.EndsWith(TemporaryInterfaceNameSuffix, StringComparison.Ordinal) ? interfaceName[..^TemporaryInterfaceNameSuffix.Length] : interfaceName;
return codeInterface.OriginalClass.Name.ToFirstCharacterUpperCase();
}

private static void GenerateModelInterfaces(CodeElement currentElement, Func<CodeClass, string> interfaceNamingCallback)
Expand Down Expand Up @@ -925,7 +926,7 @@ private static void SetTypeAsModelInterface(CodeInterface interfaceElement, Code
Name = interfaceElement.Name,
TypeDefinition = interfaceElement,
};
requestBuilder.RemoveUsingsByDeclarationName(interfaceElement.Name.Split(TemporaryInterfaceNameSuffix)[0]);
requestBuilder.RemoveUsingsByDeclarationName(ReturnFinalInterfaceName(interfaceElement));
if (!requestBuilder.Usings.Any(x => x.Declaration?.TypeDefinition == elemType.TypeDefinition))
{
requestBuilder.AddUsing(new CodeUsing
Expand Down Expand Up @@ -957,12 +958,19 @@ private static CodeInterface CreateModelInterface(CodeClass modelClass, Func<Cod
if (namespaceOfModel.FindChildByName<CodeInterface>(temporaryInterfaceName, false) is CodeInterface existing)
return existing;

var i = 1;
while (namespaceOfModel.FindChildByName<CodeClass>(temporaryInterfaceName, false) != null)
{// We already know an Interface doesn't exist with the name. Make sure we don't collide with an existing class name in the namespace.
temporaryInterfaceName = $"{temporaryInterfaceName}{i++}";
}

var insertValue = new CodeInterface
{
Name = temporaryInterfaceName,
Kind = CodeInterfaceKind.Model,
Documentation = modelClass.Documentation,
Deprecation = modelClass.Deprecation,
OriginalClass = modelClass
};

var modelInterface = modelClass.Parent is CodeClass modelParentClass ?
Expand All @@ -987,7 +995,7 @@ private static void ProcessModelClassDeclaration(CodeClass modelClass, CodeInter
var parentInterface = CreateModelInterface(baseClass, tempInterfaceNamingCallback);
var codeType = new CodeType
{
Name = ReturnFinalInterfaceName(parentInterface.Name),
Name = ReturnFinalInterfaceName(parentInterface),
TypeDefinition = parentInterface,
};
modelInterface.StartBlock.AddImplements(codeType);
Expand Down
3 changes: 2 additions & 1 deletion tests/Kiota.Builder.Tests/CodeDOM/CodeClassTests.cs
Original file line number Diff line number Diff line change
Expand Up @@ -193,7 +193,8 @@ public void AddsInnerInterface()
}).First();
codeClass.AddInnerInterface(new CodeInterface
{
Name = "subinterface"
Name = "subinterface",
OriginalClass = new CodeClass() { Name = "originalSubInterface" }
});
Assert.Single(codeClass.GetChildElements(true).OfType<CodeInterface>());
Assert.Throws<ArgumentNullException>(() =>
Expand Down
Original file line number Diff line number Diff line change
Expand Up @@ -626,6 +626,23 @@ public async Task AddsModelInterfaceForAModelClass()
Assert.Contains(codeFile.Interfaces, static x => "ModelA".Equals(x.Name, StringComparison.OrdinalIgnoreCase));
}

[Fact]
public async Task AddsModelInterfaceForAModelClassWithoutCollision()
{
var generationConfiguration = new GenerationConfiguration { Language = GenerationLanguage.TypeScript };
TestHelper.CreateModelClassInModelsNamespace(generationConfiguration, root, "hostModel");
TestHelper.CreateModelClassInModelsNamespace(generationConfiguration, root, "hostModelInterface");// a second model with the `Interface` suffix

await ILanguageRefiner.Refine(generationConfiguration, root);

var modelsNS = root.FindNamespaceByName(generationConfiguration.ModelsNamespaceName);
var codeFile = modelsNS.FindChildByName<CodeFile>(IndexFileName, false);
Assert.NotNull(codeFile);
Assert.Equal(2, codeFile.Interfaces.Count());
Assert.Contains(codeFile.Interfaces, static x => "hostModel".Equals(x.Name, StringComparison.OrdinalIgnoreCase));
Assert.Contains(codeFile.Interfaces, static x => "hostModelInterface".Equals(x.Name, StringComparison.OrdinalIgnoreCase));
}

[Fact]
public async Task ReplaceRequestConfigsQueryParams()
{
Expand Down
Original file line number Diff line number Diff line change
Expand Up @@ -27,7 +27,8 @@ public CodeInterfaceDeclarationWriterTests()
root = CodeNamespace.InitRootNamespace();
parentInterface = new()
{
Name = "parentClass"
Name = "parentClass",
OriginalClass = new CodeClass() { Name = "parentClass" }
};
root.AddInterface(parentInterface);
}
Expand Down
Original file line number Diff line number Diff line change
Expand Up @@ -99,6 +99,7 @@ public void WritesInheritance()
var interfaceDef = new CodeInterface
{
Name = "SomeInterface",
OriginalClass = new CodeClass() { Name = "SomeInterface" }
};
ns.AddInterface(interfaceDef);
var nUsing = new CodeUsing
Expand Down
Original file line number Diff line number Diff line change
Expand Up @@ -349,6 +349,7 @@ public void WritesRequestGeneratorBodyForParsable()
{
Name = "SomeComplexTypeForRequestBody",
Kind = CodeInterfaceKind.Model,
OriginalClass = new CodeClass() { Name = "SomeComplexTypeForRequestBody" }
},
};
generatorMethod.AcceptedResponseTypes.Add("application/json");
Expand Down Expand Up @@ -502,7 +503,8 @@ public void WritesIndexer()
var parentInterface = new CodeInterface
{
Name = "parentClass",
Kind = CodeInterfaceKind.RequestBuilder
Kind = CodeInterfaceKind.RequestBuilder,
OriginalClass = new CodeClass() { Name = "parentClass" }
};
method.AddParameter(new CodeParameter
{
Expand Down
Original file line number Diff line number Diff line change
Expand Up @@ -896,6 +896,7 @@ public void WritesReturnType()
{
Name = "SomeInterface",
Kind = CodeInterfaceKind.Model,
OriginalClass = new CodeClass() { Name = "SomeInterface" }
}).First();
var method = TestHelper.CreateMethod(parentClass, MethodName, ReturnTypeName);
method.Kind = CodeMethodKind.Serializer;
Expand Down Expand Up @@ -930,6 +931,7 @@ public void DoesNotAddUndefinedOnNonNullableReturnType()
{
Name = "SomeInterface",
Kind = CodeInterfaceKind.Model,
OriginalClass = new CodeClass() { Name = "SomeInterface" }
}).First();
var method = TestHelper.CreateMethod(parentClass, MethodName, ReturnTypeName);
method.Kind = CodeMethodKind.Serializer;
Expand Down Expand Up @@ -963,6 +965,7 @@ public void DoesNotAddAsyncInformationOnSyncMethods()
{
Name = "SomeInterface",
Kind = CodeInterfaceKind.Model,
OriginalClass = new CodeClass() { Name = "SomeInterface" }
}).First();
var method = TestHelper.CreateMethod(parentClass, MethodName, ReturnTypeName);
method.Kind = CodeMethodKind.Serializer;
Expand Down Expand Up @@ -997,6 +1000,7 @@ public void WritesPublicMethodByDefault()
{
Name = "SomeInterface",
Kind = CodeInterfaceKind.Model,
OriginalClass = new CodeClass() { Name = "SomeInterface" }
}).First();
var method = TestHelper.CreateMethod(parentClass, MethodName, ReturnTypeName);
method.Kind = CodeMethodKind.Serializer;
Expand Down Expand Up @@ -1029,6 +1033,7 @@ public void WritesPrivateMethod()
{
Name = "SomeInterface",
Kind = CodeInterfaceKind.Model,
OriginalClass = new CodeClass() { Name = "SomeInterface" }
}).First();
var method = TestHelper.CreateMethod(parentClass, MethodName, ReturnTypeName);
method.Kind = CodeMethodKind.Serializer;
Expand Down Expand Up @@ -1062,6 +1067,7 @@ public void WritesProtectedMethod()
{
Name = "SomeInterface",
Kind = CodeInterfaceKind.Model,
OriginalClass = new CodeClass() { Name = "SomeInterface" }
}).First();
var method = TestHelper.CreateMethod(parentClass, MethodName, ReturnTypeName);
method.Kind = CodeMethodKind.Serializer;
Expand Down
Original file line number Diff line number Diff line change
@@ -1,5 +1,6 @@
using System;
using System.IO;
using System.Linq;
using Kiota.Builder.CodeDOM;
using Xunit;

Expand All @@ -19,9 +20,11 @@ public CodeInterfaceDeclaraterWriterTests()
writer.SetTextWriter(tw);
var root = CodeNamespace.InitRootNamespace();
var ns = root.AddNamespace("graphtests.models");
var originalClass = ns.AddClass(new CodeClass() { Name = "originalParentClass" }).First();
parentInterface = new CodeInterface()
{
Name = "parent"
Name = "parent",
OriginalClass = originalClass
};
ns.AddInterface(parentInterface);
}
Expand Down
Original file line number Diff line number Diff line change
Expand Up @@ -25,7 +25,8 @@ public CodePropertyWriterTests()
var root = CodeNamespace.InitRootNamespace();
parentInterface = root.AddInterface(new CodeInterface
{
Name = "parentClass"
Name = "parentClass",
OriginalClass = new CodeClass() { Name = "parentClass" }
}).First();
property = new CodeProperty
{
Expand Down
Original file line number Diff line number Diff line change
Expand Up @@ -74,7 +74,8 @@ public void WritesImportTypeStatementForGeneratedInterfaces()
var someInterface = new CodeInterface
{
Name = "Bar",
Kind = CodeInterfaceKind.Model
Kind = CodeInterfaceKind.Model,
OriginalClass = new CodeClass() { Name = "Bar" }
};
root.AddInterface(someInterface);
var us = new CodeUsing
Expand Down
Loading