nikic/php-ast

Create a new 1.0.0 release to remove older functionality?

TysonAndre opened this issue · 11 comments

Removing unused code would make it easier to find up to date documentation and develop against the library. (E.g. make it impossible to use node kinds/flags that no longer exist).

This library has been mostly unchanged (For PHP API) over the last 6 months (So I'd say the design is close to final, and applications have had enough time to migrate to AST version 50)

Thoughts on what a 1.0.0 release would do:

  • Remove old node kinds and flags. (e.g. AST_LIST)
  • Remove AST versions prior to version 50 (version 40 isn't deprecated yet. I don't have a need for it to be deprecated, but thought I'd note that.)
  • Completely remove ast\Node\Decl

Proposed deployment:

  1. Create a 1.0.0 beta release so that application developers can test that applications would work (And so that users of applications are less likely to accidentally upgrade)
  2. Application developers make sure that applications would work against 1.0beta, update applications to support ~0.1.6 || ~1.0.0 as a version constraint in composer.json
  3. A month later, deploy a 1.0.0 stable release to packagist.
nikic commented

Doing a 1.0 release sounds reasonable. Before that I'd like to release a final 0.1.7 with a final set of deprecations, as well as AST version 60.

phan/phan#1134 "Don't exit immediately due to E_DEPRECATED from the php-ast library" - If the deprecation affects Phan (e.g. parse_code), please use 0.2.0.

The only supported Phan versions use AST version 50

Also see #71 if you plan to remove AST versions from 0.x.

nikic commented

I'm only deprecating version 40 and 45, so phan should not be affected.

Are there any other AST tweaks that should be rolled into version 60, or are we good to go?

Nope, I can't think of other AST tweaks for version 60 - Just one trivial change to move from version 50 to 60.

Phan's unit tests are affected by php-ast 0.1.7 in PHP 7.0, but that's just comparing tolerant-php-parser-to-php-ast to ast\Node - All of the other tests are unaffected.

in Phan\Tests\AST\TolerantASTConverter\ConversionTest::testFallbackFromParser

Failed asserting that two strings are identical.
--- Expected
+++ Actual
@@ @@
-                            'docComment' => NULL,

I'd prefer that the check below be changed to state->version > 50 so that the change doesn't affect version 50, but it's not a major issue. It won't affect runs of Phan itself, just people running the test suite in php 7.0

+#if PHP_VERSION_ID < 70100
+       /* Emulate docComment on constants, which is not available in PHP 7.0 */
+       if (state->version >= 50 && ast->kind == ZEND_AST_CONST_ELEM) {
+               zval tmp;
+               ZVAL_NULL(&tmp);
+               zend_hash_add_new(ht, AST_STR(str_docComment), &tmp);
+       }
+#endif

I can't think of anything else that I want added to 1.0.0/changed in 0.0.7

nikic commented

I believe I have cleaned out all the pre version 50 functionality and related constants and README notes.

Phan 1.1.0-dev works locally with the most recent version of php-ast 1.0.0-dev. (Tested with PHP 7.0 and PHP 7.3.0-dev)

nikic commented

In that case, can we skip the beta and just go for a 1.0.0 release?

Sounds good. Phan 1.1.0 was released 2 weeks ago.

nikic commented

Done :)