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

Support for typed const - PHP 8.3 #1371

Closed
ddziaduch opened this issue Feb 20, 2024 · 25 comments
Closed

Support for typed const - PHP 8.3 #1371

ddziaduch opened this issue Feb 20, 2024 · 25 comments

Comments

@ddziaduch
Copy link

ddziaduch commented Feb 20, 2024

Hey there!

Not sure what is the current status of support for PHP 8.3 but Deptrac does not support the typed constants:
Screenshot 2024-02-20 at 14 46 52

Screenshot 2024-02-20 at 14 47 02

Is there any milestone/roadmap for PHP 8.3?

@patrickkusebauch
Copy link
Collaborator

patrickkusebauch commented Feb 20, 2024

Hi, there is no roadmap, but to get rid of the error we just meet to update nikic/php-parser. That should be super simple, maybe even a composer update might suffice. To actually use the type in constant to define dependencies, would be a bit more work.

Feel free to take a stab at the first part if you want.

@ddziaduch
Copy link
Author

I will try to do it then

@ddziaduch
Copy link
Author

ddziaduch commented Feb 23, 2024

Hey, currently it is not possible as there is a blocker in infection/infection dependency:

infection/infection@873cd33

infection/infection#1909

@thomas-hiron
Copy link

Hello! I'm using nikic/php-parser v5.0.2, but I still get the error.
Any idea where the issue can be? Thanks!

@gennadigennadigennadi
Copy link
Member

gennadigennadigennadi commented Mar 13, 2024

Hello! I'm using nikic/php-parser v5.0.2, but I still get the error. Any idea where the issue can be? Thanks!

How do you use php-parser v5 ? Composer.json from Deptrac only requires v4.

are you using the main branch or the shim version?

@thomas-hiron
Copy link

Yes, I'm using shim 1.0.2 that doesn't have requirements:

composer show
name     : qossmic/deptrac-shim
descrip. : deptrac phar distribution
keywords : 
versions : * 1.0.2
released : 2022-12-02, 1 year ago
type     : library
license  : MIT License (MIT) (OSI approved) https://spdx.org/licenses/MIT.html#licenseText
homepage : 
source   : [git] https://github.com/qossmic/deptrac-shim.git 3179
dist     : [zip] https://api.github.com/repos/qossmic/deptrac-shim/zipball/3179
path     : /var/www/html/vendor/qossmic/deptrac-shim
names    : qossmic/deptrac-shim, qossmic/deptrac

support
issues : https://github.com/qossmic/deptrac-shim/issues
source : https://github.com/qossmic/deptrac-shim/tree/1.0.2

requires
ext-json *
ext-tokenizer *
ext-zlib *
php ^8.1

suggests
ext-dom For using the JUnit output formatter

replaces
qossmic/deptrac self.version

And php-parser:

composer show
name     : nikic/php-parser
descrip. : A PHP parser written in PHP
keywords : parser, php
versions : * v5.0.2
released : 2024-03-05, last week
type     : library
license  : BSD 3-Clause "New" or "Revised" License (BSD-3-Clause) (OSI approved) https://spdx.org/licenses/BSD-3-Clause.html#licenseText
homepage : 
source   : [git] https://github.com/nikic/PHP-Parser.git 13967
dist     : [zip] https://api.github.com/repos/nikic/PHP-Parser/zipball/13967
path     : /var/www/html/vendor/nikic/php-parser
names    : nikic/php-parser

support
issues : https://github.com/nikic/PHP-Parser/issues
source : https://github.com/nikic/PHP-Parser/tree/v5.0.2

autoload
psr-4
PhpParser\ => lib/PhpParser

requires
ext-ctype *
ext-json *
ext-tokenizer *
php >=7.4

requires (dev)
ircmaxell/php-yacc ^0.0.7
phpunit/phpunit ^7.0 || ^8.0 || ^9.0

I also tried with dev-main with no success.
Do you know any other conf I can try?

@gennadigennadigennadi
Copy link
Member

gennadigennadigennadi commented Mar 15, 2024

The first part is totally fine. Deptrac-shim provides a phar and is isolated from the other dependencies you install.
dev-main on the otherhand should work with typed const.
Would you be so nice and prepare a reproducer?

FYI: I don't get any errors with php8.3 code for the main branch (running deptrac with php8.1)!
image

@thomas-hiron
Copy link

Sure, here it is: https://github.com/thomas-hiron/deptrac-reproducer

The syntax error also breaks the violation detection: if you delete one of the typed const, there is one violation.
But I guess that's expected given the error.

I'm running php 8.2 and tried with 8.3 too, no success.

@gennadigennadigennadi
Copy link
Member

@thomas-hiron okay, I see what the problem is.

deptrac-shim will not support php8.3, instead it will be sunsetted soon. Please use instead qossmic/deptrac -> dev-main or give the pre-release v2 a try. Those support php8.3, because they already require php-parser v4.18.

@thomas-hiron
Copy link

There's no syntax error, thanks!

I updated the deptrac.yaml but now there is no violation (2 were expected as shown in the reproducer):

deptrac:
  paths:
    - ./src

  layers:
    - name: Api
      collectors:
        - type: classLike
          regex: .*App\\Api\\.*
    - name: Common
      collectors:
        - type: classLike
          regex: .*App\\Common\\.*

Did I miss something in the upgrade guide? Any idea?

@gennadigennadigennadi
Copy link
Member

gennadigennadigennadi commented Mar 15, 2024

With v2|main the default set of registered analyser changed.

I think for v1 the set was:

  analyser:
    types:
      - "use"
      - "file"
      - "class_superglobal"
      - "function_superglobal"
      - "function_call"

If you updated your config, It should behave exactly the same.

@gennadigennadigennadi
Copy link
Member

Did I miss something in the upgrade guide? Any idea?

See upgrade documentaion.

Changed default dependency emitters from CLASS_TOKEN + USE_TOKEN to CLASS_TOKEN + FUNCTION_TOKEN. You can get the old behaviour by explicitly specifying the old emitters in your config file.

@thomas-hiron
Copy link

It's working, thank you!

@patrickkusebauch
Copy link
Collaborator

Issue resolved.

@patrickkusebauch
Copy link
Collaborator

Reopening as we now need to generate the dependencies from the types of constant.

@sakowicz
Copy link

Is there anything we can do to make it work in ^1.0 versions? ^2.0 has a lot of config changes, and it is still in alpha/beta. Migration can take a while.

Also, I think these v1.0 shouldn't be allowed to be installed on PHP 8.3 as it doesn't fully support it.

@gennadigennadigennadi
Copy link
Member

Could you point out what config changes will be problematic for you to upgrade to v2?

Also, as soon as possible I will tag a final release. And it will properly be the same commit as the current beta.

@gennadigennadigennadi
Copy link
Member

Also, I think these v1.0 shouldn't be allowed to be installed on PHP 8.3 as it doesn't fully support it.

In theory it would be possible to tag a v1.0.3 that supports php8.

A PR would be welcome. One would just have to figure out what merges caused the BC, and than would would have to check out the previous commit to the php-parser upgrade and open a PR against the v.1.0 branch.

@sakowicz
Copy link

@gennadigennadigennadi

I've created 1.0.3 candidate but there is no 1.0 branch so I could open a PR 🤔

https://github.com/qossmic/deptrac/compare/2.0.x...sakowicz:deptrac:php-parser-update?expand=1

@gennadigennadigennadi
Copy link
Member

gennadigennadigennadi commented May 20, 2024

Check again. It seems that I removed it recently. Now it’s back again.

The v1.0 is not branch of the right commit, yet. I’ll have to do this later, because right now I’m not at a computer 😅.

@sakowicz
Copy link

@gennadigennadigennadi Did you have a chance to take look at that?

@sakowicz
Copy link

Thanks for merging @gennadigennadigennadi. I wonder if we can release qossmic/deptrac-shim for the final time ;)

@gennadigennadigennadi
Copy link
Member

Sorry, but the shim version is end of life. And I don’t know how to release the shim version.

but feel free to use the 2.0, it’s tagged now.

@sakowicz
Copy link

@dbrumann I wonder if you can help to release deptrac-shim and making .phar file.

@dbrumann
Copy link
Collaborator

Sorry, I am not interested in doing it.

Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Projects
None yet
Development

No branches or pull requests

6 participants