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

Question

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

Boldface

Line 17 is the start of your method func { ... } declaration. Seems like perl critic thinks that is the end of your module. – Sarawak

Only perl can parse Perl. perlcritic can't handle syntax extensions such as method signatures. It probably parses that method func { ... } 1; as method(func(sub{ ... }, 1));, i.e. the 1 is an argument to func. You can fix this by putting a semicolon after the closing brace, before the final 1;. 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. – Fiberglass

Unrelated, but Method::Signatures::Simple also provides a func keyword, so naming your method func is potentially confusing. I'll assume you'll use a better method name in your real program. :) – Landward

Answer 1

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.

Landward