Fail CLI when parse fail and Argu switch detected.

* Fail build when Argu parse fails AND Argu switches detected. See fsprojects#1090. * Fallback to old if parse fails and no Argu switches. * Also as WIP function to map from old to new. NOT TESTED YET OR IN * USE. * Add Argu to test project for tests.
bentayloruk · Jun 23, 2017 · a907344 · a907344
1 parent 15e1b74
commit a907344
Show file tree

Hide file tree

Showing 5 changed files with 121 additions and 16 deletions.
diff --git a/src/app/FAKE/Cli.fs b/src/app/FAKE/Cli.fs
@@ -30,13 +30,39 @@ type FakeArg =
             | Single_Target -> "Runs only the specified target and not the dependencies."
             | NoCache -> "Disables caching of compiled script"
 
-/// Return the parsed FAKE args or the parse exception.
-let parsedArgsOrEx args =
-    try
-        let args = args |> Seq.skip 1 |> Array.ofSeq
-        let parser = ArgumentParser.Create<FakeArg>()
-        Choice1Of2(parser.Parse(args))
-    with | ex -> Choice2Of2(ex)
+open Microsoft.FSharp.Reflection
+
+/// List of all the argu switches and alt switches.
+let arguSwitches = 
+    lazy(let switchUnionCases = FSharpType.GetUnionCases(typeof<FakeArg>)
+         let arguSwitchesFromUnionCase (unionCase : UnionCaseInfo) =
+            [ yield "--" + unionCase.Name.Replace("_", "-")
+              yield! unionCase.GetCustomAttributes(typeof<AltCommandLineAttribute>)
+                    |> Seq.cast<AltCommandLineAttribute>
+                    |> Seq.map (fun altAttr -> altAttr.Names)
+                    |> Seq.concat
+            ]
+         switchUnionCases |> Seq.map arguSwitchesFromUnionCase |> Seq.concat)
+
+/// Checks to see if arg starts with any of the argu or alt argu switches.
+let isArguSwitch (arg : string) = 
+    arguSwitches.Value |> Seq.exists (fun arguSwitch -> arg.StartsWith(arguSwitch, StringComparison.InvariantCultureIgnoreCase))
+
+/// Represents a parse result, with fallback context.
+type ArguParseResult =
+    | OK of ParseResults<FakeArg>
+    | FailWithArguSwitches of Exception
+    | FailWithoutArguSwitches of Exception
+
+/// Return the parsed Argu FAKE args or the parse exception with context.
+let tryParseArguArgs args = 
+    let args = args |> Seq.skip 1 |> Array.ofSeq
+    let parser = ArgumentParser.Create<FakeArg>()
+    try OK(parser.Parse(args))
+    with
+    | ex ->
+        if args |> Seq.map isArguSwitch |> Seq.exists id then FailWithArguSwitches ex
+        else FailWithoutArguSwitches ex
 
 /// Prints the FAKE argument usage.
 let printUsage () =

diff --git a/src/app/FAKE/Program.fs b/src/app/FAKE/Program.fs
@@ -46,10 +46,10 @@ try
 
         let args = Cli.parsePositionalArgs cmdArgs
 
-        match Cli.parsedArgsOrEx args.Rest with
+        match Cli.tryParseArguArgs args.Rest with
 
         //We have new style help args!
-        | Choice1Of2(fakeArgs) ->
+        | Cli.ArguParseResult.OK(fakeArgs) ->
 
             //Break to allow a debugger to be attached here
             if fakeArgs.Contains <@ Cli.Break @> then
@@ -121,13 +121,13 @@ try
 
                 ()
 
-        //None of the new style args parsed, so revert to the old skool.
-        | Choice2Of2(ex) ->
+        // Failed to parse args AND Argu switches detected, so error.
+        | Cli.ArguParseResult.FailWithArguSwitches(ex) ->
+            traceError "Failed to parse command line args.  IMPORTANT:  FAKE supports an 'old' and 'new' CLI.  A new style switch has been detected in your arguments, but the new style parse is failing.  Please see the error and usage help below. "
+            traceException ex
 
-            // #1082 print a warning as we've been invoked with invalid OR old-style args.
-            // traceImportant "Error parsing command line arguments.  You have a mistake in your args, or are using the pre-2.1.8 argument style:"
-            // exceptionAndInnersToString ex |> traceImportant
-            // trace "Attempting to run with pre-version 2.18 argument style, for backwards compat."
+        // Failed to parse args but NO Argu switches detected, so try old format.
+        | Cli.ArguParseResult.FailWithoutArguSwitches(ex) ->
 
             if (cmdArgs.Length = 2 && paramIsHelp cmdArgs.[1]) || (cmdArgs.Length = 1 && List.length buildScripts = 0) then printUsage () else
             match Boot.ParseCommandLine(cmdArgs) with

diff --git a/src/test/Test.FAKECore/CliSpecs.cs b/src/test/Test.FAKECore/CliSpecs.cs
@@ -27,4 +27,16 @@ public class when_parsing_the_fake_cli
         It should_parse_rest_of_args_when_no_positional =
             () => Cli.parsePositionalArgs(new string[] { "fake.exe", "-a", "-b", "-c" }).Rest.Length.ShouldEqual(4);
     }
+
+    public class when_parsing_the_fake_cli_with_old_and_new_arg_style
+    {
+        It should_fail_with_argu_switches_when_mixed_usage =
+            () => Cli.tryParseArguArgs(new [] { "fake.exe", "script.fsx", "blahFlag", "blahVar=blahdyblah", "-st" }).IsFailWithArguSwitches.ShouldBeTrue();
+
+        It should_fail_without_argu_switches_when_bad_switch =
+            () => Cli.tryParseArguArgs(new [] { "fake.exe", "script.fsx", "--madeUpSwitch", "madeUpValue" }).IsFailWithoutArguSwitches.ShouldBeTrue();
+
+        It should_fail_with_valid_old_style_args =
+            () => Cli.tryParseArguArgs(new string[] { "fake.exe", "script.fsx", "clean" }).IsFailWithoutArguSwitches.ShouldBeTrue();
+    }
 }
diff --git a/src/test/Test.FAKECore/Test.FAKECore.csproj b/src/test/Test.FAKECore/Test.FAKECore.csproj
@@ -413,6 +413,72 @@
   -->
   <Choose>
     <When Condition="$(TargetFrameworkIdentifier) == '.NETFramework' And ($(TargetFrameworkVersion) == 'v4.5' Or $(TargetFrameworkVersion) == 'v4.5.1' Or $(TargetFrameworkVersion) == 'v4.5.2' Or $(TargetFrameworkVersion) == 'v4.5.3' Or $(TargetFrameworkVersion) == 'v4.6' Or $(TargetFrameworkVersion) == 'v4.6.1' Or $(TargetFrameworkVersion) == 'v4.6.2' Or $(TargetFrameworkVersion) == 'v4.6.3' Or $(TargetFrameworkVersion) == 'v4.7')">
+    <When Condition="$(TargetFrameworkIdentifier) == '.NETFramework' And $(TargetFrameworkVersion) == 'v3.5'">
+      <ItemGroup>
+        <Reference Include="Argu">
+          <HintPath>..\..\..\packages\Argu\lib\net35\Argu.dll</HintPath>
+          <Private>True</Private>
+          <Paket>True</Paket>
+        </Reference>
+      </ItemGroup>
+    </When>
+    <When Condition="$(TargetFrameworkIdentifier) == '.NETFramework' And ($(TargetFrameworkVersion) == 'v4.0' Or $(TargetFrameworkVersion) == 'v4.5' Or $(TargetFrameworkVersion) == 'v4.5.1' Or $(TargetFrameworkVersion) == 'v4.5.2' Or $(TargetFrameworkVersion) == 'v4.5.3' Or $(TargetFrameworkVersion) == 'v4.6' Or $(TargetFrameworkVersion) == 'v4.6.1')">
+      <ItemGroup>
+        <Reference Include="Argu">
+          <HintPath>..\..\..\packages\Argu\lib\net40\Argu.dll</HintPath>
+          <Private>True</Private>
+          <Paket>True</Paket>
+        </Reference>
+      </ItemGroup>
+    </When>
+  </Choose>
+  <Choose>
+    <When Condition="$(TargetFrameworkIdentifier) == '.NETFramework' And ($(TargetFrameworkVersion) == 'v2.0' Or $(TargetFrameworkVersion) == 'v3.0' Or $(TargetFrameworkVersion) == 'v3.5')">
+      <ItemGroup>
+        <Reference Include="FSharp.Core">
+          <HintPath>..\..\..\packages\FSharp.Core\lib\net20\FSharp.Core.dll</HintPath>
+          <Private>True</Private>
+          <Paket>True</Paket>
+        </Reference>
+      </ItemGroup>
+    </When>
+    <When Condition="$(TargetFrameworkIdentifier) == '.NETFramework' And ($(TargetFrameworkVersion) == 'v4.0' Or $(TargetFrameworkVersion) == 'v4.5' Or $(TargetFrameworkVersion) == 'v4.5.1' Or $(TargetFrameworkVersion) == 'v4.5.2' Or $(TargetFrameworkVersion) == 'v4.5.3' Or $(TargetFrameworkVersion) == 'v4.6' Or $(TargetFrameworkVersion) == 'v4.6.1')">
+      <ItemGroup>
+        <Reference Include="FSharp.Core">
+          <HintPath>..\..\..\packages\FSharp.Core\lib\net40\FSharp.Core.dll</HintPath>
+          <Private>True</Private>
+          <Paket>True</Paket>
+        </Reference>
+      </ItemGroup>
+    </When>
+    <When Condition="($(TargetFrameworkIdentifier) == '.NETCore') Or ($(TargetFrameworkIdentifier) == 'Xamarin.Mac') Or ($(TargetFrameworkProfile) == 'Profile7') Or ($(TargetFrameworkProfile) == 'Profile44')">
+      <ItemGroup>
+        <Reference Include="FSharp.Core">
+          <HintPath>..\..\..\packages\FSharp.Core\lib\portable-net45+netcore45\FSharp.Core.dll</HintPath>
+          <Private>True</Private>
+          <Paket>True</Paket>
+        </Reference>
+      </ItemGroup>
+    </When>
+    <When Condition="($(TargetFrameworkIdentifier) == 'MonoAndroid') Or ($(TargetFrameworkIdentifier) == 'MonoTouch') Or ($(TargetFrameworkIdentifier) == 'Xamarin.iOS')">
+      <ItemGroup>
+        <Reference Include="FSharp.Core">
+          <HintPath>..\..\..\packages\FSharp.Core\lib\portable-net45+monoandroid10+monotouch10+xamarinios10\FSharp.Core.dll</HintPath>
+          <Private>True</Private>
+          <Paket>True</Paket>
+        </Reference>
+      </ItemGroup>
+    </When>
+    <When Condition="($(TargetFrameworkIdentifier) == 'Silverlight' And $(TargetFrameworkVersion) == 'v5.0') Or ($(TargetFrameworkProfile) == 'Profile24') Or ($(TargetFrameworkProfile) == 'Profile47')">
+      <ItemGroup>
+        <Reference Include="FSharp.Core">
+          <HintPath>..\..\..\packages\FSharp.Core\lib\portable-net45+sl5+netcore45\FSharp.Core.dll</HintPath>
+          <Private>True</Private>
+          <Paket>True</Paket>
+        </Reference>
+      </ItemGroup>
+    </When>
+    <When Condition="($(TargetFrameworkIdentifier) == 'WindowsPhone' And ($(TargetFrameworkVersion) == 'v8.0' Or $(TargetFrameworkVersion) == 'v8.1')) Or ($(TargetFrameworkProfile) == 'Profile31') Or ($(TargetFrameworkProfile) == 'Profile49') Or ($(TargetFrameworkProfile) == 'Profile78')">
       <ItemGroup>
         <Reference Include="FSharp.Core">
           <HintPath>..\..\..\packages\FSharp.Core\lib\net45\FSharp.Core.dll</HintPath>

diff --git a/src/test/Test.FAKECore/paket.references b/src/test/Test.FAKECore/paket.references
@@ -2,4 +2,5 @@ FSharp.Core
 Machine.Specifications.Should
 Mono.Web.Xdt
 Mono.Cecil
-Nuget.Core
+Nuget.Core
+Argu