RFC: proof-of-concept for feature-tracking and perl sub signatures (see #273)

[wchristian]

For the shortest summary, see Changes file.

---

This is a basic implementation of feature-tracking in the parser, with most of the parts that mainly need to be expanded sketched out.

Please review and comment.

(This breaks most of the other tests. feature_tracking.t is the only relevant test for this proof-of-concept.)

See #273 for more details and discussion on missing details (configuration, cpan inclusion, etc.) as well as #194 for the historical ticket and discussion, and Perl-Critic/Perl-Critic#591 for requests from the user-side.

OPEN QUESTIONS

• What tree of elements would you like to see emitted for signature?
• Which signature features do you need PPI to recognize that currently are not?
• What tree of elements would you like to see emitted for try?
• Which try features do you need PPI to recognize that currently are not?
• Which other features would you like to see parsed and into what kind of tree?
• Which common CPAN modules should be recognized as enabling specific features?
• What changes, if any, would you like to the custom feature includes facility?

---

TODO:

• We also need to support the prototype keyword which is needed when signatures are enabled.
  • It's already recognized as a generic attribute. What else should we do with it? Can for the moment only think of having https://metacpan.org/pod/PPI::Statement::Sub#prototype recognize it.
  • What is its history, from which versions on is it active?
• extend the automatic recognition of perl core enabling of features
  • we need a list of core features that affect parsing
  • MVP-ready, barring veto
• implement recognition of common CPAN-enabling of features, with an opt-out configuration
  • we need a preliminary list of common CPAN modules which enable parsing-relevant features
  • Syntax::Keyword::Try implement, review requested
  • implemented in PPI::Statement::Include::feature_mods. review requested
  • MVP-ready, barring veto
• implement common configuration of home-brew-enabling of features, opt-in
  • implemented in PPI::Document::new( custom_feature_includes => ... ). review requested
  • MVP-ready, barring veto
• implement a callback hook in possible features-enabling elements
  • implemented in PPI::Document::new( custom_feature_include_cb => ... ). review requested
  • MVP-ready, barring veto
• parse signatures as perl code and create the corresponding tree elements
  • MVP-ready, barring veto

[wchristian]

tempted to just copy in feature.pm from each latest perl release. maybe automatable via dzil?

[toddr]

@wchristian Kudos for taking the initiative on this!!!

If you're up for it, I'd really like to understand why PPI needs to know that signatures are enabled before parsing a prototype into a signature.

In common code, it's rare that you can confuse one for the other. I think the only thing that's ambiguous would be sub foo ( $ ) ... but nobody is going to write that. They're going to write sub foo ( $bar) ... or maybe sub foo ( $bar, $ ) ... in which case it's obviously an attempt at a signature not a prototype.

I know I'm punting the ball down the road but I think we can get away with ignoring features for now?

We also need to support the prototype keyword which is needed when signatures are enabled.

[wchristian]

I'd really like to understand why PPI needs to know that signatures are enabled before parsing a prototype into a signature.

Regarding signatures, i want PPI to also parse the signatures themselves as perl code, so we can have a plugin that checks for unused variables and such recognize their presence in the signature. Meanwhile prototypes are always just a string, because for the most part that's all they are to the perl parser.

Also, as much as possible, i'd like to be correct. If someone wants to have a perl-critic policy of "disallow prototypes", i want PPI to be useful in helping them actually do that.

I think we can get away with ignoring features for now?

We will need to understand features in any case, because they also affect things like keywords, so this is a framework for solving that matter as a whole. I can understand the motivation of skipping that, but i believe we can actually make this work in a manner that is cheap enough to build to do it right now. Note how the framework is already there in the POC, the nitty gritty is only in teaching it how to recognize the activation of a feature, as well as how to parse that feature.

I'd also prefer not to change the way PPI behaves into one thing, to then later have to (partially) revert that behavior, since that seems unpleasant for users.

We also need to support the prototype keyword which is needed when signatures are enabled.

I've added that to the TODO in the first post. :)

This seems to be the huge TBD. How do we know if signatures are enabled?

I've added the answers to that to the TODO in the first post as well. The short version is for now: Several levels of automated as well as configurable recognition of feature-enablers. (Mercifully limited to use/BEGIN.)

[wchristian]

Implemented pre-setting features at the document level to match -M, environment variablesand other possible related things.

Implemented disabling features via feature.pm.

Found that the $Document object needs to be available at the feature decide point in case there are no previous parented tokens to walk the tree on, which currently resulted in modifying a trillion method calls. Probably need to make it an attribute on the tokenizer. Done.

[wchristian]

perl 5.8 has no feature.pm apparently? Removed that.

[wchristian]

Replaced the tokenizer passing with an attribute on the tokenizer, and kept a backup of the passing in https://github.com/Perl-Critic/PPI/tree/document_tokenizer_passing

[wchristian]

Did some research regarding prototype attribute:

It's already recognized as a generic attribute. What else should we do with it? Can for the moment only think of having https://metacpan.org/pod/PPI::Statement::Sub#prototype recognize it. What is its history, from which versions on is it active?

[wchristian]

Implemented various ways to enable parsing features. Please see the checklist in OP post as well as the Changes file.

[wchristian]

I'm running into an interesting question. What should this parse into?

use feature 'signatures'; sub meep($) {}

[wchristian]

A todo suggested by haarg is to parse feature.pm at dist build time to generate a list of version<->feature matches.

[happy-barney]

A todo suggested by haarg is to parse feature.pm at dist build time to generate a list of version<->feature matches.

at build time you have only one version of feature, which contains only features valid for version you are building for.
But I can still build it for version say 5.16 while trying to parse code written for version 5.40

But that is still question: what is goal of PPI? does it aspire to be version independent tool for any Perl processing tool or tool just for running Perl and Perl::Critic?

[wchristian]

I don't think the matter of goals is much relevant there. This is guided by material realities:

• user-customizable feature includes are already implemented
• perl core features move relatively slowly
• we can't have PPI trying to download files when the end user runs it
• there is little reason to report features activated by features.pm if PPI code doesn't also support the parsing of it

As such, grabbing feature.pm from the most recently released perl version and parsing it into a useful data structure for that particular code to query and baking it into a PPI release, is the most useful path i can see.

[wchristian]

I just released a trial version of the signature support code to CPAN, with a blog post here: https://blogs.perl.org/users/mithaldu/2024/11/ppi-signatures-support---feedback-requested.html

[book]

A todo suggested by haarg is to parse feature.pm at dist build time to generate a list of version<->feature matches.

The issue with just parsing feature.pm is that the results depend on which version of perl the script is run with.

I've had the issue with my own Perl::Version::Bumper, which builds such a table, and aims to run on all perls from v5.10 on. The build script is at https://github.com/book/Perl-Version-Bumper/blob/main/util/build_feature_data.pl.

To get the latest table, I have to run the script with the latest perl. I then save the results in my main module.

[zmughal]

I noticed that this uses a string eval on an environment variable: b9502bf. That might be a security issue (or at least, too powerful for what it is being used for).

[zmughal]

we need a preliminary list of common CPAN modules which enable parsing-relevant features

I have a list here though it needs more distinction of what changes the parser via keywords or operators versus just prototypes.

[wchristian]

I noticed that this uses a string eval on an environment variable: b9502bf. That might be a security issue (or at least, too powerful for what it is being used for).

Thanks a bunch for bringing this up, i replaced it with YAML parsing, so users can use JSON or YAML to configure that.

To get the latest table, I have to run the script with the latest perl. I then save the results in my main module.

To be honest, i'd just grab the latest feature.pm from metacpan for that, the script still looks quite useful for future steps. :)

I have a list here though it needs more distinction of what changes the parser via keywords or operators versus just prototypes.

This also looks useful, but i'd need to put in a lot of time to actually sort out what would require PPI changes at all.

As such, for the moment, i've cleaned up some more corners on this and released it to CPAN as v1.281, without any announcements, to see what issues crop up.