Skip to content
New issue

Have a question about this project? Sign up for a free GitHub account to open an issue and contact its maintainers and the community.

By clicking “Sign up for GitHub”, you agree to our terms of service and privacy statement. We’ll occasionally send you account related emails.

Already on GitHub? Sign in to your account

Improve places where CS1661 is reported #69322

Closed
Rekkonnect opened this issue Aug 1, 2023 · 1 comment · Fixed by #69334
Closed

Improve places where CS1661 is reported #69322

Rekkonnect opened this issue Aug 1, 2023 · 1 comment · Fixed by #69334
Labels
Area-Compilers Concept-Diagnostic Clarity The issues deals with the ease of understanding of errors and warnings. untriaged Issues and PRs which have not yet been triaged by a lead

Comments

@Rekkonnect
Copy link
Contributor

Inspired while working on #69273

CS1661 (ERR_CantConvAnonMethParams) currently overshadows the real diagnostics emitted for lambda parameter mismatches. It is always being reported in cases of parameter type or ref-kindness mismatches, alongside the other useful diagnostics accurately showing what the error is and being reported on the location of intertest, not covering the entire lambda.

Consider the following examples:

Action<string, string> a = (string a, int b) => { };
//                         ~~~~~~~~~~~~~~~~~~~~~~~~
// CS1661 -- Cannot convert lambda expression to type 'Action<string, string>' because the
//           parameter types do not match the delegate parameter types

//                                        ~
// CS1678 -- Parameter 2 is declared as type 'int' but should be 'string'



Action<string, string> b = (string a, string b) => { return a; };
//                                                   ~~~~~~
// CS8030 -- Anonymous function converted to a void returning delegate cannot return a value



Func<string, string> c = (string b) => { return b.Length; };
//                                              ~~~~~~~~
// CS0029 -- Cannot implicitly convert type 'int' to 'string'

//                                              ~~~~~~~~
// CS1662 -- Cannot convert lambda expression to intended delegate type because some of the
//           return types in the block are not implicitly convertible to the delegate return type

CS1661 completely overshadows the real problem by spanning across the entire lambda expression, which offers very little diagnostic value to the user.

CS1662 is similarly overlapping with an existing CS0029 but at least it does not overshadow the real diagnostic, nor is it emitted on the entire lambda.

In this case where we have a mismatch of the type or the ref-kindness of the parameter, CS1661 is best not reported as the other errors take care of this.

It should be noted that the diagnostic is not emitted when the parameter counts do not match. The only scenario that we report CS1661 altogether is a generic wrapping, while also handling the parameter type/ref-kind mismatches.

I propose that we only report CS1661 when there is nothing else that can be reported. If we can report a diagnostic about the mismatch of the type or the ref-kindness of each individual parameter, CS1661 should never appear.

@dotnet-issue-labeler dotnet-issue-labeler bot added Area-Compilers untriaged Issues and PRs which have not yet been triaged by a lead labels Aug 1, 2023
@CyrusNajmabadi CyrusNajmabadi added Concept-Diagnostic Clarity The issues deals with the ease of understanding of errors and warnings. untriaged Issues and PRs which have not yet been triaged by a lead and removed untriaged Issues and PRs which have not yet been triaged by a lead labels Aug 1, 2023
@CyrusNajmabadi
Copy link
Member

A simple fix would be to make the lambda error get reported on the =>. Then tehre's no concern about losing information. It just becomes a matter of the diagnostics not colliding with each other.

Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
Area-Compilers Concept-Diagnostic Clarity The issues deals with the ease of understanding of errors and warnings. untriaged Issues and PRs which have not yet been triaged by a lead
Projects
None yet
Development

Successfully merging a pull request may close this issue.

2 participants