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

Improve static analysis compatibility #44

Open
MilesCranmer opened this issue Nov 12, 2022 · 7 comments
Open

Improve static analysis compatibility #44

MilesCranmer opened this issue Nov 12, 2022 · 7 comments

Comments

@MilesCranmer
Copy link
Contributor

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
Copy link
Contributor Author

MilesCranmer commented Nov 12, 2022

Related to julia-vscode/julia-vscode#2548 from @MariusDrulea.

cc @Shuenhoy @pfitzseb @davidanthoff in case of interest

@Shuenhoy
Copy link

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.

@Roger-luo
Copy link
Owner

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.

@MilesCranmer
Copy link
Contributor Author

MilesCranmer commented Nov 13, 2022

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.

@MariusDrulea
Copy link

MariusDrulea commented Nov 16, 2022

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
Copy link

MariusDrulea commented Nov 16, 2022

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: auto-complete shall work with @from included files julia-vscode/julia-vscode#2548 (comment).
  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.
    relative using/import should search current directory JuliaLang/julia#4600
    relative using/import should search current directory JuliaLang/julia#4600 (comment)

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

@dylanxyz
Copy link

dylanxyz commented Nov 9, 2023

@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.

Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
None yet
Projects
None yet
Development

No branches or pull requests

5 participants