Search code examples
perlperl-critic

Module ends in "1;", perlcritic complains that it doesn't


Have a simple module

package Rrr;
use 5.014;
use warnings;
use namespace::sweep;
use Moo;
use Method::Signatures::Simple;

BEGIN {
    our $VERSION = '0.0.1';
}

has 'root' => (
    is => 'rw',
    default => 'root'
);

method func {
    say 'This is the func method from ' . __PACKAGE__ . ' with value: ', $self->root;
}

1;

The perlcritic -1 says

Code is not tidy at line 1, column 1.  See page 33 of PBP.  (Severity: 1)
Module does not end with "1;" at line 17, column 1.  Must end with a recognizable true value.  (Severity: 4)
Return value of flagged function ignored - say at line 18, column 5.  See pages 208,278 of PBP.  (Severity: 1)

How to make perlcritic happy?

EDIT - based on @toolic's comment

Yes, the tidy helps with the 1st problem (but the Code is not tidy at line 1, column 1. isn't much helpful message), as the diff is:

13c13
<     is => 'rw',
---
>     is      => 'rw',
18c18,19
<     say 'This is the func method from ' . __PACKAGE__ . ' with value: ', $self->root;
---
>     say 'This is the func method from ' . __PACKAGE__ . ' with value: ',
>       $self->root;

But still got the:

Module does not end with "1;" at line 17, column 1.  Must end with a recognizable true value.  (Severity: 4)
Return value of flagged function ignored - say at line 18, column 5.  See pages 208,278 of PBP.  (Severity: 1)

My percritic:

$ perlcritic --version
1.125

Solution

  • It looks like the method keyword from Method::Signatures::Simple is throwing perlcritic off. Notice the difference in how PPI parses the following programs:

    $ tools/ppidump 'method foo { 1 } 1;'
                        PPI::Document
                          PPI::Statement
    [    1,   1,   1 ]     PPI::Token::Word         'method'
    [    1,   8,   8 ]     PPI::Token::Word         'foo'
                            PPI::Structure::Block   { ... }
                              PPI::Statement
    [    1,  14,  14 ]         PPI::Token::Number   '1'
    [    1,  18,  18 ]     PPI::Token::Number       '1'
    [    1,  19,  19 ]     PPI::Token::Structure    ';'
    
    $ tools/ppidump 'sub foo { 1 } 1;'
                        PPI::Document
                          PPI::Statement::Sub
    [    1,   1,   1 ]     PPI::Token::Word         'sub'
    [    1,   5,   5 ]     PPI::Token::Word         'foo'
                            PPI::Structure::Block   { ... }
                              PPI::Statement
    [    1,  11,  11 ]         PPI::Token::Number   '1'
                          PPI::Statement
    [    1,  15,  15 ]     PPI::Token::Number       '1'
    [    1,  16,  16 ]     PPI::Token::Structure    ';'
    

    When using method, the entire program is treated as a single statement; when using sub, 1; is treated as a separate statement.

    To make perlcritic be quiet, you can add a semicolon after the closing brace of your method:

    method func {
        ...
    };
    
    1;
    

    or alternatively

    method func {
        ...
    }
    
    ;1;
    

    However, I think amon made a good point in the comments:

    perlcritic can't handle syntax extensions such as method signatures...Due to such issues, I tend to choose between syntax extensions and perlcritic – and sadly have to prefer static analysis over syntactic sugar in most cases.