ronantakizawa/codereview-bench huggingface dataset:
Repository: dotnet/roslyn
File: src/Compilers/CSharp/Portable/Binder/SingleLookupResult.cs (C#)

Code before:
using Microsoft.CodeAnalysis.Text;

namespace Microsoft.CodeAnalysis.CSharp
{
    /// <summary>
    /// Represents a result of lookup operation over a 0 or 1 symbol (as opposed to a scope). The
    /// typical use is to represent that a particular symbol is good/bad/unavailable.
    /// 
    /// For more explanation of Kind, Symbol, Error - see LookupResult.
    /// </summary>
    internal readonly struct SingleLookupResult
    {
        // the kind of result.
        internal readonly LookupResultKind Kind;

        // the symbol or null.
        internal readonly Symbol? Symbol;

        // the error of the result, if it is NonViable or Inaccessible
        internal readonly DiagnosticInfo? Error;

        internal SingleLookupResult(LookupResultKind kind, Symbol? symbol, DiagnosticInfo? error)
        {
            this.Kind = kind;
            this.Symbol = symbol;
            this.Error = error;
        }
    }
}


Review comment (suggestion):
>this.Kind = kind; [](http://example.com/codeflow?start=12&length=17)

Consider adding an assert that `symbol` is not null unless `kind` is empty. #Closed

Code after:
using Microsoft.CodeAnalysis.Text;
using System.Diagnostics;

namespace Microsoft.CodeAnalysis.CSharp
{
    /// <summary>
    /// Represents a result of lookup operation over a 0 or 1 symbol (as opposed to a scope). The
    /// typical use is to represent that a particular symbol is good/bad/unavailable.
    /// 
    /// For more explanation of Kind, Symbol, Error - see LookupResult.
    /// </summary>
    internal readonly struct SingleLookupResult
    {
        // the kind of result.
        internal readonly LookupResultKind Kind;

        // the symbol or null.
        internal readonly Symbol? Symbol;

        // the error of the result, if it is NonViable or Inaccessible
        internal readonly DiagnosticInfo? Error;

        internal SingleLookupResult(LookupResultKind kind, Symbol? symbol, DiagnosticInfo? error)
        {
            Debug.Assert(symbol is not null || kind == LookupResultKind.Empty);
            this.Kind = kind;
            this.Symbol = symbol;
            this.Error = error;
        }
    }
}
Repository: dotnet/roslyn
File: src/Workspaces/SharedUtilitiesAndExtensions/Compiler/Extensions/Symbols/IMethodSymbolExtensions.cs (C#)

Code before:
    /// <summary>
    /// Returns true if the method is a primary constructor.
    /// Primary constructors are not implicitly declared and have their declaring syntax reference
    /// on the type declaration itself (not a separate constructor declaration).
    /// </summary>
    public static bool IsPrimaryConstructor(this IMethodSymbol constructor)
    {
        if (constructor.IsImplicitlyDeclared)
            return false;

        if (constructor.DeclaringSyntaxReferences is not [{ SyntaxTree: var constructorSyntaxTree, Span: var constructorSpan }])
            return false;

        // Primary constructors have their declaring syntax on the containing type's declaration
        var containingType = constructor.ContainingType;        
        if (containingType.DeclaringSyntaxReferences.Length == 0)
            return false;

        // Check if any of the containing type's syntax references match the constructor's syntax tree
        // and are at the same location (same syntax node)
        foreach (var typeRef in containingType.DeclaringSyntaxReferences)
        {
            if (typeRef.SyntaxTree == constructorSyntaxTree && typeRef.Span == constructorSpan)
            {
                return true;
            }
        }

        return false;
    }
}


Review comment (suggestion):
```suggestion
            if (typeRef.SyntaxTree == constructorSyntaxTree && typeRef.Span == constructorSpan)
                return true;
```

Code after:
    /// <summary>
    /// Returns true if the method is a primary constructor.
    /// Primary constructors are not implicitly declared and have their declaring syntax reference
    /// on the type declaration itself (not a separate constructor declaration).
    /// </summary>
    public static bool IsPrimaryConstructor(this IMethodSymbol constructor)
    {
        if (constructor.IsImplicitlyDeclared)
            return false;

        if (constructor.DeclaringSyntaxReferences is not [{ SyntaxTree: var constructorSyntaxTree, Span: var constructorSpan }])
            return false;

        // Primary constructors have their declaring syntax on the containing type's declaration
        var containingType = constructor.ContainingType;
        if (containingType.DeclaringSyntaxReferences.Length == 0)
            return false;

        // Check if any of the containing type's syntax references match the constructor's syntax tree
        // and are at the same location (same syntax node)
        foreach (var typeRef in containingType.DeclaringSyntaxReferences)
        {
            if (typeRef.SyntaxTree == constructorSyntaxTree && typeRef.Span == constructorSpan)
                return true;
        }

        return false;
    }
}
Repository: dotnet/roslyn
File: src/EditorFeatures/CSharpTest/ReassignedVariable/CSharpReassignedVariableTests.cs (C#)

Code before:
                    Foo();
                    Console.WriteLine($"{[|b|]} {[|c|]}");
                    
                    void Foo()
                    {
                        [|b|] = 2;
                        if (Environment.TickCount > 12345)
                            [|c|] = 1;
                        else
                            [|c|] = 2;
                    }
                }
            }
            """);

    [Fact]
    public Task TestDeconstructionReassignedInLocalFunction_MixedWithRegular()
        => TestAsync(
            """
            using System;
            class C
            {
                void M()
                {
                    var [|a|] = 0;
                    var ([|b|], [|c|]) = (0, 0);
                    
                    Foo();
                    Console.WriteLine($"{[|a|]} {[|b|]} {[|c|]}");
                    
                    void Foo()
                    {
                        [|a|] = 1;
                        [|b|] = 2;
                        if (Environment.TickCount > 12345)
                            [|c|] = 1;
                        else
                            [|c|] = 2;
                    }
                }
            }
            """);

    [Fact]
    public Task TestTopLevelNotReassigned()
        => TestAsync(
            """
            int p;
            p = 0;
            Console.WriteLine(p);
            """);

Review comment (suggestion):
@copilot add tests with code like:

```c#
(int b, int c) = (0, 0);
```

and

```c#
        var (a, (b, c)) = (1, (2, 3));
```

Code after:
                    Foo();
                    Console.WriteLine($"{[|b|]} {[|c|]}");
                    
                    void Foo()
                    {
                        [|b|] = 2;
                        if (Environment.TickCount > 12345)
                            [|c|] = 1;
                        else
                            [|c|] = 2;
                    }
                }
            }
            """);

    [Fact]
    public Task TestDeconstructionReassignedInLocalFunction_MixedWithRegular()
        => TestAsync(
            """
            using System;
            class C
            {
                void M()
                {
                    var [|a|] = 0;
                    var ([|b|], [|c|]) = (0, 0);
                    
                    Foo();
                    Console.WriteLine($"{[|a|]} {[|b|]} {[|c|]}");
                    
                    void Foo()
                    {
                        [|a|] = 1;
                        [|b|] = 2;
                        if (Environment.TickCount > 12345)
                            [|c|] = 1;
                        else
                            [|c|] = 2;
                    }
                }
            }
            """);

    [Fact]
    public Task TestDeconstructionReassignedInLocalFunction_ExplicitType()
        => TestAsync(
            """
            using System;
            class C
            {
                void M()
Repository: dotnet/roslyn
File: src/Compilers/CSharp/Test/Symbol/Symbols/Source/MethodTests.cs (C#)

Code before:
namespace N1
{
    partial class C1
    {
        partial void PartialM() { }
    }
}";

            var comp = CreateCompilation(new[] { source, source2 });
            comp.VerifyDiagnostics();

            var methodSymbols = comp.GetSymbolsWithName("PartialM");
            Assert.Equal(1, methodSymbols.Count());

            var method = methodSymbols.First() as IMethodSymbol;
            Assert.NotNull(method);

            // For partial methods, Locations and DeclaringSyntaxReferences contain only one location
            Assert.Equal(1, method.Locations.Length);
            Assert.Equal(1, method.DeclaringSyntaxReferences.Length);

            // The single location is the definition part
            Assert.True(method.IsPartialDefinition);
            Assert.Null(method.PartialDefinitionPart);
            Assert.NotNull(method.PartialImplementationPart);

            // To get all locations, you need to use PartialImplementationPart
            var implementationPart = method.PartialImplementationPart;
            Assert.Equal(1, implementationPart.Locations.Length);
            Assert.Equal(1, implementationPart.DeclaringSyntaxReferences.Length);

            // Verify the locations are different
            Assert.NotEqual(method.Locations[0], implementationPart.Locations[0]);
        }
    }
}


Review comment (suggestion):
By naming the sources when creating the compilation (`CreateCompilation([(source, "source"), (source2, "source2")])`) it should be easy to observe each location #Closed

Code after:
                        partial void PartialM() { }
                    }
                }
                """;

            var comp = CreateCompilation([(source1, "source1"), (source2, "source2")]);
            comp.VerifyDiagnostics();

            var method = (IMethodSymbol)comp.GetSymbolsWithName("PartialM").Single();

            // For partial methods, Locations and DeclaringSyntaxReferences contain only one location
            Assert.Equal(1, method.Locations.Length);
            Assert.Equal(1, method.DeclaringSyntaxReferences.Length);

            // The single location is the definition part
            Assert.True(method.IsPartialDefinition);
            Assert.Null(method.PartialDefinitionPart);
            Assert.NotNull(method.PartialImplementationPart);

            // To get all locations, you need to use PartialImplementationPart
            var implementationPart = method.PartialImplementationPart;
            Assert.Equal(1, implementationPart.Locations.Length);
            Assert.Equal(1, implementationPart.DeclaringSyntaxReferences.Length);

            // Verify the locations are different
            Assert.NotEqual(method.Locations[0], implementationPart.Locations[0]);

            Assert.Equal("source1", method.Locations[0].SourceTree.FilePath);
            Assert.Equal("source2", implementationPart.Locations[0].SourceTree.FilePath);
        }
    }
}
Repository: dotnet/roslyn
File: src/Compilers/CSharp/Test/Symbol/Symbols/GenericConstraintTests.cs (C#)

Code before:
            var source = """
                interface IMyInterface
                {
                    void Method();
                }
                """;
            var comp = CreateCompilation(source);

            var interfaceType = (INamedTypeSymbol)comp.GetTypeByMetadataName("IMyInterface").GetPublicSymbol();
            Assert.Null(interfaceType.BaseType);
        }

        [Fact, WorkItem("https://github.com/dotnet/roslyn/issues/41733")]
        public void PointerType_BaseType_ReturnsNull()
        {
            var source = """
                unsafe class C
                {
                    int* ptr;
                }
                """;
            var comp = CreateCompilation(source, options: TestOptions.UnsafeDebugDll);

            var pointerType = (IPointerTypeSymbol)comp.GetTypeByMetadataName("C").GetField("ptr").Type.GetPublicSymbol();
            Assert.Null(pointerType.BaseType);
        }
    }
}


Review comment (bug):
@copilot Restore the `VerifyEmitDiagnostics()` checks on compilations in every test you added. If the compilation had errors (not necessarily trivial for human reviewers to spot) that might cause the behavior observed below (null BaseType) for incorrect reasons.

Code after:
        [Fact, WorkItem("https://github.com/dotnet/roslyn/issues/41733")]
        public void InterfaceType_BaseType_ReturnsNull()
        {
            var source = """
                interface IMyInterface
                {
                    void Method();
                }
                """;
            var comp = CreateCompilation(source);
            comp.VerifyDiagnostics();

            var interfaceType = (INamedTypeSymbol)comp.GetTypeByMetadataName("IMyInterface").GetPublicSymbol();
            Assert.Null(interfaceType.BaseType);
        }

        [Fact, WorkItem("https://github.com/dotnet/roslyn/issues/41733")]
        public void PointerType_BaseType_ReturnsNull()
        {
            var source = """
                unsafe class C
                {
                    int* ptr;
                }
                """;
            var comp = CreateCompilation(source, options: TestOptions.UnsafeDebugDll);
            comp.VerifyDiagnostics(
                // (3,10): warning CS0169: The field 'C.ptr' is never used
                //     int* ptr;
                Diagnostic(ErrorCode.WRN_UnreferencedField, "ptr").WithArguments("C.ptr").WithLocation(3, 10));

            var pointerType = (IPointerTypeSymbol)comp.GetTypeByMetadataName("C").GetField("ptr").Type.GetPublicSymbol();
            Assert.Null(pointerType.BaseType);
        }
    }
}
Repository: dotnet/roslyn
File: src/EditorFeatures/CSharpTest2/Recommendations/FileKeywordRecommenderTests.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.

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 FileKeywordRecommenderTests : KeywordRecommenderTests
{
    [Fact]
    public Task TestAtRoot()
        => VerifyKeywordAsync(
@"$$");

    [Fact]
    public Task TestAfterClass()
        => VerifyKeywordAsync(
            """
            class C { }
            $$
            """);

    [Fact]
    public Task TestAfterGlobalStatement()
        => VerifyKeywordAsync(
            """
            System.Console.WriteLine();
            $$
            """);

    [Fact]
    public Task TestAfterGlobalVariableDeclaration()
        => VerifyKeywordAsync(
            """
            int i = 0;
            $$
            """);


Review comment (suggestion):
@copilot this should be indented 12 spaces.  Also applies to a bunch of the tests near the end of this file.

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 FileKeywordRecommenderTests : KeywordRecommenderTests
{
    [Fact]
    public Task TestAtRoot()
        => VerifyKeywordAsync(
            @"$$");

    [Fact]
    public Task TestAfterClass()
        => VerifyKeywordAsync(
            """
            class C { }
            $$
            """);

    [Fact]
    public Task TestNotInUsingAlias()
        => VerifyAbsenceAsync(
            @"using Goo = $$");

    [Fact]
    public Task TestNotInGlobalUsingAlias()
        => VerifyAbsenceAsync(
            @"global using Goo = $$");

    [Fact]
    public Task TestNotInEmptyStatement()
        => VerifyAbsenceAsync(AddInsideMethod(
            @"$$"));

    [Fact]
Repository: dotnet/roslyn
File: src/Features/CSharp/Portable/Completion/KeywordRecommenders/FileKeywordRecommender.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.

using System.Collections.Generic;
using System.Linq;
using System.Threading;
using Microsoft.CodeAnalysis.CSharp.Extensions.ContextQuery;
using Microsoft.CodeAnalysis.CSharp.Utilities;
using Roslyn.Utilities;

namespace Microsoft.CodeAnalysis.CSharp.Completion.KeywordRecommenders;

internal sealed class FileKeywordRecommender() : AbstractSyntacticSingleKeywordRecommender(SyntaxKind.FileKeyword)
{
    private static readonly ISet<SyntaxKind> s_validModifiers = SyntaxKindSet.AllMemberModifiers
        .Where(s => s != SyntaxKind.FileKeyword && !SyntaxFacts.IsAccessibilityModifier(s))
        .ToSet();

    protected override bool IsValidContext(int position, CSharpSyntaxContext context, CancellationToken cancellationToken)
    {
        // Don't offer 'file' after the 'extern' keyword - only 'alias' should be offered there
        if (context.TargetToken.Kind() == SyntaxKind.ExternKeyword)
            return false;

        return context.ContainingTypeDeclaration == null
            && context.IsTypeDeclarationContext(s_validModifiers, SyntaxKindSet.AllTypeDeclarations, canBePartial: true, cancellationToken);
    }
}


Review comment (suggestion):
@copilot would it make more sense to put this check in the list of s_validModifiers above?  We already do not allow 'file' if there is already 'file' or there is an accessibility modifier.  So we likely just want to ad dthis as one more case that should prevent things.

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.Collections.Generic;
using System.Linq;
using System.Threading;
using Microsoft.CodeAnalysis.CSharp.Extensions.ContextQuery;
using Microsoft.CodeAnalysis.CSharp.Utilities;
using Roslyn.Utilities;

namespace Microsoft.CodeAnalysis.CSharp.Completion.KeywordRecommenders;

internal sealed class FileKeywordRecommender() : AbstractSyntacticSingleKeywordRecommender(SyntaxKind.FileKeyword)
{
    private static readonly ISet<SyntaxKind> s_validModifiers = SyntaxKindSet.AllMemberModifiers
        .Where(s => s != SyntaxKind.FileKeyword && s != SyntaxKind.ExternKeyword && !SyntaxFacts.IsAccessibilityModifier(s))
        .ToSet();

    protected override bool IsValidContext(int position, CSharpSyntaxContext context, CancellationToken cancellationToken)
    {
        return context.ContainingTypeDeclaration == null
            && context.IsTypeDeclarationContext(s_validModifiers, SyntaxKindSet.AllTypeDeclarations, canBePartial: true, cancellationToken);
    }
}
Repository: dotnet/roslyn
File: src/Compilers/CSharp/Test/Semantic/Semantics/MethodTypeInferenceTests.cs (C#)

Code before:
        public void InterlockedExchangeWithNullableTypes()
        {
            var source = """
                #nullable enable
                using System.Threading;

                namespace InterlockedExchangeNullProblem
                {
                    public class Class1
                    {
                        public void Method()
                        {
                            object? o = new object();
                            var o1 = Interlocked.Exchange(ref o, null);

                            class2? c = new class2();
                            var c1 = Interlocked.Exchange(ref c, null);
                            var c2 = Interlocked.Exchange<class2?>(ref c, null);
                        }
                    }
                    public class class2 { }
                }
                """;

            var comp = CreateCompilation(source);
            comp.VerifyDiagnostics();
        }
    }
}


Review comment (suggestion):
```suggestion
            CreateCompilation(source).VerifyDiagnostics();
```

Code after:
        public void InterlockedExchangeWithNullableTypes()
        {
            var source = """
                #nullable enable
                using System.Threading;

                namespace InterlockedExchangeNullProblem
                {
                    public class Class1
                    {
                        public void Method()
                        {
                            object? o = new object();
                            var o1 = Interlocked.Exchange(ref o, null);

                            class2? c = new class2();
                            var c1 = Interlocked.Exchange(ref c, null);
                            var c2 = Interlocked.Exchange<class2?>(ref c, null);
                        }
                    }
                    public class class2 { }
                }
                """;

            CreateCompilation(source).VerifyEmitDiagnostics();
        }
    }
}
Repository: dotnet/roslyn
File: src/Compilers/CSharp/Test/Semantic/Semantics/MethodTypeInferenceTests.cs (C#)

Code before:
        [Fact, WorkItem("https://github.com/dotnet/roslyn/issues/49525")]
        public void InterlockedExchangeWithNullableTypes()
        {
            var source = """
                #nullable enable
                using System.Threading;

                namespace InterlockedExchangeNullProblem
                {
                    public class Class1
                    {
                        public void Method()
                        {
                            object? o = new object();
                            var o1 = Interlocked.Exchange(ref o, null);

                            class2? c = new class2();
                            var c1 = Interlocked.Exchange(ref c, null);
                            var c2 = Interlocked.Exchange<class2?>(ref c, null);
                        }
                    }
                    public class class2 { }
                }
                """;

            CreateCompilation(source).VerifyDiagnostics();
        }
    }
}


Review comment (suggestion):
```suggestion
            CreateCompilation(source).VerifyEmitDiagnostics();
```

Code after:
        [Fact, WorkItem("https://github.com/dotnet/roslyn/issues/49525")]
        public void InterlockedExchangeWithNullableTypes()
        {
            var source = """
                #nullable enable
                using System.Threading;

                namespace InterlockedExchangeNullProblem
                {
                    public class Class1
                    {
                        public void Method()
                        {
                            object? o = new object();
                            var o1 = Interlocked.Exchange(ref o, null);

                            class2? c = new class2();
                            var c1 = Interlocked.Exchange(ref c, null);
                            var c2 = Interlocked.Exchange<class2?>(ref c, null);
                        }
                    }
                    public class class2 { }
                }
                """;

            CreateCompilation(source).VerifyEmitDiagnostics();
        }
    }
}