Skip to content
New issue

Have a question about this project? Sign up for a free GitHub account to open an issue and contact its maintainers and the community.

By clicking “Sign up for GitHub”, you agree to our terms of service and privacy statement. We’ll occasionally send you account related emails.

Already on GitHub? Sign in to your account

Use full file path for parse error messages and compiler metadata #1233

Merged
merged 10 commits into from
Dec 12, 2021

Conversation

lihaoyi
Copy link
Member

@lihaoyi lihaoyi commented Dec 4, 2021

Fixes #646
Fixes #493

Previously we constructed a new io.VirtualFile with an identical name and path, both short, to ensure stack traces included only the file name. The correct thing to do is to provide separate names and paths, so stack traces can use the file name while compiler errors can use the full file path. This is done differently in Scala2 and Scala3 due to differences in the compiler APIs, but the end result is the same

This makes the parse/compile errors line up more with how the normal Scala compiler reports them, and fixes sourcecode.File to properly report the full path of the script file.

Updated the existing LineNumberTests to verify the fixed behavior, and added a new sourceCodeMetadata.sc test case to verify the fix applies to the sourcecode.* macros. For now sourcecode.* macros do not work in the REPL or scripts in Scala3, but that is an unrelated problem we can deal with later

Review by @alexarchambault and @anatoliykmetyuk

@lihaoyi
Copy link
Member Author

lihaoyi commented Dec 4, 2021

@alexarchambault looks like the only things failing are some Scala3 tests. I think I need to modify this Scala3 code:

The same way as the corresponding Scala2 code:

I'm not familiar with the Scala3 APIs, do you have any idea how we might accomplish that? In Scala2, we could pass in separate name and path, with the former going into the bytecode for runtime stack traces and the latter being reported by the compiler for compile errors. Not sure what the equivalent for Scala3 is

@lihaoyi
Copy link
Member Author

lihaoyi commented Dec 4, 2021

@alexarchambault Also, it seems the sourcecode.*.apply() functions don't work? Any idea why that might be the case?

@lihaoyi
Copy link
Member Author

lihaoyi commented Dec 11, 2021

@alexarchambault have you had a chance to look at this? If you're busy I'll probably just disable the respective Scala3 tests for now and move forward with the fix for Scala2

@lihaoyi
Copy link
Member Author

lihaoyi commented Dec 11, 2021

The sourcecode problem appears to be due to having the scala2 version of sourcecode on the scala3 ammonite test classpath

@lihaoyi lihaoyi merged commit a6696a8 into com-lihaoyi:master Dec 12, 2021
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
None yet
Projects
None yet
1 participant