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

Use CallerArgumentExpressionAttribute in throw helpers #1788

Open
wants to merge 7 commits into
base: main
Choose a base branch
from

Conversation

IliaBrahinets
Copy link

@IliaBrahinets IliaBrahinets commented Jan 20, 2025

The following changes have been made in throw helpers:

  • Added CallerArgumentExpressionAttribute to simplify throw assertions (nameof(parameterName) can be omitted in most cases)
  • Added StringSyntaxAttribute for error message templates

@CLAassistant
Copy link

CLAassistant commented Jan 20, 2025

CLA assistant check
All committers have signed the CLA.

@dlemstra
Copy link
Owner

Thanks for your pull request it will probably take a while before I have time to look at this. Only quick thing that I saw was string? message = "Must not be null". I don't want to change the default message for a ArgumentNullException so please restore those two overloads.

@IliaBrahinets
Copy link
Author

Removed the default value for IfNull message. The are multiple options with overloads:

  1. IfNull([NotNull] object? value, [CallerArgumentExpression(nameof(value))] string? paramName = null, string? message = null)
  2. IfNull([NotNull] object? value, [CallerArgumentExpression(nameof(value))] string? paramName = null) and IfNull([NotNull] object? value, string message, [CallerArgumentExpression(nameof(value))] string? paramName = null)
  3. IfNull([NotNull] object? value, [CallerArgumentExpression(nameof(value))] string? paramName = null) and IfNull([NotNull] object? value, [CallerArgumentExpression(nameof(value))] string? paramName = null, string? message = null)

The third option causes ambiguity for Throw.IfNull(value) call, while the second isn't cosistent in terms of paramName positioning. I decided to implement the first approach for all throw helpers except overloads with message arguments. Awaiting for your feedback on this.

@dlemstra
Copy link
Owner

It looks like you did not address that issue yet? Doing new ArgumentNullException(paramName, null) is not the same as new ArgumentNullException(paramName).

Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
None yet
Projects
None yet
Development

Successfully merging this pull request may close these issues.

3 participants