From 4f6e574820fdba88b3a1f5b0f87ce88e60a400dd Mon Sep 17 00:00:00 2001 From: MartinGC94 <42123497+MartinGC94@users.noreply.github.com> Date: Mon, 13 Jun 2022 19:04:35 +0200 Subject: [PATCH] Improve type inference and completions (#16963) --- .../CommandCompletion/CompletionCompleters.cs | 32 ++++-- .../engine/parser/TypeInferenceVisitor.cs | 103 ++++++++++++------ .../engine/parser/ast.cs | 56 +++++++--- .../TabCompletion/TabCompletion.Tests.ps1 | 29 +++++ .../ConstrainedLanguageRestriction.Tests.ps1 | 10 +- .../engine/Api/TypeInference.Tests.ps1 | 18 +++ 6 files changed, 188 insertions(+), 60 deletions(-) diff --git a/src/System.Management.Automation/engine/CommandCompletion/CompletionCompleters.cs b/src/System.Management.Automation/engine/CommandCompletion/CompletionCompleters.cs index 5877cfefb..40b39b71b 100644 --- a/src/System.Management.Automation/engine/CommandCompletion/CompletionCompleters.cs +++ b/src/System.Management.Automation/engine/CommandCompletion/CompletionCompleters.cs @@ -2278,7 +2278,7 @@ namespace System.Management.Automation { if (parameterName.Equals("MemberName", StringComparison.OrdinalIgnoreCase)) { - NativeCompletionMemberName(context, result, commandAst, boundArguments?[parameterName]); + NativeCompletionMemberName(context, result, commandAst, boundArguments?[parameterName], propertiesOnly: false); } break; @@ -3951,7 +3951,7 @@ namespace System.Management.Automation return prevType; } - private static void NativeCompletionMemberName(CompletionContext context, List result, CommandAst commandAst, AstParameterArgumentPair parameterInfo) + private static void NativeCompletionMemberName(CompletionContext context, List result, CommandAst commandAst, AstParameterArgumentPair parameterInfo, bool propertiesOnly = true) { IEnumerable prevType = GetInferenceTypes(context, commandAst); if (prevType is not null) @@ -3962,7 +3962,8 @@ namespace System.Management.Automation excludedMembers = GetParameterValues(pair, context.CursorPosition.Offset); } - CompleteMemberByInferredType(context.TypeInferenceContext, prevType, result, context.WordToComplete + "*", filter: IsPropertyMember, isStatic: false, excludedMembers); + Func filter = propertiesOnly ? IsPropertyMember : null; + CompleteMemberByInferredType(context.TypeInferenceContext, prevType, result, context.WordToComplete + "*", filter, isStatic: false, excludedMembers); } result.Add(CompletionResult.Null); @@ -5816,6 +5817,11 @@ namespace System.Management.Automation inferredTypes = AstTypeInference.InferTypeOf(targetExpr, context.TypeInferenceContext, TypeInferenceRuntimePermissions.AllowSafeEval).ToArray(); } + if (!@static && inferredTypes.Length == 1 && inferredTypes[0].Name.Equals("System.Void", StringComparison.OrdinalIgnoreCase)) + { + return results; + } + if (inferredTypes != null && inferredTypes.Length > 0) { // Use inferred types if we have any @@ -6070,11 +6076,23 @@ namespace System.Management.Automation getToolTip = () => GetCimPropertyToString(cimProperty); } - var memberAst = member as MemberAst; - if (memberAst != null) + if (member is MemberAst memberAst) { - memberName = memberAst is CompilerGeneratedMemberFunctionAst ? "new" : memberAst.Name; - isMethod = memberAst is FunctionMemberAst || memberAst is CompilerGeneratedMemberFunctionAst; + if (memberAst is CompilerGeneratedMemberFunctionAst) + { + memberName = "new"; + isMethod = true; + } + else if (memberAst is FunctionMemberAst functionMember) + { + memberName = functionMember.IsConstructor ? "new" : functionMember.Name; + isMethod = true; + } + else + { + memberName = memberAst.Name; + isMethod = false; + } getToolTip = memberAst.GetTooltip; } diff --git a/src/System.Management.Automation/engine/parser/TypeInferenceVisitor.cs b/src/System.Management.Automation/engine/parser/TypeInferenceVisitor.cs index fa4a8a23a..363ce3536 100644 --- a/src/System.Management.Automation/engine/parser/TypeInferenceVisitor.cs +++ b/src/System.Management.Automation/engine/parser/TypeInferenceVisitor.cs @@ -298,7 +298,7 @@ namespace System.Management.Automation else { var functionMember = (FunctionMemberAst)member; - add = functionMember.IsStatic == isStatic; + add = (functionMember.IsConstructor && isStatic) || (!functionMember.IsConstructor && functionMember.IsStatic == isStatic); foundConstructor |= functionMember.IsConstructor; } @@ -638,6 +638,13 @@ namespace System.Management.Automation object ICustomAstVisitor.VisitBinaryExpression(BinaryExpressionAst binaryExpressionAst) { + // TODO: Handle other kinds of expressions on the right side. + if (binaryExpressionAst.Operator == TokenKind.As && binaryExpressionAst.Right is TypeExpressionAst typeExpression) + { + var type = typeExpression.TypeName.GetReflectionType(); + var psTypeName = type != null ? new PSTypeName(type) : new PSTypeName(typeExpression.TypeName.FullName); + return new[] { psTypeName }; + } return InferTypes(binaryExpressionAst.Left); } @@ -1897,39 +1904,15 @@ namespace System.Management.Automation return; } - foreach (var result in InferTypes(pipelineAst.PipelineElements[0])) - { - if (result.Type != null) - { - // Assume (because we're looking at $_ and we're inside a script block that is an - // argument to some command) that the type we're getting is actually unrolled. - // This might not be right in all cases, but with our simple analysis, it's - // right more often than it's wrong. - if (result.Type.IsArray) - { - inferredTypes.Add(new PSTypeName(result.Type.GetElementType())); - continue; - } + AddInferredTypesForDollarUnderbar(pipelineAst.PipelineElements[0], inferredTypes); - if (typeof(IEnumerable).IsAssignableFrom(result.Type)) - { - // We can't deduce much from IEnumerable, but we can if it's generic. - var enumerableInterfaces = result.Type.GetInterfaces(); - foreach (var t in enumerableInterfaces) - { - if (t.IsGenericType && t.GetGenericTypeDefinition() == typeof(IEnumerable<>)) - { - inferredTypes.Add(new PSTypeName(t.GetGenericArguments()[0])); - } - } - - continue; - } - } - - inferredTypes.Add(result); - } + return; + } + if (parent.Parent is InvokeMemberExpressionAst memberExpression) + { + AddInferredTypesForDollarUnderbar(memberExpression.Expression, inferredTypes); + return; } } @@ -1970,14 +1953,19 @@ namespace System.Management.Automation return; } + if (parent is null) + { + return; + } + // Look for our variable as a parameter or on the lhs of an assignment - hopefully we'll find either // a type constraint or at least we can use the rhs to infer the type. - while (parent?.Parent != null) + while (parent.Parent != null) { parent = parent.Parent; } - if (parent?.Parent is FunctionDefinitionAst) + if (parent.Parent is FunctionDefinitionAst) { parent = parent.Parent; } @@ -2068,6 +2056,42 @@ namespace System.Management.Automation } } + private void AddInferredTypesForDollarUnderbar(Ast parentExpression, List results) + { + foreach (var result in InferTypes(parentExpression)) + { + if (result.Type != null) + { + // Assume (because we're looking at $_ and we're inside a script block that is an + // argument to some command) that the type we're getting is actually unrolled. + // This might not be right in all cases, but with our simple analysis, it's + // right more often than it's wrong. + if (result.Type.IsArray) + { + results.Add(new PSTypeName(result.Type.GetElementType())); + continue; + } + + if (typeof(IEnumerable).IsAssignableFrom(result.Type)) + { + // We can't deduce much from IEnumerable, but we can if it's generic. + var enumerableInterfaces = result.Type.GetInterfaces(); + foreach (var t in enumerableInterfaces) + { + if (t.IsGenericType && t.GetGenericTypeDefinition() == typeof(IEnumerable<>)) + { + results.Add(new PSTypeName(t.GetGenericArguments()[0])); + } + } + + continue; + } + } + + results.Add(result); + } + } + /// /// Gets the most specific array type possible from a group of inferred types. /// @@ -2254,6 +2278,17 @@ namespace System.Management.Automation continue; } + + if (type.IsGenericType && type.GetGenericTypeDefinition() == typeof(IList<>)) + { + var valueType = type.GetGenericArguments()[0]; + if (!valueType.ContainsGenericParameters) + { + foundAny = true; + yield return new PSTypeName(valueType); + } + continue; + } foreach (var iface in type.GetInterfaces()) { diff --git a/src/System.Management.Automation/engine/parser/ast.cs b/src/System.Management.Automation/engine/parser/ast.cs index bb817ec3d..7c41559ad 100644 --- a/src/System.Management.Automation/engine/parser/ast.cs +++ b/src/System.Management.Automation/engine/parser/ast.cs @@ -3469,6 +3469,8 @@ namespace System.Management.Automation.Language internal IScriptExtent NameExtent { get { return _functionDefinitionAst.NameExtent; } } + private string _toolTip; + /// /// Copy a function member ast. /// @@ -3483,28 +3485,56 @@ namespace System.Management.Automation.Language internal override string GetTooltip() { - var sb = new StringBuilder(); - if (IsStatic) + if (!string.IsNullOrEmpty(_toolTip)) { - sb.Append("static "); + return _toolTip; } - sb.Append(IsReturnTypeVoid() ? "void" : ReturnType.TypeName.FullName); - sb.Append(' '); - sb.Append(Name); - sb.Append('('); - for (int i = 0; i < Parameters.Count; i++) + var sb = new StringBuilder(); + var classMembers = ((TypeDefinitionAst)Parent).Members; + for (int i = 0; i < classMembers.Count; i++) { - if (i > 0) + var methodMember = classMembers[i] as FunctionMemberAst; + if (methodMember is null || + !Name.Equals(methodMember.Name) || + IsStatic != methodMember.IsStatic) { - sb.Append(", "); + continue; } - sb.Append(Parameters[i].GetTooltip()); + if (sb.Length > 0) + { + sb.AppendLine(); + } + + if (methodMember.IsStatic) + { + sb.Append("static "); + } + + if (!methodMember.IsConstructor) + { + sb.Append(methodMember.IsReturnTypeVoid() ? "void" : methodMember.ReturnType.TypeName.FullName); + sb.Append(' '); + } + + sb.Append(methodMember.Name); + sb.Append('('); + for (int j = 0; j < methodMember.Parameters.Count; j++) + { + if (j > 0) + { + sb.Append(", "); + } + + sb.Append(methodMember.Parameters[j].GetTooltip()); + } + + sb.Append(')'); } - sb.Append(')'); - return sb.ToString(); + _toolTip = sb.ToString(); + return _toolTip; } #region Visitors diff --git a/test/powershell/Host/TabCompletion/TabCompletion.Tests.ps1 b/test/powershell/Host/TabCompletion/TabCompletion.Tests.ps1 index 7f2270941..e8b2b60b9 100644 --- a/test/powershell/Host/TabCompletion/TabCompletion.Tests.ps1 +++ b/test/powershell/Host/TabCompletion/TabCompletion.Tests.ps1 @@ -454,14 +454,43 @@ switch ($x) $actual | Should -BeExactly $expected } + It 'ForEach-Object member completion results should include methods' { + $res = TabExpansion2 -inputScript '1..10 | ForEach-Object -MemberName ' + $res.CompletionMatches.CompletionText | Should -Contain "GetType(" + } + + It 'Should not complete void instance members' { + $res = TabExpansion2 -inputScript '([void]("")).' + $res.CompletionMatches | Should -BeNullOrEmpty + } + + It 'Should complete custom constructor from class using the AST' { + $res = TabExpansion2 -inputScript 'class ConstructorTestClass{ConstructorTestClass ([string] $s){}};[ConstructorTestClass]::' + $res.CompletionMatches | Should -HaveCount 3 + $completionText = $res.CompletionMatches.CompletionText | Sort-Object + $completionText -join ' ' | Should -BeExactly 'Equals( new( ReferenceEquals(' + } + + It 'Should show multiple constructors in the tooltip' { + $res = TabExpansion2 -inputScript 'class ConstructorTestClass{ConstructorTestClass ([string] $s){}ConstructorTestClass ([int] $i){}ConstructorTestClass ([int] $i, [bool]$b){}};[ConstructorTestClass]::new' + $res.CompletionMatches | Should -HaveCount 1 + $completionText = $res.CompletionMatches.ToolTip | Should -BeExactly @' +ConstructorTestClass(string s) +ConstructorTestClass(int i) +ConstructorTestClass(int i, bool b) +'@ + } + It 'Should complete parameter in param block' { $res = TabExpansion2 -inputScript 'Param($Param1=(Get-ChildItem -))' -cursorColumn 30 $res.CompletionMatches[0].CompletionText | Should -BeExactly '-Path' } + It 'Should complete member in param block' { $res = TabExpansion2 -inputScript 'Param($Param1=($PSVersionTable.))' -cursorColumn 31 $res.CompletionMatches[0].CompletionText | Should -BeExactly 'Count' } + It 'Should complete attribute argument in param block' { $res = TabExpansion2 -inputScript 'Param([Parameter()]$Param1)' -cursorColumn 17 $res.CompletionMatches[0].CompletionText | Should -BeExactly 'Position' diff --git a/test/powershell/Modules/Microsoft.PowerShell.Security/ConstrainedLanguageRestriction.Tests.ps1 b/test/powershell/Modules/Microsoft.PowerShell.Security/ConstrainedLanguageRestriction.Tests.ps1 index e9397f86e..72faa923f 100644 --- a/test/powershell/Modules/Microsoft.PowerShell.Security/ConstrainedLanguageRestriction.Tests.ps1 +++ b/test/powershell/Modules/Microsoft.PowerShell.Security/ConstrainedLanguageRestriction.Tests.ps1 @@ -554,22 +554,20 @@ try } } - Describe "Tab expansion in constrained language mode" -Tags 'Feature','RequireAdminOnWindows' { + Describe "Conversion in constrained language mode" -Tags 'Feature','RequireAdminOnWindows' { - It "Verifies that tab expansion cannot convert disallowed IntPtr type" { + It "Verifies that PowerShell cannot convert disallowed IntPtr type" { try { $ExecutionContext.SessionState.LanguageMode = "ConstrainedLanguage" - - $result = @(TabExpansion2 '(1234 -as [IntPtr]).' 20 | ForEach-Object CompletionMatches | Where-Object CompletionText -Match Pointer) + $result = 1234 -as [IntPtr] + $null -eq $result | Should -BeTrue } finally { Invoke-LanguageModeTestingSupportCmdlet -EnableFullLanguageMode } - - $result.Count | Should -Be 0 } } diff --git a/test/powershell/engine/Api/TypeInference.Tests.ps1 b/test/powershell/engine/Api/TypeInference.Tests.ps1 index a98b04d15..2d86b9153 100644 --- a/test/powershell/engine/Api/TypeInference.Tests.ps1 +++ b/test/powershell/engine/Api/TypeInference.Tests.ps1 @@ -1228,6 +1228,24 @@ Describe "Type inference Tests" -tags "CI" { $res.Count | Should -Be BaseType } + It 'Infers type of "as" operator statement' { + $res = [AstTypeInference]::InferTypeOf( { $null -as [int] }.Ast) + $res.Count | Should -Be 1 + $res.Name | Should -Be 'System.Int32' + } + + It 'Infers type of $_ in a method scriptblock' { + $res = [AstTypeInference]::InferTypeOf( { ("","").ForEach({$_}) }.Ast.Find({param($ast) $ast -is [System.Management.Automation.Language.VariableExpressionAst]}, $true)) + $res.Count | Should -Be 1 + $res.Name | Should -Be 'System.String' + } + + It 'Infers type of index item in an IList' { + $res = [AstTypeInference]::InferTypeOf( { ([System.Collections.Generic.IList[System.Text.StringBuilder]]$null)[0] }.Ast) + $res.Count | Should -Be 1 + $res.Name | Should -Be 'System.Text.StringBuilder' + } + It 'Infers type of generic method invocation with type parameters' { $res = [AstTypeInference]::InferTypeOf( { [array]::Empty[int]() }.Ast) $res.Count | Should -Be 1