ronantakizawa/codereview-bench huggingface dataset:
Repository: dotnet/roslyn
File: src/EditorFeatures/CSharpTest2/Recommendations/ManagedKeywordRecommenderTests.cs (C#)

Code before:
using Xunit;

namespace Microsoft.CodeAnalysis.Editor.CSharp.UnitTests.Recommendations;

[Trait(Traits.Feature, Traits.Features.KeywordRecommending)]
public sealed class ManagedKeywordRecommenderTests : KeywordRecommenderTests
{
    [Fact]
    public Task TestInFunctionPointerDeclaration()
        => VerifyKeywordAsync(
            """
            class Test {
                unsafe void N() {
                    delegate* $$
            """);

    [Fact]
    public Task TestInFunctionPointerDeclarationTouchingAsterisk()
        => VerifyKeywordAsync(
            """
            class Test {
                unsafe void N() {
                    delegate*$$
            """);

    [Fact]
    public Task TestInFunctionPointerDeclarationAfterTyping()
        => VerifyKeywordAsync(
            """
            class Test {
                unsafe void N() {
                    delegate*manag$$
            """);
}


Review comment (bug):
@copilot use `[Fact, WorkItem("...")]`.  Also, test the specific case i n the original bug, where the function pointer type was being written inside a cast.

Code after:
using Roslyn.Test.Utilities;
using Xunit;

namespace Microsoft.CodeAnalysis.Editor.CSharp.UnitTests.Recommendations;

[Trait(Traits.Feature, Traits.Features.KeywordRecommending)]
public sealed class ManagedKeywordRecommenderTests : KeywordRecommenderTests
{
    [Fact]
    public Task TestInFunctionPointerDeclaration()
        => VerifyKeywordAsync(
            """
            class Test {
                unsafe void N() {
                    delegate* $$
            """);

    [Fact]
    public Task TestInFunctionPointerDeclarationTouchingAsterisk()
        => VerifyKeywordAsync(
            """
            class Test {
                unsafe void N() {
                    delegate*$$
            """);

    [Fact, WorkItem("https://github.com/dotnet/roslyn/issues/81015")]
    public Task TestInCastExpressionAfterTyping()
        => VerifyKeywordAsync(
            """
            class C
            {
                unsafe static void M()
                {
                    _ = (delegate*$$)&M;
                }
            }
            """);
}
Repository: dotnet/roslyn
File: src/EditorFeatures/CSharpTest2/Recommendations/ManagedKeywordRecommenderTests.cs (C#)

Code before:
    public Task TestInFunctionPointerDeclarationTouchingAsterisk()
        => VerifyKeywordAsync(
            """
            class Test {
                unsafe void N() {
                    delegate*$$
            """);

    [Fact, WorkItem("https://github.com/dotnet/roslyn/issues/81015")]
    public Task TestInFunctionPointerDeclarationAfterTyping()
        => VerifyKeywordAsync(
            """
            class Test {
                unsafe void N() {
                    delegate*manag$$
            """);

    [Fact, WorkItem("https://github.com/dotnet/roslyn/issues/81015")]
    public Task TestInCastExpressionAfterTyping()
        => VerifyKeywordAsync(
            """
            class C
            {
                unsafe static void M()
                {
                    _ = (delegate*manag$$)&M;
                }
            }
            """);
}


Review comment (suggestion):
```suggestion
                    _ = (delegate*$$)&M;
```

Code after:
﻿// Licensed to the .NET Foundation under one or more agreements.
// The .NET Foundation licenses this file to you under the MIT license.
// See the LICENSE file in the project root for more information.

using System.Threading.Tasks;
using Microsoft.CodeAnalysis.Test.Utilities;
using Roslyn.Test.Utilities;
using Xunit;

namespace Microsoft.CodeAnalysis.Editor.CSharp.UnitTests.Recommendations;

[Trait(Traits.Feature, Traits.Features.KeywordRecommending)]
public sealed class ManagedKeywordRecommenderTests : KeywordRecommenderTests
{
    [Fact]
    public Task TestInFunctionPointerDeclaration()
        => VerifyKeywordAsync(
            """
            class Test {
                unsafe void N() {
                    delegate* $$
            """);

    [Fact]
    public Task TestInFunctionPointerDeclarationTouchingAsterisk()
        => VerifyKeywordAsync(
            """
            class Test {
                unsafe void N() {
                    delegate*$$
            """);

    [Fact, WorkItem("https://github.com/dotnet/roslyn/issues/81015")]
    public Task TestInCastExpressionAfterTyping()
        => VerifyKeywordAsync(
            """
            class C
            {
                unsafe static void M()
                {
                    _ = (delegate*$$)&M;
                }
            }
            """);
}
Repository: dotnet/roslyn
File: src/Features/CSharpTest/GenerateVariable/GenerateVariableTests.cs (C#)

Code before:
                void Method(D? c)
                {
                    c?.[|Goo|] = 1;
                }
            }

            struct D
            {
            }
            """,
            """
            class Class
            {
                void Method(D? c)
                {
                    c?.Goo = 1;
                }
            }
            
            struct D
            {
                public int Goo { get; internal set; }
            }
            """);

    [Fact]
    public Task TestNotOfferedInEventAddAccessor()
        => TestMissingInRegularAndScriptAsync(
            """
            class C
            {
                event EventHandler E
                {
                    add { [|ev|] += value; }
                    remove { ev -= value; }
                }
            }
            """);

    [Fact]
    public Task TestNotOfferedInEventRemoveAccessor()
        => TestMissingInRegularAndScriptAsync(
            """
            class C
            {
                event EventHandler E
                {
                    add { ev += value; }
                    remove { [|ev|] -= value; }
                }
            }

Review comment (suggestion):
@copilot  please do `[Fact, WorkItem("https://github.com/dotnet/roslyn/issues/81071")]` on all tests you've added.

Code after:
                void Method(D? c)
                {
                    c?.[|Goo|] = 1;
                }
            }

            struct D
            {
            }
            """,
            """
            class Class
            {
                void Method(D? c)
                {
                    c?.Goo = 1;
                }
            }
            
            struct D
            {
                public int Goo { get; internal set; }
            }
            """);

    [Fact, WorkItem("https://github.com/dotnet/roslyn/issues/81071")]
    public Task TestNotOfferedInEventAddAccessor()
        => TestExactActionSetOfferedAsync(
            """
            class C
            {
                event EventHandler E
                {
                    add { [|ev|] += value; }
                    remove { ev -= value; }
                }
            }
            """, [string.Format(CodeFixesResources.Generate_field_0, "ev"), string.Format(CodeFixesResources.Generate_property_0, "ev"), string.Format(CodeFixesResources.Generate_local_0, "ev")]);

    [Fact, WorkItem("https://github.com/dotnet/roslyn/issues/81071")]
    public Task TestNotOfferedInEventRemoveAccessor()
        => TestExactActionSetOfferedAsync(
            """
            class C
            {
                event EventHandler E
                {
                    add { ev += value; }
                    remove { [|ev|] -= value; }
                }
            }
Repository: dotnet/roslyn
File: src/Compilers/CSharp/Portable/BoundTree/BoundExpression.cs (C#)

Code before:
            : this(syntax, localSymbol, BoundLocalDeclarationKind.None, constantValueOpt, false, type, hasErrors)
        {
        }

        public BoundLocal Update(LocalSymbol localSymbol, ConstantValue? constantValueOpt, TypeSymbol type)
        {
            return this.Update(localSymbol, this.DeclarationKind, constantValueOpt, this.IsNullableUnknown, type);
        }
    }

    internal partial class BoundFieldAccess
    {
        public override Symbol? ExpressionSymbol
        {
            get { return this.FieldSymbol; }
        }
    }

    internal partial class BoundPropertyAccess
    {
        public override Symbol? ExpressionSymbol
        {
            get { return this.PropertySymbol; }
        }

        public BoundPropertyAccess Clone()
            => (BoundPropertyAccess)this.MemberwiseClone();
    }

    internal enum AccessorKind : byte
    {
        Unknown,
        Get,
        Set,
        Both
    }

    internal partial class BoundIndexerAccess
    {
        public override Symbol? ExpressionSymbol
        {
            get { return this.Indexer; }
        }

        public override LookupResultKind ResultKind
        {
            get
            {
                return !this.OriginalIndexersOpt.IsDefault ? LookupResultKind.OverloadResolutionFailure : base.ResultKind;
            }
        }

Review comment (suggestion):
>public BoundPropertyAccess Clone() [](http://example.com/codeflow?start=8&length=34)

I forgot when do we prefer to add APIs in this file vs. into a partial declaration dedicated for the specific node type.

Code after:
            : this(syntax, localSymbol, BoundLocalDeclarationKind.None, constantValueOpt, false, type, hasErrors)
        {
        }

        public BoundLocal Update(LocalSymbol localSymbol, ConstantValue? constantValueOpt, TypeSymbol type)
        {
            return this.Update(localSymbol, this.DeclarationKind, constantValueOpt, this.IsNullableUnknown, type);
        }
    }

    internal partial class BoundFieldAccess
    {
        public override Symbol? ExpressionSymbol
        {
            get { return this.FieldSymbol; }
        }
    }

    internal partial class BoundPropertyAccess
    {
        public override Symbol? ExpressionSymbol
        {
            get { return this.PropertySymbol; }
        }
    }

    internal enum AccessorKind : byte
    {
        Unknown,
        Get,
        Set,
        Both
    }

    internal partial class BoundIndexerAccess
    {
        public override Symbol? ExpressionSymbol
        {
            get { return this.Indexer; }
        }

        public override LookupResultKind ResultKind
        {
            get
            {
                return !this.OriginalIndexersOpt.IsDefault ? LookupResultKind.OverloadResolutionFailure : base.ResultKind;
            }
        }

        public BoundIndexerAccess Update(AccessorKind accessorKind)
        {
Repository: dotnet/roslyn
File: src/Workspaces/Core/Portable/Shared/Extensions/ISolutionExtensions.cs (C#)

Code before:
#nullable enable
            }
        }

        return results.ToImmutableAndFree();
    }

    public static TextDocumentKind? GetDocumentKind(this Solution solution, DocumentId documentId)
        => solution.GetTextDocument(documentId)?.Kind;

    public static Solution WithTextDocumentText(this Solution solution, DocumentId documentId, SourceText text, PreservationMode mode = PreservationMode.PreserveIdentity)
    {
        var documentKind = solution.GetDocumentKind(documentId);
        switch (documentKind)
        {
            case TextDocumentKind.Document:
                return solution.WithDocumentText(documentId, text, mode);

            case TextDocumentKind.AnalyzerConfigDocument:
                return solution.WithAnalyzerConfigDocumentText(documentId, text, mode);

            case TextDocumentKind.AdditionalDocument:
                return solution.WithAdditionalDocumentText(documentId, text, mode);

            case null:
                throw ISolutionExtensions.CreateDocumentNotFoundException(documentId.DebugName ?? "Unknown");

            default:
                throw ExceptionUtilities.UnexpectedValue(documentKind);
        }
    }

    public static Workspace? TryGetWorkspace(this Solution solution)
        => solution.WorkspaceKind == WorkspaceKind.RemoteWorkspace
            ? null
            : solution.Workspace;
}


Review comment (suggestion):
@copilot should not need to pass anything in except a basic string into `CreateDocumentNotFoundException`.  The body of that can check if it got null and use `"Unknown"` internally. This applies to all callsites of CreateDocumentNotFoundException

Code after:
#nullable enable
            }
        }

        return results.ToImmutableAndFree();
    }

    public static TextDocumentKind? GetDocumentKind(this Solution solution, DocumentId documentId)
        => solution.GetTextDocument(documentId)?.Kind;

    public static Solution WithTextDocumentText(this Solution solution, DocumentId documentId, SourceText text, PreservationMode mode = PreservationMode.PreserveIdentity)
    {
        var documentKind = solution.GetDocumentKind(documentId);
        switch (documentKind)
        {
            case TextDocumentKind.Document:
                return solution.WithDocumentText(documentId, text, mode);

            case TextDocumentKind.AnalyzerConfigDocument:
                return solution.WithAnalyzerConfigDocumentText(documentId, text, mode);

            case TextDocumentKind.AdditionalDocument:
                return solution.WithAdditionalDocumentText(documentId, text, mode);

            case null:
                throw CreateDocumentNotFoundException(documentId);

            default:
                throw ExceptionUtilities.UnexpectedValue(documentKind);
        }
    }

    public static Workspace? TryGetWorkspace(this Solution solution)
        => solution.WorkspaceKind == WorkspaceKind.RemoteWorkspace
            ? null
            : solution.Workspace;
}
Repository: dotnet/roslyn
File: src/Analyzers/CSharp/Tests/UseCoalesceExpression/UseCoalesceExpressionForIfNullStatementCheckTests.cs (C#)

Code before:

                unsafe void* FindItem() => null;
            }
            """
        }.RunAsync();

    [Fact, WorkItem("https://github.com/dotnet/roslyn/issues/81987")]
    public Task TestPointer2_B()
        => new VerifyCS.Test
        {
            TestCode = """
            class C
            {
                unsafe void M(void* item)
                {
                    item = FindItem();
                    if (item == null)
                        throw new System.InvalidOperationException();
                }

                unsafe void* FindItem() => null;
            }
            """
        }.RunAsync();

    [Fact]
    public Task TestNotOfferedWithDirectivesOnIfStatement_LeadingPragma()
        => new VerifyCS.Test
        {
            TestCode = """
            class C
            {
                public void M()
                {
            #pragma warning disable
                    var value = M2();
                    // Test
            #pragma warning restore
                    if (value == null)
                    {
                        throw new System.InvalidOperationException();
                    }
                }

                string? M2() => null;
            }
            """
        }.RunAsync();

    [Fact]
    public Task TestNotOfferedWithDirectivesOnIfStatement_LeadingRegion()

Review comment (suggestion):
@copilot  add `[Fact, WorkItem("https://github.com/dotnet/roslyn/issues/82037")]` to all the tests you added.

Code after:

                unsafe void* FindItem() => null;
            }
            """
        }.RunAsync();

    [Fact, WorkItem("https://github.com/dotnet/roslyn/issues/81987")]
    public Task TestPointer2_B()
        => new VerifyCS.Test
        {
            TestCode = """
            class C
            {
                unsafe void M(void* item)
                {
                    item = FindItem();
                    if (item == null)
                        throw new System.InvalidOperationException();
                }

                unsafe void* FindItem() => null;
            }
            """
        }.RunAsync();

    [Fact, WorkItem("https://github.com/dotnet/roslyn/issues/82037")]
    public Task TestNotOfferedWithDirectivesOnIfStatement_LeadingPragma()
        => new VerifyCS.Test
        {
            TestCode = """
            #nullable enable
            class C
            {
                public void M()
                {
            #pragma warning disable
                    var value = M2();
                    // Test
            #pragma warning restore
                    if (value == null)
                    {
                        throw new System.InvalidOperationException();
                    }
                }

                string? M2() => null;
            }
            """
        }.RunAsync();

    [Fact, WorkItem("https://github.com/dotnet/roslyn/issues/82037")]
Repository: dotnet/roslyn
File: src/Analyzers/CSharp/Tests/UseCoalesceExpression/UseCoalesceExpressionForIfNullStatementCheckTests.cs (C#)

Code before:

    [Fact]
    public Task TestNotOfferedWithDirectivesOnIfStatement_LeadingRegion()
        => new VerifyCS.Test
        {
            TestCode = """
            class C
            {
                public void M()
                {
                    var value = M2();
            #region Test
                    if (value == null)
                    {
                        throw new System.InvalidOperationException();
                    }
            #endregion
                }

                string? M2() => null;
            }
            """
        }.RunAsync();

    [Fact]
    public Task TestNotOfferedWithDirectivesOnIfStatement_TrailingDefine()
        => new VerifyCS.Test
        {
            TestCode = """
            class C
            {
                public void M()
                {
                    var value = M2();
                    if (value == null)
            #define TEST
                    {
                        throw new System.InvalidOperationException();
                    }
                }

                string? M2() => null;
            }
            """
        }.RunAsync();
}


Review comment (suggestion):
@copilot it's ok if this case passes.  it doesn't represent a pattern we expect users to have that we care about working around.

Code after:
        }.RunAsync();

    [Fact, WorkItem("https://github.com/dotnet/roslyn/issues/82037")]
    public Task TestNotOfferedWithDirectivesOnIfStatement_LeadingRegion()
        => new VerifyCS.Test
        {
            TestCode = """
            #nullable enable
            class C
            {
                public void M()
                {
                    var value = M2();
            #region Test
                    if (value == null)
                    {
                        throw new System.InvalidOperationException();
                    }
            #endregion
                }

                string? M2() => null;
            }
            """
        }.RunAsync();

    [Fact, WorkItem("https://github.com/dotnet/roslyn/issues/82037")]
    public Task TestPreprocessorInBlock1()
        => new VerifyCS.Test
        {
            TestCode = """
            #nullable enable
            class C
            {
                public void M()
                {
                    var value = M2();
                    [|if|] (value == null)
                    {
            #if true
                        throw new System.InvalidOperationException();
            #endif
                    }
                }

                string? M2() => null;
            }
            """,
            FixedCode = """
            #nullable enable
            class C
Repository: dotnet/roslyn
File: src/Compilers/Core/Portable/InternalUtilities/RoslynExperiments.cs (C#)

Code before:
﻿// Licensed to the .NET Foundation under one or more agreements.
// The .NET Foundation licenses this file to you under the MIT license.
// See the LICENSE file in the project root for more information.

namespace Microsoft.CodeAnalysis;

/// <summary>
/// Defines diagnostic info for Roslyn experimental APIs.
/// </summary>
internal static class RoslynExperiments
{
    internal const string NullableDisabledSemanticModel = "RSEXPERIMENTAL001";
    internal const string NullableDisabledSemanticModel_Url = "https://github.com/dotnet/roslyn/issues/70609";

    internal const string GeneratorHostOutputs = "RSEXPERIMENTAL004";
    internal const string GeneratorHostOutputs_Url = "https://github.com/dotnet/roslyn/issues/74753";

    // The UrlFormat property is customized per-api to point at the test plan for the feature, not a single general issue.
    internal const string PreviewLanguageFeatureApi = "RSEXPERIMENTAL006";

    // Previously taken: RSEXPERIMENTAL003 - https://github.com/dotnet/roslyn/issues/73002 (SyntaxTokenParser)
    // Previously taken: RSEXPERIMENTAL005 - https://github.com/dotnet/roslyn/issues/77697
}


Review comment (suggestion):
>to point at the test plan for the feature [](http://example.com/codeflow?start=52&length=41)

I think we agreed to use a dedicated issue, not a test plan issue #Closed

Code after:
﻿// Licensed to the .NET Foundation under one or more agreements.
// The .NET Foundation licenses this file to you under the MIT license.
// See the LICENSE file in the project root for more information.

namespace Microsoft.CodeAnalysis;

/// <summary>
/// Defines diagnostic info for Roslyn experimental APIs.
/// </summary>
internal static class RoslynExperiments
{
    internal const string NullableDisabledSemanticModel = "RSEXPERIMENTAL001";
    internal const string NullableDisabledSemanticModel_Url = "https://github.com/dotnet/roslyn/issues/70609";

    internal const string GeneratorHostOutputs = "RSEXPERIMENTAL004";
    internal const string GeneratorHostOutputs_Url = "https://github.com/dotnet/roslyn/issues/74753";

    // The UrlFormat property is customized per-api to point at a public API tracking issue for the feature, not a single general issue.
    internal const string PreviewLanguageFeatureApi = "RSEXPERIMENTAL006";

    // Previously taken: RSEXPERIMENTAL003 - https://github.com/dotnet/roslyn/issues/73002 (SyntaxTokenParser)
    // Previously taken: RSEXPERIMENTAL005 - https://github.com/dotnet/roslyn/issues/77697
}