Skip to content

Commit

Permalink
Adding option to warn if a file is unexpectedly missing
Browse files Browse the repository at this point in the history
Plus fixes, phpcs, etc
  • Loading branch information
weaverryan committed May 22, 2023
1 parent cd380c0 commit 1d1e0f9
Show file tree
Hide file tree
Showing 5 changed files with 57 additions and 44 deletions.
4 changes: 2 additions & 2 deletions src/Configurator/AbstractConfigurator.php
Original file line number Diff line number Diff line change
Expand Up @@ -43,15 +43,15 @@ abstract public function unconfigure(Recipe $recipe, $config, Lock $lock);

abstract public function update(RecipeUpdate $recipeUpdate, array $originalConfig, array $newConfig): void;

protected function write($messages)
protected function write($messages, $verbosity = IOInterface::VERBOSE)
{
if (!\is_array($messages)) {
$messages = [$messages];
}
foreach ($messages as $i => $message) {
$messages[$i] = ' '.$message;
}
$this->io->writeError($messages, true, IOInterface::VERBOSE);
$this->io->writeError($messages, true, $verbosity);
}

protected function isFileMarked(Recipe $recipe, string $file): bool
Expand Down
13 changes: 9 additions & 4 deletions src/Configurator/AddLinesConfigurator.php
Original file line number Diff line number Diff line change
Expand Up @@ -2,6 +2,7 @@

namespace Symfony\Flex\Configurator;

use Composer\IO\IOInterface;
use Symfony\Flex\Lock;
use Symfony\Flex\Recipe;
use Symfony\Flex\Update\RecipeUpdate;
Expand Down Expand Up @@ -35,7 +36,11 @@ public function configure(Recipe $recipe, $config, Lock $lock, array $options =

$file = $this->path->concatenate([$this->options->get('root-dir'), $patch['file']]);
if (!is_file($file)) {
$this->write(sprintf('Skipping patch on file "%s": it does not exist.', $patch['file']));
$this->write(
sprintf('Skipping update of file "%s": it does not exist.', $patch['file']),
isset($patch['warn_if_missing']) && $patch['warn_if_missing'] ? IOInterface::NORMAL : IOInterface::VERBOSE
);

continue;
}

Expand All @@ -50,7 +55,7 @@ public function configure(Recipe $recipe, $config, Lock $lock, array $options =
throw new \InvalidArgumentException(sprintf('The "position" key is required for the "add-lines" configurator for recipe "%s".', $recipe->getName()));
}
$position = $patch['position'];
if (!in_array($position, self::VALID_POSITIONS, true)) {
if (!\in_array($position, self::VALID_POSITIONS, true)) {
throw new \InvalidArgumentException(sprintf('The "position" key must be one of "%s" for the "add-lines" configurator for recipe "%s".', implode('", "', self::VALID_POSITIONS), $recipe->getName()));
}

Expand Down Expand Up @@ -96,7 +101,7 @@ public function update(RecipeUpdate $recipeUpdate, array $originalConfig, array
return !isset($item['requires']) || $this->isPackageInstalled($item['requires']);
});

$filterDuplicates = function(array $sourceConfig, array $comparisonConfig) {
$filterDuplicates = function (array $sourceConfig, array $comparisonConfig) {
$filtered = [];
foreach ($sourceConfig as $sourceItem) {
$found = false;
Expand Down Expand Up @@ -177,7 +182,7 @@ private function unPatchFile(string $file, $value)
}

$position = strpos($fileContents, $value);
$fileContents = substr_replace($fileContents, '', $position, strlen($value));
$fileContents = substr_replace($fileContents, '', $position, \strlen($value));

file_put_contents($file, $fileContents);
}
Expand Down
12 changes: 8 additions & 4 deletions src/PackageJsonSynchronizer.php
Original file line number Diff line number Diff line change
Expand Up @@ -126,7 +126,7 @@ private function resolvePackageJsonDependencies($phpPackage): array
}

if ($packageJson->read()['symfony']['needsPackageAsADependency'] ?? true) {
$dependencies['@' . $phpPackage['name']] = 'file:' . substr($packageJson->getPath(), 1 + \strlen($this->rootDir), -13);
$dependencies['@'.$phpPackage['name']] = 'file:'.substr($packageJson->getPath(), 1 + \strlen($this->rootDir), -13);
}

foreach ($packageJson->read()['peerDependencies'] ?? [] as $peerDependency => $constraint) {
Expand All @@ -147,7 +147,7 @@ private function resolveImportMapPackages($phpPackage): array
foreach ($packageJson->read()['symfony']['importmap'] ?? [] as $dependency => $constraint) {
if (0 === strpos($constraint, 'path:')) {
$dependencies[$dependency] = [
'path' => dirname($packageJson->getPath()).'/'.substr($constraint, 5)
'path' => \dirname($packageJson->getPath()).'/'.substr($constraint, 5),
];

continue;
Expand Down Expand Up @@ -226,7 +226,7 @@ private function updateImportMap(array $importMapEntries): void
return;
}

$importMapData = include($this->rootDir.'/importmap.php');
$importMapData = include $this->rootDir.'/importmap.php';

$remotePackageNameAndConstraints = [];
foreach ($importMapEntries as $name => $importMapEntry) {
Expand All @@ -250,7 +250,11 @@ private function updateImportMap(array $importMapEntries): void
continue;
}

throw new \InvalidArgumentException(sprintf('Invalid importmap entry: %s', var_export($importMapEntry, true)));
throw new \InvalidArgumentException(sprintf('Invalid importmap entry: "%s".', var_export($importMapEntry, true)));
}

if (!$remotePackageNameAndConstraints) {
return;
}

$this->scriptExecutor->execute(
Expand Down
2 changes: 1 addition & 1 deletion src/ScriptExecutor.php
Original file line number Diff line number Diff line change
Expand Up @@ -117,7 +117,7 @@ private function expandPhpScript(string $cmd, array $scriptArguments): string

$arguments = $phpFinder->findArguments();

if ($env = (string) (getenv('COMPOSER_ORIGINAL_INIS'))) {
if ($env = (string) getenv('COMPOSER_ORIGINAL_INIS')) {
$paths = explode(\PATH_SEPARATOR, $env);
$ini = array_shift($paths);
} else {
Expand Down
70 changes: 37 additions & 33 deletions tests/Configurator/AddLinesConfiguratorTest.php
Original file line number Diff line number Diff line change
Expand Up @@ -32,11 +32,10 @@ protected function setUp(): void
$filesystem->remove(FLEX_TEST_DIR);
}


public function testFileDoesNotExistSkipped()
{
$this->runConfigure([
['file' => 'non-existent.php']
['file' => 'non-existent.php'],
]);
$this->assertFileDoesNotExist(FLEX_TEST_DIR.'/non-existent.php');
}
Expand All @@ -54,7 +53,7 @@ public function testLinesAddedToTopOfFile()
[
'file' => 'assets/app.js',
'position' => 'top',
'content' => "import './bootstrap';"
'content' => "import './bootstrap';",
],
]);
$actualContents = $this->readFile('assets/app.js');
Expand All @@ -63,7 +62,8 @@ public function testLinesAddedToTopOfFile()
import * as Turbo from '@hotwired/turbo';
console.log(Turbo);
EOF,
EOF
,
$actualContents);
}

Expand All @@ -80,7 +80,7 @@ public function testLinesAddedToBottomOfFile()
[
'file' => 'assets/app.js',
'position' => 'bottom',
'content' => "import './bootstrap';"
'content' => "import './bootstrap';",
],
]);
$actualContents = $this->readFile('assets/app.js');
Expand All @@ -89,7 +89,8 @@ public function testLinesAddedToBottomOfFile()
console.log(Turbo);
import './bootstrap';
EOF,
EOF
,
$actualContents);
}

Expand Down Expand Up @@ -122,7 +123,6 @@ public function testLinesAddedAfterTarget()
// enables the Symfony UX Stimulus bridge (used in assets/bootstrap.js)
.enableStimulusBridge('./assets/controllers.json')
EOF

],
]);

Expand All @@ -144,7 +144,8 @@ public function testLinesAddedAfterTarget()
;
module.exports = Encore.getWebpackConfig();
EOF,
EOF
,
$actualContents);
}

Expand All @@ -171,7 +172,6 @@ public function testSkippedIfTargetCannotBeFound()
// some new line
EOF

],
]);

Expand All @@ -193,7 +193,7 @@ public function testPatchIgnoredIfValueAlreadyExists()
[
'file' => 'assets/app.js',
'position' => 'top',
'content' => "import './bootstrap';"
'content' => "import './bootstrap';",
],
]);
$actualContents = $this->readFile('assets/app.js');
Expand All @@ -212,30 +212,31 @@ public function testLinesAddedToMultipleFiles()
EOF
);


$this->runConfigure([
[
'file' => 'assets/app.js',
'position' => 'top',
'content' => "import './bootstrap';"
'content' => "import './bootstrap';",
],
[
'file' => 'assets/bootstrap.js',
'position' => 'bottom',
'content' => "console.log('on the bottom');"
'content' => "console.log('on the bottom');",
],
]);

$this->assertSame(<<<EOF
import './bootstrap';
import * as Turbo from '@hotwired/turbo';
EOF,
EOF
,
$this->readFile('assets/app.js'));

$this->assertSame(<<<EOF
console.log('bootstrap.js');
console.log('on the bottom');
EOF,
EOF
,
$this->readFile('assets/bootstrap.js'));
}

Expand All @@ -248,20 +249,22 @@ public function testLineSkippedIfRequiredPackageMissing()
EOF
);

$composer = $this->createComposerMockWithPackagesInstalled([]);
$this->runConfigure([
[
'file' => 'assets/app.js',
'position' => 'top',
'content' => "import './bootstrap';",
'requires' => 'symfony/invented-package',
],
]);
], $composer);
$actualContents = $this->readFile('assets/app.js');
$this->assertSame(<<<EOF
import * as Turbo from '@hotwired/turbo';
console.log(Turbo);
EOF,
EOF
,
$actualContents);
}

Expand Down Expand Up @@ -293,7 +296,8 @@ public function testLineProcessedIfRequiredPackageIsPresent()
import * as Turbo from '@hotwired/turbo';
console.log(Turbo);
EOF,
EOF
,
$actualContents);
}

Expand Down Expand Up @@ -356,7 +360,7 @@ public function getUnconfigureTests()
console.log(Turbo);
EOF
,
"console.log(Turbo);",
'console.log(Turbo);',
<<<EOF
import * as Turbo from '@hotwired/turbo';
import './bootstrap';
Expand Down Expand Up @@ -390,7 +394,7 @@ public function getUnconfigureTests()
console.log(Turbo);
EOF
,
"console.log(Turbo);",
'console.log(Turbo);',
<<<EOF
import * as Turbo from '@hotwired/turbo';
import './bootstrap';
Expand Down Expand Up @@ -439,14 +443,13 @@ public function getUpdateTests()
console.log('on the bottom');
EOF;


yield 'recipe_changes_patch_contents' => [
['assets/app.js' => $appJs],
[
['file' => 'assets/app.js', 'position' => 'top', 'content' => "import './bootstrap';"]
['file' => 'assets/app.js', 'position' => 'top', 'content' => "import './bootstrap';"],
],
[
['file' => 'assets/app.js', 'position' => 'top', 'content' => "import './stimulus_bootstrap';"]
['file' => 'assets/app.js', 'position' => 'top', 'content' => "import './stimulus_bootstrap';"],
],
['assets/app.js' => <<<EOF
import './stimulus_bootstrap';
Expand All @@ -460,28 +463,29 @@ public function getUpdateTests()
yield 'recipe_file_and_value_same_before_and_after' => [
['assets/app.js' => $appJs],
[
['file' => 'assets/app.js', 'position' => 'top', 'content' => "import * as Turbo from '@hotwired/turbo';"]
['file' => 'assets/app.js', 'position' => 'top', 'content' => "import * as Turbo from '@hotwired/turbo';"],
],
[
['file' => 'assets/app.js', 'position' => 'top', 'content' => "import * as Turbo from '@hotwired/turbo';"]
['file' => 'assets/app.js', 'position' => 'top', 'content' => "import * as Turbo from '@hotwired/turbo';"],
],
['assets/app.js' => $appJs],
];

yield 'different_files_unconfigures_old_and_configures_new' => [
['assets/app.js' => $appJs, 'assets/bootstrap.js' => $bootstrapJs],
[
['file' => 'assets/app.js', 'position' => 'top', 'content' => "import * as Turbo from '@hotwired/turbo';"]
['file' => 'assets/app.js', 'position' => 'top', 'content' => "import * as Turbo from '@hotwired/turbo';"],
],
[
['file' => 'assets/bootstrap.js', 'position' => 'top', 'content' => "import * as Turbo from '@hotwired/turbo';"]
['file' => 'assets/bootstrap.js', 'position' => 'top', 'content' => "import * as Turbo from '@hotwired/turbo';"],
],
[
'assets/app.js' => <<<EOF
import './bootstrap';
console.log(Turbo);
EOF,
EOF
,
'assets/bootstrap.js' => <<<EOF
import * as Turbo from '@hotwired/turbo';
console.log('bootstrap.js');
Expand All @@ -494,21 +498,21 @@ public function getUpdateTests()
yield 'recipe_changes_but_ignored_because_package_not_installed' => [
['assets/app.js' => $appJs],
[
['file' => 'assets/app.js', 'position' => 'top', 'content' => "import './bootstrap';", 'requires' => 'symfony/not-installed']
['file' => 'assets/app.js', 'position' => 'top', 'content' => "import './bootstrap';", 'requires' => 'symfony/not-installed'],
],
[
['file' => 'assets/app.js', 'position' => 'top', 'content' => "import './stimulus_bootstrap';", 'requires' => 'symfony/not-installed']
['file' => 'assets/app.js', 'position' => 'top', 'content' => "import './stimulus_bootstrap';", 'requires' => 'symfony/not-installed'],
],
['assets/app.js' => $appJs],
];

yield 'recipe_changes_are_applied_if_required_package_installed' => [
['assets/app.js' => $appJs],
[
['file' => 'assets/app.js', 'position' => 'top', 'content' => "import './bootstrap';", 'requires' => 'symfony/installed-package']
['file' => 'assets/app.js', 'position' => 'top', 'content' => "import './bootstrap';", 'requires' => 'symfony/installed-package'],
],
[
['file' => 'assets/app.js', 'position' => 'top', 'content' => "import './stimulus_bootstrap';", 'requires' => 'symfony/installed-package']
['file' => 'assets/app.js', 'position' => 'top', 'content' => "import './stimulus_bootstrap';", 'requires' => 'symfony/installed-package'],
],
['assets/app.js' => <<<EOF
import './stimulus_bootstrap';
Expand Down Expand Up @@ -541,7 +545,7 @@ private function runUnconfigure(array $config)
private function createConfigurator(Composer $composer = null)
{
return new AddLinesConfigurator(
$composer ? $composer : $this->getMockBuilder(Composer::class)->getMock(),
$composer ?: $this->getMockBuilder(Composer::class)->getMock(),
$this->getMockBuilder(IOInterface::class)->getMock(),
new Options(['config-dir' => 'config', 'root-dir' => FLEX_TEST_DIR])
);
Expand Down

0 comments on commit 1d1e0f9

Please sign in to comment.