Skip to content

Commit

Permalink
Fail CLI when parse fail and Argu switch detected.
Browse files Browse the repository at this point in the history
* 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.
  • Loading branch information
bentayloruk committed Feb 4, 2016
1 parent d381c1f commit 1b93bce
Show file tree
Hide file tree
Showing 5 changed files with 108 additions and 16 deletions.
73 changes: 66 additions & 7 deletions src/app/FAKE/Cli.fs
Original file line number Diff line number Diff line change
Expand Up @@ -30,13 +30,72 @@ 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

/// Convert old style args to the Argu style.
/// NOTE: Will stop mapping if encounters any switch other than -d:. -d is mapped as was supported in the old style.
let experimentalTryMapOldArgsWip (args : string list) =
// Extract the old style fsi options first.
let isOldStyleFsiOptionArg (arg : string) = arg.StartsWith("-d:", StringComparison.InvariantCultureIgnoreCase)
let oldStyleFsiOptions = args |> List.filter isOldStyleFsiOptionArg
let nonFsiOptionArgs = args |> List.filter (fun arg -> not (isOldStyleFsiOptionArg arg))

// Function to map the other args, if we can.
let rec inner (args : string list) accFake =
match args with
| arg::args ->
if arg.StartsWith("-") then
// Stop processing, as we can't reliably map Argu switches without replicating the parse logic!
let unmappedArgs = arg::args
(List.rev accFake) @ unmappedArgs
elif arg.Contains("=") then
let parts = arg.Split([| '=' |])
if parts.[0].Equals("logfile", StringComparison.InvariantCultureIgnoreCase) then
inner args (parts.[1] :: "--logfile" :: accFake)
else
inner args (parts.[1] :: "--envvar" :: accFake)
else
inner args (arg :: "--envflag" :: accFake)
| [] -> List.rev accFake

let mappedArgs = inner nonFsiOptionArgs []
let fsiArgs =
if not oldStyleFsiOptions.IsEmpty then
mappedArgs @ [ yield "--fsiargs"; yield! oldStyleFsiOptions]
else []
mappedArgs @ fsiArgs

/// 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 () =
Expand Down
16 changes: 8 additions & 8 deletions src/app/FAKE/Program.fs
Original file line number Diff line number Diff line change
Expand Up @@ -45,10 +45,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
Expand Down Expand Up @@ -120,13 +120,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
Expand Down
12 changes: 12 additions & 0 deletions src/test/Test.FAKECore/CliSpecs.cs
Original file line number Diff line number Diff line change
Expand Up @@ -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();
}
}
20 changes: 20 additions & 0 deletions src/test/Test.FAKECore/Test.FAKECore.csproj
Original file line number Diff line number Diff line change
Expand Up @@ -407,6 +407,26 @@
<Target Name="AfterBuild">
</Target>
-->
<Choose>
<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>
Expand Down
3 changes: 2 additions & 1 deletion src/test/Test.FAKECore/paket.references
Original file line number Diff line number Diff line change
Expand Up @@ -2,4 +2,5 @@ FSharp.Core
Machine.Specifications.Should
Mono.Web.Xdt
Mono.Cecil
Nuget.Core
Nuget.Core
Argu

0 comments on commit 1b93bce

Please sign in to comment.