Merge pull request #2311 from alco/option-parser-fixes

Do not assume invalid options are of boolean type

Author: José Valim

lib/elixir/lib/option_parser.ex

@@ -216,13 +216,13 @@ defmodule OptionParser do
 
   defp next(["-" <> option|rest], aliases, switches, strict) do
     {option, value} = split_option(option)
-    opt = tag_option(option, value, switches, aliases)
+    tagged = tag_option(option, value, switches, aliases)
 
-    if strict and not option_defined?(opt, switches) do
-      {_, opt_name} = opt
+    if strict and not option_defined?(tagged, switches) do
+      {_, opt_name} = tagged
       {:undefined, opt_name, value, rest}
     else
-      {opt_name, kinds, value} = normalize_option(opt, value, switches)
+      {opt_name, kinds, value} = normalize_option(tagged, value, switches)
       {value, kinds, rest} = normalize_value(value, kinds, rest, strict)
       case validate_option(opt_name, value, kinds) do
         {:ok, new_value} -> {:ok, opt_name, new_value, rest}
@@ -330,7 +330,7 @@ defmodule OptionParser do
       kinds == [] ->
         {option, kinds, true}
       true ->
-        {option, [:invalid], false}
+        {reverse_negated(option), [:invalid], nil}
     end
   end
 
@@ -343,17 +343,17 @@ defmodule OptionParser do
   end
 
   defp normalize_value(nil, kinds, t, strict) do
-    null = if strict, do: nil, else: true
+    nil_or_true = if strict, do: nil, else: true
     cond do
       :boolean in kinds ->
         {true, kinds, t}
       value_in_tail?(t) ->
         [h|t] = t
         {h, kinds, t}
       kinds == [] ->
-        {null, kinds, t}
+        {nil_or_true, kinds, t}
       true ->
-        {null, [:invalid], t}
+        {nil, [:invalid], t}
     end
   end
 
@@ -382,6 +382,10 @@ defmodule OptionParser do
     option |> to_underscore |> String.to_atom
   end
 
+  defp reverse_negated(negated) do
+    String.to_atom("no_" <> Atom.to_string(negated))
+  end
+
   defp get_negated("no-" <> rest = option, value, switches) do
     negated = get_option(rest)
     option  = if Keyword.has_key?(switches, negated) and value == nil do

lib/elixir/test/elixir/option_parser_test.exs

@@ -42,6 +42,15 @@ defmodule OptionParserTest do
            == {[source: "from_docs/"], ["other"], []}
   end
 
+  test "does not interpret undefined options with value as boolean" do
+    assert OptionParser.parse(["--no-bool"])
+           == {[no_bool: true], [], []}
+    assert OptionParser.parse(["--no-bool"], strict: [])
+           == {[], [], [no_bool: nil]}
+    assert OptionParser.parse(["--no-bool=...", "other"])
+           == {[], ["other"], [no_bool: "..."]}
+  end
+
   test "does not parse -- as an alias" do
     assert OptionParser.parse(["--s=from_docs/"], aliases: [s: :source])
            == {[s: "from_docs/"], [], []}
@@ -83,7 +92,7 @@ defmodule OptionParserTest do
            == {[require: "foo", require: "bar"], ["baz"], []}
 
     assert OptionParser.parse(["--require"], switches: [require: :keep])
-           == {[], [], [require: true]}
+           == {[], [], [require: nil]}
   end
 
   test "parses configured strings" do
@@ -92,9 +101,9 @@ defmodule OptionParserTest do
     assert OptionParser.parse(["--value=1", "foo"], switches: [value: :string])
            == {[value: "1"], ["foo"], []}
     assert OptionParser.parse(["--value"], switches: [value: :string])
-           == {[], [], [value: true]}
+           == {[], [], [value: nil]}
     assert OptionParser.parse(["--no-value"], switches: [value: :string])
-           == {[], [], [value: false]}
+           == {[], [], [no_value: nil]}
   end
 
   test "parses configured integers" do