xslate / p5-Mouse

Lightweight class builder for Perl, as a subset of Moose
https://metacpan.org/release/Mouse
Other
46 stars 32 forks source link

Mouse won't warn when overwriting a method with an accessor #86

Open ybrliiu opened 6 years ago

ybrliiu commented 6 years ago

In Moose

package Foo {
  use Moose;
  has hello => ( is => 'ro' );
  sub hello { }
}

=> You are overwriting a locally defined method (hello) with an accessor

In Mouse

package Foo {
  use Mouse;
  has hello => ( is => 'ro' );
  sub hello { }
}

=> No warning and error

I guess Mouse should warn in such case.

ybrliiu commented 6 years ago

Warning will be issued if code in comments restored (lib/Mouse/Meta/Attribute.pm line 260 ~ 264). But we must do something for compatibility with Moose (lib/Mouse/Meta/Attribute.pm line 259).

ybrliiu commented 6 years ago

Probably we must pass the tests in t/020_attributes/027_accessor_override_method.t?

ybrliiu commented 6 years ago

メソッドをアクセッサでオーバーライドしたとき警告を出す処理の部分(lib/Mouse/Meta/Attribute.pm の 260 ~ 264行目)のコメントアウトを解除し、 t/020_attributes/027_accessor_override_method.t のテストをスキップせずすべて実行したところ、passしてました。 もし問題なければ警告を出す部分のコメントアウトを解除して警告がでるようにしたいのですが、その上の部分のコメント(we must do something for compatibility with Moose)というのが気になっていまして・・・ Mooseとの互換性のために他に何か実装しなければならないことがあったりするのでしょうか?

skaji commented 5 years ago

@ybrliiu

use strict;
use warnings;

{
    package Foo::Role;
    use Mouse::Role;

    has 'bar' => (
        is      => 'rw',
        isa     => 'Int',
        default => sub { 10 },
    );

    package Foo;
    use Mouse;

    with 'Foo::Role';
    has '+bar' => (default => sub { 100 });
}

This code is valid and should not emit redefine warnings, so I've merged PR #93.

On the other hand, it may cause crashes in perl 5.28.0 :/ So I reverted the whole change in #94 for now.

I will consider how to fix/workaround this. Thanks for your patience.

ybrliiu commented 5 years ago

I see. Thanks for your follow up.