perl11 / cperl

A perl5 with classes, types, compilable, company friendly, security
http://perl11.org/
Other
142 stars 17 forks source link

add warnings 'shadow' when method shadows package #368

Closed rurban closed 6 years ago

rurban commented 6 years ago

warn about when

methods are favored over packages, which is fine. but some may be confused about it, and it's very hard to analyze it and find the root cause, when someone adds a package in front of yours, overriding your package name. The classical autobox or UNIVERSAL::isa problem/solution.

See https://gist.github.com/rurban/e7e3873613c4658bc552fba4356d7022 Detected by @mziescha @dresden-pm

This needs to be added to core (for max. performance), and also as cpan package (for older perls). shadow is off by default, i.e. use warnings does not turn it on. you need to explicitly enable it or use warnings "all" (which should have been called use warnings :all btw)

This is somehow related to use warnings 'closure', see perldiag 'Subroutine "&%s" is not available', but technically something completely different.

rurban commented 6 years ago

Proposed perldiag.pod warning entry:

=item Subroutine &%s masks existing package %s

(W shadow) A subroutine has been added in the outer package with the
same name of the package, shadowing the package, effectively
eliminating all access to the package via methods.  This is almost always an
error, unless you want to override method access to the package.

=item Subroutine &%s masks new package %s
=item Subroutine &%s masks new class %s

(W shadow) A package or class has been added, but a subroutine already
exist with the same name of the package, shadowing the package,
effectively eliminating all access to the package via methods.  This
is almost always an error, unless you want to override method access
to the package.

The first check is done when adding a subroutine, i.e. at compile time. The second check is done when adding a package via the keywords package or class, i.e. at compile time. The check is not done when a package is created implicitly via a fully qualified symbol.

Unfortunately the check if the shadowed package contains a sub (i.e. potential method) at all is too expensive, so the warning is also emitted for data-only packages without subs, even if there the warning can be ignored. TODO dont warn if no method is shadowed

See cperl branch smoke/gh368-warnshadow

rurban commented 6 years ago

Merged into v5.29.0c