cleanup: Add analyzer for public API and revert an unintentional brea…

…king change from 2.3 (#264)
natemcmaster · Aug 13, 2019 · 3d08a2b · 3d08a2b
1 parent 38a5c76
commit 3d08a2b
Show file tree

Hide file tree

Showing 14 changed files with 722 additions and 27 deletions.
diff --git a/CommandLineUtils.sln b/CommandLineUtils.sln
@@ -1,6 +1,6 @@
 
 Microsoft Visual Studio Solution File, Format Version 12.00
-# Visual Studio 16
+# Visual Studio Version 16
 VisualStudioVersion = 16.0.0.0
 MinimumVisualStudioVersion = 16.0.0.0
 Project("{2150E333-8FDC-42A3-9474-1A3956D46DE8}") = "src", "src", "{95D4B35E-0A21-4D64-8BAF-27DD6C019FC5}"
@@ -13,7 +13,6 @@ Project("{9A19103F-16F7-4668-BE54-9A1E7A4F7556}") = "McMaster.Extensions.Command
 EndProject
 Project("{2150E333-8FDC-42A3-9474-1A3956D46DE8}") = "files", "files", "{509D6286-77FA-49C1-9EC6-7461DBE79536}"
 	ProjectSection(SolutionItems) = preProject
-		.appveyor.yml = .appveyor.yml
 		.editorconfig = .editorconfig
 		.gitattributes = .gitattributes
 		.gitignore = .gitignore
@@ -26,14 +25,12 @@ Project("{2150E333-8FDC-42A3-9474-1A3956D46DE8}") = "files", "files", "{509D6286
 		NOTICE.txt = NOTICE.txt
 		NuGet.config = NuGet.config
 		README.md = README.md
-		releasenotes.props = releasenotes.props
 		version.props = version.props
-		.travis.yml = .travis.yml
 	EndProjectSection
 EndProject
-Project("{FAE04EC0-301F-11D3-BF4B-00C04F79EFBC}") = "McMaster.Extensions.Hosting.CommandLine", "src\Hosting.CommandLine\McMaster.Extensions.Hosting.CommandLine.csproj", "{407245F7-3F2C-4634-8578-7EFCA9BD26BD}"
+Project("{9A19103F-16F7-4668-BE54-9A1E7A4F7556}") = "McMaster.Extensions.Hosting.CommandLine", "src\Hosting.CommandLine\McMaster.Extensions.Hosting.CommandLine.csproj", "{407245F7-3F2C-4634-8578-7EFCA9BD26BD}"
 EndProject
-Project("{FAE04EC0-301F-11D3-BF4B-00C04F79EFBC}") = "Hosting.CommandLine.Tests", "test\Hosting.CommandLine.Tests\McMaster.Extensions.Hosting.CommandLine.Tests.csproj", "{04A5D2B8-18E4-4C75-AEF9-79D171FAC210}"
+Project("{9A19103F-16F7-4668-BE54-9A1E7A4F7556}") = "McMaster.Extensions.Hosting.CommandLine.Tests", "test\Hosting.CommandLine.Tests\McMaster.Extensions.Hosting.CommandLine.Tests.csproj", "{04A5D2B8-18E4-4C75-AEF9-79D171FAC210}"
 EndProject
 Global
 	GlobalSection(SolutionConfigurationPlatforms) = preSolution

diff --git a/src/CommandLineUtils/CommandLineApplication.Execute.cs b/src/CommandLineUtils/CommandLineApplication.Execute.cs
@@ -31,6 +31,7 @@ public static int Execute<TApp>(CommandLineContext context)
             where TApp : class
             => ExecuteAsync<TApp>(context).GetAwaiter().GetResult();
 
+#pragma warning disable RS0026 // Do not add multiple public overloads with optional parameters
         /// <summary>
         /// Creates an instance of <typeparamref name="TApp"/>, matching <see cref="CommandLineContext.Arguments"/>
         /// to all attributes on the type, and then invoking a method named "OnExecute" or "OnExecuteAsync" if it exists.
@@ -43,6 +44,7 @@ public static int Execute<TApp>(CommandLineContext context)
         /// <exception cref="InvalidOperationException">Thrown when attributes are incorrectly configured.</exception>
         /// <returns>The process exit code</returns>
         public static async Task<int> ExecuteAsync<TApp>(CommandLineContext context, CancellationToken cancellationToken = default)
+#pragma warning restore RS0026 // Do not add multiple public overloads with optional parameters
             where TApp : class
         {
             if (context == null)
@@ -134,6 +136,7 @@ public static Task<int> ExecuteAsync<TApp>(params string[] args)
         where TApp : class
             => ExecuteAsync<TApp>(PhysicalConsole.Singleton, args);
 
+#pragma warning disable RS0026 // Do not add multiple public overloads with optional parameters
         /// <summary>
         /// Creates an instance of <typeparamref name="TApp"/>, matching <paramref name="args"/>
         /// to all attributes on the type, and then invoking a method named "OnExecute" or "OnExecuteAsync" if it exists.
@@ -146,6 +149,7 @@ public static Task<int> ExecuteAsync<TApp>(params string[] args)
         /// <exception cref="InvalidOperationException">Thrown when attributes are incorrectly configured.</exception>
         /// <returns>The process exit code</returns>
         public static Task<int> ExecuteAsync<TApp>(string[] args, CancellationToken cancellationToken = default)
+#pragma warning restore RS0026 // Do not add multiple public overloads with optional parameters
         where TApp : class
         {
             args ??= Util.EmptyArray<string>();

diff --git a/src/CommandLineUtils/CommandLineApplication.cs b/src/CommandLineUtils/CommandLineApplication.cs
@@ -57,11 +57,13 @@ static CommandLineApplication()
         private readonly ConventionContext _conventionContext;
         private readonly List<IConvention> _conventions = new List<IConvention>();
 
+#pragma warning disable RS0027 // Public API with optional parameter(s) should have the most parameters amongst its public overloads.
         /// <summary>
         /// Initializes a new instance of <see cref="CommandLineApplication"/>.
         /// </summary>
         /// <param name="throwOnUnexpectedArg">Initial value for <see cref="ThrowOnUnexpectedArgument"/>.</param>
         public CommandLineApplication(bool throwOnUnexpectedArg = true)
+#pragma warning restore RS0027 // Public API with optional parameter(s) should have the most parameters amongst its public overloads.
             : this(null, DefaultHelpTextGenerator.Singleton, new DefaultCommandLineContext(), throwOnUnexpectedArg)
         {
         }
@@ -470,6 +472,7 @@ private void AssertCommandNameIsUnique(string? name, CommandLineApplication? com
             }
         }
 
+#pragma warning disable RS0026 // Do not add multiple public overloads with optional parameters
         /// <summary>
         /// Adds a subcommand.
         /// </summary>
@@ -478,6 +481,7 @@ private void AssertCommandNameIsUnique(string? name, CommandLineApplication? com
         /// <param name="throwOnUnexpectedArg"></param>
         /// <returns></returns>
         public CommandLineApplication Command(string name, Action<CommandLineApplication> configuration,
+#pragma warning restore RS0026 // Do not add multiple public overloads with optional parameters
             bool throwOnUnexpectedArg = true)
         {
             var command = new CommandLineApplication(this, name, throwOnUnexpectedArg);
@@ -489,6 +493,7 @@ public CommandLineApplication Command(string name, Action<CommandLineApplication
             return command;
         }
 
+#pragma warning disable RS0026 // Do not add multiple public overloads with optional parameters
         /// <summary>
         /// Adds a subcommand with model of type <typeparamref name="TModel" />.
         /// </summary>
@@ -498,6 +503,7 @@ public CommandLineApplication Command(string name, Action<CommandLineApplication
         /// <typeparam name="TModel">The model type of the subcommand.</typeparam>
         /// <returns></returns>
         public CommandLineApplication<TModel> Command<TModel>(string name, Action<CommandLineApplication<TModel>> configuration,
+#pragma warning restore RS0026 // Do not add multiple public overloads with optional parameters
             bool throwOnUnexpectedArg = true)
             where TModel : class
         {
@@ -592,6 +598,7 @@ public CommandOption<T> Option<T>(string template, string description, CommandOp
             return option;
         }
 
+#pragma warning disable RS0026 // Do not add multiple public overloads with optional parameters
         /// <summary>
         /// Adds a command line argument
         /// </summary>
@@ -600,8 +607,10 @@ public CommandOption<T> Option<T>(string template, string description, CommandOp
         /// <param name="multipleValues"></param>
         /// <returns></returns>
         public CommandArgument Argument(string name, string description, bool multipleValues = false)
+#pragma warning restore RS0026 // Do not add multiple public overloads with optional parameters
             => Argument(name, description, _ => { }, multipleValues);
 
+#pragma warning disable RS0026 // Do not add multiple public overloads with optional parameters
         /// <summary>
         /// Adds a command line argument.
         /// </summary>
@@ -611,6 +620,7 @@ public CommandArgument Argument(string name, string description, bool multipleVa
         /// <param name="multipleValues"></param>
         /// <returns></returns>
         public CommandArgument Argument(string name, string description, Action<CommandArgument> configuration, bool multipleValues = false)
+#pragma warning restore RS0026 // Do not add multiple public overloads with optional parameters
         {
             var argument = new CommandArgument
             {
@@ -623,6 +633,7 @@ public CommandArgument Argument(string name, string description, Action<CommandA
             return argument;
         }
 
+#pragma warning disable RS0026 // Do not add multiple public overloads with optional parameters
         /// <summary>
         /// Adds a command line argument with values that should be parsable into <typeparamref name="T" />.
         /// </summary>
@@ -633,6 +644,7 @@ public CommandArgument Argument(string name, string description, Action<CommandA
         /// <typeparam name="T">The type of the values on the option</typeparam>
         /// <returns></returns>
         public CommandArgument<T> Argument<T>(string name, string description, Action<CommandArgument> configuration, bool multipleValues = false)
+#pragma warning restore RS0026 // Do not add multiple public overloads with optional parameters
         {
             var parser = ValueParsers.GetParser<T>();
 
@@ -801,6 +813,7 @@ public int Execute(params string[] args)
             return ExecuteAsync(args).GetAwaiter().GetResult();
         }
 
+#pragma warning disable RS0026 // Do not add multiple public overloads with optional parameters
         /// <summary>
         /// Parses an array of strings using <see cref="Parse(string[])"/>.
         /// <para>
@@ -820,6 +833,7 @@ public int Execute(params string[] args)
         /// <param name="cancellationToken"></param>
         /// <returns>The return code from <see cref="Invoke"/>.</returns>
         public async Task<int> ExecuteAsync(string[] args, CancellationToken cancellationToken = default)
+#pragma warning restore RS0026 // Do not add multiple public overloads with optional parameters
         {
             var parseResult = Parse(args);
             var command = parseResult.SelectedCommand;
@@ -900,6 +914,7 @@ public CommandOption HelpOption(string template, bool inherited)
             return OptionHelp;
         }
 
+#pragma warning disable RS0026 // Do not add multiple public overloads with optional parameters
         /// <summary>
         /// Helper method that adds a version option from known versions strings.
         /// </summary>
@@ -908,6 +923,7 @@ public CommandOption HelpOption(string template, bool inherited)
         /// <param name="longFormVersion"></param>
         /// <returns></returns>
         public CommandOption VersionOption(string template,
+#pragma warning restore RS0026 // Do not add multiple public overloads with optional parameters
             string? shortFormVersion,
             string? longFormVersion = null)
         {
@@ -921,6 +937,7 @@ public CommandOption VersionOption(string template,
             }
         }
 
+#pragma warning disable RS0026 // Do not add multiple public overloads with optional parameters
         /// <summary>
         /// Helper method that adds a version option.
         /// </summary>
@@ -929,6 +946,7 @@ public CommandOption VersionOption(string template,
         /// <param name="longFormVersionGetter"></param>
         /// <returns></returns>
         public CommandOption VersionOption(string template,
+#pragma warning restore RS0026 // Do not add multiple public overloads with optional parameters
             Func<string?>? shortFormVersionGetter,
             Func<string?>? longFormVersionGetter = null)
         {
@@ -995,7 +1013,9 @@ public void ShowHelp(bool usePager)
         [Obsolete("This method has been marked as obsolete and will be removed in a future version. " +
             "The recommended replacement is ShowHelp()")]
         [EditorBrowsable(EditorBrowsableState.Never)]
+#pragma warning disable RS0027 // Public API with optional parameter(s) should have the most parameters amongst its public overloads.
         public void ShowHelp(string? commandName = null)
+#pragma warning restore RS0027 // Public API with optional parameter(s) should have the most parameters amongst its public overloads.
         {
             if (commandName == null)
             {

diff --git a/src/CommandLineUtils/CommandLineApplication{T}.cs b/src/CommandLineUtils/CommandLineApplication{T}.cs
@@ -19,11 +19,13 @@ public class CommandLineApplication<TModel> : CommandLineApplication, IModelAcce
         private Func<TModel> _modelFactory = DefaultModelFactory;
 
 #nullable disable
+#pragma warning disable RS0027 // Public API with optional parameter(s) should have the most parameters amongst its public overloads.
         /// <summary>
         /// Initializes a new instance of <see cref="CommandLineApplication"/>.
         /// </summary>
         /// <param name="throwOnUnexpectedArg">Initial value for <see cref="CommandLineApplication.ThrowOnUnexpectedArgument"/>.</param>
         public CommandLineApplication(bool throwOnUnexpectedArg = true)
+#pragma warning restore RS0027 // Public API with optional parameter(s) should have the most parameters amongst its public overloads.
             : base(throwOnUnexpectedArg)
         {
             Initialize();

diff --git a/src/CommandLineUtils/HelpText/HangingIndentWriter.cs b/src/CommandLineUtils/HelpText/HangingIndentWriter.cs
@@ -18,10 +18,10 @@ public class HangingIndentWriter
         /// </summary>
         public const int DefaultConsoleWidth = 80;
 
-        private bool _indentFirstLine;
-        private int _indentSize;
-        private int _maxLineLength;
-        private string _paddedLine;
+        private readonly bool _indentFirstLine;
+        private readonly int _indentSize;
+        private readonly int _maxLineLength;
+        private readonly string _paddedLine;
 
         /// <summary>
         /// A description formatter for dynamically wrapping the description to print in a CLI usage.

diff --git a/src/CommandLineUtils/IO/PhysicalConsole.cs b/src/CommandLineUtils/IO/PhysicalConsole.cs
@@ -17,9 +17,11 @@ public class PhysicalConsole : IConsole
         /// </summary>
         public static IConsole Singleton { get; } = new PhysicalConsole();
 
-        private PhysicalConsole()
-        {
-        }
+        // TODO: in 3.0 make this type truly a singleton by adding a private ctor
+        // this is techinally a breaking change, so wait till 3.0
+        // private PhysicalConsole()
+        // {
+        // }
 
         /// <summary>
         /// <see cref="Console.CancelKeyPress"/>.

diff --git a/src/CommandLineUtils/Internal/ResponseFileParser.cs b/src/CommandLineUtils/Internal/ResponseFileParser.cs
@@ -13,17 +13,13 @@ internal class ResponseFileParser
     {
         public static IList<string> Parse(string filePath, ResponseFileHandling handling)
         {
-            switch (handling)
+            return handling switch
             {
-                case ResponseFileHandling.Disabled:
-                    return new[] { filePath };
-                case ResponseFileHandling.ParseArgsAsSpaceSeparated:
-                    return ParseAsSpaceSeparated(filePath);
-                case ResponseFileHandling.ParseArgsAsLineSeparated:
-                    return ParseAsLineSeparated(filePath);
-                default:
-                    throw new ArgumentOutOfRangeException(nameof(handling));
-            }
+                ResponseFileHandling.Disabled => new[] { filePath },
+                ResponseFileHandling.ParseArgsAsSpaceSeparated => ParseAsSpaceSeparated(filePath),
+                ResponseFileHandling.ParseArgsAsLineSeparated => ParseAsLineSeparated(filePath),
+                _ => throw new ArgumentOutOfRangeException(nameof(handling)),
+            };
         }
 
         private static IList<string> ParseAsLineSeparated(string filePath)

diff --git a/src/CommandLineUtils/McMaster.Extensions.CommandLineUtils.csproj b/src/CommandLineUtils/McMaster.Extensions.CommandLineUtils.csproj
@@ -4,6 +4,11 @@
 
   <PropertyGroup>
     <TargetFrameworks>netstandard2.0;netstandard1.6;net45</TargetFrameworks>
+    <!--
+      The ns1.6 API is a subset of the API available in ns2.0 and net45. The public API tool doesn't handle multiple TFMs.
+      https://github.com/dotnet/roslyn-analyzers/issues/2621
+    -->
+    <DisablePublicApiAnalyzer Condition="'$(TargetFramework)' == 'netstandard1.6'">true</DisablePublicApiAnalyzer>
     <GenerateDocumentationFile>true</GenerateDocumentationFile>
     <IsPackable>true</IsPackable>
     <Description>Command-line parsing API.</Description>