Skip to content

Commit

Permalink
Continue visiting bad named argument if not target-typed in NullableW…
Browse files Browse the repository at this point in the history
…alker (#73326)

* Add a test

* Continue visiting bad named argument if not target-typed

* Simplify an assert
  • Loading branch information
jjonescz authored May 10, 2024
1 parent 19c7e71 commit 18614bc
Show file tree
Hide file tree
Showing 2 changed files with 69 additions and 15 deletions.
44 changes: 29 additions & 15 deletions src/Compilers/CSharp/Portable/FlowAnalysis/NullableWalker.cs
Original file line number Diff line number Diff line change
Expand Up @@ -6719,27 +6719,26 @@ private ImmutableArray<VisitResult> VisitArguments(
(ParameterSymbol? parameter, TypeWithAnnotations parameterType, FlowAnalysisAnnotations parameterAnnotations, bool isExpandedParamsArgument) =
GetCorrespondingParameter(i, parametersOpt, argsToParamsOpt, expanded, ref paramsIterationType);

if (// This is known to happen for certain error scenarios, because
// the parameter matching logic above is not as flexible as the one we use in `Binder.BuildArgumentsForErrorRecovery`
// so we may end up with a pending conversion completion for an argument apparently without a corresponding parameter.
parameter is null ||
// In error recovery with named arguments, target-typing cannot work as we can get a different parameter type
// from our GetCorrespondingParameter logic than Binder.BuildArgumentsForErrorRecovery does.
node is BoundCall { HasErrors: true, ArgumentNamesOpt.IsDefaultOrEmpty: false, ArgsToParamsOpt.IsDefault: true })
// This is known to happen for certain error scenarios, because
// the parameter matching logic above is not as flexible as the one we use in `Binder.BuildArgumentsForErrorRecovery`
// so we may end up with a pending conversion completion for an argument apparently without a corresponding parameter.
if (parameter is null)
{
if (IsTargetTypedExpression(argumentNoConversion) && _targetTypedAnalysisCompletionOpt?.TryGetValue(argumentNoConversion, out var completion) is true)
if (tryShortCircuitTargetTypedExpression(argument, argumentNoConversion))
{
// We've done something wrong if we have a target-typed expression and registered an analysis continuation for it
// (we won't be able to complete that continuation)
// We flush the completion with a plausible/dummy type and remove it.
completion(TypeWithAnnotations.Create(argument.Type));
TargetTypedAnalysisCompletion.Remove(argumentNoConversion);

Debug.Assert(parameter is not null || method is ErrorMethodSymbol);
Debug.Assert(method is ErrorMethodSymbol);
}
continue;
}

// In error recovery with named arguments, target-typing cannot work as we can get a different parameter type
// from our GetCorrespondingParameter logic than Binder.BuildArgumentsForErrorRecovery does.
if (node is BoundCall { HasErrors: true, ArgumentNamesOpt.IsDefaultOrEmpty: false, ArgsToParamsOpt.IsDefault: true } &&
tryShortCircuitTargetTypedExpression(argument, argumentNoConversion))
{
continue;
}

// We disable diagnostics when:
// 1. the containing call has errors (to reduce cascading diagnostics)
// 2. on implicit default arguments (since that's really only an issue with the declaration)
Expand Down Expand Up @@ -6926,6 +6925,21 @@ static void expandParamsCollection(ref ImmutableArray<BoundExpression> arguments
}
}
}

bool tryShortCircuitTargetTypedExpression(BoundExpression argument, BoundExpression argumentNoConversion)
{
if (IsTargetTypedExpression(argumentNoConversion) && _targetTypedAnalysisCompletionOpt?.TryGetValue(argumentNoConversion, out var completion) is true)
{
// We've done something wrong if we have a target-typed expression and registered an analysis continuation for it
// (we won't be able to complete that continuation)
// We flush the completion with a plausible/dummy type and remove it.
completion(TypeWithAnnotations.Create(argument.Type));
TargetTypedAnalysisCompletion.Remove(argumentNoConversion);
return true;
}

return false;
}
}

private void ApplyMemberPostConditions(BoundExpression? receiverOpt, MethodSymbol? method)
Expand Down
Original file line number Diff line number Diff line change
Expand Up @@ -158691,5 +158691,45 @@ static void M<T>() where T : struct, I
// y.Value.Item.ToString();
Diagnostic(ErrorCode.WRN_NullReferenceReceiver, "y.Value.Item").WithLocation(15, 9));
}

[Fact, WorkItem("https://devdiv.visualstudio.com/DevDiv/_workitems/edit/2045970")]
public void BadAsyncLambdaInNamedArgument()
{
var source = """
#nullable enable
using System;
using System.Threading.Tasks;

class D
{
void M()
{
var c1 = new C();
var c2 = new C();
C.M1(f: async () =>
{
if (c2 != null)
{
c1.
await c2.M2();
}
});
}
}

class C
{
public static void M1(Func<Task> f) { }
public async Task M2() => await Task.Yield();
}
""";
CreateCompilation(source).VerifyDiagnostics(
// (15,20): error CS1001: Identifier expected
// c1.
Diagnostic(ErrorCode.ERR_IdentifierExpected, "").WithLocation(15, 20),
// (15,20): error CS1002: ; expected
// c1.
Diagnostic(ErrorCode.ERR_SemicolonExpected, "").WithLocation(15, 20));
}
}
}

0 comments on commit 18614bc

Please sign in to comment.