feature #33496 Deprecated not passing dash symbol (-) to STDIN comman…

…ds (yceruto) This PR was merged into the 4.4 branch. Discussion ---------- Deprecated not passing dash symbol (-) to STDIN commands | Q | A | ------------- | --- | Branch? | 4.4 | Bug fix? | no | New feature? | no | BC breaks? | no | Deprecations? | yes | Tests pass? | yes | Fixed tickets | #33446 (comment) | License | MIT | Doc PR | - Follow-up #33446 > There's a conflict here: when no argument was provided, the command also reads from STDIN. So now, it reads from STDIN, and if there is nothing there, reads from the default template. This has been caught in php/php-src#4672 > This creates an ambiguous situation - maybe one did pipe nothing but doesn't expect the default template dir to be linted. > I'd suggest resolving the ambiguity by reading from STDIN only when explicitly asked for. Passing - as argument could the way. And we could trigger a deprecation for now. For consistency, the other 2 lint commands (`lint:yaml` and `lint:xliff`) have been touched as well. The plan for 5.0 is read from the STDIN only when `-` is given. /cc @nicolas-grekas Commits ------- 586f299 deprecated not passing dash symbol (-) to STDIN commands
symfony · Sep 8, 2019 · 4234601 · 4234601
2 parents 8f84347 + 586f299
commit 4234601
Show file tree

Hide file tree

Showing 8 changed files with 56 additions and 31 deletions.
diff --git a/UPGRADE-4.4.md b/UPGRADE-4.4.md
@@ -207,12 +207,14 @@ Translation
 -----------
 
  * Deprecated support for using `null` as the locale in `Translator`.
+ * Deprecated accepting STDIN implicitly when using the `lint:xliff` command, use `lint:xliff -` (append a dash) instead to make it explicit.
 
 TwigBridge
 ----------
 
  * Deprecated to pass `$rootDir` and `$fileLinkFormatter` as 5th and 6th argument respectively to the
    `DebugCommand::__construct()` method, swap the variables position.
+ * Deprecated accepting STDIN implicitly when using the `lint:twig` command, use `lint:twig -` (append a dash) instead to make it explicit.
 
 TwigBundle
 ----------
@@ -326,3 +328,8 @@ WebServerBundle
 ---------------
 
  * The bundle is deprecated and will be removed in 5.0.
+
+Yaml
+----
+
+* Deprecated accepting STDIN implicitly when using the `lint:yaml` command, use `lint:yaml -` (append a dash) instead to make it explicit.
diff --git a/UPGRADE-5.0.md b/UPGRADE-5.0.md
@@ -531,6 +531,7 @@ Translation
  * The `MessageSelector`, `Interval` and `PluralizationRules` classes have been removed, use `IdentityTranslator` instead
  * The `Translator::getFallbackLocales()` and `TranslationDataCollector::getFallbackLocales()` method are now internal
  * The `Translator::transChoice()` method has been removed in favor of using `Translator::trans()` with "%count%" as the parameter driving plurals
+ * Removed support for implicit STDIN usage in the `lint:xliff` command, use `lint:xliff -` (append a dash) instead to make it explicit.
 
 TwigBundle
 ----------
@@ -548,6 +549,7 @@ TwigBridge
  * removed the `$requestStack` and `$requestContext` arguments of the
    `HttpFoundationExtension`, pass a `Symfony\Component\HttpFoundation\UrlHelper`
    instance as the only argument instead
+ * Removed support for implicit STDIN usage in the `lint:twig` command, use `lint:twig -` (append a dash) instead to make it explicit.
 
 Validator
 --------
@@ -653,6 +655,7 @@ Yaml
 
  * The parser is now stricter and will throw a `ParseException` when a
    mapping is found inside a multi-line string.
+ * Removed support for implicit STDIN usage in the `lint:yaml` command, use `lint:yaml -` (append a dash) instead to make it explicit.
 
 WebProfilerBundle
 -----------------

diff --git a/src/Symfony/Bridge/Twig/CHANGELOG.md b/src/Symfony/Bridge/Twig/CHANGELOG.md
@@ -8,6 +8,7 @@ CHANGELOG
  * deprecated to pass `$rootDir` and `$fileLinkFormatter` as 5th and 6th argument respectively to the 
    `DebugCommand::__construct()` method, swap the variables position.
  * the `LintCommand` lints all the templates stored in all configured Twig paths if none argument is provided
+ * deprecated accepting STDIN implicitly when using the `lint:twig` command, use `lint:twig -` (append a dash) instead to make it explicit.
 
 4.3.0
 -----

diff --git a/src/Symfony/Bridge/Twig/Command/LintCommand.php b/src/Symfony/Bridge/Twig/Command/LintCommand.php
@@ -50,7 +50,7 @@ protected function configure()
         $this
             ->setDescription('Lints a template and outputs encountered errors')
             ->addOption('format', null, InputOption::VALUE_REQUIRED, 'The output format', 'txt')
-            ->addArgument('filename', InputArgument::IS_ARRAY)
+            ->addArgument('filename', InputArgument::IS_ARRAY, 'A file, a directory or "-" for reading from STDIN')
             ->setHelp(<<<'EOF'
 The <info>%command.name%</info> command lints a template and outputs to STDOUT
 the first encountered syntax error.
@@ -77,15 +77,15 @@ protected function execute(InputInterface $input, OutputInterface $output)
     {
         $io = new SymfonyStyle($input, $output);
         $filenames = $input->getArgument('filename');
+        $hasStdin = '-' === ($filenames[0] ?? '');
 
-        if (0 === \count($filenames)) {
-            if (0 === ftell(STDIN)) {
-                $template = '';
-                while (!feof(STDIN)) {
-                    $template .= fread(STDIN, 1024);
+        if ($hasStdin || 0 === \count($filenames)) {
+            if ($hasStdin || 0 === ftell(STDIN)) { // remove 0 === ftell(STDIN) check in 5.0
+                if (!$hasStdin) {
+                    @trigger_error('Calling to the "lint:twig" command providing pipe file content to STDIN without passing the dash symbol "-" explicitly is deprecated since Symfony 4.4.', E_USER_DEPRECATED);
                 }
 
-                return $this->display($input, $output, $io, [$this->validate($template, uniqid('sf_', true))]);
+                return $this->display($input, $output, $io, [$this->validate($this->getStdin(), uniqid('sf_', true))]);
             }
 
             $loader = $this->twig->getLoader();
@@ -107,6 +107,16 @@ protected function execute(InputInterface $input, OutputInterface $output)
         return $this->display($input, $output, $io, $filesInfo);
     }
 
+    private function getStdin(): string
+    {
+        $template = '';
+        while (!feof(STDIN)) {
+            $template .= fread(STDIN, 1024);
+        }
+
+        return $template;
+    }
+
     private function getFilesInfo(array $filenames)
     {
         $filesInfo = [];

diff --git a/src/Symfony/Component/Translation/CHANGELOG.md b/src/Symfony/Component/Translation/CHANGELOG.md
@@ -5,6 +5,7 @@ CHANGELOG
 -----
 
  * deprecated support for using `null` as the locale in `Translator`
+ * deprecated accepting STDIN implicitly when using the `lint:xliff` command, use `lint:xliff -` (append a dash) instead to make it explicit.
 
 4.3.0
 -----

diff --git a/src/Symfony/Component/Translation/Command/XliffLintCommand.php b/src/Symfony/Component/Translation/Command/XliffLintCommand.php
@@ -53,7 +53,7 @@ protected function configure()
     {
         $this
             ->setDescription('Lints a XLIFF file and outputs encountered errors')
-            ->addArgument('filename', InputArgument::IS_ARRAY, 'A file or a directory or STDIN')
+            ->addArgument('filename', InputArgument::IS_ARRAY, 'A file, a directory or "-" for reading from STDIN')
             ->addOption('format', null, InputOption::VALUE_REQUIRED, 'The output format', 'txt')
             ->setHelp(<<<EOF
 The <info>%command.name%</info> command lints a XLIFF file and outputs to STDOUT
@@ -83,13 +83,18 @@ protected function execute(InputInterface $input, OutputInterface $output)
         $filenames = (array) $input->getArgument('filename');
         $this->format = $input->getOption('format');
         $this->displayCorrectFiles = $output->isVerbose();
+        $hasStdin = '-' === ($filenames[0] ?? '');
 
-        if (0 === \count($filenames)) {
-            if (!$stdin = $this->getStdin()) {
+        if ($hasStdin || 0 === \count($filenames)) {
+            if (!$hasStdin && 0 !== ftell(STDIN)) { // remove 0 !== ftell(STDIN) check in 5.0
                 throw new RuntimeException('Please provide a filename or pipe file content to STDIN.');
             }
 
-            return $this->display($io, [$this->validate($stdin)]);
+            if (!$hasStdin) {
+                @trigger_error('Calling to the "lint:xliff" command providing pipe file content to STDIN without passing the dash symbol "-" explicitly is deprecated since Symfony 4.4.', E_USER_DEPRECATED);
+            }
+
+            return $this->display($io, [$this->validate($this->getStdin())]);
         }
 
         $filesInfo = [];
@@ -223,18 +228,14 @@ private function getFiles(string $fileOrDirectory)
         }
     }
 
-    private function getStdin(): ?string
+    private function getStdin(): string
     {
-        if (0 !== ftell(STDIN)) {
-            return null;
-        }
-
-        $inputs = '';
+        $xliff = '';
         while (!feof(STDIN)) {
-            $inputs .= fread(STDIN, 1024);
+            $xliff .= fread(STDIN, 1024);
         }
 
-        return $inputs;
+        return $xliff;
     }
 
     private function getDirectoryIterator(string $directory)

diff --git a/src/Symfony/Component/Yaml/CHANGELOG.md b/src/Symfony/Component/Yaml/CHANGELOG.md
@@ -5,6 +5,7 @@ CHANGELOG
 -----
 
  * Added support to dump `null` as `~` by using the `Yaml::DUMP_NULL_AS_TILDE` flag.
+ * deprecated accepting STDIN implicitly when using the `lint:yaml` command, use `lint:yaml -` (append a dash) instead to make it explicit.
 
 4.3.0
 -----

diff --git a/src/Symfony/Component/Yaml/Command/LintCommand.php b/src/Symfony/Component/Yaml/Command/LintCommand.php
@@ -54,7 +54,7 @@ protected function configure()
     {
         $this
             ->setDescription('Lints a file and outputs encountered errors')
-            ->addArgument('filename', InputArgument::IS_ARRAY, 'A file or a directory or STDIN')
+            ->addArgument('filename', InputArgument::IS_ARRAY, 'A file, a directory or "-" for reading from STDIN')
             ->addOption('format', null, InputOption::VALUE_REQUIRED, 'The output format', 'txt')
             ->addOption('parse-tags', null, InputOption::VALUE_NONE, 'Parse custom tags')
             ->setHelp(<<<EOF
@@ -86,13 +86,18 @@ protected function execute(InputInterface $input, OutputInterface $output)
         $this->format = $input->getOption('format');
         $this->displayCorrectFiles = $output->isVerbose();
         $flags = $input->getOption('parse-tags') ? Yaml::PARSE_CUSTOM_TAGS : 0;
+        $hasStdin = '-' === ($filenames[0] ?? '');
 
-        if (0 === \count($filenames)) {
-            if (!$stdin = $this->getStdin()) {
+        if ($hasStdin || 0 === \count($filenames)) {
+            if (!$hasStdin && 0 !== ftell(STDIN)) { // remove 0 !== ftell(STDIN) check in 5.0
                 throw new RuntimeException('Please provide a filename or pipe file content to STDIN.');
             }
 
-            return $this->display($io, [$this->validate($stdin, $flags)]);
+            if (!$hasStdin) {
+                @trigger_error('Calling to the "lint:yaml" command providing pipe file content to STDIN without passing the dash symbol "-" explicitly is deprecated since Symfony 4.4.', E_USER_DEPRECATED);
+            }
+
+            return $this->display($io, [$this->validate($this->getStdin(), $flags)]);
         }
 
         $filesInfo = [];
@@ -199,18 +204,14 @@ private function getFiles(string $fileOrDirectory)
         }
     }
 
-    private function getStdin(): ?string
+    private function getStdin(): string
     {
-        if (0 !== ftell(STDIN)) {
-            return null;
-        }
-
-        $inputs = '';
+        $yaml = '';
         while (!feof(STDIN)) {
-            $inputs .= fread(STDIN, 1024);
+            $yaml .= fread(STDIN, 1024);
         }
 
-        return $inputs;
+        return $yaml;
     }
 
     private function getParser()