How does one add a failing test case?

[NotionCommotion]

Question

I earlier asked another question here, but then that question morphed to something totally different so thought it best to create a new issue which is on topic. I am trying to gain a better understand how to perform https://github.com/rectorphp/rector/blob/main/docs/how_to_add_test_for_rector_rule.md, and have a few questions.

1. Should Symfony be installed first (i.e. new symfony my_project)
2. Should phpunit be installed first (i.e. composer require --dev phpunit/phpunit ^9)
3. Is PhpStorm required? Note that I do not use it.
4. Is there any other software (other than composer require rector/rector --dev of course) that needs to be installed?
5. Must content from rector-src be used? Reason I ask is it has rules-tests/Privatization/Rector/Class_/FinalizeClassesWithoutChildrenRector/Fixture.
6. When performing Step 3 Find the Rector Test Case, where are these directories and files located, and are they somehow automatically created by Rector (which they are not being done so for me) or do I manually add them? If I must manually add them, how do I know what content to include other than copying what is in rector-src?
7. If PHPUnit fails and I've successfully added a test case, where do I find it so I can include with any bugs I might find?

I ended up copying rules-tests from rector-src to my project and then added add_final.php.inc to match the the script shown in step 4 Add Change or No-Change Test Fixture File. But when executing vendor/bin/phpunit rules-tests/Privatization/Rector/Class_/FinalizeClassesWithoutChildrenRector/FinalizeClassesWithoutChildrenRectorTest.php, got some errors that /src/constants.php and /preload.php were missing so just copied them from rector-src, and then that Class Symfony\Bridge\PhpUnit\SymfonyTestsListener does not exist and concluded I am obviously doing something wrong :(

Appreciate any help and apologize if I should already know how to do this.

[TomasVotruba]

Hi, thanks for asking. I'll try to navigate you 👍

If it's your first time contributing Rector, I understand it might be a bit complicated. There is a release repo (this one) and development repository (https://github.com/rectorphp/rector-src) - that's where all the contributions happen.

1, 2, 4, 5 can be answered like this:

• fork https://github.com/rectorphp/rector-src
• clone it locally

git clone <...> rector-src
cd rector-src

• install dependencies

composer install

• create new branch for your test
• add test
• push the branch
• create new pull request to https://github.com/rectorphp/rector-src

✔️

There rest like IDE is optional. It's good to use something you're expert with.

[NotionCommotion]

Ah, so that was it! Thank you for letting me know! Maybe it was obvious and all should have known, but if not, maybe consider adding this on the main documentation page.

When executing composer install, got the following error, however, I then executed it a second time and maybe not an issue.

I will create a test and pull request and send you the steps I take in hopes that it will be helpful to add to the documents.

Thanks! Michael

[michael@devserver www]$ git clone https://github.com/rectorphp/rector-src.git rector-src
Cloning into 'rector-src'...
remote: Enumerating objects: 236371, done.
remote: Counting objects: 100% (1505/1505), done.
remote: Compressing objects: 100% (816/816), done.
remote: Total 236371 (delta 681), reused 1342 (delta 612), pack-reused 234866
Receiving objects: 100% (236371/236371), 47.55 MiB | 22.18 MiB/s, done.
Resolving deltas: 100% (140652/140652), done.
[michael@devserver www]$ cd rector-src
[michael@devserver rector-src]$ composer install
No lock file found. Updating dependencies instead of installing from lock file. Use composer update over composer install if you do not have a lock file.
Loading composer repositories with package information
Updating dependencies
Lock file operations: 119 installs, 0 updates, 0 removals
  - Locking composer/semver (3.2.5)
  ...
  - Locking sebastian/version (3.0.2)
  - Locking ssch/typo3-rector (dev-main e75102b)
  - Locking symfony/config (v5.3.3)
  ...
  - Locking webmozart/assert (1.10.0)
Writing lock file
Installing dependencies from lock file (including require-dev)
Package operations: 119 installs, 0 updates, 0 removals
  - Downloading rector/extension-installer (0.10.2)
  ...
  - Downloading helmich/typo3-typoscript-parser (v2.3.1)
  - Syncing ssch/typo3-rector (dev-main e75102b) into cache
  - Downloading symfony/polyfill-php72 (v1.23.0)
  - Downloading timeweb/phpstan-enum (v2.3.1)
  ...
  - Downloading tracy/tracy (v2.8.5)
  - Installing rector/extension-installer (0.10.2): Extracting archive
  ...
  - Installing helmich/typo3-typoscript-parser (v2.3.1): Extracting archive
  - Installing ssch/typo3-rector (dev-main e75102b): Cloning e75102b from cache
    e75102b is gone (history was rewritten?)
    Install of ssch/typo3-rector failed
  - Installing symfony/options-resolver (v5.3.0): Extracting archive
  ...
  - Installing tracy/tracy (v2.8.5): Extracting archive
 103/103 [============================] 100%

  [RuntimeException]
  Failed to execute git checkout 'e75102b' -- && git reset --hard 'e75102b' --

  fatal: invalid reference: e75102b


install [--prefer-source] [--prefer-dist] [--dry-run] [--dev] [--no-suggest] [--no-dev] [--no-autoloader] [--no-scripts] [--no-progress] [--no-install] [-v|vv|vvv|--verbose] [-o|--optimize-autoloader] [-a|--classmap-authoritative] [--apcu-autoloader] [--apcu-autoloader-prefix APCU-AUTOLOADER-PREFIX] [--ignore-platform-req IGNORE-PLATFORM-REQ] [--ignore-platform-reqs] [--] [<packages>]...

[michael@devserver rector-src]$ composer install
Installing dependencies from lock file (including require-dev)
Verifying lock file contents can be installed on current platform.
Nothing to install, update or remove
Generating autoload files
80 packages you are using are looking for funding.
Use the `composer fund` command to find out more!
phpstan/extension-installer: Extensions installed
> phpstan/phpstan-nette: installed
> phpstan/phpstan-phpunit: installed
> rector/phpstan-rules: installed
> symplify/phpstan-extensions: installed
> symplify/phpstan-rules: installed
> timeweb/phpstan-enum: installed
rector/rector-installer: Extensions installed
> rector/rector-cakephp: installed
> rector/rector-doctrine: installed
> rector/rector-generator: installed
> rector/rector-laravel: installed
> rector/rector-nette: installed
> rector/rector-phpunit: installed
> rector/rector-symfony: installed
> ssch/typo3-rector: installed
[michael@devserver rector-src]$

[TomasVotruba]

No troubles, I'm glad it was useful for you.

Such workflow is typical for any contribution to git repository that you don't own, so ther is no need to add to documentation. It's there in README.md https://github.com/rectorphp/rector#how-to-contribute, but people don't find it :)

The other issue is just composer glitch. Re-running is a way to go 👍

[NotionCommotion]

Thanks again Tomas, Most is now making sense now :)

I actually saw https://github.com/rectorphp/rector#how-to-contribute as well as https://github.com/rectorphp/rector/blob/main/CONTRIBUTING.md, but didn't realize the significance. I went back to read it in more detail and realized I didn't execute composer complete-check first. When doing it, I received an excessive memory error so increased PHP's memory limit above 128M and kept doing so up to 2048M and it went longer but still errored out. How much memory is typically needed?

[michael@devserver rector-src]$ composer complete-check
> vendor/bin/ecs check --ansi
 488/488 [▓▓▓▓▓▓▓▓▓▓▓▓▓▓▓▓▓▓▓▓▓▓▓▓▓▓▓▓] 100%


 [OK] No errors found. Great job - your code is shiny in style!


> vendor/bin/ecs check-markdown README.md --ansi
 1/1 [▓▓▓▓▓▓▓▓▓▓▓▓▓▓▓▓▓▓▓▓▓▓▓▓▓▓▓▓] 100%


 [OK] No errors found. Great job - your code is shiny in style!


> vendor/bin/phpstan analyse --ansi --error-format symplify
Note: Using configuration file /var/www/rector-src/phpstan.neon.
 1395/1395 [▓▓▓▓▓▓▓▓▓▓▓▓▓▓▓▓▓▓▓▓▓▓▓▓▓▓▓▓] 100%



 [OK] No errors


> vendor/bin/rule-doc-generator generate packages rules --output-file build/rector_rules_overview.md --ansi --categorize

 ! [NOTE] Found 491 documented rule classes

 * Rector\Arguments\Rector\ClassMethod\ArgumentAdderRector
 ...
 * Rector\TypeDeclaration\Rector\Property\TypedPropertyFromStrictConstructorRector
 * Rector\Visibility\Rector\ClassConst\ChangeConstantVisibilityRector
 * Rector\Visibility\Rector\ClassMethod\ChangeMethodVisibilityRector

PHP Fatal error:  Allowed memory size of 2147483648 bytes exhausted (tried to allocate 262144 bytes) in /var/www/rector-src/vendor/symplify/php-config-printer/src/NodeFactory/NewValueObjectFactory.php on line 54
Script vendor/bin/rule-doc-generator generate packages rules --output-file build/rector_rules_overview.md --ansi --categorize handling the docs event returned with error code 255
Script @docs was called via complete-check
[michael@devserver rector-src]$

Do I want to use vendor/bin/rector process app/SomeFile.php per the docs or bin/rector process app/SomeFile.php? As seen below, rector doesn't appear to exist in vendor/bin/rector.

[michael@devserver rector-src]$ ls -l vendor/bin
total 0
lrwxrwxrwx 1 michael michael 31 Jul  9 16:30 easy-ci -> ../symplify/easy-ci/bin/easy-ci
lrwxrwxrwx 1 michael michael 41 Jul  9 16:30 easy-testing -> ../symplify/easy-testing/bin/easy-testing
lrwxrwxrwx 1 michael michael 40 Jul  9 16:30 ecs -> ../symplify/easy-coding-standard/bin/ecs
lrwxrwxrwx 1 michael michael 41 Jul  9 16:30 php-cs-fixer -> ../friendsofphp/php-cs-fixer/php-cs-fixer
lrwxrwxrwx 1 michael michael 33 Jul  9 16:30 php-parse -> ../nikic/php-parser/bin/php-parse
lrwxrwxrwx 1 michael michael 26 Jul  9 16:30 phpstan -> ../phpstan/phpstan/phpstan
lrwxrwxrwx 1 michael michael 31 Jul  9 16:30 phpstan.phar -> ../phpstan/phpstan/phpstan.phar
lrwxrwxrwx 1 michael michael 26 Jul  9 16:30 phpunit -> ../phpunit/phpunit/phpunit
lrwxrwxrwx 1 michael michael 53 Jul  9 16:30 rule-doc-generator -> ../symplify/rule-doc-generator/bin/rule-doc-generator
lrwxrwxrwx 1 michael michael 51 Jul  9 16:30 var-dump-server -> ../symfony/var-dumper/Resources/bin/var-dump-server
lrwxrwxrwx 1 michael michael 39 Jul  9 16:30 yaml-lint -> ../symfony/yaml/Resources/bin/yaml-lint
[michael@devserver rector-src]$ ls -l bin
total 12
-rwxrwxr-x 1 michael michael   64 Jul  9 16:28 rector
-rwxrwxr-x 1 michael michael 4652 Jul  9 16:28 rector.php
[michael@devserver rector-src]$

Used the later but now got an error. Strangely, the error occurs in StaticInputDetector::isDebug(), but $x = $argvInput->hasParameterOption(['--debug', '-v', '-vv', '-vvv']); does not cause an error.

Any thoughts? Thank you

[michael@devserver rector-src]$ bin/rector process app/SomeFile.php


 [ERROR] File path "/var/www/rector-src/vendor/ssch/typo3-rector/config/config.php" was not found while creating
         "Symplify\SmartFileSystem\SmartFileInfo" object.

bin/rector.php:

$container = $rectorContainerFactory->createFromBootstrapConfigs($bootstrapConfigs);

namespace Rector\Core\DependencyInjection;

final class RectorContainerFactory
{
	
    /**
     * @param SmartFileInfo[] $configFileInfos
     * @api
     */
    public function createFromConfigs(array $configFileInfos): ContainerInterface
    {
        // to override the configs without clearing cache
        $isDebug = StaticInputDetector::isDebug();
		// ...
    }

    public function createFromBootstrapConfigs(BootstrapConfigs $bootstrapConfigs): ContainerInterface
    {
        $container = $this->createFromConfigs($bootstrapConfigs->getConfigFileInfos());
		//...
    }
}

<?php

declare(strict_types=1);

namespace Symplify\PackageBuilder\Console\Input;

use Symfony\Component\Console\Input\ArgvInput;

final class StaticInputDetector
{
    public static function isDebug(): bool
    {
        $argvInput = new ArgvInput();
        return $argvInput->hasParameterOption(['--debug', '-v', '-vv', '-vvv']);
    }
}

[TomasVotruba]

Hi, I'm glad it helped :)

Rector tests are memory heavy, as every test case is basically a new dependency injection container. We're using Symfony DI here and if we wanted to make tests independent, there is no other way.

Personally I don't run composer complete-check as that's very slow feedback. I prefer running these 2 then letting rest on CI server:

composer fix-cs
composer phpstan

Do I want to use vendor/bin/rector process app/SomeFile.php per the docs

That part works with Rector installed as dependency.

If you work with the repository, it's not located in vendor, but in the root. So it's always bin/<name>. Same goes for vendor/bin/phpunit when using as a dependency, but bin/rector while contributing. See https://github.com/rectorphp/rector-src/tree/main/bin

Generally just clicking around https://github.com/rectorphp/rector-src/tree/main/bin will help you to understand where to look for what type of file.

The bin/rector process app/SomeFile.php looks like a bug.
Could you share what exact commit do you use?

git rev-parse HEAD

[NotionCommotion]

Thanks!

composer fix-cs and composer phpstan worked without errors. composer fix-cs made minor changes to rules/Defluent/NodeAnalyzer/FluentChainMethodCallNodeAnalyzer.php

The bin/rector process app/SomeFile.php looks like a bug.
Could you share what exact commit do you use?

I started over and when executing composer install did not get the composer glitch that I got the first time, and no longer have this config.php was not found error. Now I am in business! :)

I then checked the TypedPropertyRector rule which was previously not showing results when using the composer installed rector and it now shows results (Not at first because it only applied RemoveUnusedPrivatePropertyRector, but after I made the script more compliant, produced the desired results).

[michael@devserver rector-src]$ bin/rector process app/SomeClass.php --dry-run
 1/1 [▓▓▓▓▓▓▓▓▓▓▓▓▓▓▓▓▓▓▓▓▓▓▓▓▓▓▓▓] 100%
1 file with changes
===================

1) app/SomeClass.php

    ---------- begin diff ----------
--- Original
+++ New
@@ @@
 <?php
 final class SomeClass
 {
-    /**
-     * @var int
-     */
-    private $count;
+    private int $count;

     public function __construct()
     {
    ----------- end diff -----------

Applied rules:
 * TypedPropertyRector (https://wiki.php.net/rfc/typed_properties_v2#proposal)

 [OK] 1 file would have changed (dry-run) by Rector

So, now I look whether there are existing rules in rules-tests/Php74/Rector/Property/TypedPropertyRector/Fixture, and it looks like property.php.inc. Not a big deal but why is the class name ClassWithProperty in property.php.inc and not just Property and where is the by convention first fixture fixture.php.inc? Regardless, I run phpunit and no errors. Just to make sure how it works, I created my own michael_test to mimic my class and it passes and then changed it to fail and saw how it displays the error.

But then I go to my composer installed version and run the test, and it applies the rule but doesn't display the diff. Where do I go from here?

[michael@devserver symfony2]$ vendor/bin/rector process src/RectorTesting/SomeClass.php --dry-run --debug
 0/1 [░░░░░░░░░░░░░░░░░░░░░░░░░░░░]   0% < 1 sec/< 1 sec 46.0 MiB[parsing] src/RectorTesting/SomeClass.php
[refactoring] src/RectorTesting/SomeClass.php
    [applying] Rector\Php74\Rector\Property\TypedPropertyRector
[post rectors] src/RectorTesting/SomeClass.php
[print] src/RectorTesting/SomeClass.php


 [OK] Rector is done!


[michael@devserver symfony2]$ cat src/RectorTesting/SomeClass.php
<?php
final class SomeClass
{
    /**
     * @var int
     */
    private $count;

    public function __construct()
    {
        $this->count = 123;
    }

    public function getCount():int
    {
        return $this->count;
    }
}

[michael@devserver symfony2]$ cat rector.php
<?php
use Rector\Php74\Rector\Property\TypedPropertyRector;
use Symfony\Component\DependencyInjection\Loader\Configurator\ContainerConfigurator;

return static function (ContainerConfigurator $containerConfigurator): void {
    $services = $containerConfigurator->services();

    $services->set(TypedPropertyRector::class)
        ->call('configure', [[
            TypedPropertyRector::CLASS_LIKE_TYPE_ONLY => false,
        ]]);
};
[michael@devserver symfony2]$

I next checked the AnnotationToAttributeRector rule which also previously didn't show results when using the composer installed rector. But still it produces no results and I don't even see AnnotationToAttributeRector when I run bin/rector process. Do I need to change bin/rector.php? Note that it does work at https://getrector.org/demo/1ebdf12f-7d6b-6e66-8acd-f12c34e3b427.

[michael@devserver rector-src]$ bin/rector process app/SymfonyRoute.php --dry-run --debug
 0/1 [░░░░░░░░░░░░░░░░░░░░░░░░░░░░]   0% < 1 sec/< 1 sec 60.5 MiB[parsing] app/SymfonyRoute.php
[refactoring] app/SymfonyRoute.php
    [applying] Rector\DeadCode\Rector\Node\RemoveNonExistingVarAnnotationRector
    [applying] Rector\CodingStyle\Rector\Use_\RemoveUnusedAliasRector
    [applying] Rector\CodingStyle\Rector\Use_\SeparateMultiUseImportsRector
    [applying] Rector\DeadCode\Rector\Stmt\RemoveUnreachableStatementRector
    [applying] Rector\CodeQuality\Rector\Name\FixClassCaseSensitivityNameRector
    [applying] Rector\CodingStyle\Rector\Class_\AddArrayDefaultToArrayPropertyRector
    [applying] Rector\CodeQuality\Rector\Class_\CompleteDynamicPropertiesRector
    [applying] Rector\Privatization\Rector\Class_\FinalizeClassesWithoutChildrenRector
    [applying] Rector\Privatization\Rector\Class_\ChangeReadOnlyVariableWithDefaultValueToConstantRector
    [applying] Rector\Privatization\Rector\Class_\RepeatedLiteralToClassConstantRector
    [applying] Rector\Naming\Rector\Class_\RenamePropertyToMatchTypeRector
    [applying] Rector\Php80\Rector\Class_\StringableForToStringRector
    [applying] Rector\Php80\Rector\Class_\ClassPropertyAssignToConstructorPromotionRector
    [applying] Rector\PHPUnit\Rector\Class_\ConstructClassMethodToSetUpTestCaseRector
    [applying] Rector\CodingStyle\Rector\ClassMethod\NewlineBeforeNewAssignSetRector
    [applying] Rector\CodingStyle\Rector\ClassMethod\MakeInheritedMethodVisibilitySameAsParentRector
    [applying] Rector\CodingStyle\Rector\ClassMethod\RemoveDoubleUnderscoreInMethodNameRector
    [applying] Rector\CodingStyle\Rector\ClassMethod\UnSpreadOperatorRector
    [applying] Rector\CodeQuality\Rector\FunctionLike\RemoveAlwaysTrueConditionSetInConstructorRector
    [applying] Rector\CodeQuality\Rector\ClassMethod\DateTimeToDateTimeInterfaceRector
    [applying] Rector\CodeQuality\Rector\ClassMethod\NarrowUnionTypeDocRector
    [applying] Rector\CodingStyle\Rector\ClassMethod\FuncGetArgsToVariadicParamRector
    [applying] Rector\DeadCode\Rector\ClassMethod\RemoveEmptyClassMethodRector
    [applying] Rector\DeadCode\Rector\FunctionLike\RemoveOverriddenValuesRector
    [applying] Rector\DeadCode\Rector\ClassMethod\RemoveUnusedPrivateMethodRector
    [applying] Rector\DeadCode\Rector\FunctionLike\RemoveCodeAfterReturnRector
    [applying] Rector\DeadCode\Rector\ClassMethod\RemoveDeadConstructorRector
    [applying] Rector\DeadCode\Rector\FunctionLike\RemoveDeadReturnRector
    [applying] Rector\DeadCode\Rector\ClassMethod\RemoveDelegatingParentCallRector
    [applying] Rector\PHPUnit\Rector\ClassMethod\RemoveEmptyTestMethodRector
    [applying] Rector\DeadCode\Rector\FunctionLike\RemoveDuplicatedIfReturnRector
    [applying] Rector\DeadCode\Rector\ClassMethod\RemoveUnusedConstructorParamRector
    [applying] Rector\DeadCode\Rector\ClassMethod\RemoveUnusedPrivateMethodParameterRector
    [applying] Rector\DeadCode\Rector\ClassMethod\RemoveUselessParamTagRector
    [applying] Rector\DeadCode\Rector\ClassMethod\RemoveUselessReturnTagRector
    [applying] Rector\DeadCode\Rector\ClassMethod\RemoveUnusedPromotedPropertyRector
    [applying] Rector\Privatization\Rector\ClassMethod\ChangeGlobalVariablesToPropertiesRector
    [applying] Rector\Privatization\Rector\ClassMethod\PrivatizeFinalClassMethodRector
    [applying] Rector\Naming\Rector\ClassMethod\RenameParamToMatchTypeRector
    [applying] Rector\Naming\Rector\ClassMethod\RenameVariableToMatchNewTypeRector
    [applying] Rector\TypeDeclaration\Rector\FunctionLike\ParamTypeDeclarationRector
    [applying] Rector\TypeDeclaration\Rector\FunctionLike\ReturnTypeDeclarationRector
    [applying] Rector\TypeDeclaration\Rector\ClassMethod\AddArrayParamDocTypeRector
    [applying] Rector\TypeDeclaration\Rector\ClassMethod\AddArrayReturnDocTypeRector
    [applying] Rector\Php80\Rector\FunctionLike\UnionTypesRector
    [applying] Rector\Arguments\Rector\ClassMethod\ArgumentAdderRector
    [applying] Rector\Php80\Rector\ClassMethod\SetStateToStaticRector
    [applying] Rector\Php80\Rector\ClassMethod\FinalPrivateToPrivateVisibilityRector
    [applying] Rector\Php80\Rector\ClassMethod\OptionalParametersAfterRequiredRector
    [applying] Rector\TypeDeclaration\Rector\ClassMethod\ReturnTypeFromStrictTypedPropertyRector
    [applying] Rector\TypeDeclaration\Rector\ClassMethod\ReturnTypeFromStrictTypedCallRector
    [applying] Rector\TypeDeclaration\Rector\ClassMethod\AddVoidReturnTypeWhereNoReturnRector
    [applying] Rector\TypeDeclaration\Rector\ClassMethod\ReturnTypeFromReturnNewRector
    [applying] Rector\CodingStyle\Rector\ClassMethod\ReturnArrayClassMethodToYieldRector
    [applying] Rector\Restoration\Rector\ClassMethod\InferParamFromClassMethodReturnRector
    [applying] Rector\CodeQuality\Rector\Return_\SimplifyUselessVariableRector
    [applying] Rector\DeadCode\Rector\Return_\RemoveDeadConditionAboveReturnRector
    [applying] Rector\EarlyReturn\Rector\Return_\ReturnBinaryAndToEarlyReturnRector
    [applying] Rector\EarlyReturn\Rector\Return_\PreparedValueToEarlyReturnRector
    [applying] Rector\EarlyReturn\Rector\Return_\ReturnBinaryOrToEarlyReturnRector
    [applying] Rector\Php74\Rector\LNumber\AddLiteralSeparatorToNumberRector
    [applying] Rector\Nette\Rector\LNumber\ReplaceTimeNumberWithDateTimeConstantRector
[post rectors] app/SymfonyRoute.php
[print] app/SymfonyRoute.php


 [OK] Rector is done!

 [michael@devserver rector-src]$


[michael@devserver rector-src]$ cat app/SymfonyRoute.php
<?php

 use Symfony\Component\Routing\Annotation\Route;

 final class SymfonyRoute
 {
    /**
     * @Route("/path", name="action")
     */
     public function action():int
     {
         return 123;    //Used to prevent rule RemoveEmptyClassMethodRector
     }
 }

Even through I didn't get a diff, I knew the rule in question and verified that an applicable rule existed and then used phpunit to test and all passed.

[michael@devserver rector-src]$ vendor/bin/phpunit rules-tests/Php80/Rector/Class_/AnnotationToAttributeRector/AnnotationToAttributeRectorTest.php
PHPUnit 9.5.6 by Sebastian Bergmann and contributors.

............ 12 / 12 (100%)

Time: 00:00.362, Memory: 50.00 MB

OK (12 tests, 24 assertions)
[michael@devserver rector-src]$

What is the next step if no diff is outputted?

[TomasVotruba]

Wow, you're on fire :)

Running Rector on your code can be helpful, but it's just for trying things out. In the end we always add a test fixtures first. When running Rector, there might dozens of intervening variables, so we always add a test fixture to be sure it's the rule and nothing else.

Next step would be adding a test fixture in /Fixture/... directory. The smaller the better and then send PR. I can work with than then and we can try to break it together :)

[NotionCommotion]

Wow, you're on fire :)

Just excited about removing all legacy PHP from my code!

Running Rector on your code can be helpful, but it's just for trying things out. In the end we always add a test fixtures first. When running Rector, there might dozens of intervening variables, so we always add a test fixture to be sure it's the rule and nothing else.

Next step would be adding a test fixture in /Fixture/... directory. The smaller the better and then send PR. I can work with than then and we can try to break it together :)

I am still missing something.

For my application, I use the composer installed rector and use vendor/bin/rector process to fix legacy code.

Then, if rector doesn't do something correct, I identify the misbehaving rule, use rector-src to create a test fixture that identifies where there are issues, and push it to the repository and create a pull request.

But the two issues I have are:

1. On my app, a diff isn't returned even though a diff is returned on my local rector-src version (this is the case with TypedPropertyRector).
2. On my local rector-src version, a diff isn't returned even though a diff is returned on https://getrector.org/demo/1ebdf12f-7d6b-6e66-8acd-f12c34e3b427 (this is the case with AnnotationToAttributeRector).

I did what I thought you said I should do, however, think I am not doing it correctly. Note that my rule I created is pretty useless as it is already addressed by property.php.inc, and also will show an error because I forgot to test it and inappropriately included a comment line in it.

Appreciate the help!

gh auth login

# fork https://github.com/rectorphp/rector-src
gh repo fork https://github.com/rectorphp/rector-src

# clone it locally
# Done with above command

cd rector-src

# install dependencies
composer install

# Test install
composer fix-cs
composer phpstan

# create new branch for your test
git checkout -b add_fixture

# Detect the Minimal File
mkdir app
cat << 'EOT' > app/SomeClass.php
<?php
final class SomeClass
{
    /**
     * @var int
     */
    private $count;

    public function __construct()
    {
        $this->count = 123;
    }

    public function getCount():int
    {
        return $this->count;
    }
}
EOT

bin/rector process app/SomeClass.php
# returns TypedPropertyRector

# add test
cat << 'EOT' > rules-tests/Php74/Rector/Property/TypedPropertyRector/Fixture/michael_test.php.inc
<?php
// rules-tests/Php74/Rector/Property/TypedPropertyRector/Fixture/michael_test.php.inc

namespace Rector\Tests\Php74\Rector\Property\TypedPropertyRector\Fixture;

final class MichaelTest
{
    /**
     * @var int
     */
    private $count;
    
    public function __construct()
    {
        $this->count = 123;
    }

    public function getCount():int
    {
        return $this->count;
    }
}

?>
-----
<?php

namespace Rector\Tests\Php74\Rector\Property\TypedPropertyRector\Fixture;

final class MichaelTest
{
    private int $count;
    
    public function __construct()
    {
        $this->count = 123;
    }

    public function getCount():int
    {
        return $this->count;
    }
}

?>
EOT

# Test the rule
vendor/bin/phpunit rules-tests/Php74/Rector/Property/TypedPropertyRector/TypedPropertyRectorTest.php --debug

git rev-parse HEAD
fb9ad83c32770022a3816ee06c88d92cd84dc322

git add -A
git commit -m'Adding MichaelTest TypedPropertyRector fixture which is redundent'

git rev-parse HEAD
ab3dad0e886994952ecf74771a050e54f0236613

# push the branch
git push -u origin add_fixture

# create new pull request to https://github.com/rectorphp/rector-src
gh pr create --title "Adding a fixture" --body "Is this correct?"

[NotionCommotion]

Appreciate the help Tomas,

I feel I now have a good understanding how to add a failing test case (granted the only reason the one I added failed was because it had an inappropriate comment in the <code before>).

My problem still exists, however, and it is not that the rule is modifying the PHP script incorrectly but is just is not being applied consistently. Not positive, but think it might be related to php-stan.

Since the stated issue has been resolved (i.e. how to add a failing test case), would it be appropriate for me to close this issue and open another one which is more on topic (i.e. rector doesn't apply rule during refactoring)?

[TomasVotruba]

Thanks for adding a test fixture. It seems the problem in invalid detectoin of rule of file. In most cases it is :) that's why this step is so important, as it saves us lot of energy of false positive fixes. Test is golden.

There is too much text and code in here, but do you have a reproducible repository with the bug and failing Github Action? That can help if rule/fixture is incorrectly detected. I would try to explain how you can figure it out.

[NotionCommotion]

Hi Tomas,

I totally understand why the first step is to create a test fixture as otherwise you would spend all your time investigating false positives.

I created two github repositories:

• https://github.com/NotionCommotion/rector-src. This uses the cloned source, and mostly works as expected except for one exception where some of the doctrine annotation are not converted to PHP attributes, but this is a separate issue and I will later create a failing test case. AnnotationToAttributeRector previously wasn't working but now does!
• https://github.com/NotionCommotion/rector-symfony. This uses rectorphp installed by composer, and unlike the source version, it doesn't produce a diff for TypedPropertyRector and AnnotationToAttributeRector.

I've never used Github Actions before but tried creating one at each repository using the file github/workflows/michael-test.yaml (composer installed version shown below and the source version is similar but uses a different path bin/rector instead of vendor/bin/rector).

name: Testing if rector rules are being applied
on: [push]
jobs:
  Test-Rector-Rules:
    runs-on: ubuntu-latest
    steps:
      - run: vendor/bin/rector process src/rector-testing --dry-run --debug
      - run: vendor/bin/rector process src/rector-testing/SomeFile.php --dry-run --debug --config rector_FinalizeClassesWithoutChildrenRector.php
      - run: vendor/bin/rector process src/rector-testing/SomeClass.php --dry-run --debug --config rector_TypedPropertyRector.php
      - run: vendor/bin/rector process src/rector-testing/SymfonyRoute.php --dry-run --debug --config rector_AnnotationToAttributeRector.php
      - run: vendor/bin/rector process src/rector-testing/SomeAnnotation.php --dry-run --debug --config rector_DoctrineAnnotationClassToAttributeRector.php
      - run: vendor/bin/rector process src/rector-testing/SomeAnnotation2.php --dry-run --debug --config rector_DoctrineAnnotationClassToAttributeRector.php
      - run: vendor/bin/phpstan analyse src/rector-testing/*

I wasn't surprised it failed for the following reasons:

• PHP is not running within the github action workflow. Not positive, but looks like you figured this out with your code_analysis.yaml.
• Composer dependencies might not be available. Might need to create a docker?
• Different operating system. Linux version for Github Actions is ubuntu-latest, however, my server is centos7 (don't expect this to be an issue).

Seems like a lot of work, and considering that most functionally works using the source version, it is just a matter of time before you publish it to packagelist and in the mean time I can use a local composer repository which contains the source code. How difficult is it to use Github Actions to reproduce the event?

[TomasVotruba]

Hi,

thanks for making the repositories. We'll need a repository that is based on your real usage with composer. The rector/rector must be a depenency there, not a fork.

Could you update those?

[NotionCommotion]

Thanks Tomas, The version where rector is installed using composer, right? Like it is now? Thanks

https://github.com/NotionCommotion/rector-symfony

[TomasVotruba]

Thanks! We're getting there.

Could you update rector.php to contain only minimal configuratoin that causes the error?

As a bonus, removing /vendor and composer.lock would clean up downloaded code, e.g. by putting them into .gitignore