Skip to content

Update codebase to use Array.Empty<T> - #4127

Merged
mikkelbu merged 8 commits into
nunit:masterfrom
mjedrzejek:master
Jul 31, 2022
Merged

Update codebase to use Array.Empty<T>#4127
mikkelbu merged 8 commits into
nunit:masterfrom
mjedrzejek:master

Conversation

@mjedrzejek

@mjedrzejek mjedrzejek commented Jun 11, 2022

Copy link
Copy Markdown
Contributor

Update codebase to use Array.Empty

Fixes #4051

@dnfadmin

dnfadmin commented Jun 11, 2022

Copy link
Copy Markdown

CLA assistant check
All CLA requirements met.

@mjedrzejek mjedrzejek changed the title ss Update codebase to use Array.Empty<T> Jun 11, 2022

@manfred-brands manfred-brands left a comment

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.

This is only a subset of the zero element array allocations.
There are many more.
You can find them with a regular expression: new [a-zA-Z_0-9]*\[0\]

@mjedrzejek

Copy link
Copy Markdown
Contributor Author

@manfred-brands thanks!
I used the regular expression and added more changes to the MR. I was not able to replace 5 occurrences where empty array is created as a parameter for attribute - if I try to change the code there, I am getting a compiler error and I was not able to find a workaround (error: CS0182 -
An attribute argument must be a constant expression, typeof expression or array creation expression of an attribute parameter type).

@manfred-brands manfred-brands left a comment

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.

Technically the conversion are done correctly.
Looking at the code, two of them should actually be Enumerable.Empty<int>().
Please update those two please.

Comment thread src/NUnitFramework/tests/Assertions/ConditionAssertTests.cs Outdated
Comment thread src/NUnitFramework/tests/Assertions/ConditionAssertTests.cs Outdated
@manfred-brands

manfred-brands commented Jun 26, 2022

Copy link
Copy Markdown
Member

For next time, note that there is a 'Quickfix' for CA1825, that does the simple conversion for you.
It also knows that you cannot convert the ones in attributes.

Go to one place where there was an empty array allocation and select Ctrl+. or select the lightbulb.
image

Unfortunately that quickfix doesn't add using statements, nor does the Enumerable.Empty conversion.

@mikkelbu mikkelbu left a comment

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.

LGTM. Thanks for the work @mjedrzejek 👍

I've updated the first message in this PR, so that it will close #4051

@mikkelbu
mikkelbu merged commit bada876 into nunit:master Jul 31, 2022
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.

Update codebase to use Array.Empty<T>

5 participants