From ef64132f0a6e8d0e05ae8dc23753931caa89be7f Mon Sep 17 00:00:00 2001 From: Dongbo Wang Date: Mon, 17 Mar 2025 15:40:25 -0700 Subject: [PATCH] Fix `TypeName.GetReflectionType()` to work when the `TypeName` instance represents a generic type definition within a `GenericTypeName` (#24985) --- .../engine/parser/Parser.cs | 2 +- .../engine/parser/ast.cs | 72 ++++++++++++++++--- .../Language/Parser/Parsing.Tests.ps1 | 47 ++++++++++++ 3 files changed, 109 insertions(+), 12 deletions(-) diff --git a/src/System.Management.Automation/engine/parser/Parser.cs b/src/System.Management.Automation/engine/parser/Parser.cs index 1a7b1adaca..1592d2e7e7 100644 --- a/src/System.Management.Automation/engine/parser/Parser.cs +++ b/src/System.Management.Automation/engine/parser/Parser.cs @@ -1487,7 +1487,7 @@ namespace System.Management.Automation.Language rBracketToken = null; } - var openGenericType = new TypeName(genericTypeName.Extent, genericTypeName.Text); + var openGenericType = new TypeName(genericTypeName.Extent, genericTypeName.Text, genericArguments.Count); var result = new GenericTypeName( ExtentOf(genericTypeName.Extent, ExtentFromFirstOf(rBracketToken, genericArguments.LastOrDefault(), firstToken)), openGenericType, diff --git a/src/System.Management.Automation/engine/parser/ast.cs b/src/System.Management.Automation/engine/parser/ast.cs index c9c54bcd4b..0325cf94ae 100644 --- a/src/System.Management.Automation/engine/parser/ast.cs +++ b/src/System.Management.Automation/engine/parser/ast.cs @@ -7263,7 +7263,7 @@ namespace System.Management.Automation.Language FunctionName.Extent, new TypeName( FunctionName.Extent, - typeof(System.Management.Automation.Language.DynamicKeyword).FullName)), + typeof(DynamicKeyword).FullName)), new StringConstantExpressionAst( FunctionName.Extent, "GetKeyword", @@ -8424,9 +8424,11 @@ namespace System.Management.Automation.Language /// public sealed class TypeName : ITypeName, ISupportsTypeCaching { - internal readonly string _name; - internal Type _type; - internal readonly IScriptExtent _extent; + private readonly string _name; + private readonly IScriptExtent _extent; + private readonly int _genericArgumentCount; + private Type _type; + internal TypeDefinitionAst _typeDefinitionAst; /// @@ -8485,6 +8487,23 @@ namespace System.Management.Automation.Language AssemblyName = assembly; } + /// + /// Construct a typename that represents a generic type definition. + /// + /// The extent of the typename. + /// The name of the type. + /// The number of generic arguments. + internal TypeName(IScriptExtent extent, string name, int genericArgumentCount) + : this(extent, name) + { + ArgumentOutOfRangeException.ThrowIfLessThan(genericArgumentCount, 0); + + if (genericArgumentCount > 0 && !_name.Contains('`')) + { + _genericArgumentCount = genericArgumentCount; + } + } + /// /// Returns the full name of the type. /// @@ -8561,8 +8580,25 @@ namespace System.Management.Automation.Language { if (_type == null) { - Exception e; - Type type = _typeDefinitionAst != null ? _typeDefinitionAst.Type : TypeResolver.ResolveTypeName(this, out e); + Type type = _typeDefinitionAst != null ? _typeDefinitionAst.Type : TypeResolver.ResolveTypeName(this, out _); + + if (type is null && _genericArgumentCount > 0) + { + // We try an alternate name only if it failed to resolve with the original name. + // This is because for a generic type like `System.Tuple`, the original name `System.Tuple` + // can be resolved and hence `genericTypeName.TypeName.GetReflectionType()` in that case has always been + // returning the type `System.Tuple`. If we change to directly use the alternate name for resolution, the + // return value will become 'System.Tuple`1' in that case, and that's a breaking change. + TypeName newTypeName = new( + _extent, + string.Create(CultureInfo.InvariantCulture, $"{_name}`{_genericArgumentCount}")) + { + AssemblyName = AssemblyName + }; + + type = TypeResolver.ResolveTypeName(newTypeName, out _); + } + if (type != null) { try @@ -8597,7 +8633,11 @@ namespace System.Management.Automation.Language var result = GetReflectionType(); if (result == null || !typeof(Attribute).IsAssignableFrom(result)) { - var attrTypeName = new TypeName(_extent, FullName + "Attribute"); + TypeName attrTypeName = new(_extent, $"{_name}Attribute", _genericArgumentCount) + { + AssemblyName = AssemblyName + }; + result = attrTypeName.GetReflectionType(); if (result != null && !typeof(Attribute).IsAssignableFrom(result)) { @@ -8890,8 +8930,13 @@ namespace System.Management.Automation.Language { if (!TypeName.FullName.Contains('`')) { - var newTypeName = new TypeName(Extent, - string.Create(CultureInfo.InvariantCulture, $"{TypeName.FullName}`{GenericArguments.Count}")); + TypeName newTypeName = new( + Extent, + string.Create(CultureInfo.InvariantCulture, $"{TypeName.Name}`{GenericArguments.Count}")) + { + AssemblyName = TypeName.AssemblyName + }; + generic = newTypeName.GetReflectionType(); } } @@ -8917,8 +8962,13 @@ namespace System.Management.Automation.Language { if (!TypeName.FullName.Contains('`')) { - var newTypeName = new TypeName(Extent, - string.Create(CultureInfo.InvariantCulture, $"{TypeName.FullName}Attribute`{GenericArguments.Count}")); + TypeName newTypeName = new( + Extent, + string.Create(CultureInfo.InvariantCulture, $"{TypeName.Name}Attribute`{GenericArguments.Count}")) + { + AssemblyName = TypeName.AssemblyName + }; + generic = newTypeName.GetReflectionType(); } } diff --git a/test/powershell/Language/Parser/Parsing.Tests.ps1 b/test/powershell/Language/Parser/Parsing.Tests.ps1 index d7bc39b077..b85ef72c43 100644 --- a/test/powershell/Language/Parser/Parsing.Tests.ps1 +++ b/test/powershell/Language/Parser/Parsing.Tests.ps1 @@ -683,6 +683,53 @@ Describe "Additional tests" -Tag CI { $result.EndBlock.Statements[0].PipelineElements[0].Expression.TypeName.FullName | Should -Be 'System.Tuple[System.String[],System.Int32[]]' } + It "Should correctly set the cached type for 'GenericTypeName.TypeName' as needed when the generic type is found in cache" { + $tks = $null + $ers = $null + $Script = '[System.Collections.Generic.List[string]]' + + ## See https://github.com/PowerShell/PowerShell/issues/24982 for details about the issue. + $result = [System.Management.Automation.Language.Parser]::ParseInput($Script, [ref]$tks, [ref]$ers) + $typeExpr = $result.EndBlock.Statements[0].PipelineElements[0].Expression + $typeExpr.TypeName.FullName | Should -Be 'System.Collections.Generic.List[string]' + $typeExpr.TypeName.TypeName.FullName | Should -Be 'System.Collections.Generic.List' + $typeExpr.TypeName.TypeName.GetReflectionType() | Should -Not -BeNullOrEmpty + $typeExpr.TypeName.TypeName.GetReflectionType().FullName | Should -Be 'System.Collections.Generic.List`1' + + $result2 = [System.Management.Automation.Language.Parser]::ParseInput($Script, [ref]$tks, [ref]$ers) + $typeExpr2 = $result2.EndBlock.Statements[0].PipelineElements[0].Expression + $typeExpr2.TypeName.FullName | Should -Be 'System.Collections.Generic.List[string]' + $typeExpr2.TypeName.TypeName.FullName | Should -Be 'System.Collections.Generic.List' + $typeExpr2.TypeName.TypeName.GetReflectionType() | Should -Not -BeNullOrEmpty + $typeExpr2.TypeName.TypeName.GetReflectionType().FullName | Should -Be 'System.Collections.Generic.List`1' + + $Script = '[System.Tuple[System.String[],System.Int32[]]]' + $result3 = [System.Management.Automation.Language.Parser]::ParseInput($Script, [ref]$tks, [ref]$ers) + $result3.EndBlock.Statements[0].PipelineElements[0].Expression.TypeName.TypeName.GetReflectionType().FullName | Should -Be 'System.Tuple' + + ## Generic type with assembly name can be resolved. + [System.Collections.Generic.List[string], System.Private.CoreLib].FullName | Should -BeLike 'System.Collections.Generic.List``1`[`[System.String, *`]`]' + + $Script = '[System.Collections.Generic.List[string], System.Private.CoreLib]' + $result4 = [System.Management.Automation.Language.Parser]::ParseInput($Script, [ref]$tks, [ref]$ers) + $typeExpr4 = $result4.EndBlock.Statements[0].PipelineElements[0].Expression + $typeExpr4.TypeName.FullName | Should -Be 'System.Collections.Generic.List[string],System.Private.CoreLib' + $typeExpr4.TypeName.TypeName.FullName | Should -Be 'System.Collections.Generic.List,System.Private.CoreLib' + $typeExpr4.TypeName.TypeName.GetReflectionType() | Should -Not -BeNullOrEmpty + $typeExpr4.TypeName.TypeName.GetReflectionType().FullName | Should -Be 'System.Collections.Generic.List`1' + + ## Generic type with '`' in name can be resolved. + [System.Collections.Generic.List`1[string]].FullName | Should -BeLike 'System.Collections.Generic.List``1`[`[System.String, *`]`]' + + $Script = '[System.Collections.Generic.List`1[string]]' + $result5 = [System.Management.Automation.Language.Parser]::ParseInput($Script, [ref]$tks, [ref]$ers) + $typeExpr5 = $result5.EndBlock.Statements[0].PipelineElements[0].Expression + $typeExpr5.TypeName.FullName | Should -Be 'System.Collections.Generic.List`1[string]' + $typeExpr5.TypeName.TypeName.FullName | Should -Be 'System.Collections.Generic.List`1' + $typeExpr5.TypeName.TypeName.GetReflectionType() | Should -Not -BeNullOrEmpty + $typeExpr5.TypeName.TypeName.GetReflectionType().FullName | Should -Be 'System.Collections.Generic.List`1' + } + It "Should get correct offsets for number constant parsing error" { $tks = $null $ers = $null