Skip to content
New issue

Have a question about this project? Sign up for a free GitHub account to open an issue and contact its maintainers and the community.

By clicking “Sign up for GitHub”, you agree to our terms of service and privacy statement. We’ll occasionally send you account related emails.

Already on GitHub? Sign in to your account

show warnings if enable/enable_if is called in non-void context #669

Open
wants to merge 2 commits into
base: master
Choose a base branch
from

Conversation

karupanerura
Copy link

@karupanerura karupanerura commented Aug 23, 2021

Example: the following code looks like the correct code, but in fact, it mistakenly uses a comma as a semicolon.

builder {
     enable 'ReverseProxy';
     enable 'Session::Cookie',
         session_key => 'session',
         expires     => 3600,
         secret      => 'mysecret', # incorrect: should be terminated by ;
     enable 'Static',
         path => qr!^/assets/!,
         root => './assets/';
     $app;
};

It expect to see errors and warnings, but in fact this code has the correct syntax for Perl, so no errors or warnings will be shown.
In this code, the Middleware will be applied in the following order: ReverseProxy -> Static -> Session::Cookie (not ReverseProxy -> Session::Cookie -> Static)

The following is the result of B::Deparse on this code:

&builder(sub {
    enable('ReverseProxy');
    enable('Session::Cookie', 'session_key', 'session', 'expires', 3600, 'secret', 'mysecret', enable('Static', 'path', qr"^/assets/", 'root', './assets/'));
    $app;
})

To warning such a mistake, how about displaying warnings like this patch when enable/enable_if is called on void context?
I think enable/enable_if should be called in a void context in general.

related blog entry (Japanese): https://memo.sugyan.com/entry/2021/08/23/003752

lib/Plack/Builder.pm Outdated Show resolved Hide resolved
@karupanerura karupanerura requested a review from miyagawa August 23, 2021 12:16
@karupanerura karupanerura changed the title show warnings if enable/enable_if is called on non-void context show warnings if enable/enable_if is called in non-void context Aug 23, 2021
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
None yet
Projects
None yet
Development

Successfully merging this pull request may close these issues.

2 participants