Dependency upgrade to propel 2.0.0-beta4 and symfony 7 #41

ldaspt · 2024-08-08T20:37:37Z

No description provided.

smhg · 2024-08-27T19:06:56Z

@SkyFoxvn would you be able to approve the GH workflow run? I'm curious to see if this works.

SkyFoxvn · 2024-08-27T20:16:20Z

@smhg sorry for the delay im seeing this pull request just now.
i will check it as soon as possible but probably it will be next week and if this work only for symfony 7 i will make a new branch.
if you wish your pull request to be merged to the new branch(if i make new) say so and i will write you here so you can make new pull request to the new branch

smhg · 2024-08-28T07:07:13Z

Github notifications are tricky, that's why I tagged you. No rush.
I'm interested to see what is the result of CI run. If the workflow runs fine, the PR can be moved to a new branch indeed. But if it fails, and needs more work first, that can happen later too I guess.

SkyFoxvn · 2024-08-29T13:34:10Z

@smhg as i can see tests didnt go right :) look like problem with actions/checkout i dont really have much experiance with this think for a public repos so im not sure how exactly to configure it for forked pull requests im open for suggestions about that

smhg · 2024-08-31T06:55:13Z

@SkyFoxvn I'll try it out in a local copy. But I'm just starting to look at propel-bundle. Not sure if I can be of any help.

ldaspt · 2024-09-02T07:32:19Z

@SkyFoxvn All checks have passed

SkyFoxvn · 2024-09-02T07:42:04Z

@ldaspt check the tests there are some deprecations and other stuffs on the different versions

ldaspt · 2024-09-02T10:59:55Z

@SkyFoxvn The deprecations are not triggered by the bundle but by PHPunit and propel

There were 2 PHPUnit test runner deprecations:

1) Metadata found in doc-comment for method Propel\Bundle\PropelBundle\Tests\Form\TypeGuesserTest::testGuessType(). Metadata in doc-comments is deprecated and will no longer be supported in PHPUnit 12. Update your test code to use attributes instead.

2) Metadata found in doc-comment for method Propel\Bundle\PropelBundle\Tests\Util\PropelInflectorTest::testCamelize(). Metadata in doc-comments is deprecated and will no longer be supported in PHPUnit 12. Update your test code to use attributes instead.

--

2 tests triggered 2 PHPUnit deprecations:

1) Propel\Bundle\PropelBundle\Tests\Service\SchemaLocatorTest::testLocateFromBundle
returnCallback() is deprecated and will be removed in PHPUnit 12. Use $double->willReturnCallback() instead of $double->will($this->returnCallback())

/home/runner/work/PropelBundle/PropelBundle/Tests/Service/SchemaLocatorTest.php:79

2) Propel\Bundle\PropelBundle\Tests\Service\SchemaLocatorTest::testLocateFromBundlesAndConfiguration
returnCallback() is deprecated and will be removed in PHPUnit 12. Use $double->willReturnCallback() instead of $double->will($this->returnCallback())

/home/runner/work/PropelBundle/PropelBundle/Tests/Service/SchemaLocatorTest.php:91

The bundle should migrate to phpunit 10 but phpunit 10 require php: >=8.1 and the bundle is compatible with php 8.0

1 test triggered 5 PHP deprecations:

1) /home/runner/work/PropelBundle/PropelBundle/vendor/propel/propel/src/Propel/Generator/Behavior/ConcreteInheritance/ConcreteInheritanceBehavior.php:90
Creation of dynamic property Propel\Generator\Model\ForeignKey::$isParentChild is deprecated

Triggered by:

* Propel\Bundle\PropelBundle\Tests\DataFixtures\Loader\YamlDataLoaderTest::testLoadWithInheritedRelationship
  /home/runner/work/PropelBundle/PropelBundle/Tests/DataFixtures/Loader/YamlDataLoaderTest.php:358

2) vfs://root/propelQuickBuild/YamlInheritedRelationshipBook_YamlInheritedRelationshipAuthor_YamlInheritedRelationshipNobelizedAuthor.php:7221
Use of "parent" in callables is deprecated

Triggered by:

* Propel\Bundle\PropelBundle\Tests\DataFixtures\Loader\YamlDataLoaderTest::testLoadWithInheritedRelationship
  /home/runner/work/PropelBundle/PropelBundle/Tests/DataFixtures/Loader/YamlDataLoaderTest.php:358

3) vfs://root/propelQuickBuild/YamlInheritedRelationshipBook_YamlInheritedRelationshipAuthor_YamlInheritedRelationshipNobelizedAuthor.php:7246
Use of "parent" in callables is deprecated

Triggered by:

* Propel\Bundle\PropelBundle\Tests\DataFixtures\Loader\YamlDataLoaderTest::testLoadWithInheritedRelationship
  /home/runner/work/PropelBundle/PropelBundle/Tests/DataFixtures/Loader/YamlDataLoaderTest.php:358

4) vfs://root/propelQuickBuild/YamlInheritedRelationshipBook_YamlInheritedRelationshipAuthor_YamlInheritedRelationshipNobelizedAuthor.php:7259
Use of "parent" in callables is deprecated

Triggered by:

* Propel\Bundle\PropelBundle\Tests\DataFixtures\Loader\YamlDataLoaderTest::testLoadWithInheritedRelationship
  /home/runner/work/PropelBundle/PropelBundle/Tests/DataFixtures/Loader/YamlDataLoaderTest.php:358

5) vfs://root/propelQuickBuild/YamlInheritedRelationshipBook_YamlInheritedRelationshipAuthor_YamlInheritedRelationshipNobelizedAuthor.php:7234
Use of "parent" in callables is deprecated

Triggered by:

* Propel\Bundle\PropelBundle\Tests\DataFixtures\Loader\YamlDataLoaderTest::testLoadWithInheritedRelationship
  /home/runner/work/PropelBundle/PropelBundle/Tests/DataFixtures/Loader/YamlDataLoaderTest.php:358

The deprecations are not triggered by Propel, it is not possible to fix in the bundle, I think it is necessary to modify the file
https://github.com/propelorm/Propel2/blob/2.0.0-beta4/templates/Builder/Om/baseObjectMethodHook.php

smhg · 2024-09-02T11:12:58Z

I got rid of the warnings like this:
smhg@561c964

SkyFoxvn · 2024-09-02T11:54:55Z

@ldaspt ok i will get rid of the deprecations later i made new branch "7.0" can you change the version to which you want to merge this commits from 6.0 to 7.0 if you can just make new pull request to 7.0 tnx

ldaspt · 2024-09-02T12:03:15Z

@smhg Why not use typehint instead of phpdoc ? I think the classes are not tested and not triggered in the test suite

@SkyFoxvn Rebase done on branch 7.0

smhg · 2024-09-02T12:12:57Z

@ldaspt can't typehints break things if they differ from what people are doing with those methods today? This looked like the safest option which still gives some benefits (phpstan). But please ignore if not relevant.

SkyFoxvn · 2024-09-02T12:53:23Z

@ldaspt type hints and phpdoc dosant exclude each other i actually use both quite often type hints are very nice improvement last few years but they lack the ability for additional context for which you use phpdoc.
example: if you define parameter as array php will push you to use only array but you wack any information for the array structure you need
@ldaspt "I think the classes are not tested and not triggered in the test suite" can you be more specific because on first look its look like all tests are running and passing with success we only have warnings for deprecations

@smhg im not sure i understand you last comment

for now everything here looks ok to me with the deprecations i can deal later but i will delay the merge few days because i want to check few thinks on me pc and this week its just crazy :)

ldaspt · 2024-09-02T13:28:41Z

@SkyFoxvn Add a type hint to protected function execute(InputInterface $input, OutputInterface $output) doesn't break signature for SF6 and it is required in SF7, see https://github.com/symfony/symfony/blob/7.0/src/Symfony/Component/Console/Command/Command.php#L183 Other bundle commands has already the type hint @see https://github.com/SkyFoxvn/PropelBundle/blob/6.0/Command/DatabaseCreateCommand.php#L46

I think the classes are not tested and not triggered in the test suite"

The bundle does not have tests for command, there are no usage of FixturesLoadCommand in the directory Tests and I do not find of propel:fixtures:load in the directory

The warnings are triggered indirectly by the Propel\Bundle\PropelBundle\Tests\DataFixtures\Loader\YamlDataLoaderTest not by the FixturesLoadCommand

@smhg

can't typehints break things if they differ from what people are doing with those methods today?

It is possible that this will break the command if the developer uses SF6 and extends the command but all the others commands has already the typehint @see https://github.com/SkyFoxvn/PropelBundle/tree/6.0/Command

smhg · 2024-09-02T13:31:04Z

I didn't notice that. Good catch! Please ignore everything I said :)

smhg · 2024-09-02T13:36:22Z

@ldaspt please add the deprecation fixes to this PR as you proposed (if @SkyFoxvn doesn't disagree). Your solution is very straightforward.
Please don't release without those. As these warnings take away your focus when testing a Symfony project with PHPUnit.

ldaspt · 2024-09-02T14:03:51Z

@smhg Done

Fix phpstan error with SF7

Line   Command/FixturesLoadCommand.php                                                                                                                                                                      
 ------ ----------------------------------------------------------------------------------------------------------------------------------------------------------------------------------------------------- 
  112    Return type mixed of method Propel\Bundle\PropelBundle\Command\FixturesLoadCommand::execute() is not covariant with return type int of method Symfony\Component\Console\Command\Command::execute().

Note: It misses the phpstan-baseline.neon in the repository, I need to remove lines in phpstan.neon

includes:
  - phpstan-baseline.neon

There are others PHPStan errors

------ ----------------------------------------------------------------------------------------------------------------------------------------------------------------------------------------------------- 
  Line   Command/FixturesLoadCommand.php                                                                                                                                                                      
 ------ ----------------------------------------------------------------------------------------------------------------------------------------------------------------------------------------------------- 
  112    Return type mixed of method Propel\Bundle\PropelBundle\Command\FixturesLoadCommand::execute() is not covariant with return type int of method Symfony\Component\Console\Command\Command::execute().  
 ------ ----------------------------------------------------------------------------------------------------------------------------------------------------------------------------------------------------- 

 ------ ------------------------------------------------------------------------------------------------- 
  Line   DataFixtures/Loader/AbstractDataLoader.php                                                       
 ------ ------------------------------------------------------------------------------------------------- 
  110    Call to an undefined method Propel\Runtime\Map\TableMap::doDeleteAll().                          
  148    Call to an undefined method Propel\Runtime\Map\TableMap::getFieldnames().                        
  211    Call to an undefined method Propel\Runtime\ActiveRecord\ActiveRecordInterface::getByName().      
  227    Call to an undefined method Propel\Runtime\ActiveRecord\ActiveRecordInterface::setByPosition().  
  235    Call to an undefined method Propel\Runtime\ActiveRecord\ActiveRecordInterface::save().           
  307    Variable $setter might not be defined.                                                           
  308    Variable $relatedSetter might not be defined.                                                    
 ------ ------------------------------------------------------------------------------------------------- 

 ------ ------------------------------------------------------------------------------------------------- 
  Line   Form/ChoiceList/PropelChoiceLoader.php                                                           
 ------ ------------------------------------------------------------------------------------------------- 
  185    Call to an undefined method Propel\Runtime\ActiveRecord\ActiveRecordInterface::getPrimaryKey().  
 ------ ------------------------------------------------------------------------------------------------- 

 ------ --------------------------------------------------------------------------------------------- 
  Line   Request/ParamConverter/PropelParamConverter.php                                              
 ------ --------------------------------------------------------------------------------------------- 
  200    Call to an undefined method Propel\Runtime\ActiveQuery\ModelCriteria::filterByPrimaryKey().  
 ------ --------------------------------------------------------------------------------------------- 

 ------ ------------------------------------------------------------------------------------------------------------------------------------------------------------------------------------------------- 
  Line   Security/User/PropelUserProvider.php                                                                                                                                                             
 ------ ------------------------------------------------------------------------------------------------------------------------------------------------------------------------------------------------- 
  24     Class Propel\Bundle\PropelBundle\Security\User\PropelUserProvider implements generic interface Symfony\Component\Security\Core\User\UserProviderInterface but does not specify its types: TUser  
  95     Call to an undefined method Symfony\Component\Security\Core\User\UserInterface::getPrimaryKey().                                                                                                 
 ------ ------------------------------------------------------------------------------------------------------------------------------------------------------------------------------------------------- 

 ------ ------------------------------------------------------------------------------------------------------------------------------------------------------------------- 
  Line   Validator/Constraints/UniqueObject.php                                                                                                                             
 ------ ------------------------------------------------------------------------------------------------------------------------------------------------------------------- 
  49     Negated boolean expression is always false.                                                                                                                        
         💡 Because the type is coming from a PHPDoc, you can turn off this check by setting treatPhpDocTypesAsCertain: false in your phpstan.neon.                         
  49     Result of && is always false.                                                                                                                                      
         💡 Because the type is coming from a PHPDoc, you can turn off this check by setting treatPhpDocTypesAsCertain: false in your phpstan.neon.                         
  57     Negated boolean expression is always false.                                                                                                                        
         💡 Because the type is coming from a PHPDoc, you can turn off this check by setting treatPhpDocTypesAsCertain: false in your phpstan.neon.                         
  57     Result of && is always false.                                                                                                                                      
         💡 Because the type is coming from a PHPDoc, you can turn off this check by setting treatPhpDocTypesAsCertain: false in your phpstan.neon.                         
  65     Return type mixed of method Propel\Bundle\PropelBundle\Validator\Constraints\UniqueObject::getRequiredOptions() is not covariant with return type array of method  
         Symfony\Component\Validator\Constraint::getRequiredOptions().                                                                                                      
  73     Return type mixed of method Propel\Bundle\PropelBundle\Validator\Constraints\UniqueObject::getTargets() is not covariant with return type array|string of method   
         Symfony\Component\Validator\Constraint::getTargets().                                                                                                              
 ------ ------------------------------------------------------------------------------------------------------------------------------------------------------------------- 

 ------ ----------------------------------------------------------------------------------------------------------------------------------------- 
  Line   Validator/Constraints/UniqueObjectValidator.php                                                                                          
 ------ ----------------------------------------------------------------------------------------------------------------------------------------- 
  30     Method Propel\Bundle\PropelBundle\Validator\Constraints\UniqueObjectValidator::validate() has parameter $object with no type specified.  
  36     Cannot access constant TABLE_MAP on class-string|false.                                                                                  
 ------ -----------------------------------------------------------------------------------------------------------------------------------------

SkyFoxvn · 2024-09-02T14:51:38Z

sorry i did forget to fix github actions for this branch so can you apply the changes from last 2 commits on your branch so we can run the tests here too https://github.com/SkyFoxvn/PropelBundle/commits/7.0/
tnx

smhg · 2024-09-02T14:55:19Z

@ldaspt please also apply the array return type on this line. This was the second deprecation warning.

…ndle\Tests\Fixtures\Model\User::eraseCredentials() must be compatible with Symfony\Component\Security\Core\User\UserInterface::eraseCredentials(): void

Fix error phpstan Line Command/FixturesLoadCommand.php ------ ----------------------------------------------------------------------------------------------------------------------------------------------------------------------------------------------------- 112 Return type mixed of method Propel\Bundle\PropelBundle\Command\FixturesLoadCommand::execute() is not covariant with return type int of method Symfony\Component\Console\Command\Command::execute().

SkyFoxvn · 2024-09-02T15:02:46Z

btw about "The bundle does not have tests for command, there are no usage of FixturesLoadCommand in the directory Tests and I do not find of propel:fixtures:load in the directory" feel free to go wild with writing more tests :D im kind of lazy(and busy) more tests its always good

ldaspt · 2024-09-02T15:06:32Z

@SkyFoxvn Rebase done

@smhg Can you give me the depreciation warning, because it seems not necessary. There are no phpstan errors on the file Twig/Extension/SyntaxExtension.php and the signature of ExtensionInterface is

    /**
     * Returns a list of filters to add to the existing list.
     *
     * @return TwigFilter[]
     */
    public function getFilters();

https://github.com/twigphp/Twig/blob/3.x/src/Extension/ExtensionInterface.php

ldaspt · 2024-09-02T15:10:18Z

btw about "The bundle does not have tests for command, there are no usage of FixturesLoadCommand in the directory Tests and I do not find of propel:fixtures:load in the directory" feel free to go wild with writing more tests :D im kind of lazy(and busy) more tests its always good

Sorry, it was not a criticism, it was just an explanation of why the error was not present in the tests. Thank you very much for maintaining the bundle.
/

smhg · 2024-09-02T15:13:26Z

@ldaspt strange. I get this in a Symfony 6.4/PHP 8.1 project running PHPUnit:

 1x: Method "Twig\Extension\ExtensionInterface::getFilters()" might add "array" as a native return type declaration in the future. Do the same in implementation "Propel\Bundle\PropelBundle\Twig\Extension\SyntaxExtension" now to avoid errors or add an explicit @return annotation to suppress this message.

I guess they've already added the return phpdoc in Symfony 7.x. But an explicit return typehint is a good thing to have, no? Since it only returns an array, it's possible to add it. Afterwards it is warning-free in both versions.

SkyFoxvn · 2024-09-02T15:22:13Z

@ldaspt dont worry i dont take it as criticism and its not like i cant do more i just i guess i dont want to invest more time then necessary i made the fork mostly with the idea to keep it alive i dont even use propel anymore(not me choice do as you know sometimes we work with what we must :) )

also i appreciate the help @ldaspt @smhg tnx feel free to make more changes, tests etc

@return

1x: Method "Twig\Extension\ExtensionInterface::getFilters()" might add "array" as a native return type declaration in the future. Do the same in implementation "Propel\Bundle\PropelBundle\Twig\Extension\SyntaxExtension" now to avoid errors or add an explicit @return annotation to suppress this message.

See by `SYMFONY_PATCH_TYPE_DECLARATIONS=force=2 vendor/bin/patch-type-declarations` Commit contains only classes implementing Symfony interfaces or extends Synfony classes https://symfony.com/blog/symfony-7-0-type-declarations#preparing-your-applications

ldaspt · 2024-09-03T09:09:37Z

@ldaspt strange. I get this in a Symfony 6.4/PHP 8.1 project running PHPUnit:
 1x: Method "Twig\Extension\ExtensionInterface::getFilters()" might add "array" as a native return type declaration in the future. Do the same in implementation "Propel\Bundle\PropelBundle\Twig\Extension\SyntaxExtension" now to avoid errors or add an explicit @return annotation to suppress this message.
I guess they've already added the return phpdoc in Symfony 7.x. But an explicit return typehint is a good thing to have, no? Since it only returns an array, it's possible to add it. Afterwards it is warning-free in both versions.

@smhg Running SYMFONY_PATCH_TYPE_DECLARATIONS="2" vendor/bin/patch-type-declarations adds indeed the typehint. The commit contains only classes implementing Symfony interfaces or extends Symfony classes. @see https://symfony.com/blog/symfony-7-0-type-declarations#preparing-your-applications

smhg · 2024-09-03T09:36:14Z

@SkyFoxvn I think this looks great. If you can run CI and everything is green, a release would be awesome!

Thank you both!

ldaspt · 2024-09-03T10:23:43Z

For information, I created a PR in the Propel repository to fix the PHP depreciations Use of "parent" in callables is deprecated @see propelorm/Propel2#2015

SkyFoxvn · 2024-09-04T11:01:06Z

@ldaspt it look ok but can you just edit the readme file to point the right branch and i will merge it after that but the release will wait for few days i want to remove all the warning and deprecation messages before i make a release

ldaspt · 2024-09-04T12:58:53Z

@SkyFoxvn Readme updated

ldaspt marked this pull request as ready for review September 2, 2024 07:32

ldaspt force-pushed the propel2-beta4 branch from 3bec690 to 9e1a16a Compare September 2, 2024 12:01

ldaspt changed the base branch from 6.0 to 7.0 September 2, 2024 12:01

Ludovic Daoudal and others added 3 commits September 2, 2024 16:56

Dependency upgrade to propel 2.0.0-beta4 and symfony 7

98ad94f

Fix tests \n\n PHP Fatal error: Declaration of Propel\Bundle\PropelBu…

e0eddb6

…ndle\Tests\Fixtures\Model\User::eraseCredentials() must be compatible with Symfony\Component\Security\Core\User\UserInterface::eraseCredentials(): void

ldaspt force-pushed the propel2-beta4 branch from 486693b to cd0e02a Compare September 2, 2024 14:56

LDA added 2 commits September 3, 2024 10:25

Fix others symfony warnings

3ec28a0

See by `SYMFONY_PATCH_TYPE_DECLARATIONS=force=2 vendor/bin/patch-type-declarations` Commit contains only classes implementing Symfony interfaces or extends Synfony classes https://symfony.com/blog/symfony-7-0-type-declarations#preparing-your-applications

Update README for branch 7.0

fc775f7

SkyFoxvn merged commit 1b8d723 into SkyFoxvn:7.0 Sep 4, 2024
35 checks passed

Provide feedback

Saved searches

Use saved searches to filter your results more quickly

Dependency upgrade to propel 2.0.0-beta4 and symfony 7 #41

Dependency upgrade to propel 2.0.0-beta4 and symfony 7 #41

ldaspt commented Aug 8, 2024

smhg commented Aug 27, 2024

SkyFoxvn commented Aug 27, 2024

smhg commented Aug 28, 2024

SkyFoxvn commented Aug 29, 2024

smhg commented Aug 31, 2024

ldaspt commented Sep 2, 2024

SkyFoxvn commented Sep 2, 2024

ldaspt commented Sep 2, 2024

smhg commented Sep 2, 2024

SkyFoxvn commented Sep 2, 2024 •

edited

Loading

ldaspt commented Sep 2, 2024

smhg commented Sep 2, 2024

SkyFoxvn commented Sep 2, 2024

ldaspt commented Sep 2, 2024

smhg commented Sep 2, 2024

smhg commented Sep 2, 2024

ldaspt commented Sep 2, 2024

SkyFoxvn commented Sep 2, 2024

smhg commented Sep 2, 2024 •

edited

Loading

SkyFoxvn commented Sep 2, 2024

ldaspt commented Sep 2, 2024

ldaspt commented Sep 2, 2024

smhg commented Sep 2, 2024 •

edited

Loading

SkyFoxvn commented Sep 2, 2024

ldaspt commented Sep 3, 2024

smhg commented Sep 3, 2024

ldaspt commented Sep 3, 2024

SkyFoxvn commented Sep 4, 2024

ldaspt commented Sep 4, 2024

Dependency upgrade to propel 2.0.0-beta4 and symfony 7 #41

Dependency upgrade to propel 2.0.0-beta4 and symfony 7 #41

Conversation

ldaspt commented Aug 8, 2024

smhg commented Aug 27, 2024

SkyFoxvn commented Aug 27, 2024

smhg commented Aug 28, 2024

SkyFoxvn commented Aug 29, 2024

smhg commented Aug 31, 2024

ldaspt commented Sep 2, 2024

SkyFoxvn commented Sep 2, 2024

ldaspt commented Sep 2, 2024

smhg commented Sep 2, 2024

SkyFoxvn commented Sep 2, 2024 • edited Loading

ldaspt commented Sep 2, 2024

smhg commented Sep 2, 2024

SkyFoxvn commented Sep 2, 2024

ldaspt commented Sep 2, 2024

smhg commented Sep 2, 2024

smhg commented Sep 2, 2024

ldaspt commented Sep 2, 2024

SkyFoxvn commented Sep 2, 2024

smhg commented Sep 2, 2024 • edited Loading

SkyFoxvn commented Sep 2, 2024

ldaspt commented Sep 2, 2024

ldaspt commented Sep 2, 2024

smhg commented Sep 2, 2024 • edited Loading

SkyFoxvn commented Sep 2, 2024

ldaspt commented Sep 3, 2024

smhg commented Sep 3, 2024

ldaspt commented Sep 3, 2024

SkyFoxvn commented Sep 4, 2024

ldaspt commented Sep 4, 2024

SkyFoxvn commented Sep 2, 2024 •

edited

Loading

smhg commented Sep 2, 2024 •

edited

Loading

smhg commented Sep 2, 2024 •

edited

Loading