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
Suggested warning on attempt to 'use' with arguments when no import() subroutine exists #16237
Comments
From @epaIf a module doesn't define an import subroutine, but the caller tries % echo 'package A; sub foo {}; 1;' >A.pm This should print something like You asked to import 'foo' from package A, but that package does not Of course, if the module uses Exporter or some other code, that will - the module does not define any import() subroutine, and Since those extra arguments are useless, and quite likely indicate Flags: Site configuration information for perl 5.22.2: Configured by Red Hat, Inc. at Fri Nov 4 14:35:02 UTC 2016. Summary of my perl5 (revision 5 version 22 subversion 2) configuration: Platform: Locally applied patches: @INC for perl 5.22.2: Environment for perl 5.22.2: |
From @epaThe bug tracker ate the message text, or at least refuses to show it on the web page, so here it is: If a module doesn't define an import subroutine, but the caller tries % echo 'package A; sub foo {}; 1;' >A.pm This should print something like You asked to import 'foo' from package A, but that package does not Of course, if the module uses Exporter or some other code, that will - the module does not define any import() subroutine, and Since those extra arguments are useless, and quite likely indicate Flags: Site configuration information for perl 5.22.2: Configured by Red Hat, Inc. at Fri Nov 4 14:35:02 UTC 2016. Summary of my perl5 (revision 5 version 22 subversion 2) configuration: Locally applied patches: @INC for perl 5.22.2: Environment for perl 5.22.2: |
The RT System itself - Status changed from 'new' to 'open' |
From @hvdsOn Fri, 10 Nov 2017 04:33:52 -0800, eda@waniasset.com wrote:
But every module gets an import subroutine: if not their own, their parent's (and at least UNIVERSAL's). Hugo |
From zefram@fysh.orgHugo van der Sanden via RT wrote:
No, UNIVERSAL doesn't contain an import method. There's specific logic $ perl -lwe 'print "main"->can("import") // "no method"; print eval { "main"->import; 1 } ? "ok" : "fail"' -zefram |
From @hvdsOn Sat, 11 Nov 2017 02:44:01 -0800, zefram@fysh.org wrote:
That looks hard to distinguish from 'has an import method, but can() lies about it'. :) But in any case the answer changes if anything has loaded UNIVERSAL: For Ed's original suggestion: if we can reliably detect that there's no explicit import() in the inheritance chain, the suggestion of warning on a useless import arg seems valid, but the above suggests to me that the result would vary depending on whether UNIVERSAL had been loaded, which would at best make it less useful and at worst invalidate the idea. Hugo |
From @epaWouldn't the answer be to also tweak UNIVERSAL::import? It could be something like sub import { Combined with a warning in the core language to cover the case when UNIVERSAL hasn't been loaded. |
From @xsawyerxOn 11/11/2017 06:43 PM, Hugo van der Sanden via RT wrote:
I agree this is a good suggestion if we could reliably detect it. I'm not versed enough with the technical details on this but if we can |
From zefram@fysh.orgSawyer X wrote:
The import method from the UNIVERSAL module is itself a way of detecting It's nasty to have a non-empty UNIVERSAL module. That import method -zefram |
From @davidnicoluntested draft. Extra points for revising to pull file, line data out of cat > NoImportWarning.pm <<END sub UNIVERSAL::import { On Fri, Nov 10, 2017 at 6:33 AM, Ed Avis <perlbug-followup@perl.org> wrote:
-- |
From zefram@fysh.orgEd Avis wrote:
I'm not interested in this warning per se, but it seems like We can expect that this deprecation wouldn't get triggered very much, and A few days ago I had a stab at a more radical deprecation that would also If there's a consensus that this deprecation is in principle a good idea, -zefram |
From @hvdsOn Mon, 11 Dec 2017 17:41:34 -0800, zefram@fysh.org wrote:
I like this; I suspect it's going to reveal a bunch of bugs on CPAN though (and probably, eventually, in the darkPAN). It also suggests to me the perlfunc entry for 'use' may be insufficient (despite already being pretty long), and would benefit from a more canonically proscriptive definition of VERSION and a couple of examples - I'd have walked straight past C<use App::Cpan '1.64';> or C<use ExtUtils::ParseXS::Constants $VERSION;>, for example, without noticing anything amiss. Hugo |
From zefram@fysh.orgHugo van der Sanden via RT wrote:
Yes, good point. I've expanded it a bit in commit -zefram |
From @epaThanks for your work on this. How do you feel about another warning (or deprecation) |
From zefram@fysh.orgEd Avis via RT wrote:
I'm still not interested in warnings per se. The question of what should But module/package mismatch is a tricky area. Unlike There are multiple things not being checked around here, which interact At a previous workplace I instituted a stricture to avoid these problems. We could potentially implement some version of that stricture in the core Incidentally, there's a question about how warnings should be managed -zefram |
From @epaMaybe the small number of oddball modules that don't want any package declaration should have a way to declare that explicitly. no package; or whatever syntax you think appropriate. If a module source file is loaded by 'use', and does not declare a package or explicitly declare no-package, then it triggers a diagnostic. |
From @cpansproutOn Tue, 12 Dec 2017 03:07:11 -0800, zefram@fysh.org wrote:
All right, I’ll make your complaint prediction come true. ‘require "path/to/_utils_private_to_this_module.pl"’ is quite useful. Why should require be stricter when the file ends in .pm? -- Father Chrysostomos |
From @epaWhy have two different kinds of library, .pl and .pm, unless they differ in behaviour in some way? Generally though I think that 'require' can stay as it is. It's used only by those who know what they are doing and have consciously decided to roll their own library loading and importing. It's 'use' that ought to be stricter, in my view. And if that means that 'use' is no longer exactly equivalent to BEGIN { require blah; import blah }, so be it. |
From zefram@fysh.orgFather Chrysostomos via RT wrote:
I didn't say that it should. But this raises another issue, of exactly An obvious way to keep interfaces clean would be for "require" to keep Another way is what you suggest, for "require" to apply the stricture Another option is that "require" behaves as it does now except that in Is your concern strictly about keeping clean behaviour for "require", -zefram |
From @cpansproutOn Tue, 12 Dec 2017 09:25:14 -0800, zefram@fysh.org wrote:
‘use’ has already departed from equivalence with ‘require’, so I don’t think changing that equivalence further would be a problem. I do worry, though, that requiring the very first statement to be a package declaration will break too much working code. Some people even put ‘use strict’ before the package declaration. Maybe another approach would be to have ‘use’ check afterwards whether the right package has been defined. Of course, that does mean that code like this would prevent the check: use Hash::Util::FieldHash; (In case someone reading this does not understand why it would not work: The Hash::Util::FieldHash package is *inside* the Hash::Util package, so HU has to exist for HUFH to exist.) But we can get around that by making package declarations set a flag on the hash. That part of the implementation would be trivial. -- Father Chrysostomos |
From @xsawyerxOn 12/12/2017 03:41 AM, Zefram wrote:
I'm worried about this for several reasons. The sum of which are that I personally would turn it on, but I'm concerned about this as a removed A contrived off-the-cuff example is a class that dictates you should |
From zefram@fysh.orgSawyer X wrote:
If the deprecation were implemented, it would provide the prompt for that -zefram |
From @xsawyerxOn 01/02/2018 04:12 PM, Zefram wrote:
Following these are two questions: * Do we know how rare it really is? Maybe it's time to make use of |
From zefram@fysh.orgSawyer X wrote:
CPAN-smoking the deprecation branch, or a proof branch with that
Yes: it's a better language with these things ruled out. -zefram |
From @xsawyerxOn 01/03/2018 02:53 PM, Zefram wrote:
Which is more worrisome since we cannot account on how much breakage
Arguably one of the values of the language is its permissibility. |
From zefram@fysh.orgSawyer X wrote:
That's a different question. A CPAN smoke can tell us how many modules
Arguably your brain would be better if every neuron were connected to It would be a rather superficial argument if permissitivity trumped In this case, the flexibility adds no value, and ruling out these -zefram |
From @xsawyerxOn 01/03/2018 03:57 PM, Zefram wrote:
I would prefer to not the degree of affected modules before moving
You sweet talker. :)
Not everything, but in many cases, yes.
Are we certain of this in *every* case? Is every case caught here a bug? The problem is that any case in which it is not a bug requires an
I would be more comfortable if there was a smoked branch we could see If not, there is always the next release. |
From zefram@fysh.orgSawyer X wrote:
Kinda; it depends on the details of the smoking. If we attempt to install Another possible approach is to construct a branch in which the Either way, we can't readily discern whether a module that depends on
By the dynamic and reflective nature of Perl it is impossible to rule Do you have some specific potential usage in mind?
Every case caught within the core distro was clearly a bug. Whether any
True, it is not free. That cost has to be weighed against the cost of
There's a branch there, you're free to give instructions to smoke it. -zefram |
From @xsawyerxOn 01/15/2018 10:34 PM, Zefram wrote:
This is what I was thinking. Having a branch in which it is fatal. Then
This is good enough for me. I'm not expecting to find also odd
No. Just the basics.
This is why I would like to smoke it more.
But we do not know that cost. Testing can help illuminate both parts.
If you can make a version of it fatal, we can take Andreas on his offer |
From @AbigailOn Tue, Dec 12, 2017 at 11:06:03AM +0000, Zefram wrote:
Tons of code at $WORK would break if this were enforced. $ cat Foo.pm $ cat Foo/Special.pm is a not uncommon practise. Abigail |
From zefram@fysh.orgSawyer X wrote:
OK. I've rebased zefram/deprecate_some_fake_import on blead, -zefram |
Migrated from rt.perl.org#132425 (status was 'open')
Searchable as RT132425$
The text was updated successfully, but these errors were encountered: