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

[NUnit2045] Incorrect refactoring #541

Closed
MaceWindu opened this issue May 5, 2023 · 1 comment · Fixed by #555
Closed

[NUnit2045] Incorrect refactoring #541

MaceWindu opened this issue May 5, 2023 · 1 comment · Fixed by #555
Assignees

Comments

@MaceWindu
Copy link

using NUnit.Framework;

[TestFixture]
public class Class1
{
	[Test]
	public void TestValueHandler_Counted()
	{
		var handler = new Handler();

		Assert.That(handler.Build(null), Is.EqualTo(null));
		Assert.That(handler.Build(new object()), Is.EqualTo(0d));
		Assert.That(handler.Build(new { }), Is.EqualTo(0d));
	}
}

internal class Handler
{
	internal object? Build(object? value)
	{
		throw new NotImplementedException();
	}
}

When single refactoring applied, code refactored properly, but when user selects to apply it to some scope, e.g. Document, it produce non-compilable results. Note duplicate Assert.Multiple(() => lines

image

using NUnit.Framework;

[TestFixture]
public class Class1
{
	[Test]
	public void TestValueHandler_Counted()
	{
		var handler = new Handler();
		Assert.Multiple(() =>
		{
			Assert.That(handler.Build(null), Is.EqualTo(null));
			Assert.Multiple(() =>
		{
			Assert.That(handler.Build(new object()), Is.EqualTo(0d));
			Assert.That(handler.Build(new { }), Is.EqualTo(0d));
		});
	}
}

internal class Handler
{
	internal object? Build(object? value)
	{
		throw new NotImplementedException();
	}
}
@manfred-brands
Copy link
Member

manfred-brands commented May 6, 2023

Thanks for the report @MaceWindu

When more than two statements can be merged, the Analyzer raises multiple suggestions.
If you manually pick the first, it combines all, but if you start further down you get what you are seeing:

Assert.That(handler.Build(null), Is.EqualTo(null));
Assert.That(handler.Build(new object()), Is.EqualTo(0d));
Assert.That(handler.Build(new { }), Is.EqualTo(0d));

First step:

Assert.That(handler.Build(null), Is.EqualTo(null));
Assert.Multiple(() =>
{
		Assert.That(handler.Build(new object()), Is.EqualTo(0d));
		Assert.That(handler.Build(new { }), Is.EqualTo(0d));
}

Second step then combines these in another Assert.Multiple.

I will see if I can either limit the amount of suggestions to the first line only or ensure that applying all fixes does the longest fix.

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