From dc2120a1ddd2fb290349a80db56928fd9e6b1d85 Mon Sep 17 00:00:00 2001 From: =?UTF-8?q?Th=C3=A9o=20FIDRY?= <5175937+theofidry@users.noreply.github.com> Date: Thu, 7 Dec 2023 12:04:39 +0100 Subject: [PATCH] Revert "feat: Mark all declarations as internal (#882)" (#928) This reverts commit 7dbccc0e1c21a06137909bf8e5fe97156d458419. --- specs/misc/internal-declarations.php | 223 ------------------ src/Configuration/Configuration.php | 12 +- src/Configuration/ConfigurationFactory.php | 2 - src/Configuration/ConfigurationKeys.php | 2 - .../NodeVisitor/InternalCommenter.php | 100 -------- src/PhpParser/TraverserFactory.php | 14 +- src/Scoper/ScoperFactory.php | 1 - src/scoper.inc.php.tpl | 4 - .../ConfigurationFactoryTest.php | 1 - tests/Configuration/ConfigurationTest.php | 10 +- tests/PhpParser/TraverserFactoryTest.php | 1 - tests/Scoper/PhpScoperSpecTest.php | 13 +- tests/Scoper/PhpScoperTest.php | 1 - tests/Scoper/ScoperFactoryTest.php | 1 - 14 files changed, 7 insertions(+), 378 deletions(-) delete mode 100644 specs/misc/internal-declarations.php delete mode 100644 src/PhpParser/NodeVisitor/InternalCommenter.php diff --git a/specs/misc/internal-declarations.php b/specs/misc/internal-declarations.php deleted file mode 100644 index 943537597..000000000 --- a/specs/misc/internal-declarations.php +++ /dev/null @@ -1,223 +0,0 @@ -, - * Pádraic Brady - * - * 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' - <<<'PHP' - <<<'PHP' - <<<'PHP' - <<<'PHP' - $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, @@ -47,8 +46,7 @@ public function __construct( private array $filesWithContents, private array $excludedFilesWithContents, private Patcher $patcher, - private SymbolsConfiguration $symbolsConfiguration, - private bool $tagDeclarationsAsInternal, + private SymbolsConfiguration $symbolsConfiguration ) { self::validatePrefix($prefix); @@ -84,7 +82,6 @@ public function withPrefix(string $prefix): self $this->excludedFilesWithContents, $this->patcher, $this->symbolsConfiguration, - $this->tagDeclarationsAsInternal, ); } @@ -109,7 +106,6 @@ public function withFilesWithContents(array $filesWithContents): self $this->excludedFilesWithContents, $this->patcher, $this->symbolsConfiguration, - $this->tagDeclarationsAsInternal, ); } @@ -139,7 +135,6 @@ public function withPatcher(Patcher $patcher): self $this->excludedFilesWithContents, $patcher, $this->symbolsConfiguration, - $this->tagDeclarationsAsInternal, ); } @@ -153,11 +148,6 @@ public function getSymbolsConfiguration(): SymbolsConfiguration 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 5a0da2fb3..6735996d3 100644 --- a/src/Configuration/ConfigurationFactory.php +++ b/src/Configuration/ConfigurationFactory.php @@ -96,7 +96,6 @@ public function create(?string $path = null, array $paths = []): Configuration $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, @@ -106,7 +105,6 @@ public function create(?string $path = null, array $paths = []): Configuration self::retrieveFilesWithContents($excludedFiles), new PatcherChain($patchers), $symbolsConfiguration, - $tagDeclarationsAsInternal, ); } diff --git a/src/Configuration/ConfigurationKeys.php b/src/Configuration/ConfigurationKeys.php index a4f2b1c8d..9712e58d2 100644 --- a/src/Configuration/ConfigurationKeys.php +++ b/src/Configuration/ConfigurationKeys.php @@ -25,7 +25,6 @@ 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'; @@ -47,7 +46,6 @@ 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 deleted file mode 100644 index c041a19cb..000000000 --- a/src/PhpParser/NodeVisitor/InternalCommenter.php +++ /dev/null @@ -1,100 +0,0 @@ -, - * Pádraic Brady - * - * 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 5cd07d0e7..855bf50d3 100644 --- a/src/PhpParser/TraverserFactory.php +++ b/src/PhpParser/TraverserFactory.php @@ -14,7 +14,6 @@ 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; @@ -35,7 +34,6 @@ public function __construct( private readonly EnrichedReflector $reflector, private readonly string $prefix, private readonly SymbolsRegistry $symbolsRegistry, - private readonly bool $tagDeclarationsAsInternal, ) { } @@ -47,7 +45,6 @@ public function create(PhpScoper $scoper): NodeTraverserInterface $this->reflector, $scoper, $this->symbolsRegistry, - $this->tagDeclarationsAsInternal, ), ); } @@ -75,8 +72,7 @@ private static function createNodeVisitors( string $prefix, EnrichedReflector $reflector, PhpScoper $scoper, - SymbolsRegistry $symbolsRegistry, - bool $tagDeclarationsAsInternal, + SymbolsRegistry $symbolsRegistry ): array { $namespaceStatements = new NamespaceStmtCollection(); $useStatements = new UseStmtCollection(); @@ -88,7 +84,7 @@ private static function createNodeVisitors( $identifierResolver = new IdentifierResolver($nameResolver); $stringNodePrefixer = new StringNodePrefixer($scoper); - $resolvers = [ + return [ $nameResolver, new NodeVisitor\AttributeAppender\ParentNodeAppender(), new NodeVisitor\AttributeAppender\IdentifierNameAppender($identifierResolver), @@ -143,11 +139,5 @@ private static function createNodeVisitors( $reflector, ), ]; - - if ($tagDeclarationsAsInternal) { - $resolvers[] = new InternalCommenter(); - } - - return $resolvers; } } diff --git a/src/Scoper/ScoperFactory.php b/src/Scoper/ScoperFactory.php index 08854b982..2b8460541 100644 --- a/src/Scoper/ScoperFactory.php +++ b/src/Scoper/ScoperFactory.php @@ -70,7 +70,6 @@ public function createScoper( $enrichedReflector, $prefix, $symbolsRegistry, - $configuration->shouldTagDeclarationsAsInternal(), ), $this->printer, $this->lexer, diff --git a/src/scoper.inc.php.tpl b/src/scoper.inc.php.tpl index 27dd90842..a1b3065d8 100644 --- a/src/scoper.inc.php.tpl +++ b/src/scoper.inc.php.tpl @@ -84,10 +84,6 @@ 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 diff --git a/tests/Configuration/ConfigurationFactoryTest.php b/tests/Configuration/ConfigurationFactoryTest.php index 7ef3b6a23..4ce49f11c 100644 --- a/tests/Configuration/ConfigurationFactoryTest.php +++ b/tests/Configuration/ConfigurationFactoryTest.php @@ -98,7 +98,6 @@ public function test_it_can_create_a_complete_configuration(): void '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 162861ad3..23a26cb99 100644 --- a/tests/Configuration/ConfigurationTest.php +++ b/tests/Configuration/ConfigurationTest.php @@ -47,7 +47,6 @@ public function test_it_validates_the_prefix( [], new PatcherChain([]), SymbolsConfiguration::create(), - true, ); } @@ -61,7 +60,6 @@ public function test_it_can_create_a_new_instance_with_a_different_prefix(): voi ['/path/to/fileB' => ['/path/to/fileB', 'fileBContent']], new FakePatcher(), SymbolsConfiguration::create(), - true, ]; $config = new Configuration(...$values); @@ -88,7 +86,6 @@ public function test_it_can_create_a_new_instance_with_a_different_patcher(): vo ['/path/to/fileB' => ['/path/to/fileB', 'fileBContent']], new FakePatcher(), SymbolsConfiguration::create(), - true, ]; $config = new Configuration(...$values); @@ -127,8 +124,7 @@ private static function assertStateIs( array $expectedFilesWithContents, array $expectedExcludedFilesWithContents, Patcher $expectedPatcher, - SymbolsConfiguration $expectedSymbolsConfiguration, - bool $expectedShouldTagDeclarationsAsInternal, + SymbolsConfiguration $expectedSymbolsConfiguration ): void { self::assertSame($expectedPath, $configuration->getPath()); self::assertSame($expectedOutputDir, $configuration->getOutputDir()); @@ -146,9 +142,5 @@ private static function assertStateIs( $expectedSymbolsConfiguration, $configuration->getSymbolsConfiguration(), ); - self::assertSame( - $expectedShouldTagDeclarationsAsInternal, - $configuration->shouldTagDeclarationsAsInternal(), - ); } } diff --git a/tests/PhpParser/TraverserFactoryTest.php b/tests/PhpParser/TraverserFactoryTest.php index 08295b3a0..1450631d3 100644 --- a/tests/PhpParser/TraverserFactoryTest.php +++ b/tests/PhpParser/TraverserFactoryTest.php @@ -50,7 +50,6 @@ public function test_creates_a_new_traverser_at_each_call(): void ), $prefix, $symbolsRegistry, - true, ); $firstTraverser = $traverserFactory->create($phpScoper); diff --git a/tests/Scoper/PhpScoperSpecTest.php b/tests/Scoper/PhpScoperSpecTest.php index bcd40a0bb..164a90cc5 100644 --- a/tests/Scoper/PhpScoperSpecTest.php +++ b/tests/Scoper/PhpScoperSpecTest.php @@ -76,7 +76,6 @@ 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 @@ -86,7 +85,6 @@ 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 @@ -130,8 +128,7 @@ public function test_can_scope_valid_files( array $expectedRegisteredClasses, array $expectedRegisteredFunctions, ?int $minPhpVersion, - ?int $maxPhpVersion, - bool $tagDeclarationsAsInternal, + ?int $maxPhpVersion ): void { if (null !== $minPhpVersion && $minPhpVersion > PHP_VERSION_ID) { self::markTestSkipped(sprintf('Min PHP version not matched for spec %s', $spec)); @@ -149,7 +146,6 @@ public function test_can_scope_valid_files( $prefix, $symbolsConfiguration, $symbolsRegistry, - $tagDeclarationsAsInternal, ); try { @@ -265,7 +261,7 @@ public static function provideValidFiles(): iterable sprintf( 'An error occurred while parsing the file "%s": %s', $file, - $throwable->getMessage().PHP_EOL.$throwable->getTraceAsString(), + $throwable->getMessage(), ), ); } @@ -275,8 +271,7 @@ public static function provideValidFiles(): iterable private static function createScoper( string $prefix, SymbolsConfiguration $symbolsConfiguration, - SymbolsRegistry $symbolsRegistry, - bool $tagDeclarationsAsInternal, + SymbolsRegistry $symbolsRegistry ): Scoper { $container = new Container(); @@ -299,7 +294,6 @@ private static function createScoper( $enrichedReflector, $prefix, $symbolsRegistry, - $tagDeclarationsAsInternal, ), $container->getPrinter(), $container->getLexer(), @@ -378,7 +372,6 @@ private static function parseSpecFile(string $file, array $meta, int|string $fix $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 9cbe286a7..b2468ed18 100644 --- a/tests/Scoper/PhpScoperTest.php +++ b/tests/Scoper/PhpScoperTest.php @@ -106,7 +106,6 @@ protected function setUp(): void ), self::PREFIX, $this->symbolsRegistry, - true, ), $this->printer, $this->lexer, diff --git a/tests/Scoper/ScoperFactoryTest.php b/tests/Scoper/ScoperFactoryTest.php index c8ec49e86..71ef20b5a 100644 --- a/tests/Scoper/ScoperFactoryTest.php +++ b/tests/Scoper/ScoperFactoryTest.php @@ -52,7 +52,6 @@ public function test_it_can_create_a_scoper(): void [], new FakePatcher(), SymbolsConfiguration::create(), - true, ), new SymbolsRegistry(), );