bscan / PerlNavigator

Perl Language Server that includes syntax checking, perl critic, and code navigation
MIT License
190 stars 35 forks source link

Add Perlimports #22

Closed bscan closed 1 year ago

bscan commented 2 years ago

This would be a nice feature to include in the Navigator. I think the challenge here will be figuring how to bundle it up with the dependencies.

https://metacpan.org/dist/App-perlimports/view/script/perlimports

oalders commented 2 years ago

Let me know if you need any help with this.

bscan commented 2 years ago

Thanks @oalders! Excited to see you chiming in here. We're doing a hackathon at the Perl Conference on Tuesday, so I added this ticket as part of a pile of tickets that people might be interested in working on. I hope someone is interested because this would be a great feature. https://github.com/perlconference/tprc-2022-hou/wiki/Hackathons

One key feature of the Navigator currently is the ease of installation (for vscode at least; it's a bit tricky for other editors). When installing from the vscode marketplace, it includes the dependencies needed for the basic functionality such as autocomplete and go-to definition (Devel::Symdump for example is bundled). However, I'm not currently bundling PPI and Perl::Critic, so those are required to be manually installed for critic diagnostics.

Somewhat just organizing my thoughts: I think step 1 would be getting this working without the bundling (similar to Perl::Critic). Ideally if someone doesn't have it installed and they run "Sort Imports" it could pop-up a nice error message saying "App-perlimports not installed, please install from CPAN and specify path in settings" or similar. Step 2 would be either bundling the dependencies or fatpacking them.

oalders commented 2 years ago

That sounds great, @bscan! We could all use some better Perl support in our editors. Happy to see this is being organized. I spend my days in nvim, so I don't know too much about VSCode, but having more advanced tooling generally available is a big win.

oalders commented 1 year ago

Let me know if I can help out with this. perlimports now has a --lint flag and I'm hopeful people will find that to be useful.

bscan commented 1 year ago

Sure, contributions always welcomed. Is there a way to use this as a library instead of an command line interface? Currently, users set their perl executable, and then everything runs based on that. This allows the configuration to be simpler, so the user doesn't need to specify the executable path for perlcritic, perltidy, etc. If so, the key thing I would need would be a short pl script to wrap or call the library.

For both Perl::Critic and Perl::Tidy, I pipe the source code via stdin into these helper scripts which do the needed processing and then return their outputs via stdout. For critic at least, I needed to tweak things beyond what the CLI offered. For example, perlcritic allows piping from stdin, but doesn't allow setting the filename, which then causes some policies to return false positives. It also has the profile resolution logic that allows me to swap out a default profile.

If we use perlimports for linting, it could show similar to the Perl::Critic linting where it has some diagnostic level that corresponds to a particular color squiggly underline. Or it could be used more like Perl::Tidy to return the formatted source when someone does "Sort Imports". Example from the PyLance language server attached for Sort Imports.

https://github.com/bscan/PerlNavigator/blob/main/server/src/perl/tidyWrapper.pl https://github.com/bscan/PerlNavigator/blob/main/server/src/perl/criticWrapper.pl

sortImports

oalders commented 1 year ago

Is there a way to use this as a library instead of an command line interface?

perlimports is essentially this:

#!perl

use strict;
use warnings;

use App::perlimports::CLI ();
use Try::Tiny qw( catch try );

my $exit_code = 0;
try {
    $exit_code = App::perlimports::CLI->new->run;
}
catch {
    print STDERR $_;
    $exit_code = 1;
};

exit($exit_code);

CLI.pm relies on @ARGV being present. Not sure if this is suitable for you. If not, we can look at adding a new_from_args() or something like that.

For both Perl::Critic and Perl::Tidy, I pipe the source code via stdin into these helper scripts which do the needed processing and then return their outputs via stdout. For critic at least, I needed to tweak things beyond what the CLI offered. For example, perlcritic allows piping from stdin, but doesn't allow setting the filename, which then causes some policies to return false positives. It also has the profile resolution logic that allows me to swap out a default profile.

perlimports has a --read-stdin flag and also accepts a filename as an argument, so this should work.

If we use perlimports for linting, it could show similar to the Perl::Critic linting where it has some diagnostic level that corresponds to a particular color squiggly underline. Or it could be used more like Perl::Tidy to return the formatted source when someone does "Sort Imports". Example from the PyLance language server attached for Sort Imports.

It can actually be used as a linter or a tidier. Not sure if that fits into your model, but there are some options there.

oalders commented 1 year ago

I had a look at your wrapper scripts and they look to be very straightforward. I'll see if I can put a PR together for this.

bscan commented 1 year ago

Sounds great, thanks! I think using it as as a tidier first would be easiest and most consistent with the language server model, although both modes sound good. If you get the wrapper working, I'm happy to work on the typescript side. That part would be mostly a copy of the Perl::Tidy code, which is straightforward as well.

I'd essentially copy this file that does a system call out to tidyWrapper with the source: https://github.com/bscan/PerlNavigator/blob/main/server/src/formatting.ts

and then update the server.ts to call it. I'm not quite sure how the protocol works for imports, so I'd need to do some testing. Looking at the javascript lsp, they have a variety of code actions available related to imports. I think "sort imports" vs "organize imports" is the difference between --preserve-unused and --no-preserve-unused in the perlimports world.

image

oalders commented 1 year ago

Thanks @bscan! I have updated VSCode and installed Perl Navigator. Any tips on how I could best test out my changes?

bscan commented 1 year ago

Sure, I think you have two options:

  1. If you want to run a full environment of the Navigator, you can check out the repo and use the built-in Run configurations. The navigator was originally a clone of the Microsoft language server example from here: https://github.com/microsoft/vscode-extension-samples/tree/main/lsp-sample and includes the run configurations from it. It pops open a new window running the extension.

image

  1. An easier way to test could be overriding the Perl::Tidy functionality. In your vscode install, the Navigator will be in a path similar something like .vscode/extensions/bscan.perlnavigator-0.3.0/server/src/perl/tidyWrapper.pl. You could swap out the tidyWrapper.pl with the proposed perlImports wrapper that produces the fixed output. Then for testing, "Format Document" would call your script. Of course, we'd still need the separate code action for Imports, but it would still be a great test. If you get stuck, the log can be pretty helpful:

image

Thanks!

bscan commented 1 year ago

One additional piece. You'll likely want to set "perlnavigator.trace.server": "verbose", if you are looking at the logs