dotnet / roslyn

The Roslyn .NET compiler provides C# and Visual Basic languages with rich code analysis APIs.
https://docs.microsoft.com/dotnet/csharp/roslyn-sdk/
MIT License
18.89k stars 4.01k forks source link

In SDK 8.0.2xx IDE0300 (collection expression) uses loose type matching which should not happen according to documentation #72532

Open ArturDorochowicz opened 5 months ago

ArturDorochowicz commented 5 months ago

Version Used: SDK 8.0.202

Steps to Reproduce:

dotnet new classlib
dotnet new editorconfig --empty
// .editorconfig
root = true
[*.cs]
dotnet_diagnostic.IDE0300.severity = warning
// Class1.cs
namespace src;

public class Class1
{
    void Method() {
        IEnumerable<int> i = new int[] { 1, 2, 3 };
    }
}
$> dotnet build -p:EnforceCodeStyleInBuild=true
MSBuild version 17.9.6+a4ecab324 for .NET
  Determining projects to restore...
  All projects are up-to-date for restore.
/src/Class1.cs(6,24): warning IDE0300: Collection initialization can be simplified (https://learn.microsoft.com/dotnet/fundamentals/code-analysis/style-rules/ide0300) [/src/src.csproj]
/src/Class1.cs(6,24): warning IDE0300: Collection initialization can be simplified (https://learn.microsoft.com/dotnet/fundamentals/code-analysis/style-rules/ide0300) [/src/src.csproj]
  src -> /src/bin/Debug/net8.0/src.dll

Build succeeded.

/src/Class1.cs(6,24): warning IDE0300: Collection initialization can be simplified (https://learn.microsoft.com/dotnet/fundamentals/code-analysis/style-rules/ide0300) [/src/src.csproj]
/src/Class1.cs(6,24): warning IDE0300: Collection initialization can be simplified (https://learn.microsoft.com/dotnet/fundamentals/code-analysis/style-rules/ide0300) [/src/src.csproj]
    2 Warning(s)
    0 Error(s)

Time Elapsed 00:00:01.19

Diagnostic Id:

IDE0300, but others that depend on dotnet_style_prefer_collection_expression probably behave the same.

Expected Behavior: The analyzer should not trigger. According to documentation at https://learn.microsoft.com/en-us/dotnet/fundamentals/code-analysis/style-rules/ide0300 loose type matching is behavior from .Net 9 and later. Unless 8.0.202 somehow is not .Net 8, it should default to exact match.

Of course it's also possible that the documentation is wrong.

Actual Behavior: The analyzer triggers, see build output above.

CyrusNajmabadi commented 5 months ago

The docs are incorrect if they specify .net9.

ArturDorochowicz commented 5 months ago

Forgot to add that it the issue does not appear in 8.0.1xx - exact type match is used here.

Docs say:

Default option value: true in .NET 8 when_types_loosely_match in .NET 9 and later versions

https://github.com/dotnet/docs/blob/04ad5ea0572f3df56c6b4e133b080987ae59daea/docs/fundamentals/code-analysis/style-rules/ide0300.md#L31-L39

CyrusNajmabadi commented 5 months ago

Yeah. Looks like a doc bug. There's no association with this feature and version 9.

cremor commented 4 months ago

@gewarren You added the wrong documentation in dotnet/docs#39199. Is this issue enough to get the documentation fixed or should we create a new issue in dotnet/docs?

cremor commented 4 months ago

@gewarren Sorry for hijacking this issue, but I think there are a few other small issues with that EditorConfig setting documentation that could be fixed at the same time:

a) IDE0302 doesn't mention the new options. I saw that you discussed this with @CyrusNajmabadi in https://github.com/dotnet/docs/pull/39199#pullrequestreview-1830767957. But the current situation is not perfect either, because the page now mentions a wrong default value for the setting itself (even if that default value when_types_loosely_match doesn't change anything for IDE0302 compared to true. In my opinion all possible settings should be documented (just like on other IDE03xx pages), but an additional note should mention that the one other setting does not apply to IDE0302.

b) IDE0028 should also mention the additional option and changed default value.

gewarren commented 4 months ago

@genlu Can you transfer this to the dotnet/docs repo?

glen-84 commented 2 months ago

What is the reason for when_types_loosely_match being the default? If it changes semantics, isn't is safer to default to when_types_exactly_match?

CyrusNajmabadi commented 2 months ago

Most code does not need the runtime type to be exactly the same.

This also follows the default of nearly all our other fixers which will change semantics based on common patterns and normal user coding styles.

Allowing the type to change also enables more compiler optimization possibilities.

So this allows the majority of code to move over by default, while getting more succinct and faster, but with a small risk of an undesired change happening. Users who don't like that can opt into the stricter mode.

cremor commented 1 month ago

Can someone please transfer this issue to the dotnet/docs repo?