Use KeyedCollection instead of Dictionary

Author: xt0rted

CHANGELOG.md

@@ -2,6 +2,9 @@
 
 ## Unreleased
 
+- Script names are no longer case insensitive which matches NPM's behavior
+- Project scripts are no longer backed by `Dictionary<string, string>` which should guarantee they're executed in the order they're found in the `global.json`
+
 ## [0.5.0](https://github.com/xt0rted/dotnet-run-script/compare/v0.4.0...v0.5.0) - 2022-10-11
 
 ### Added

Directory.Build.props

@@ -1,7 +1,7 @@
 <Project>
 
   <PropertyGroup>
-    <TargetFrameworks>netcoreapp3.1;net6.0</TargetFrameworks>
+    <TargetFrameworks>net6.0;netcoreapp3.1</TargetFrameworks>
     <LangVersion>latest</LangVersion>
     <ImplicitUsings>enable</ImplicitUsings>
     <Nullable>enable</Nullable>

src/CommandGroupRunner.cs

@@ -6,15 +6,15 @@ internal class CommandGroupRunner : ICommandGroupRunner
 {
     private readonly IConsoleWriter _writer;
     private readonly IEnvironment _environment;
-    private readonly IDictionary<string, string?> _scripts;
+    private readonly ScriptCollection _scripts;
     private readonly ProcessContext _processContext;
     private readonly bool _captureOutput;
     private readonly CancellationToken _cancellationToken;
 
     public CommandGroupRunner(
         IConsoleWriter writer,
         IEnvironment environment,
-        IDictionary<string, string?> scripts,
+        ScriptCollection scripts,
         ProcessContext processContext,
         bool captureOutput,
         CancellationToken cancellationToken)
@@ -38,10 +38,10 @@ public async Task<int> RunAsync(string name, string[]? scriptArgs)
     {
         var scriptNames = ImmutableArray.Create(new[] { "pre" + name, name, "post" + name });
 
-        foreach (var subScript in scriptNames.Where(scriptName => _scripts.ContainsKey(scriptName) || scriptName == "env"))
+        foreach (var subScript in scriptNames.Where(scriptName => _scripts.Contains(scriptName) || scriptName == "env"))
         {
             // At this point we should have done enough checks to make sure the only not found script is `env`
-            if (!_scripts.ContainsKey(subScript))
+            if (!_scripts.Contains(subScript))
             {
                 GlobalCommands.PrintEnvironmentVariables(_writer, _environment);
 
@@ -56,7 +56,7 @@ public async Task<int> RunAsync(string name, string[]? scriptArgs)
 
             var result = await command.RunAsync(
                 subScript,
-                _scripts[subScript]!,
+                _scripts[subScript].Script,
                 args);
 
             if (result != 0)

src/GlobalCommands.cs

@@ -7,15 +7,15 @@ internal static class GlobalCommands
     /// </summary>
     /// <param name="writer">The console logger instance to use.</param>
     /// <param name="scripts">The project's scripts.</param>
-    public static void PrintAvailableScripts(IConsoleWriter writer, IDictionary<string, string?> scripts)
+    public static void PrintAvailableScripts(IConsoleWriter writer, ScriptCollection scripts)
     {
         writer.Line("Available via `{0}`:", writer.ColorText(ConsoleColor.Blue, "dotnet r"));
         writer.BlankLine();
 
-        foreach (var script in scripts.Keys)
+        foreach (var (name, script) in scripts)
         {
-            writer.Line("  {0}", script);
-            writer.SecondaryLine("    {0}", scripts[script]);
+            writer.Line("  {0}", name);
+            writer.SecondaryLine("    {0}", script);
             writer.BlankLine();
         }
     }

src/Project.cs

@@ -2,12 +2,11 @@ namespace RunScript;
 
 using System.Text.Json.Serialization;
 
-using RunScript.Serialization;
-
 public class Project
 {
+    [JsonPropertyName("scriptShell")]
     public string? ScriptShell { get; set; }
 
-    [JsonConverter(typeof(CaseInsensitiveDictionaryConverter<string?>))]
-    public Dictionary<string, string?>? Scripts { get; set; }
+    [JsonPropertyName("scripts")]
+    public ScriptCollection? Scripts { get; set; }
 }

src/ProjectLoader.cs

@@ -57,10 +57,9 @@ internal class ProjectLoader
             return JsonSerializer.Deserialize<Project>(
                 json,
                 new JsonSerializerOptions
-                    {
-                        PropertyNameCaseInsensitive = true,
-                        ReadCommentHandling = JsonCommentHandling.Skip,
-                    });
+                {
+                    ReadCommentHandling = JsonCommentHandling.Skip,
+                });
         }
         catch
         {

src/RunScriptCommand.cs

@@ -134,15 +134,15 @@ public async Task<int> InvokeAsync(InvocationContext context)
     }
 
     internal static List<ScriptResult> FindScripts(
-        IDictionary<string, string?> projectScripts,
+        ScriptCollection projectScripts,
         string[] scripts)
     {
         var results = new List<ScriptResult>();
 
         foreach (var script in scripts)
         {
             // The `env` script is special so if it's not explicitly declared we act like it was
-            if (projectScripts.ContainsKey(script) || string.Equals(script, "env", StringComparison.OrdinalIgnoreCase))
+            if (projectScripts.Contains(script) || string.Equals(script, "env", StringComparison.Ordinal))
             {
                 results.Add(new(script, true));
 
@@ -160,7 +160,7 @@ internal static List<ScriptResult> FindScripts(
                     }
                 });
 
-            foreach (var projectScript in projectScripts.Keys)
+            foreach (var (projectScript, _) in projectScripts)
             {
                 if (matcher.IsMatch(projectScript.AsSpan()))
                 {

src/ScriptCollection.cs

@@ -0,0 +1,20 @@
+namespace RunScript;
+
+using System.Collections.ObjectModel;
+using System.Text.Json.Serialization;
+
+using RunScript.Serialization;
+
+[JsonConverter(typeof(ScriptCollectionConverter))]
+public class ScriptCollection : KeyedCollection<string, ScriptMapping>
+{
+    protected override string GetKeyForItem(ScriptMapping item)
+    {
+        if (item is null) throw new ArgumentNullException(nameof(item));
+
+        return item.Name;
+    }
+
+    public void Add(string name, string script)
+        => Add(new ScriptMapping(name, script));
+}

src/ScriptMapping.cs

@@ -0,0 +1,3 @@
+namespace RunScript;
+
+public record ScriptMapping(string Name, string Script);

src/Serialization/CaseInsensitiveDictionaryConverter.cs

@@ -0,0 +1,42 @@
+namespace RunScript.Serialization;
+
+using System.Text.Json;
+using System.Text.Json.Serialization;
+
+internal class ScriptCollectionConverter : JsonConverter<ScriptCollection>
+{
+    public override ScriptCollection Read(
+        ref Utf8JsonReader reader,
+        Type typeToConvert,
+        JsonSerializerOptions options)
+    {
+        var scripts = new ScriptCollection();
+
+        using (var jsonDoc = JsonDocument.ParseValue(ref reader))
+        using (var jsonScripts = jsonDoc.RootElement.EnumerateObject())
+        {
+            foreach (var script in jsonScripts)
+            {
+                scripts.Add(script.Name, script.Value.ToString());
+            }
+        }
+
+        return scripts;
+    }
+
+    public override void Write(
+        Utf8JsonWriter writer,
+        ScriptCollection value,
+        JsonSerializerOptions options)
+    {
+        writer.WriteStartObject();
+
+        foreach (var element in value)
+        {
+            writer.WritePropertyName(element.Name);
+            writer.WriteStringValue(element.Script);
+        }
+
+        writer.WriteEndObject();
+    }
+}

src/Serialization/ScriptCollectionConverter.cs

@@ -101,7 +101,7 @@ private static CommandBuilder SetUpTest(bool isWindows, string? comSpec = Defaul
 
         var project = new Project
         {
-            Scripts = new Dictionary<string, string?>(),
+            Scripts = new ScriptCollection(),
         };
 
         var environment = new TestEnvironment(

test/CommandBuilderTests.cs

@@ -240,7 +240,7 @@ public async Task Should_return_first_error_hit(bool isWindows)
     private static (TestConsole console, CommandGroupRunner groupRunner) SetUpTest(
         TestCommandRunner[] commandRunners,
         bool isWindows,
-        Action<Dictionary<string, string?>>? scriptSetup = null)
+        Action<ScriptCollection>? scriptSetup = null)
     {
         var console = new TestConsole();
         var consoleFormatProvider = new ConsoleFormatInfo
@@ -249,7 +249,7 @@ private static (TestConsole console, CommandGroupRunner groupRunner) SetUpTest(
         };
         var consoleWriter = new ConsoleWriter(console, consoleFormatProvider, verbose: true);
 
-        var scripts = new Dictionary<string, string?>(StringComparer.OrdinalIgnoreCase)
+        var scripts = new ScriptCollection
         {
             // clean
             { "clean", "echo clean" },

test/CommandGroupRunnerTests.cs

@@ -20,7 +20,7 @@ public async Task Should_log_all_available_scripts()
         };
         var consoleWriter = new ConsoleWriter(console, consoleFormatProvider, verbose: true);
 
-        var scripts = new Dictionary<string, string?>(StringComparer.OrdinalIgnoreCase)
+        var scripts = new ScriptCollection
         {
             { "clean", "echo clean" },
             { "prebuild", "echo prebuild" },

test/GlobalCommandsTests.cs

@@ -53,7 +53,7 @@ private static async Task Should_execute_single_script_in_shell(bool isWindows,
 
         var project = new Project
         {
-            Scripts = new Dictionary<string, string?>(StringComparer.OrdinalIgnoreCase)
+            Scripts = new ScriptCollection
             {
                 { "test", "echo testing" },
             },

test/Integration/CommandBuilderTests.cs

@@ -8,5 +8,6 @@ public static class ModuleInitializer
     public static void Init()
     {
         VerifierSettings.AddExtraSettings(settings => settings.Converters.Add(new ConsoleConverter()));
+        VerifierSettings.AddExtraSettings(settings => settings.Converters.Add(new ScriptCollectionConverter()));
     }
 }

test/ModuleInitializer.cs

@@ -0,0 +1,7 @@
+{
+  Scripts: {
+    bUiLD: build,
+    TEST: test,
+    pack: pack
+  }
+}

test/ProjectLoaderTests.Should_not_treat_script_names_as_always_lowercase.verified.txt

@@ -85,7 +85,7 @@ public async Task Should_look_up_the_tree()
     }
 
     [Fact]
-    public async Task Should_treat_script_names_as_lowercase()
+    public async Task Should_not_treat_script_names_as_always_lowercase()
     {
         // Given
         var testPath = TestPath("script-names");
@@ -95,8 +95,6 @@ public async Task Should_treat_script_names_as_lowercase()
         var (project, _) = await projectLoader.LoadAsync(testPath);
 
         // Then
-        project.Scripts?.Comparer.ShouldBe(StringComparer.OrdinalIgnoreCase);
-
         await Verify(project);
     }
 

test/ProjectLoaderTests.cs

@@ -9,11 +9,11 @@ public static class RunScriptCommandTests
     [Trait("category", "unit")]
     public class FindScripts
     {
-        private readonly Dictionary<string, string?> _projectScripts;
+        private readonly ScriptCollection _projectScripts;
 
         public FindScripts()
         {
-            _projectScripts = new Dictionary<string, string?>(StringComparer.OrdinalIgnoreCase)
+            _projectScripts = new ScriptCollection
             {
                 { "clean", "echo clean" },
                 { "prebuild", "echo prebuild" },

test/RunScriptCommandTests.cs

@@ -0,0 +1,20 @@
+namespace RunScript;
+
+public class ScriptCollectionConverter : WriteOnlyJsonConverter<ScriptCollection>
+{
+    public override void Write(VerifyJsonWriter writer, ScriptCollection value)
+    {
+        if (writer is null) throw new ArgumentNullException(nameof(writer));
+        if (value is null) throw new ArgumentNullException(nameof(value));
+
+        writer.WriteStartObject();
+
+        foreach (var (name, script) in value)
+        {
+            writer.WritePropertyName(name);
+            writer.WriteValue(script);
+        }
+
+        writer.WriteEndObject();
+    }
+}