Refactor signature parsing into its own class #96
Conversation
This file contains bidirectional Unicode text that may be interpreted or compiled differently than what appears below. To review, open the file in an editor that reveals hidden Unicode characters.
Learn more about bidirectional Unicode characters
If it uses ->{overall}{num_named} it breaks the yadayada handling. Must be a leftover
from before yadayada.
MS::Signature is just a basic data container object for now to prepare for moving the functionality out of MS and into MS::Signature.
This turns out to be important for neg_and_odd_prime() in t/where.t and in general our order of evaluation should be predictable.
…uses. Checking 'if $sig->where' will always return true, so the clause was a waste of time anyway. $sig->where is always initialized, no reason to spend the extra time checking if there's anything in it, the for loop will do that for us. Save a method call.
Makes life easier down the line, especially when it's being passed into the MS::Signature object which has a Str type.
Eventually we'll want the Signature to handle the default invocant logic, but for refactoring it's just getting in the way.
Its really "reduce the signature string down to just the parameter list". I'm kinda waffling between "do everything in one build step" and "only calculate on demand".
No need to split them anymore. parse_signature() seemed more namey of the two.
No need for the special check that the prototype has already been split, we're only calling it once and it's going away soon.
MS::Signature will need it shortly. MS::Parser is rapidly becoming a utility class.
All the logic for parsing the signature is now in MS::Siganture. MS::Signature and MS::Parameter can now check themselves via ->check methods. Tokenizing via split_proto() remains in MS::Parser for the moment just to keep this refactoring small.
parse_signature() is a better name, but all that functionality will move into MS::Signature so it's moot.
MS::Parser is now just a utility library and needs to be renamed. MS::Signature now completely handles signature parsing. I'm pretty sure MS::Signature could hand MS::Parameter pieces of the PPI document and avoid having to reparse each parameter.
Parsing is done in MS::Signature and MS::Parameter now.
Leftover from a58cebe before the stack handling was moved to carp_location_for().
|
The Travis failure is due to experimental being broken on 5.8. Merge #97 and resubmit to Travis. |
Conflicts: lib/Method/Signatures.pm
|
Code looks good to me. I merged in the code from #97 and Travis now passes. I'll merge this in as well. |
barefootcoder
added a commit
that referenced
this pull request
Mar 24, 2014
Refactor signature parsing into its own class
Sign up for free
to join this conversation on GitHub.
Already have an account?
Sign in to comment
Add this suggestion to a batch that can be applied as a single commit.
This suggestion is invalid because no changes were made to the code.
Suggestions cannot be applied while the pull request is closed.
Suggestions cannot be applied while viewing a subset of changes.
Only one suggestion per line can be applied in a batch.
Add this suggestion to a batch that can be applied as a single commit.
Applying suggestions on deleted lines is not supported.
You must change the existing code in this line in order to create a valid suggestion.
Outdated suggestions cannot be applied.
This suggestion has been applied or marked resolved.
Suggestions cannot be applied from pending reviews.
Suggestions cannot be applied on multi-line comments.
Suggestions cannot be applied while the pull request is queued to merge.
Suggestion cannot be applied right now. Please check back later.
This moves all the code for tokenizing, parsing and checking signatures and parameters into its own class, the awkwardly named Method::Signatures::Signature. This makes the code make more sense and it is potentially easier to test and work with separate from the Devel::Declare madness.
Right now this is an internal refactoring. The eventual external goal is to provide a way for users to parsed and examine signatures separate from the whole Devel::Declare thing like Parse::Method::Signatures does. The immediate use is to replace PMS in TryCatch to de-Moosify and speed up TryCatch's load time.
It would also make sense to move the code generation functionality into MS::Signature and MS::Parameter, leaving MS to deal just with the Devel::Declare magic, but that can happen later.
As this touches a lot of code, it would be good to see this merged sooner rather than later. I did my best to break it up into reviewable chunks.