Base.InvalidCharError{Char}('\x95')

[davidanthoff]

From crash reporting here.

Exception type: Base.InvalidCharError{Char}
Failed method: invalid_char(::Char)
Error message: Base.InvalidCharError{Char}('\x95')
Stack trace:

Base.InvalidCharError{Char}:
   at invalid_char(::Char) (char.jl85)
   at UInt32(::Char) (char.jl130)
   at mark_errors(::LanguageServer.Document, ::Array{LanguageServer.Diagnostic,1}) (./scripts/languageserver/packages/LanguageServer/src/requests/textdocument.jl291)
   at parse_all(::LanguageServer.Document, ::LanguageServerInstance) (./scripts/languageserver/packages/LanguageServer/src/requests/textdocument.jl264)
   at process(::LanguageServer.JSONRPC.Request{Val{Symbol("workspace/didChangeWatchedFiles")},LanguageServer.DidChangeWatchedFilesParams}, ::LanguageServerInstance) (./scripts/languageserver/packages/LanguageServer/src/requests/workspace.jl41)
   at run(::LanguageServerInstance) (./scripts/languageserver/packages/LanguageServer/src/languageserverinstance.jl217)
   at top-level scope (./scripts/languageserver/main.jl28)
   at include (boot.jl328)
   at include_relative(::Module, ::String) (loading.jl1105)
   at include(::Module, ::String) (Base.jl31)
   at exec_options(::Base.JLOptions) (client.jl287)
   at _start() (client.jl460)

[davidanthoff]

So I think we have like three options, right?

1. If we read a file with invalid UTF-8 from disc, we pretend it doesn't exist and don't add it to the documents list. If a user opened that file in VS Code, we would still have it in the docs list while it is open, but we would have a cleaned copy where VS Code somehow got rid of the invalid UTF-8. I think the only code that we need to harden in that case to work with this situation might be the file system notification handlers.
2. We read the files like we do today, but harden the whole stack from CSTParser through every piece of code to be able to handle invalid UTF-8 sequences. To me that seems daunting because there are soo many different places where this can go wrong, and this will always be a corner case in terms of testing...
3. When we read such a file from disc and detect invalid UTF-8, we set a flag invalid_text or something like that, and then we make sure that this file is mostly ignored by our code. We might still publish a diagnostics though that points out that this is an invalid file.

Are there other options?

To me 1 seems easiest in the short run, but maybe I'm missing something important...

Is the Julia parser hardened against these invalid UTF-8 sequences? So can I for example run a .jl file that has some invalid UTF-8 in a comment?

Another aspect here is that I think whenever a user opens such a file in VS Code, it will say that something is wrong with the encoding, so there will already be a UI hint that something is wrong with this file...

[ZacLN]

I'm not getting (or maybe just not seeing) any warning from VS Code when opening files with invalid chars.

Julia both parses and runs files with them (within comments, strings or character literals as far as I'm aware, maybe elsewhere as well). I think this should guide the behaviour we choose (unless this is expected to change in the future)..

Personally, I'm keen on (2) - where only minor changes are probably required to Tokenize as once things are tokenized we're not much bothered about the string representation.

Another approach could be to mirror the handling by vscode: replace invalid characters with a placeholder (�). This could be easier all round...

[davidanthoff]

I'm not getting (or maybe just not seeing) any warning from VS Code when opening files with invalid chars.

Ah, I don't get those either if I open the file that I'm creating with the code below. I did get a message when I just opened an arbitrary binary file. So maybe VS Code only shows the message if the whole file is super clearly not a text file, but is more tolerant with some smaller mistakes...

Julia both parses and runs files with them

Are you sure? Here is what I tried:

ba = UInt8['a', 245, 'b']
s = String(ba)
f = open("foo.jl", "w")
println(f, "println('a')")
println(f, "# Hallo", s)
println(f, "println('c')")
close(f)

When I then run julia foo.jl, I get this output:

a

That to me looks like Julia stops processing the file after the comment?

I think this should guide the behaviour we choose (unless this is expected to change in the future)

Agreed, we should just mimic what Julia itself does! I'll ask a question on the internals channel on Slack what the core team's thinking about this is.

[davidanthoff]

I had a bunch of conversations on Slack with the core team. I think the sense is that Julia should just not process source files that have an invalid UTF-8 sequence in them, i.e. it should just throw an error and give up (JuliaLang/julia#34967). It doesn't do that right now, but that seems the direction where this is heading.

So I think we should follow that, and just ignore those files. I think just pretending that they don't exist seems the easiest option by far, right? So essentially what we would do is that if a file has either a) invalid UTF-8 in it, or b) can't be read for permission reasons for example, we all treat these cases identical, i.e. we just don't add those files to the documents collection in the first place.

Those files would still be handled if we get a didOpen message for them, because in those cases we won't get invalid UTF-8 nor file permission errors.

In terms of sequencing, I would suggest we first merge #574, and then I can open a follow up PR that adds the UTF-8 checking, it should be very easy to integrate it at the same points where we now have these try catch blocks.

[davidanthoff]

#574