Proposed changes for 0.1.0 #349
Conversation
|
This all looks fine so far, thanks! Moved to a v0.1.0 branch. Will call it that for now, that still implies a backwards-incompatible change under semver for 0.x.y. |
Drop support for byRefToken in ArgumentExpression - it's a syntax error in php 7.0+ and is prohibited in php 5.4+. Convert the foo and otherFooList to fooList for union types in catch name lists, parameters, properties, and return union types. Remove ThrowStatement in favor of ThrowExpression for php 8.0. Remove EchoExpression in favor of EchoStatement - echo cannot be used as an expression. Remove statement from NamedLabelStatement - it's always null since a named label is a single statement by itself. TODO: Consistently support MissingToken in a single-element list in cases where a non-empty list is required, such as after `?` in types (php 8.0 union types forbid combining `?` with multiple types) TODO: Check for any regressions in diagnostics TODO: Validate that this can be used with real projects
TysonAndre
changed the title
Raise the minimum php version from 7.0 to 7.2. Depend on phpunit 8 for local development Unify method names for Node and Token to getFullStartPosition and getStartPosition (Consistently end public API methods with Position). Avoid the need for callers to check if an object is a Node/Token before calling a method. Convert UnsetIntrinsicExpression to UnsetStatement - it can only be used as a statement. Unify previously split up lists into a single list in some AST node properties (initially done that way for backward compatibility) Clean up unnecessary special case for throw - This continues to be parsed as an ExpressionStatement with the same precedence Rename throwStatement to throwExpression in test file names.
(Using short arrays for array destructuring requires php 7.1)
|
This is ready for review. Note that the target Note that in |
|
@roblourens thoughts? |
|
Sorry @TysonAndre, I will look at it this week |
| class MissingToken extends Token { | ||
| public function __construct(int $kind, int $fullStart) { | ||
| parent::__construct($kind, $fullStart, $fullStart, 0); | ||
| } | ||
|
|
||
| #[ReturnTypeWillChange] | ||
| public function jsonSerialize() { |
There was a problem hiding this comment.
https://wiki.php.net/rfc/internal_method_return_types adds support for tentative return types, and :mixed was later added to JsonSerializable's method in 8.1 in php/php-src#7051 . This means that implementations will have to return something (or throw), and also that the absence of a return type will emit a warning.
- fixes
vendor/microsoft/tolerant-php-parser/src/Token.php:113 [8192] Declaration of Microsoft\PhpParser\Token::jsonSerialize() should be compatible with JsonSerializable::jsonSerialize(): mixed(a warning, not an error)
Because php 7 is still supported, add an annotation to suppress the warning instead.
Eventually this may have to drop support for 7.x because the mixed type requires 8.0, but that's probably enough time, but that depends on how many 8.x minor releases there are, which is unknown
There was a problem hiding this comment.
Thanks for your work @TysonAndre, sorry for the slow review. I am happy with this. Is this ready to merge and publish as 0.1.0?
Yes, it is |
Drop support for byRefToken in ArgumentExpression - it's a syntax error
in php 7.0+ and is prohibited in php 5.4+.
Convert the foo and otherFooList to fooList for union types in catch
name lists, parameters, properties, and return union types.
Remove ThrowStatement in favor of ThrowExpression for php 8.0.
Remove EchoExpression in favor of EchoStatement - echo cannot be used as
an expression.
Remove statement from NamedLabelStatement - it's always null since a
named label is a single statement by itself.
TODO: Consistently support MissingToken in a single-element list in cases where
a non-empty list is required, such as after
?in types(php 8.0 union types forbid combining
?with multiple types)TODO: Check for any regressions in diagnostics
TODO: Validate that this can be used with real projects if they are refactored to use the new API
Rename to getFullStartPosition and getStartPosition
Consistently end public API methods with Position.
Avoid the need for callers to check if an object is a Node/Token before
calling a method.
For #329
NOTE: It may be useful to create a new
v1branch or other name for the newly created major version - I don't think it was created yet