ronantakizawa/codereview-bench huggingface dataset:
Repository: dotnet/roslyn
File: src/Analyzers/CSharp/Tests/RemoveUnusedMembers/RemoveUnusedMembersTests.cs (C#)

Code before:
        }.RunAsync();
    }

    [Fact, WorkItem("https://github.com/dotnet/roslyn/issues/63892")]
    public async Task FieldIsRead_LambdaDefaultParameter_MultipleParameters()
    {
        var code = """
            class MyClass
            {
                private const int _goo = 42;
                private const int _bar = 10;
                public void M()
                {
                    var lam = (int x = _goo, int y = _bar) => x + y;
                    lam();
                }
            }
            """;

        await new VerifyCS.Test
        {
            TestCode = code,
            FixedCode = code,
            LanguageVersion = LanguageVersion.CSharp12,
        }.RunAsync();
    }

    [Fact]
    public async Task FieldIsRead_Delegate()
    {
        var code = """
            using System;
            class MyClass
            {
                private int _goo;
                public void M()
                {
                    Func<int> getGoo = delegate { return _goo; };
                }
            }
            """;

        await VerifyCS.VerifyCodeFixAsync(code, code);
    }

    [Fact]
    public async Task FieldIsRead_ExpressionBodyLocalFunction()
    {
        var code = """
            class MyClass
            {

Review comment (suggestion):
can you add a test like so:  `var lam = (int x = _goo + _bar) => x + y;`

also `var lam = (int x = MyClass._goo) => x;`

thanks!

Code after:
        }.RunAsync();
    }

    [Fact, WorkItem("https://github.com/dotnet/roslyn/issues/63892")]
    public async Task FieldIsRead_LambdaDefaultParameter_MultipleParameters()
    {
        var code = """
            class MyClass
            {
                private const int _goo = 42;
                private const int _bar = 10;
                public void M()
                {
                    var lam = (int x = _goo, int y = _bar) => x + y;
                    lam();
                }
            }
            """;

        await new VerifyCS.Test
        {
            TestCode = code,
            FixedCode = code,
            LanguageVersion = LanguageVersion.CSharp12,
        }.RunAsync();
    }

    [Fact, WorkItem("https://github.com/dotnet/roslyn/issues/63892")]
    public async Task FieldIsRead_LambdaDefaultParameter_BinaryExpression()
    {
        var code = """
            class MyClass
            {
                private const int _goo = 42;
                private const int _bar = 10;
                public void M()
                {
                    var lam = (int x = _goo + _bar) => x;
                    lam();
                }
            }
            """;

        await new VerifyCS.Test
        {
            TestCode = code,
            FixedCode = code,
            LanguageVersion = LanguageVersion.CSharp12,
        }.RunAsync();
    }
Repository: dotnet/roslyn
File: src/Compilers/Core/MSBuildTask/ManagedToolTask.cs (C#)

Code before:
        protected override bool ValidateParameters()
        {
            // Set DOTNET_ROOT so that the apphost executables launch properly.
            // Unset all other DOTNET_ROOT* variables so for example DOTNET_ROOT_X64 does not override ours.
            if (RuntimeHostInfo.GetToolDotNetRoot() is { } dotNetRoot)
            {
                Log.LogMessage("Setting {0} to '{1}'", RuntimeHostInfo.DotNetRootEnvironmentName, dotNetRoot);
                EnvironmentVariables =
                [
                    .. EnvironmentVariables?.Where(static e => !e.StartsWith(RuntimeHostInfo.DotNetRootEnvironmentName, StringComparison.OrdinalIgnoreCase)) ?? [],
                    .. Environment.GetEnvironmentVariables().Cast<System.Collections.DictionaryEntry>()
                        .Where(e => ((string)e.Key).StartsWith(RuntimeHostInfo.DotNetRootEnvironmentName, StringComparison.OrdinalIgnoreCase))
                        .Select(e => $"{e.Key}="),
                    $"{RuntimeHostInfo.DotNetRootEnvironmentName}={dotNetRoot}",
                ];
            }

            return base.ValidateParameters();
        }

        internal static class TestAccessor
        {
            public static void SetUseAppHost(ManagedToolTask task, bool value)
            {
                task._useAppHost = value;
            }
        }
    }
}


Review comment (suggestion):
Rather than a `TestAccessor` consider just exposing as an `internal` property. 

Code after:
                // through these members.
                throw new InvalidOperationException("Unable to determine the location of the build task assembly.");
            }

            return buildTaskDirectory;
        }

        protected override bool ValidateParameters()
        {
            // Set DOTNET_ROOT so that the apphost executables launch properly.
            // Unset all other DOTNET_ROOT* variables so for example DOTNET_ROOT_X64 does not override ours.
            if (RuntimeHostInfo.GetToolDotNetRoot() is { } dotNetRoot)
            {
                Log.LogMessage("Setting {0} to '{1}'", RuntimeHostInfo.DotNetRootEnvironmentName, dotNetRoot);
                EnvironmentVariables =
                [
                    .. EnvironmentVariables?.Where(static e => !e.StartsWith(RuntimeHostInfo.DotNetRootEnvironmentName, StringComparison.OrdinalIgnoreCase)) ?? [],
                    .. Environment.GetEnvironmentVariables().Cast<System.Collections.DictionaryEntry>()
                        .Where(e => ((string)e.Key).StartsWith(RuntimeHostInfo.DotNetRootEnvironmentName, StringComparison.OrdinalIgnoreCase))
                        .Select(e => $"{e.Key}="),
                    $"{RuntimeHostInfo.DotNetRootEnvironmentName}={dotNetRoot}",
                ];
            }

            return base.ValidateParameters();
        }
    }
}
Repository: dotnet/roslyn
File: src/EditorFeatures/Core/Extensibility/Commands/PredefinedCommandHandlerNames.cs (C#)

Code before:
    public const string AddImportsPaste = "Add Imports Paste Command Handler";

    /// <summary>
    /// Command handler name for Edit and Continue file save handler.
    /// </summary>
    public const string EditAndContinueFileSave = "Edit and Continue Save File Handler";

    /// <summary>
    /// Command handler name for showing the Value Tracking tool window.
    /// </summary>
    public const string ShowValueTracking = "Show Value Tracking";

    /// <summary>
    /// Command handler name for showing the Callstack Explorer tool window.
    /// </summary>
    public const string ShowCallstackExplorer = "Show Callstack Explorer";

    /// <summary>
    /// Command handler name for LSP Go To Definition.
    /// </summary>
    public const string LspGoToDefinition = "LSP Go To Definition Command Handler";

    /// <summary>
    /// Command handler name for LSP Find References.
    /// </summary>
    public const string LspFindReferences = "LSP Find References Command Handler";
}


Review comment (suggestion):
@copilot use exactly `LSP FindReferenceCommandHandler` as the value.

Code after:
    public const string AddImportsPaste = "Add Imports Paste Command Handler";

    /// <summary>
    /// Command handler name for Edit and Continue file save handler.
    /// </summary>
    public const string EditAndContinueFileSave = "Edit and Continue Save File Handler";

    /// <summary>
    /// Command handler name for showing the Value Tracking tool window.
    /// </summary>
    public const string ShowValueTracking = "Show Value Tracking";

    /// <summary>
    /// Command handler name for showing the Callstack Explorer tool window.
    /// </summary>
    public const string ShowCallstackExplorer = "Show Callstack Explorer";

    /// <summary>
    /// Command handler name for LSP Go To Definition.
    /// This name is agreed upon externally and cannot be changed.
    /// </summary>
    public const string LspGoToDefinition = "LSP GoToDefinitionCommandHandler";

    /// <summary>
    /// Command handler name for LSP Find References.
    /// This name is agreed upon externally and cannot be changed.
    /// </summary>
    public const string LspFindReferences = "LSP FindReferenceCommandHandler";

    /// <summary>
    /// Command handler name for LSP Go To Implementation.
    /// This name is agreed upon externally and cannot be changed.
    /// </summary>
    public const string LspGoToImplementation = "LSP GoToImplementationCommandHandler";
}
Repository: dotnet/roslyn
File: src/EditorFeatures/Core/Extensibility/Commands/PredefinedCommandHandlerNames.cs (C#)

Code before:
    public const string PasteTrackingPaste = "Paste Tracking Paste Command Handler";

    /// <summary>
    /// Command handler name for Paste in Add Imports.
    /// </summary>
    public const string AddImportsPaste = "Add Imports Paste Command Handler";

    /// <summary>
    /// Command handler name for Edit and Continue file save handler.
    /// </summary>
    public const string EditAndContinueFileSave = "Edit and Continue Save File Handler";

    /// <summary>
    /// Command handler name for showing the Value Tracking tool window.
    /// </summary>
    public const string ShowValueTracking = "Show Value Tracking";

    /// <summary>
    /// Command handler name for showing the Callstack Explorer tool window.
    /// </summary>
    public const string ShowCallstackExplorer = "Show Callstack Explorer";

    /// <summary>
    /// Command handler name for LSP Go To Definition.
    /// </summary>
    public const string LspGoToDefinition = "LSP Go To Definition Command Handler";

    /// <summary>
    /// Command handler name for LSP Find References.
    /// </summary>
    public const string LspFindReferences = "LSP Find References Command Handler";
}


Review comment (suggestion):
@copilot use exactly `LSP GoToDefinitionCommandHandler` as the value

Code after:
    public const string PasteTrackingPaste = "Paste Tracking Paste Command Handler";

    /// <summary>
    /// Command handler name for Paste in Add Imports.
    /// </summary>
    public const string AddImportsPaste = "Add Imports Paste Command Handler";

    /// <summary>
    /// Command handler name for Edit and Continue file save handler.
    /// </summary>
    public const string EditAndContinueFileSave = "Edit and Continue Save File Handler";

    /// <summary>
    /// Command handler name for showing the Value Tracking tool window.
    /// </summary>
    public const string ShowValueTracking = "Show Value Tracking";

    /// <summary>
    /// Command handler name for showing the Callstack Explorer tool window.
    /// </summary>
    public const string ShowCallstackExplorer = "Show Callstack Explorer";

    /// <summary>
    /// Command handler name for LSP Go To Definition.
    /// This name is agreed upon externally and cannot be changed.
    /// </summary>
    public const string LspGoToDefinition = "LSP GoToDefinitionCommandHandler";

    /// <summary>
    /// Command handler name for LSP Find References.
    /// This name is agreed upon externally and cannot be changed.
    /// </summary>
    public const string LspFindReferences = "LSP FindReferenceCommandHandler";

    /// <summary>
    /// Command handler name for LSP Go To Implementation.
    /// This name is agreed upon externally and cannot be changed.
    /// </summary>
    public const string LspGoToImplementation = "LSP GoToImplementationCommandHandler";
}
Repository: dotnet/roslyn
File: src/VisualStudio/Core/Def/ProjectSystem/SdkAnalyzerAssemblyRedirector.cs (C#)

Code before:
            var index = directoryPath.LastIndexOf(pathSuffix, StringComparison.OrdinalIgnoreCase);
            if (index < 0)
            {
                return false;
            }

            var directoryPathVersion = Path.GetFileName(Path.GetDirectoryName(directoryPath.Substring(0, index)));

            return AreVersionMajorMinorPartEqual(directoryPathVersion, version);
        }

        static bool AreVersionMajorMinorPartEqual(string version1, string version2)
        {
            var firstDotIndex = version1.IndexOf('.');
            if (firstDotIndex < 0)
            {
                return false;
            }

            var secondDotIndex = version1.IndexOf('.', firstDotIndex + 1);
            if (secondDotIndex < 0)
            {
                return false;
            }

            return 0 == string.Compare(version1, 0, version2, 0, secondDotIndex, StringComparison.OrdinalIgnoreCase);
        }
    }

    private void Log(string message, __ACTIVITYLOG_ENTRYTYPE level = __ACTIVITYLOG_ENTRYTYPE.ALE_INFORMATION)
    {
        _log?.LogEntry(
            (uint)level,
            "Roslyn" + nameof(SdkAnalyzerAssemblyRedirector),
            message);
    }
}

internal sealed class MetadataEntry
{
    public required string Version { get; init; }
    public ImmutableArray<string> Files { get; init; }
}


Review comment (suggestion):
This could match `8.0.1` and `8.01.1` as unlikely as that might be. Perhaps we should include the second `.` in our match
```suggestion
            return 0 == string.Compare(version1, 0, version2, 0, secondDotIndex + 1, StringComparison.OrdinalIgnoreCase);
```

Code after:
            var index = directoryPath.LastIndexOf(pathSuffix, StringComparison.OrdinalIgnoreCase);
            if (index < 0)
            {
                return false;
            }

            var directoryPathVersion = Path.GetFileName(Path.GetDirectoryName(directoryPath.Substring(0, index)));

            return AreVersionMajorMinorPartEqual(directoryPathVersion, version);
        }

        static bool AreVersionMajorMinorPartEqual(string version1, string version2)
        {
            var firstDotIndex = version1.IndexOf('.');
            if (firstDotIndex < 0)
            {
                return false;
            }

            var secondDotIndex = version1.IndexOf('.', firstDotIndex + 1);
            if (secondDotIndex < 0)
            {
                return false;
            }

            return 0 == string.Compare(version1, 0, version2, 0, secondDotIndex + 1, StringComparison.OrdinalIgnoreCase);
        }
    }

    private void Log(string message, __ACTIVITYLOG_ENTRYTYPE level = __ACTIVITYLOG_ENTRYTYPE.ALE_INFORMATION)
    {
        _log?.LogEntry(
            (uint)level,
            "Roslyn" + nameof(SdkAnalyzerAssemblyRedirector),
            message);
    }
}

internal sealed class MetadataEntry
{
    public required string Version { get; init; }
    public ImmutableArray<string> Files { get; init; }
}
Repository: dotnet/roslyn
File: src/Compilers/Core/Portable/Text/SourceTextComparer.cs (C#)

Code before:
    {
        public static readonly SourceTextComparer Instance = new SourceTextComparer();

        public bool Equals(SourceText? x, SourceText? y)
        {
            if (x == null)
            {
                return y == null;
            }
            else if (y == null)
            {
                return false;
            }

            return x.ContentEquals(y);
        }

        public int GetHashCode(SourceText? obj)
        {
            if (obj is null)
            {
                return 0;
            }

            var contentHash = obj.GetContentHash();
            return !contentHash.IsDefault ? Hash.CombineValues(contentHash) : 0;
        }
    }
}


Review comment (suggestion):
@copilot You won't need to check for .IsDefault.  Also, just use `MemoryMarshal.TryRead(contentHash.AsSpan(), ...)` to just read out the first 4 bytes of the already strongly hashed bytes into the int you need.

Code after:
    {
        public static readonly SourceTextComparer Instance = new SourceTextComparer();

        public bool Equals(SourceText? x, SourceText? y)
        {
            if (x == null)
            {
                return y == null;
            }
            else if (y == null)
            {
                return false;
            }

            return x.ContentEquals(y);
        }

        public int GetHashCode(SourceText? obj)
        {
            if (obj is null)
            {
                return 0;
            }

            // GetContentHash returns a 16-byte, well-distributed, xx-hash-128 value.
            // So reading the first 4 bytes as an int is always safe, and will give a good hash code back for this instance.
            var contentHash = obj.GetContentHash();
            return MemoryMarshal.Read<int>(contentHash.AsSpan());
        }
    }
}
Repository: dotnet/roslyn
File: src/Compilers/Core/Portable/Text/SourceTextComparer.cs (C#)

Code before:
        public static readonly SourceTextComparer Instance = new SourceTextComparer();

        public bool Equals(SourceText? x, SourceText? y)
        {
            if (x == null)
            {
                return y == null;
            }
            else if (y == null)
            {
                return false;
            }

            return x.ContentEquals(y);
        }

        public int GetHashCode(SourceText? obj)
        {
            if (obj is null)
            {
                return 0;
            }

            var contentHash = obj.GetContentHash();
            MemoryMarshal.TryRead(contentHash.AsSpan(), out int hashCode);
            return hashCode;
        }
    }
}


Review comment (suggestion):
@copilot actually, can just be `MemoryMarshal.Read<int>(...`  Add comment stating that the content hash is 16 bytes, so reading off the first 4 bytes is always safe.

Code after:
        public static readonly SourceTextComparer Instance = new SourceTextComparer();

        public bool Equals(SourceText? x, SourceText? y)
        {
            if (x == null)
            {
                return y == null;
            }
            else if (y == null)
            {
                return false;
            }

            return x.ContentEquals(y);
        }

        public int GetHashCode(SourceText? obj)
        {
            if (obj is null)
            {
                return 0;
            }

            // GetContentHash returns a 16-byte, well-distributed, xx-hash-128 value.
            // So reading the first 4 bytes as an int is always safe, and will give a good hash code back for this instance.
            var contentHash = obj.GetContentHash();
            return MemoryMarshal.Read<int>(contentHash.AsSpan());
        }
    }
}
Repository: dotnet/roslyn
File: src/Compilers/Core/Portable/Text/SourceTextComparer.cs (C#)

Code before:
    internal class SourceTextComparer : IEqualityComparer<SourceText?>
    {
        public static readonly SourceTextComparer Instance = new SourceTextComparer();

        public bool Equals(SourceText? x, SourceText? y)
        {
            if (x == null)
            {
                return y == null;
            }
            else if (y == null)
            {
                return false;
            }

            return x.ContentEquals(y);
        }

        public int GetHashCode(SourceText? obj)
        {
            if (obj is null)
            {
                return 0;
            }

            // GetContentHash returns a 16-byte hash, so reading the first 4 bytes as an int is always safe.
            var contentHash = obj.GetContentHash();
            return MemoryMarshal.Read<int>(contentHash.AsSpan());
        }
    }
}


Review comment (suggestion):
```suggestion
            // GetContentHash returns a 16-byte, well-distributed, xx-hash-128 value.
            // So reading the first 4 bytes as an int is always safe, and will give a good hash code back for this instance.
```

Code after:
    internal class SourceTextComparer : IEqualityComparer<SourceText?>
    {
        public static readonly SourceTextComparer Instance = new SourceTextComparer();

        public bool Equals(SourceText? x, SourceText? y)
        {
            if (x == null)
            {
                return y == null;
            }
            else if (y == null)
            {
                return false;
            }

            return x.ContentEquals(y);
        }

        public int GetHashCode(SourceText? obj)
        {
            if (obj is null)
            {
                return 0;
            }

            // GetContentHash returns a 16-byte, well-distributed, xx-hash-128 value.
            // So reading the first 4 bytes as an int is always safe, and will give a good hash code back for this instance.
            var contentHash = obj.GetContentHash();
            return MemoryMarshal.Read<int>(contentHash.AsSpan());
        }
    }
}
Repository: dotnet/roslyn
File: src/Features/CSharpTest/IntroduceParameter/IntroduceParameterTests.cs (C#)

Code before:
                    [|a|] = new { },
                };
            }
            """);

    [Fact]
    public Task TestNotOnAnonymousObjectMemberNameWithValue()
        => TestMissingInRegularAndScriptAsync(
            """
            class C
            {
                object M() => new
                {
                    [|x|] = 5,
                };
            }
            """);

    [Fact]
    public Task TestNotOnObjectInitializerMemberName()
        => TestMissingInRegularAndScriptAsync(
            """
            class Foo { public int X { get; set; } }
            class C
            {
                Foo M() => new Foo
                {
                    [|X|] = 5,
                };
            }
            """);

    [Fact]
    public Task TestNotOnObjectInitializerMemberNameNested()
        => TestMissingInRegularAndScriptAsync(
            """
            class Foo { public int X { get; set; } public Foo Inner { get; set; } }
            class C
            {
                Foo M() => new Foo
                {
                    [|X|] = 5,
                    Inner = new Foo { X = 10 }
                };
            }
            """);
}


Review comment (suggestion):
@copilot use 'Goo' instead of 'Foo' in all tests.  
use `[Fact, WorkItem("...")]` for all the tests you added.

Code after:
                {
                    [|a|] = new { },
                };
            }
            """);

    [Fact, WorkItem("https://github.com/dotnet/roslyn/issues/81013")]
    public Task TestNotOnAnonymousObjectMemberNameWithValue()
        => TestMissingInRegularAndScriptAsync(
            """
            class C
            {
                object M() => new
                {
                    [|x|] = 5,
                };
            }
            """);

    [Fact, WorkItem("https://github.com/dotnet/roslyn/issues/81013")]
    public Task TestNotOnObjectInitializerMemberName()
        => TestMissingInRegularAndScriptAsync(
            """
            class Goo { public int X { get; set; } }
            class C
            {
                Goo M() => new Goo
                {
                    [|X|] = 5,
                };
            }
            """);

    [Fact, WorkItem("https://github.com/dotnet/roslyn/issues/81013")]
    public Task TestNotOnObjectInitializerMemberNameNested()
        => TestMissingInRegularAndScriptAsync(
            """
            class Goo { public int X { get; set; } public Goo Inner { get; set; } }
            class C
            {
                Goo M() => new Goo
                {
                    [|X|] = 5,
                    Inner = new Goo { X = 10 }
                };
            }
            """);
}