julia-vscode icon indicating copy to clipboard operation
julia-vscode copied to clipboard

auto-complete shall work with @from included files

Open MariusDrulea opened this issue 3 years ago • 17 comments

Auto-complete works with the usual include("..."), using ... image

Auto-complete does not work with @from "..." using ... If I use @from (see the FromFile julia package) to include "a.jl" and "b.jl" into "c.jl", the auto-complete no longer works. https://github.com/Roger-luo/FromFile.jl https://github.com/JuliaLang/julia/issues/4600 image

Why FromFile? The standard way of including files in Julia can lead to duplicate includes. The management of included files is entirely left to the developer. FromFile takes over this responsibility from the developer and manages the includes in a centralized way. The include behavior is just like in Java or C#.

Remark:. From the Language Server point of view @from "a.jl" using am means that the module am in the file "a.jl" is available for use. It's the equivalent of include("a.jl") \n using .am, like in my above code. We can also have @from "a.jl" import am and that means we can use am.myfun_a(). This is just like include("a.jl") \n import .am. To me this looks like it can be easily be implemented with the existing Language Server implementation.

MariusDrulea avatar Nov 11 '21 00:11 MariusDrulea

Side info: how @from works? For each @from "my_file" using mymod a module Main.absolute_path_to_my_file.mymod is created globally. Any new @from of the same file will simply take the definition from this globally created module. In the picture bellow you can see the "Global" modules for my example code, in a debug session.

image

MariusDrulea avatar Nov 11 '21 00:11 MariusDrulea

Mh, this is tricky. I'd very much like to avoid special-casing package-contributed macros...

pfitzseb avatar Nov 11 '21 09:11 pfitzseb

I think it does not hurt to have the feature. @from has a specific format that we can clearly match in the StaticLint. It looks like there is no danger of having such a resolve_from_include(x, state). This resolve_from_include will only match @from statements. I can give it a try and if it works, come with a PR.

@from "path.jl" using my_module
@from "path.jl" import my_module
@from "path.jl" using my_module: Symbol1, Symbol2
@from "path.jl" import my_module: Symbol1, Symbol2
function (state::Toplevel)(x::EXPR)
    resolve_import(x, state)
    mark_bindings!(x, state)
    add_binding(x, state)
    mark_globals(x, state)
    handle_macro(x, state)
    s0 = scopes(x, state)
    resolve_ref(x, state)
    followinclude(x, state)

MariusDrulea avatar Nov 12 '21 21:11 MariusDrulea

here is my PR for StaticLint: https://github.com/julia-vscode/StaticLint.jl/pull/319

MariusDrulea avatar Nov 18 '21 14:11 MariusDrulea

Mh, this is tricky. I'd very much like to avoid special-casing package-contributed macros...

I'm wondering if StaticLint can provide a plug-in system to allow package authors to support custom lint tho like some of the python linter. From a first glance it doesn't look like having such mechanism.

Originally posted by @Roger-luo in https://github.com/Roger-luo/FromFile.jl/issues/34#issuecomment-981887004

I agree with these ideas. Besides FromFile, there are many macros heavily used in Julia, lacking IntelliSense support. It is clear that julia-vscode should not and could not try to implement IntelliSense for each of them. However, A plugin mechanism should benefit. The user could put something like a configuration file in their workdir to declare what plugins they want, provided by the community without merging them into julia-vscode.

Shuenhoy avatar Dec 01 '21 06:12 Shuenhoy

@Shuenhoy I also like the idea of this plugin mechanism. The only thing I added is a function resolve_FromFile_import(x::EXPR, state::State). The first lines of code of this functions checks if we match an @from "file.jl" using mymodule expression. If there is a match, then I instruct the linter what to do with such an expression. This function can be included from a plugin folder.

MariusDrulea avatar Dec 02 '21 16:12 MariusDrulea

A plugin mechanism is going to take a bit of design work -- the crux here is that it needs to be purely declarative, as the linter doesn't run any userspace code.

pfitzseb avatar Dec 02 '21 16:12 pfitzseb

The only other idea I had at some point was that maybe one could use an interpreter to only run a very limited subset of Julia code for plugins, with hard timeouts and for example a hard rule that functions have to be pure or something like that for plugins. But that might be much more complicated to pull off than a declarative plugin system...

davidanthoff avatar Dec 02 '21 17:12 davidanthoff

Isn't ok to have "scripted" plugins for now, instead of declarative plugins? I understand declarative plugins is a better design choice, but does it really hurt to have those "scripted" plugins, as bellow.

We can have the file `plugins/my_plugin.jl"

module my_plugin
function match_and_parse(expr, server)
    println(expr)
end
end

And the file StaticLint.jl would load all the files in the "plugins" directory. Of course we have to get the module name for each plugin automatically, but this can be done pretty easily.

files = readdir("plugins", join=true)
for fl in files
    include(fl)
end  

function StaticLint()
    my_plugin.match_and_parse("some cst expression")
end

StaticLint()

MariusDrulea avatar Dec 02 '21 19:12 MariusDrulea

Isn't ok to have "scripted" plugins for now, instead of declarative plugins? I understand declarative plugins is a better design choice, but does it really hurt to have those "scripted" plugins, as bellow.

just a general question, since I'm not very familiar with implementing linter, so how does this handle dependencies if the script has any other dependencies other than just StaticLint or this is actually not something we need to worry about at all

Roger-luo avatar Dec 02 '21 19:12 Roger-luo

The StaticLint has the following top level function, see the code bellow. x is a CSTParser expression, which is similar to a julia metaprogramming expression. Let's take this resolve_FromFile_import(x, state). Inside the function I check if x is an expression of the following format @from "file.jl" using some_module. If there is a match, then I do what is needed to load the desired symbols in the file.jl. So, any new plugin shall have this entry my_plugin_fun(x, state) function that checks for a match and loads some symbols.

Now your question is, what happens if my_plugin_fun(x, state) is using some external package. If so, I think it's the responsibility of the author to handle this. StaticLint shall only load and call my_plugin_fun(x, state) and nothing more.

function (state::Toplevel)(x::EXPR)
    resolve_import(x, state)
    mark_bindings!(x, state)
    add_binding(x, state)
    mark_globals(x, state)
    handle_macro(x, state)
    s0 = scopes(x, state)
    resolve_ref(x, state)
    followinclude(x, state)
    resolve_FromFile_import(x, state) # this function is the custom code used to handle @from includes

MariusDrulea avatar Dec 02 '21 20:12 MariusDrulea

Isn't ok to have "scripted" plugins for now, instead of declarative plugins? I understand declarative plugins is a better design choice, but does it really hurt to have those "scripted" plugins, as bellow.

just a general question, since I'm not very familiar with implementing linter, so how does this handle dependencies if the script has any other dependencies other than just StaticLint or this is actually not something we need to worry about at all

Maybe the plugin can be a Julia package with specified structure instead of a single .jl file, then no need to worry about dependencies.

Shuenhoy avatar Dec 03 '21 04:12 Shuenhoy

The reason we are not running arbitrary user code is that it can easily destabilize the entire editing experience. Just imagine a user adds some package with some custom linter code to a project, that code has a bug, and now all of a sudden the entire editing experience provided by the language server stops working. We also at some point want to ship the LS as a precompiled binary in the extension, which would probably also be more difficult if we were to have an option to run arbitrary user code.

davidanthoff avatar Dec 03 '21 06:12 davidanthoff

The reason we are not running arbitrary user code is that it can easily destabilize the entire editing experience. Just imagine a user adds some package with some custom linter code to a project, that code has a bug, and now all of a sudden the entire editing experience provided by the language server stops working.

I think it is fine as long as no user code is executed unexpectedly. The users could take responsibility for their editing experiences if they want extra functionalities that are not provided by LS officially. This could be a workaround and trade-off before a mature and well-designed declarative solution while avoiding waiting too long.

Shuenhoy avatar Dec 03 '21 11:12 Shuenhoy

I think the medium-term solution could be something where the LS spawns another isolated Julia process and would then run some very targeted custom lint extension code inside that process. It would essentially be similar to the symbol server process we already have. We could also do things like check how long any given piece of custom lint code takes to run, and if it is too slow, or crashes, but that on a list of extensions to no longer run.

davidanthoff avatar Dec 26 '21 22:12 davidanthoff

@davidanthoff I like your proposal and it looks quite feasible to do that. I might have a simpler solution. I'm thinking to avoid managing the abstract syntax tree, if possible. The reason is that the StaticLint implementation changes over time and therefore I might have to update my custom scripts too. Also, understanding and manipulating AST takes some time.

@from "path_to_file.jl" using A We can provide the StaticLinter with a custom macro expansion here and tell it to interpret this as include("path_to_file"); using A. The StaticLinter can handle this from now on. Therefore we would need a plugin mechanism to tell the StaticLint how to interpret @from macros. This has to be custom, only people using the FromFile package shall have this. Is there a way to do that?

MariusDrulea avatar Jul 20 '22 09:07 MariusDrulea

No, there is no such extension mechanism.

davidanthoff avatar Jul 20 '22 23:07 davidanthoff

@davidanthoff, @pfitzseb, see the popularity of the FromFile package: https://github.com/JuliaLang/julia/issues/4600#issuecomment-1221666767. This @from macro handles a fundamental part of Julia, the include system. If it is naturally used with Julia then it shall be naturally supported by the StaticLint, in my opinion.

If you follow the long discussions for the new include system of Julia, https://github.com/JuliaLang/julia/issues/4600, you will see there is a debate to use FromFile as it is or use something similar but with an implicit module for each julia file. The decision + extra implementation my take a long time to reach Julia Base. In the meantime we are stuck with the current rudimentary include system which repels professional developers from adopting Julia. For the users @Roger-luo mentions, having FromFile officially supported by the vscode extension is a necessary transition phase.

MariusDrulea avatar Nov 30 '22 22:11 MariusDrulea