Merge pull request #3261 from barucden/remove-dict-ctor

Remove weird constructors
JuliaLang · Nov 28, 2022 · d5ac7ca · d5ac7ca
2 parents 742c852 + e50d8bf
commit d5ac7ca
Show file tree

Hide file tree

Showing 4 changed files with 45 additions and 34 deletions.
diff --git a/src/REPLMode/REPLMode.jl b/src/REPLMode/REPLMode.jl
@@ -32,7 +32,7 @@ end
 
 # TODO assert names matching lex regex
 # assert now so that you don't fail at user time
-# see function `REPLMode.APIOptions`
+# see function `REPLMode.api_options`
 function OptionSpec(;name::String,
                     short_name::Union{Nothing,String}=nothing,
                     takes_arg::Bool=false,
@@ -293,20 +293,23 @@ parse(input::String) =
 #------------#
 # APIOptions #
 #------------#
+
+# Do NOT introduce a constructor for APIOptions
+# as long as it's an alias for Dict
 const APIOptions = Dict{Symbol, Any}
-function APIOptions(options::Vector{Option},
-                    specs::Dict{String, OptionSpec},
-                    )::APIOptions
-    api_options = Dict{Symbol, Any}()
+function api_options(options::Vector{Option},
+                     specs::Dict{String, OptionSpec})
+    api_opts = APIOptions()
     enforce_option(options, specs)
     for option in options
         spec = specs[option.val]
-        api_options[spec.api.first] = spec.takes_arg ?
+        api_opts[spec.api.first] = spec.takes_arg ?
             spec.api.second(option.argument) :
             spec.api.second
     end
-    return api_options
+    return api_opts
 end
+
 Context!(ctx::APIOptions)::Context = Types.Context!(collect(ctx))
 
 #---------#
@@ -361,7 +364,7 @@ This step is distinct from `parse` in that it relies on the command specificatio
 """
 function Command(statement::Statement)::Command
     # options
-    options = APIOptions(statement.options, statement.spec.option_specs)
+    options = api_options(statement.options, statement.spec.option_specs)
     # arguments
     arg_spec = statement.spec.argument_spec
     arguments = arg_spec.parser(statement.arguments, options)

diff --git a/src/REPLMode/argument_parsers.jl b/src/REPLMode/argument_parsers.jl
@@ -1,5 +1,32 @@
 import ..isdir_nothrow, ..Registry.RegistrySpec, ..isurl
 
+struct PackageIdentifier
+    val::String
+end
+
+struct VersionToken
+    version::String
+end
+
+struct Rev
+    rev::String
+end
+
+struct Subdir
+    dir::String
+end
+
+const PackageToken = Union{PackageIdentifier,
+                           VersionToken,
+                           Rev,
+                           Subdir}
+
+packagetoken(word::String)::PackageToken =
+    first(word) == '@' ? VersionToken(word[2:end]) :
+    first(word) == '#' ? Rev(word[2:end]) :
+    first(word) == ':' ? Subdir(word[2:end]) :
+    PackageIdentifier(word)
+
 ###############
 # PackageSpec #
 ###############
@@ -17,26 +44,11 @@ function parse_package(args::Vector{QString}, options; add_or_dev=false)::Vector
             push!(words, word)
         end
     end
-    args = PackageToken[PackageToken(pkgword) for pkgword in words]
+    args = PackageToken[packagetoken(pkgword) for pkgword in words]
 
     return parse_package_args(args; add_or_dev=add_or_dev)
 end
 
-struct VersionToken
-    version::String
-end
-
-struct Rev
-    rev::String
-end
-
-struct Subdir
-    dir::String
-end
-
-const PackageIdentifier = String
-const PackageToken = Union{PackageIdentifier, VersionToken, Rev, Subdir}
-
     # Match a git repository URL. This includes uses of `@` and `:` but
     # requires that it has `.git` at the end.
 let url = raw"((git|ssh|http(s)?)|(git@[\w\-\.]+))(:(//)?)([\w\.@\:/\-~]+)(\.git$)(/)?",
@@ -78,12 +90,6 @@ function package_lex(qwords::Vector{QString})::Vector{String}
     return words
 end
 
-PackageToken(word::String)::PackageToken =
-    first(word) == '@' ? VersionToken(word[2:end]) :
-    first(word) == '#' ? Rev(word[2:end]) :
-    first(word) == ':' ? Subdir(word[2:end]) :
-    String(word)
-
 function parse_package_args(args::Vector{PackageToken}; add_or_dev=false)::Vector{PackageSpec}
     # check for and apply PackageSpec modifier (e.g. `#foo` or `@v1.0.2`)
     function apply_modifier!(pkg::PackageSpec, args::Vector{PackageToken})
@@ -131,7 +137,8 @@ let uuid = raw"(?i)[0-9a-z]{8}-[0-9a-z]{4}-[0-9a-z]{4}-[0-9a-z]{4}-[0-9a-z]{12}(
 end
 # packages can be identified through: uuid, name, or name+uuid
 # additionally valid for add/develop are: local path, url
-function parse_package_identifier(word::AbstractString; add_or_develop=false)::PackageSpec
+function parse_package_identifier(pkg_id::PackageIdentifier; add_or_develop=false)::PackageSpec
+    word = pkg_id.val
     if add_or_develop
         if isurl(word)
             return PackageSpec(; url=word)

diff --git a/src/REPLMode/completions.jl b/src/REPLMode/completions.jl
@@ -173,7 +173,7 @@ function complete_argument(spec::CommandSpec, options::Vector{String},
     # finish parsing opts
     local opts
     try
-        opts = APIOptions(map(parse_option, options), spec.option_specs)
+        opts = api_options(map(parse_option, options), spec.option_specs)
     catch e
         e isa PkgError && return String[]
         rethrow()

diff --git a/test/repl.jl b/test/repl.jl
@@ -571,8 +571,9 @@ temp_pkg_dir() do project_path
 end
 
 @testset "parse package url win" begin
-    @test typeof(Pkg.REPLMode.parse_package_identifier("https://github.com/abc/ABC.jl";
-                                                       add_or_develop=true)) == Pkg.Types.PackageSpec
+    pkg_id = Pkg.REPLMode.PackageIdentifier("https://github.com/abc/ABC.jl")
+    pkg_spec = Pkg.REPLMode.parse_package_identifier(pkg_id; add_or_develop=true)
+    @test typeof(pkg_spec) == Pkg.Types.PackageSpec
 end
 
 @testset "parse git url (issue #1935) " begin