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

[Rector] Update rector to ^0.10 #4468

Merged
merged 4 commits into from
Mar 23, 2021
Merged

Conversation

samsonasik
Copy link
Member

Checklist:

  • Securely signed commits

@samsonasik
Copy link
Member Author

samsonasik commented Mar 22, 2021

Hi @TomasVotruba , I tried to update rector/rector to ^0.10, and in this case, I got the following notice:

vendor/bin/rector process system/CLI/BaseCommand.php --dry-run

Could not process "system/CLI/BaseCommand.php" file, due to:                                                   
         "Analyze error: "Error (Call to undefined function CodeIgniter\CLI\is_cli()) thrown while looking for class    
         CodeIgniter\CLI\CLI.". Include your files in "$parameters->set(Option::AUTOLOAD_PATHS, [...]);" in "rector.php"
         config.                                                                                                        
         See https://github.com/rectorphp/rector#configuration".  

The is_cli() function is on system/Common.php. Looking at autoload paths, it should already included as __DIR__ . '/system/Test/bootstrap.php' include it:

	// Rector relies on autoload setup of your project; Composer autoload is included by default; to add more:
	$parameters->set(Option::AUTOLOAD_PATHS, [
		// autoload specific file
		__DIR__ . '/system/Test/bootstrap.php',
	]);

even if I tried manually add system/Common.php:

	// Rector relies on autoload setup of your project; Composer autoload is included by default; to add more:
	$parameters->set(Option::AUTOLOAD_PATHS, [
		// autoload specific file
		__DIR__ . '/system/Test/bootstrap.php',
		__DIR__ . '/system/Common.php',
	]);

it still got above notice, any idea to resolve it? Thank you.

Copy link
Member

@MGatner MGatner left a comment

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Looks like some work yet to be done here, but I would be glad to see us on the latest.

@TomasVotruba
Copy link

TomasVotruba commented Mar 22, 2021

I'm looking into this. I tried to isolate the code but there is too many other packages.

It seems it fails on this minimal code:

use CodeIgniter\CLI\CLI;

class SomeClass
{
	public function showHelp()
	{
		CLI::write('...');
	}
}
class CLI
{
	public static function init()
    {
       return is_cli();
    }
}

// this is called for some reason
CLI::init();

Could you create minimal reproducible repository?

@samsonasik
Copy link
Member Author

I will try

@samsonasik
Copy link
Member Author

@TomasVotruba I created a minimal repo that show that the autoload file with code die; is skipped there:

https://github.com/samsonasik/rector-issue-010-autoload

Somehow, the system/Test/bootstrap.php is not loaded/ignored, as I tried add :

die('stop here');

in top of system/Test/bootstrap.php, and it not stopped:

Screen Shot 2021-03-22 at 23 27 19

In rector 0.9, it will print : "stop here", while in rector 0.10, it just skipped.

@samsonasik
Copy link
Member Author

@TomasVotruba I created rector issue for it rectorphp/rector#5951

@samsonasik
Copy link
Member Author

@TomasVotruba I found temporary solution by add :

require_once __DIR__ . '/system/Test/bootstrap.php';

outside rector config 1eb86d8 to ensure require_once loaded.

@samsonasik samsonasik requested a review from MGatner March 23, 2021 10:25
@samsonasik
Copy link
Member Author

All green 🎉

@samsonasik samsonasik merged commit 0a2cce8 into codeigniter4:develop Mar 23, 2021
@samsonasik samsonasik deleted the rector-010 branch March 23, 2021 12:24
@TomasVotruba
Copy link

@samsonasik This should help rectorphp/rector#5964

@samsonasik
Copy link
Member Author

@TomasVotruba thank you 👍

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

Successfully merging this pull request may close these issues.

3 participants