qossmic / deptrac

Keep your architecture clean.
https://qossmic.github.io/deptrac
MIT License
2.6k stars 135 forks source link

Support for typed const - PHP 8.3 #1371

Closed ddziaduch closed 3 months ago

ddziaduch commented 6 months ago

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 commented 6 months ago

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 commented 6 months ago

I will try to do it then

ddziaduch commented 6 months ago

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

https://github.com/infection/infection/commit/873cd3335774a114bef9ca93388e623bf362d820

https://github.com/infection/infection/pull/1909

thomas-hiron commented 5 months ago

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 commented 5 months ago

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 commented 5 months ago

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 commented 5 months ago

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 commented 5 months ago

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 commented 5 months ago

@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 commented 5 months ago

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 commented 5 months ago

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 commented 5 months ago

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 commented 5 months ago

It's working, thank you!

patrickkusebauch commented 5 months ago

Issue resolved.

patrickkusebauch commented 5 months ago

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

sakowicz commented 3 months ago

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 commented 3 months ago

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 commented 3 months ago

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 commented 3 months ago

@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 commented 3 months ago

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 commented 3 months ago

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

sakowicz commented 3 months ago

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

gennadigennadigennadi commented 3 months ago

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 commented 3 months ago

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

dbrumann commented 3 months ago

Sorry, I am not interested in doing it.