From 571dd02b4f07a309c445c129e869074bc585a855 Mon Sep 17 00:00:00 2001 From: Daniel Grunwald Date: Wed, 8 Jan 2014 22:10:56 +0100 Subject: [PATCH] Fix CS0126 and CS0127 issues with async methods. The resolver now reports 'void' as expected type for return expressions in void methods. --- ...0126ReturnMustBeFollowedByAnyExpression.cs | 22 ++++-- ...7ReturnMustNotBeFollowedByAnyExpression.cs | 58 +--------------- .../Resolver/ResolveVisitor.cs | 39 ++--------- ICSharpCode.NRefactory.Demo/CSDemo.cs | 15 +++- .../SemanticTreeDialog.cs | 9 ++- ...turnMustBeFollowedByAnyExpressionTestes.cs | 57 ++++++++++++++++ ...rnMustNotBeFollowedByAnyExpressionTests.cs | 57 ++++++++++++++++ .../CSharp/Resolver/LambdaTests.cs | 44 ++++++++++++ .../ICSharpCode.NRefactory.csproj | 1 + ICSharpCode.NRefactory/TypeSystem/TaskType.cs | 68 +++++++++++++++++++ 10 files changed, 271 insertions(+), 99 deletions(-) create mode 100644 ICSharpCode.NRefactory/TypeSystem/TaskType.cs diff --git a/ICSharpCode.NRefactory.CSharp.Refactoring/CodeIssues/Custom/CompilerErrors/CS0126ReturnMustBeFollowedByAnyExpression.cs b/ICSharpCode.NRefactory.CSharp.Refactoring/CodeIssues/Custom/CompilerErrors/CS0126ReturnMustBeFollowedByAnyExpression.cs index 159a6d69c..78b1fca6a 100644 --- a/ICSharpCode.NRefactory.CSharp.Refactoring/CodeIssues/Custom/CompilerErrors/CS0126ReturnMustBeFollowedByAnyExpression.cs +++ b/ICSharpCode.NRefactory.CSharp.Refactoring/CodeIssues/Custom/CompilerErrors/CS0126ReturnMustBeFollowedByAnyExpression.cs @@ -48,21 +48,29 @@ internal static IType GetRequestedReturnType (BaseRefactoringContext ctx, AstNod { entityNode = returnStatement.GetParent(p => p is LambdaExpression || p is AnonymousMethodExpression || !(p is Accessor) && p is EntityDeclaration); if (entityNode == null) - return null; + return SpecialType.UnknownType; if (entityNode is EntityDeclaration) { var rr = ctx.Resolve(entityNode) as MemberResolveResult; if (rr == null) - return null; + return SpecialType.UnknownType; + if (((EntityDeclaration)entityNode).HasModifier(Modifiers.Async)) + return TaskType.UnpackTask(ctx.Compilation, rr.Member.ReturnType); return rr.Member.ReturnType; } + bool isAsync = false; + if (entityNode is LambdaExpression) + isAsync = ((LambdaExpression)entityNode).IsAsync; + if (entityNode is AnonymousMethodExpression) + isAsync = ((AnonymousMethodExpression)entityNode).IsAsync; foreach (var type in TypeGuessing.GetValidTypes(ctx.Resolver, entityNode)) { if (type.Kind != TypeKind.Delegate) continue; var invoke = type.GetDelegateInvokeMethod(); - if (invoke != null && !invoke.ReturnType.IsKnownType(KnownTypeCode.Void)) - return invoke.ReturnType; + if (invoke != null) { + return isAsync ? TaskType.UnpackTask(ctx.Compilation, invoke.ReturnType) : invoke.ReturnType; + } } - return null; + return SpecialType.UnknownType; } @@ -149,8 +157,10 @@ public override void VisitReturnStatement(ReturnStatement returnStatement) return; AstNode entityNode; var rr = GetRequestedReturnType (ctx, returnStatement, out entityNode); + if (rr.Kind == TypeKind.Void) + return; var actions = new List(); - if (rr != null) { + if (rr.Kind != TypeKind.Unknown) { actions.Add(new CodeAction(ctx.TranslateString("Return default value"), script => { Expression p; if (rr.IsKnownType(KnownTypeCode.Boolean)) { diff --git a/ICSharpCode.NRefactory.CSharp.Refactoring/CodeIssues/Custom/CompilerErrors/CS0127ReturnMustNotBeFollowedByAnyExpression.cs b/ICSharpCode.NRefactory.CSharp.Refactoring/CodeIssues/Custom/CompilerErrors/CS0127ReturnMustNotBeFollowedByAnyExpression.cs index 7f2d02883..61b0168a1 100644 --- a/ICSharpCode.NRefactory.CSharp.Refactoring/CodeIssues/Custom/CompilerErrors/CS0127ReturnMustNotBeFollowedByAnyExpression.cs +++ b/ICSharpCode.NRefactory.CSharp.Refactoring/CodeIssues/Custom/CompilerErrors/CS0127ReturnMustNotBeFollowedByAnyExpression.cs @@ -45,71 +45,15 @@ protected override IGatherVisitor CreateVisitor(BaseRefactoringContext context) class GatherVisitor : GatherVisitorBase { - bool skip; - public GatherVisitor (BaseRefactoringContext ctx) : base (ctx) { } - static bool AnonymousMethodMayReturnNonVoid(BaseRefactoringContext ctx, Expression anonymousMethodExpression) - { - foreach (var type in TypeGuessing.GetValidTypes(ctx.Resolver, anonymousMethodExpression)) { - if (type.Kind != TypeKind.Delegate) - continue; - var invoke = type.GetDelegateInvokeMethod(); - if (invoke != null && !invoke.ReturnType.IsKnownType(KnownTypeCode.Void)) - return true; - } - return false; - } - - public override void VisitMethodDeclaration(MethodDeclaration methodDeclaration) - { - var primitiveType = methodDeclaration.ReturnType as PrimitiveType; - if (primitiveType == null || primitiveType.Keyword != "void") - return; - base.VisitMethodDeclaration(methodDeclaration); - } - - public override void VisitOperatorDeclaration(OperatorDeclaration operatorDeclaration) - { - } - - public override void VisitAccessor(Accessor accessor) - { - if (accessor.Role == PropertyDeclaration.SetterRole || - accessor.Role == IndexerDeclaration.SetterRole ) - base.VisitAccessor(accessor); - } - - - public override void VisitCustomEventDeclaration(CustomEventDeclaration eventDeclaration) - { - } - - public override void VisitAnonymousMethodExpression(AnonymousMethodExpression anonymousMethodExpression) - { - bool old = skip; - skip = AnonymousMethodMayReturnNonVoid(ctx, anonymousMethodExpression); - base.VisitAnonymousMethodExpression(anonymousMethodExpression); - skip = old; - } - - public override void VisitLambdaExpression(LambdaExpression lambdaExpression) - { - bool old = skip; - skip = AnonymousMethodMayReturnNonVoid(ctx, lambdaExpression); - base.VisitLambdaExpression(lambdaExpression); - skip = old; - } - public override void VisitReturnStatement(ReturnStatement returnStatement) { base.VisitReturnStatement(returnStatement); - if (skip) - return; - if (!returnStatement.Expression.IsNull) { + if (ctx.GetExpectedType(returnStatement.Expression).Kind == TypeKind.Void) { var actions = new List(); actions.Add(new CodeAction(ctx.TranslateString("Remove returned expression"), script => { script.Replace(returnStatement, new ReturnStatement()); diff --git a/ICSharpCode.NRefactory.CSharp/Resolver/ResolveVisitor.cs b/ICSharpCode.NRefactory.CSharp/Resolver/ResolveVisitor.cs index b4ce9d98c..56e6ac92b 100644 --- a/ICSharpCode.NRefactory.CSharp/Resolver/ResolveVisitor.cs +++ b/ICSharpCode.NRefactory.CSharp/Resolver/ResolveVisitor.cs @@ -359,7 +359,7 @@ void ProcessConversion(Expression expr, ResolveResult rr, IType targetType) /// void ResolveAndProcessConversion(Expression expr, IType targetType) { - if (targetType.Kind == TypeKind.Unknown || targetType.Kind == TypeKind.Void) { + if (targetType.Kind == TypeKind.Unknown) { // no need to resolve the expression right now Scan(expr); } else { @@ -2163,7 +2163,7 @@ public void ApplyReturnType(ResolveVisitor parentVisitor, IType returnType) Analyze(); IType unpackedReturnType = isAsync ? visitor.UnpackTask(returnType) : returnType; Log.WriteLine("Applying return type {0} to explicitly-typed lambda {1}", unpackedReturnType, this.LambdaExpression); - if (unpackedReturnType.Kind != TypeKind.Void) { + if (unpackedReturnType.Kind != TypeKind.Void || body is BlockStatement) { for (int i = 0; i < returnExpressions.Count; i++) { visitor.ProcessConversion(returnExpressions[i], returnValues[i], unpackedReturnType); } @@ -2502,7 +2502,7 @@ public void MergeInto(ResolveVisitor parentVisitor, IType returnType) } Log.WriteLine("Applying return type {0} to implicitly-typed lambda {1}", returnType, lambda.LambdaExpression); - if (returnType.Kind != TypeKind.Void) { + if (returnType.Kind != TypeKind.Void || lambda.BodyExpression is Statement) { for (int i = 0; i < returnExpressions.Count; i++) { visitor.ProcessConversion(returnExpressions[i], returnValues[i], returnType); } @@ -2697,7 +2697,7 @@ static bool IsValidLambda(bool isValidAsVoidMethod, bool isEndpointUnreachable, // can be converted to delegates with void return type. // This holds for both async and regular lambdas. return isValidAsVoidMethod; - } else if (isAsync && IsTask(returnType) && returnType.TypeParameterCount == 0) { + } else if (isAsync && TaskType.IsTask(returnType) && returnType.TypeParameterCount == 0) { // Additionally, async lambdas with the above property can be converted to non-generic Task. return isValidAsVoidMethod; } else { @@ -2705,7 +2705,7 @@ static bool IsValidLambda(bool isValidAsVoidMethod, bool isEndpointUnreachable, return isEndpointUnreachable; if (isAsync) { // async lambdas must return Task - if (!(IsTask(returnType) && returnType.TypeParameterCount == 1)) + if (!(TaskType.IsTask(returnType) && returnType.TypeParameterCount == 1)) return false; // unpack Task for testing the implicit conversions returnType = ((ParameterizedType)returnType).GetTypeArgument(0); @@ -2718,34 +2718,9 @@ static bool IsValidLambda(bool isValidAsVoidMethod, bool isEndpointUnreachable, } } - /// - /// Gets the T in Task<T>. - /// Returns void for non-generic Task. - /// Any other type is returned unmodified. - /// IType UnpackTask(IType type) { - if (!IsTask(type)) - return type; - if (type.TypeParameterCount == 0) - return resolver.Compilation.FindType(KnownTypeCode.Void); - else - return ((ParameterizedType)type).GetTypeArgument(0); - } - - /// - /// Gets whether the specified type is Task or Task<T>. - /// - static bool IsTask(IType type) - { - ITypeDefinition def = type.GetDefinition(); - if (def != null) { - if (def.KnownTypeCode == KnownTypeCode.Task) - return true; - if (def.KnownTypeCode == KnownTypeCode.TaskOfT) - return type is ParameterizedType; - } - return false; + return TaskType.UnpackTask(resolver.Compilation, type); } sealed class AnalyzeLambdaVisitor : DepthFirstAstVisitor @@ -3035,7 +3010,7 @@ ResolveResult IAstVisitor.VisitReturnStatement(ReturnStatement re { if (resolverEnabled && !resolver.IsWithinLambdaExpression && resolver.CurrentMember != null) { IType type = resolver.CurrentMember.ReturnType; - if (IsTask(type)) { + if (TaskType.IsTask(type)) { var methodDecl = returnStatement.Ancestors.OfType().FirstOrDefault(); if (methodDecl != null && (methodDecl.Modifiers & Modifiers.Async) == Modifiers.Async) type = UnpackTask(type); diff --git a/ICSharpCode.NRefactory.Demo/CSDemo.cs b/ICSharpCode.NRefactory.Demo/CSDemo.cs index bf9fd95d2..523c5cc09 100644 --- a/ICSharpCode.NRefactory.Demo/CSDemo.cs +++ b/ICSharpCode.NRefactory.Demo/CSDemo.cs @@ -213,11 +213,18 @@ void ResolveButtonClick(object sender, EventArgs e) ICompilation compilation = project.CreateCompilation(); ResolveResult result; + IType expectedType = null; + Conversion conversion = null; if (csharpTreeView.SelectedNode != null) { var selectedNode = (AstNode)csharpTreeView.SelectedNode.Tag; CSharpAstResolver resolver = new CSharpAstResolver(compilation, syntaxTree, unresolvedFile); result = resolver.Resolve(selectedNode); // CSharpAstResolver.Resolve() never returns null + Expression expr = selectedNode as Expression; + if (expr != null) { + expectedType = resolver.GetExpectedType(expr); + conversion = resolver.GetConversion(expr); + } } else { TextLocation location = GetTextLocation(csharpCodeTextBox, csharpCodeTextBox.SelectionStart); result = ResolveAtLocation.Resolve(compilation, unresolvedFile, syntaxTree, location); @@ -226,8 +233,14 @@ void ResolveButtonClick(object sender, EventArgs e) return; } } - using (var dlg = new SemanticTreeDialog(result)) + using (var dlg = new SemanticTreeDialog()) { + dlg.AddRoot("Resolve() = ", result); + if (expectedType != null) + dlg.AddRoot("GetExpectedType() = ", expectedType); + if (conversion != null) + dlg.AddRoot("GetConversion() = ", conversion); dlg.ShowDialog(); + } } void CSharpCodeTextBoxKeyDown(object sender, KeyEventArgs e) diff --git a/ICSharpCode.NRefactory.Demo/SemanticTreeDialog.cs b/ICSharpCode.NRefactory.Demo/SemanticTreeDialog.cs index 7fb57b6d8..85d9e16a4 100644 --- a/ICSharpCode.NRefactory.Demo/SemanticTreeDialog.cs +++ b/ICSharpCode.NRefactory.Demo/SemanticTreeDialog.cs @@ -32,14 +32,17 @@ namespace ICSharpCode.NRefactory.Demo /// public partial class SemanticTreeDialog : Form { - public SemanticTreeDialog(ResolveResult rr) + public SemanticTreeDialog() { // // The InitializeComponent() call is required for Windows Forms designer support. // InitializeComponent(); - - var rootNode = MakeObjectNode("Resolve() = ", rr); + } + + public void AddRoot(string prefix, object obj) + { + var rootNode = MakeObjectNode(prefix, obj); rootNode.Expand(); treeView.Nodes.Add(rootNode); } diff --git a/ICSharpCode.NRefactory.Tests/CSharp/CodeIssues/CS0126ReturnMustBeFollowedByAnyExpressionTestes.cs b/ICSharpCode.NRefactory.Tests/CSharp/CodeIssues/CS0126ReturnMustBeFollowedByAnyExpressionTestes.cs index d52a20892..0e9fea18c 100644 --- a/ICSharpCode.NRefactory.Tests/CSharp/CodeIssues/CS0126ReturnMustBeFollowedByAnyExpressionTestes.cs +++ b/ICSharpCode.NRefactory.Tests/CSharp/CodeIssues/CS0126ReturnMustBeFollowedByAnyExpressionTestes.cs @@ -330,6 +330,63 @@ class Test } }"); } + + [Test] + public void TestAsyncMethod_Void() + { + var input = @"using System; +using System.Threading.Tasks; + +class Test +{ + public async void M() + { + return; + } +}"; + + TestRefactoringContext context; + var issues = GetIssues (new CS0126ReturnMustBeFollowedByAnyExpression (), input, out context); + Assert.AreEqual (0, issues.Count); + } + + [Test] + public void TestAsyncMethod_Task() + { + var input = @"using System; +using System.Threading.Tasks; + +class Test +{ + public async Task M() + { + return; + } +}"; + + TestRefactoringContext context; + var issues = GetIssues (new CS0126ReturnMustBeFollowedByAnyExpression (), input, out context); + Assert.AreEqual (0, issues.Count); + } + + [Test] + public void TestAsyncMethod_TaskOfInt() + { + var input = @"using System; +using System.Threading.Tasks; + +class Test +{ + public async Task M() + { + return; + } +}"; + + TestRefactoringContext context; + var issues = GetIssues (new CS0126ReturnMustBeFollowedByAnyExpression (), input, out context); + Assert.AreEqual (1, issues.Count); + } } } diff --git a/ICSharpCode.NRefactory.Tests/CSharp/CodeIssues/CS0127ReturnMustNotBeFollowedByAnyExpressionTests.cs b/ICSharpCode.NRefactory.Tests/CSharp/CodeIssues/CS0127ReturnMustNotBeFollowedByAnyExpressionTests.cs index 132221889..efb08465d 100644 --- a/ICSharpCode.NRefactory.Tests/CSharp/CodeIssues/CS0127ReturnMustNotBeFollowedByAnyExpressionTests.cs +++ b/ICSharpCode.NRefactory.Tests/CSharp/CodeIssues/CS0127ReturnMustNotBeFollowedByAnyExpressionTests.cs @@ -301,6 +301,63 @@ void Test () } }"); } + + [Test] + public void TestAsyncMethod_Void() + { + var input = @"using System; +using System.Threading.Tasks; + +class Test +{ + public async void M() + { + return 1; + } +}"; + + TestRefactoringContext context; + var issues = GetIssues (new CS0127ReturnMustNotBeFollowedByAnyExpression (), input, out context); + Assert.AreEqual (1, issues.Count); + } + + [Test] + public void TestAsyncMethod_Task() + { + var input = @"using System; +using System.Threading.Tasks; + +class Test +{ + public async Task M() + { + return 1; + } +}"; + + TestRefactoringContext context; + var issues = GetIssues (new CS0127ReturnMustNotBeFollowedByAnyExpression (), input, out context); + Assert.AreEqual (1, issues.Count); + } + + [Test] + public void TestAsyncMethod_TaskOfInt() + { + var input = @"using System; +using System.Threading.Tasks; + +class Test +{ + public async Task M() + { + return 1; + } +}"; + + TestRefactoringContext context; + var issues = GetIssues (new CS0127ReturnMustNotBeFollowedByAnyExpression (), input, out context); + Assert.AreEqual (0, issues.Count); + } } } diff --git a/ICSharpCode.NRefactory.Tests/CSharp/Resolver/LambdaTests.cs b/ICSharpCode.NRefactory.Tests/CSharp/Resolver/LambdaTests.cs index 97c7f73a2..86dfc69c5 100644 --- a/ICSharpCode.NRefactory.Tests/CSharp/Resolver/LambdaTests.cs +++ b/ICSharpCode.NRefactory.Tests/CSharp/Resolver/LambdaTests.cs @@ -891,5 +891,49 @@ public void M() { var c = GetConversion(program); Assert.IsTrue(c.IsValid); } + + [Test] + public void InvalidConversionToVoid_ImplicitStatementLambda() + { + string program = @"using System; +class TestClass { + static void Main() { + Action a = () => { + return $1$; + }; + } +}"; + Assert.AreEqual(TypeKind.Void, GetExpectedType(program).Kind); + Assert.IsFalse(GetConversion(program).IsValid); + } + + [Test] + public void InvalidConversionToVoid_AnonymousMethod() + { + string program = @"using System; +class TestClass { + static void Main() { + Action a = delegate { + return $1$; + }; + } +}"; + Assert.AreEqual(TypeKind.Void, GetExpectedType(program).Kind); + Assert.IsFalse(GetConversion(program).IsValid); + } + + [Test] + public void VoidLamda_Uses_Identity_Conversion() + { + string program = @"using System; +class TestClass { + static int GetInt() { return 4; } + static void Main() { + Action a = () => $GetInt()$; + } +}"; + Assert.AreEqual("System.Int32", GetExpectedType(program).FullName); + Assert.IsTrue(GetConversion(program).IsValid); + } } } diff --git a/ICSharpCode.NRefactory/ICSharpCode.NRefactory.csproj b/ICSharpCode.NRefactory/ICSharpCode.NRefactory.csproj index b9ff77ae3..e704ffd95 100644 --- a/ICSharpCode.NRefactory/ICSharpCode.NRefactory.csproj +++ b/ICSharpCode.NRefactory/ICSharpCode.NRefactory.csproj @@ -152,6 +152,7 @@ + diff --git a/ICSharpCode.NRefactory/TypeSystem/TaskType.cs b/ICSharpCode.NRefactory/TypeSystem/TaskType.cs new file mode 100644 index 000000000..f83e0dd1a --- /dev/null +++ b/ICSharpCode.NRefactory/TypeSystem/TaskType.cs @@ -0,0 +1,68 @@ +/* + * Created by SharpDevelop. + * User: Daniel + * Date: 1/8/2014 + * Time: 21:26 + * + * To change this template use Tools | Options | Coding | Edit Standard Headers. + */ +using System; + +namespace ICSharpCode.NRefactory.TypeSystem +{ + /// + /// Helper class for dealing with System.Threading.Tasks.Task. + /// + public static class TaskType + { + /// + /// Gets the T in Task<T>. + /// Returns void for non-generic Task. + /// Any other type is returned unmodified. + /// + public static IType UnpackTask(ICompilation compilation, IType type) + { + if (!IsTask(type)) + return type; + if (type.TypeParameterCount == 0) + return compilation.FindType(KnownTypeCode.Void); + else + return type.TypeArguments[0]; + } + + /// + /// Gets whether the specified type is Task or Task<T>. + /// + public static bool IsTask(IType type) + { + ITypeDefinition def = type.GetDefinition(); + if (def != null) { + if (def.KnownTypeCode == KnownTypeCode.Task) + return true; + if (def.KnownTypeCode == KnownTypeCode.TaskOfT) + return type is ParameterizedType; + } + return false; + } + + /// + /// Creates a task type. + /// + public static IType Create(ICompilation compilation, IType elementType) + { + if (compilation == null) + throw new ArgumentNullException("compilation"); + if (elementType == null) + throw new ArgumentNullException("elementType"); + + if (elementType.Kind == TypeKind.Void) + return compilation.FindType(KnownTypeCode.Task); + IType taskType = compilation.FindType(KnownTypeCode.TaskOfT); + ITypeDefinition taskTypeDef = taskType.GetDefinition(); + if (taskTypeDef != null) + return new ParameterizedType(taskTypeDef, new [] { elementType }); + else + return taskType; + } + } +}