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 S6966 FP: Mock.Raise from Moq should be ignored #9697

Open
a10r opened this issue Nov 25, 2024 · 2 comments
Open

Fix S6966 FP: Mock.Raise from Moq should be ignored #9697

a10r opened this issue Nov 25, 2024 · 2 comments
Labels
Area: C# C# rules related issues. Type: False Positive Rule IS triggered when it shouldn't be.

Comments

@a10r
Copy link

a10r commented Nov 25, 2024

Description

In an async test method, S6966 is raised for Moq's Mock.Raise method, suggesting to use RaiseAsync instead.

RaiseAsync does not work for normal events and requires a Task-returning event signature instead: https://github.com/moq/moq/pull/1313.

I've seen that this rule has a list of exceptions for specific APIs, so maybe it would make sense to add this case to that list.

Repro steps

Consider this:

public interface IReproInterface
{
	event EventHandler SomethingHappened;
}

public class Repro
{
	[Test]
	public async Task Test()
	{
		await Task.Yield();
		
		var mock = new Mock<IReproInterface>();
		
		mock.Raise(i => i.SomethingHappened += null, new EventArgs()); // "Await RaiseAsync instead"
	}
}

Expected behavior

Should not suggest RaiseAsync.

Actual behavior

Suggests to use RaiseAsync instead of Raise.

Known workarounds

--

Related information

  • C#/VB.NET Plugins version: SonarLint 8.6.0.10679
  • Visual Studio version: 17.12.0
  • MSBuild / dotnet version: dotnet 9.0.100
  • Operating System: Windows 11
@a10r a10r changed the title Fix S6966 FP/FN: Should not suggest Moq's RaiseAsync in async test method Fix S6966 FP: Should not suggest Moq's RaiseAsync in async test method Nov 25, 2024
@sebastien-marichal
Copy link
Contributor

Hello @a10r,

I confirm this is a false positive.

We might need to do a bit more than just add it to the exclusion list.
I am thinking about checking if the event handler returns a task; however, I am not sure this would add value to the rule.

We should also add an exclusion list in RSPEC.

@sebastien-marichal sebastien-marichal added Area: C# C# rules related issues. Type: False Positive Rule IS triggered when it shouldn't be. labels Nov 28, 2024
@sebastien-marichal sebastien-marichal changed the title Fix S6966 FP: Should not suggest Moq's RaiseAsync in async test method Fix S6966 FP: Mock.Raise from Moq should be ignored Nov 28, 2024
@aKzenT
Copy link

aKzenT commented Dec 11, 2024

Please also allow user to configure manual exclusions. We have internal APIs that trigger false positives on this.

Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
Area: C# C# rules related issues. Type: False Positive Rule IS triggered when it shouldn't be.
Projects
None yet
Development

No branches or pull requests

3 participants