Update version

Changelog.md

@@ -1,3 +1,8 @@
+### v1.3.2 (31-Jul-2020)
+
+- Fixed an issue where a command was incorrectly allowed to execute when the user did not specify any value for a non-scalar parameter. Since they are always required, a parameter needs to be bound to (at least) one value. (Thanks [@Daniel Hix](https://github.com/ADustyOldMuffin))
+- Fixed an issue where `CliApplication.RunAsync(...)` threw `ArgumentException` if there were two environment variables, whose names differed only in case. Environment variable names are now treated case-sensitively. (Thanks [@Ron Myers](https://github.com/ron-myers))
+
 ### v1.3.1 (19-Jul-2020)
 
 - Running the application with the debug directive (`myapp [debug]`) will now also try to launch a debugger instance. In most cases it will save time as you won't need to attach the debugger manually. (Thanks [@Volodymyr Shkolka](https://github.com/BlackGad))

CliFx.Analyzers/CommandSchemaAnalyzer.cs

@@ -206,7 +206,7 @@ namespace CliFx.Analyzers
             // Duplicate environment variable name
             var duplicateEnvironmentVariableNameOptions = options
                 .Where(p => !string.IsNullOrWhiteSpace(p.EnvironmentVariableName))
-                .GroupBy(p => p.EnvironmentVariableName, StringComparer.OrdinalIgnoreCase)
+                .GroupBy(p => p.EnvironmentVariableName, StringComparer.Ordinal)
                 .Where(g => g.Count() > 1)
                 .SelectMany(g => g.AsEnumerable())
                 .ToArray();

CliFx.Tests/ArgumentBindingSpecs.Commands.cs

@@ -134,11 +134,17 @@ namespace CliFx.Tests
         [Command]
         private class RequiredOptionCommand : ICommand
         {
-            [CommandOption(nameof(OptionA))]
+            [CommandOption(nameof(Option), IsRequired = true)]
-            public string? OptionA { get; set; }
+            public string? Option { get; set; }
 
-            [CommandOption(nameof(OptionB), IsRequired = true)]
+            public ValueTask ExecuteAsync(IConsole console) => default;
-            public string? OptionB { get; set; }
+        }
+
+        [Command]
+        private class RequiredArrayOptionCommand : ICommand
+        {
+            [CommandOption(nameof(Option), IsRequired = true)]
+            public IReadOnlyList<string>? Option { get; set; }
 
             public ValueTask ExecuteAsync(IConsole console) => default;
         }

CliFx.Tests/ArgumentBindingSpecs.cs

@@ -825,88 +825,6 @@ namespace CliFx.Tests
             });
         }
 
-        [Fact]
-        public void Property_annotated_as_an_option_can_be_bound_from_multiple_values_even_if_the_inputs_use_mixed_naming()
-        {
-            // Arrange
-            var input = new CommandInputBuilder()
-                .AddOption("option", "foo")
-                .AddOption("o", "bar")
-                .AddOption("option", "baz")
-                .Build();
-
-            // Act
-            var instance = CommandHelper.ResolveCommand<ArrayOptionCommand>(input);
-
-            // Assert
-            instance.Should().BeEquivalentTo(new ArrayOptionCommand
-            {
-                Option = new[] {"foo", "bar", "baz"}
-            });
-        }
-
-        [Fact]
-        public void Property_annotated_as_a_required_option_must_always_be_set()
-        {
-            // Arrange
-            var input = new CommandInputBuilder()
-                .AddOption(nameof(RequiredOptionCommand.OptionA), "foo")
-                .Build();
-
-            // Act & assert
-            var ex = Assert.Throws<CliFxException>(() => CommandHelper.ResolveCommand<RequiredOptionCommand>(input));
-            _output.WriteLine(ex.Message);
-        }
-
-        [Fact]
-        public void Property_annotated_as_a_required_option_must_always_be_bound_to_some_value()
-        {
-            // Arrange
-            var input = new CommandInputBuilder()
-                .AddOption(nameof(RequiredOptionCommand.OptionB))
-                .Build();
-
-            // Act & assert
-            var ex = Assert.Throws<CliFxException>(() => CommandHelper.ResolveCommand<RequiredOptionCommand>(input));
-            _output.WriteLine(ex.Message);
-        }
-
-        [Fact]
-        public void Property_annotated_as_parameter_is_bound_directly_from_argument_value_according_to_the_order()
-        {
-            // Arrange
-            var input = new CommandInputBuilder()
-                .AddParameter("foo")
-                .AddParameter("bar")
-                .AddParameter("hello")
-                .AddParameter("world")
-                .Build();
-
-            // Act
-            var instance = CommandHelper.ResolveCommand<ParametersCommand>(input);
-
-            // Assert
-            instance.Should().BeEquivalentTo(new ParametersCommand
-            {
-                ParameterA = "foo",
-                ParameterB = "bar",
-                ParameterC = new[] {"hello", "world"}
-            });
-        }
-
-        [Fact]
-        public void Property_annotated_as_parameter_must_always_be_bound_to_some_value()
-        {
-            // Arrange
-            var input = new CommandInputBuilder()
-                .AddParameter("foo")
-                .Build();
-
-            // Act & assert
-            var ex = Assert.Throws<CliFxException>(() => CommandHelper.ResolveCommand<ParametersCommand>(input));
-            _output.WriteLine(ex.Message);
-        }
-
         [Fact]
         public void Property_of_custom_type_that_implements_IEnumerable_can_only_be_bound_if_that_type_has_a_constructor_accepting_an_array()
         {
@@ -959,6 +877,114 @@ namespace CliFx.Tests
             _output.WriteLine(ex.Message);
         }
 
+        [Fact]
+        public void Property_annotated_as_an_option_can_be_bound_from_multiple_values_even_if_the_inputs_use_mixed_naming()
+        {
+            // Arrange
+            var input = new CommandInputBuilder()
+                .AddOption("option", "foo")
+                .AddOption("o", "bar")
+                .AddOption("option", "baz")
+                .Build();
+
+            // Act
+            var instance = CommandHelper.ResolveCommand<ArrayOptionCommand>(input);
+
+            // Assert
+            instance.Should().BeEquivalentTo(new ArrayOptionCommand
+            {
+                Option = new[] {"foo", "bar", "baz"}
+            });
+        }
+
+        [Fact]
+        public void Property_annotated_as_a_required_option_must_always_be_set()
+        {
+            // Arrange
+            var input = new CommandInputBuilder()
+                .Build();
+
+            // Act & assert
+            var ex = Assert.Throws<CliFxException>(() => CommandHelper.ResolveCommand<RequiredOptionCommand>(input));
+            _output.WriteLine(ex.Message);
+        }
+
+        [Fact]
+        public void Property_annotated_as_a_required_option_must_always_be_bound_to_some_value()
+        {
+            // Arrange
+            var input = new CommandInputBuilder()
+                .AddOption(nameof(RequiredOptionCommand.Option))
+                .Build();
+
+            // Act & assert
+            var ex = Assert.Throws<CliFxException>(() => CommandHelper.ResolveCommand<RequiredOptionCommand>(input));
+            _output.WriteLine(ex.Message);
+        }
+
+        [Fact]
+        public void Property_annotated_as_a_required_option_must_always_be_bound_to_at_least_one_value_if_it_expects_multiple_values()
+        {
+            // Arrange
+            var input = new CommandInputBuilder()
+                .AddOption(nameof(RequiredOptionCommand.Option))
+                .Build();
+
+            // Act & assert
+            var ex = Assert.Throws<CliFxException>(() => CommandHelper.ResolveCommand<RequiredArrayOptionCommand>(input));
+            _output.WriteLine(ex.Message);
+        }
+
+        [Fact]
+        public void Property_annotated_as_parameter_is_bound_directly_from_argument_value_according_to_the_order()
+        {
+            // Arrange
+            var input = new CommandInputBuilder()
+                .AddParameter("foo")
+                .AddParameter("bar")
+                .AddParameter("hello")
+                .AddParameter("world")
+                .Build();
+
+            // Act
+            var instance = CommandHelper.ResolveCommand<ParametersCommand>(input);
+
+            // Assert
+            instance.Should().BeEquivalentTo(new ParametersCommand
+            {
+                ParameterA = "foo",
+                ParameterB = "bar",
+                ParameterC = new[] {"hello", "world"}
+            });
+        }
+
+        [Fact]
+        public void Property_annotated_as_parameter_must_always_be_bound_to_some_value()
+        {
+            // Arrange
+            var input = new CommandInputBuilder()
+                .AddParameter("foo")
+                .Build();
+
+            // Act & assert
+            var ex = Assert.Throws<CliFxException>(() => CommandHelper.ResolveCommand<ParametersCommand>(input));
+            _output.WriteLine(ex.Message);
+        }
+
+        [Fact]
+        public void Property_annotated_as_parameter_must_always_be_bound_to_at_least_one_value_if_it_expects_multiple_values()
+        {
+            // Arrange
+            var input = new CommandInputBuilder()
+                .AddParameter("foo")
+                .AddParameter("bar")
+                .Build();
+
+            // Act & assert
+            var ex = Assert.Throws<CliFxException>(() => CommandHelper.ResolveCommand<ParametersCommand>(input));
+            _output.WriteLine(ex.Message);
+        }
+
         [Fact]
         public void All_provided_option_arguments_must_be_bound_to_corresponding_properties()
         {

CliFx.Tests/EnvironmentVariablesSpecs.cs

@@ -89,5 +89,27 @@ namespace CliFx.Tests
                 Option = $"foo{Path.PathSeparator}bar"
             });
         }
+        
+        [Fact]
+        public void Option_can_use_a_specific_environment_variable_as_fallback_while_respecting_case()
+        {
+            // Arrange
+            const string expected = "foobar";
+            var input = CommandInput.Empty;
+            var envVars = new Dictionary<string, string>
+            {
+                ["ENV_OPT"] = expected,
+                ["env_opt"] = "2"
+            };
+
+            // Act
+            var instance = CommandHelper.ResolveCommand<EnvironmentVariableCommand>(input, envVars);
+
+            // Assert
+            instance.Should().BeEquivalentTo(new EnvironmentVariableCommand
+            {
+                Option = expected
+            });
+        }
     }
-}
+}

CliFx.props

@@ -1,7 +1,7 @@
 <Project>
 
   <PropertyGroup>
-    <Version>1.3.1</Version>
+    <Version>1.3.2</Version>
     <Company>Tyrrrz</Company>
     <Copyright>Copyright (C) Alexey Golub</Copyright>
     <LangVersion>latest</LangVersion>

CliFx/CliApplication.cs

@@ -218,9 +218,10 @@ namespace CliFx
         /// </remarks>
         public async ValueTask<int> RunAsync(IReadOnlyList<string> commandLineArguments)
         {
+            // Environment variable names are case-insensitive on Windows but are case-sensitive on Linux and macOS
             var environmentVariables = Environment.GetEnvironmentVariables()
                 .Cast<DictionaryEntry>()
-                .ToDictionary(e => (string) e.Key, e => (string) e.Value, StringComparer.OrdinalIgnoreCase);
+                .ToDictionary(e => (string) e.Key, e => (string) e.Value, StringComparer.Ordinal);
 
             return await RunAsync(commandLineArguments, environmentVariables);
         }
@@ -264,4 +265,4 @@ namespace CliFx
             public ValueTask ExecuteAsync(IConsole console) => default;
         }
     }
-}
+}

CliFx/Domain/CommandOptionSchema.cs

@@ -45,7 +45,7 @@ namespace CliFx.Domain
 
         public bool MatchesEnvironmentVariableName(string environmentVariableName) =>
             !string.IsNullOrWhiteSpace(EnvironmentVariableName) &&
-            string.Equals(EnvironmentVariableName, environmentVariableName, StringComparison.OrdinalIgnoreCase);
+            string.Equals(EnvironmentVariableName, environmentVariableName, StringComparison.Ordinal);
 
         public string GetUserFacingDisplayString()
         {

CliFx/Domain/CommandSchema.cs

@@ -103,12 +103,16 @@ namespace CliFx.Domain
 
             if (nonScalarParameter != null)
             {
-                // TODO: Should it verify that at least one value is passed?
                 var nonScalarValues = parameterInputs
                     .Skip(scalarParameters.Length)
                     .Select(p => p.Value)
                     .ToArray();
 
+                // Parameters are required by default and so a non-scalar parameter must
+                // be bound to at least one value
+                if(!nonScalarValues.Any())
+                    throw CliFxException.ParameterNotSet(nonScalarParameter);
+
                 nonScalarParameter.BindOn(instance, nonScalarValues);
                 remainingParameterInputs.Clear();
             }