Merge pull request #5144 from jmarolf/RedundantPrivateCodeFixProvider

Harden against null arguments in simplifier Fixes #5121
dotnet · Sep 11, 2015 · 4176470 · 4176470
2 parents e4a7be0 + 1eeffcf
commit 4176470
Show file tree

Hide file tree

Showing 5 changed files with 240 additions and 1 deletion.
diff --git a/docs/Breaking API Changes.md b/docs/Breaking API Changes.md
@@ -10,3 +10,8 @@ It was never the intent for these members to be a part of the supported API surf
 
 PR: https://github.com/dotnet/roslyn/pull/4169
 
+### Changed Simplifier methods to throw ArgumentNullExceptions 
+Changed Simplifier.ReduceAsync, Simplifier.ExpandAsync, and Simplifier.Expand methods to throw ArgumentNullExceptions if any non-optional, nullable arguments are passed in.  Previously the user would get a NullReferenceException for synchronous methods and an AggregateException containing a NullReferenceException for asynchronous methods.
+
+PR: https://github.com/dotnet/roslyn/pull/5144
+
diff --git a/src/EditorFeatures/Test2/EditorServicesTest2.vbproj b/src/EditorFeatures/Test2/EditorServicesTest2.vbproj
@@ -175,6 +175,7 @@
     <Compile Include="Diagnostics\InMemoryAssemblyLoader.vb" />
     <Compile Include="Diagnostics\UseAutoProperty\UseAutoPropertyTests.vb" />
     <Compile Include="LanguageServices\SyntaxFactsServiceTests.vb" />
+    <Compile Include="Simplification\SimplifierAPITests.vb" />
     <Compile Include="Utilities\MockDocumentNavigationServiceProvider.vb" />
     <Compile Include="Utilities\MockSymbolNavigationServiceProvider.vb" />
     <Compile Include="CodeFixes\CodeFixServiceTests.vb" />

diff --git a/src/EditorFeatures/Test2/Simplification/SimplifierAPITests.vb b/src/EditorFeatures/Test2/Simplification/SimplifierAPITests.vb
@@ -0,0 +1,163 @@
+Imports Microsoft.CodeAnalysis
+Imports Microsoft.CodeAnalysis.MSBuild.MSBuildWorkspace
+Imports Microsoft.CodeAnalysis.Simplification.Simplifier
+Imports Microsoft.CodeAnalysis.Text
+Imports Microsoft.CodeAnalysis.UnitTests
+Imports Microsoft.CodeAnalysis.VisualBasic.SyntaxFactory
+Imports Xunit.Assert
+
+Public Class SimplifierAPITests
+    Inherits WorkspaceTestBase
+
+    <Fact, Trait(Traits.Feature, Traits.Features.Formatting)>
+    Public Sub TestExpandAsync()
+        AssertThrows(Of ArgumentNullException)(
+            Sub()
+                Dim expandedNode = ExpandAsync(Of SyntaxNode)(Nothing, Nothing).Result
+            End Sub,
+            Sub(exception) Equal(exception.ParamName, "node"))
+    End Sub
+
+    <Fact, Trait(Traits.Feature, Traits.Features.Formatting)>
+    Public Sub TestExpandAsync2()
+        Dim node = GetSyntaxNode()
+        AssertThrows(Of ArgumentNullException)(
+            Sub()
+                Dim expandedNode = ExpandAsync(node, Nothing).Result
+            End Sub,
+            Sub(exception) Equal(exception.ParamName, "document"))
+    End Sub
+
+    <Fact, Trait(Traits.Feature, Traits.Features.Formatting)>
+    Public Sub TestExpand()
+        AssertThrows(Of ArgumentNullException)(
+            Sub()
+                Dim expandedNode = Expand(Of SyntaxNode)(Nothing, Nothing, Nothing)
+            End Sub,
+            Sub(exception) Equal(exception.ParamName, "node"))
+    End Sub
+
+    <Fact, Trait(Traits.Feature, Traits.Features.Formatting)>
+    Public Sub TestExpand2()
+        Dim node = GetSyntaxNode()
+        AssertThrows(Of ArgumentNullException)(
+            Sub()
+                Dim expandedNode = Expand(node, Nothing, Nothing)
+            End Sub,
+            Sub(exception) Equal(exception.ParamName, "semanticModel"))
+    End Sub
+
+    <Fact, Trait(Traits.Feature, Traits.Features.Formatting)>
+    Public Sub TestExpand3()
+        Dim node = GetSyntaxNode()
+        Dim semanticModel = GetSemanticModel()
+        AssertThrows(Of ArgumentNullException)(
+            Sub()
+                Dim expandedNode = Expand(node, semanticModel, Nothing)
+            End Sub,
+            Sub(exception) Equal(exception.ParamName, "workspace"))
+    End Sub
+
+    <Fact, Trait(Traits.Feature, Traits.Features.Formatting)>
+    Public Sub TestTokenExpandAsync()
+        AssertThrows(Of ArgumentNullException)(
+            Sub()
+                Dim expandedNode = ExpandAsync(Nothing, Nothing).Result
+            End Sub,
+            Sub(exception) Equal(exception.ParamName, "document"))
+    End Sub
+
+    <Fact, Trait(Traits.Feature, Traits.Features.Formatting)>
+    Public Sub TestTokenExpand()
+        AssertThrows(Of ArgumentNullException)(
+            Sub()
+                Dim expandedNode = Expand(Nothing, Nothing, Nothing)
+            End Sub,
+            Sub(exception) Equal(exception.ParamName, "semanticModel"))
+    End Sub
+
+    <Fact, Trait(Traits.Feature, Traits.Features.Formatting)>
+    Public Sub TestTokenExpand2()
+        Dim semanticModel = GetSemanticModel()
+        AssertThrows(Of ArgumentNullException)(
+            Sub()
+                Dim expandedNode = Expand(Nothing, semanticModel, Nothing)
+            End Sub,
+            Sub(exception) Equal(exception.ParamName, "workspace"))
+    End Sub
+
+    <Fact, Trait(Traits.Feature, Traits.Features.Formatting)>
+    Public Sub TestReduceAsync()
+        AssertThrows(Of ArgumentNullException)(
+            Sub()
+                Dim simplifiedNode = ReduceAsync(Nothing).Result
+            End Sub,
+            Sub(exception) Equal(exception.ParamName, "document"))
+    End Sub
+
+    <Fact, Trait(Traits.Feature, Traits.Features.Formatting)>
+    Public Sub TestReduceAsync2()
+        Dim syntaxAnnotation As SyntaxAnnotation = Nothing
+        AssertThrows(Of ArgumentNullException)(
+            Sub()
+                Dim simplifiedNode = ReduceAsync(Nothing, syntaxAnnotation).Result
+            End Sub,
+            Sub(exception) Equal(exception.ParamName, "document"))
+    End Sub
+
+    <Fact, Trait(Traits.Feature, Traits.Features.Formatting)>
+    Public Sub TestReduceAsync3()
+        Dim syntaxAnnotation As SyntaxAnnotation = Nothing
+        Dim document = GetDocument()
+        AssertThrows(Of ArgumentNullException)(
+            Sub()
+                Dim simplifiedNode = ReduceAsync(document, syntaxAnnotation).Result
+            End Sub,
+            Sub(exception) Equal(exception.ParamName, "annotation"))
+    End Sub
+
+    <Fact, Trait(Traits.Feature, Traits.Features.Formatting)>
+    Public Sub TestReduceAsync4()
+        Dim textSpan As TextSpan = Nothing
+        AssertThrows(Of ArgumentNullException)(
+            Sub()
+                Dim simplifiedNode = ReduceAsync(Nothing, textSpan).Result
+            End Sub,
+            Sub(exception) Equal(exception.ParamName, "document"))
+    End Sub
+
+    <Fact, Trait(Traits.Feature, Traits.Features.Formatting)>
+    Public Sub TestReduceAsync5()
+        Dim spans As IEnumerable(Of TextSpan) = Nothing
+        AssertThrows(Of ArgumentNullException)(
+            Sub()
+                Dim simplifiedNode = ReduceAsync(Nothing, spans).Result
+            End Sub,
+            Sub(exception) Equal(exception.ParamName, "document"))
+    End Sub
+
+    <Fact, Trait(Traits.Feature, Traits.Features.Formatting)>
+    Public Sub TestReduceAsync6()
+        Dim document = GetDocument()
+        Dim spans As IEnumerable(Of TextSpan) = Nothing
+        AssertThrows(Of ArgumentNullException)(
+            Sub()
+                Dim simplifiedNode = ReduceAsync(document, spans).Result
+            End Sub,
+            Sub(exception) Equal(exception.ParamName, "spans"))
+    End Sub
+
+    Private Function GetDocument() As Document
+        CreateFiles(GetSimpleCSharpSolutionFiles())
+        Dim sol = Create(properties:=New Dictionary(Of String, String) From {{"Configuration", "Release"}}).OpenSolutionAsync(GetSolutionFileName("TestSolution.sln")).Result
+        Return sol.Projects.First.Documents.First
+    End Function
+
+    Private Function GetSemanticModel() As SemanticModel
+        Return GetDocument().GetSemanticModelAsync().Result
+    End Function
+
+    Private Function GetSyntaxNode() As SyntaxNode
+        Return IdentifierName(Identifier("Test"))
+    End Function
+End Class
diff --git a/src/Workspaces/Core/Portable/Simplification/Simplifier.cs b/src/Workspaces/Core/Portable/Simplification/Simplifier.cs
@@ -47,6 +47,16 @@ public static partial class Simplifier
         /// </summary>
         public static async Task<TNode> ExpandAsync<TNode>(TNode node, Document document, Func<SyntaxNode, bool> expandInsideNode = null, bool expandParameter = false, CancellationToken cancellationToken = default(CancellationToken)) where TNode : SyntaxNode
         {
+            if (node == null)
+            {
+                throw new ArgumentNullException(nameof(node));
+            }
+
+            if (document == null)
+            {
+                throw new ArgumentNullException(nameof(document));
+            }
+
             var semanticModel = await document.GetSemanticModelAsync(cancellationToken).ConfigureAwait(false);
             return Expand(node, semanticModel, document.Project.Solution.Workspace, expandInsideNode, expandParameter, cancellationToken);
         }
@@ -56,6 +66,21 @@ public static partial class Simplifier
         /// </summary>
         public static TNode Expand<TNode>(TNode node, SemanticModel semanticModel, Workspace workspace, Func<SyntaxNode, bool> expandInsideNode = null, bool expandParameter = false, CancellationToken cancellationToken = default(CancellationToken)) where TNode : SyntaxNode
         {
+            if (node == null)
+            {
+                throw new ArgumentNullException(nameof(node));
+            }
+
+            if (semanticModel == null)
+            {
+                throw new ArgumentNullException(nameof(semanticModel));
+            }
+
+            if (workspace == null)
+            {
+                throw new ArgumentNullException(nameof(workspace));
+            }
+
             var result = workspace.Services.GetLanguageServices(node.Language).GetService<ISimplificationService>()
                 .Expand(node, semanticModel, annotationForReplacedAliasIdentifier: null, expandInsideNode: expandInsideNode, expandParameter: expandParameter, cancellationToken: cancellationToken);
 
@@ -67,6 +92,11 @@ public static partial class Simplifier
         /// </summary>
         public static async Task<SyntaxToken> ExpandAsync(SyntaxToken token, Document document, Func<SyntaxNode, bool> expandInsideNode = null, CancellationToken cancellationToken = default(CancellationToken))
         {
+            if (document == null)
+            {
+                throw new ArgumentNullException(nameof(document));
+            }
+
             var semanticModel = await document.GetSemanticModelAsync(cancellationToken).ConfigureAwait(false);
             return Expand(token, semanticModel, document.Project.Solution.Workspace, expandInsideNode, cancellationToken);
         }
@@ -76,6 +106,16 @@ public static async Task<SyntaxToken> ExpandAsync(SyntaxToken token, Document do
         /// </summary>
         public static SyntaxToken Expand(SyntaxToken token, SemanticModel semanticModel, Workspace workspace, Func<SyntaxNode, bool> expandInsideNode = null, CancellationToken cancellationToken = default(CancellationToken))
         {
+            if (semanticModel == null)
+            {
+                throw new ArgumentNullException(nameof(semanticModel));
+            }
+
+            if (workspace == null)
+            {
+                throw new ArgumentNullException(nameof(workspace));
+            }
+
             return workspace.Services.GetLanguageServices(token.Language).GetService<ISimplificationService>()
                 .Expand(token, semanticModel, expandInsideNode, cancellationToken);
         }
@@ -85,6 +125,11 @@ public static SyntaxToken Expand(SyntaxToken token, SemanticModel semanticModel,
         /// </summary>
         public static async Task<Document> ReduceAsync(Document document, OptionSet optionSet = null, CancellationToken cancellationToken = default(CancellationToken))
         {
+            if (document == null)
+            {
+                throw new ArgumentNullException(nameof(document));
+            }
+
             var root = await document.GetSyntaxRootAsync(cancellationToken).ConfigureAwait(false);
             return await ReduceAsync(document, root.FullSpan, optionSet, cancellationToken).ConfigureAwait(false);
         }
@@ -95,6 +140,16 @@ public static async Task<Document> ReduceAsync(Document document, OptionSet opti
         /// </summary>
         public static async Task<Document> ReduceAsync(Document document, SyntaxAnnotation annotation, OptionSet optionSet = null, CancellationToken cancellationToken = default(CancellationToken))
         {
+            if (document == null)
+            {
+                throw new ArgumentNullException(nameof(document));
+            }
+
+            if (annotation == null)
+            {
+                throw new ArgumentNullException(nameof(annotation));
+            }
+
             var root = await document.GetSyntaxRootAsync(cancellationToken).ConfigureAwait(false);
             return await ReduceAsync(document, root.GetAnnotatedNodesAndTokens(annotation).Select(t => t.FullSpan), optionSet, cancellationToken).ConfigureAwait(false);
         }
@@ -105,6 +160,11 @@ public static async Task<Document> ReduceAsync(Document document, SyntaxAnnotati
         /// </summary>
         public static Task<Document> ReduceAsync(Document document, TextSpan span, OptionSet optionSet = null, CancellationToken cancellationToken = default(CancellationToken))
         {
+            if (document == null)
+            {
+                throw new ArgumentNullException(nameof(document));
+            }
+
             return ReduceAsync(document, SpecializedCollections.SingletonEnumerable(span), optionSet, cancellationToken);
         }
 
@@ -114,6 +174,16 @@ public static Task<Document> ReduceAsync(Document document, TextSpan span, Optio
         /// </summary>
         public static Task<Document> ReduceAsync(Document document, IEnumerable<TextSpan> spans, OptionSet optionSet = null, CancellationToken cancellationToken = default(CancellationToken))
         {
+            if (document == null)
+            {
+                throw new ArgumentNullException(nameof(document));
+            }
+
+            if (spans == null)
+            {
+                throw new ArgumentNullException(nameof(spans));
+            }
+
             return document.Project.LanguageServices.GetService<ISimplificationService>().ReduceAsync(document, spans, optionSet, cancellationToken: cancellationToken);
         }
 

diff --git a/src/Workspaces/CoreTest/ServicesTest.csproj b/src/Workspaces/CoreTest/ServicesTest.csproj
@@ -292,4 +292,4 @@
     <Import Project="..\..\..\build\Targets\Roslyn.Toolsets.Xunit.targets" />
     <Import Project="$(SolutionDir)\.nuget\NuGet.targets" Condition="Exists('$(SolutionDir)\.nuget\NuGet.targets')" />
   </ImportGroup>
-</Project>
+</Project>