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

Disallow null checking discard parameters #58952

Merged
merged 3 commits into from
Jan 20, 2022

Conversation

RikkiGibson
Copy link
Contributor

@RikkiGibson RikkiGibson commented Jan 19, 2022

Related to #36024

LDM: allow on lambda discard parameter? resolution: Disallow. (delaying till after feature merge)

@RikkiGibson RikkiGibson marked this pull request as ready for review January 19, 2022 23:47
@RikkiGibson RikkiGibson requested review from a team as code owners January 19, 2022 23:47

class C
{
Action<string> lambda = _ =>
Copy link
Member

@jcouv jcouv Jan 20, 2022

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Please test the scenario with two underscores. A single underscore is not a discard.
https://github.com/dotnet/csharplang/blob/main/proposals/csharp-9.0/lambda-discard-parameters.md #Closed

Copy link
Contributor Author

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Have added the test, thanks.

{
public C()
{
Func<string, int> f = ([||]_) => { return 0; };
Copy link
Member

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Same here

Copy link
Contributor Author

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

See TestOnDiscardLambdaParameter2

Copy link
Member

@jcouv jcouv left a comment

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Compiler side LGTM Thanks (iteration 2). Some IDE test suggestions

@jcouv
Copy link
Member

jcouv commented Jan 20, 2022

@cston @chsienki for second review. Thanks

Diagnostic(ErrorCode.ERR_DiscardCannotBeNullChecked, "_").WithLocation(8, 46),
// (8,51): error CS8990: Discard parameter cannot be null-checked.
// public Action<string, string> action3 = (_!!, _!!) => { }; // 3, 4
Diagnostic(ErrorCode.ERR_DiscardCannotBeNullChecked, "_").WithLocation(8, 51));
}
Copy link
Member

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

}

Consider checking IParameterSymbol.IsNullChecked for one or more of these cases.

@RikkiGibson RikkiGibson merged commit aad378e into dotnet:main Jan 20, 2022
@RikkiGibson RikkiGibson deleted the pnc-discard branch January 20, 2022 22:11
@ghost ghost added this to the Next milestone Jan 20, 2022
333fred added a commit to 333fred/roslyn that referenced this pull request Jan 21, 2022
* upstream/main: (1035 commits)
  Add missing header
  Mark IVSTypeScriptFormattingServiceImplementation as optional, but require it in the constructor
  Fix Go To Base for a symbol with a single metadata location (dotnet#58965)
  [EnC] Store entire spans instead of line deltas (dotnet#58831)
  Delete CodeAnalysisRules.ruleset (dotnet#58968)
  Allow xml docs to still be created when 'emit metadata only' is on. (dotnet#57667)
  Fix ParseVBErrorOrWarning (dotnet#47833)
  Update parameter nullability to match implementation
  Ensure CSharpUseParameterNullCheckingDiagnosticAnalyzer works with nullable parameters
  Add tests for issues fixed by previous PR (dotnet#58764)
  Update src/Features/CSharp/Portable/Completion/CompletionProviders/ExplicitInterfaceMemberCompletionProvider.CompletionSymbolDisplay.cs
  Disallow null checking discard parameters (dotnet#58952)
  Add extension method
  Escape type arguments
  Few fixes
  Update tests.
  Add Analyzers layer to CODEOWNERS
  Add formatting analyzer test for param nullchecking (dotnet#58936)
  Move reading HideAdvancedMembers option up (dotnet#58747)
  List patterns: Slice value is assumed to be never null (dotnet#57457)
  ...
@RikkiGibson RikkiGibson modified the milestones: Next, 17.2.P1 Feb 4, 2022
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Projects
None yet
Development

Successfully merging this pull request may close these issues.

4 participants