usethesource / rascal

The implementation of the Rascal meta-programming language (including interpreter, type checker, parser generator, compiler and JVM based run-time system)
http://www.rascal-mpl.org
Other
406 stars 78 forks source link

lib scheme needs version numbers to resolve to unique locations. #1777

Open DavyLandman opened 1 year ago

DavyLandman commented 1 year ago

Describe the bug

If I click a loc of an exception, vscode shows the the wrong file.

|lib://typepal/src/analysis/typepal/TestFramework.rsc|(2986,1,<78,16>,<78,17>): Expected datetime (str, str), but got int
Advice: |https://www.rascal-mpl.org/docs/Rascal/Errors/CompileTimeErrors/UnexpectedType/UnexpectedType.html|

VS Code opens the typepal version that the rascal-lsp is running under, while lib://typepal of the project I'm running is a different version.

To Reproduce

Steps to reproduce the behavior:

  1. make a rascal project that depends on a newer version of typepal than the one shipped with rascal-lsp
  2. open repl for that project (see : INFO: resolved |lib://typepal| at |jar+file:///C:/Users/Davy/.m2/repository/org/rascalmpl/typepal/0.8.0/typepal-0.8.0.jar!/| )
  3. import a module for example import analysis::typepal::TestFramework;
  4. type in runTests
  5. click on any of the locs in the overload links, and see a file range selection that is not the body of the function you wanted to see

Expected behavior I expect links in the repl to correctly map to links in vscode.

Either we should change the hyperlink detector to rewrite any lib link to a resolved jar path, or we should consider adding the version of the jar to the lib scheme?

Additional context So, lib://typepal means something else depending on which jvm we are running (repl vs lsp server for example). Note, typepal is a good example of this problem, but it can also occur while having 2 projects open in parallel that depend on the same library but different versions (say for example clair).

jurgenvinju commented 1 year ago

Yes, this is known behavior. The main reason is that lib://<projectName> is not actually a unique resource identifier. To identify a library we also need the version, or a version range. This problem does not only affect the above situation but many others. I'm currently fighting a similar issue with lib://rascal which comes from three directions due to the maven-plugin, the project at hand, and the nested interpreter of the tutor command executor. Same problem, same solution.

jurgenvinju commented 1 year ago

One thing with solving this issue is that in RASCAL.MF the version of the library would also show up and we'd have to duplicate the version number from the pom into the RASCAL.MF. So for good measure, I believe that when we fix lib to have a version number, we probably need to abolish the RASCAL.MF duplicate declarations. I don't know how.

DavyLandman commented 1 year ago

Agreed, it's all about that scheme.

But this is a user facing variant of it, clicking on the link in the terminal brings you to the wrong version of it.

I'm thinking of doing something special in the link detector for now.

On Wed, 15 Feb 2023, 11:02 Jurgen J. Vinju, @.***> wrote:

Yes, this is known behavior. The main reason is that lib:// is not actually a unique resource identifier. To identify a library we also need the version, or a version range. This problem does not only affect the above situation but many others. I'm currently fighting a similar issue with lib://rascal which comes from three directions due to the maven-plugin, the project at hand, and the nested interpreter of the tutor command executor. Same problem, same solution.

— Reply to this email directly, view it on GitHub https://github.com/usethesource/rascal/issues/1777, or unsubscribe https://github.com/notifications/unsubscribe-auth/AABL3EZNNST4IHLOKZF45BTWXSST3ANCNFSM6AAAAAAU4RQWII . You are receiving this because you authored the thread.Message ID: @.***>

jurgenvinju commented 1 year ago

This is one of those situations where if you solve it on one end, the problem goes to the other end. Temporary workarounds are ok, but we better solve this issue soon before it starts to rot and spread like a virus.

DavyLandman commented 1 year ago

we could also add the context of the lib in the uri, so that you know that lib://typepal-a should be interpreted in the context of the a project?

that would make it easier to keep lib uri's in the source code of your project. (instead of having to update them every version change).

jurgenvinju commented 1 year ago

yes that would seem to help but the lib scheme should also work pre-deployment for the library under development, and without changes to the loc values, in the deployed setting. With that, the context changes, but the semantics should not. So we better come up with a real URI, and I think versions are the only sane way. It's also how everybody else solves this problem. The reason I did not add it from the start, is that we did not have notions of library versions before we started depending on the pom files. Now that we do, we can bunny-hop to the next level.

jurgenvinju commented 1 year ago

Another discussion that intersects this one is the access of resources from the "current project". It may help to separate these issues and introduce a resource scheme which is always always always relative to the Rascal module it is used in. However, this is not implementable as far as I know without an actual Java class inside of the project's jar. Since the interpreter does not demand this, I do not know how to implement this feature yet.

jurgenvinju commented 1 year ago

This is my current problem, caused by the same issue:

INFO: detected |lib://rascal| at |jar+file:///Users/jurgenv/.m2/repository/org/rascalmpl/rascal/0.28.2/rascal-0.28.2.jar!/|
[INFO]  registered library location: |jar+file:///Users/jurgenv/.m2/repository/org/rascalmpl/rascal/0.29.1-RC1/rascal-0.29.1-RC1.jar!/|

Inside we use lib://rascal and to which version should it point? In my case, it should have been 0.29.1-RC1 but accidentally it uses the 0.28.2 version. There are situations where the inverse is required as well.

jurgenvinju commented 1 year ago

For my current issue I have configured inside the rascal-maven-plugin tutor compiler: for every library in pathConfig.libs a logical resolver for the "lib" scheme that resolves to the respective jar location. This way, what is configured in the pom is leading and not what is accidentally on the compiler's own classpath. It would be better with version numbers.

jurgenvinju commented 1 year ago

A similar temporary workaround in the LSP server would work, but all projects would still have to have the same notion of the library since the scope of the logical resolvers is still global per JVM instance.

jurgenvinju commented 1 year ago

Note to self: current uses of the lib pattern in source code would also have to have version numbers. This would spread out the version number of a dependency over many places instead just the pom.xml; this is why a second scheme for retrieving resources based on a different resource identifier mechanism is needed next to the library identification mechanism.

jurgenvinju commented 1 year ago

If first the lib mechanism makes sure the right library is loaded, then next a modulname-based mechanism could work. I.e. module://util::FileSystem/relative/path/to/exampleFile would look in the jar that contains util::FileSystem, having benefitted from precise resolution of that library via lib://rascal-0.30.0 and then locate the file relative to the position of the module name.

jurgenvinju commented 1 year ago

@DavyLandman shall we move this bug to the rascal project?

DavyLandman commented 1 year ago

fine with me. you seem to have an idea of how to fix this.