From e5856d1b759169a110cdcf685aba8c67d932bb8d Mon Sep 17 00:00:00 2001 From: xtqqczze Date: Sun, 1 Nov 2020 18:55:07 +0000 Subject: [PATCH 1/7] Enable CA1825: Avoid zero-length array allocations https://docs.microsoft.com/dotnet/fundamentals/code-analysis/quality-rules/ca1825 --- .globalconfig | 2 +- .../utility/ImplicitRemotingCommands.cs | 2 +- .../commands/utility/MatchString.cs | 8 +-- .../commands/utility/New-Object.cs | 2 +- .../engine/EventManager.cs | 2 +- .../engine/LanguagePrimitives.cs | 2 +- .../engine/Modules/ModuleCmdletBase.cs | 2 +- .../engine/ParameterBinderBase.cs | 2 +- .../engine/PseudoParameters.cs | 2 +- .../engine/hostifaces/LocalConnection.cs | 2 +- .../remoting/host/RemoteHostMethodInfo.cs | 54 +++++++++---------- .../engine/runtime/Operations/ArrayOps.cs | 2 +- .../engine/runtime/Operations/MiscOps.cs | 2 +- .../namespaces/RegistryProvider.cs | 4 +- .../utils/GraphicalHostReflectionWrapper.cs | 8 +-- test/xUnit/csharp/test_CommandLineParser.cs | 6 +-- 16 files changed, 51 insertions(+), 51 deletions(-) diff --git a/.globalconfig b/.globalconfig index 0439bba2947..56dccdce4e8 100644 --- a/.globalconfig +++ b/.globalconfig @@ -286,7 +286,7 @@ dotnet_diagnostic.CA1823.severity = none dotnet_diagnostic.CA1824.severity = suggestion # CA1825: Avoid zero-length array allocations -dotnet_diagnostic.CA1825.severity = suggestion +dotnet_diagnostic.CA1825.severity = warning # CA1826: Do not use Enumerable methods on indexable collections dotnet_diagnostic.CA1826.severity = suggestion diff --git a/src/Microsoft.PowerShell.Commands.Utility/commands/utility/ImplicitRemotingCommands.cs b/src/Microsoft.PowerShell.Commands.Utility/commands/utility/ImplicitRemotingCommands.cs index 87825d2dd1d..a94ea514502 100644 --- a/src/Microsoft.PowerShell.Commands.Utility/commands/utility/ImplicitRemotingCommands.cs +++ b/src/Microsoft.PowerShell.Commands.Utility/commands/utility/ImplicitRemotingCommands.cs @@ -480,7 +480,7 @@ public ModuleSpecification[] FullyQualifiedModule } } - private ModuleSpecification[] _moduleSpecifications = new ModuleSpecification[0]; + private ModuleSpecification[] _moduleSpecifications = Array.Empty(); internal bool IsFullyQualifiedModuleSpecified = false; private bool _commandParameterSpecified; // initialized to default value in the constructor diff --git a/src/Microsoft.PowerShell.Commands.Utility/commands/utility/MatchString.cs b/src/Microsoft.PowerShell.Commands.Utility/commands/utility/MatchString.cs index 667f55e50cf..155a134ca7b 100644 --- a/src/Microsoft.PowerShell.Commands.Utility/commands/utility/MatchString.cs +++ b/src/Microsoft.PowerShell.Commands.Utility/commands/utility/MatchString.cs @@ -366,7 +366,7 @@ private string FormatLine(string lineStr, int displayLineNumber, string displayP /// /// Gets or sets a list of all Regex matches on the matching line. /// - public Match[] Matches { get; set; } = new Match[] { }; + public Match[] Matches { get; set; } = Array.Empty(); /// /// Create a deep copy of this MatchInfo instance. @@ -1897,8 +1897,8 @@ private bool DoMatchWorker(string operandString, MatchInfo matchInfo, out MatchI if (matchInfo.Context != null) { matchResult = matchInfo.Clone(); - matchResult.Context.DisplayPreContext = new string[] { }; - matchResult.Context.DisplayPostContext = new string[] { }; + matchResult.Context.DisplayPreContext = Array.Empty(); + matchResult.Context.DisplayPostContext = Array.Empty(); } else { @@ -1924,7 +1924,7 @@ private bool DoMatchWorker(string operandString, MatchInfo matchInfo, out MatchI // Matches should be an empty list, rather than null, // in the cases of notMatch and simpleMatch. - matchResult.Matches = matches ?? new Match[] { }; + matchResult.Matches = matches ?? Array.Empty(); return true; } diff --git a/src/Microsoft.PowerShell.Commands.Utility/commands/utility/New-Object.cs b/src/Microsoft.PowerShell.Commands.Utility/commands/utility/New-Object.cs index 39fd390b1ce..a8b4b69aa1f 100644 --- a/src/Microsoft.PowerShell.Commands.Utility/commands/utility/New-Object.cs +++ b/src/Microsoft.PowerShell.Commands.Utility/commands/utility/New-Object.cs @@ -207,7 +207,7 @@ protected override void BeginProcessing() ConstructorInfo ci = type.GetConstructor(Type.EmptyTypes); if (ci != null && ci.IsPublic) { - _newObject = CallConstructor(type, new ConstructorInfo[] { ci }, new object[] { }); + _newObject = CallConstructor(type, new ConstructorInfo[] { ci }, Array.Empty()); if (_newObject != null && Property != null) { // The method invocation is disabled for "Hashtable to Object conversion" (Win8:649519), but we need to keep it enabled for New-Object for compatibility to PSv2 diff --git a/src/System.Management.Automation/engine/EventManager.cs b/src/System.Management.Automation/engine/EventManager.cs index 012f5c0857c..b44d99eb3e6 100644 --- a/src/System.Management.Automation/engine/EventManager.cs +++ b/src/System.Management.Automation/engine/EventManager.cs @@ -599,7 +599,7 @@ private void OnElapsedEvent(object source) if (_engineEventSubscribers.TryGetValue(PSEngineEvent.OnIdle, out subscribers) && subscribers.Count > 0) { // We send out on-idle event and keep enabling the timer only if there still are subscribers to the on-idle event - GenerateEvent(PSEngineEvent.OnIdle, null, new object[] { }, null, false, false); + GenerateEvent(PSEngineEvent.OnIdle, null, Array.Empty(), null, false, false); EnableTimer(); } else diff --git a/src/System.Management.Automation/engine/LanguagePrimitives.cs b/src/System.Management.Automation/engine/LanguagePrimitives.cs index 5929a5637e9..a7c1f8c2e19 100644 --- a/src/System.Management.Automation/engine/LanguagePrimitives.cs +++ b/src/System.Management.Automation/engine/LanguagePrimitives.cs @@ -384,7 +384,7 @@ private void CreateGetEnumerator() emitter.Emit(OpCodes.Ldarg_0); emitter.Emit(OpCodes.Castclass, _enumerableType); - MethodInfo methodInfo = _enumerableType.GetMethod("GetEnumerator", new Type[] { }); + MethodInfo methodInfo = _enumerableType.GetMethod("GetEnumerator", Array.Empty()); emitter.Emit(OpCodes.Callvirt, methodInfo); emitter.Emit(OpCodes.Ret); } diff --git a/src/System.Management.Automation/engine/Modules/ModuleCmdletBase.cs b/src/System.Management.Automation/engine/Modules/ModuleCmdletBase.cs index a70ac75dfc5..b2967ded1ab 100644 --- a/src/System.Management.Automation/engine/Modules/ModuleCmdletBase.cs +++ b/src/System.Management.Automation/engine/Modules/ModuleCmdletBase.cs @@ -6882,7 +6882,7 @@ internal PSModuleInfo LoadBinaryModule(PSModuleInfo parentModule, bool trySnapIn iss.Bind(Context, updateOnly: true, module, options.NoClobber, options.Local, setLocation: false); // Scan all of the types in the assembly to register JobSourceAdapters. - IEnumerable allTypes = new Type[] { }; + IEnumerable allTypes = Array.Empty(); if (assembly != null) { allTypes = assembly.ExportedTypes; diff --git a/src/System.Management.Automation/engine/ParameterBinderBase.cs b/src/System.Management.Automation/engine/ParameterBinderBase.cs index ddf8072b941..b3b56ec6143 100644 --- a/src/System.Management.Automation/engine/ParameterBinderBase.cs +++ b/src/System.Management.Automation/engine/ParameterBinderBase.cs @@ -1559,7 +1559,7 @@ private object EncodeCollection( toType, 0, null, - new object[] { }, + Array.Empty(), System.Globalization.CultureInfo.InvariantCulture); if (collectionTypeInformation.ParameterCollectionType == ParameterCollectionType.IList) resultAsIList = (IList)resultCollection; diff --git a/src/System.Management.Automation/engine/PseudoParameters.cs b/src/System.Management.Automation/engine/PseudoParameters.cs index 78ef034cf5b..97b1d60e8ad 100644 --- a/src/System.Management.Automation/engine/PseudoParameters.cs +++ b/src/System.Management.Automation/engine/PseudoParameters.cs @@ -236,6 +236,6 @@ public string HelpFile /// public object Data { get; set; } - internal static readonly RuntimeDefinedParameter[] EmptyParameterArray = new RuntimeDefinedParameter[0]; + internal static readonly RuntimeDefinedParameter[] EmptyParameterArray = Array.Empty(); } } diff --git a/src/System.Management.Automation/engine/hostifaces/LocalConnection.cs b/src/System.Management.Automation/engine/hostifaces/LocalConnection.cs index 92740acf8dd..61b63015744 100644 --- a/src/System.Management.Automation/engine/hostifaces/LocalConnection.cs +++ b/src/System.Management.Automation/engine/hostifaces/LocalConnection.cs @@ -851,7 +851,7 @@ private void DoCloseHelper() // Generate the shutdown event if (Events != null) - Events.GenerateEvent(PSEngineEvent.Exiting, null, new object[] { }, null, true, false); + Events.GenerateEvent(PSEngineEvent.Exiting, null, Array.Empty(), null, true, false); // Stop all running pipelines // Note:Do not perform the Cancel in lock. Reason is diff --git a/src/System.Management.Automation/engine/remoting/host/RemoteHostMethodInfo.cs b/src/System.Management.Automation/engine/remoting/host/RemoteHostMethodInfo.cs index 63185ebc13f..e24f8961b9a 100644 --- a/src/System.Management.Automation/engine/remoting/host/RemoteHostMethodInfo.cs +++ b/src/System.Management.Automation/engine/remoting/host/RemoteHostMethodInfo.cs @@ -144,35 +144,35 @@ internal static RemoteHostMethodInfo LookUp(RemoteHostMethodId methodId) typeof(PSHost), "get_Name", typeof(string), - new Type[] { }); + Array.Empty()); case RemoteHostMethodId.GetVersion: return new RemoteHostMethodInfo( typeof(PSHost), "get_Version", typeof(Version), - new Type[] { }); + Array.Empty()); case RemoteHostMethodId.GetInstanceId: return new RemoteHostMethodInfo( typeof(PSHost), "get_InstanceId", typeof(Guid), - new Type[] { }); + Array.Empty()); case RemoteHostMethodId.GetCurrentCulture: return new RemoteHostMethodInfo( typeof(PSHost), "get_CurrentCulture", typeof(System.Globalization.CultureInfo), - new Type[] { }); + Array.Empty()); case RemoteHostMethodId.GetCurrentUICulture: return new RemoteHostMethodInfo( typeof(PSHost), "get_CurrentUICulture", typeof(System.Globalization.CultureInfo), - new Type[] { }); + Array.Empty()); // Host methods. @@ -188,28 +188,28 @@ internal static RemoteHostMethodInfo LookUp(RemoteHostMethodId methodId) typeof(PSHost), "EnterNestedPrompt", typeof(void), - new Type[] { }); + Array.Empty()); case RemoteHostMethodId.ExitNestedPrompt: return new RemoteHostMethodInfo( typeof(PSHost), "ExitNestedPrompt", typeof(void), - new Type[] { }); + Array.Empty()); case RemoteHostMethodId.NotifyBeginApplication: return new RemoteHostMethodInfo( typeof(PSHost), "NotifyBeginApplication", typeof(void), - new Type[] { }); + Array.Empty()); case RemoteHostMethodId.NotifyEndApplication: return new RemoteHostMethodInfo( typeof(PSHost), "NotifyEndApplication", typeof(void), - new Type[] { }); + Array.Empty()); // Host UI methods. @@ -218,14 +218,14 @@ internal static RemoteHostMethodInfo LookUp(RemoteHostMethodId methodId) typeof(PSHostUserInterface), "ReadLine", typeof(string), - new Type[] { }); + Array.Empty()); case RemoteHostMethodId.ReadLineAsSecureString: return new RemoteHostMethodInfo( typeof(PSHostUserInterface), "ReadLineAsSecureString", typeof(System.Security.SecureString), - new Type[] { }); + Array.Empty()); case RemoteHostMethodId.Write1: return new RemoteHostMethodInfo( @@ -246,7 +246,7 @@ internal static RemoteHostMethodInfo LookUp(RemoteHostMethodId methodId) typeof(PSHostUserInterface), "WriteLine", typeof(void), - new Type[] { }); + Array.Empty()); case RemoteHostMethodId.WriteLine2: return new RemoteHostMethodInfo( @@ -339,7 +339,7 @@ internal static RemoteHostMethodInfo LookUp(RemoteHostMethodId methodId) typeof(PSHostRawUserInterface), "get_ForegroundColor", typeof(ConsoleColor), - new Type[] { }); + Array.Empty()); case RemoteHostMethodId.SetForegroundColor: return new RemoteHostMethodInfo( @@ -353,7 +353,7 @@ internal static RemoteHostMethodInfo LookUp(RemoteHostMethodId methodId) typeof(PSHostRawUserInterface), "get_BackgroundColor", typeof(ConsoleColor), - new Type[] { }); + Array.Empty()); case RemoteHostMethodId.SetBackgroundColor: return new RemoteHostMethodInfo( @@ -367,7 +367,7 @@ internal static RemoteHostMethodInfo LookUp(RemoteHostMethodId methodId) typeof(PSHostRawUserInterface), "get_CursorPosition", typeof(Coordinates), - new Type[] { }); + Array.Empty()); case RemoteHostMethodId.SetCursorPosition: return new RemoteHostMethodInfo( @@ -381,7 +381,7 @@ internal static RemoteHostMethodInfo LookUp(RemoteHostMethodId methodId) typeof(PSHostRawUserInterface), "get_WindowPosition", typeof(Coordinates), - new Type[] { }); + Array.Empty()); case RemoteHostMethodId.SetWindowPosition: return new RemoteHostMethodInfo( @@ -395,7 +395,7 @@ internal static RemoteHostMethodInfo LookUp(RemoteHostMethodId methodId) typeof(PSHostRawUserInterface), "get_CursorSize", typeof(int), - new Type[] { }); + Array.Empty()); case RemoteHostMethodId.SetCursorSize: return new RemoteHostMethodInfo( @@ -409,7 +409,7 @@ internal static RemoteHostMethodInfo LookUp(RemoteHostMethodId methodId) typeof(PSHostRawUserInterface), "get_BufferSize", typeof(Size), - new Type[] { }); + Array.Empty()); case RemoteHostMethodId.SetBufferSize: return new RemoteHostMethodInfo( @@ -423,7 +423,7 @@ internal static RemoteHostMethodInfo LookUp(RemoteHostMethodId methodId) typeof(PSHostRawUserInterface), "get_WindowSize", typeof(Size), - new Type[] { }); + Array.Empty()); case RemoteHostMethodId.SetWindowSize: return new RemoteHostMethodInfo( @@ -437,7 +437,7 @@ internal static RemoteHostMethodInfo LookUp(RemoteHostMethodId methodId) typeof(PSHostRawUserInterface), "get_WindowTitle", typeof(string), - new Type[] { }); + Array.Empty()); case RemoteHostMethodId.SetWindowTitle: return new RemoteHostMethodInfo( @@ -453,21 +453,21 @@ internal static RemoteHostMethodInfo LookUp(RemoteHostMethodId methodId) typeof(PSHostRawUserInterface), "get_MaxWindowSize", typeof(Size), - new Type[] { }); + Array.Empty()); case RemoteHostMethodId.GetMaxPhysicalWindowSize: return new RemoteHostMethodInfo( typeof(PSHostRawUserInterface), "get_MaxPhysicalWindowSize", typeof(Size), - new Type[] { }); + Array.Empty()); case RemoteHostMethodId.GetKeyAvailable: return new RemoteHostMethodInfo( typeof(PSHostRawUserInterface), "get_KeyAvailable", typeof(bool), - new Type[] { }); + Array.Empty()); // Host raw UI methods. @@ -483,7 +483,7 @@ internal static RemoteHostMethodInfo LookUp(RemoteHostMethodId methodId) typeof(PSHostRawUserInterface), "FlushInputBuffer", typeof(void), - new Type[] { }); + Array.Empty()); case RemoteHostMethodId.SetBufferContents1: return new RemoteHostMethodInfo( @@ -527,7 +527,7 @@ internal static RemoteHostMethodInfo LookUp(RemoteHostMethodId methodId) typeof(IHostSupportsInteractiveSession), "PopRunspace", typeof(void), - new Type[] { }); + Array.Empty()); // IHostSupportsInteractiveSession properties. @@ -536,14 +536,14 @@ internal static RemoteHostMethodInfo LookUp(RemoteHostMethodId methodId) typeof(IHostSupportsInteractiveSession), "get_IsRunspacePushed", typeof(bool), - new Type[] { }); + Array.Empty()); case RemoteHostMethodId.GetRunspace: return new RemoteHostMethodInfo( typeof(IHostSupportsInteractiveSession), "get_Runspace", typeof(System.Management.Automation.Runspaces.Runspace), - new Type[] { }); + Array.Empty()); default: Dbg.Assert(false, "All RemoteHostMethodId's should be handled. This code should not be reached."); diff --git a/src/System.Management.Automation/engine/runtime/Operations/ArrayOps.cs b/src/System.Management.Automation/engine/runtime/Operations/ArrayOps.cs index 5ce3b857c31..3712a1ad653 100644 --- a/src/System.Management.Automation/engine/runtime/Operations/ArrayOps.cs +++ b/src/System.Management.Automation/engine/runtime/Operations/ArrayOps.cs @@ -52,7 +52,7 @@ internal static T[] Multiply(T[] array, uint times) if (times == 0 || array.Length == 0) { - return new T[0]; // don't use Utils.EmptyArray, always return a new array + return Array.Empty(); // don't use Utils.EmptyArray, always return a new array } var context = LocalPipeline.GetExecutionContextFromTLS(); diff --git a/src/System.Management.Automation/engine/runtime/Operations/MiscOps.cs b/src/System.Management.Automation/engine/runtime/Operations/MiscOps.cs index dc4a8fbe204..223c208e822 100644 --- a/src/System.Management.Automation/engine/runtime/Operations/MiscOps.cs +++ b/src/System.Management.Automation/engine/runtime/Operations/MiscOps.cs @@ -3228,7 +3228,7 @@ internal static object Multiply(IEnumerator enumerator, uint times) if (originalList.Count == 0) { - return new object[0]; // don't use Utils.EmptyArray, always return a new array + return Array.Empty(); // don't use Utils.EmptyArray, always return a new array } return ArrayOps.Multiply(originalList.ToArray(), times); diff --git a/src/System.Management.Automation/namespaces/RegistryProvider.cs b/src/System.Management.Automation/namespaces/RegistryProvider.cs index bccbe80ab25..bac9796e0f6 100644 --- a/src/System.Management.Automation/namespaces/RegistryProvider.cs +++ b/src/System.Management.Automation/namespaces/RegistryProvider.cs @@ -3814,7 +3814,7 @@ private static object ConvertValueToKind(object value, RegistryValueKind kind) value, typeof(byte[]), CultureInfo.CurrentCulture) - : new byte[] { }; + : Array.Empty(); break; case RegistryValueKind.DWord: @@ -3851,7 +3851,7 @@ private static object ConvertValueToKind(object value, RegistryValueKind kind) value, typeof(string[]), CultureInfo.CurrentCulture) - : new string[] { }; + : Array.Empty(); break; case RegistryValueKind.QWord: diff --git a/src/System.Management.Automation/utils/GraphicalHostReflectionWrapper.cs b/src/System.Management.Automation/utils/GraphicalHostReflectionWrapper.cs index 7579245980b..f38bdb18f87 100644 --- a/src/System.Management.Automation/utils/GraphicalHostReflectionWrapper.cs +++ b/src/System.Management.Automation/utils/GraphicalHostReflectionWrapper.cs @@ -128,12 +128,12 @@ internal static GraphicalHostReflectionWrapper GetGraphicalHostReflectionWrapper ConstructorInfo constructor = returnValue._graphicalHostHelperType.GetConstructor( BindingFlags.NonPublic | BindingFlags.Instance, null, - new Type[] { }, + Array.Empty(), null); if (constructor != null) { - returnValue._graphicalHostHelperObject = constructor.Invoke(new object[] { }); + returnValue._graphicalHostHelperObject = constructor.Invoke(Array.Empty()); Diagnostics.Assert(returnValue._graphicalHostHelperObject != null, "the constructor does not throw anything"); } @@ -187,7 +187,7 @@ internal object GetPropertyValue(string propertyName) Diagnostics.Assert(_graphicalHostHelperObject != null, "there should be a constructor in order to get an instance property value"); PropertyInfo property = _graphicalHostHelperType.GetProperty(propertyName, BindingFlags.NonPublic | BindingFlags.Instance); Diagnostics.Assert(property != null, "property " + propertyName + " exists in graphicalHostHelperType is verified by caller"); - return property.GetValue(_graphicalHostHelperObject, new object[] { }); + return property.GetValue(_graphicalHostHelperObject, Array.Empty()); } /// @@ -199,7 +199,7 @@ internal object GetStaticPropertyValue(string propertyName) { PropertyInfo property = _graphicalHostHelperType.GetProperty(propertyName, BindingFlags.NonPublic | BindingFlags.Static); Diagnostics.Assert(property != null, "property " + propertyName + " exists in graphicalHostHelperType is verified by caller"); - return property.GetValue(null, new object[] { }); + return property.GetValue(null, Array.Empty()); } /// diff --git a/test/xUnit/csharp/test_CommandLineParser.cs b/test/xUnit/csharp/test_CommandLineParser.cs index 2c44d9aa851..f52fcdf2fee 100644 --- a/test/xUnit/csharp/test_CommandLineParser.cs +++ b/test/xUnit/csharp/test_CommandLineParser.cs @@ -19,7 +19,7 @@ public static void TestDefaults() { var cpp = new CommandLineParameterParser(); - cpp.Parse(new string[0]); + cpp.Parse(System.Array.Empty()); Assert.False(cpp.AbortStartup); Assert.Empty(cpp.Args); @@ -60,9 +60,9 @@ public static void Test_Throws_On_Reuse() { var cpp = new CommandLineParameterParser(); - cpp.Parse(new string[0]); + cpp.Parse(System.Array.Empty()); - Assert.Throws(() => cpp.Parse(new string[0])); + Assert.Throws(() => cpp.Parse(System.Array.Empty())); } [Theory] From 3864aee3d3dd9b7b5d3611cf7448bc83985234ca Mon Sep 17 00:00:00 2001 From: xtqqczze Date: Sun, 1 Nov 2020 19:27:58 +0000 Subject: [PATCH 2/7] Remove outdated comments --- .../engine/runtime/Operations/ArrayOps.cs | 2 +- .../engine/runtime/Operations/MiscOps.cs | 2 +- 2 files changed, 2 insertions(+), 2 deletions(-) diff --git a/src/System.Management.Automation/engine/runtime/Operations/ArrayOps.cs b/src/System.Management.Automation/engine/runtime/Operations/ArrayOps.cs index 3712a1ad653..2b236237a68 100644 --- a/src/System.Management.Automation/engine/runtime/Operations/ArrayOps.cs +++ b/src/System.Management.Automation/engine/runtime/Operations/ArrayOps.cs @@ -52,7 +52,7 @@ internal static T[] Multiply(T[] array, uint times) if (times == 0 || array.Length == 0) { - return Array.Empty(); // don't use Utils.EmptyArray, always return a new array + return Array.Empty(); } var context = LocalPipeline.GetExecutionContextFromTLS(); diff --git a/src/System.Management.Automation/engine/runtime/Operations/MiscOps.cs b/src/System.Management.Automation/engine/runtime/Operations/MiscOps.cs index 223c208e822..2f91295d98a 100644 --- a/src/System.Management.Automation/engine/runtime/Operations/MiscOps.cs +++ b/src/System.Management.Automation/engine/runtime/Operations/MiscOps.cs @@ -3228,7 +3228,7 @@ internal static object Multiply(IEnumerator enumerator, uint times) if (originalList.Count == 0) { - return Array.Empty(); // don't use Utils.EmptyArray, always return a new array + return Array.Empty(); } return ArrayOps.Multiply(originalList.ToArray(), times); From 7be3ba3eef7419a4d34f5c718f320d2c1b52b0d0 Mon Sep 17 00:00:00 2001 From: xtqqczze Date: Sun, 1 Nov 2020 19:28:42 +0000 Subject: [PATCH 3/7] Remove outdated content --- docs/dev-process/coding-guidelines.md | 3 --- 1 file changed, 3 deletions(-) diff --git a/docs/dev-process/coding-guidelines.md b/docs/dev-process/coding-guidelines.md index 3472296251b..dab26aa5bf7 100644 --- a/docs/dev-process/coding-guidelines.md +++ b/docs/dev-process/coding-guidelines.md @@ -111,9 +111,6 @@ Some general guidelines: * Avoid using string interpolations and overloads with implicit parameters such as `Culture` and `StringComparison`. Instead, use overloads with more explicit parameters such as `String.Format(IFormatProvider, String, Object[])` and `Equals(String, String, StringComparison)`. -* Avoid creating empty arrays. - Instead, reuse the static ones via `Utils.EmptyArray`. - * Avoid unnecessary memory allocation in a loop. Move the memory allocation outside the loop if possible. From e6243413215b754ee22b6dca2e9cf14960a383d6 Mon Sep 17 00:00:00 2001 From: xtqqczze Date: Mon, 2 Nov 2020 10:33:19 +0000 Subject: [PATCH 4/7] Revert changes and suppress rule address @iSazonov review --- .../engine/runtime/Operations/ArrayOps.cs | 6 +++++- .../engine/runtime/Operations/MiscOps.cs | 6 +++++- 2 files changed, 10 insertions(+), 2 deletions(-) diff --git a/src/System.Management.Automation/engine/runtime/Operations/ArrayOps.cs b/src/System.Management.Automation/engine/runtime/Operations/ArrayOps.cs index 2b236237a68..18fe90801b0 100644 --- a/src/System.Management.Automation/engine/runtime/Operations/ArrayOps.cs +++ b/src/System.Management.Automation/engine/runtime/Operations/ArrayOps.cs @@ -41,6 +41,10 @@ internal static object[] SlicingIndex(object target, object[] indexes, FuncCollection to multiply. /// Number of times the collection is to be multiplied/copied. /// Collection multiplied by integer. + [System.Diagnostics.CodeAnalysis.SuppressMessage( + "Performance", + "CA1825:Avoid zero-length array allocations", + Justification = "Code from source-depot includes comment: 'don't use Utils.EmptyArray, always return a new array'.")] internal static T[] Multiply(T[] array, uint times) { Diagnostics.Assert(array != null, "Caller should verify the arguments for array multiplication"); @@ -52,7 +56,7 @@ internal static T[] Multiply(T[] array, uint times) if (times == 0 || array.Length == 0) { - return Array.Empty(); + return new T[0]; } var context = LocalPipeline.GetExecutionContextFromTLS(); diff --git a/src/System.Management.Automation/engine/runtime/Operations/MiscOps.cs b/src/System.Management.Automation/engine/runtime/Operations/MiscOps.cs index 2f91295d98a..fcfb0340691 100644 --- a/src/System.Management.Automation/engine/runtime/Operations/MiscOps.cs +++ b/src/System.Management.Automation/engine/runtime/Operations/MiscOps.cs @@ -3208,6 +3208,10 @@ internal static object MethodInvoker(PSInvokeMemberBinder binder, return result.ToArray(); } + [System.Diagnostics.CodeAnalysis.SuppressMessage( + "Performance", + "CA1825:Avoid zero-length array allocations", + Justification = "Code from source-depot includes comment: 'don't use Utils.EmptyArray, always return a new array'.")] internal static object Multiply(IEnumerator enumerator, uint times) { var fakeEnumerator = enumerator as NonEnumerableObjectEnumerator; @@ -3228,7 +3232,7 @@ internal static object Multiply(IEnumerator enumerator, uint times) if (originalList.Count == 0) { - return Array.Empty(); + return new object[0]; } return ArrayOps.Multiply(originalList.ToArray(), times); From 6518cc69334d88585160123899765972549d7326 Mon Sep 17 00:00:00 2001 From: xtqqczze Date: Mon, 2 Nov 2020 10:37:41 +0000 Subject: [PATCH 5/7] Add comments --- .../engine/runtime/Operations/ArrayOps.cs | 1 + .../engine/runtime/Operations/MiscOps.cs | 1 + 2 files changed, 2 insertions(+) diff --git a/src/System.Management.Automation/engine/runtime/Operations/ArrayOps.cs b/src/System.Management.Automation/engine/runtime/Operations/ArrayOps.cs index 18fe90801b0..43d08c54806 100644 --- a/src/System.Management.Automation/engine/runtime/Operations/ArrayOps.cs +++ b/src/System.Management.Automation/engine/runtime/Operations/ArrayOps.cs @@ -56,6 +56,7 @@ internal static T[] Multiply(T[] array, uint times) if (times == 0 || array.Length == 0) { + // Always return a new array. return new T[0]; } diff --git a/src/System.Management.Automation/engine/runtime/Operations/MiscOps.cs b/src/System.Management.Automation/engine/runtime/Operations/MiscOps.cs index fcfb0340691..14442381ca0 100644 --- a/src/System.Management.Automation/engine/runtime/Operations/MiscOps.cs +++ b/src/System.Management.Automation/engine/runtime/Operations/MiscOps.cs @@ -3232,6 +3232,7 @@ internal static object Multiply(IEnumerator enumerator, uint times) if (originalList.Count == 0) { + // Always return a new array. return new object[0]; } From 1fa434a7eb96f6c0c014bb175417c9029d67c415 Mon Sep 17 00:00:00 2001 From: xtqqczze Date: Mon, 2 Nov 2020 11:50:06 +0000 Subject: [PATCH 6/7] Use local suppression --- .../engine/runtime/Operations/ArrayOps.cs | 6 ++---- .../engine/runtime/Operations/MiscOps.cs | 6 ++---- 2 files changed, 4 insertions(+), 8 deletions(-) diff --git a/src/System.Management.Automation/engine/runtime/Operations/ArrayOps.cs b/src/System.Management.Automation/engine/runtime/Operations/ArrayOps.cs index 43d08c54806..3d60b0d22c1 100644 --- a/src/System.Management.Automation/engine/runtime/Operations/ArrayOps.cs +++ b/src/System.Management.Automation/engine/runtime/Operations/ArrayOps.cs @@ -41,10 +41,6 @@ internal static object[] SlicingIndex(object target, object[] indexes, FuncCollection to multiply. /// Number of times the collection is to be multiplied/copied. /// Collection multiplied by integer. - [System.Diagnostics.CodeAnalysis.SuppressMessage( - "Performance", - "CA1825:Avoid zero-length array allocations", - Justification = "Code from source-depot includes comment: 'don't use Utils.EmptyArray, always return a new array'.")] internal static T[] Multiply(T[] array, uint times) { Diagnostics.Assert(array != null, "Caller should verify the arguments for array multiplication"); @@ -56,8 +52,10 @@ internal static T[] Multiply(T[] array, uint times) if (times == 0 || array.Length == 0) { +#pragma warning disable CA1825 // Avoid zero-length array allocations // Always return a new array. return new T[0]; +#pragma warning restore CA1825 // Avoid zero-length array allocations } var context = LocalPipeline.GetExecutionContextFromTLS(); diff --git a/src/System.Management.Automation/engine/runtime/Operations/MiscOps.cs b/src/System.Management.Automation/engine/runtime/Operations/MiscOps.cs index 14442381ca0..fa36fed6a31 100644 --- a/src/System.Management.Automation/engine/runtime/Operations/MiscOps.cs +++ b/src/System.Management.Automation/engine/runtime/Operations/MiscOps.cs @@ -3208,10 +3208,6 @@ internal static object MethodInvoker(PSInvokeMemberBinder binder, return result.ToArray(); } - [System.Diagnostics.CodeAnalysis.SuppressMessage( - "Performance", - "CA1825:Avoid zero-length array allocations", - Justification = "Code from source-depot includes comment: 'don't use Utils.EmptyArray, always return a new array'.")] internal static object Multiply(IEnumerator enumerator, uint times) { var fakeEnumerator = enumerator as NonEnumerableObjectEnumerator; @@ -3232,8 +3228,10 @@ internal static object Multiply(IEnumerator enumerator, uint times) if (originalList.Count == 0) { +#pragma warning disable CA1825 // Avoid zero-length array allocations // Always return a new array. return new object[0]; +#pragma warning restore CA1825 // Avoid zero-length array allocations } return ArrayOps.Multiply(originalList.ToArray(), times); From b6d1de1e0e781d506413593757ad710a7742bc80 Mon Sep 17 00:00:00 2001 From: xtqqczze Date: Mon, 2 Nov 2020 11:54:14 +0000 Subject: [PATCH 7/7] Update comments --- .../engine/runtime/Operations/ArrayOps.cs | 2 +- .../engine/runtime/Operations/MiscOps.cs | 2 +- 2 files changed, 2 insertions(+), 2 deletions(-) diff --git a/src/System.Management.Automation/engine/runtime/Operations/ArrayOps.cs b/src/System.Management.Automation/engine/runtime/Operations/ArrayOps.cs index 3d60b0d22c1..f6b71fad1ef 100644 --- a/src/System.Management.Automation/engine/runtime/Operations/ArrayOps.cs +++ b/src/System.Management.Automation/engine/runtime/Operations/ArrayOps.cs @@ -53,7 +53,7 @@ internal static T[] Multiply(T[] array, uint times) if (times == 0 || array.Length == 0) { #pragma warning disable CA1825 // Avoid zero-length array allocations - // Always return a new array. + // Don't use Array.Empty(); always return a new instance. return new T[0]; #pragma warning restore CA1825 // Avoid zero-length array allocations } diff --git a/src/System.Management.Automation/engine/runtime/Operations/MiscOps.cs b/src/System.Management.Automation/engine/runtime/Operations/MiscOps.cs index fa36fed6a31..ea80c64dbee 100644 --- a/src/System.Management.Automation/engine/runtime/Operations/MiscOps.cs +++ b/src/System.Management.Automation/engine/runtime/Operations/MiscOps.cs @@ -3229,7 +3229,7 @@ internal static object Multiply(IEnumerator enumerator, uint times) if (originalList.Count == 0) { #pragma warning disable CA1825 // Avoid zero-length array allocations - // Always return a new array. + // Don't use Array.Empty(); always return a new instance. return new object[0]; #pragma warning restore CA1825 // Avoid zero-length array allocations }