Repository: humbug/php-scoper (php)

Issue:
Mark scoped symbols as private
IDEs support to not complete private classes when they come from third-party dependencies.

Pull request:
feat: Mark all declarations as internal
This should allow IDEs such as PHPStorm to honour the tag and avoid showing the results in the auto-complete.

Closes #861.

Patch:
diff --git a/specs/misc/internal-declarations.php b/specs/misc/internal-declarations.php
new file mode 100644
index 0000000..9435375
--- /dev/null
+++ b/specs/misc/internal-declarations.php
@@ -0,0 +1,223 @@
+<?php
+
+declare(strict_types=1);
+
+/*
+ * This file is part of the humbug/php-scoper package.
+ *
+ * Copyright (c) 2017 Théo FIDRY <theo.fidry@gmail.com>,
+ *                    Pádraic Brady <padraic.brady@gmail.com>
+ *
+ * For the full copyright and license information, please view the LICENSE
+ * file that was distributed with this source code.
+ */
+
+return [
+    'meta' => [
+        'title' => 'It adds @internal annotations to all declarations',
+        // Default values. If not specified will be the one used
+        'prefix' => 'Humbug',
+
+        'tag-declarations-as-internal' => true,
+
+        'expose-global-constants' => false,
+        'expose-global-classes' => false,
+        'expose-global-functions' => false,
+        'expose-namespaces' => [],
+        'expose-constants' => [],
+        'expose-classes' => [],
+        'expose-functions' => [],
+
+        'exclude-namespaces' => [],
+        'exclude-constants' => [],
+        'exclude-classes' => [],
+        'exclude-functions' => [],
+
+        'expected-recorded-classes' => [],
+        'expected-recorded-functions' => [],
+    ],
+
+    'Declarations without any comment' => <<<'PHP'
+    <?php
+    
+    class RegularClass {
+        public function aMethod() {}
+    }
+    
+    interface RegularInterface {}
+    
+    abstract class RegularAbstractClass {}
+    
+    function regular_function () {}
+    
+    trait RegularTrait {}
+    
+    const REGULAR_CONSTANT = 'FOO';
+    
+    enum RegularEnum {}
+    
+    ----
+    <?php
+    
+    namespace Humbug;
+
+    /** @internal */
+    class RegularClass
+    {
+        public function aMethod()
+        {
+        }
+    }
+    /** @internal */
+    interface RegularInterface
+    {
+    }
+    /** @internal */
+    abstract class RegularAbstractClass
+    {
+    }
+    /** @internal */
+    function regular_function()
+    {
+    }
+    /** @internal */
+    trait RegularTrait
+    {
+    }
+    /** @internal */
+    const REGULAR_CONSTANT = 'FOO';
+    /** @internal */
+    enum RegularEnum
+    {
+    }
+    
+    PHP,
+
+    'Declarations without with comments' => <<<'PHP'
+    <?php
+    
+    // Smth
+    class RegularClass {
+        public function aMethod() {}
+    }
+    ----
+    <?php
+    
+    namespace Humbug;
+
+    // Smth
+    /** @internal */
+    class RegularClass
+    {
+        public function aMethod()
+        {
+        }
+    }
+    
+    PHP,
+
+    'Declarations with existing phpDoc' => <<<'PHP'
+    <?php
+    
+    /**
+     * A comment.
+     */
+    class RegularClass {
+        public function aMethod() {}
+    }
+    ----
+    <?php
+    
+    namespace Humbug;
+
+    /**
+     * A comment.
+     * @internal
+     */
+    class RegularClass
+    {
+        public function aMethod()
+        {
+        }
+    }
+    
+    PHP,
+
+    'Declarations with inlined phpDoc' => <<<'PHP'
+    <?php
+    
+    /** A comment. */
+    class RegularClass {
+        public function aMethod() {}
+    }
+    ----
+    <?php
+    
+    namespace Humbug;
+
+    /** A comment.
+     * @internal
+     */
+    class RegularClass
+    {
+        public function aMethod()
+        {
+        }
+    }
+    
+    PHP,
+
+    'Declarations with inlined phpDoc already containing the @internal tag' => <<<'PHP'
+    <?php
+    
+    /** @internal */
+    class RegularClass {
+        public function aMethod() {}
+    }
+    ----
+    <?php
+    
+    namespace Humbug;
+
+    /** @internal */
+    class RegularClass
+    {
+        public function aMethod()
+        {
+        }
+    }
+    
+    PHP,
+
+    'Declarations with existing phpDoc containing the @internal tag' => <<<'PHP'
+    <?php
+    
+    /**
+     * A comment.
+     * 
+     * @private
+     * @internal
+     */
+    class RegularClass {
+        public function aMethod() {}
+    }
+    ----
+    <?php
+    
+    namespace Humbug;
+
+    /**
+     * A comment.
+     * 
+     * @private
+     * @internal
+     */
+    class RegularClass
+    {
+        public function aMethod()
+        {
+        }
+    }
+    
+    PHP,
+];
diff --git a/src/Configuration/Configuration.php b/src/Configuration/Configuration.php
index 4ad285a..2793111 100644
--- a/src/Configuration/Configuration.php
+++ b/src/Configuration/Configuration.php
@@ -38,6 +38,7 @@ final class Configuration
      * @param array<string, array{string, string}> $excludedFilesWithContents Array of tuple
      *                                                                        with the first argument being the file path and
      *                                                                        the second its contents
+     * @param bool                                 $tagDeclarationsAsInternal Whether a @internal tag should be added to the symbols declarations.
      */
     public function __construct(
         private ?string $path,
@@ -46,7 +47,8 @@ final class Configuration
         private array $filesWithContents,
         private array $excludedFilesWithContents,
         private Patcher $patcher,
-        private SymbolsConfiguration $symbolsConfiguration
+        private SymbolsConfiguration $symbolsConfiguration,
+        private bool $tagDeclarationsAsInternal,
     ) {
         self::validatePrefix($prefix);
 
@@ -82,6 +84,7 @@ final class Configuration
             $this->excludedFilesWithContents,
             $this->patcher,
             $this->symbolsConfiguration,
+            $this->tagDeclarationsAsInternal,
         );
     }
 
@@ -106,6 +109,7 @@ final class Configuration
             $this->excludedFilesWithContents,
             $this->patcher,
             $this->symbolsConfiguration,
+            $this->tagDeclarationsAsInternal,
         );
     }
 
@@ -135,6 +139,7 @@ final class Configuration
             $this->excludedFilesWithContents,
             $patcher,
             $this->symbolsConfiguration,
+            $this->tagDeclarationsAsInternal,
         );
     }
 
@@ -148,6 +153,11 @@ final class Configuration
         return $this->symbolsConfiguration;
     }
 
+    public function shouldTagDeclarationsAsInternal(): bool
+    {
+        return $this->tagDeclarationsAsInternal;
+    }
+
     private static function validatePrefix(string $prefix): void
     {
         if (1 !== preg_match(self::PREFIX_PATTERN, $prefix)) {
diff --git a/src/Configuration/ConfigurationFactory.php b/src/Configuration/ConfigurationFactory.php
index 12303a9..38ffcdf 100644
--- a/src/Configuration/ConfigurationFactory.php
+++ b/src/Configuration/ConfigurationFactory.php
@@ -96,6 +96,7 @@ final class ConfigurationFactory
         $finders = self::retrieveFinders($config);
         $filesFromPaths = self::retrieveFilesFromPaths($paths);
         $filesWithContents = self::retrieveFilesWithContents(chain($filesFromPaths, ...$finders));
+        $tagDeclarationsAsInternal = $config[ConfigurationKeys::TAG_DECLARATIONS_AS_INTERNAL] ?? true;
 
         return new Configuration(
             $path,
@@ -105,6 +106,7 @@ final class ConfigurationFactory
             self::retrieveFilesWithContents($excludedFiles),
             new PatcherChain($patchers),
             $symbolsConfiguration,
+            $tagDeclarationsAsInternal,
         );
     }
 
diff --git a/src/Configuration/ConfigurationKeys.php b/src/Configuration/ConfigurationKeys.php
index 9712e58..a4f2b1c 100644
--- a/src/Configuration/ConfigurationKeys.php
+++ b/src/Configuration/ConfigurationKeys.php
@@ -25,6 +25,7 @@ final class ConfigurationKeys
     public const EXCLUDED_FILES_KEYWORD = 'exclude-files';
     public const FINDER_KEYWORD = 'finders';
     public const PATCHERS_KEYWORD = 'patchers';
+    public const TAG_DECLARATIONS_AS_INTERNAL = 'tag-declarations-as-internal';
 
     public const EXPOSE_GLOBAL_CONSTANTS_KEYWORD = 'expose-global-constants';
     public const EXPOSE_GLOBAL_CLASSES_KEYWORD = 'expose-global-classes';
@@ -46,6 +47,7 @@ final class ConfigurationKeys
         self::EXCLUDED_FILES_KEYWORD,
         self::FINDER_KEYWORD,
         self::PATCHERS_KEYWORD,
+        self::TAG_DECLARATIONS_AS_INTERNAL,
         self::EXPOSE_GLOBAL_CONSTANTS_KEYWORD,
         self::EXPOSE_GLOBAL_CLASSES_KEYWORD,
         self::EXPOSE_GLOBAL_FUNCTIONS_KEYWORD,
diff --git a/src/PhpParser/NodeVisitor/InternalCommenter.php b/src/PhpParser/NodeVisitor/InternalCommenter.php
new file mode 100644
index 0000000..c041a19
--- /dev/null
+++ b/src/PhpParser/NodeVisitor/InternalCommenter.php
@@ -0,0 +1,100 @@
+<?php
+
+declare(strict_types=1);
+
+/*
+ * This file is part of the humbug/php-scoper package.
+ *
+ * Copyright (c) 2017 Théo FIDRY <theo.fidry@gmail.com>,
+ *                    Pádraic Brady <padraic.brady@gmail.com>
+ *
+ * For the full copyright and license information, please view the LICENSE
+ * file that was distributed with this source code.
+ */
+
+namespace Humbug\PhpScoper\PhpParser\NodeVisitor;
+
+use PhpParser\Comment\Doc;
+use PhpParser\Node;
+use PhpParser\Node\Stmt\Class_;
+use PhpParser\Node\Stmt\Const_;
+use PhpParser\Node\Stmt\Enum_;
+use PhpParser\Node\Stmt\Function_;
+use PhpParser\Node\Stmt\Interface_;
+use PhpParser\Node\Stmt\Trait_;
+use PhpParser\NodeVisitorAbstract;
+use function array_splice;
+use function count;
+use function explode;
+use function implode;
+use function is_string;
+use function rtrim;
+use function sprintf;
+use function str_contains;
+use function strlen;
+use function substr;
+
+final class InternalCommenter extends NodeVisitorAbstract
+{
+    public function enterNode(Node $node): Node
+    {
+        if ($node instanceof Class_
+            || $node instanceof Const_
+            || $node instanceof Enum_
+            || $node instanceof Function_
+            || $node instanceof Interface_
+            || $node instanceof Trait_
+        ) {
+            self::addTagToPhpDoc($node);
+        }
+
+        return $node;
+    }
+
+    private static function addTagToPhpDoc(Class_|Const_|Enum_|Function_|Interface_|Trait_ $node): void
+    {
+        $node->setDocComment(
+            new Doc(
+                self::createText($node->getDocComment()),
+            ),
+        );
+    }
+
+    private static function createText(?Doc $existingDoc): string
+    {
+        if (null === $existingDoc) {
+            return '/** @internal */';
+        }
+
+        $reformattedText = $existingDoc->getReformattedText();
+
+        if (!is_string($reformattedText)) {
+            return $existingDoc->getText();
+        }
+
+        if (str_contains($reformattedText, '@internal')) {
+            return $reformattedText;
+        }
+
+        $textLines = explode("\n", $reformattedText);
+
+        if (count($textLines) > 1) {
+            array_splice(
+                $textLines,
+                -1,
+                0,
+                [' * @internal'],
+            );
+        } else {
+            $line = $textLines[0];
+            $textLines = [
+                sprintf(
+                    "%s\n * @internal\n */",
+                    rtrim(substr($line, 0, strlen($line) - 2)),
+                ),
+            ];
+        }
+
+        return implode("\n", $textLines);
+    }
+}
diff --git a/src/PhpParser/TraverserFactory.php b/src/PhpParser/TraverserFactory.php
index 855bf50..5cd07d0 100644
--- a/src/PhpParser/TraverserFactory.php
+++ b/src/PhpParser/TraverserFactory.php
@@ -14,6 +14,7 @@ declare(strict_types=1);
 
 namespace Humbug\PhpScoper\PhpParser;
 
+use Humbug\PhpScoper\PhpParser\NodeVisitor\InternalCommenter;
 use Humbug\PhpScoper\PhpParser\NodeVisitor\NamespaceStmt\NamespaceStmtCollection;
 use Humbug\PhpScoper\PhpParser\NodeVisitor\Resolver\IdentifierResolver;
 use Humbug\PhpScoper\PhpParser\NodeVisitor\UseStmt\UseStmtCollection;
@@ -34,6 +35,7 @@ class TraverserFactory
         private readonly EnrichedReflector $reflector,
         private readonly string $prefix,
         private readonly SymbolsRegistry $symbolsRegistry,
+        private readonly bool $tagDeclarationsAsInternal,
     ) {
     }
 
@@ -45,6 +47,7 @@ class TraverserFactory
                 $this->reflector,
                 $scoper,
                 $this->symbolsRegistry,
+                $this->tagDeclarationsAsInternal,
             ),
         );
     }
@@ -72,7 +75,8 @@ class TraverserFactory
         string $prefix,
         EnrichedReflector $reflector,
         PhpScoper $scoper,
-        SymbolsRegistry $symbolsRegistry
+        SymbolsRegistry $symbolsRegistry,
+        bool $tagDeclarationsAsInternal,
     ): array {
         $namespaceStatements = new NamespaceStmtCollection();
         $useStatements = new UseStmtCollection();
@@ -84,7 +88,7 @@ class TraverserFactory
         $identifierResolver = new IdentifierResolver($nameResolver);
         $stringNodePrefixer = new StringNodePrefixer($scoper);
 
-        return [
+        $resolvers = [
             $nameResolver,
             new NodeVisitor\AttributeAppender\ParentNodeAppender(),
             new NodeVisitor\AttributeAppender\IdentifierNameAppender($identifierResolver),
@@ -139,5 +143,11 @@ class TraverserFactory
                 $reflector,
             ),
         ];
+
+        if ($tagDeclarationsAsInternal) {
+            $resolvers[] = new InternalCommenter();
+        }
+
+        return $resolvers;
     }
 }
diff --git a/src/Scoper/ScoperFactory.php b/src/Scoper/ScoperFactory.php
index 2b84605..08854b9 100644
--- a/src/Scoper/ScoperFactory.php
+++ b/src/Scoper/ScoperFactory.php
@@ -70,6 +70,7 @@ class ScoperFactory
                     $enrichedReflector,
                     $prefix,
                     $symbolsRegistry,
+                    $configuration->shouldTagDeclarationsAsInternal(),
                 ),
                 $this->printer,
                 $this->lexer,
diff --git a/src/scoper.inc.php.tpl b/src/scoper.inc.php.tpl
index a1b3065..27dd908 100644
--- a/src/scoper.inc.php.tpl
+++ b/src/scoper.inc.php.tpl
@@ -84,6 +84,10 @@ return [
         },
     ],
 
+    // Whether symbols declarations should have the PHPDoc tag "@internal" added. This helps some IDEs to not indicate
+    // to not include the symbol for auto-completion suggestions for example.
+    'tag-declarations-as-internal' => true,
+
     // List of symbols to consider internal i.e. to leave untouched.
     //
     // For more information see: https://github.com/humbug/php-scoper/blob/master/docs/configuration.md#excluded-symbols


Test patch:
diff --git a/tests/Configuration/ConfigurationFactoryTest.php b/tests/Configuration/ConfigurationFactoryTest.php
index 4ce49f1..7ef3b6a 100644
--- a/tests/Configuration/ConfigurationFactoryTest.php
+++ b/tests/Configuration/ConfigurationFactoryTest.php
@@ -98,6 +98,7 @@ class ConfigurationFactoryTest extends FileSystemTestCase
                     'exclude-files' => ['file1', 'file2'],
                     'patchers' => [],
                     'finders' => [],
+                    'tag-declarations-as-internal' => true,
 
                     'expose-global-constants' => false,
                     'expose-global-classes' => false,
diff --git a/tests/Configuration/ConfigurationTest.php b/tests/Configuration/ConfigurationTest.php
index 23a26cb..162861a 100644
--- a/tests/Configuration/ConfigurationTest.php
+++ b/tests/Configuration/ConfigurationTest.php
@@ -47,6 +47,7 @@ final class ConfigurationTest extends TestCase
             [],
             new PatcherChain([]),
             SymbolsConfiguration::create(),
+            true,
         );
     }
 
@@ -60,6 +61,7 @@ final class ConfigurationTest extends TestCase
             ['/path/to/fileB' => ['/path/to/fileB', 'fileBContent']],
             new FakePatcher(),
             SymbolsConfiguration::create(),
+            true,
         ];
 
         $config = new Configuration(...$values);
@@ -86,6 +88,7 @@ final class ConfigurationTest extends TestCase
             ['/path/to/fileB' => ['/path/to/fileB', 'fileBContent']],
             new FakePatcher(),
             SymbolsConfiguration::create(),
+            true,
         ];
 
         $config = new Configuration(...$values);
@@ -124,7 +127,8 @@ final class ConfigurationTest extends TestCase
         array $expectedFilesWithContents,
         array $expectedExcludedFilesWithContents,
         Patcher $expectedPatcher,
-        SymbolsConfiguration $expectedSymbolsConfiguration
+        SymbolsConfiguration $expectedSymbolsConfiguration,
+        bool $expectedShouldTagDeclarationsAsInternal,
     ): void {
         self::assertSame($expectedPath, $configuration->getPath());
         self::assertSame($expectedOutputDir, $configuration->getOutputDir());
@@ -142,5 +146,9 @@ final class ConfigurationTest extends TestCase
             $expectedSymbolsConfiguration,
             $configuration->getSymbolsConfiguration(),
         );
+        self::assertSame(
+            $expectedShouldTagDeclarationsAsInternal,
+            $configuration->shouldTagDeclarationsAsInternal(),
+        );
     }
 }
diff --git a/tests/PhpParser/TraverserFactoryTest.php b/tests/PhpParser/TraverserFactoryTest.php
index 1450631..08295b3 100644
--- a/tests/PhpParser/TraverserFactoryTest.php
+++ b/tests/PhpParser/TraverserFactoryTest.php
@@ -50,6 +50,7 @@ class TraverserFactoryTest extends TestCase
             ),
             $prefix,
             $symbolsRegistry,
+            true,
         );
 
         $firstTraverser = $traverserFactory->create($phpScoper);
diff --git a/tests/Scoper/PhpScoperSpecTest.php b/tests/Scoper/PhpScoperSpecTest.php
index 164a90c..bcd40a0 100644
--- a/tests/Scoper/PhpScoperSpecTest.php
+++ b/tests/Scoper/PhpScoperSpecTest.php
@@ -76,6 +76,7 @@ class PhpScoperSpecTest extends TestCase
         // SPECS_CONFIG_KEYS included
         'expected-recorded-classes',
         'expected-recorded-functions',
+        ConfigurationKeys::TAG_DECLARATIONS_AS_INTERNAL,
     ];
 
     // Keys allowed on a spec level
@@ -85,6 +86,7 @@ class PhpScoperSpecTest extends TestCase
         'expected-recorded-classes',
         'expected-recorded-functions',
         'payload',
+        ConfigurationKeys::TAG_DECLARATIONS_AS_INTERNAL,
     ];
 
     // Keys kept and used to build the symbols configuration
@@ -128,7 +130,8 @@ class PhpScoperSpecTest extends TestCase
         array $expectedRegisteredClasses,
         array $expectedRegisteredFunctions,
         ?int $minPhpVersion,
-        ?int $maxPhpVersion
+        ?int $maxPhpVersion,
+        bool $tagDeclarationsAsInternal,
     ): void {
         if (null !== $minPhpVersion && $minPhpVersion > PHP_VERSION_ID) {
             self::markTestSkipped(sprintf('Min PHP version not matched for spec %s', $spec));
@@ -146,6 +149,7 @@ class PhpScoperSpecTest extends TestCase
             $prefix,
             $symbolsConfiguration,
             $symbolsRegistry,
+            $tagDeclarationsAsInternal,
         );
 
         try {
@@ -261,7 +265,7 @@ class PhpScoperSpecTest extends TestCase
                     sprintf(
                         'An error occurred while parsing the file "%s": %s',
                         $file,
-                        $throwable->getMessage(),
+                        $throwable->getMessage().PHP_EOL.$throwable->getTraceAsString(),
                     ),
                 );
             }
@@ -271,7 +275,8 @@ class PhpScoperSpecTest extends TestCase
     private static function createScoper(
         string $prefix,
         SymbolsConfiguration $symbolsConfiguration,
-        SymbolsRegistry $symbolsRegistry
+        SymbolsRegistry $symbolsRegistry,
+        bool $tagDeclarationsAsInternal,
     ): Scoper {
         $container = new Container();
 
@@ -294,6 +299,7 @@ class PhpScoperSpecTest extends TestCase
                 $enrichedReflector,
                 $prefix,
                 $symbolsRegistry,
+                $tagDeclarationsAsInternal,
             ),
             $container->getPrinter(),
             $container->getLexer(),
@@ -372,6 +378,7 @@ class PhpScoperSpecTest extends TestCase
             $fixtureSet['expected-recorded-functions'] ?? $meta['expected-recorded-functions'],
             $meta['minPhpVersion'] ?? null,
             $meta['maxPhpVersion'] ?? null,
+            $fixtureSet[ConfigurationKeys::TAG_DECLARATIONS_AS_INTERNAL] ?? $meta[ConfigurationKeys::TAG_DECLARATIONS_AS_INTERNAL] ?? false,
         ];
     }
 
diff --git a/tests/Scoper/PhpScoperTest.php b/tests/Scoper/PhpScoperTest.php
index b2468ed..9cbe286 100644
--- a/tests/Scoper/PhpScoperTest.php
+++ b/tests/Scoper/PhpScoperTest.php
@@ -106,6 +106,7 @@ class PhpScoperTest extends TestCase
                 ),
                 self::PREFIX,
                 $this->symbolsRegistry,
+                true,
             ),
             $this->printer,
             $this->lexer,
diff --git a/tests/Scoper/ScoperFactoryTest.php b/tests/Scoper/ScoperFactoryTest.php
index 71ef20b..c8ec49e 100644
--- a/tests/Scoper/ScoperFactoryTest.php
+++ b/tests/Scoper/ScoperFactoryTest.php
@@ -52,6 +52,7 @@ final class ScoperFactoryTest extends TestCase
                 [],
                 new FakePatcher(),
                 SymbolsConfiguration::create(),
+                true,
             ),
             new SymbolsRegistry(),
         );