FromFile.jl icon indicating copy to clipboard operation
FromFile.jl copied to clipboard

Improve static analysis compatibility

Open MilesCranmer opened this issue 3 years ago • 7 comments

Right now when you use @from "...", IDEs are not smart enough to expand the macro and locate the definition of a symbol. This means that useful features like hover help will not work when developing locally with FromFile.jl. I think this really hurts the rate of adoption.

I see two ways of fixing this:

  1. Patch LanguageServer.jl to recognize custom macros for including files. Perhaps there could be a configuration file in the root of a project which tells the language server what symbols should be treated as equivalent to include.
  2. Modify the syntax of FromFile.jl to use include. For example:
@from include("types.jl") import ...

it is one more symbol, but would successfully trick the language server to pick up the definition of symbols. (You can even trick the language server with macro ignore(args...) end; @ignore include("types.jl"), which is useful for enabling hover help in test files).

For 2, since this is slightly different syntax, I would also encourage you to use a different macro name instead, like @import include("types.jl"): ..., which is closer to existing Julia style (and would help win support from any core devs with allergic reactions to Python).

MilesCranmer avatar Nov 12 '22 17:11 MilesCranmer

Related to https://github.com/julia-vscode/julia-vscode/issues/2548 from @MariusDrulea.

cc @Shuenhoy @pfitzseb @davidanthoff in case of interest

MilesCranmer avatar Nov 12 '22 18:11 MilesCranmer

Tricking language server could be possible. I used to do that in https://github.com/Shuenhoy/ThinModules.jl, which takes an even smaller step that still keeps the standard include structures but only reorders the module definitions by auto topo sorting (now it's like rust somehow).

The problem here is there are bigger gaps between the semantics of include and from .. import ... And my concern is that sometimes this may work, but other times language server would provide the wrong intelligence, depending on the actual structure formed by from .. import ...

But if it could be proved that the wrong situations are rare or easily avoided, this would be a very practical and cheap way.

Shuenhoy avatar Nov 13 '22 03:11 Shuenhoy

I think since this package was made as a proposal for issue 4600, we should expect these things to be supported by the Julia compiler in some way instead of special case in LanguageServer (otherwise, other package defines a macro named @from would have wrong linting). As I mentioned in the other discussion, I think the cleanest solution would be supporting custom package plugin for the linter, which is possible but hard to do in Julia based on the previous discussions in julia-vscode/julia-vscode#2548.

I'm not a fan of changing the syntax as this is more of a technical problem rather than the issue of the syntax design.

You can even trick the language server with macro ignore(args...) end; @ignore include("types.jl"), which is useful for enabling hover help in test files

IMO, this just seems wrong for a language server... one cannot guarantee the correctness of linting a macro, as the syntax is customed. The only way is. to specialize the corresponding linting for a specific macro.

Roger-luo avatar Nov 13 '22 18:11 Roger-luo

Very fair points! If language server can be fixed soon that would be great. But it is definitely a significant change vs simply allowing “include” in the syntax.

Since (1) I just want hover help to work, and (2) the technical issue seems like it has an undefined timeline, I think modifying the syntax to allow @from include(“myfile.jl”) import … is a simple short-term solution. It would probably only require a couple lines changed in the macro, and you could still support the current syntax by default. Then when (if) language server is patched you could deprecate it.

MilesCranmer avatar Nov 13 '22 20:11 MilesCranmer

As a temporary solution to this problem, I have forked the StaticLint package here: https://github.com/MariusDrulea/StaticLint.jl. This fork has support for the @from macro. What happens there is that the LanguageSever transforms @from "a.jl" into include("a.jl") on the fly, by modifying the abstract syntax tree. You can simply replace the default StaticLint of the LanguageServer julia environment. This has three steps: navigate inside the julia vscode extension into the language server folder, activate the language server environment and add the desired StaticLint package. Restart vscode and now the language server is able to hancle the @from macro. image

MariusDrulea avatar Nov 16 '22 09:11 MariusDrulea

I am strongly in favor of keeping the syntax of FromFile as it is. The problems are elsewhere and we shall focus on these problems by:

  1. Making the language server support custom macros; David Anthoff mentioned a good medium-term solution here: https://github.com/julia-vscode/julia-vscode/issues/2548#issuecomment-1001250804.
  2. Contribute to the julia's include system. Julia's include system is really ugly and we shall have it more like it is in java, C#, python. https://github.com/JuliaLang/julia/issues/4600/ https://github.com/JuliaLang/julia/issues/4600#issuecomment-1221666767

As a short-term solution I think it's best to maintain the StaticLint fork mentioned in the above comment.

MariusDrulea avatar Nov 16 '22 10:11 MariusDrulea

@MariusDrulea i did the steps you described to replace StaticLint but the LanguageServer stopped working altogether. Luckily, i figure out how to make it work with the following steps:

  1. Locate the julia-vscode extension folder. Generally, this directory lives at ~/.vscode/extensions/julialang.language-julia-<VERSION>/.
  2. Navigate to the scripts/packages directory.
  3. Locate the StaticLint directory and delete it or rename to something else in case it doesn't work.
  4. Run git clone https://github.com/MariusDrulea/StaticLint.jl StaticLint
  5. Open VSCode and check if everything works.

dylanxyz avatar Nov 09 '23 16:11 dylanxyz