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

Review code fixer for CA1861 (Avoid constant arrays) #6697

Closed
mavasani opened this issue Jun 19, 2023 · 3 comments · Fixed by #6714
Closed

Review code fixer for CA1861 (Avoid constant arrays) #6697

mavasani opened this issue Jun 19, 2023 · 3 comments · Fixed by #6714
Labels
Area-CodeFix Bug in code fixer Bug The product is not behaving according to its current intended design help wanted The issue is up-for-grabs, and can be claimed by commenting
Milestone

Comments

@mavasani
Copy link
Contributor

The codefix here is quite concerning. Though if there is a problem, it's out of scope for the PR anyway.

If the input code was:

public static List<object> Cases { get; } = new() { new object[0] };

Is there a diagnostic? How the codefix behaves? If the codefix extracted the array to a field below the Cases static property, then it's semantically different.

Originally posted by @Youssef1313 in #6696 (comment)

@mavasani mavasani added Bug The product is not behaving according to its current intended design help wanted The issue is up-for-grabs, and can be claimed by commenting Area-CodeFix Bug in code fixer labels Jun 22, 2023
@mavasani mavasani added this to the Unknown milestone Jun 22, 2023
@buyaa-n
Copy link
Member

buyaa-n commented Jun 26, 2023

Thank you @mavasani for fixing NullReferenceException from CA1861

If the input code was:

public static List<object> Cases { get; } = new() { new object[0] };

Is there a diagnostic? How the codefix behaves? If the codefix extracted the array to a field below the Cases static property, then it's semantically different.

Good point, keep warning in this case looks wrong, especially when the array length is not a constant (literal). From the hits found in runtime repo I do not see a valid diagnostic (did not check all), looks most cases it will be false positives, so I am going to put up a PR that exclude these cases.

Let me know if you think there is a valid case, I could not think of any. CC @Youssef1313 @stephentoub

@stephentoub
Copy link
Member

stephentoub commented Jun 27, 2023

If the input code was:
public static List<object> Cases { get; } = new() { new object[0] };

That should trigger the Array.Empty analyzer.

@buyaa-n
Copy link
Member

buyaa-n commented Jun 30, 2023

As mentioned in #6714 (comment) Array.Empty analyzer triggered in VS

Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
Area-CodeFix Bug in code fixer Bug The product is not behaving according to its current intended design help wanted The issue is up-for-grabs, and can be claimed by commenting
Projects
None yet
Development

Successfully merging a pull request may close this issue.

3 participants