Uh oh!
There was an error while loading. Please reload this page.
- Notifications
You must be signed in to change notification settings - Fork 5.6k
Address feedback from config binding gen PR to improve enum parsing#89952
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
Merged
Uh oh!
There was an error while loading. Please reload this page.
Merged
Changes from all commits
Commits
Show all changes
22 commits
Select commit
Hold shift + click to select a range
78f22c7
Address feedback from config binding gen PR to improve enum parsing
pedrobsaila 135be17
drop preprocessors and generate ArgumentNullException.ThrowIfNull whe…
pedrobsaila e80c9ae
fix merge issues
pedrobsaila f5a2abc
fix remarks
pedrobsaila 31f19a3
merge from main
pedrobsaila 75e001c
fix merge issues
pedrobsaila d5d319e
Merge remote-tracking branch 'upstream/main' into 89936
pedrobsaila 497a900
delete useless references
pedrobsaila 1ce13fe
run tests for net462
pedrobsaila 4e18e33
revert
pedrobsaila 2ccc919
fix remak about ArgumentNullException
pedrobsaila d3c36fe
fix rollback
pedrobsaila 9576f12
fix revert 2
pedrobsaila f9c193e
merge from main
pedrobsaila 8e8de8d
fix merge issues
pedrobsaila ce80449
fix merge issues 2
pedrobsaila 3c91359
merge from main
pedrobsaila 080fca3
fix merge conflict
pedrobsaila 686082a
Merge remote-tracking branch 'upstream/main' into 89936
pedrobsaila 576e784
fix merge issues
pedrobsaila 438b371
fix remarks
pedrobsaila a08f4cd
fix remark on baseline files path
pedrobsaila File filter
Filter by extension
Conversations
Failed to load comments.
Loading
Uh oh!
There was an error while loading. Please reload this page.
Jump to
Jump to file
Failed to load files.
Loading
Uh oh!
There was an error while loading. Please reload this page.
Diff view
Diff view
There are no files selected for viewing
6 changes: 6 additions & 0 deletions
6 ...es/Microsoft.Extensions.Configuration.Binder/gen/ConfigurationBindingGenerator.Emitter.cs
This file contains hidden or bidirectional Unicode text that may be interpreted or compiled differently than what appears below. To review, open the file in an editor that reveals hidden Unicode characters.
Learn more about bidirectional Unicode characters
45 changes: 45 additions & 0 deletions
45 ...ies/Microsoft.Extensions.Configuration.Binder/gen/ConfigurationBindingGenerator.Parser.cs
This file contains hidden or bidirectional Unicode text that may be interpreted or compiled differently than what appears below. To review, open the file in an editor that reveals hidden Unicode characters.
Learn more about bidirectional Unicode characters
| Original file line number | Diff line number | Diff line change |
|---|---|---|
| @@ -26,6 +26,8 @@ internal sealed partial class Parser(CompilationData compilationData) | ||
| private readonly InterceptorInfo.Builder _interceptorInfoBuilder = new(); | ||
| private BindingHelperInfo.Builder? _helperInfoBuilder; // Init'ed with type index when registering interceptors, after creating type specs. | ||
| private bool _emitEnumParseMethod; | ||
| private bool _emitGenericParseEnum; | ||
| public List<DiagnosticInfo>? Diagnostics { get; private set; } | ||
| @@ -45,12 +47,16 @@ internal sealed partial class Parser(CompilationData compilationData) | ||
| ParseInvocations(invocations); | ||
| CreateTypeSpecs(cancellationToken); | ||
| RegisterInterceptors(); | ||
| CheckIfToEmitParseEnumMethod(); | ||
| return new SourceGenerationSpec | ||
| { | ||
| InterceptorInfo = _interceptorInfoBuilder.ToIncrementalValue(), | ||
| BindingHelperInfo = _helperInfoBuilder!.ToIncrementalValue(), | ||
| ConfigTypes = _createdTypeSpecs.Values.OrderBy(s => s.TypeRef.FullyQualifiedName).ToImmutableEquatableArray(), | ||
| EmitEnumParseMethod = _emitEnumParseMethod, | ||
| EmitGenericParseEnum = _emitGenericParseEnum, | ||
| EmitThrowIfNullMethod = IsThrowIfNullMethodToBeEmitted() | ||
| }; | ||
| } | ||
| @@ -842,6 +848,45 @@ private void RecordDiagnostic(DiagnosticDescriptor descriptor, Location trimmedL | ||
| Diagnostics ??= new List<DiagnosticInfo>(); | ||
| Diagnostics.Add(DiagnosticInfo.Create(descriptor, trimmedLocation, messageArgs)); | ||
| } | ||
| private void CheckIfToEmitParseEnumMethod() | ||
| { | ||
| foreach (var typeSymbol in _createdTypeSpecs.Keys) | ||
| { | ||
| if (IsEnum(typeSymbol)) | ||
| { | ||
| _emitEnumParseMethod = true; | ||
| _emitGenericParseEnum = _typeSymbols.Enum.GetMembers("Parse").Any(m => m is IMethodSymbol methodSymbol && methodSymbol.IsGenericMethod); | ||
| return; | ||
| } | ||
| } | ||
| } | ||
| private bool IsThrowIfNullMethodToBeEmitted() | ||
| { | ||
| if (_typeSymbols.ArgumentNullException is not null) | ||
| { | ||
| var throwIfNullMethods = _typeSymbols.ArgumentNullException.GetMembers("ThrowIfNull"); | ||
tarekgh marked this conversation as resolved.
Uh oh!There was an error while loading. Please reload this page. | ||
| foreach (var throwIfNullMethod in throwIfNullMethods) | ||
| { | ||
| if (throwIfNullMethod is IMethodSymbol throwIfNullMethodSymbol && throwIfNullMethodSymbol.IsStatic && throwIfNullMethodSymbol.Parameters.Length == 2) | ||
| { | ||
| var parameters = throwIfNullMethodSymbol.Parameters; | ||
| var firstParam = parameters[0]; | ||
| var secondParam = parameters[1]; | ||
| if (firstParam.Name == "argument" && firstParam.Type.SpecialType == SpecialType.System_Object | ||
| && secondParam.Name == "paramName" && secondParam.Type.Equals(_typeSymbols.String, SymbolEqualityComparer.Default)) | ||
| { | ||
| return true; | ||
| } | ||
| } | ||
| } | ||
| } | ||
| return false; | ||
| } | ||
| } | ||
| } | ||
| } | ||
4 changes: 2 additions & 2 deletions
4 src/libraries/Microsoft.Extensions.Configuration.Binder/gen/Emitter/ConfigurationBinder.cs
This file contains hidden or bidirectional Unicode text that may be interpreted or compiled differently than what appears below. To review, open the file in an editor that reveals hidden Unicode characters.
Learn more about bidirectional Unicode characters
37 changes: 14 additions & 23 deletions
37 src/libraries/Microsoft.Extensions.Configuration.Binder/gen/Emitter/CoreBindingHelpers.cs
This file contains hidden or bidirectional Unicode text that may be interpreted or compiled differently than what appears below. To review, open the file in an editor that reveals hidden Unicode characters.
Learn more about bidirectional Unicode characters
26 changes: 19 additions & 7 deletions
26 src/libraries/Microsoft.Extensions.Configuration.Binder/gen/Emitter/Helpers.cs
This file contains hidden or bidirectional Unicode text that may be interpreted or compiled differently than what appears below. To review, open the file in an editor that reveals hidden Unicode characters.
Learn more about bidirectional Unicode characters
| Original file line number | Diff line number | Diff line change |
|---|---|---|
| @@ -229,18 +229,30 @@ private void EmitBlankLineIfRequired() | ||
| _emitBlankLineBeforeNextStatement = true; | ||
| } | ||
| private void EmitCheckForNullArgument_WithBlankLine(string paramName, bool voidReturn = false) | ||
| private void EmitCheckForNullArgument_WithBlankLine(string paramName, bool useThrowIfNullMethod, bool voidReturn = false) | ||
| { | ||
| string returnExpr = voidReturn | ||
| ? "return" | ||
| : $"throw new ArgumentNullException(nameof({paramName}))"; | ||
| _writer.WriteLine($$""" | ||
| if (voidReturn) | ||
| { | ||
| _writer.WriteLine($$""" | ||
| if ({{paramName}} is null) | ||
| { | ||
| {{returnExpr}}; | ||
| return; | ||
| } | ||
| """); | ||
| } | ||
| else | ||
| { | ||
| string throwIfNullExpr = useThrowIfNullMethod | ||
| ? $"ArgumentNullException.ThrowIfNull({paramName});" | ||
tarekgh marked this conversation as resolved.
Uh oh!There was an error while loading. Please reload this page. | ||
| : $$""" | ||
| if ({{paramName}} is null) | ||
| { | ||
| throw new ArgumentNullException(nameof({{paramName}})); | ||
| } | ||
| """; | ||
| _writer.WriteLine(throwIfNullExpr); | ||
| } | ||
| _writer.WriteLine(); | ||
| } | ||
8 changes: 4 additions & 4 deletions
8 ...soft.Extensions.Configuration.Binder/gen/Emitter/OptionsBuilderConfigurationExtensions.cs
This file contains hidden or bidirectional Unicode text that may be interpreted or compiled differently than what appears below. To review, open the file in an editor that reveals hidden Unicode characters.
Learn more about bidirectional Unicode characters
4 changes: 2 additions & 2 deletions
4 ...sions.Configuration.Binder/gen/Emitter/OptionsConfigurationServiceCollectionExtensions.cs
This file contains hidden or bidirectional Unicode text that may be interpreted or compiled differently than what appears below. To review, open the file in an editor that reveals hidden Unicode characters.
Learn more about bidirectional Unicode characters
8 changes: 8 additions & 0 deletions
8 src/libraries/Microsoft.Extensions.Configuration.Binder/gen/Parser/KnownTypeSymbols.cs
This file contains hidden or bidirectional Unicode text that may be interpreted or compiled differently than what appears below. To review, open the file in an editor that reveals hidden Unicode characters.
Learn more about bidirectional Unicode characters
3 changes: 3 additions & 0 deletions
3 src/libraries/Microsoft.Extensions.Configuration.Binder/gen/Specs/SourceGenerationSpec.cs
This file contains hidden or bidirectional Unicode text that may be interpreted or compiled differently than what appears below. To review, open the file in an editor that reveals hidden Unicode characters.
Learn more about bidirectional Unicode characters
File renamed without changes.
File renamed without changes.
File renamed without changes.
File renamed without changes.
File renamed without changes.
File renamed without changes.
File renamed without changes.
File renamed without changes.
File renamed without changes.
File renamed without changes.
File renamed without changes.
File renamed without changes.
File renamed without changes.
File renamed without changes.
File renamed without changes.
File renamed without changes.
File renamed without changes.
File renamed without changes.
File renamed without changes.
File renamed without changes.
File renamed without changes.
Oops, something went wrong.
Uh oh!
There was an error while loading. Please reload this page.
Add this suggestion to a batch that can be applied as a single commit.This suggestion is invalid because no changes were made to the code.Suggestions cannot be applied while the pull request is closed.Suggestions cannot be applied while viewing a subset of changes.Only one suggestion per line can be applied in a batch.Add this suggestion to a batch that can be applied as a single commit.Applying suggestions on deleted lines is not supported.You must change the existing code in this line in order to create a valid suggestion.Outdated suggestions cannot be applied.This suggestion has been applied or marked resolved.Suggestions cannot be applied from pending reviews.Suggestions cannot be applied on multi-line comments.Suggestions cannot be applied while the pull request is queued to merge.Suggestion cannot be applied right now. Please check back later.
Uh oh!
There was an error while loading. Please reload this page.