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

Support nullable parameters #147

Merged
merged 1 commit into from
Nov 22, 2023
Merged

Support nullable parameters #147

merged 1 commit into from
Nov 22, 2023

Conversation

skarllot
Copy link
Contributor

@skarllot skarllot commented Nov 20, 2023

The issue or feature being addressed

Details on the issue fix or feature implementation

  • Fix handling of nullable parameters
  • New diagnostics JAB0013 and JAB0014 requesting to define a default value for nullable dependency

@skarllot
Copy link
Contributor Author

Added diagnostic test

@skarllot
Copy link
Contributor Author

Do you think that JAB0013 should be raised even when dependency is found?

@pakrym
Copy link
Owner

pakrym commented Nov 21, 2023

Do you think that JAB0013 should be raised even when dependency is found?

I think the warning should be something like "{} parameter will never be null when constructing using a service provider. Add a default value to make the service reference optional." and fire for all nullable required parameters. It feels like there is no point in defining a nullable non-optional parameter.

Feel free to pull the warning out of this PR, I will be away from the computer for more than a week, so I'd rather get this PR in and you unblocked.

@skarllot
Copy link
Contributor Author

I've created 2 diagnostics, one for non-registered nullable dependency and other for registered nullable dependency. And changed CodeWriter to always display type as not null.

@skarllot skarllot changed the title Support nullable as optional dependency Support nullable parameters Nov 22, 2023
@pakrym
Copy link
Owner

pakrym commented Nov 22, 2023

Great work! Thank you!

@pakrym pakrym merged commit 0059d44 into pakrym:main Nov 22, 2023
6 checks passed
@pakrym
Copy link
Owner

pakrym commented Nov 22, 2023

Out it 0.8.7

@Marv51
Copy link

Marv51 commented Nov 28, 2023

Hi, could I ask about the reasoning behind "JAB0014: X parameter to construct Y will never be null when constructing using a service provider."

my class that raises the warning looks like this:

public Foo(Bar? service, Bar2 service2){
}

I use Jab for my application, but construct the service manually in test cases.

The warning is absolutely correct Jab will always provide a service for the nullable parameter but for convenience in test cases this parameter is optional.
I could pretty easily rearrange the parameter to Foo(Bar2 service2, Bar? service = null) to make this warning go away, but I don't quite understand how that is an improvement?

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