ronantakizawa/codereview-bench huggingface dataset:
Repository: PowerShell/PowerShell
File: src/System.Management.Automation/engine/CommandCompletion/ScopeArgumentCompleter.cs (C#)

Code before:
    /// <summary>
    /// Provides argument completion for Scope parameter.
    /// </summary>
    public class ScopeArgumentCompleter : IArgumentCompleter
    {
        private static readonly string[] s_Scopes = new string[] { "Global", "Local", "Script" };

        /// <summary>
        /// Returns completion results for scope parameter.
        /// </summary>
        /// <param name="commandName">The command name.</param>
        /// <param name="parameterName">The parameter name.</param>
        /// <param name="wordToComplete">The word to complete.</param>
        /// <param name="commandAst">The command AST.</param>
        /// <param name="fakeBoundParameters">The fake bound parameters.</param>
        /// <returns>List of completion results.</returns>
        public IEnumerable<CompletionResult> CompleteArgument(
            string commandName,
            string parameterName,
            string wordToComplete,
            CommandAst commandAst,
            IDictionary fakeBoundParameters)
                => CompletionCompleters.GetMatchingResults(
                    wordToComplete,
                    possibleCompletionValues: s_Scopes,
                    resultType: CompletionResultType.ParameterValue);
    }
}


Review comment (bug):
Adding `CompletionResultType.ParameterValue` look like a bug fix. In the case please move to another PR with new tests. Can we see real difference after changing the parameter from default `Text` to `ParameterValue`?

Code after:
    /// <summary>
    /// Provides argument completion for Scope parameter.
    /// </summary>
    public class ScopeArgumentCompleter : IArgumentCompleter
    {
        private static readonly string[] s_Scopes = new string[] { "Global", "Local", "Script" };

        /// <summary>
        /// Returns completion results for scope parameter.
        /// </summary>
        /// <param name="commandName">The command name.</param>
        /// <param name="parameterName">The parameter name.</param>
        /// <param name="wordToComplete">The word to complete.</param>
        /// <param name="commandAst">The command AST.</param>
        /// <param name="fakeBoundParameters">The fake bound parameters.</param>
        /// <returns>List of completion results.</returns>
        public IEnumerable<CompletionResult> CompleteArgument(
            string commandName,
            string parameterName,
            string wordToComplete,
            CommandAst commandAst,
            IDictionary fakeBoundParameters)
                => CompletionCompleters.GetMatchingResults(
                    wordToComplete,
                    possibleCompletionValues: s_Scopes);
    }
}
Repository: PowerShell/PowerShell
File: test/xUnit/csharp/test_Utils.cs (C#)

Code before:

        [Fact]
        public static void TestConvertToJsonCancellation()
        {
            var source = new CancellationTokenSource();
            var context = new JsonObject.ConvertToJsonContext(
                maxDepth: 1,
                enumsAsStrings: true,
                compressOutput: false,
                Newtonsoft.Json.StringEscapeHandling.Default,
                targetCmdlet: null,
                source.Token);

            source.Cancel();
            Hashtable hash = new Hashtable {
                {"type", CommandTypes.Alias}
            };

            string json = JsonObject.ConvertToJson(hash, in context);
            Assert.Null(json);
        }

        [Fact]
        public static void TestEmptyErrorRecordToString()
        {
            Assert.Equal("", new ErrorRecord(new Exception(""), null, ErrorCategory.NotSpecified, null).ToString());
        }

        [Fact]
        public static void TestNonEmptyErrorRecordToString()
        {
            Assert.Equal("test", new ErrorRecord(new Exception("test"), null, ErrorCategory.NotSpecified, null).ToString());
        }
    }
}


Review comment (bug):
Fix CI fail:
```suggestion
            Assert.Equal(string.Empty, new ErrorRecord(new Exception(string.Empty), null, ErrorCategory.NotSpecified, null).ToString());
```

Code after:

        [Fact]
        public static void TestConvertToJsonCancellation()
        {
            var source = new CancellationTokenSource();
            var context = new JsonObject.ConvertToJsonContext(
                maxDepth: 1,
                enumsAsStrings: true,
                compressOutput: false,
                Newtonsoft.Json.StringEscapeHandling.Default,
                targetCmdlet: null,
                source.Token);

            source.Cancel();
            Hashtable hash = new Hashtable {
                {"type", CommandTypes.Alias}
            };

            string json = JsonObject.ConvertToJson(hash, in context);
            Assert.Null(json);
        }

        [Fact]
        public static void TestEmptyErrorRecordToString()
        {
            Assert.Equal(string.Empty, new ErrorRecord(new Exception(string.Empty), null, ErrorCategory.NotSpecified, null).ToString());
        }

        [Fact]
        public static void TestNonEmptyErrorRecordToString()
        {
            Assert.Equal("test", new ErrorRecord(new Exception("test"), null, ErrorCategory.NotSpecified, null).ToString());
        }
    }
}
Repository: PowerShell/PowerShell
File: src/System.Management.Automation/engine/Modules/GetModuleCommand.cs (C#)

Code before:
                }
            }
        }
    }

    /// <summary>
    /// Provides argument completion for PSEdition parameter.
    /// </summary>
    public class PSEditionArgumentCompleter : IArgumentCompleter
    {
        /// <summary>
        /// Returns completion results for PSEdition parameter.
        /// </summary>
        /// <param name="commandName">The command name.</param>
        /// <param name="parameterName">The parameter name.</param>
        /// <param name="wordToComplete">The word to complete.</param>
        /// <param name="commandAst">The command AST.</param>
        /// <param name="fakeBoundParameters">The fake bound parameters.</param>
        /// <returns>List of completion results.</returns>
        public IEnumerable<CompletionResult> CompleteArgument(
            string commandName,
            string parameterName,
            string wordToComplete,
            CommandAst commandAst,
            IDictionary fakeBoundParameters) 
                => CompletionCompleters.GetMatchingResults(wordToComplete, Utils.AllowedEditionValues);
    }
}


Review comment (suggestion):
```suggestion
                => CompletionCompleters.GetMatchingResults(wordToComplete, possibleCompletionValues: Utils.AllowedEditionValues);
```

Code after:
                }
            }
        }
    }

    /// <summary>
    /// Provides argument completion for PSEdition parameter.
    /// </summary>
    public class PSEditionArgumentCompleter : IArgumentCompleter
    {
        /// <summary>
        /// Returns completion results for PSEdition parameter.
        /// </summary>
        /// <param name="commandName">The command name.</param>
        /// <param name="parameterName">The parameter name.</param>
        /// <param name="wordToComplete">The word to complete.</param>
        /// <param name="commandAst">The command AST.</param>
        /// <param name="fakeBoundParameters">The fake bound parameters.</param>
        /// <returns>List of completion results.</returns>
        public IEnumerable<CompletionResult> CompleteArgument(
            string commandName,
            string parameterName,
            string wordToComplete,
            CommandAst commandAst,
            IDictionary fakeBoundParameters) 
                => CompletionCompleters.GetMatchingResults(wordToComplete, possibleCompletionValues: Utils.AllowedEditionValues);
    }
}
Repository: PowerShell/PowerShell
File: src/System.Management.Automation/engine/CommandCompletion/CompletionHelpers.cs (C#)

Code before:
        /// </param>
        /// <param name="quote">
        /// The quote character to use for enclosing the text. Defaults to a single quote if not provided.
        /// </param>
        /// <param name="escapeSingleQuoteChars">
        /// Indicates whether single quote characters in the text should be escaped. Defaults to <c>true</c>.
        /// </param>
        /// <returns>
        /// The quoted and optionally escaped version of the <paramref name="completionText"/>.
        /// </returns>
        internal static string QuoteCompletionText(
            string completionText,
            string quote,
            bool escapeSingleQuoteChars = true)
        {
            if (!CompletionRequiresQuotes(completionText))
            {
                return quote + completionText + quote;
            }

            string quoteInUse = string.IsNullOrEmpty(quote) ? "'" : quote;

            if (escapeSingleQuoteChars && quoteInUse == "'")
            {
                completionText = CodeGeneration.EscapeSingleQuotedStringContent(completionText);
            }

            return quoteInUse + completionText + quoteInUse;
        }
    }
}


Review comment (security):
In original code single quote is used always. As result we had to escape the single quote. Now if we detect quote type dynamically we should escape dynamically too based on actual quote type. In this case, CompletionRequiresQuotes should be changed too. Therefore, I suggest postponing this until we improve this code as a whole based on the module.

Code after:
        /// </param>
        /// <param name="quote">
        /// The quote character to use for enclosing the text. Defaults to a single quote if not provided.
        /// </param>
        /// <returns>
        /// The quoted <paramref name="completionText"/>.
        /// </returns>
        internal static string QuoteCompletionText(string completionText, string quote)
        {
            if (!CompletionRequiresQuotes(completionText))
            {
                return quote + completionText + quote;
            }

            string quoteInUse = string.IsNullOrEmpty(quote) ? "'" : quote;

            if (quoteInUse == "'")
            {
                completionText = CodeGeneration.EscapeSingleQuotedStringContent(completionText);
            }

            return quoteInUse + completionText + quoteInUse;
        }
    }
}
Repository: PowerShell/PowerShell
File: test/xUnit/csharp/test_CompletionHelpers.cs (C#)

Code before:
// Copyright (c) Microsoft Corporation.
// Licensed under the MIT License.

using System.Management.Automation;
using Xunit;

namespace PSTests.Parallel
{
    public class CompletionHelpersTests
    {
        [Theory]
        [InlineData("", "", "")]
        [InlineData("\"", "", "\"")]
        [InlineData("'", "", "'")]
        [InlineData("\"word\"", "word", "\"")]
        [InlineData("'word'", "word", "'")]
        [InlineData("\"word", "word", "\"")]
        [InlineData("'word", "word", "'")]
        [InlineData("word\"", "word\"", "")]
        [InlineData("word'", "word'", "")]
        [InlineData("\"word's\"", "word's", "\"")]
        [InlineData("'word\"", "'word\"", "")]
        [InlineData("\"word'", "\"word'", "")]
        public void TestHandleDoubleAndSingleQuote(string wordToComplete, string expectedWordToComplete, string expectedQuote)
        {
            string quote = CompletionHelpers.HandleDoubleAndSingleQuote(ref wordToComplete);
            Assert.Equal(expectedQuote, quote);
            Assert.Equal(expectedWordToComplete, wordToComplete);
        }
    }
}


Review comment (suggestion):
One more test:
```c#
        [InlineData("'word\"s'", "word\"s", "'")]
```

Code after:
// Copyright (c) Microsoft Corporation.
// Licensed under the MIT License.

using System.Management.Automation;
using Xunit;

namespace PSTests.Parallel
{
    public class CompletionHelpersTests
    {
        [Theory]
        [InlineData("", "", "")]
        [InlineData("\"", "", "\"")]
        [InlineData("'", "", "'")]
        [InlineData("\"word\"", "word", "\"")]
        [InlineData("'word'", "word", "'")]
        [InlineData("\"word", "word", "\"")]
        [InlineData("'word", "word", "'")]
        [InlineData("word\"", "word\"", "")]
        [InlineData("word'", "word'", "")]
        [InlineData("\"word's\"", "word's", "\"")]
        [InlineData("'word\"", "'word\"", "")]
        [InlineData("\"word'", "\"word'", "")]
        [InlineData("'word\"s'", "word\"s", "'")]
        public void TestHandleDoubleAndSingleQuote(string wordToComplete, string expectedWordToComplete, string expectedQuote)
        {
            string quote = CompletionHelpers.HandleDoubleAndSingleQuote(ref wordToComplete);
            Assert.Equal(expectedQuote, quote);
            Assert.Equal(expectedWordToComplete, wordToComplete);
        }
    }
}
Repository: PowerShell/PowerShell
File: src/Microsoft.PowerShell.Commands.Utility/commands/utility/WebCmdlet/Common/ContentHelper.Common.cs (C#)

Code before:
                }
            }

            return isText;
        }

        internal static bool IsXml([NotNullWhen(true)] string? contentType)
        {
            if (string.IsNullOrEmpty(contentType))
            {
                return false;
            }

            // RFC 3023: Media types with the suffix "+xml" are XML
            bool isXml = contentType.Equals("application/xml", StringComparison.OrdinalIgnoreCase)
                       || contentType.Equals("application/xml-external-parsed-entity", StringComparison.OrdinalIgnoreCase)
                       || contentType.Equals("application/xml-dtd", StringComparison.OrdinalIgnoreCase)
                       || contentType.EndsWith("+xml", StringComparison.OrdinalIgnoreCase);

            return isXml;
        }

        internal static bool IsTextBasedContentType([NotNullWhen(true)] string? contentType) =>
            IsText(contentType)
            || IsJson(contentType)
            || IsXml(contentType);

        #endregion Internal Methods
    }
}


Review comment (suggestion):
Too many lines.
```suggestion
        internal static bool IsTextBasedContentType([NotNullWhen(true)] string? contentType)
            => IsText(contentType) || IsJson(contentType) || IsXml(contentType);
```

Code after:
                            }
                        }
                    }
                }
            }

            return isText;
        }

        internal static bool IsXml([NotNullWhen(true)] string? contentType)
        {
            if (string.IsNullOrEmpty(contentType))
            {
                return false;
            }

            // RFC 3023: Media types with the suffix "+xml" are XML
            bool isXml = contentType.Equals("application/xml", StringComparison.OrdinalIgnoreCase)
                       || contentType.Equals("application/xml-external-parsed-entity", StringComparison.OrdinalIgnoreCase)
                       || contentType.Equals("application/xml-dtd", StringComparison.OrdinalIgnoreCase)
                       || contentType.EndsWith("+xml", StringComparison.OrdinalIgnoreCase);

            return isXml;
        }

        internal static bool IsTextBasedContentType([NotNullWhen(true)] string? contentType)
            => IsText(contentType) || IsJson(contentType) || IsXml(contentType);

        #endregion Internal Methods
    }
}
Repository: PowerShell/PowerShell
File: src/System.Management.Automation/engine/Interop/Windows/NetApiBufferFree.cs (C#)

Code before:
// Copyright (c) Microsoft Corporation.
// Licensed under the MIT License.

#nullable enable

using System.Runtime.InteropServices;

internal static partial class Interop
{
    internal static unsafe partial class Windows
    {

        [LibraryImport("Netapi32.dll")]
        internal static partial int NetApiBufferFree(nint Buffer);
    }
}


Review comment (suggestion):
https://learn.microsoft.com/en-us/openspecs/windows_protocols/ms-dfsnm/a6913914-6f16-4aa4-95eb-d2ad93aabaf5
```suggestion
        internal static partial uint NetApiBufferFree(nint Buffer);
```

Code after:
// Copyright (c) Microsoft Corporation.
// Licensed under the MIT License.

#nullable enable

using System.Runtime.InteropServices;

internal static partial class Interop
{
    internal static unsafe partial class Windows
    {

        [LibraryImport("Netapi32.dll")]
        internal static partial uint NetApiBufferFree(nint Buffer);
    }
}
Repository: PowerShell/PowerShell
File: src/System.Management.Automation/engine/Interop/Windows/NetApiBufferFree.cs (C#)

Code before:
// Copyright (c) Microsoft Corporation.
// Licensed under the MIT License.

#nullable enable

using System.Runtime.InteropServices;

internal static partial class Interop
{
    internal static unsafe partial class Windows
    {

        [LibraryImport("Netapi32.dll")]
        internal static partial uint NetApiBufferFree(IntPtr Buffer);
    }
}


Review comment (suggestion):
```suggestion
        internal static partial uint NetApiBufferFree(nint Buffer);
```

Code after:
// Copyright (c) Microsoft Corporation.
// Licensed under the MIT License.

#nullable enable

using System.Runtime.InteropServices;

internal static partial class Interop
{
    internal static unsafe partial class Windows
    {

        [LibraryImport("Netapi32.dll")]
        internal static partial uint NetApiBufferFree(nint Buffer);
    }
}
Repository: PowerShell/PowerShell
File: src/System.Management.Automation/security/MshSignature.cs (C#)

Code before:
                }
            }

            return message;
        }

        /// <summary>
        /// Extracts the Subject Alternative Name from the certificate.
        /// </summary>
        /// <param name="certificate">The certificate to extract SAN from.</param>
        /// <returns>Formatted SAN string or null if not found.</returns>
        private static string GetSubjectAlternativeName(X509Certificate2 certificate)
        {
            if (certificate == null)
            {
                return null;
            }

            // Look for the Subject Alternative Name extension (OID: 2.5.29.17)
            foreach (X509Extension extension in certificate.Extensions)
            {
                // Check if this is the Subject Alternative Name extension
                if (extension.Oid != null && extension.Oid.Value == "2.5.29.17")
                {
                    // Format the extension with multiline format (parameter 1)
                    return extension.Format(multiLine: true);
                }
            }

            return null;
        }
    }
}


Review comment (suggestion):
Interesting, why is it multiline with prefixes?
If there could be some values I'd expect the type should be `string[]`.

Code after:
            return message;
        }

        /// <summary>
        /// Extracts the Subject Alternative Name from the certificate.
        /// </summary>
        /// <param name="certificate">The certificate to extract SAN from.</param>
        /// <returns>Array of SAN entries or null if not found.</returns>
        private static string[] GetSubjectAlternativeName(X509Certificate2 certificate)
        {
            if (certificate == null)
            {
                return null;
            }

            foreach (X509Extension extension in certificate.Extensions)
            {
                if (extension.Oid != null && extension.Oid.Value == CertificateFilterInfo.SubjectAlternativeNameOid)
                {
                    string formatted = extension.Format(multiLine: true);
                    if (string.IsNullOrEmpty(formatted))
                    {
                        return null;
                    }

                    return formatted.Split(new[] { "\r\n", "\n", "\r" }, StringSplitOptions.RemoveEmptyEntries);
                }
            }

            return null;
        }
    }
}
Repository: PowerShell/PowerShell
File: src/System.Management.Automation/security/MshSignature.cs (C#)

Code before:
            return message;
        }

        /// <summary>
        /// Extracts the Subject Alternative Name from the certificate.
        /// </summary>
        /// <param name="certificate">The certificate to extract SAN from.</param>
        /// <returns>Array of SAN entries or null if not found.</returns>
        private static string[] GetSubjectAlternativeName(X509Certificate2 certificate)
        {
            if (certificate == null)
            {
                return null;
            }

            foreach (X509Extension extension in certificate.Extensions)
            {
                if (extension.Oid != null && extension.Oid.Value == CertificateFilterInfo.SubjectAlternativeNameOid)
                {
                    string formatted = extension.Format(multiLine: true);
                    if (string.IsNullOrEmpty(formatted))
                    {
                        return null;
                    }

                    return formatted.Split(new[] { Environment.NewLine }, StringSplitOptions.RemoveEmptyEntries);
                }
            }

            return null;
        }
    }
}


Review comment (suggestion):
If we used false for multiline we could avoid splitting:
```suggestion
                    return  extension.Format(multiLine: false);
```

Code after:
        /// <summary>
        /// Extracts the Subject Alternative Name from the certificate.
        /// </summary>
        /// <param name="certificate">The certificate to extract SAN from.</param>
        /// <returns>Array of SAN entries or null if not found.</returns>
        private static string[] GetSubjectAlternativeName(X509Certificate2 certificate)
        {
            if (certificate == null)
            {
                return null;
            }

            foreach (X509Extension extension in certificate.Extensions)
            {
                if (extension.Oid != null && extension.Oid.Value == CertificateFilterInfo.SubjectAlternativeNameOid)
                {
                    string formatted = extension.Format(multiLine: true);
                    if (string.IsNullOrEmpty(formatted))
                    {
                        return null;
                    }

                    return formatted.Split(new[] { "\r\n", "\n", "\r" }, StringSplitOptions.RemoveEmptyEntries);
                }
            }

            return null;
        }
    }
}