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

Fix S6605 and S6617 FP: Should not be applied to expressions used by EntityFramework #7286

Closed
KuraiAndras opened this issue May 26, 2023 · 2 comments · Fixed by #7332 or #7334
Closed
Assignees
Labels
Area: C# C# rules related issues. Type: False Positive Rule IS triggered when it shouldn't be.
Projects
Milestone

Comments

@KuraiAndras
Copy link

KuraiAndras commented May 26, 2023

Description

The S6605 analyzer flags code inside expressions which will cause EF queries to be uncompilable.

I understand that checking for usage specifically related to EF might be problematic from the analyzer's side, but it is a very common use case and manually suppressing the issue is problematic. The analyzer itself is very useful in all other settings so turning it off by default is not the best solution.

Repro steps

public class MyEntity
{
    public Guid Id { get; set; }
}

public class MyDbContext : DbContext
{
    public DbSet<MyEntity> MyEntities { get; init; } = default!;
}

public class MyService
{
    private readonly MyDbContext _dbContext;

    public MyService(MyDbContext dbContext) => _dbContext = dbContext;

    // This can be compiled by EF but is flagged by S6605
    public async Task DeleteEntities(List<Guid> ids)
    {
        var entitiesToDelete = await _dbContext
            .MyEntities
            .Where(e => ids.Any(i => e.Id == i))
            .ToListAsync();

        _dbContext.RemoveRange(entitiesToDelete);

        await _dbContext.SaveChangesAsync();
    }

    // This cannot be compiled by EF but is not flagged by S6605
    public async Task DeleteEntities2(List<Guid> ids)
    {
        var entitiesToDelete = await _dbContext
            .MyEntities
            .Where(e => ids.Exists(i => e.Id == i))
            .ToListAsync();

        _dbContext.RemoveRange(entitiesToDelete);

        await _dbContext.SaveChangesAsync();
    }
}

Expected behavior

Not all collection-specific methods should be used inside expressions used by EntityFramework

Actual behavior

Warning is displayed

Known workarounds

  1. Suppress the warning
  2. Change types passed into EF expressions to not be concrete collections, use IEnumerable<T> or ICollection<T> instead

Related information

Sonar analyzer version 9.1.0.70676

This case might also apply to other collection-specific warnings

sebastien-marichal added a commit that referenced this issue Jun 1, 2023
@sebastien-marichal sebastien-marichal self-assigned this Jun 1, 2023
@sebastien-marichal sebastien-marichal added Type: False Positive Rule IS triggered when it shouldn't be. Area: C# C# rules related issues. labels Jun 1, 2023
@sebastien-marichal
Copy link
Contributor

Hello @KuraiAndras,

Thank you for your feedback.

This is confirmed as a false positive.

@github-actions github-actions bot added this to Review in progress in Best Kanban Jun 2, 2023
@github-actions github-actions bot moved this from Review in progress to Review approved in Best Kanban Jun 2, 2023
@github-actions github-actions bot moved this from Review approved to In progress in Best Kanban Jun 2, 2023
@pavel-mikula-sonarsource pavel-mikula-sonarsource changed the title Fix S6605 FP: Should not be applied to expressions used by EntityFramework Fix S6605 and S6617 FP: Should not be applied to expressions used by EntityFramework Jun 2, 2023
@github-actions github-actions bot moved this from In progress to Review in progress in Best Kanban Jun 2, 2023
@github-actions github-actions bot moved this from Review in progress to Review approved in Best Kanban Jun 2, 2023
andrei-epure-sonarsource pushed a commit that referenced this issue Jun 2, 2023
…EntityFramework #7286 (#7332)

Co-authored-by: Sebastien Marichal <sebastien.marichal@sonarsource.com>
Best Kanban automation moved this from Review approved to Validate Peach Jun 2, 2023
@KuraiAndras
Copy link
Author

Nice work guys 🚀

Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment