Skip to content

Commit

Permalink
Fix CS0126 and CS0127 issues with async methods.
Browse files Browse the repository at this point in the history
The resolver now reports 'void' as expected type for return expressions in void methods.
  • Loading branch information
dgrunwald committed Jan 8, 2014
1 parent 1240a5f commit 571dd02
Show file tree
Hide file tree
Showing 10 changed files with 271 additions and 99 deletions.
Original file line number Diff line number Diff line change
Expand Up @@ -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;
}


Expand Down Expand Up @@ -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<CodeAction>();
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)) {
Expand Down
Original file line number Diff line number Diff line change
Expand Up @@ -45,71 +45,15 @@ protected override IGatherVisitor CreateVisitor(BaseRefactoringContext context)

class GatherVisitor : GatherVisitorBase<CS0127ReturnMustNotBeFollowedByAnyExpression>
{
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<CodeAction>();
actions.Add(new CodeAction(ctx.TranslateString("Remove returned expression"), script => {
script.Replace(returnStatement, new ReturnStatement());
Expand Down
39 changes: 7 additions & 32 deletions ICSharpCode.NRefactory.CSharp/Resolver/ResolveVisitor.cs
Original file line number Diff line number Diff line change
Expand Up @@ -359,7 +359,7 @@ void ProcessConversion(Expression expr, ResolveResult rr, IType targetType)
/// </summary>
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 {
Expand Down Expand Up @@ -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);
}
Expand Down Expand Up @@ -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);
}
Expand Down Expand Up @@ -2697,15 +2697,15 @@ 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 {
if (returnValues.Count == 0)
return isEndpointUnreachable;
if (isAsync) {
// async lambdas must return Task<T>
if (!(IsTask(returnType) && returnType.TypeParameterCount == 1))
if (!(TaskType.IsTask(returnType) && returnType.TypeParameterCount == 1))
return false;
// unpack Task<T> for testing the implicit conversions
returnType = ((ParameterizedType)returnType).GetTypeArgument(0);
Expand All @@ -2718,34 +2718,9 @@ static bool IsValidLambda(bool isValidAsVoidMethod, bool isEndpointUnreachable,
}
}

/// <summary>
/// Gets the T in Task&lt;T&gt;.
/// Returns void for non-generic Task.
/// Any other type is returned unmodified.
/// </summary>
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);
}

/// <summary>
/// Gets whether the specified type is Task or Task&lt;T&gt;.
/// </summary>
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
Expand Down Expand Up @@ -3035,7 +3010,7 @@ ResolveResult IAstVisitor<ResolveResult>.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<EntityDeclaration>().FirstOrDefault();
if (methodDecl != null && (methodDecl.Modifiers & Modifiers.Async) == Modifiers.Async)
type = UnpackTask(type);
Expand Down
15 changes: 14 additions & 1 deletion ICSharpCode.NRefactory.Demo/CSDemo.cs
Original file line number Diff line number Diff line change
Expand Up @@ -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);
Expand All @@ -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)
Expand Down
9 changes: 6 additions & 3 deletions ICSharpCode.NRefactory.Demo/SemanticTreeDialog.cs
Original file line number Diff line number Diff line change
Expand Up @@ -32,14 +32,17 @@ namespace ICSharpCode.NRefactory.Demo
/// </summary>
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);
}
Expand Down
Original file line number Diff line number Diff line change
Expand Up @@ -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<int> M()
{
return;
}
}";

TestRefactoringContext context;
var issues = GetIssues (new CS0126ReturnMustBeFollowedByAnyExpression (), input, out context);
Assert.AreEqual (1, issues.Count);
}
}
}

Original file line number Diff line number Diff line change
Expand Up @@ -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<int> M()
{
return 1;
}
}";

TestRefactoringContext context;
var issues = GetIssues (new CS0127ReturnMustNotBeFollowedByAnyExpression (), input, out context);
Assert.AreEqual (0, issues.Count);
}
}
}

Loading

0 comments on commit 571dd02

Please sign in to comment.