Fix #1540 (#1545)

* refactor BindingContext to enforce single source of truth on child objects; remove ConfigureConsole and IConsoleFactory

* fix #1540

* split CompletionContext classes into separate files

* fix warning

* update API approvals

Author: jonsequitur

src/System.CommandLine.ApiCompatibility.Tests/ApiCompatibilityApprovalTests.System_CommandLine_api_is_not_changed.approved.txt

@@ -226,9 +226,8 @@ System.CommandLine.Binding
   public abstract class BinderBase<T>, IValueDescriptor<T>, IValueDescriptor, IValueSource
     protected T GetBoundValue(BindingContext bindingContext)
   public class BindingContext, System.IServiceProvider
-    .ctor(System.CommandLine.Parsing.ParseResult parseResult, System.CommandLine.IConsole console = null)
     public System.CommandLine.IConsole Console { get; }
-    public System.CommandLine.Parsing.ParseResult ParseResult { get; set; }
+    public System.CommandLine.Parsing.ParseResult ParseResult { get; }
     public System.Void AddService(System.Type serviceType, System.Func<System.IServiceProvider,System.Object> factory)
     public System.Void AddService<T>(Func<System.IServiceProvider,T> factory)
     public System.Object GetService(System.Type serviceType)
@@ -259,7 +258,6 @@ System.CommandLine.Builder
     public static CommandLineBuilder AddMiddleware(this CommandLineBuilder builder, System.CommandLine.Invocation.InvocationMiddleware middleware, System.CommandLine.Invocation.MiddlewareOrder order = Default)
     public static CommandLineBuilder AddMiddleware(this CommandLineBuilder builder, System.Action<System.CommandLine.Invocation.InvocationContext> onInvoke, System.CommandLine.Invocation.MiddlewareOrder order = Default)
     public static CommandLineBuilder CancelOnProcessTermination(this CommandLineBuilder builder)
-    public static CommandLineBuilder ConfigureConsole(this CommandLineBuilder builder, System.Func<System.CommandLine.Binding.BindingContext,System.CommandLine.IConsole> createConsole)
     public static CommandLineBuilder EnableDirectives(this CommandLineBuilder builder, System.Boolean value = True)
     public static CommandLineBuilder EnableLegacyDoubleDashBehavior(this CommandLineBuilder builder, System.Boolean value = True)
     public static CommandLineBuilder EnablePosixBundling(this CommandLineBuilder builder, System.Boolean value = True)
@@ -368,7 +366,7 @@ System.CommandLine.Invocation
   public class InvocationContext, System.IDisposable
     .ctor(System.CommandLine.Parsing.ParseResult parseResult, System.CommandLine.IConsole console = null)
     public System.CommandLine.Binding.BindingContext BindingContext { get; }
-    public System.CommandLine.IConsole Console { get; }
+    public System.CommandLine.IConsole Console { get; set; }
     public System.Int32 ExitCode { get; set; }
     public System.CommandLine.Help.HelpBuilder HelpBuilder { get; }
     public IInvocationResult InvocationResult { get; set; }

src/System.CommandLine.NamingConventionBinder.Tests/ModelBinderTests.cs

@@ -2,8 +2,8 @@
 // Licensed under the MIT license. See LICENSE file in the project root for full license information.
 
 using System.Collections.Generic;
-using System.CommandLine.Binding;
 using System.CommandLine.Builder;
+using System.CommandLine.Invocation;
 using System.CommandLine.Parsing;
 using System.CommandLine.Tests.Binding;
 using System.IO;
@@ -33,7 +33,7 @@ public void Option_arguments_are_bound_by_name_to_constructor_parameters(
             new Option("--value", argumentType: type)
         };
 
-        var bindingContext = new BindingContext(command.Parse(commandLine));
+        var bindingContext = new InvocationContext(command.Parse(commandLine)).BindingContext;
 
         var instance = binder.CreateInstance(bindingContext);
 
@@ -64,7 +64,7 @@ public void Command_arguments_are_bound_by_name_to_constructor_parameters(
             }
         };
 
-        var bindingContext = new BindingContext(command.Parse(commandLine));
+        var bindingContext = new InvocationContext(command.Parse(commandLine)).BindingContext;
 
         var instance = binder.CreateInstance(bindingContext);
 
@@ -91,7 +91,7 @@ public void Command_arguments_are_bound_by_name_to_complex_constructor_parameter
             }
         };
 
-        var bindingContext = new BindingContext(command.Parse(commandLine));
+        var bindingContext = new InvocationContext(command.Parse(commandLine)).BindingContext;
 
         var instance = binder.CreateInstance(bindingContext);
 
@@ -113,7 +113,7 @@ public void Explicitly_configured_default_values_can_be_bound_by_name_to_constru
         var binder = new ModelBinder(typeof(ClassWithMultiLetterCtorParameters));
 
         var parser = new Parser(command);
-        var bindingContext = new BindingContext(parser.Parse(""));
+        var bindingContext = new InvocationContext(parser.Parse("")).BindingContext;
 
         var instance = (ClassWithMultiLetterCtorParameters)binder.CreateInstance(bindingContext);
 
@@ -139,7 +139,7 @@ public void Option_arguments_are_bound_by_name_to_property_setters(
         };
         var parser = new Parser(command);
 
-        var bindingContext = new BindingContext(parser.Parse(commandLine));
+        var bindingContext = new InvocationContext(parser.Parse(commandLine)).BindingContext;
 
         var instance = binder.CreateInstance(bindingContext);
 
@@ -171,7 +171,7 @@ public void Command_arguments_are_bound_by_name_to_property_setters(
         };
         var parser = new Parser(command);
 
-        var bindingContext = new BindingContext(parser.Parse(commandLine));
+        var bindingContext = new InvocationContext(parser.Parse(commandLine)).BindingContext;
 
         var instance = binder.CreateInstance(bindingContext);
 
@@ -190,7 +190,7 @@ public void Types_having_constructors_accepting_a_single_string_are_bound_using_
         var command = new Command("the-command");
         command.AddOption(option);
         var binder = new ModelBinder(typeof(ClassWithCtorParameter<DirectoryInfo>));
-        var bindingContext = new BindingContext(command.Parse($"--value \"{tempPath}\""));
+        var bindingContext = new InvocationContext(command.Parse($"--value \"{tempPath}\"")).BindingContext;
 
         var instance = (ClassWithCtorParameter<DirectoryInfo>)binder.CreateInstance(bindingContext);
 
@@ -207,7 +207,7 @@ public void Explicitly_configured_default_values_can_be_bound_by_name_to_propert
         var binder = new ModelBinder(typeof(ClassWithSetter<string>));
 
         var parser = new Parser(command);
-        var bindingContext = new BindingContext(parser.Parse(""));
+        var bindingContext = new InvocationContext(parser.Parse("")).BindingContext;
 
         var instance = (ClassWithSetter<string>)binder.CreateInstance(bindingContext);
 
@@ -225,8 +225,8 @@ public void Property_setters_with_no_default_value_and_no_matching_option_are_no
         var binder = new ModelBinder(typeof(ClassWithSettersAndCtorParametersWithDifferentNames));
 
         var parser = new Parser(command);
-        var bindingContext = new BindingContext(
-            parser.Parse(""));
+        var bindingContext = new InvocationContext(
+            parser.Parse("")).BindingContext;
 
         var instance = (ClassWithSettersAndCtorParametersWithDifferentNames)binder.CreateInstance(bindingContext);
 
@@ -240,7 +240,7 @@ public void Parse_result_can_be_used_to_create_an_instance_without_doing_handler
         {
             new Option<int>("--int-option")
         });
-        var bindingContext = new BindingContext(parser.Parse("the-command --int-option 123"));
+        var bindingContext = new InvocationContext(parser.Parse("the-command --int-option 123")).BindingContext;
         var binder = new ModelBinder(typeof(ClassWithMultiLetterSetters));
 
         var instance = (ClassWithMultiLetterSetters)binder.CreateInstance(bindingContext);
@@ -256,7 +256,7 @@ public void Parse_result_can_be_used_to_modify_an_existing_instance_without_doin
             new Option<int>("--int-option")
         });
         var instance = new ClassWithMultiLetterSetters();
-        var bindingContext = new BindingContext(parser.Parse("the-command --int-option 123"));
+        var bindingContext = new InvocationContext(parser.Parse("the-command --int-option 123")).BindingContext;
         var binder = new ModelBinder(typeof(ClassWithMultiLetterSetters));
 
         binder.UpdateInstance(instance, bindingContext);
@@ -270,7 +270,7 @@ public void Modify_an_existing_instance_should_keep_all_default_values_if_no_arg
         var parser = new Parser(new Command("the-command"));
 
         var instance = new ClassWithComplexTypes();
-        var bindingContext = new BindingContext(parser.Parse("the-command"));
+        var bindingContext = new InvocationContext(parser.Parse("the-command")).BindingContext;
         var binder = new ModelBinder(typeof(ClassWithComplexTypes));
 
         binder.UpdateInstance(instance, bindingContext);
@@ -291,7 +291,7 @@ public void Values_from_options_on_parent_commands_are_bound_by_name_by_default(
 
         var parseResult = parentCommand.Parse("parent-command --int-option 123 child-command");
 
-        var bindingContext = new BindingContext(parseResult);
+        var bindingContext = new InvocationContext(parseResult).BindingContext;
 
         var instance = (ClassWithMultiLetterSetters)binder.CreateInstance(bindingContext);
 
@@ -311,7 +311,7 @@ public void Default_values_from_options_on_parent_commands_are_bound_by_name_by_
 
         var parseResult = parentCommand.Parse("parent-command child-command");
 
-        var bindingContext = new BindingContext(parseResult);
+        var bindingContext = new InvocationContext(parseResult).BindingContext;
 
         var instance = (ClassWithMultiLetterSetters)binder.CreateInstance(bindingContext);
 
@@ -334,7 +334,7 @@ public void Values_from_parent_command_arguments_are_bound_by_name_by_default()
 
         var parseResult = parentCommand.Parse("parent-command 123 child-command");
 
-        var bindingContext = new BindingContext(parseResult);
+        var bindingContext = new InvocationContext(parseResult).BindingContext;
 
         var instance = (ClassWithMultiLetterSetters)binder.CreateInstance(bindingContext);
 
@@ -357,7 +357,7 @@ public void Default_values_from_parent_command_arguments_are_bound_by_name_by_de
 
         var parseResult = parentCommand.Parse("parent-command child-command");
 
-        var bindingContext = new BindingContext(parseResult);
+        var bindingContext = new InvocationContext(parseResult).BindingContext;
 
         var instance = (ClassWithMultiLetterSetters)binder.CreateInstance(bindingContext);
 
@@ -381,7 +381,7 @@ public void Values_from_options_on_parent_commands_can_be_bound_regardless_of_na
             c => c.IntOption,
             option);
 
-        var bindingContext = new BindingContext(parentCommand.Parse("parent-command -x 123 child-command"));
+        var bindingContext = new InvocationContext(parentCommand.Parse("parent-command -x 123 child-command")).BindingContext;
 
         var instance = (ClassWithMultiLetterSetters)binder.CreateInstance(bindingContext);
 
@@ -399,7 +399,7 @@ public void Arbitrary_values_can_be_bound()
             c => c.IntOption,
             _ => 123);
 
-        var bindingContext = new BindingContext(command.Parse("the-command"));
+        var bindingContext = new InvocationContext(command.Parse("the-command")).BindingContext;
 
         var instance = (ClassWithMultiLetterSetters)binder.CreateInstance(bindingContext);
 
@@ -421,7 +421,7 @@ public void PropertyInfo_can_be_bound_to_option()
             propertyInfo,
             option);
 
-        var bindingContext = new BindingContext(command.Parse("the-command --fred 42"));
+        var bindingContext = new InvocationContext(command.Parse("the-command --fred 42")).BindingContext;
 
         var instance = (ClassWithMultiLetterSetters)binder.CreateInstance(bindingContext);
 
@@ -441,7 +441,7 @@ public void PropertyInfo_can_be_bound_to_argument()
 
         binder.BindMemberFromValue(propertyInfo, argument);
 
-        var bindingContext = new BindingContext(command.Parse("the-command 42"));
+        var bindingContext = new InvocationContext(command.Parse("the-command 42")).BindingContext;
 
         var instance = (ClassWithMultiLetterSetters)binder.CreateInstance(bindingContext);
 
@@ -461,7 +461,7 @@ public void PropertyExpression_can_be_bound_to_option()
             i => i.IntOption,
             option);
 
-        var bindingContext = new BindingContext(command.Parse("the-command --fred 42"));
+        var bindingContext = new InvocationContext(command.Parse("the-command --fred 42")).BindingContext;
 
         var instance = (ClassWithMultiLetterSetters)binder.CreateInstance(bindingContext);
 
@@ -481,7 +481,7 @@ public void PropertyExpression_can_be_bound_to_argument()
             i => i.IntOption,
             argument);
 
-        var bindingContext = new BindingContext(command.Parse("the-command 42"));
+        var bindingContext = new InvocationContext(command.Parse("the-command 42")).BindingContext;
 
         var instance = (ClassWithMultiLetterSetters)binder.CreateInstance(bindingContext);
 
@@ -493,7 +493,7 @@ public void Option_argument_is_bound_to_longest_constructor()
     {
         var option = new Option<int>("--int-property");
 
-        var bindingContext = new BindingContext(option.Parse("--int-property 42"));
+        var bindingContext = new InvocationContext(option.Parse("--int-property 42")).BindingContext;
         var binder = new ModelBinder<ClassWithMultipleCtor>();
         var instance = binder.CreateInstance(bindingContext) as ClassWithMultipleCtor;
 
@@ -508,7 +508,7 @@ public void Command_argument_is_bound_to_longest_constructor()
         rootCommand.AddArgument(new Argument<int> { Name = nameof(ClassWithMultipleCtor.IntProperty) });
         var parser = new Parser(rootCommand);
 
-        var bindingContext = new BindingContext(parser.Parse("42"));
+        var bindingContext = new InvocationContext(parser.Parse("42")).BindingContext;
         var binder = new ModelBinder<ClassWithMultipleCtor>();
         var instance = binder.CreateInstance(bindingContext) as ClassWithMultipleCtor;
 
@@ -523,7 +523,7 @@ public void Explicit_model_binder_binds_only_to_configured_properties()
         var stringOption = new Option<string>("--string-property");
         var parser = new Parser(new RootCommand { intOption, stringOption });
 
-        var bindingContext = new BindingContext(parser.Parse("--int-property 42 --string-property Hello"));
+        var bindingContext = new InvocationContext(parser.Parse("--int-property 42 --string-property Hello")).BindingContext;
         var binder = new ModelBinder<ClassWithMultiLetterSetters>
         {
             EnforceExplicitBinding = true

src/System.CommandLine.Rendering.Tests/TerminalModeTests.cs

@@ -5,7 +5,6 @@
 using System.CommandLine.Invocation;
 using System.CommandLine.IO;
 using System.CommandLine.Parsing;
-using System.CommandLine.Tests;
 using System.CommandLine.Tests.Utility;
 using System.Threading.Tasks;
 using FluentAssertions;

src/System.CommandLine.Rendering/CommandLineBuilderExtensions.cs

@@ -3,7 +3,6 @@
 
 using System.CommandLine.Binding;
 using System.CommandLine.Builder;
-using System.CommandLine.Invocation;
 using System.Linq;
 
 namespace System.CommandLine.Rendering
@@ -13,15 +12,15 @@ public static class CommandLineBuilderExtensions
         public static CommandLineBuilder UseAnsiTerminalWhenAvailable(
             this CommandLineBuilder builder)
         {
-            builder.ConfigureConsole(context =>
+            builder.AddMiddleware(context =>
             {
                 var console = context.Console;
 
                 var terminal = console.GetTerminal(
-                    PreferVirtualTerminal(context),
-                    OutputMode(context));
+                    PreferVirtualTerminal(context.BindingContext),
+                    OutputMode(context.BindingContext));
 
-                return terminal ?? console;
+                context.Console = terminal ?? console;
             });
 
             return builder;

src/System.CommandLine.Tests/CommandTests.cs

@@ -419,7 +419,7 @@ public void Command_argument_of_ICollection_of_T_defaults_to_empty_when_not_spec
         }
 
         [Fact]
-        public void AddGlobalOption_updates_Options_and_GlobalOptions_property()
+        public void AddGlobalOption_updates_Options_property()
         {
             var option = new Option<string>("-x");
             var command = new Command("mycommand");

src/System.CommandLine.Tests/GlobalOptionTests.cs

@@ -4,6 +4,7 @@
 // Copyright (c) .NET Foundation and contributors. All rights reserved.
 // Licensed under the MIT license. See LICENSE file in the project root for full license information.
 
+using System.Threading.Tasks;
 using FluentAssertions;
 using Xunit;
 
@@ -41,6 +42,43 @@ public void Global_options_appear_in_options_list()
             root.Options.Should().Contain(option);
         }
 
+        [Fact] // https://github.com/dotnet/command-line-api/issues/1540
+        public void When_a_required_global_option_is_omitted_it_results_in_an_error()
+        {
+            var command = new Command("child");
+            var rootCommand = new RootCommand { command };
+            command.SetHandler(() => { });
+            var requiredOption = new Option<bool>("--i-must-be-set")
+            {
+                IsRequired = true
+            };
+            rootCommand.AddGlobalOption(requiredOption);
+
+            var result = rootCommand.Parse("child");
+
+            result.Errors
+                  .Should()
+                  .ContainSingle()
+                  .Which.Message.Should().Be("Option '--i-must-be-set' is required.");
+        }
+        
+        [Fact]
+        public void When_a_required_global_option_is_present_on_child_of_command_it_was_added_to_it_does_not_result_in_an_error()
+        {
+            var command = new Command("child");
+            var rootCommand = new RootCommand { command };
+            command.SetHandler(() => { });
+            var requiredOption = new Option<bool>("--i-must-be-set")
+            {
+                IsRequired = true
+            };
+            rootCommand.AddGlobalOption(requiredOption);
+
+            var result = rootCommand.Parse("child --i-must-be-set");
+
+            result.Errors.Should().BeEmpty();
+        }
+
         [Fact]
         public void Subcommands_added_after_a_global_option_is_added_to_parent_will_recognize_the_global_option()
         {

src/System.CommandLine.Tests/ParserTests.MultiplePositions.cs

@@ -1,9 +1,6 @@
 // Copyright (c) .NET Foundation and contributors. All rights reserved.
 // Licensed under the MIT license. See LICENSE file in the project root for full license information.
 
-using System.CommandLine.Invocation;
-using System.CommandLine.IO;
-using System.CommandLine.Parsing;
 using System.Linq;
 using FluentAssertions;
 using Xunit;

src/System.CommandLine.Tests/ParsingValidationTests.cs

@@ -2,7 +2,6 @@
 // Licensed under the MIT license. See LICENSE file in the project root for full license information.
 
 using System.Collections.Generic;
-using System.CommandLine.Invocation;
 using System.CommandLine.Parsing;
 using System.IO;
 using FluentAssertions;