Fix string escaping in REPL completion of paths

REPL completion of paths within strings need to be escaped according to
the usual escaping rules, and delimited by the starting " rather than
whitespace.

This differs from completion of paths within cmd backticks which need to
be escaped according to shell escaping rules.

Separate these cases and fix string escaping.

This was found because JuliaSyntax emits an Expr(:error) rather than
Expr(:incomplete) for paths inside strings with invalid escape sequences
before whitespace.

Author: c42f

stdlib/REPL/src/REPLCompletions.jl

@@ -232,7 +232,10 @@ function complete_keyword(s::Union{String,SubString{String}})
     Completion[KeywordCompletion(kw) for kw in sorted_keywords[r]]
 end
 
-function complete_path(path::AbstractString, pos::Int; use_envpath=false, shell_escape=false)
+function complete_path(path::AbstractString, pos::Int;
+                       use_envpath=false, shell_escape=false,
+                       string_escape=false)
+    @assert !(shell_escape && string_escape)
     if Base.Sys.isunix() && occursin(r"^~(?:/|$)", path)
         # if the path is just "~", don't consider the expanded username as a prefix
         if path == "~"
@@ -259,9 +262,9 @@ function complete_path(path::AbstractString, pos::Int; use_envpath=false, shell_
     matches = Set{String}()
     for file in files
         if startswith(file, prefix)
-            id = try isdir(joinpath(dir, file)) catch; false end
-            # joinpath is not used because windows needs to complete with double-backslash
-            push!(matches, id ? file * (@static Sys.iswindows() ? "\\\\" : "/") : file)
+            p = joinpath(dir, file)
+            is_dir = try isdir(p) catch; false end
+            push!(matches, is_dir ? joinpath(file, "") : file)
         end
     end
 
@@ -307,8 +310,14 @@ function complete_path(path::AbstractString, pos::Int; use_envpath=false, shell_
         end
     end
 
-    matchList = Completion[PathCompletion(shell_escape ? replace(s, r"\s" => s"\\\0") : s) for s in matches]
-    startpos = pos - lastindex(prefix) + 1 - count(isequal(' '), prefix)
+    function do_escape(s)
+        return shell_escape ? replace(s, r"(\s|\\)" => s"\\\0") :
+               string_escape ? escape_string(s, ('\"','$')) :
+               s
+    end
+
+    matchList = Completion[PathCompletion(do_escape(s)) for s in matches]
+    startpos = pos - lastindex(do_escape(prefix)) + 1
     # The pos - lastindex(prefix) + 1 is correct due to `lastindex(prefix)-lastindex(prefix)==0`,
     # hence we need to add one to get the first index. This is also correct when considering
     # pos, because pos is the `lastindex` a larger string which `endswith(path)==true`.
@@ -767,7 +776,7 @@ end
 function close_path_completion(str, startpos, r, paths, pos)
     length(paths) == 1 || return false  # Only close if there's a single choice...
     _path = str[startpos:prevind(str, first(r))] * (paths[1]::PathCompletion).path
-    path = expanduser(replace(_path, r"\\ " => " "))
+    path = expanduser(unescape_string(replace(_path, "\\\$"=>"\$", "\\\""=>"\"")))
     # ...except if it's a directory...
     try
         isdir(path)
@@ -1039,23 +1048,44 @@ function completions(string::String, pos::Int, context_module::Module=Main, shif
         dotpos = something(findprev(isequal('.'), string, first(varrange)-1), 0)
         return complete_identifiers!(Completion[], ffunc, context_module, string,
             string[startpos:pos], pos, dotpos, startpos)
-    # otherwise...
-    elseif inc_tag in [:cmd, :string]
+    elseif inc_tag === :cmd
         m = match(r"[\t\n\r\"`><=*?|]| (?!\\)", reverse(partial))
         startpos = nextind(partial, reverseind(partial, m.offset))
         r = startpos:pos
 
+        # This expansion with "\\ "=>' ' replacement and shell_escape=true
+        # assumes the path isn't further quoted within the cmd backticks.
         expanded = complete_expanduser(replace(string[r], r"\\ " => " "), r)
         expanded[3] && return expanded  # If user expansion available, return it
 
-        paths, r, success = complete_path(replace(string[r], r"\\ " => " "), pos)
+        paths, r, success = complete_path(replace(string[r], r"\\ " => " "), pos,
+                                          shell_escape=true)
+
+        return sort!(paths, by=p->p.path), r, success
+    elseif inc_tag === :string
+        # Find first non-escaped quote
+        m = match(r"\"(?!\\)", reverse(partial))
+        startpos = nextind(partial, reverseind(partial, m.offset))
+        r = startpos:pos
+
+        expanded = complete_expanduser(string[r], r)
+        expanded[3] && return expanded  # If user expansion available, return it
 
-        if inc_tag === :string && close_path_completion(string, startpos, r, paths, pos)
-            paths[1] = PathCompletion((paths[1]::PathCompletion).path * "\"")
+        path_prefix = try
+            unescape_string(replace(string[r], "\\\$"=>"\$", "\\\""=>"\""))
+        catch
+            nothing
         end
+        if !isnothing(path_prefix)
+            paths, r, success = complete_path(path_prefix, pos, string_escape=true)
 
-        #Latex symbols can be completed for strings
-        (success || inc_tag === :cmd) && return sort!(paths, by=p->p.path), r, success
+            if close_path_completion(string, startpos, r, paths, pos)
+                paths[1] = PathCompletion((paths[1]::PathCompletion).path * "\"")
+            end
+
+            # Fallthrough allowed so that Latex symbols can be completed in strings
+            success && return sort!(paths, by=p->p.path), r, success
+        end
     end
 
     ok, ret = bslash_completions(string, pos)

stdlib/REPL/test/replcompletions.jl

@@ -1177,7 +1177,7 @@ let current_dir, forbidden
             catch e
                 e isa Base.IOError && occursin("ELOOP", e.msg)
             end
-            c, r = test_complete("\"$(joinpath(path, "selfsym"))")
+            c, r = test_complete("\""*escape_string(joinpath(path, "selfsym")))
             @test c == ["selfsymlink"]
         end
     end
@@ -1207,26 +1207,62 @@ end
 mktempdir() do path
     space_folder = randstring() * " α"
     dir = joinpath(path, space_folder)
-    dir_space = replace(space_folder, " " => "\\ ")
-
     mkdir(dir)
     cd(path) do
-        open(joinpath(space_folder, "space .file"),"w") do f
-            s = Sys.iswindows() ? "rm $dir_space\\\\space" : "cd $dir_space/space"
-            c, r = test_scomplete(s)
-            @test r == lastindex(s)-4:lastindex(s)
-            @test "space\\ .file" in c
+        touch(joinpath(space_folder, "space .file"))
+
+        dir_space = replace(space_folder, " " => "\\ ")
+        s = Sys.iswindows() ? "cd $dir_space\\\\space" : "cd $dir_space/space"
+        c, r = test_scomplete(s)
+        @test s[r] == "space"
+        @test "space\\ .file" in c
+        # Also use shell escape rules within cmd backticks
+        s = "`$s"
+        c, r = test_scomplete(s)
+        @test s[r] == "space"
+        @test "space\\ .file" in c
+
+        # escape string according to Julia escaping rules
+        julia_esc(str) = escape_string(str, ('\"','$'))
+
+        # For normal strings the string should be properly escaped according to
+        # the usual rules for Julia strings.
+        s = "cd(\"" * julia_esc(joinpath(path, space_folder, "space"))
+        c, r = test_complete(s)
+        @test s[r] == "space"
+        @test "space .file\"" in c
+
+        # '$' is the only character which can appear in a windows filename and
+        # which needs to be escaped in Julia strings (on unix we could do this
+        # test with all sorts of special chars)
+        touch(joinpath(space_folder, "needs_escape\$.file"))
+        escpath = julia_esc(joinpath(path, space_folder, "needs_escape\$"))
+        s = "cd(\"$escpath"
+        c, r = test_complete(s)
+        @test s[r] == "needs_escape\\\$"
+        @test "needs_escape\\\$.file\"" in c
 
-            s = Sys.iswindows() ? "cd(\"β $dir_space\\\\space" : "cd(\"β $dir_space/space"
+        if !Sys.iswindows()
+            touch(joinpath(space_folder, "needs_escape2\n\".file"))
+            escpath = julia_esc(joinpath(path, space_folder, "needs_escape2\n\""))
+            s = "cd(\"$escpath"
             c, r = test_complete(s)
-            @test r == lastindex(s)-4:lastindex(s)
-            @test "space .file\"" in c
+            @test s[r] == "needs_escape2\\n\\\""
+            @test "needs_escape2\\n\\\".file\"" in c
+
+            touch(joinpath(space_folder, "needs_escape3\\.file"))
+            escpath = julia_esc(joinpath(path, space_folder, "needs_escape3\\"))
+            s = "cd(\"$escpath"
+            c, r = test_complete(s)
+            @test s[r] == "needs_escape3\\\\"
+            @test "needs_escape3\\\\.file\"" in c
         end
+
         # Test for issue #10324
-        s = "cd(\"$dir_space"
+        s = "cd(\"$space_folder"
         c, r = test_complete(s)
-        @test r == 5:15
-        @test s[r] ==  dir_space
+        @test r == 5:14
+        @test s[r] == space_folder
 
         #Test for #18479
         for c in "'`@\$;&"
@@ -1240,8 +1276,9 @@ mktempdir() do path
                     @test c[1] == test_dir*(Sys.iswindows() ? "\\\\" : "/")
                     @test res
                 end
-                c, r, res = test_complete("\""*test_dir)
-                @test c[1] == test_dir*(Sys.iswindows() ? "\\\\" : "/")
+                escdir = julia_esc(test_dir)
+                c, r, res = test_complete("\""*escdir)
+                @test c[1] == escdir*(Sys.iswindows() ? "\\\\" : "/")
                 @test res
             finally
                 rm(joinpath(path, test_dir), recursive=true)
@@ -1285,7 +1322,7 @@ if Sys.iswindows()
         @test r == length(s)-1:length(s)
         @test file in c
 
-        s = "cd(\"..\\"
+        s = "cd(\"..\\\\"
         c,r = test_complete(s)
         @test r == length(s)+1:length(s)
         @test temp_name * "\\\\" in c