FractalBoy / perl-language-server

101 stars 12 forks source link

Spurious warnings in .pm files #105

Closed rabbiveesh closed 1 year ago

rabbiveesh commented 2 years ago

I've been using this as my language server through neovim. I have it setup as per the documentation, and it's working fine.

The problem is that I always get a warning "Useless use of a constant in void context" at the end of my modules. I have not yet figured out a way to get PLS to realize that my .pm files SHOULD return a true value, which tends to be in void context.

FractalBoy commented 2 years ago

I've never had this problem. Can you share a minimal reproducible example of code that triggers this?

rabbiveesh commented 2 years ago

I'm getting it consistently with the following minmal .pm file:

package Package;
use warnings;

9001

Now, it doesn't warn when the file ends with 1, but I have a little joke of ending my modules with a value that's over 9000. It trips the following warning:

annoying warning

don't mind the sub in the picture, it trips this warning even w/o

FractalBoy commented 2 years ago

Ah, that's the problem. If you end your package with anything other than one, you will get that warning.

You can disable that warning or just use 1. This isn't something
special PLS is doing; it's coming from Perl.

rabbiveesh commented 2 years ago

i guess that I'll take care of it on my end.

rabbiveesh commented 2 years ago

After a discussion on IRC, I realized that normally, modules have scalar context b/c of the use or require that loads them. That's why I've never seen this warning once in my code.

I noticed that in the PublishDiagnostics class, you are always compiling the code via the filename and a -c command line argument. Is it possible to have modules loaded using the -M flag, and then use -e1 or something similar as the script to run? That would avoid this issue, and apparently there are subtle issues with circular dependencies.

FractalBoy commented 2 years ago

While that would be possible, I don't want to have to parse the file to determine what package(s) are in it. -c allows me to pass a file, where as -MModule requires that you know what the package name is, and it may or may not find the correct one depending on @INC.

Running perl -c is the fastest and probably best option here. There may be circular dependencies that show up as warnings, but I have code to ignore them.

rabbiveesh commented 2 years ago

You wouldn't need to parse out package names, when you use a package it always is loaded via the name. I see that it's not worth the complexity, closing again

rabbiveesh commented 2 years ago

I've never seen the warning from perl itself. There's gotta be a way w/in PLS to have it also not warn

On Mon, Jun 13, 2022 at 4:05 PM Marc Reisner @.***> wrote:

Ah, that's the problem. If you end your package with anything other than one, you will get that warning.

You can disable that warning or just use 1. This isn't something special PLS is doing; it's coming from Perl.

— Reply to this email directly, view it on GitHub https://github.com/FractalBoy/perl-language-server/issues/105#issuecomment-1153889704, or unsubscribe https://github.com/notifications/unsubscribe-auth/AFURPKS5PXYP5NQL64D6I6TVO4W2HANCNFSM5YQRDFGA . You are receiving this because you authored the thread.Message ID: @.***>

FractalBoy commented 2 years ago

Please submit a PR to fix this. If you can figure out a way to reliably use -M instead of -c, I'd be happy to merge it in.

rabbiveesh commented 2 years ago

PR has been opened. Could you take a look and let me know if this direction is sane? I also have some questions there.

Thanks!

PS, i believe I sent that email a long time ago, not sure why it showed up recently. In any case, hope we can resolve this!

FractalBoy commented 2 years ago

I was on vacation until now, only got to this issue being reopened today. Looks like you reopened it on 11 Oct.

FractalBoy commented 2 years ago

PR has been opened. Could you take a look and let me know if this direction is sane? I also have some questions there.

I think your PR is considerably outdated - can you redo it using the current codebase? Then I'll take a look.

rabbiveesh commented 2 years ago

My bad, didn't realize my branch had my fork as the upstream.

Have rebased + force pushed

FractalBoy commented 1 year ago

This was fixed in #119