Skip to content

Commit 4bf1568

Browse files
committed
Revert "Address code review feedback: fix duplicate config, improve regex patterns, enhance security (#65)"
This reverts commit f77c7ac.
1 parent f77c7ac commit 4bf1568

File tree

5 files changed

+79
-50
lines changed

5 files changed

+79
-50
lines changed

docs/commands.md

Lines changed: 1 addition & 1 deletion
Original file line numberDiff line numberDiff line change
@@ -205,7 +205,7 @@ bin/magento mageforge:hyva:compatibility:check [options]
205205
**Options**:
206206

207207
- `--show-all` / `-a` - Show all modules including compatible ones
208-
- `--third-party-only` / `-t` - Check only third-party modules (exclude Magento_* modules)
208+
- `--third-party-only` / `-t` - Check only third-party modules (exclude Magento\_\* modules)
209209
- `--include-vendor` - Include Magento core modules in scan (default: third-party only)
210210
- `--detailed` / `-d` - Show detailed file-level issues for incompatible modules
211211

src/Console/Command/Hyva/CompatibilityCheckCommand.php

Lines changed: 16 additions & 27 deletions
Original file line numberDiff line numberDiff line change
@@ -96,7 +96,8 @@ private function runInteractiveMode(InputInterface $input, OutputInterface $outp
9696
label: 'Select scan options',
9797
options: [
9898
'show-all' => 'Show all modules including compatible ones',
99-
'include-vendor' => 'Include vendor modules (default: excluded)',
99+
'incompatible-only' => 'Show only incompatible modules (default behavior)',
100+
'include-vendor' => 'Include Magento core modules (default: third-party only)',
100101
'detailed' => 'Show detailed file-level issues with line numbers',
101102
],
102103
default: [],
@@ -110,6 +111,7 @@ private function runInteractiveMode(InputInterface $input, OutputInterface $outp
110111

111112
// Apply selected options to input
112113
$showAll = in_array('show-all', $selectedOptions);
114+
$incompatibleOnly = in_array('incompatible-only', $selectedOptions);
113115
$includeVendor = in_array('include-vendor', $selectedOptions);
114116
$detailed = in_array('detailed', $selectedOptions);
115117
$thirdPartyOnly = false; // Not needed in interactive mode
@@ -119,11 +121,13 @@ private function runInteractiveMode(InputInterface $input, OutputInterface $outp
119121
$config = [];
120122
if ($showAll) {
121123
$config[] = 'Show all modules';
124+
} elseif ($incompatibleOnly) {
125+
$config[] = 'Show incompatible only';
122126
} else {
123127
$config[] = 'Show modules with issues';
124128
}
125129
if ($includeVendor) {
126-
$config[] = 'Include vendor modules';
130+
$config[] = 'Include Magento core';
127131
} else {
128132
$config[] = 'Third-party modules only';
129133
}
@@ -133,8 +137,8 @@ private function runInteractiveMode(InputInterface $input, OutputInterface $outp
133137
$this->io->comment('Configuration: ' . implode(', ', $config));
134138
$this->io->newLine();
135139

136-
// Run scan with selected options
137-
return $this->runScan($showAll, $thirdPartyOnly, $includeVendor, $detailed, false, $output);
140+
// Run scan with selected options (pass incompatibleOnly flag)
141+
return $this->runScan($showAll, $thirdPartyOnly, $includeVendor, $detailed, $incompatibleOnly, $output);
138142
} catch (\Exception $e) {
139143
$this->resetPromptEnvironment();
140144
$this->io->error('Interactive mode failed: ' . $e->getMessage());
@@ -172,11 +176,12 @@ private function runScan(
172176
): int {
173177

174178
// Determine filter logic for vendor and third-party modules:
175-
// - By default (no flags): scan third-party modules excluding those in vendor/
176-
// - With --include-vendor: include vendor modules in the scan
179+
// - excludeVendor controls whether to scan modules in the vendor/ directory
180+
// - By default (no flags): scan third-party modules including those in vendor/
181+
// - With --include-vendor: scan everything including Magento_* core modules
177182
// - With --third-party-only: explicitly scan only third-party modules
178183
$scanThirdPartyOnly = $thirdPartyOnly || (!$includeVendor && !$thirdPartyOnly);
179-
$excludeVendor = !$includeVendor;
184+
$excludeVendor = false; // Always include vendor for third-party scanning
180185

181186
// Run the compatibility check
182187
$results = $this->compatibilityChecker->check(
@@ -364,25 +369,9 @@ private function isInteractiveTerminal(OutputInterface $output): bool
364369
}
365370
}
366371

367-
// Additional check: detect if running in a proper TTY using safer methods
368-
if (\function_exists('stream_isatty') && \defined('STDIN')) {
369-
try {
370-
return \stream_isatty(\STDIN);
371-
} catch (\Throwable $e) {
372-
// Fall through to next check
373-
}
374-
}
375-
376-
if (\function_exists('posix_isatty') && \defined('STDIN')) {
377-
try {
378-
return \posix_isatty(\STDIN);
379-
} catch (\Throwable $e) {
380-
// Fall through to default
381-
}
382-
}
383-
384-
// Conservative default if no TTY-detection functions are available
385-
return false;
372+
// Additional check: try to detect if running in a proper TTY
373+
$sttyOutput = shell_exec('stty -g 2>/dev/null');
374+
return !empty($sttyOutput);
386375
}
387376

388377
/**
@@ -461,7 +450,7 @@ private function getServerVar(string $name): ?string
461450
private function setEnvVar(string $name, string $value): void
462451
{
463452
$this->secureEnvStorage[$name] = $value;
464-
$_SERVER[$name] = $value;
453+
putenv("$name=$value");
465454
}
466455

467456
/**

src/Service/Hyva/IncompatibilityDetector.php

Lines changed: 3 additions & 3 deletions
Original file line numberDiff line numberDiff line change
@@ -43,7 +43,7 @@ class IncompatibilityDetector
4343
'severity' => self::SEVERITY_WARNING,
4444
],
4545
[
46-
'pattern' => '/(?:define|require)\s*\(\s*\[[^\]]*["\']mage\/[^"\']*["\']\s*[^\]]*\]/',
46+
'pattern' => '/(?:define|require)\s*\(\s*\[[^\]]*["\']mage\/[^"\']*["\']/',
4747
'description' => 'Magento RequireJS module reference',
4848
'severity' => self::SEVERITY_CRITICAL,
4949
],
@@ -65,7 +65,7 @@ class IncompatibilityDetector
6565
'severity' => self::SEVERITY_CRITICAL,
6666
],
6767
[
68-
'pattern' => '/<referenceBlock\b[^>]*\bremove\s*=\s*"true"[^>]*>/s',
68+
'pattern' => '/<referenceBlock.*remove="true">/',
6969
'description' => 'Block removal (review for Hyvä compatibility)',
7070
'severity' => self::SEVERITY_WARNING,
7171
],
@@ -82,7 +82,7 @@ class IncompatibilityDetector
8282
'severity' => self::SEVERITY_CRITICAL,
8383
],
8484
[
85-
'pattern' => '/\$\([^)]*\)\s*\.(on|click|ready|change|keyup|keydown|submit|ajax|each|css|hide|show|addClass|removeClass|toggleClass|append|prepend|html|text|val|attr|prop|data|trigger|find|parent|children)\s*\(/',
85+
'pattern' => '/\$\(.*\)\..*\(/',
8686
'description' => 'jQuery DOM manipulation',
8787
'severity' => self::SEVERITY_WARNING,
8888
],

src/Service/Hyva/ModuleScanner.php

Lines changed: 26 additions & 16 deletions
Original file line numberDiff line numberDiff line change
@@ -107,7 +107,31 @@ private function findRelevantFiles(string $directory): array
107107
}
108108

109109
/**
110-
* Check if module has Hyvä compatibility package
110+
* Check if module has Hyvä compatibility package based on composer data
111+
*
112+
* @param array $composerData Parsed composer.json data
113+
*/
114+
private function isHyvaCompatibilityPackage(array $composerData): bool
115+
{
116+
// Check if this IS a Hyvä compatibility package
117+
$packageName = $composerData['name'] ?? '';
118+
if (str_starts_with($packageName, 'hyva-themes/') && str_contains($packageName, '-compat')) {
119+
return true;
120+
}
121+
122+
// Check dependencies for Hyvä packages
123+
$requires = $composerData['require'] ?? [];
124+
foreach ($requires as $package => $version) {
125+
if (str_starts_with($package, 'hyva-themes/')) {
126+
return true;
127+
}
128+
}
129+
130+
return false;
131+
}
132+
133+
/**
134+
* Check if module has Hyvä compatibility package (public wrapper)
111135
*/
112136
public function hasHyvaCompatibilityPackage(string $modulePath): bool
113137
{
@@ -125,21 +149,7 @@ public function hasHyvaCompatibilityPackage(string $modulePath): bool
125149
return false;
126150
}
127151

128-
// Check if this IS a Hyvä compatibility package
129-
$packageName = $composerData['name'] ?? '';
130-
if (str_starts_with($packageName, 'hyva-themes/') && str_contains($packageName, '-compat')) {
131-
return true;
132-
}
133-
134-
// Check dependencies for Hyvä packages
135-
$requires = $composerData['require'] ?? [];
136-
foreach ($requires as $package => $version) {
137-
if (str_starts_with($package, 'hyva-themes/')) {
138-
return true;
139-
}
140-
}
141-
142-
return false;
152+
return $this->isHyvaCompatibilityPackage($composerData);
143153
} catch (\Exception $e) {
144154
// Log error when debugging to help identify JSON or file read issues
145155
if (getenv('MAGEFORGE_DEBUG')) {

src/etc/di.xml

Lines changed: 33 additions & 3 deletions
Original file line numberDiff line numberDiff line change
@@ -3,6 +3,39 @@
33
xmlns:xsi="http://www.w3.org/2001/XMLSchema-instance"
44
xsi:noNamespaceSchemaLocation="urn:magento:framework:ObjectManager/etc/config.xsd"
55
>
6+
<type name="Magento\Framework\Console\CommandList">
7+
<arguments>
8+
<argument
9+
name="commands"
10+
xsi:type="array"
11+
>
12+
<item name="mageforge_system_version"
13+
xsi:type="object"
14+
>
15+
OpenForgeProject\MageForge\Console\Command\System\VersionCommand</item>
16+
<item name="mageforge_system_check"
17+
xsi:type="object"
18+
>
19+
OpenForgeProject\MageForge\Console\Command\System\CheckCommand</item>
20+
<item name="mageforge_theme_list"
21+
xsi:type="object"
22+
>
23+
OpenForgeProject\MageForge\Console\Command\Theme\ListCommand</item>
24+
<item name="mageforge_theme_build"
25+
xsi:type="object"
26+
>
27+
OpenForgeProject\MageForge\Console\Command\Theme\BuildCommand</item>
28+
<item name="mageforge_theme_watch"
29+
xsi:type="object"
30+
>
31+
OpenForgeProject\MageForge\Console\Command\Theme\WatchCommand</item>
32+
<item name="mageforge_hyva_compatibility_check"
33+
xsi:type="object"
34+
>
35+
OpenForgeProject\MageForge\Console\Command\Hyva\CompatibilityCheckCommand</item>
36+
</argument>
37+
</arguments>
38+
</type>
639
<type name="Magento\Framework\Console\CommandList">
740
<arguments>
841
<argument
@@ -27,9 +60,6 @@
2760
<item name="mageforge_static_clean"
2861
xsi:type="object"
2962
>OpenForgeProject\MageForge\Console\Command\Static\CleanCommand</item>
30-
<item name="mageforge_hyva_compatibility_check"
31-
xsi:type="object"
32-
>OpenForgeProject\MageForge\Console\Command\Hyva\CompatibilityCheckCommand</item>
3363
</argument>
3464
</arguments>
3565
</type>

0 commit comments

Comments
 (0)