Use List<T>.RemoveAll in System.Management.Automation.ModuleIntrinsics
#15686
Conversation
|
@xtqqczze I made a slight change to remove the 'firstTime' variable. Please take a look and see if you have any concerns. |
| output.Add(item); | ||
| } | ||
|
|
||
| bool match = previousKey != null && currentKey.Equals(previousKey, StringComparison.OrdinalIgnoreCase); |
There was a problem hiding this comment.
What if item-s is null? Second null will be not removed?
There was a problem hiding this comment.
This was my thought, but this is private code, and keyGetter never returns null in practice.
I think it would be best to add null annotations and maybe a Debug.Assert.
There was a problem hiding this comment.
I think it would be best to add null annotations
We're doing it at such a rate that it will take us a hundred years to finish the job. 😸
Adding #nullable enable locally add no value and doesn't protect from errors coming from highest level of code.
Debug.Assert assumes we cover the code fully by tests but we don't run tests on Debug build at all.
Nevertheless I'd agree with local #nullable enable if we add a comment that sessionState.ExportedFunctions, sessionState.Module.CompiledExports, sessionState.ExportedVariables and sessionState.ExportedAliases do not contain nulls by design.
|
@daxian-dbw I've pushed some minor changes. |
| @@ -1438,7 +1438,7 @@ internal static void RemoveNestedModuleFunctions(PSModuleInfo module) | |||
| #nullable enable | |||
| private static void SortAndRemoveDuplicates<T>(List<T> input, Func<T, string> keyGetter) | |||
| { | |||
| Dbg.Assert(input != null, "Caller should verify that input != null"); | |||
| Dbg.Assert(input is not null, "Caller should verify that input != null"); | |||
There was a problem hiding this comment.
Please remove - we said about null items in the List.
There was a problem hiding this comment.
I thought I would update the code style while touching the method.
|
🎉 Handy links: |
Save allocation of temporary lists.