Skip to content
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

Preserve information about used array syntax and number format. #146

Closed
suralc opened this issue Nov 15, 2014 · 8 comments
Closed

Preserve information about used array syntax and number format. #146

suralc opened this issue Nov 15, 2014 · 8 comments

Comments

@suralc
Copy link
Contributor

suralc commented Nov 15, 2014

Is it currently impossible to retrieve the syntax type used to define an array (array() vs []) or did I overlook something? (The grammar file already makes a difference, but creates the same Array_ instance, maybe pass an optional parameter if the new syntax is used?)

Also binary and octal numbers (0b1010 and 0777) are represented as LNumber. It may be useful to have the source format available there. (type attribute with a LNumber::TYPE_BINARY constant or similiar)

This may be used for some static analysis scenarios and detection of minimal php version requirements.

edit: I read about the second part in #26 It may still be nice to have atleast the used array notation as a flag on the node.

@suralc
Copy link
Contributor Author

suralc commented Dec 27, 2014

@nikic Both topics I raised here can be partially solved by extending the emulative Lexer and overriding getNextToken. Are you aware of any possible pitfalls when using the following code or should it be safe to use?

    public function getNextToken(&$value = null, &$startAttributes = null, &$endAttributes = null)
    {
        $tokenId = parent::getNextToken($value, $startAttributes, $endAttributes);
        // snipped....
        } elseif ($tokenId == Parser::T_ARRAY) {
            $startAttributes['traditionalArray'] = true;
        }

        return $tokenId;
    }
        elseif ($node instanceof Node\Expr\Array_) {
            if (!$node->hasAttribute('traditionalArray')) {
                $this->getResult()->addRequirement(RequirementReason::SHORT_ARRAY_DECLARATION, $node->getLine());
            }
        }

@nikic
Copy link
Owner

nikic commented Dec 28, 2014

@suralc Nope, that's what I'd do as well. The lexer docs (https://github.com/nikic/PHP-Parser/blob/master/doc/component/Lexer.markdown) also suggest this in the lower part (the originalValue for literals). The top part of the page has a more general approach using token offsets. This can be used in some more complicated cases where just looking at the first or last token of a node is not enough.

@gonzaloserrano
Copy link

Hi, this addition would be great! Since we have part of our codebase that supports PHP 5.3 and sometimes our devs use short array syntax by mistake, we could write a script to analyze the changed files and check for this behaviour.

@suralc
Copy link
Contributor Author

suralc commented Jan 23, 2015

@gonzaloserrano Most of this should be managable to implement, I guess.
You could use http://php5.laurent-laville.org/compatinfo/blog/ for most circumstances (not short arrays, though). Some things are however not (yet) implemented there:
Take a look at the snippet above and the implementations(my project (some rewrites required), not related to the one linked above) here (Feel free to copy):

@gonzaloserrano
Copy link

Very cool projects @suralc, thanks!

@llaville
Copy link
Contributor

@suralc Very interesting project (pvra). I've just take a look, and seems really promising.

As, I said, in an old version of my roadmap, I'd like to implement such PHP features analyser.
But my priorities changed. So I'm happy to see that an alternative exist !

Perharps, before the stable release of new major version 4.0, a similar analyser will be born.
Until this day come true, I've a big challenge, almost reach, detection of conditional code.

@nikic
Copy link
Owner

nikic commented Mar 9, 2016

Added kind attribute for LNumber nodes in ae30f97.

@nikic
Copy link
Owner

nikic commented Mar 9, 2016

And same is now also supported for array nodes: aa19912

As such closing this issue, finally :)

@nikic nikic closed this as completed Mar 9, 2016
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
None yet
Projects
None yet
Development

No branches or pull requests

4 participants