Refuse unknown options in mob.deploy instead of ignoring them

CHANGELOG.md

@@ -10,6 +10,14 @@ Full module documentation: [hexdocs.pm/mob_dev](https://hexdocs.pm/mob_dev).
 
 ## [Unreleased]
 
+### Fixed
+
+- **`mix mob.deploy` silently ignored unknown options.** `-d` was never aliased
+ to `--device` here — `mob.connect` has aliased it all along — so
+ `mix mob.deploy -d <udid>` deployed to *every* connected device instead of
+ the one named, without a word. A typo'd `--devcie` did the same. Parsing is
+ strict now and the run refuses, naming the offending options.
+
 ### Changed
 
 - **`mix mob.deploy` no longer exits 0 after shipping nothing.** Four ways it

lib/mix/tasks/mob.deploy.ex

@@ -179,7 +179,18 @@ defmodule Mix.Tasks.Mob.Deploy do
 
  @impl Mix.Task
  def run(args) do
- {opts, _, _} = OptionParser.parse(args, switches: @switches)
+ {opts, _argv, invalid} =
+ OptionParser.parse(args, strict: @switches, aliases: [d: :device])
+
+ # `switches:` silently discards anything it does not recognise, so
+ # `mix mob.deploy -d <udid>` — `-d` was never aliased here, though
+ # `mob.connect` has aliased it all along — deployed to every device
+ # instead of the one named, and said nothing. A typo'd `--devcie` did the
+ # same. `strict:` collects them so the run can refuse rather than do
+ # something other than what was asked.
+ unless invalid == [] do
+ Mix.raise(invalid_options_message(invalid))
+ end
 
  # Under --json, stdout must carry ONE document and nothing else. Every
  # progress line in this task, the deployer and the native build is a plain
@@ -410,6 +421,21 @@ defmodule Mix.Tasks.Mob.Deploy do
  end
  end
 
+ @doc """
+ The error for options the task does not accept.
+
+ Names them, because the failure this replaces was silent: the flag was
+ dropped and the deploy proceeded as if it had never been passed.
+ """
+ @spec invalid_options_message([{String.t(), String.t() | nil}]) :: String.t()
+ def invalid_options_message(invalid) do
+ names = invalid |> Enum.map(&elem(&1, 0)) |> Enum.join(", ")
+
+ "Unknown option(s): #{names}\n\n" <>
+ "Run `mix help mob.deploy` for the accepted options. " <>
+ "Short forms are not aliases except `-d` for `--device`."
+ end
+
  @doc """
  The machine-readable result of a finished deploy.
 

test/mix/tasks/mob_deploy_beam_flags_test.exs

@@ -463,4 +463,19 @@ defmodule Mix.Tasks.Mob.DeployBeamFlagsTest do
  assert result |> Jason.encode!() |> Jason.decode!() == result
  end
  end
+
+ describe "unknown options are refused, not discarded" do
+ # `mix mob.deploy -d <udid>` deployed to every device instead of the one
+ # named, and said nothing: `-d` was never aliased here even though
+ # `mob.connect` has aliased it all along, and `switches:` silently drops
+ # what it does not recognise. Found while verifying MOB-151, when two
+ # native builds went to the wrong device for this reason.
+ test "the message names the offending options" do
+ message = Deploy.invalid_options_message([{"-d", nil}, {"--devcie", "x"}])
+
+ assert message =~ "-d"
+ assert message =~ "--devcie"
+ assert message =~ "Unknown option"
+ end
+ end
 end

test/mob_dev/wiring_test.exs

@@ -179,4 +179,33 @@ defmodule MobDev.WiringTest do
  ~r/emit_json\(opts, \[\], \[\], \[\], "Native build failed"\)\s*Mix\.raise/
  end
  end
+
+ describe "mob.deploy refuses what it does not recognise" do
+ # `-d` was never aliased here, and `switches:` drops unknown flags in
+ # silence — so `mix mob.deploy -d <udid>` deployed to every device instead
+ # of the one named. Source assertions because the parse happens inside
+ # run/1, before anything testable is reached.
+ test "-d is aliased to --device, matching mob.connect" do
+ source = @deploy_task
+
+ assert source =~ "aliases: [d: :device]"
+ end
+
+ test "parsing is strict, so nothing is dropped in silence" do
+ # `switches:` puts an unrecognised flag in `invalid` and carries on;
+ # `strict:` is what makes the run able to refuse. Reverting this one word
+ # restores the silent-ignore.
+ source = @deploy_task
+
+ assert source =~ "OptionParser.parse(args, strict: @switches, aliases: [d: :device])"
+ refute source =~ "OptionParser.parse(args, switches: @switches)"
+ end
+
+ test "the run raises rather than proceeding" do
+ source = @deploy_task
+
+ assert source =~
+ ~r/unless invalid == \[\] do\s*Mix\.raise\(invalid_options_message\(invalid\)\)/
+ end
+ end
 end