bscan / PerlNavigator

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

Incorrect syntax errors reported with Carp::cluck #112

Open dearieme opened 8 months ago

dearieme commented 8 months ago

I get spurious syntax errors when working with a particular codebase and I think the culprit is Moose using Carp::cluck to emit warnings at compile time.

As far as I can tell, the problem is that the clucked line get's correctly flagged as a warning but the following stacktrace lines are not, so you end up with a syntax error reported for each line of stacktrace, and with Moose stacktraces are deep! I've had a look through the Inquisitor code but I can't see an obvious way to fix it. All the stacktrace lines are indented, but it might be a bit brittle to rely on that.

Yes, I should just not ignore the warnings and fix them, and I will eventually, but it's huge untested codebase and fixing all of them would require extensive re-factoring. Specifically I'm hitting a warning about accessors having the same name as existing functions in the namespace.

Here's a minimal test case to demonstrate...

package CluckTest;

use Carp qw(cluck);
BEGIN {
  cluck "This is a cluck";
}
1;
#! /usr/bin/perl

# cluck_test.pl
use strict;
use warnings;

use CluckTest;
scott@ardbeg ~/dev/perlnavtest $ perl -c -I lib/ -I /home/scott/.local/share/nvim/mason/packages/perlnavigator/node_modules/perlnavigator-server/src/perl -MInquisitor cluck_test.pl 
=PerlWarning=This is a cluck at lib/CluckTest.pm line 6.
        CluckTest::BEGIN() called at lib/CluckTest.pm line 7
        eval {...} called at lib/CluckTest.pm line 7
        require CluckTest.pm called at cluck_test.pl line 6
        main::BEGIN() called at lib/CluckTest.pm line 7
        eval {...} called at lib/CluckTest.pm line 7
Running inquisitor
CluckTest       m               lib/CluckTest.pm        CluckTest       0
$!      c
%!      h
$"      c                                        
$0      c                                       cluck_test.pl
$@      c
%ENV    h
@INC    c                                       /home/scott/.local/share/nvim/mason/packages/perlnavigator/node_modules/perlnavigator-server/src/perl/lib_bs22, lib/, /home/scott/.local/share/nvim/mason/packages/perlnavigator/node_modules/perlnavigator-server/src/perl, /etc/perl, /usr/local/lib/x86_64-linux-gnu/perl/5.32.1, /usr/local/share/perl/5.32.1, /usr/lib/x86_64-linux-gnu/perl5/5.32, /usr/share/perl5, /usr/lib/x86_64-linux-gnu/perl-base, /usr/lib/x86_64-linux-gnu/perl/5.32, /usr/share/perl/5.32, /usr/local/lib/site_perl
%INC    h
%SIG    h
$]      c                                       5.032001
@_      c                                       main
Encode::Internal::Define        t       Encode::Encoding::Define        /usr/lib/x86_64-linux-gnu/perl/5.32/Encode/Encoding.pm  Encode::Encoding        16
Encode::Internal::decode        t       Encode::Internal::decode        /usr/lib/x86_64-linux-gnu/perl/5.32/Encode.pm   Encode::Internal        210
Encode::Internal::encode        t       Encode::Internal::decode        /usr/lib/x86_64-linux-gnu/perl/5.32/Encode.pm   Encode::Internal        210
Encode::Internal::fromUnicode   t       Encode::Encoding::fromUnicode   /usr/lib/x86_64-linux-gnu/perl/5.32/Encode/Encoding.pm  Encode::Encoding        51
Encode::Internal::mime_name     t       Encode::Encoding::mime_name     /usr/lib/x86_64-linux-gnu/perl/5.32/Encode/Encoding.pm  Encode::Encoding        27
Encode::Internal::name  t       Encode::Encoding::name  /usr/lib/x86_64-linux-gnu/perl/5.32/Encode/Encoding.pm  Encode::Encoding        24
Encode::Internal::needs_lines   t       Encode::Encoding::needs_lines   /usr/lib/x86_64-linux-gnu/perl/5.32/Encode/Encoding.pm  Encode::Encoding        42
Encode::Internal::new_sequence  t       Encode::Encoding::renew /usr/lib/x86_64-linux-gnu/perl/5.32/Encode/Encoding.pm  Encode::Encoding        31
Encode::Internal::perlio_ok     t       Encode::Encoding::perlio_ok     /usr/lib/x86_64-linux-gnu/perl/5.32/Encode/Encoding.pm  Encode::Encoding        45
Encode::Internal::renew t       Encode::Encoding::renew /usr/lib/x86_64-linux-gnu/perl/5.32/Encode/Encoding.pm  Encode::Encoding        31
Encode::Internal::renewed       t       Encode::Encoding::renewed       /usr/lib/x86_64-linux-gnu/perl/5.32/Encode/Encoding.pm  Encode::Encoding        38
Encode::Internal::toUnicode     t       Encode::Encoding::toUnicode     /usr/lib/x86_64-linux-gnu/perl/5.32/Encode/Encoding.pm  Encode::Encoding        50
Encode::Internal::DEBUG t       Encode::Encoding::DEBUG /home/scott/.local/share/nvim/mason/packages/perlnavigator/node_modules/perlnavigator-server/src/perl/lib_bs22/Inspectorito.pm Encode::Encoding
Encode::Internal::DESTROY       t       Encode::Encoding::DESTROY       /usr/lib/x86_64-linux-gnu/perl/5.32/Encode/Encoding.pm  Encode::Encoding        69

--------------Now Building the new pltags ---------------------
Done with pltags. Now dumping same-file packages
cluck_test.pl syntax OK
scott@ardbeg ~/dev/perlnavtest $ 
bscan commented 8 months ago

Hi @dearieme, thanks for filing the issue. I'm able to reproduce the problem:

image

In this case, the Inquistor.pm code is already intercepting the warning itself and re-issuing the warning with a prefix of =PerlWarning= as shown in your inquisitor output. This is to allow us to detect the difference between a warning and an error. Code here: https://github.com/bscan/PerlNavigator/blob/8353d5370c91bf457798276749633b9976c6108f/server/src/perl/Inquisitor.pm#L14

I suspect the solution here would be to change this function to format the warning onto a single line. Perhaps replacing newlines with \n or similar replacement character, and then restoring the newlines before posting the warnings to vscode: https://github.com/bscan/PerlNavigator/blob/8353d5370c91bf457798276749633b9976c6108f/server/src/diagnostics.ts#L108

A pull request would be welcome here, or I will eventually get around to it. Thanks!

dearieme commented 8 months ago

Hi @bscan thanks for looking so quickly! That seems like a neat solution - I'll have a go and send a PR.