From 931c28a26d98f62f103240b4b3ca3da1df29ff16 Mon Sep 17 00:00:00 2001 From: Jesse Leite Date: Fri, 10 May 2024 15:29:27 -0400 Subject: [PATCH 1/4] Track warnings in `NullConsole` as well. --- src/Console/NullConsole.php | 24 ++++++++++++++++++++++++ 1 file changed, 24 insertions(+) diff --git a/src/Console/NullConsole.php b/src/Console/NullConsole.php index 9bb9cdd6ac..3fe63504c0 100644 --- a/src/Console/NullConsole.php +++ b/src/Console/NullConsole.php @@ -5,6 +5,7 @@ class NullConsole { protected $errors = []; + protected $warnings = []; /** * Store error output. @@ -29,6 +30,29 @@ public function getErrors() return collect($this->errors); } + /** + * Store warning output. + * + * @param string $warning + * @return $this + */ + public function warn($warning) + { + $this->warnings[] = $warning; + + return $this; + } + + /** + * Get warning output. + * + * @return \Illuminate\Support\Collection + */ + public function getWarnings() + { + return collect($this->warnings); + } + /** * Any calls to this class just return itself so you can chain forever and ever. * From 24bc830adc8ba677ab86032db8a9ba11a14b012d Mon Sep 17 00:00:00 2001 From: Jesse Leite Date: Fri, 10 May 2024 15:30:01 -0400 Subject: [PATCH 2/4] Return `$script` so we can inspect `$script->console()` in our tests. --- tests/UpdateScripts/Concerns/RunsUpdateScripts.php | 2 ++ 1 file changed, 2 insertions(+) diff --git a/tests/UpdateScripts/Concerns/RunsUpdateScripts.php b/tests/UpdateScripts/Concerns/RunsUpdateScripts.php index 8e4a994749..86d909ec03 100644 --- a/tests/UpdateScripts/Concerns/RunsUpdateScripts.php +++ b/tests/UpdateScripts/Concerns/RunsUpdateScripts.php @@ -15,6 +15,8 @@ protected function runUpdateScript($fqcn, $package = 'statamic/cms') $script = new $fqcn($package); $script->update(); + + return $script; } protected function assertUpdateScriptRegistered($class) From 1ea204f5c2b65494e0d332e310bb6c5330378e6a Mon Sep 17 00:00:00 2001 From: Jesse Leite Date: Fri, 10 May 2024 15:31:16 -0400 Subject: [PATCH 3/4] Test that it warns when it detects custom `env()` calls. --- .../MigrateSitesConfigToYamlTest.php | 39 ++++++++++++++++++- 1 file changed, 38 insertions(+), 1 deletion(-) diff --git a/tests/UpdateScripts/MigrateSitesConfigToYamlTest.php b/tests/UpdateScripts/MigrateSitesConfigToYamlTest.php index 500139d17a..a9f7e69bed 100644 --- a/tests/UpdateScripts/MigrateSitesConfigToYamlTest.php +++ b/tests/UpdateScripts/MigrateSitesConfigToYamlTest.php @@ -224,6 +224,41 @@ public function it_can_migrate_dynamic_whitelisted_env_function_calls() ]); } + /** @test */ + public function it_warns_when_it_detects_other_non_whitelisted_env_calls() + { + File::put(config_path('statamic/sites.php'), <<<'CONFIG' + [ + 'default' => [ + 'name' => env('CUSTOM_NAME'), + 'url' => env('CUSTOM_URL'), + 'locale' => 'en_US', + ], + ], +]; +CONFIG); + + $script = $this->migrateSitesConfig(); + + $this->assertMultisiteEnabledConfigIs(false); + + $this->assertSitesYamlHas([ + 'default' => [ + 'name' => null, + 'url' => null, + 'locale' => 'en_US', + ], + ]); + + $this->assertCount(0, $script->console()->getErrors()); + $this->assertCount(1, $warnings = $script->console()->getWarnings()); + $this->assertStringContainsString('CUSTOM_NAME', $warnings->first()); + $this->assertStringContainsString('CUSTOM_URL', $warnings->first()); + } + /** @test */ public function it_can_append_multisite_config_to_bottom_if_the_nicer_str_replace_fails() { @@ -325,12 +360,14 @@ public function it_removes_text_direction_since_this_no_longer_does_anything_in_ private function migrateSitesConfig() { - $this->runUpdateScript(MigrateSitesConfigToYaml::class); + $script = $this->runUpdateScript(MigrateSitesConfigToYaml::class); $this->assertFileDoesNotExist(config_path('statamic/sites.php')); $this->assertNull(config('statamic.sites.sites')); $this->assertFileExists(resource_path('sites.yaml')); + + return $script; } private function assertMultisiteEnabledConfigIs($boolean) From 35a5808958d404e96724f75dbec74ef728d6d09e Mon Sep 17 00:00:00 2001 From: Jesse Leite Date: Fri, 10 May 2024 15:34:33 -0400 Subject: [PATCH 4/4] Warn if custom `env()` calls are detected, so user can manually handle. --- src/UpdateScripts/MigrateSitesConfigToYaml.php | 6 ++++++ 1 file changed, 6 insertions(+) diff --git a/src/UpdateScripts/MigrateSitesConfigToYaml.php b/src/UpdateScripts/MigrateSitesConfigToYaml.php index e08ee2caed..7930ca6a7e 100644 --- a/src/UpdateScripts/MigrateSitesConfigToYaml.php +++ b/src/UpdateScripts/MigrateSitesConfigToYaml.php @@ -92,6 +92,12 @@ private function replaceWhitelistedEnvFuncCalls(string $config): string // Convert `env('APP_URL')` references $config = preg_replace('/env\([\'"]APP_URL[\'"]\)/', '\'{{ config:app:url }}\'', $config); + // Warn if any other `env()` calls are detected, so that user can handle them manually + if (preg_match_all('/env\([\'"]([^\'"]+)[\'"]/', $config, $matches)) { + $this->console->warn('The following .env vars were referenced in your sites.php config: '.collect($matches[1])->implode(', ')); + $this->console->line('If you wish for these values to remain dynamic, please refactor to use `{{ config:... }}`.'); + } + return $config; }