Add support for having a list of arguments just before the start of t… (

#62)

* Add support for having a list of arguments just before the start of the optional arguments.

Add checks for declaring multiple required collection attributes in the same type.

* Ensure that we detect both the case where the attribute is defined twice and the case when the attribute is not the last one defined.

* Add more tests

* Add more test coverage.

* Remove dead code and add additional test.

* Fix codeFactor violations.

* Remove missing files.

CommandLine.sln

@@ -1,7 +1,7 @@
 
 Microsoft Visual Studio Solution File, Format Version 12.00
-# Visual Studio 15
-VisualStudioVersion = 15.0.26730.0
+# Visual Studio Version 16
+VisualStudioVersion = 16.0.29926.136
 MinimumVisualStudioVersion = 10.0.40219.1
 Project("{9A19103F-16F7-4668-BE54-9A1E7A4F7556}") = "CommandLine", "src\CommandLine.csproj", "{45D20C96-4630-49E9-AFEA-C72CBA265E29}"
 EndProject
@@ -12,8 +12,6 @@ EndProject
 Project("{2150E333-8FDC-42A3-9474-1A3956D46DE8}") = "misc", "misc", "{2B7CA3C0-82A2-4147-BFD6-DADA614D1DB4}"
  ProjectSection(SolutionItems) = preProject
  .gitignore = .gitignore
- appveyor.yml = appveyor.yml
- build-and-test.cmd = build-and-test.cmd
  build.cmd = build.cmd
  build.sh = build.sh
  codecov.yml = codecov.yml

analyzer/CommandLine.Analyzer.Test/CommandLine.Analyzer.Test.csproj

@@ -6,7 +6,6 @@
  </PropertyGroup>
 
  <ItemGroup>
- <PackageReference Include="CommandLine.Net" Version="2.1.2" />
  <PackageReference Include="coverlet.collector" Version="1.2.1">
  <PrivateAssets>all</PrivateAssets>
  <IncludeAssets>runtime; build; native; contentfiles; analyzers; buildtransitive</IncludeAssets>
@@ -20,6 +19,7 @@
  </ItemGroup>
 
  <ItemGroup>
+ <ProjectReference Include="..\..\src\CommandLine.csproj" />
  <ProjectReference Include="..\CommandLine.Analyzer\CommandLine.Analyzer.csproj" />
  </ItemGroup>
 

analyzer/CommandLine.Analyzer.Test/CommandLineUnitTests.cs

@@ -514,7 +514,7 @@ internal class CommandLineOptions
 
  [TestCategory("Analyzer")]
  [TestMethod]
- public void RedirectRequiredArg()
+ public void RequiredCollectionArgShouldBeTheOnlyOne()
  {
  var test = @"
 using CommandLine.Attributes;
@@ -534,7 +534,7 @@ internal class CmdLineArgs
  [RequiredArgument(1, ""repos"", ""The list of repositories where to add the milestones to. The format is: owner\\repoName."", true)]
  public List<string> Repositories { get; set; }
 
- [ArgumentGroup(nameof(Action.List)1)]
+ [ArgumentGroup(nameof(Action.List))]
  [RequiredArgument(1, ""repos2"", ""The list of repositories where to add the milestones to. The format is: owner\\repoName."", true)]
  public List<string> Repositories2 { get; set; }
  }
@@ -547,7 +547,56 @@ public enum Action
 
 ";
 
- VerifyCommandLineDiagnostic(test);
+ var expected = new DiagnosticResult
+ {
+ Id = "CMDNET09",
+ Message = "Both arguments 'repos' and 'repos2' are marked as required collection arguments. Only one can be required. The other should be changed to optional.",
+ Severity = DiagnosticSeverity.Error,
+ Locations =
+ new[] {
+ new DiagnosticResultLocation("Test0.cs", 21, 29)
+ }
+ };
+
+ VerifyCommandLineDiagnostic(test,expected);
+ }
+
+ [TestCategory("Analyzer")]
+ [TestMethod]
+ public void RequiredArgumentShouldBeLast()
+ {
+ var test = @"
+using CommandLine.Attributes;
+using CommandLine.Attributes.Advanced;
+
+ internal class CmdLineArgs
+ {
+ [RequiredArgument(1, ""milestoneInputFile"", ""The file containing the list of milestones to create."")]
+ public string MilestoneFile { get; set; }
+
+ [RequiredArgument(0, ""repos"", ""The list of repositories where to add the milestones to. The format is: owner\\repoName."", true)]
+ public List<string> Repositories { get; set; }
+ }
+
+ public enum Action
+ {
+ Create,
+ List
+ }
+
+";
+ var expected = new DiagnosticResult
+ {
+ Id = "CMDNET08",
+ Message = "The collection argument 'repos' needs to be the last argument in the list. Otherwise, it will not be possible to parse it at runtime.",
+ Severity = DiagnosticSeverity.Error,
+ Locations =
+ new[] {
+ new DiagnosticResultLocation("Test0.cs", 11, 29)
+ }
+ };
+
+ VerifyCommandLineDiagnostic(test, expected);
  }
 
  [TestCategory("Analyzer")]
@@ -621,7 +670,7 @@ public string Action1
 
  VerifyCommandLineDiagnostic(test);
  }
- 
+
  [TestCategory("Analyzer")]
  [TestMethod]
  public void InvalidCode4()
@@ -670,6 +719,91 @@ enum Action
  VerifyCommandLineDiagnostic(test);
  }
 
+ [TestCategory("Analyzer")]
+ [TestMethod]
+ public void TwoRequiredCollectionProperties()
+ {
+ var test = @"
+using CommandLine.Attributes;
+using CommandLine.Attributes.Advanced;
+using System;
+using System.Collections.Generic;
+using System.Linq;
+
+namespace CommandLine.Tests.TestObjects
+{
+ class ComplexType2
+ {
+ [RequiredArgument(0, ""repos"", ""The list of repositories where to add the milestones to."", true)]
+ public List<string> Repositories { get; set; }
+
+ [RequiredArgument(1, ""list"", ""Another list"", true)]
+ public List<string> List { get; set; }
+ }
+}";
+
+ var expected = new DiagnosticResult
+ {
+ Id = "CMDNET09",
+ Message = "Both arguments 'repos' and 'list' are marked as required collection arguments. Only one can be required. The other should be changed to optional.",
+ Severity = DiagnosticSeverity.Error,
+ Locations =
+ new[] {
+ new DiagnosticResultLocation("Test0.cs", 16, 29)
+ }
+ };
+
+ VerifyCommandLineDiagnostic(test, expected);
+ }
+
+ [TestCategory("Analyzer")]
+ [TestMethod]
+ public void TwoCollectionPropertiesOnArgumentGroup()
+ {
+ var test = @"
+using CommandLine.Attributes;
+using CommandLine.Attributes.Advanced;
+
+ internal class CmdLineArgs
+ {
+ [ActionArgument]
+ public Action Action { get; set; }
+
+ [ArgumentGroup(nameof(Action.Create))]
+ [RequiredArgument(0, ""milestoneInputFile"", ""The file containing the list of milestones to create."")]
+ public string MilestoneFile { get; set; }
+
+ [ArgumentGroup(nameof(Action.Create))]
+ [RequiredArgument(1, ""repos"", ""The list of repositories where to add the milestones to. The format is: owner\\repoName."", true)]
+ public List<string> Repositories { get; set; }
+
+ [ArgumentGroup(nameof(Action.Create))]
+ [RequiredArgument(2, ""repos2"", ""The list of repositories where to add the milestones to. The format is: owner\\repoName."", true)]
+ public List<string> Repositories2 { get; set; }
+ }
+
+ public enum Action
+ {
+ Create,
+ List
+ }
+
+";
+
+ var expected = new DiagnosticResult
+ {
+ Id = "CMDNET09",
+ Message = "Both arguments 'repos' and 'repos2' are marked as required collection arguments. Only one can be required. The other should be changed to optional.",
+ Severity = DiagnosticSeverity.Error,
+ Locations =
+ new[] {
+ new DiagnosticResultLocation("Test0.cs", 20, 29)
+ }
+ };
+
+ VerifyCommandLineDiagnostic(test, expected);
+ }
+
  protected override DiagnosticAnalyzer GetCSharpDiagnosticAnalyzer()
  {
  return new CommandLineAnalyzer();

analyzer/CommandLine.Analyzer/CommandLineAnalyzer.Diagnostics.cs

@@ -51,6 +51,12 @@ public partial class CommandLineAnalyzer : DiagnosticAnalyzer
  private static DiagnosticDescriptor RequiredPositionalArgumentNotFound =
  new DiagnosticDescriptor("CMDNET07", "Required argument not found", "The type declares '{0}' properties as required. The property positions are 0-based. Could not find required argument at position '{1}'.", Category, DiagnosticSeverity.Error, isEnabledByDefault: true);
 
+ private static DiagnosticDescriptor CollectionArgumentShouldBeLast =
+ new DiagnosticDescriptor("CMDNET08", "Required collection argument should be the last argument.", "The collection argument '{0}' needs to be the last argument in the list. Otherwise, it will not be possible to parse it at runtime.", Category, DiagnosticSeverity.Error, isEnabledByDefault: true);
+
+ private static DiagnosticDescriptor OnlyOneRequiredCollection =
+ new DiagnosticDescriptor("CMDNET09", "Only one required collection argument is allowed", "Both arguments '{0}' and '{1}' are marked as required collection arguments. Only one can be required. The other should be changed to optional.", Category, DiagnosticSeverity.Error, isEnabledByDefault: true);
+
  #endregion
 
  #region Warnings
@@ -73,7 +79,9 @@ public override ImmutableArray<DiagnosticDescriptor> SupportedDiagnostics
  DuplicateActionArgumentRule,
  ConflictingPropertyDeclarationRule,
  ActionWithoutArgumentsInGroup,
- RequiredPositionalArgumentNotFound
+ RequiredPositionalArgumentNotFound,
+ CollectionArgumentShouldBeLast,
+ OnlyOneRequiredCollection
  );
  }
  }

analyzer/CommandLine.Analyzer/CommandLineAnalyzer.cs

@@ -1,6 +1,7 @@
 using System;
 using System.Collections.Generic;
 using System.Collections.Immutable;
+using System.Collections.ObjectModel;
 using System.Linq;
 using Microsoft.CodeAnalysis;
 using Microsoft.CodeAnalysis.Diagnostics;
@@ -319,6 +320,8 @@ private static void ValidateArguments(List<Argument> args, SymbolAnalysisContext
  HashSet<string> namesOfAllArgs = new HashSet<string>();
  HashSet<int> positionsOrRequiredArgs = new HashSet<int>();
  int numberOfPositionalArgs = 0;
+ int indexOfCollectionArg = -1;
+ RequiredArgument collectionArg = null;
  foreach (var item in args)
  {
  if (item is OptionalArgument)
@@ -351,6 +354,19 @@ private static void ValidateArguments(List<Argument> args, SymbolAnalysisContext
  context.ReportDiagnostic(Diagnostic.Create(DuplicateArgumentNameRule, rag.Symbol.Locations.First(), rag.Name));
  }
 
+ // is the required collection argument the last one AND do we only have one of them?
+ if (indexOfCollectionArg >= 0 && rag.Position > indexOfCollectionArg)
+ {
+ context.ReportDiagnostic(Diagnostic.Create(OnlyOneRequiredCollection, rag.Symbol.Locations.First(), collectionArg.Name, rag.Name));
+ }
+
+ // do we have a collection argument specified?
+ if (rag.IsCollection)
+ {
+ indexOfCollectionArg = rag.Position;
+ collectionArg = rag;
+ }
+
  namesOfAllArgs.Add(rag.Name);
  positionsOrRequiredArgs.Add(rag.Position);
  }
@@ -368,6 +384,12 @@ private static void ValidateArguments(List<Argument> args, SymbolAnalysisContext
  break;
  }
  }
+
+ // Ensure that the required collection argument (if present) is last.
+ if (indexOfCollectionArg >= 0 && indexOfCollectionArg != numberOfPositionalArgs - 1)
+ {
+ context.ReportDiagnostic(Diagnostic.Create(CollectionArgumentShouldBeLast, collectionArg.Symbol.Locations.First(), collectionArg.Name));
+ }
  }
 
  /// <summary>

src/Analysis/ArgumentGroupInfo.cs

@@ -9,5 +9,6 @@ internal class ArgumentGroupInfo
  public Dictionary<int, PropertyInfo> RequiredArguments { get; } = new Dictionary<int, PropertyInfo>();
  public Dictionary<string, PropertyInfo> OptionalArguments { get; } = new Dictionary<string, PropertyInfo>(StringComparer.OrdinalIgnoreCase);
  public Dictionary<PropertyInfo, int> OverridePositions { get; } = new Dictionary<PropertyInfo, int>();
+ public int IndexOfCollectionArgument { get; set; } = -1;
  }
 }

src/Analysis/PropertyHelpers.cs

@@ -54,7 +54,7 @@ public static object GetValueFromArgsArray(string[] args, int offsetInArray, ref
  Array.Copy(args, offsetInArray + currentLogicalPosition, list, 0, indexLastEntry - currentLogicalPosition - offsetInArray);
 
  argValue = new List<string>(list);
- currentLogicalPosition = indexLastEntry;
+ currentLogicalPosition = indexLastEntry - offsetInArray; // we need to take into account the offset in the array here.
  }
 
  return Convert.ChangeType(argValue, targetType.PropertyType);

src/Analysis/TypeHelpers.cs

@@ -49,14 +49,14 @@ public static void ScanTypeForProperties<TOptions>(out TypeArgumentInfo tInfo)
  }
 
  // parse the rest of the properties
- foreach (var property in propertiesOnType)
+ foreach (PropertyInfo property in propertiesOnType)
  {
  // get the group containing this property (note: more than one group can have the same property)
  // this allows common required parameters
 
- var groupsWhereThePropertyIsParticipating = GetGroupsForProperty(tInfo, property);
+ List<ArgumentGroupInfo> groupsWhereThePropertyIsParticipating = GetGroupsForProperty(tInfo, property);
+ List<ActualArgumentAttribute> actualAttribs = property.GetCustomAttributes<ActualArgumentAttribute>().ToList();
 
- var actualAttribs = property.GetCustomAttributes<ActualArgumentAttribute>().ToList();
  if (actualAttribs.Count > 1)
  {
  throw new ArgumentException($"Only one of Required/Optional attribute are allowed per property ({property.Name}). Help information might be incorrect!");
@@ -69,7 +69,7 @@ public static void ScanTypeForProperties<TOptions>(out TypeArgumentInfo tInfo)
  continue;
  }
 
- // add the property to add the groups it is a part of
+ // add the property to the groups it is a part of
  if (baseAttrib is RequiredArgumentAttribute)
  {
  foreach (ArgumentGroupInfo grpPropInfo in groupsWhereThePropertyIsParticipating)
@@ -85,6 +85,17 @@ public static void ScanTypeForProperties<TOptions>(out TypeArgumentInfo tInfo)
  throw new ArgumentException("Two required arguments share the same position!!");
  }
 
+ // if we have a collection argument, keep track of it
+ if (baseAttrib.IsCollection)
+ {
+ // if we already have defined a required collection argument, throw an error
+ if (grpPropInfo.IndexOfCollectionArgument >= 0)
+ {
+ throw new ArgumentException("Cannot declare two required collection arguments. Please convert one of them to an optional collection one.");
+ }
+ grpPropInfo.IndexOfCollectionArgument = requiredPositionIndex;
+ }
+
  grpPropInfo.RequiredArguments[requiredPositionIndex] = property;
  }
  }
@@ -102,13 +113,28 @@ public static void ScanTypeForProperties<TOptions>(out TypeArgumentInfo tInfo)
  }
  }
 
- ArgumentGroupInfo grp;
  // remove the empty one, if empty
- if (tInfo.ArgumentGroups.TryGetValue(string.Empty, out grp))
+ if (tInfo.ArgumentGroups.TryGetValue(string.Empty, out ArgumentGroupInfo grp))
  {
  if (grp.OptionalArguments.Count == 0 && grp.RequiredArguments.Count == 0)
  tInfo.ArgumentGroups.Remove(string.Empty);
  }
+
+ // validate that the collection is the last argument
+ foreach (KeyValuePair<string, ArgumentGroupInfo> group in tInfo.ArgumentGroups)
+ {
+ if (group.Value.IndexOfCollectionArgument >= 0 && group.Value.IndexOfCollectionArgument != group.Value.RequiredArguments.Count - 1)
+ {
+ if (!string.IsNullOrEmpty(group.Key))
+ {
+ throw new ArgumentException($"The required collection argument for group `{group.Key}` needs to be on the last position of the required arguments.");
+ }
+ else
+ {
+ throw new ArgumentException("The required collection argument needs to be on the last position of the required arguments.");
+ }
+ }
+ }
  }
 
  private static List<ArgumentGroupInfo> GetGroupsForProperty(TypeArgumentInfo tInfo, PropertyInfo property)

src/CommandLine.csproj

@@ -7,9 +7,9 @@
  </PropertyGroup>
 
  <PropertyGroup>
- <AssemblyVersion>2.1.3</AssemblyVersion>
+ <AssemblyVersion>2.2.0</AssemblyVersion>
  <FileVersion>$(AssemblyVersion)</FileVersion>
- <VersionPrefix>2.1.3</VersionPrefix>
+ <VersionPrefix>2.2.0</VersionPrefix>
  </PropertyGroup>
 
  <PropertyGroup>