Skip to content

Commit

Permalink
feat: Control generated types access modifier for C# (#5284)
Browse files Browse the repository at this point in the history
* Control generated class access modifier for C#

- Add `--class-access-modifier` flag.

* Review feedback

- Rename `--class-access-modifier` to `--type-access-modifier`.
- Use `AccessModifier` enum instead of string and extend to `Internal` value.
- Add `Access` property to `CodeClass` and `CodeEnum`.
- Use `CSharpRefiner` to set `Access` property from configuration.
- Extend `CodeClassDeclarationWriter` and `CodeEnumWriter` to write actual type access modifier.

* Review feedback 2

- Add `TypeAccessModifierOption` to client handlers.
- Add `TypeAccessModifier` to workspace `ApiClientConfiguration` and `ApiClientConfigurationComparer`.
- Add default value of `Public` for stringified `TypeAccessModifier` properties.
- Add test case for `CSharpRefiner`.
- Implement `IAccessibleElement` for `CodeClass` and `CodeEnum`.

* Review feedback 3

- Use parenthesized pattern matching for type checks.

* Move changelog to unreleased section

* chore: runs dotnet format

---------

Co-authored-by: Vincent Biret <[email protected]>
  • Loading branch information
NKnusperer and baywet authored Sep 13, 2024
1 parent 5f6f5cb commit fac7928
Show file tree
Hide file tree
Showing 23 changed files with 141 additions and 6 deletions.
1 change: 1 addition & 0 deletions CHANGELOG.md
Original file line number Diff line number Diff line change
Expand Up @@ -10,6 +10,7 @@ and this project adheres to [Semantic Versioning](https://semver.org/spec/v2.0.0
## [Unreleased]

### Added
- Control generated type access modifier for C# via `--type-access-modifier` flag. [#4788](https://github.com/microsoft/kiota/issues/4788)

### Changed

Expand Down
1 change: 1 addition & 0 deletions src/Kiota.Builder/CodeDOM/AccessModifier.cs
Original file line number Diff line number Diff line change
@@ -1,6 +1,7 @@
namespace Kiota.Builder.CodeDOM;
public enum AccessModifier
{
Internal = 3,
Public = 2,
Protected = 1,
Private = 0
Expand Down
5 changes: 4 additions & 1 deletion src/Kiota.Builder/CodeDOM/CodeClass.cs
Original file line number Diff line number Diff line change
Expand Up @@ -30,9 +30,12 @@ public enum CodeClassKind
/// <summary>
/// CodeClass represents an instance of a Class to be generated in source code
/// </summary>
public class CodeClass : ProprietableBlock<CodeClassKind, ClassDeclaration>, ITypeDefinition, IDiscriminatorInformationHolder, IDeprecableElement
public class CodeClass : ProprietableBlock<CodeClassKind, ClassDeclaration>, ITypeDefinition, IDiscriminatorInformationHolder, IDeprecableElement, IAccessibleElement
{
private readonly ConcurrentDictionary<string, CodeProperty> PropertiesByWireName = new(StringComparer.OrdinalIgnoreCase);

public AccessModifier Access { get; set; } = AccessModifier.Public;

public bool IsErrorDefinition
{
get; set;
Expand Down
3 changes: 2 additions & 1 deletion src/Kiota.Builder/CodeDOM/CodeEnum.cs
Original file line number Diff line number Diff line change
Expand Up @@ -5,9 +5,10 @@

namespace Kiota.Builder.CodeDOM;
#pragma warning disable CA1711
public class CodeEnum : CodeBlock<BlockDeclaration, BlockEnd>, IDocumentedElement, ITypeDefinition, IDeprecableElement
public class CodeEnum : CodeBlock<BlockDeclaration, BlockEnd>, IDocumentedElement, ITypeDefinition, IDeprecableElement, IAccessibleElement
{
#pragma warning restore CA2227
public AccessModifier Access { get; set; } = AccessModifier.Public;
public bool Flags
{
get; set;
Expand Down
2 changes: 2 additions & 0 deletions src/Kiota.Builder/Configuration/GenerationConfiguration.cs
Original file line number Diff line number Diff line change
Expand Up @@ -3,6 +3,7 @@
using System.IO;
using System.Linq;
using System.Text.Json.Nodes;
using Kiota.Builder.CodeDOM;
using Kiota.Builder.Extensions;
using Kiota.Builder.Lock;
using Microsoft.OpenApi.ApiManifest;
Expand Down Expand Up @@ -38,6 +39,7 @@ public ConsumerOperation? Operation
public string ApiManifestPath { get; set; } = "apimanifest.json";
public string OutputPath { get; set; } = "./output";
public string ClientClassName { get; set; } = "ApiClient";
public AccessModifier TypeAccessModifier { get; set; } = AccessModifier.Public;
public string ClientNamespaceName { get; set; } = "ApiSdk";
public string NamespaceNameSeparator { get; set; } = ".";
public bool ExportPublicApi
Expand Down
10 changes: 9 additions & 1 deletion src/Kiota.Builder/Lock/KiotaLock.cs
Original file line number Diff line number Diff line change
@@ -1,6 +1,7 @@
using System;
using System.Collections.Generic;
using System.Linq;
using Kiota.Builder.CodeDOM;
using Kiota.Builder.Configuration;

namespace Kiota.Builder.Lock;
Expand Down Expand Up @@ -31,6 +32,10 @@ public class KiotaLock
/// </summary>
public string ClientClassName { get; set; } = string.Empty;
/// <summary>
/// The type access modifier to use for the client types.
/// </summary>
public string TypeAccessModifier { get; set; } = "Public";
/// <summary>
/// The main namespace for this client.
/// </summary>
public string ClientNamespaceName { get; set; } = string.Empty;
Expand Down Expand Up @@ -102,9 +107,11 @@ public void UpdateGenerationConfigurationFromLock(GenerationConfiguration config
{
ArgumentNullException.ThrowIfNull(config);
config.ClientClassName = ClientClassName;
config.ClientNamespaceName = ClientNamespaceName;
if (Enum.TryParse<GenerationLanguage>(Language, out var parsedLanguage))
config.Language = parsedLanguage;
config.ClientNamespaceName = ClientNamespaceName;
if (Enum.TryParse<AccessModifier>(TypeAccessModifier, out var parsedAccessModifier))
config.TypeAccessModifier = parsedAccessModifier;
config.UsesBackingStore = UsesBackingStore;
config.ExcludeBackwardCompatible = ExcludeBackwardCompatible;
config.IncludeAdditionalData = IncludeAdditionalData;
Expand Down Expand Up @@ -132,6 +139,7 @@ public KiotaLock(GenerationConfiguration config)
ArgumentNullException.ThrowIfNull(config);
Language = config.Language.ToString();
ClientClassName = config.ClientClassName;
TypeAccessModifier = config.TypeAccessModifier.ToString();
ClientNamespaceName = config.ClientNamespaceName;
UsesBackingStore = config.UsesBackingStore;
ExcludeBackwardCompatible = config.ExcludeBackwardCompatible;
Expand Down
2 changes: 2 additions & 0 deletions src/Kiota.Builder/Lock/KiotaLockComparer.cs
Original file line number Diff line number Diff line change
Expand Up @@ -35,6 +35,7 @@ public bool Equals(KiotaLock? x, KiotaLock? y)
&& _stringComparer.Equals(x.ClientClassName, y.ClientClassName)
&& _stringComparer.Equals(x.ClientNamespaceName, y.ClientNamespaceName)
&& _stringComparer.Equals(x.Language, y.Language)
&& _stringComparer.Equals(x.TypeAccessModifier, y.TypeAccessModifier)
&& _stringIEnumerableDeepComparer.Equals(x.DisabledValidationRules, y.DisabledValidationRules)
&& _stringIEnumerableDeepComparer.Equals(x.Serializers, y.Serializers)
&& _stringIEnumerableDeepComparer.Equals(x.Deserializers, y.Deserializers)
Expand All @@ -56,6 +57,7 @@ public int GetHashCode([DisallowNull] KiotaLock obj)
hash.Add(obj.ClientClassName, _stringComparer);
hash.Add(obj.ClientNamespaceName, _stringComparer);
hash.Add(obj.Language, _stringComparer);
hash.Add(obj.TypeAccessModifier, _stringComparer);
hash.Add(obj.ExcludeBackwardCompatible);
hash.Add(obj.UsesBackingStore);
hash.Add(obj.IncludeAdditionalData);
Expand Down
11 changes: 11 additions & 0 deletions src/Kiota.Builder/Refiners/CSharpRefiner.cs
Original file line number Diff line number Diff line change
Expand Up @@ -110,6 +110,7 @@ public override Task RefineAsync(CodeNamespace generatedCode, CancellationToken
generatedCode,
"IParseNode"
);
SetTypeAccessModifiers(generatedCode);
}, cancellationToken);
}
protected static void DisambiguatePropertiesWithClassNames(CodeElement currentElement)
Expand Down Expand Up @@ -260,4 +261,14 @@ protected static void CorrectIndexerType(CodeIndexer currentIndexer)
})
},
};

private void SetTypeAccessModifiers(CodeElement currentElement)
{
if (currentElement is IAccessibleElement accessibleElement and (CodeEnum or CodeClass))
{
accessibleElement.Access = _configuration.TypeAccessModifier;
}

CrawlTree(currentElement, SetTypeAccessModifiers);
}
}
Original file line number Diff line number Diff line change
Expand Up @@ -2,6 +2,7 @@
using System.Collections.Generic;
using System.IO;
using System.Linq;
using Kiota.Builder.CodeDOM;
using Kiota.Builder.Configuration;
using Microsoft.OpenApi.ApiManifest;

Expand All @@ -15,6 +16,10 @@ public class ApiClientConfiguration : BaseApiConsumerConfiguration, ICloneable
/// </summary>
public string Language { get; set; } = string.Empty;
/// <summary>
/// The type access modifier to use for the client types.
/// </summary>
public string TypeAccessModifier { get; set; } = "Public";
/// <summary>
/// The structured mime types used for this client.
/// </summary>
#pragma warning disable CA1002
Expand Down Expand Up @@ -64,6 +69,7 @@ public ApiClientConfiguration(GenerationConfiguration config) : base(config)
{
ArgumentNullException.ThrowIfNull(config);
Language = config.Language.ToString();
TypeAccessModifier = config.TypeAccessModifier.ToString();
ClientNamespaceName = config.ClientNamespaceName;
UsesBackingStore = config.UsesBackingStore;
ExcludeBackwardCompatible = config.ExcludeBackwardCompatible;
Expand All @@ -84,6 +90,8 @@ public void UpdateGenerationConfigurationFromApiClientConfiguration(GenerationCo
config.ClientNamespaceName = ClientNamespaceName;
if (Enum.TryParse<GenerationLanguage>(Language, out var parsedLanguage))
config.Language = parsedLanguage;
if (Enum.TryParse<AccessModifier>(TypeAccessModifier, out var parsedTypeAccessModifier))
config.TypeAccessModifier = parsedTypeAccessModifier;
config.UsesBackingStore = UsesBackingStore;
config.ExcludeBackwardCompatible = ExcludeBackwardCompatible;
config.IncludeAdditionalData = IncludeAdditionalData;
Expand All @@ -97,6 +105,7 @@ public object Clone()
var result = new ApiClientConfiguration
{
Language = Language,
TypeAccessModifier = TypeAccessModifier,
StructuredMimeTypes = [.. StructuredMimeTypes],
ClientNamespaceName = ClientNamespaceName,
UsesBackingStore = UsesBackingStore,
Expand Down
Original file line number Diff line number Diff line change
Expand Up @@ -28,6 +28,7 @@ public override bool Equals(ApiClientConfiguration? x, ApiClientConfiguration? y
if (x.IncludeAdditionalData != y.IncludeAdditionalData) return false;
if (!_stringComparer.Equals(x.ClientNamespaceName, y.ClientNamespaceName)) return false;
if (!_stringComparer.Equals(x.Language, y.Language)) return false;
if (!_stringComparer.Equals(x.TypeAccessModifier, y.TypeAccessModifier)) return false;

// slow deep comparison
return _stringIEnumerableDeepComparer.Equals(x.DisabledValidationRules, y.DisabledValidationRules)
Expand All @@ -42,6 +43,7 @@ public override int GetHashCode([DisallowNull] ApiClientConfiguration obj)
hash.Add(obj.DisabledValidationRules, _stringIEnumerableDeepComparer); // _stringIEnumerableDeepComparer orders
hash.Add(obj.ClientNamespaceName, _stringComparer);
hash.Add(obj.Language, _stringComparer);
hash.Add(obj.TypeAccessModifier, _stringComparer);
hash.Add(obj.ExcludeBackwardCompatible);
hash.Add(obj.UsesBackingStore);
hash.Add(obj.IncludeAdditionalData);
Expand Down
Original file line number Diff line number Diff line change
Expand Up @@ -96,6 +96,7 @@ public override string GetAccessModifier(AccessModifier access)
{
return access switch
{
AccessModifier.Internal => "internal",
AccessModifier.Public => "public",
AccessModifier.Protected => "protected",
_ => "private",
Expand Down
Original file line number Diff line number Diff line change
Expand Up @@ -41,7 +41,7 @@ public override void WriteCodeElement(ClassDeclaration codeElement, LanguageWrit
conventions.WriteDeprecationAttribute(parentClass, writer);
writer.WriteLine(GeneratedCodeAttribute);
if (!hasDescription) conventions.WritePragmaDisable(writer, CSharpConventionService.CS1591);
writer.WriteLine($"public partial class {codeElement.Name.ToFirstCharacterUpperCase()} {derivation}");
writer.WriteLine($"{conventions.GetAccessModifier(parentClass.Access)} partial class {codeElement.Name.ToFirstCharacterUpperCase()} {derivation}");
if (!hasDescription) conventions.WritePragmaRestore(writer, CSharpConventionService.CS1591);
writer.StartBlock();
}
Expand Down
2 changes: 1 addition & 1 deletion src/Kiota.Builder/Writers/CSharp/CodeEnumWriter.cs
Original file line number Diff line number Diff line change
Expand Up @@ -37,7 +37,7 @@ public override void WriteCodeElement(CodeEnum codeElement, LanguageWriter write
writer.WriteLine("[Flags]");
conventions.WriteDeprecationAttribute(codeElement, writer);
if (!hasDescription) writer.WriteLine("#pragma warning disable CS1591");
writer.WriteLine($"public enum {codeElement.Name.ToFirstCharacterUpperCase()}");
writer.WriteLine($"{conventions.GetAccessModifier(codeElement.Access)} enum {codeElement.Name.ToFirstCharacterUpperCase()}");
if (!hasDescription) writer.WriteLine("#pragma warning restore CS1591");
writer.StartBlock();
var idx = 0;
Expand Down
7 changes: 7 additions & 0 deletions src/kiota/Handlers/Client/AddHandler.cs
Original file line number Diff line number Diff line change
Expand Up @@ -2,6 +2,7 @@
using System.CommandLine.Invocation;
using System.Text.Json;
using Kiota.Builder;
using Kiota.Builder.CodeDOM;
using Kiota.Builder.Configuration;
using Kiota.Builder.Extensions;
using Kiota.Builder.WorkspaceManagement;
Expand All @@ -27,6 +28,10 @@ public required Option<GenerationLanguage> LanguageOption
{
get; init;
}
public required Option<AccessModifier> TypeAccessModifierOption
{
get; init;
}
public required Option<string> DescriptionOption
{
get; init;
Expand Down Expand Up @@ -69,6 +74,7 @@ public override async Task<int> InvokeAsync(InvocationContext context)
{
string output = context.ParseResult.GetValueForOption(OutputOption) ?? string.Empty;
GenerationLanguage language = context.ParseResult.GetValueForOption(LanguageOption);
AccessModifier typeAccessModifier = context.ParseResult.GetValueForOption(TypeAccessModifierOption);
string openapi = context.ParseResult.GetValueForOption(DescriptionOption) ?? string.Empty;
bool backingStore = context.ParseResult.GetValueForOption(BackingStoreOption);
bool excludeBackwardCompatible = context.ParseResult.GetValueForOption(ExcludeBackwardCompatibleOption);
Expand All @@ -90,6 +96,7 @@ public override async Task<int> InvokeAsync(InvocationContext context)
Configuration.Generation.IncludeAdditionalData = includeAdditionalData;
Configuration.Generation.Language = language;
WarnUsingPreviewLanguage(language);
Configuration.Generation.TypeAccessModifier = typeAccessModifier;
Configuration.Generation.SkipGeneration = skipGeneration;
Configuration.Generation.Operation = ConsumerOperation.Add;
if (includePatterns.Count != 0)
Expand Down
8 changes: 8 additions & 0 deletions src/kiota/Handlers/Client/EditHandler.cs
Original file line number Diff line number Diff line change
Expand Up @@ -2,6 +2,7 @@
using System.CommandLine.Invocation;
using System.Text.Json;
using Kiota.Builder;
using Kiota.Builder.CodeDOM;
using Kiota.Builder.Configuration;
using Kiota.Builder.Extensions;
using Kiota.Builder.WorkspaceManagement;
Expand All @@ -27,6 +28,10 @@ public required Option<GenerationLanguage?> LanguageOption
{
get; init;
}
public required Option<AccessModifier?> TypeAccessModifierOption
{
get; init;
}
public required Option<string> DescriptionOption
{
get; init;
Expand Down Expand Up @@ -69,6 +74,7 @@ public override async Task<int> InvokeAsync(InvocationContext context)
{
string output = context.ParseResult.GetValueForOption(OutputOption) ?? string.Empty;
GenerationLanguage? language = context.ParseResult.GetValueForOption(LanguageOption);
AccessModifier? typeAccessModifier = context.ParseResult.GetValueForOption(TypeAccessModifierOption);
string openapi = context.ParseResult.GetValueForOption(DescriptionOption) ?? string.Empty;
bool? backingStore = context.ParseResult.GetValueForOption(BackingStoreOption);
bool? excludeBackwardCompatible = context.ParseResult.GetValueForOption(ExcludeBackwardCompatibleOption);
Expand Down Expand Up @@ -109,6 +115,8 @@ public override async Task<int> InvokeAsync(InvocationContext context)
clientConfiguration.UpdateGenerationConfigurationFromApiClientConfiguration(Configuration.Generation, className);
if (language.HasValue)
Configuration.Generation.Language = language.Value;
if (typeAccessModifier.HasValue)
Configuration.Generation.TypeAccessModifier = typeAccessModifier.Value;
if (backingStore.HasValue)
Configuration.Generation.UsesBackingStore = backingStore.Value;
if (excludeBackwardCompatible.HasValue)
Expand Down
7 changes: 7 additions & 0 deletions src/kiota/Handlers/KiotaGenerateCommandHandler.cs
Original file line number Diff line number Diff line change
Expand Up @@ -3,6 +3,7 @@
using System.Text.Json;

using Kiota.Builder;
using Kiota.Builder.CodeDOM;
using Kiota.Builder.Extensions;

using Microsoft.Extensions.Logging;
Expand All @@ -27,6 +28,10 @@ public required Option<string> ClassOption
{
get; init;
}
public required Option<AccessModifier> TypeAccessModifierOption
{
get; init;
}
public required Option<string> NamespaceOption
{
get; init;
Expand Down Expand Up @@ -72,6 +77,7 @@ public override async Task<int> InvokeAsync(InvocationContext context)
bool disableSSLValidation = context.ParseResult.GetValueForOption(DisableSSLValidationOption);
bool includeAdditionalData = context.ParseResult.GetValueForOption(AdditionalDataOption);
string className = context.ParseResult.GetValueForOption(ClassOption) ?? string.Empty;
AccessModifier typeAccessModifier = context.ParseResult.GetValueForOption(TypeAccessModifierOption);
string namespaceName = context.ParseResult.GetValueForOption(NamespaceOption) ?? string.Empty;
List<string> serializer = context.ParseResult.GetValueForOption(SerializerOption) ?? [];
List<string> deserializer = context.ParseResult.GetValueForOption(DeserializerOption) ?? [];
Expand All @@ -86,6 +92,7 @@ public override async Task<int> InvokeAsync(InvocationContext context)
AssignIfNotNullOrEmpty(manifest, (c, s) => c.ApiManifestPath = s);
AssignIfNotNullOrEmpty(className, (c, s) => c.ClientClassName = s);
AssignIfNotNullOrEmpty(namespaceName, (c, s) => c.ClientNamespaceName = s);
Configuration.Generation.TypeAccessModifier = typeAccessModifier;
Configuration.Generation.UsesBackingStore = backingStore;
Configuration.Generation.ExcludeBackwardCompatible = excludeBackwardCompatible;
Configuration.Generation.IncludeAdditionalData = includeAdditionalData;
Expand Down
6 changes: 6 additions & 0 deletions src/kiota/KiotaClientCommands.cs
Original file line number Diff line number Diff line change
Expand Up @@ -33,6 +33,7 @@ public static Command GetAddCommand()
{
var defaultConfiguration = new GenerationConfiguration();
var languageOption = KiotaHost.GetLanguageOption();
var typeAccessModifierOption = KiotaHost.GetTypeAccessModifierOption();
var outputOption = KiotaHost.GetOutputPathOption(defaultConfiguration.OutputPath);
var descriptionOption = KiotaHost.GetDescriptionOption(defaultConfiguration.OpenAPIFilePath, true);
var namespaceOption = KiotaHost.GetNamespaceOption(defaultConfiguration.ClientNamespaceName);
Expand All @@ -50,6 +51,7 @@ public static Command GetAddCommand()
descriptionOption,
outputOption,
languageOption,
typeAccessModifierOption,
clientNameOption,
namespaceOption,
logLevelOption,
Expand All @@ -67,6 +69,7 @@ public static Command GetAddCommand()
DescriptionOption = descriptionOption,
OutputOption = outputOption,
LanguageOption = languageOption,
TypeAccessModifierOption = typeAccessModifierOption,
ClassOption = clientNameOption,
NamespaceOption = namespaceOption,
LogLevelOption = logLevelOption,
Expand Down Expand Up @@ -104,6 +107,7 @@ public static Command GetRemoveCommand()
public static Command GetEditCommand()
{
var languageOption = KiotaHost.GetOptionalLanguageOption();
var typeAccessModifierOption = KiotaHost.GetOptionalTypeAccessModifierOption();
var outputOption = KiotaHost.GetOutputPathOption(string.Empty);
var descriptionOption = KiotaHost.GetDescriptionOption(string.Empty);
var namespaceOption = KiotaHost.GetNamespaceOption(string.Empty);
Expand All @@ -121,6 +125,7 @@ public static Command GetEditCommand()
descriptionOption,
outputOption,
languageOption,
typeAccessModifierOption,
clientNameOption,
namespaceOption,
logLevelOption,
Expand All @@ -138,6 +143,7 @@ public static Command GetEditCommand()
DescriptionOption = descriptionOption,
OutputOption = outputOption,
LanguageOption = languageOption,
TypeAccessModifierOption = typeAccessModifierOption,
ClassOption = clientNameOption,
NamespaceOption = namespaceOption,
LogLevelOption = logLevelOption,
Expand Down
Loading

0 comments on commit fac7928

Please sign in to comment.