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

Nullability of reference types in type of parameter 'b' of 'lambda expression' doesn't match the target delegate 'Action<KeysetPaginationBuilder<Category?>>' (possibly because of nullability attributes). csharp(CS8622) #6

Closed
ugumerie opened this issue Feb 3, 2023 · 6 comments · Fixed by #7

Comments

@ugumerie
Copy link

ugumerie commented Feb 3, 2023

Version Used: 2.0.0

I'm making using of Roslynator extension in VSCode for code refactoring, fixing, analyzing etc.
Nullable is also enabled
Target framework: 6.0

Code snippet

....
return await _paginationService.KeysetPaginateAsync(
      dbContext.Categories!,
      b => b.Descending(x => x!.CreatedOn),
      async id => await _categories.FindAsync(Guid.Parse(id)),
      query => query.ProjectToType<GetKeysetPagedCategoriesResponse>()
 );

Compilation warnings:
Nullability of reference types in type of parameter 'b' of 'lambda expression' doesn't match the target delegate 'Action<KeysetPaginationBuilder<Category?>>' (possibly because of nullability attributes). csharp(CS8622)

The type 'SoftPurse.Modules.ServiceCatalog.Core.Catalog.Aggregates.Category?' cannot be used as type parameter 'T' in the generic type or method 'IPaginationService.KeysetPaginateAsync<T, TOut>(IQueryable<T>, Action<KeysetPaginationBuilder<T>>, Func<string, Task<T>>, Func<IQueryable<T>, IQueryable<TOut>>, KeysetQueryModel)'. Nullability of type argument 'SoftPurse.Modules.ServiceCatalog.Core.Catalog.Aggregates.Category?' doesn't match 'class' constraint. csharp(CS8634)

Note: Category mention here is the entity.
It is being inferred as await _paginationService.KeysetPaginateAsync<Category?, GetKeysetPagedCategoriesResponse>(...)

@mrahhal
Copy link
Owner

mrahhal commented Feb 4, 2023

I'm not familiar with Roslynator so I can't comment much on that, but the usage of null suppressions (!) is a red flag. Nullable properties are not supported in keysets in general, as mentioned in the caveats in the KeysetPagination package. So that might be the cause of that analyzer warning.

@ugumerie
Copy link
Author

ugumerie commented Feb 4, 2023

Thank you very much. I just worked on it and explicitly passed in the T and TOut type.
Here is the code like so:

....
 return await _paginationService.KeysetPaginateAsync<Category, GetKeysetPagedCategoriesResponse>(
        _dbContext.Categories.Where(c => c.IsActive),
        b => b.Descending(x => x.CreatedOn),
        async id => await_dbContext.Categories.FindAsync(Guid.Parse(id)),
        query => query.ProjectToType<GetKeysetPagedCategoriesResponse>(),
        queryModel: new KeysetQueryModel
        {
            First = true,
            Size = 10
        }
 );

Warning: Possible null reference return on async id => await _dbContext.Categories.FindAsync(Guid.Parse(id))
I still get a Possible null reference return warning on the Func<string, Task<T>> getReferenceAsync delegate. since FindAsync() returns an object or null. But the code works perfectly and does the pagination. Just that the warning is not good for the eyes :)

PaginationService code snippet:

....
else if (queryModel.After != null)
{
	var reference = await getReferenceAsync(queryModel.After);
	keysetContext = query.KeysetPaginate(builderAction, KeysetPaginationDirection.Forward, reference);
}
else if (queryModel.Before != null)
{
	var reference = await getReferenceAsync(queryModel.Before);
	// This is a special case.
	// If reference is null (maybe the entity was deleted from the database), we want to
	// always return the first page, so enforce a Forward direction.
	var direction = reference != null ? KeysetPaginationDirection.Backward : KeysetPaginationDirection.Forward;
	keysetContext = query.KeysetPaginate(builderAction, direction, reference);
}
....

Question on PaginationService code snippet:
Is there a way you could also check whether the reference returned from queryModel.After is null, just like you did for queryModel.Before such that the getReferenceAsync delegate parameter would look like this - Func<string, Task<T?>> getReferenceAsync where T could be null?

@mrahhal
Copy link
Owner

mrahhal commented Feb 4, 2023

These warnings are actually true in that there's a mismatch, so you can for example do a null suppression in the getReferenceAsync arg:

async id => await _dbContext.Categories.FindAsync(Guid.Parse(id))!,

Since getReferenceAsync doesn't expect a null to be returned (some kind of exception will happen if a null is returned), so it's your responsibility to handle that in a way that's appropriate in your application. Example:

async id =>
{
     var r = await _dbContext.Categories.FindAsync(Guid.Parse(id));
     if (r == null) throw new ...Exception();
     return r;
}

I wondered before if this should be handled on the library level (so getReferenceAsync would start allowing nulls), but I didn't reach a decision on that. But related to your last question, it's weird that I'm actually handling the null only for Before. I can't remember why I did that. If I add that for the Before as well, then this means I'm expecting nulls and so the generic constraint should be relaxed here. I think it's reasonable. I'll think about it a bit more.

@ugumerie
Copy link
Author

ugumerie commented Feb 4, 2023

Aha, great, thank you. Will do just that.

@mrahhal
Copy link
Owner

mrahhal commented Feb 10, 2023

So regarding this:

else if (queryModel.After != null)
{
var reference = await getReferenceAsync(queryModel.After);
keysetContext = query.KeysetPaginate(builderAction, KeysetPaginationDirection.Forward, reference);
}
else if (queryModel.Before != null)
{
var reference = await getReferenceAsync(queryModel.Before);
// This is a special case.
// If reference is null (maybe the entity was deleted from the database), we want to
// always return the first page, so enforce a Forward direction.
var direction = reference != null ? KeysetPaginationDirection.Backward : KeysetPaginationDirection.Forward;
keysetContext = query.KeysetPaginate(builderAction, direction, reference);
}

The reason the After block didn't have the same logic is because it's already Forward in direction, so the behavior does actually match the Before block on nulls. Before is a special case because we want to enforce Forward. In any case, this issue still stands to resolve the nullability mismatch for analysis.

@mrahhal
Copy link
Owner

mrahhal commented Feb 10, 2023

Should be fixed in v2.0.1.

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 a pull request may close this issue.

2 participants