Skip to content

Commit

Permalink
Disable expression parsing, support custom types
Browse files Browse the repository at this point in the history
Fixes #17
  • Loading branch information
carlobaldassi committed Oct 4, 2015
1 parent 555c119 commit d9c1d7a
Show file tree
Hide file tree
Showing 3 changed files with 112 additions and 34 deletions.
54 changes: 43 additions & 11 deletions doc/argparse.rst
Original file line number Diff line number Diff line change
Expand Up @@ -131,7 +131,7 @@ Also, we can see how invoking it with the wrong arguments produces errors::
usage: myprog1.jl [--opt1 OPT1] [-o OPT2] [--flag1] [-h] arg1

$ julia myprog1.jl --opt2 1.5 somearg
invalid argument: 1.5 (must be of type Int64)
invalid argument: 1.5 (conversion to type Int64 failed; you may need to overload ArgParse.parse_item)
usage: myprog1.jl [--opt1 OPT1] [-o OPT2] [--flag1] [-h] arg1

When everything goes fine instead, our program will print the resulting ``Dict``::
Expand All @@ -143,7 +143,7 @@ When everything goes fine instead, our program will print the resulting ``Dict``
opt1 => nothing
flag1 => false

$ julia myprog1.jl --opt1 "2+2" --opt2 "2+2" somearg --flag
$ julia myprog1.jl --opt1 "2+2" --opt2 "4" somearg --flag
Parsed args:
arg1 => somearg
opt2 => 4
Expand All @@ -155,8 +155,8 @@ From these examples, a number of things can be noticed:
* ``opt1`` defaults to ``nothing``, since no ``default`` setting was used for it in ``@add_arg_table``
* ``opt1`` argument type, begin unspecified, defaults to ``Any``, but in practice it's parsed as a
string (e.g. ``"2+2"``)
* ``opt2`` instead has ``Int`` argument type, so ``"2+2"`` will be parsed as an expression and converted
to an integer
* ``opt2`` instead has ``Int`` argument type, so ``"4"`` will be parsed and converted to an integer,
an error is emitted if the conversion fails
* positional arguments can be passed in between options
* long options can be passed in abbreviated form (e.g. ``--flag`` instead of ``--flag1``) as long as
there's no ambiguity
Expand Down Expand Up @@ -423,7 +423,9 @@ This is the list of all available settings:
``:store_arg`` and ``"store_arg"`` are accepted). The default action is ``:store_arg`` unless ``nargs`` is ``0``, in which case the
default is ``:store_true``. See :ref:`this section <argparse-actions-and-nargs>` for a list of all available actions and a detailed
explanation.
* ``arg_type`` (default = ``Any``): the type of the argument. Only makes sense with non-flag arguments.
* ``arg_type`` (default = ``Any``): the type of the argument. Only makes sense with non-flag arguments. Only works out-of-the-box with
string and number types, but see :ref:`this section <argparse-custom-parsing>` for details on how to make it work for general types
(including user-defined ones).
* ``default`` (default = ``nothing``): the default value if the option or positional argument is not parsed. Only makes sense with
non-flag arguments, or when the action is ``:store_const`` or ``:append_const``. Unless it's ``nothing``, it must be consistent with
``arg_type`` and ``range_tester``.
Expand All @@ -445,6 +447,10 @@ This is the list of all available settings:
<argparse-conflicts>`). By default, it follows the general ``error_on_conflict`` settings.
* ``group``: the option group to which the argument will be assigned to (see :ref:`this section <argparse-groups>`). By default, the
current default group is used if specified, otherwise the assignment is automatic.
* ``eval_arg`` (default: ``false``): if ``true``, the argument will be parsed as a Julia expression and evaluated, which means that
for example ``"2+2"`` will yield the integer ``4`` rather than a string. Note that this is a security risk for outside-facing
programs and should generally be avoided: overload `ArgParse.parse_item` instead (see :ref:`this section <argparse-custom-parsing>`).
Only makes sense for non-flag arguments.

.. _argparse-actions-and-nargs:

Expand Down Expand Up @@ -813,6 +819,31 @@ the resolution of most of the conflicts in favor of the newest added entry. The
a command override another command when added with ``@add_arg_table`` (compatible commands are merged
by ``import_settings`` though)

.. _argparse-custom-parsing:

-----------------------
Parsing to custom types
-----------------------

If you specify an ``arg_type`` setting (see :ref:`this section <argparse-arg-entry-settings>`) for an option
or an argument, ``parse_args`` will try to parse it, i.e. to convert the string to the specified type. This
only works for a limited number of types, which can either be directly constructed from strings or be parsed via
the Julia's built-in `parse` function. In order to extend this functionality to other types, including user-defined
custom types, you need to overload the `ArgParse.parse_item` function. Example::

type CustomType
val::Int
end

function ArgParse.parse_item(::Type{CustomType}, x::AbstractString)
return CustomType(parse(Int, x))
end

Note that the second argument needs to be of type `AbstractString` (or `String` in Julia 0.3) to avoid ambiguity
warnings. Also note that if your type is parametric (e.g. ``CustomType{T}``), you need to overload the function
like this: ``function ArgParse.parse_item{T}(::Type{CustomType{T}, x::AbstractString)``.


.. _argparse-details:

---------------
Expand All @@ -827,10 +858,10 @@ always recognized as non-options. However, if the ``allow_ambiguous_opts`` gener
options in the argument table will take precedence: for example, if the option ``-1`` is added, and it takes an
argument, then ``-123`` will be parsed as that option, and ``23`` will be its argument.

Some ambiguities still remains though, because the ``ArgParse`` module will actually accept and parse expressions,
not only numbers, and therefore one may try to pass arguments like ``-e`` or ``-pi``; in that case, these will
always be at risk of being recognized as options. The easiest workaround is to put them in parentheses,
e.g. ``(-e)``.
Some ambiguities still remains though, because the ``ArgParse`` module can actually accept and parse expressions,
not only numbers (although this is not the default), and therefore one may try to pass arguments like ``-e`` or
``-pi``; in that case, these will always be at risk of being recognized as options. The easiest workaround is to
put them in parentheses, e.g. ``(-e)``.

When an option is declared to accept a fixed positive number of arguments or the remainder of the command line
(i.e. if ``nargs`` is a non-zero number, or ``'A'``, or ``'R'``), ``parse_args`` will not try to check if the
Expand All @@ -844,8 +875,9 @@ follows as an argument (i.e. not an option); all which follows goes under the ru
when short option groups are being parsed. For example, if the option in question is ``-x``, then both
``-y -x=-2 4 -y`` and ``-yx-2 4 -y`` will parse ``"-2"`` and ``"4"`` as the arguments of ``-x``.

Finally, since expressions may be evaluated during parsing, note that there is no safeguard against passing
things like ``run(`rm -fr ~`)`` and seeing your data evaporate (don't try that!). Be careful.
Finally, note that with the `eval_arg` setting expressions are evaluated during parsing, which means that there is no
safeguard against passing things like ``run(`rm -fr someimportantthing`)`` and seeing your data evaporate
(don't try that!). Be careful and generally try to avoid using the `eval_arg` setting.

.. _argparse-table-styles:

Expand Down
47 changes: 32 additions & 15 deletions src/ArgParse.jl
Original file line number Diff line number Diff line change
Expand Up @@ -17,6 +17,7 @@ export
set_default_arg_group,
import_settings,
usage_string,
parse_item,
parse_args

import Base: show, getindex, setindex!, haskey
Expand Down Expand Up @@ -110,13 +111,14 @@ type ArgParseField
constant
range_tester::Function
required::Bool
eval_arg::Bool
help::AbstractString
metavar::AbstractString
group::AbstractString
fake::Bool
function ArgParseField()
return new("", AbstractString[], AbstractString[], Any, :store_true, ArgConsumerType(),
nothing, nothing, x->true, false, "", "", "", false)
nothing, nothing, x->true, false, false, "", "", "", false)

This comment has been minimized.

Copy link
@c42f

c42f Oct 4, 2015

Nice, eval_arg is somewhat dangerous but oh-so-convenient for quick hacking. Having it available but turned off by default seems like a good compromise.

end
end

Expand Down Expand Up @@ -707,6 +709,7 @@ end
constant = nothing
required = false
range_tester = x->true
eval_arg = false
dest_name = ""
help = ""
metavar = ""
Expand Down Expand Up @@ -777,7 +780,7 @@ end
found_a_bug()
end
elseif is_opt
append!(valid_keys, [:arg_type, :default, :range_tester, :dest_name, :required, :metavar])
append!(valid_keys, [:arg_type, :default, :range_tester, :dest_name, :required, :metavar, :eval_arg])
nargs.desc == :? && push!(valid_keys, :constant)
elseif action != :command_arg
append!(valid_keys, [:arg_type, :default, :range_tester, :required, :metavar])
Expand Down Expand Up @@ -806,6 +809,7 @@ end
set_if_valid(:required, required)
set_if_valid(:help, help)
set_if_valid(:metavar, metavar)
set_if_valid(:eval_arg, eval_arg)

if !is_flag
isempty(new_arg.metavar) && (new_arg.metavar = auto_metavar(new_arg.dest_name, is_opt))
Expand Down Expand Up @@ -1193,10 +1197,22 @@ end

# parsing aux functions
#{{{
function parse_item_wrapper(it_type::Type, x::AbstractString)
local r::it_type
try
r = parse_item(it_type, x)
catch
argparse_error("invalid argument: $x (conversion to type $it_type failed; you may need to overload ArgParse.parse_item)")

This comment has been minimized.

Copy link
@c42f

c42f Oct 4, 2015

Would you be able to report the details of the exception? It would give the user a better idea of what exactly went wrong during parsing, particularly if the thrown error has a carefully crafted error message.

This comment has been minimized.

Copy link
@c42f

c42f Oct 4, 2015

This isn't just theoretical by the way - I've had it occur in practice that the error message is swallowed, leaving me hunting for the root cause. The same issue occurs in what is now parse_item_eval.

This comment has been minimized.

Copy link
@carlobaldassi

carlobaldassi Oct 4, 2015

Author Owner

seems a good idea: ecb2bdd

This comment has been minimized.

Copy link
@c42f

c42f Oct 4, 2015

Perfect, thanks a lot :)

end
return r
end

parse_item(it_type::Type{Any}, x::AbstractString) = x
parse_item{T<:AbstractString}(it_type::Type{T}, x::AbstractString) = convert(T, x)
function parse_item(it_type::Type, x::AbstractString)
local r
parse_item{T<:Number}(it_type::Type{T}, x::AbstractString) = parse(it_type, x)

function parse_item_eval(it_type::Type, x::AbstractString)
local r::it_type
try
if isempty(x)
y = ""
Expand Down Expand Up @@ -1357,7 +1373,7 @@ function gen_help_text(arg::ArgParseField, settings::ArgParseSettings)
const_str = ""
if !is_command_action(arg.action)
if arg.arg_type != Any && !(arg.arg_type <: AbstractString)
type_str = pre * "(type: " * string(arg.arg_type)
type_str = pre * "(type: " * string_compact(arg.arg_type)
end
if arg.default !== nothing && !isequal(arg.default, [])
mid = isempty(type_str) ? " (" : ", "
Expand Down Expand Up @@ -1693,6 +1709,7 @@ function parse1_optarg(state::ParserState, settings::ArgParseSettings, f::ArgPar
out_dict = state.out_dict

arg_consumed = false
parse_function = f.eval_arg ? parse_item_eval : parse_item_wrapper
command = nothing
is_multi_nargs(f.nargs) && (opt_arg = Array(f.arg_type, 0))
if isa(f.nargs.desc, Int)
Expand All @@ -1703,49 +1720,49 @@ function parse1_optarg(state::ParserState, settings::ArgParseSettings, f::ArgPar
argparse_error("$name requires $num argument", num > 1 ? "s" : "")
end
if rest !== nothing
a = parse_item(f.arg_type, rest)
a = parse_function(f.arg_type, rest)
test_range(f.range_tester, a, name)
push!(opt_arg, a)
arg_consumed = true
end
for i = (1+corr):num
a = parse_item(f.arg_type, shift!(args_list))
a = parse_function(f.arg_type, shift!(args_list))
test_range(f.range_tester, a, name)
push!(opt_arg, a)
end
elseif f.nargs.desc == :A
if rest !== nothing
a = parse_item(f.arg_type, rest)
a = parse_function(f.arg_type, rest)
test_range(f.range_tester, a, name)
opt_arg = a
arg_consumed = true
else
if isempty(args_list)
argparse_error("option $name requires an argument")
end
a = parse_item(f.arg_type, shift!(args_list))
a = parse_function(f.arg_type, shift!(args_list))
test_range(f.range_tester, a, name)
opt_arg = a
end
elseif f.nargs.desc == :?
if rest !== nothing
a = parse_item(f.arg_type, rest)
a = parse_function(f.arg_type, rest)
test_range(f.range_tester, a, name)
opt_arg = a
arg_consumed = true
else
if isempty(args_list)
opt_arg = deepcopy(f.constant)
else
a = parse_item(f.arg_type, shift!(args_list))
a = parse_function(f.arg_type, shift!(args_list))
test_range(f.range_tester, a, name)
opt_arg = a
end
end
elseif f.nargs.desc == :* || f.nargs.desc == :+
arg_found = false
if rest !== nothing
a = parse_item(f.arg_type, rest)
a = parse_function(f.arg_type, rest)
test_range(f.range_tester, a, name)
push!(opt_arg, a)
arg_consumed = true
Expand All @@ -1755,7 +1772,7 @@ function parse1_optarg(state::ParserState, settings::ArgParseSettings, f::ArgPar
if !arg_delim_found && looks_like_an_option(args_list[1], settings)
break
end
a = parse_item(f.arg_type, shift!(args_list))
a = parse_function(f.arg_type, shift!(args_list))
test_range(f.range_tester, a, name)
push!(opt_arg, a)
arg_found = true
Expand All @@ -1765,13 +1782,13 @@ function parse1_optarg(state::ParserState, settings::ArgParseSettings, f::ArgPar
end
elseif f.nargs.desc == :R
if rest !== nothing
a = parse_item(f.arg_type, rest)
a = parse_function(f.arg_type, rest)
test_range(f.range_tester, a, name)
push!(opt_arg, a)
arg_consumed = true
end
while !isempty(args_list)
a = parse_item(f.arg_type, shift!(args_list))
a = parse_function(f.arg_type, shift!(args_list))
test_range(f.range_tester, a, name)
push!(opt_arg, a)
end
Expand Down
45 changes: 37 additions & 8 deletions test/argparse_test3.jl
Original file line number Diff line number Diff line change
@@ -1,5 +1,16 @@
# test 3: dest_name, metavar, range_tester, alternative
# actions
# actions, custom parser

immutable CustomType
end

function ArgParse.parse_item(::Type{CustomType}, x::AbstractString)
@assert x == "custom"
return CustomType()
end

Base.showcompact(io::IO, ::Type{CustomType}) = print(io, "CustomType") # not called on 0.4 due to dispatch bug?
Base.showcompact(io::IO, c::CustomType) = print(io, "CustomType()")

function ap_settings3()

Expand Down Expand Up @@ -31,8 +42,13 @@ function ap_settings3()
help = "provide the answer as floating point"
"--array"
default = [7, 3, 2]
arg_type = Array{Int64,1}
arg_type = Vector{Int64}
eval_arg = true # enables evaluation of the argument. NOTE: security risk!
help = "create an array"
"--custom"
default = CustomType()
arg_type = CustomType # uses the user-defined version of ArgParse.parse_item
help = "the only accepted argument is \"custom\""
"--awkward-option"
nargs = '+' # eats up as many argument as found (at least 1)
action = :append_arg # argument chunks are appended when the option is
Expand All @@ -53,6 +69,7 @@ let s = ap_settings3()

@test stringhelp(s) == """
usage: $(basename(Base.source_path())) [--opt1] [--opt2] [-k] [-u] [--array ARRAY]
[--custom CUSTOM]
[--awkward-option XY [XY...]]
Test 3 for ArgParse.jl
Expand All @@ -64,19 +81,31 @@ let s = ap_settings3()
-u provide the answer as floating point
--array ARRAY create an array (type: Array{Int64,1},
default: [7,3,2])
""" * (VERSION < v"0.4-" ?
"""
--custom CUSTOM the only accepted argument is "custom" (type:
CustomType, default: CustomType())
""" :
"""
--custom CUSTOM the only accepted argument is "custom" (type:
ArgParseTests.CustomType, default:
CustomType())
""") *
"""
--awkward-option XY [XY...]
either X or Y; all XY's are stored in chunks
(default: $(vecanyopen)$(vecanyopen)"X"$(vecanyclose)$(vecanyclose))
"""

@compat @test ap_test3([]) == Dict{AbstractString,Any}("O_stack"=>AbstractString[], "k"=>0, "u"=>0, "array"=>[7, 3, 2], "awk"=>Any[Any["X"]])
@compat @test ap_test3(["--opt1", "--awk", "X", "X", "--opt2", "--opt2", "-k", "-u", "--array=[4]", "--awkward-option=Y", "X", "--opt1"]) ==
Dict{AbstractString,Any}("O_stack"=>AbstractString["O1", "O2", "O2", "O1"], "k"=>42, "u"=>42.0, "array"=>[4], "awk"=>Any[Any["X"], Any["X", "X"], Any["Y", "X"]])
@compat @test ap_test3([]) == Dict{AbstractString,Any}("O_stack"=>AbstractString[], "k"=>0, "u"=>0, "array"=>[7, 3, 2], "custom"=>CustomType(), "awk"=>Any[Any["X"]])
@compat @test ap_test3(["--opt1", "--awk", "X", "X", "--opt2", "--opt2", "-k", "-u", "--array=[4]", "--custom", "custom", "--awkward-option=Y", "X", "--opt1"]) ==
Dict{AbstractString,Any}("O_stack"=>AbstractString["O1", "O2", "O2", "O1"], "k"=>42, "u"=>42.0, "array"=>[4], "custom"=>CustomType(), "awk"=>Any[Any["X"], Any["X", "X"], Any["Y", "X"]])
@ap_test_throws ap_test3(["X"])
@ap_test_throws ap_test3(["--awk", "Z"])
@ap_test_throws ap_test3(["--awk", "-2"])
@ap_test_throws ap_test3(["--array", "7"])
@ap_test_throws ap_test3(["--custom", "default"])

# invalid option name
@ee_test_throws @add_arg_table(s, "-2", action = :store_true)
Expand Down Expand Up @@ -105,9 +134,9 @@ let s = ap_settings3()
# allow ambiguous options
s.allow_ambiguous_opts = true
@add_arg_table(s, "-2", action = :store_true)
@compat @test ap_test3([]) == Dict{AbstractString,Any}("O_stack"=>AbstractString[], "k"=>0, "u"=>0, "array"=>[7, 3, 2], "awk"=>Any[Any["X"]], "2"=>false)
@compat @test ap_test3(["-2"]) == Dict{AbstractString,Any}("O_stack"=>AbstractString[], "k"=>0, "u"=>0, "array"=>[7, 3, 2], "awk"=>Any[["X"]], "2"=>true)
@compat @test ap_test3(["--awk", "X", "-2"]) == Dict{AbstractString,Any}("O_stack"=>AbstractString[], "k"=>0, "u"=>0, "array"=>[7, 3, 2], "awk"=>Any[Any["X"], Any["X"]], "2"=>true)
@compat @test ap_test3([]) == Dict{AbstractString,Any}("O_stack"=>AbstractString[], "k"=>0, "u"=>0, "array"=>[7, 3, 2], "custom"=>CustomType(), "awk"=>Any[Any["X"]], "2"=>false)
@compat @test ap_test3(["-2"]) == Dict{AbstractString,Any}("O_stack"=>AbstractString[], "k"=>0, "u"=>0, "array"=>[7, 3, 2], "custom"=>CustomType(), "awk"=>Any[["X"]], "2"=>true)
@compat @test ap_test3(["--awk", "X", "-2"]) == Dict{AbstractString,Any}("O_stack"=>AbstractString[], "k"=>0, "u"=>0, "array"=>[7, 3, 2], "custom"=>CustomType(), "awk"=>Any[Any["X"], Any["X"]], "2"=>true)
@ap_test_throws ap_test3(["--awk", "X", "-3"])

end

1 comment on commit d9c1d7a

@sjkelly
Copy link

@sjkelly sjkelly commented on d9c1d7a Oct 4, 2015

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

This works great!! Thanks for doing this!

Please sign in to comment.