Forbid declaring parameters of void type in 'Change Signature'->"Ad…

…d parameter" dialog
dibarbet · Aug 2, 2023 · 8cb1abb · 8cb1abb
1 parent b3dc95e
commit 8cb1abb
Show file tree

Hide file tree

Showing 21 changed files with 143 additions and 20 deletions.
diff --git a/src/VisualStudio/CSharp/Impl/ChangeSignature/CSharpChangeSignatureViewModelFactoryService.cs b/src/VisualStudio/CSharp/Impl/ChangeSignature/CSharpChangeSignatureViewModelFactoryService.cs
@@ -9,6 +9,7 @@
 using System.Composition;
 using Microsoft.CodeAnalysis;
 using Microsoft.CodeAnalysis.CSharp;
+using Microsoft.CodeAnalysis.CSharp.Syntax;
 using Microsoft.CodeAnalysis.Host.Mef;
 using Microsoft.VisualStudio.LanguageServices.Implementation.ChangeSignature;
 using static Microsoft.VisualStudio.LanguageServices.Implementation.ChangeSignature.ChangeSignatureDialogViewModel;
@@ -55,5 +56,7 @@ public override SymbolDisplayPart[] GeneratePreviewDisplayParts(AddedParameterVi
         public override bool IsTypeNameValid(string typeName) => !SyntaxFactory.ParseTypeName(typeName, options: s_langVersionLatestParseOptions).ContainsDiagnostics;
 
         public override SyntaxNode GetTypeNode(string typeName) => SyntaxFactory.ParseTypeName(typeName);
+
+        public override bool IsVoidTypeSyntax(SyntaxNode node) => node is PredefinedTypeSyntax { Keyword.RawKind: (int)SyntaxKind.VoidKeyword };
     }
 }
diff --git a/src/VisualStudio/Core/Def/ChangeSignature/AddParameterDialog.xaml b/src/VisualStudio/Core/Def/ChangeSignature/AddParameterDialog.xaml
@@ -49,7 +49,7 @@
                      AutomationProperties.LabeledBy="{Binding ElementName=TypeNameLabelName}"/>
                     <StackPanel Orientation="Horizontal" Grid.Row="2" Margin="0,5,0,5">
                         <imaging:CrispImage Moniker="{x:Static imagecatalog:KnownMonikers.StatusInformation}" Visibility="{Binding TypeIsEmptyImage, Mode=OneWay}"/>
-                        <imaging:CrispImage Moniker="{x:Static imagecatalog:KnownMonikers.StatusError}" Visibility="{Binding TypeDoesNotParseImage, Mode=OneWay}"/>
+                        <imaging:CrispImage Moniker="{x:Static imagecatalog:KnownMonikers.StatusError}" Visibility="{Binding TypeDoesNotParseOrInvalidTypeImage, Mode=OneWay}"/>
                         <imaging:CrispImage Moniker="{x:Static imagecatalog:KnownMonikers.StatusOK}" Visibility="{Binding TypeBindsImage, Mode=OneWay}"/>
                         <imaging:CrispImage Moniker="{x:Static imagecatalog:KnownMonikers.StatusWarning}" Visibility="{Binding TypeDoesNotBindImage, Mode=OneWay}"/>
                         <vs:LiveTextBlock Padding="5 0 0 0" Grid.Row="2" x:Name="TypeBindsTextBlock"

diff --git a/src/VisualStudio/Core/Def/ChangeSignature/AddParameterDialogViewModel.cs b/src/VisualStudio/Core/Def/ChangeSignature/AddParameterDialogViewModel.cs
@@ -30,7 +30,7 @@ public AddParameterDialogViewModel(Document document, int positionForTypeBinding
 
             TypeIsEmptyImage = Visibility.Visible;
             TypeBindsImage = Visibility.Collapsed;
-            TypeDoesNotParseImage = Visibility.Collapsed;
+            TypeDoesNotParseOrInvalidTypeImage = Visibility.Collapsed;
             TypeDoesNotBindImage = Visibility.Collapsed;
             TypeBindsDynamicStatus = ServicesVSResources.Please_enter_a_type_name;
 
@@ -104,7 +104,7 @@ public bool IsOptional
         public string TypeBindsDynamicStatus { get; set; }
         public Visibility TypeBindsImage { get; set; }
         public Visibility TypeDoesNotBindImage { get; set; }
-        public Visibility TypeDoesNotParseImage { get; set; }
+        public Visibility TypeDoesNotParseOrInvalidTypeImage { get; set; }
         public Visibility TypeIsEmptyImage { get; set; }
 
         private string _verbatimTypeName = string.Empty;
@@ -120,6 +120,8 @@ public string VerbatimTypeName
             }
         }
 
+        private bool _isVoidParameterType;
+
         internal bool CanSubmit([NotNullWhen(false)] out string? message)
         {
             if (string.IsNullOrEmpty(VerbatimTypeName) || string.IsNullOrEmpty(ParameterName))
@@ -128,6 +130,12 @@ internal bool CanSubmit([NotNullWhen(false)] out string? message)
                 return false;
             }
 
+            if (_isVoidParameterType)
+            {
+                message = ServicesVSResources.void_is_not_a_valid_type_for_a_parameter;
+                return false;
+            }
+
             if (TypeSymbol == null || !IsParameterTypeSyntacticallyValid(VerbatimTypeName))
             {
                 message = ServicesVSResources.Parameter_type_contains_invalid_characters;
@@ -175,11 +183,12 @@ private void SendFailureNotification(string message)
         private void SetCurrentTypeTextAndUpdateBindingStatus(string typeName)
         {
             VerbatimTypeName = typeName;
+            _isVoidParameterType = false;
 
             if (string.IsNullOrWhiteSpace(typeName))
             {
                 TypeIsEmptyImage = Visibility.Visible;
-                TypeDoesNotParseImage = Visibility.Collapsed;
+                TypeDoesNotParseOrInvalidTypeImage = Visibility.Collapsed;
                 TypeDoesNotBindImage = Visibility.Collapsed;
                 TypeBindsImage = Visibility.Collapsed;
                 TypeBindsDynamicStatus = ServicesVSResources.Please_enter_a_type_name;
@@ -191,20 +200,28 @@ private void SetCurrentTypeTextAndUpdateBindingStatus(string typeName)
                 TypeIsEmptyImage = Visibility.Collapsed;
 
                 var languageService = Document.GetRequiredLanguageService<IChangeSignatureViewModelFactoryService>();
-                TypeSymbol = _semanticModel.GetSpeculativeTypeInfo(PositionForTypeBinding, languageService.GetTypeNode(typeName), SpeculativeBindingOption.BindAsTypeOrNamespace).Type;
+                var typeNode = languageService.GetTypeNode(typeName);
+                TypeSymbol = _semanticModel.GetSpeculativeTypeInfo(PositionForTypeBinding, typeNode, SpeculativeBindingOption.BindAsTypeOrNamespace).Type;
 
-                var typeParses = IsParameterTypeSyntacticallyValid(typeName);
-                if (!typeParses || TypeSymbol == null)
+                if (languageService.IsVoidTypeSyntax(typeNode) || TypeSymbol is { SpecialType: SpecialType.System_Void })
+                {
+                    _isVoidParameterType = true;
+                    TypeDoesNotParseOrInvalidTypeImage = Visibility.Visible;
+                    TypeDoesNotBindImage = Visibility.Collapsed;
+                    TypeBindsImage = Visibility.Collapsed;
+                    TypeBindsDynamicStatus = ServicesVSResources.void_is_not_a_valid_type_for_a_parameter;
+                }
+                else if (!IsParameterTypeSyntacticallyValid(typeName) || TypeSymbol == null)
                 {
-                    TypeDoesNotParseImage = Visibility.Visible;
+                    TypeDoesNotParseOrInvalidTypeImage = Visibility.Visible;
                     TypeDoesNotBindImage = Visibility.Collapsed;
                     TypeBindsImage = Visibility.Collapsed;
                     TypeBindsDynamicStatus = ServicesVSResources.Type_name_has_a_syntax_error;
                 }
                 else
                 {
                     var parameterTypeBinds = DoesTypeFullyBind(TypeSymbol);
-                    TypeDoesNotParseImage = Visibility.Collapsed;
+                    TypeDoesNotParseOrInvalidTypeImage = Visibility.Collapsed;
 
                     TypeBindsImage = parameterTypeBinds ? Visibility.Visible : Visibility.Collapsed;
                     TypeDoesNotBindImage = !parameterTypeBinds ? Visibility.Visible : Visibility.Collapsed;
@@ -217,7 +234,7 @@ private void SetCurrentTypeTextAndUpdateBindingStatus(string typeName)
             NotifyPropertyChanged(nameof(TypeBindsDynamicStatus));
             NotifyPropertyChanged(nameof(TypeBindsImage));
             NotifyPropertyChanged(nameof(TypeDoesNotBindImage));
-            NotifyPropertyChanged(nameof(TypeDoesNotParseImage));
+            NotifyPropertyChanged(nameof(TypeDoesNotParseOrInvalidTypeImage));
             NotifyPropertyChanged(nameof(TypeIsEmptyImage));
         }
 

diff --git a/src/VisualStudio/Core/Def/ChangeSignature/ChangeSignatureViewModelFactoryService.cs b/src/VisualStudio/Core/Def/ChangeSignature/ChangeSignatureViewModelFactoryService.cs
@@ -18,5 +18,7 @@ public abstract SymbolDisplayPart[] GeneratePreviewDisplayParts(
         public abstract bool IsTypeNameValid(string typeName);
 
         public abstract SyntaxNode GetTypeNode(string typeName);
+
+        public abstract bool IsVoidTypeSyntax(SyntaxNode node);
     }
 }
diff --git a/src/VisualStudio/Core/Def/ChangeSignature/IChangeSignatureViewModelFactoryService.cs b/src/VisualStudio/Core/Def/ChangeSignature/IChangeSignatureViewModelFactoryService.cs
@@ -15,5 +15,7 @@ internal interface IChangeSignatureViewModelFactoryService : ILanguageService
         bool IsTypeNameValid(string typeName);
 
         SyntaxNode GetTypeNode(string typeName);
+
+        bool IsVoidTypeSyntax(SyntaxNode node);
     }
 }
diff --git a/src/VisualStudio/Core/Def/ServicesVSResources.resx b/src/VisualStudio/Core/Def/ServicesVSResources.resx
@@ -2025,4 +2025,8 @@ Additional information: {1}</value>
   <data name="Prefer_collection_expression" xml:space="preserve">
     <value>Prefer collection expression</value>
   </data>
+  <data name="void_is_not_a_valid_type_for_a_parameter" xml:space="preserve">
+    <value>'void' is not a valid type for a parameter</value>
+    <comment>{Locked="void"} "void" represents CLR's 'System.Void' type and should not be localized</comment>
+  </data>
 </root>
diff --git a/src/VisualStudio/Core/Def/xlf/ServicesVSResources.cs.xlf b/src/VisualStudio/Core/Def/xlf/ServicesVSResources.cs.xlf
diff --git a/src/VisualStudio/Core/Def/xlf/ServicesVSResources.de.xlf b/src/VisualStudio/Core/Def/xlf/ServicesVSResources.de.xlf
diff --git a/src/VisualStudio/Core/Def/xlf/ServicesVSResources.es.xlf b/src/VisualStudio/Core/Def/xlf/ServicesVSResources.es.xlf
diff --git a/src/VisualStudio/Core/Def/xlf/ServicesVSResources.fr.xlf b/src/VisualStudio/Core/Def/xlf/ServicesVSResources.fr.xlf
diff --git a/src/VisualStudio/Core/Def/xlf/ServicesVSResources.it.xlf b/src/VisualStudio/Core/Def/xlf/ServicesVSResources.it.xlf
diff --git a/src/VisualStudio/Core/Def/xlf/ServicesVSResources.ja.xlf b/src/VisualStudio/Core/Def/xlf/ServicesVSResources.ja.xlf
diff --git a/src/VisualStudio/Core/Def/xlf/ServicesVSResources.ko.xlf b/src/VisualStudio/Core/Def/xlf/ServicesVSResources.ko.xlf
diff --git a/src/VisualStudio/Core/Def/xlf/ServicesVSResources.pl.xlf b/src/VisualStudio/Core/Def/xlf/ServicesVSResources.pl.xlf
diff --git a/src/VisualStudio/Core/Def/xlf/ServicesVSResources.pt-BR.xlf b/src/VisualStudio/Core/Def/xlf/ServicesVSResources.pt-BR.xlf
diff --git a/src/VisualStudio/Core/Def/xlf/ServicesVSResources.ru.xlf b/src/VisualStudio/Core/Def/xlf/ServicesVSResources.ru.xlf
diff --git a/src/VisualStudio/Core/Def/xlf/ServicesVSResources.tr.xlf b/src/VisualStudio/Core/Def/xlf/ServicesVSResources.tr.xlf
diff --git a/src/VisualStudio/Core/Def/xlf/ServicesVSResources.zh-Hans.xlf b/src/VisualStudio/Core/Def/xlf/ServicesVSResources.zh-Hans.xlf
diff --git a/src/VisualStudio/Core/Def/xlf/ServicesVSResources.zh-Hant.xlf b/src/VisualStudio/Core/Def/xlf/ServicesVSResources.zh-Hant.xlf