Skip to content

Add support for Assert.EnterMultipleScope#816

Merged
manfred-brands merged 2 commits intomasterfrom
Issue769_EnterMultipleScope
Jan 8, 2025
Merged

Add support for Assert.EnterMultipleScope#816
manfred-brands merged 2 commits intomasterfrom
Issue769_EnterMultipleScope

Conversation

@manfred-brands
Copy link
Copy Markdown
Member

Fixes #769

Detects that code inside a using (Assert.EnterMultipleScope()) is inside a multiple context.
Adds a codefix for to use Assert.EnterMultipleScope.

@manfred-brands
Copy link
Copy Markdown
Member Author

When referencing NUnit >= 4.2.0

{11E56E81-99D5-405D-A9BB-F312078DFAE7}

And
{BE7F9BE2-6B67-4F8C-A87A-F909AF1127E2}

When referencing NUnit 3.14.0:
{6E962561-C638-4082-8F16-DCD0B05B85AA}

@manfred-brands manfred-brands force-pushed the Issue769_EnterMultipleScope branch from 23a6195 to 9fef008 Compare December 23, 2024 08:47
Copy link
Copy Markdown
Member

@mikkelbu mikkelbu left a comment

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Looks great. I'm happy to merge this when you are. Thanks for the improvement @manfred-brands

Comment thread src/nunit.analyzers/UseAssertMultiple/UseAssertMultipleCodeFix.cs Outdated
RoslynAssert.Valid(this.analyzer, testCode);
}

#if WOULD_SOMEONE_ACTUALLY_USE_THIS
Copy link
Copy Markdown
Member

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Is there a reason for why we wouldn't want to run this test :) ?

Copy link
Copy Markdown
Member Author

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Yes, it won't work as we don't have support for it.
It would require tracking the declared variable and check if it is manually disposed somewhere within the scope.

Unless someone has a good reason for using this pattern, I don't see any, I won't spend my time on it.

@manfred-brands manfred-brands force-pushed the Issue769_EnterMultipleScope branch from 9fef008 to cf7eacb Compare January 8, 2025 06:10
@manfred-brands manfred-brands merged commit 18f6a70 into master Jan 8, 2025
@manfred-brands manfred-brands deleted the Issue769_EnterMultipleScope branch January 8, 2025 07:47
@mikkelbu mikkelbu added this to the Release 4.6 milestone Jan 9, 2025
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.

Recognized the new Assert.EnterMultipleScope() concept.

2 participants