MichaelBrauner commented 3 years ago

Symfony version(s) affected: 5.3.0-BETA2 "symfony/maker-bundle": "^1.0", -> 1.31.1

Description

michaelbrauner@Michaels-MacBook-Pro cardedu % symfony console make:entity               

 Class name of the entity to create or update (e.g. GentleChef):
 > Source

 Mark this class as an API Platform resource (expose a CRUD API for it) (yes/no) [no]:
 > 

In Generator.php line 140:

  Warning: Uninitialized string offset 0

michaelbrauner@Michaels-MacBook-Pro cardedu % symfony console make:entity

 Class name of the entity to create or update (e.g. VictoriousGnome):
 > Source

 Mark this class as an API Platform resource (expose a CRUD API for it) (yes/no) [no]:
 > yes

 [ERROR] "App\Entity\1" is not valid as a PHP class name (it must start with a letter or underscore, followed by any    
         number of letters, numbers, or underscores)                                                                    

exit status 1

Additional context

marbulk commented 3 years ago

This issue should be reported in the MakerBundle repository.

marbulk commented 3 years ago

Please update the version affected i.e. Maker Bundle version you are using, not Symfony version.

MichaelBrauner commented 3 years ago

Ok, I updated the version of the MakerBundle . Thank you much for transfering this issue.

marbulk commented 3 years ago

I could not reproduce the issue with my reproducer https://github.com/marbulk/maker_bundle_878.

$ php -v
PHP 8.0.2 (cli) (built: Feb 18 2021 00:18:34) ( NTS )
Copyright (c) The PHP Group
Zend Engine v4.0.2, Copyright (c) Zend Technologies
$ composer --version
Composer version 2.0.11 2021-02-24 14:57:23
$ composer create-project symfony/skeleton
$ composer require orm api --no-unpack
$ composer require symfony/maker-bundle:1.31.1

bin/console make:entity output:

Please provide you reproducer app as this will be easier for everyone.

PS. Next time use -vvv option when running console application so we can see the full stack trace. EDIT: Maker Bundle silences normal exception handling for exceptions of type RuntimeCommandException however adding -vvv option to the screenshots is a good habit when reporting issues.

MichaelBrauner commented 3 years ago

OK. Thank you for dealing with this error.

How do I create such a reproducer app?

If I have the time, I can dig myself and find out why this error happens inside my app.

marbulk commented 3 years ago

Reproducer app is a minimum code that reproduces a potential bug. Often it is not enough to create an issue and say "This does not work" and then we have to somehow reproduce the bug. Most people encounter bugs when coding commercial applications and cannot share it's code (event if they could then these apps are probably big enough and community will not spend time to get the app up and running to try reproduce an issue) so a simple reproducer is needed.

I did create such an app where I tried to reproduce your issue but I wasn't successful because everything worked. I pushed my reproducer app to GitHub so everyone can see the code I used to reproduce a potential bug. In my previous comment I also added some information what steps I did to create my app (shell code sample).

MichaelBrauner commented 3 years ago

Ok, thank you for your explanation. Without installing such a reproducer app - I finally found the problem by old-school try and error dumping.

It did not work because you did not install the new Symonfy UX Turbo Bundle.

In symfony/maker-bundle - src/Maker/MakeEntity.php on Line 136

You ask for $this->generator->createClassNameDetails($value, 'Entity\\')->getFullName(). Since you dont have installed Symfony UX Turbo you don't have a Attribute/Broadcast.php and the if statement never goes that far - so you get no error.

The $value is not the name of the Class anymore, because it has been overwritten in Line 128:

$value = $io->askQuestion($question);

By asking for making an api resource of it or not.

So when I select no an undefined offset of 0 is given as Classname. When I select yes - internally the value is 1- so the generator asks for the ClassNameDetails of a Class ''App\Entity\1"

That's it. I hope I could have helped.

marbulk commented 3 years ago

Nice to hear you reproduced this. Yes, I've looked at code and this is a bug. I think we can just rename the variable $value and this will do.

lines 117, 119, 124, 136: $value -> $entityClassName
lines 128, 130: $value -> $isApiResource
lines 140, 142: $value -> $isBroadcast

Will you come up with a PR? Also failing test case would be nice.

MichaelBrauner commented 3 years ago

Ok. I mean, I never did a PR or something and I did not write any PHPUnit tests before. But I can try it. Such a small bug ist maybe a good starting point to learn.

MichaelBrauner commented 3 years ago

So. I cloned the symfony/maker-bundle repository into my /lib folder inside my projekt where my self written bundles goes in.

I added the path to composer.json:

        {
            "type": "path",
            "url": "./lib/SymfonyMakerBundle"
        }

And ran composer update.

It symlinked the new repo and now I use the cloned maker-bundle.

So first I want to make all written tests running - for knowing - things work fine.

I started to install PHPUnit (PHPUnit Bridge) and after running one time vendor/bin/simple-phpunit for setting things up - I ran:

vendor/bin/simple-phpunit ./lib/SymfonyMakerBundle/tests/Maker/MakeEntityTest.php

And all 25 Test fail :).

Testing Symfony\Bundle\MakerBundle\Tests\Maker\MakeEntityTest EEEEEEEEEEEEEEEEEEEEEEEEE 25 / 25 (100%)

Time: 20.12 seconds, Memory: 32.00 MB

There were 25 errors:

1) Symfony\Bundle\MakerBundle\Tests\Maker\MakeEntityTest::testExecute with data set "entity_new" (Symfony\Bundle\MakerBundle\Test\MakerTestDetails Object (...)) Exception: Error running command: "php dep_runner.php". Output: " Warning: require(/Users/michaelbrauner/Sites/Private/cardedu/lib/SymfonyMakerBundle/tests/tmp/cache/maker_app_a415fad3bbeef5284f98d1d9e50b9051/vendor/composer/../symfony/phpunit-bridge/bootstrap.php): Failed to open stream: No such file or directory in /Users/michaelbrauner/Sites/Private/cardedu/lib/SymfonyMakerBundle/tests/tmp/cache/maker_app_a415fad3bbeef5284f98d1d9e50b9051/vendor/composer/autoload_real.php on line 71

Fatal error: Uncaught Error: Failed opening required '/Users/michaelbrauner/Sites/Private/cardedu/lib/SymfonyMakerBundle/tests/tmp/cache/maker_app_a415fad3bbeef5284f98d1d9e50b9051/vendor/composer/../symfony/phpunit-bridge/bootstrap.php' (include_path='.:/usr/local/Cellar/php/8.0.1_1/share/php/pear') in /Users/michaelbrauner/Sites/Private/cardedu/lib/SymfonyMakerBundle/tests/tmp/cache/maker_app_a415fad3bbeef5284f98d1d9e50b9051/vendor/composer/autoload_real.php:71 Stack trace:

0 /Users/michaelbrauner/Sites/Private/cardedu/lib/SymfonyMakerBundle/tests/tmp/cache/maker_app_a415fad3bbeef5284f98d1d9e50b9051/vendor/composer/autoload_real.php(61): composerRequireceb688a4da51dc45adc3a0495417bcf0('92c8763cd6170fc...', '/Users/michaelb...')

1 /Users/michaelbrauner/Sites/Private/cardedu/lib/SymfonyMakerBundle/tests/tmp/cache/maker_app_a415fad3bbeef5284f98d1d9e50b9051/vendor/autoload.php(7): ComposerAutoloaderInitceb688a4da51dc45adc3a0495417bcf0::getLoader()

2 /Users/michaelbrauner/Sites/Private/cardedu/lib/SymfonyMakerBundle/tests/tmp/cache/maker_app_a415fad3bbeef5284f98d1d9e50b9051/dep_runner.php(3): require('/Users/michaelb...')

3 {main}

thrown in /Users/michaelbrauner/Sites/Private/cardedu/lib/SymfonyMakerBundle/tests/tmp/cache/maker_app_a415fad3bbeef5284f98d1d9e50b9051/vendor/composer/autoload_real.php on line 71 ". Error: ""

/Users/michaelbrauner/Sites/Private/cardedu/lib/SymfonyMakerBundle/src/Test/MakerTestProcess.php:51 /Users/michaelbrauner/Sites/Private/cardedu/lib/SymfonyMakerBundle/src/Test/MakerTestEnvironment.php:438 /Users/michaelbrauner/Sites/Private/cardedu/lib/SymfonyMakerBundle/src/Test/MakerTestEnvironment.php:151 /Users/michaelbrauner/Sites/Private/cardedu/lib/SymfonyMakerBundle/src/Test/MakerTestCase.php:55 /Users/michaelbrauner/Sites/Private/cardedu/lib/SymfonyMakerBundle/src/Test/MakerTestCase.php:33

When I have time I will see, that I get further. But if you can help me a bit, it would be nice.

marbulk commented 3 years ago

You can create a PR with just the changes I proposed and later with another PR with a test case. I think this bug is serious enough to merge the fix and we can add the test later.

PS. I will look at your last comment later, not sure if today.

MichaelBrauner commented 3 years ago

Ok.

I've done the changes.

I created a new branch on my cloned repo:

git checkout -b make_entity_fix_878

Did the changes and made a commit:

git commit -m "Fix - error on make:entity when API-Platform is installed. Fixes #878"

I created a new remote:

git remote add upstream git@github.com:MichaelBrauner/maker-bundle.git

And pushed it:

git push -u upstream make_entity_fix_878

But now - I can't find a way to create a PR out of this.

Sorry, but this is my first PR ever. Can you please tell me what I did wrong?

marbulk commented 3 years ago

Did you manage to make test up and running? I was just about to take a look at your yesterday's comment.

If you want to create a Pull Request you have to fork symfony/maker-bundle repository (see at the right top corner). Then push your changes to new branch in your fork. Then in "Pull Requests" in forked repo click "Create new pull request" (you will be redirected to symfony/maker-bundle repo - see image I've pasted) and choose your branch in "compare" select.

MichaelBrauner commented 3 years ago

Ah ok. I just cloned it. Sorry.

I read yesterday into the testing documentation of symfony and I think I have to create the testing database first.

And run the command like symfony run vendor/bin/simple-phpunit ./lib/SymfonyMakerBundle/tests/Maker/MakeEntityTest.php

I will try to get the tests running, when I created the PR.

MichaelBrauner commented 3 years ago

I created the PR ;)

But why the tests all fail is a mystery to me.

I created the database and migrated it.

But all tests fail:

michaelbrauner@Michaels-MacBook-Pro cardedu % APP_ENV=test symfony console doctrine:database:create
Created database "main_test" for connection named default
michaelbrauner@Michaels-MacBook-Pro cardedu % APP_ENV=test symfony console doctrine:migrations:migrate -n
[notice] Migrating up to DoctrineMigrations\Version20210516031111
[warning] Migration DoctrineMigrations\Version20210313185742 was executed but did not result in any SQL statements.
[warning] Migration DoctrineMigrations\Version20210313185936 was executed but did not result in any SQL statements.
[warning] Migration DoctrineMigrations\Version20210514194205 was executed but did not result in any SQL statements.
[warning] Migration DoctrineMigrations\Version20210514194225 was executed but did not result in any SQL statements.
[warning] Migration DoctrineMigrations\Version20210515110620 was executed but did not result in any SQL statements.
[notice] finished in 552.2ms, used 28M memory, 26 migrations executed, 177 sql queries

michaelbrauner@Michaels-MacBook-Pro cardedu % APP_ENV=test symfony php vendor/bin/simple-phpunit ./lib/SymfonyMakerBundle/tests/Maker/MakeEntityTest.php
PHPUnit 8.5.15 by Sebastian Bergmann and contributors.

Testing Symfony\Bundle\MakerBundle\Tests\Maker\MakeEntityTest
EEEEEEEEEEEEEEEEEEEEEEEEE                                         25 / 25 (100%)

Time: 16.24 seconds, Memory: 32.00 MB

There were 25 errors:

1) Symfony\Bundle\MakerBundle\Tests\Maker\MakeEntityTest::testExecute with data set "entity_new" (Symfony\Bundle\MakerBundle\Test\MakerTestDetails Object (...))
Exception: Error running command: "php dep_runner.php". Output: "". Error: ""

/Users/michaelbrauner/Sites/Private/cardedu/lib/SymfonyMakerBundle/src/Test/MakerTestProcess.php:51
/Users/michaelbrauner/Sites/Private/cardedu/lib/SymfonyMakerBundle/src/Test/MakerTestEnvironment.php:438
/Users/michaelbrauner/Sites/Private/cardedu/lib/SymfonyMakerBundle/src/Test/MakerTestEnvironment.php:151
/Users/michaelbrauner/Sites/Private/cardedu/lib/SymfonyMakerBundle/src/Test/MakerTestCase.php:55
/Users/michaelbrauner/Sites/Private/cardedu/lib/SymfonyMakerBundle/src/Test/MakerTestCase.php:33

2) Symfony\Bundle\MakerBundle\Tests\Maker\MakeEntityTest::testExecute with data set "entity_new_api_resource" (Symfony\Bundle\MakerBundle\Test\MakerTestDetails Object (...))
Exception: Error running command: "php dep_runner.php". Output: "". Error: ""

/Users/michaelbrauner/Sites/Private/cardedu/lib/SymfonyMakerBundle/src/Test/MakerTestProcess.php:51
/Users/michaelbrauner/Sites/Private/cardedu/lib/SymfonyMakerBundle/src/Test/MakerTestEnvironment.php:438
/Users/michaelbrauner/Sites/Private/cardedu/lib/SymfonyMakerBundle/src/Test/MakerTestEnvironment.php:151
/Users/michaelbrauner/Sites/Private/cardedu/lib/SymfonyMakerBundle/src/Test/MakerTestCase.php:55
/Users/michaelbrauner/Sites/Private/cardedu/lib/SymfonyMakerBundle/src/Test/MakerTestCase.php:33

3) Symfony\Bundle\MakerBundle\Tests\Maker\MakeEntityTest::testExecute with data set "entity_with_fields" (Symfony\Bundle\MakerBundle\Test\MakerTestDetails Object (...))
Exception: Could not find "postgresql://db_user:db_password@127.0.0.1:5432/db_name?serverVersion=13&charset=utf8" inside ".env"

marbulk commented 3 years ago

if you want to run symfony/maker-bundle tests you have to run phpunit with the config file from this repository. You did run this with your's app phpunit.xml. Plus, you have to run phpunit from maker bundle directory, not your application root. I recommend moving maker bundle directory outside of your project directory. This will be better for everyone.
you have to run composer install against symfony/maker-bundle dependencies. So in lib/SymfonyMakerBundle you have to have vendor/ directory created.
You have to inspect phpunit.xml.dist and see what to adjust in local environment for the tests to run. In this case it is only TEST_DATABASE_DSN env var. (I recommend copy pasting phpunit.xml.dist to phpunit.xml which is ignored by git)
symfony/maker-bundle tests require a database. You have to set it's dsn in TEST_DATABASE_DSN env var.
You don't have to create a database and execute any migrations (what's more these are your migrations so it doesn't matter for maker bundle). Just set the correct TEST_DATABASE_DSN and maker bundle will take care of the rest.
in case of this issue I don't think you have to run all the tests. You have to run a unit test that tests the class you've made changes to. Then you can create a PR and the GitHub Actions will run all the tests and we will see if something else had broken accidentally.
your PR is failing but I can see all the PRs are failing recently. Failures in Appveyor are not related to you PR so maintainers will take care of that. For now just make sure the test for class you modified passes locally.

MichaelBrauner commented 3 years ago

Ok.

I moved SymfonyMakerBundle outside of project root.
composer install was successfull
Yesterday I created a test database. I exported the vars: symfony var:export and copied the DATABASE_URL into the TEST_DATABASE_DSN env var inside my newly created phpunit.xml (copied from phpunit.xml.dist)
I ran only the MakeEntityTest file: APP_ENV=test symfony run vendor/bin/simple-phpunit ./tests/Maker/MakeEntityTest.php from inside the SymfonyMakerBundle folder - but tests are all failing with the exact same errors as before:

michaelbrauner@Michaels-MacBook-Pro SymfonyMakerBundle % APP_ENV=test symfony run  vendor/bin/simple-phpunit ./tests/Maker/MakeEntityTest.php                        
PHPUnit 9.4.4 by Sebastian Bergmann and contributors.

Testing Symfony\Bundle\MakerBundle\Tests\Maker\MakeEntityTest
EEEEEEEEEEEEEEEEEEEEEEEEE                                         25 / 25 (100%)

Time: 00:01.684, Memory: 26.00 MB

There were 25 errors:

1) Symfony\Bundle\MakerBundle\Tests\Maker\MakeEntityTest::testExecute with data set "entity_new" (Symfony\Bundle\MakerBundle\Test\MakerTestDetails Object (...))
Exception: Could not find "postgresql://db_user:db_password@127.0.0.1:5432/db_name?serverVersion=13&charset=utf8" inside ".env"

/Users/michaelbrauner/Bundles/Symfony/PR/SymfonyMakerBundle/src/Test/MakerTestEnvironment.php:411
/Users/michaelbrauner/Bundles/Symfony/PR/SymfonyMakerBundle/src/Test/MakerTestEnvironment.php:187
/Users/michaelbrauner/Bundles/Symfony/PR/SymfonyMakerBundle/src/Test/MakerTestCase.php:55
/Users/michaelbrauner/Bundles/Symfony/PR/SymfonyMakerBundle/src/Test/MakerTestCase.php:33

2) Symfony\Bundle\MakerBundle\Tests\Maker\MakeEntityTest::testExecute with data set "entity_new_api_resource" (Symfony\Bundle\MakerBundle\Test\MakerTestDetails Object (...))
Exception: Could not find "postgresql://db_user:db_password@127.0.0.1:5432/db_name?serverVersion=13&charset=utf8" inside ".env"

/Users/michaelbrauner/Bundles/Symfony/PR/SymfonyMakerBundle/src/Test/MakerTestEnvironment.php:411
/Users/michaelbrauner/Bundles/Symfony/PR/SymfonyMakerBundle/src/Test/MakerTestEnvironment.php:187
/Users/michaelbrauner/Bundles/Symfony/PR/SymfonyMakerBundle/src/Test/MakerTestCase.php:55
/Users/michaelbrauner/Bundles/Symfony/PR/SymfonyMakerBundle/src/Test/MakerTestCase.php:33

3) Symfony\Bundle\MakerBundle\Tests\Maker\MakeEntityTest::testExecute with data set "entity_with_fields" (Symfony\Bundle\MakerBundle\Test\MakerTestDetails Object (...))
Exception: Could not find "postgresql://db_user:db_password@127.0.0.1:5432/db_name?serverVersion=13&charset=utf8" inside ".env"

I really want to get this running and write a test for that change. Can you imagine, what this could be? Maybe I need a .env file ?

marbulk commented 3 years ago

There must be something wrong with your development environment. This is how MakeEntityTest works:

it executes \Symfony\Bundle\MakerBundle\Test\MakerTestCase::testExecute() 25 times, 1 time per data set.
it creates new Symfony application for every data set, however if some conditions are met (e.g. dependencies are the same) it uses already existing application. When I run phpunit against MakeEntityTest.php I have 4 new Symfony applications (3x makerapp and 1x makercustom) in the tests/tmp/cache/ directory` (you may have less as you are experiencing exceptions):

The problem you have with Exception: Could not find "postgresql://db_user:db_password@127.0.0.1:5432/db_name?serverVersion=13&charset=utf8" inside ".env" suggests there is something wrong with .env file generated when composer require orm is executed by maker-bundle under the hood (I don't want to go into details because it would be too complicated for you as I can see you are a beginner). Please check the .env files in the generated directories like maker_app_301415c0e85e895b74710618531cd645. You can even execute git diff .env inside this directory (yes, this directory is a git repository and yes, this is a git repository inside a maker-bundle/ directory which itself is a git repository) $ cd tests/tmp/cache/maker_app_eb51c85c20df9ad547da242bdaaee04f && git diff .env

and for the DATABASE_URL the proper output is (your DATABASE_URL in the green line may differ, It's the one taken from phpunit.xml):

The red colored DATABASE_URL="postgresql://db_user:db_password@127.0.0.1:5432/db_name?serverVersion=13&charset=utf8" is what doctrine-bundle recipe adds and this is what maker-bundle expects when running tests so it can replace it with the database dsn you set in phunit.xml. Error you are experiencing suggests there is other (or none) DATABASE_URL in this .env file by default.

PS. You don't have to add APP_ENV=test in front of symfony run .... All the configuration is inside phpunit.xml. PS 2. I do not exclude that the problem might be caused by invalid Symfony binary usage. If so then I won't help with this as I don't use this tool. PS 3. When you finally solve this problem you will still have a test for one data set failing (data set entity_new_broadcast) because you have to have Mercure server up and running at 127.0.0.1:1337. Of course you can ignore this data set by running phpunit for a single data set but as you want to write a test case for the problem you reported in this issue you will have to set Mercure up ultimately. PS 4. Please try your best because I won't be able to write such a large explanatory comments any more. I think you've gained a lot of knowledge from my help, however I have to move on. Maybe @weaverryan will be happy to take control over answering your future questions.

MichaelBrauner commented 3 years ago

Thank you so much for giving me all this information.

Actually yesterday I digged into MakeTestEnvironment.php and MakerTestCase.php and found out myself that there are created new symfony projects per case with git.

So, there is no DATABASE_URL at all inside this .env files for the created projects.

Hm, today in the evening after my work day - I wil try it wihtout the symfony binary - maybe thats the problem.

I really appreciate that you spent so long on me and this little thing.

MichaelBrauner commented 3 years ago

Ok. Now I researched a while and I have the 4 projects inside my tmp directory and as I (and you) said - there is no DATABASE_URL inside by default - also nothing to replace - so I get the error messages.

I tried it without the symfony binary and I get the same results.

Might this be the part of the MakerBundle tests that are broken at the moment? Because when I execute phpunit for all maker-bundle tests - I have a good bunch of succeding tests:

symfony / maker-bundle

Symfony console make:entity returns error after asking to "mark this class as an API Platform resource" #878

0 /Users/michaelbrauner/Sites/Private/cardedu/lib/SymfonyMakerBundle/tests/tmp/cache/maker_app_a415fad3bbeef5284f98d1d9e50b9051/vendor/composer/autoload_real.php(61): composerRequireceb688a4da51dc45adc3a0495417bcf0('92c8763cd6170fc...', '/Users/michaelb...')

1 /Users/michaelbrauner/Sites/Private/cardedu/lib/SymfonyMakerBundle/tests/tmp/cache/maker_app_a415fad3bbeef5284f98d1d9e50b9051/vendor/autoload.php(7): ComposerAutoloaderInitceb688a4da51dc45adc3a0495417bcf0::getLoader()

2 /Users/michaelbrauner/Sites/Private/cardedu/lib/SymfonyMakerBundle/tests/tmp/cache/maker_app_a415fad3bbeef5284f98d1d9e50b9051/dep_runner.php(3): require('/Users/michaelb...')

3 {main}