Commit 6adc956a authored by Alexander Schnitzler's avatar Alexander Schnitzler Committed by Anja Leichsenring
Browse files

[TASK] Introduce a doc block checker job for bamboo

In order to avoid issues creating proper ClassSchema instances
for core classes and in order to have a properly renderable
documentation with phpdocumentor/phpdocumentor, this patch
introduces a new bamboo job that checks all php doc blocks
of classes and their properties and methods.

Releases: master
Resolves: #89023
Change-Id: I13ec766c3ac7c4cea8de84a89e66382ded6d46ba
Reviewed-on: https://review.typo3.org/c/Packages/TYPO3.CMS/+/61557


Tested-by: default avatarTYPO3com <noreply@typo3.com>
Tested-by: Andreas Fernandez's avatarAndreas Fernandez <a.fernandez@scripting-base.de>
Tested-by: Anja Leichsenring's avatarAnja Leichsenring <aleichsenring@ab-softlab.de>
Reviewed-by: Andreas Fernandez's avatarAndreas Fernandez <a.fernandez@scripting-base.de>
Reviewed-by: Anja Leichsenring's avatarAnja Leichsenring <aleichsenring@ab-softlab.de>
parent 593fb820
#!/usr/bin/env php
<?php
declare(strict_types = 1);
/*
* This file is part of the TYPO3 CMS project.
*
* It is free software; you can redistribute it and/or modify it under
* the terms of the GNU General Public License, either version 2
* of the License, or any later version.
*
* For the full copyright and license information, please read the
* LICENSE.txt file that was distributed with this source code.
*
* The TYPO3 project - inspiring people to share!
*/
use phpDocumentor\Reflection\DocBlockFactory;
use PhpParser\Error;
use PhpParser\Node;
use PhpParser\NodeTraverser;
use PhpParser\ParserFactory;
use Symfony\Component\Console\Output\ConsoleOutput;
use TYPO3\CMS\Extbase\Reflection\DocBlock\Tags\Var_;
require_once __DIR__ . '/../../vendor/autoload.php';
class NodeVisitor implements \PhpParser\NodeVisitor
{
/**
* @var DocBlockFactory
*/
private $docBlockFactory;
/**
* @var string|null
*/
public $namespace;
/**
* @var string|null
*/
public $className;
/**
* @var string|null
*/
public $classCommentError;
/**
* @var array
*/
public $properties = [];
/**
* @var array
*/
public $methods = [];
/**
* @var bool
*/
public $hasErrors = false;
/**
* Called once before traversal.
*
* Return value semantics:
* * null: $nodes stays as-is
* * otherwise: $nodes is set to the return value
*
* @param Node[] $nodes Array of nodes
*
* @return Node[]|null Array of nodes
*/
public function beforeTraverse(array $nodes)
{
$this->docBlockFactory = DocBlockFactory::createInstance();
$this->docBlockFactory->registerTagHandler('var', Var_::class);
return null;
}
/**
* Called when entering a node.
*
* Return value semantics:
* * null
* => $node stays as-is
* * NodeTraverser::DONT_TRAVERSE_CHILDREN
* => Children of $node are not traversed. $node stays as-is
* * NodeTraverser::STOP_TRAVERSAL
* => Traversal is aborted. $node stays as-is
* * otherwise
* => $node is set to the return value
*
* @param Node $node Node
*
* @return int|Node|null Replacement node (or special return value)
*/
public function enterNode(Node $node)
{
switch (get_class($node)) {
case Node\Stmt\Namespace_::class:
/** @var Node\Stmt\Namespace_ $node */
$this->namespace = (string)$node->name;
break;
case Node\Stmt\Class_::class:
/** @var Node\Stmt\Class_ $node */
$this->className = (string)$node->name;
try {
$docComment = $node->getDocComment();
if ($docComment instanceof \PhpParser\Comment) {
$this->docBlockFactory->create($docComment->getText());
}
} catch (\Throwable $e) {
$this->hasErrors = true;
$this->classCommentError = $e->getMessage();
}
break;
case Node\Stmt\Property::class:
/** @var Node\Stmt\Property $node */
$property = [
'name' => (string)$node->props[0]->name,
'error' => null
];
try {
$docComment = $node->getDocComment();
if ($docComment instanceof \PhpParser\Comment) {
$this->docBlockFactory->create($docComment->getText());
}
} catch (\Throwable $e) {
$this->hasErrors = true;
$property['error'] = $e->getMessage();
}
$this->properties[] = $property;
break;
case Node\Stmt\ClassMethod::class:
/** @var Node\Stmt\ClassMethod $node */
$method = [
'name' => (string)$node->name,
'error' => null
];
try {
$docComment = $node->getDocComment();
if ($docComment instanceof \PhpParser\Comment) {
$this->docBlockFactory->create($docComment->getText());
}
} catch (\Throwable $e) {
$this->hasErrors = true;
$method['error'] = $e->getMessage();
}
$this->methods[] = $method;
break;
default:
break;
}
return null;
}
/**
* Called when leaving a node.
*
* Return value semantics:
* * null
* => $node stays as-is
* * NodeTraverser::REMOVE_NODE
* => $node is removed from the parent array
* * NodeTraverser::STOP_TRAVERSAL
* => Traversal is aborted. $node stays as-is
* * array (of Nodes)
* => The return value is merged into the parent array (at the position of the $node)
* * otherwise
* => $node is set to the return value
*
* @param Node $node Node
*
* @return int|Node|Node[]|null Replacement node (or special return value)
*/
public function leaveNode(Node $node)
{
return null;
}
/**
* Called once after traversal.
*
* Return value semantics:
* * null: $nodes stays as-is
* * otherwise: $nodes is set to the return value
*
* @param Node[] $nodes Array of nodes
*
* @return Node[]|null Array of nodes
*/
public function afterTraverse(array $nodes)
{
return null;
}
}
$parser = (new ParserFactory)->create(ParserFactory::ONLY_PHP7);
$finder = new Symfony\Component\Finder\Finder();
$finder->files()
->in(__DIR__ . '/../../typo3/sysext/*/Classes/')
->in(__DIR__ . '/../../typo3/sysext/*/Tests/')
->name('/\.php$/')
// ->notName('ServiceProviderRegistry.php')
;
$output = new ConsoleOutput();
$errors = [];
foreach ($finder as $file) {
try {
$ast = $parser->parse($file->getContents());
} catch (Error $error) {
$output->writeln('<error>Parse error: ' . $error->getMessage() . '</error>');
exit(1);
}
$visitor = new NodeVisitor();
$traverser = new NodeTraverser();
$traverser->addVisitor($visitor);
$ast = $traverser->traverse($ast);
if ($visitor->className === null || $visitor->namespace === null) {
// only process files that contain classes for now
continue;
}
if ($visitor->hasErrors) {
$errors[$file->getRealPath()]['fqcn'] = $visitor->namespace . '\\' . $visitor->className;
if ($visitor->classCommentError !== null) {
$errors[$file->getRealPath()]['class'] = $visitor->classCommentError;
}
foreach ($visitor->properties as $property) {
if (empty($property['error'])) {
continue;
}
$errors[$file->getRealPath()]['properties'][$property['name']] = $property['error'];
}
foreach ($visitor->methods as $method) {
if (empty($method['error'])) {
continue;
}
$errors[$file->getRealPath()]['methods'][$method['name']] = $method['error'];
}
$output->write('<error>F</error>');
} else {
$output->write('<fg=green>.</>');
}
}
$output->writeln('');
if (!empty($errors)) {
foreach ($errors as $file => $errorsInFile) {
$output->writeln('');
$output->writeln('');
$output->writeln('<error>' . $file . '</error>');
$output->writeln('</>');
if (isset($errorsInFile['class'])) {
$table = new \Symfony\Component\Console\Helper\Table($output);
$table->setHeaders(['Class', 'Errors']);
$table->addRow([$errorsInFile['fqcn'], $errorsInFile['class']]);
$table->setStyle('borderless');
$table->render();
}
$properties = $errorsInFile['properties'] ?? [];
if (count($properties)) {
$table = new \Symfony\Component\Console\Helper\Table($output);
$table->setHeaders(['Properties', 'Errors']);
foreach ($properties as $propertyName => $error) {
$table->addRow([$errorsInFile['fqcn'] . '::' . $propertyName, $error]);
}
$table->setStyle('borderless');
$table->render();
}
$methods = $errorsInFile['methods'] ?? [];
if (count($methods)) {
$table = new \Symfony\Component\Console\Helper\Table($output);
$table->setHeaders(['Methods', 'Errors']);
foreach ($methods as $methodName => $error) {
$table->addRow([$errorsInFile['fqcn'] . '::' . $methodName . '()', $error]);
}
$table->setStyle('borderless');
$table->render();
}
}
exit(1);
}
exit(0);
......@@ -864,6 +864,48 @@ abstract public class AbstractCoreSpec {
.cleanWorkingDirectory(true);
}
/**
* Job with integration test checking for valid php doc blocks
*
* @param int stageNumber
* @param String requirementIdentifier
* @param Task composerTask
* @param Boolean isSecurity
*/
protected Job getJobIntegrationDocBlocks(int stageNumber, String requirementIdentifier, Task composerTask, Boolean isSecurity) {
return new Job("Integration doc blocks " + stageNumber, new BambooKey("IDB" + stageNumber))
.description("Check doc blocks by executing Build/Scripts/docBlockChecker.php script")
.pluginConfigurations(this.getDefaultJobPluginConfiguration())
.tasks(
this.getTaskGitCloneRepository(),
this.getTaskGitCherryPick(isSecurity),
this.getTaskStopDanglingContainers(),
composerTask,
new ScriptTask()
.description("Execute doc block check script")
.interpreter(ScriptTaskProperties.Interpreter.BINSH_OR_CMDEXE)
.inlineBody(
this.getScriptTaskBashInlineBody() +
"function dockBlockChecker() {\n" +
" docker run \\\n" +
" -u ${HOST_UID} \\\n" +
" -v /bamboo-data/${BAMBOO_COMPOSE_PROJECT_NAME}/passwd:/etc/passwd \\\n" +
" -v ${BAMBOO_COMPOSE_PROJECT_NAME}_bamboo-data:/srv/bamboo/xml-data/build-dir/ \\\n" +
" --name ${BAMBOO_COMPOSE_PROJECT_NAME}sib_adhoc \\\n" +
" --rm \\\n" +
" typo3gmbh/" + requirementIdentifier.toLowerCase() + ":latest \\\n" +
" bin/bash -c \"cd ${PWD}; ./Build/Scripts/docBlockChecker.php $*\"\n" +
"}\n" +
"\n" +
"dockBlockChecker"
)
)
.requirements(
this.getRequirementDocker10()
)
.cleanWorkingDirectory(true);
}
/**
* Job with various smaller script tests
*
......
......@@ -94,6 +94,7 @@ public class NightlySpec extends AbstractCoreSpec {
jobsMainStage.add(this.getJobCglCheckFullCore(0, "PHP72", this.getTaskComposerInstall("PHP72"), false));
jobsMainStage.add(this.getJobIntegrationDocBlocks(0, "PHP72", this.getTaskComposerInstall("PHP72"), false));
jobsMainStage.add(this.getJobIntegrationAnnotations(0, "PHP72", this.getTaskComposerInstall("PHP72"), false));
jobsMainStage.add(this.getJobIntegrationVarious(0, "PHP72", this.getTaskComposerInstall("PHP72"), false));
......@@ -140,6 +141,7 @@ public class NightlySpec extends AbstractCoreSpec {
jobsComposerMaxStage.add(this.getJobCglCheckFullCore(1, "PHP72", this.getTaskComposerUpdateMax("PHP72"), false));
jobsComposerMaxStage.add(this.getJobIntegrationDocBlocks(1, "PHP72", this.getTaskComposerUpdateMax("PHP72"), false));
jobsComposerMaxStage.add(this.getJobIntegrationAnnotations(1, "PHP72", this.getTaskComposerUpdateMax("PHP72"), false));
jobsComposerMaxStage.add(this.getJobIntegrationVarious(1, "PHP72", this.getTaskComposerUpdateMax("PHP72"), false));
......@@ -182,6 +184,7 @@ public class NightlySpec extends AbstractCoreSpec {
jobsComposerMinStage.add(this.getJobCglCheckFullCore(2, "PHP72", this.getTaskComposerUpdateMin("PHP72"), false));
jobsComposerMinStage.add(this.getJobIntegrationDocBlocks(2, "PHP72", this.getTaskComposerUpdateMin("PHP72"), false));
jobsComposerMinStage.add(this.getJobIntegrationAnnotations(2, "PHP72", this.getTaskComposerUpdateMin("PHP72"), false));
jobsComposerMinStage.add(this.getJobIntegrationVarious(2, "PHP72", this.getTaskComposerUpdateMin("PHP72"), false));
......
......@@ -94,6 +94,7 @@ public class PreMergeSpec extends AbstractCoreSpec {
jobsMainStage.addAll(this.getJobsAcceptanceTestsBackendMysql(0, this.numberOfAcceptanceTestJobs, "PHP73", this.getTaskComposerInstall("PHP73"), false));
jobsMainStage.add(this.getJobIntegrationDocBlocks(0, "PHP72", this.getTaskComposerInstall("PHP72"), false));
jobsMainStage.add(this.getJobIntegrationAnnotations(0, "PHP72", this.getTaskComposerInstall("PHP72"), false));
jobsMainStage.add(this.getJobIntegrationVarious(0, "PHP72", this.getTaskComposerInstall("PHP72"), false));
......
......@@ -94,6 +94,7 @@ public class SecuritySpec extends AbstractCoreSpec {
jobsMainStage.addAll(this.getJobsAcceptanceTestsBackendMysql(0, this.numberOfAcceptanceTestJobs, "PHP73", this.getTaskComposerInstall("PHP73"), true));
jobsMainStage.add(this.getJobIntegrationDocBlocks(0, "PHP72", this.getTaskComposerInstall("PHP72"), false));
jobsMainStage.add(this.getJobIntegrationAnnotations(0, "PHP72", this.getTaskComposerInstall("PHP72"), true));
jobsMainStage.add(this.getJobIntegrationVarious(0, "PHP72", this.getTaskComposerInstall("PHP72"), true));
......
......@@ -351,7 +351,6 @@ function jumpToUrl(URL) {
*
* @param string $thisLocation URL to "this location" / current script
* @return string Urls are returned as JavaScript variables T3_RETURN_URL and T3_THIS_LOCATION
* @see typo3/db_list.php
*/
public function redirectUrls($thisLocation = '')
{
......
......@@ -637,7 +637,6 @@ class ModuleTemplate
*
* @param string $thisLocation URL to "this location" / current script
* @return string Urls are returned as JavaScript variables T3_RETURN_URL and T3_THIS_LOCATION
* @see typo3/db_list.php
* @internal
*/
public function redirectUrls($thisLocation = '')
......
......@@ -3061,7 +3061,8 @@ class BackendUtility
* @param int $pid Record pid, could be negative then pointing to a record from same table whose pid to find and return
* @return int
* @internal
* @see \TYPO3\CMS\Core\DataHandling\DataHandler::copyRecord(), getTSCpid()
* @see \TYPO3\CMS\Core\DataHandling\DataHandler::copyRecord()
* @see \TYPO3\CMS\Backend\Utility\BackendUtility::getTSCpid()
*/
public static function getTSconfig_pidValue($table, $uid, $pid)
{
......@@ -3130,7 +3131,8 @@ class BackendUtility
* @return array Array of two ints; first is the REAL PID of a record and if its a new record negative values are resolved to the true PID,
* second value is the PID value for TSconfig (uid if table is pages, otherwise the pid)
* @internal
* @see \TYPO3\CMS\Core\DataHandling\DataHandler::setHistory(), \TYPO3\CMS\Core\DataHandling\DataHandler::process_datamap()
* @see \TYPO3\CMS\Core\DataHandling\DataHandler::setHistory()
* @see \TYPO3\CMS\Core\DataHandling\DataHandler::process_datamap()
*/
public static function getTSCpid($table, $uid, $pid)
{
......
......@@ -47,7 +47,7 @@ class LogEntryRepository extends \TYPO3\CMS\Extbase\Persistence\Repository
* Finds all log entries that match all given constraints.
*
* @param \TYPO3\CMS\Belog\Domain\Model\Constraint $constraint
* @return \TYPO3\CMS\Extbase\Persistence\QueryResultInterface<\TYPO3\CMS\Belog\Domain\Model\LogEntry>
* @return \TYPO3\CMS\Extbase\Persistence\QueryResultInterface
*/
public function findByConstraint(\TYPO3\CMS\Belog\Domain\Model\Constraint $constraint)
{
......@@ -66,7 +66,7 @@ class LogEntryRepository extends \TYPO3\CMS\Extbase\Persistence\Repository
*
* @param \TYPO3\CMS\Extbase\Persistence\QueryInterface $query
* @param \TYPO3\CMS\Belog\Domain\Model\Constraint $constraint
* @return array<\TYPO3\CMS\Extbase\Persistence\Generic\Qom\ConstraintInterface>
* @return array|\TYPO3\CMS\Extbase\Persistence\Generic\Qom\ConstraintInterface[]
*/
protected function createQueryConstraints(\TYPO3\CMS\Extbase\Persistence\QueryInterface $query, \TYPO3\CMS\Belog\Domain\Model\Constraint $constraint)
{
......
......@@ -34,7 +34,7 @@ class BackendUserRepository extends BackendUserGroupRepository
* Finds Backend Users on a given list of uids
*
* @param array $uidList
* @return \TYPO3\CMS\Extbase\Persistence\Generic\QueryResult<\TYPO3\CMS\Beuser\Domain\Model\BackendUser>
* @return \TYPO3\CMS\Extbase\Persistence\Generic\QueryResult
*/
public function findByUidList(array $uidList)
{
......@@ -49,7 +49,7 @@ class BackendUserRepository extends BackendUserGroupRepository
* Find Backend Users matching to Demand object properties
*
* @param Demand $demand
* @return \TYPO3\CMS\Extbase\Persistence\Generic\QueryResult<\TYPO3\CMS\Beuser\Domain\Model\BackendUser>
* @return \TYPO3\CMS\Extbase\Persistence\Generic\QueryResult
*/
public function findDemanded(Demand $demand)
{
......@@ -113,7 +113,7 @@ class BackendUserRepository extends BackendUserGroupRepository
/**
* Find Backend Users currently online
*
* @return \TYPO3\CMS\Extbase\Persistence\Generic\QueryResult<\TYPO3\CMS\Beuser\Domain\Model\BackendUser>
* @return \TYPO3\CMS\Extbase\Persistence\Generic\QueryResult
*/
public function findOnline()
{
......
......@@ -349,7 +349,7 @@ class AuthenticationService extends AbstractAuthenticationService
* parameters. The syntax is the same as for sprintf()
*
* @param string $message Message to output
* @param array<int, mixed> $params
* @param array|mixed[] $params
*/
protected function writeLogMessage(string $message, ...$params): void
{
......
......@@ -1927,7 +1927,7 @@ class BackendUserAuthentication extends AbstractUserAuthentication
* of the user is used.
*
* @return \TYPO3\CMS\Core\Resource\Folder|null
* @see \TYPO3\CMS\Core\Authentication\BackendUserAuthentication::getDefaultUploadFolder();
* @see \TYPO3\CMS\Core\Authentication\BackendUserAuthentication::getDefaultUploadFolder()
*/
public function getDefaultUploadTemporaryFolder()
{
......
......@@ -1807,7 +1807,8 @@ class DataHandler implements LoggerAwareInterface
* @param string $field Field name
* @param array $incomingFieldArray the fields being explicitly set by the outside (unlike $fieldArray) for the record
* @return array $res The result array. The processed value (if any!) is set in the "value" key.
* @see SlugEnricher, SlugHelper
* @see SlugEnricher
* @see SlugHelper
*/
protected function checkValueForSlug(string $value, array $tcaFieldConf, string $table, $id, int $realPid, string $field, array $incomingFieldArray = []): array
{
......@@ -3738,7 +3739,8 @@ class DataHandler implements LoggerAwareInterface
* @param string $_3 Not used.
* @param array $workspaceOptions
* @return array Result array with key "value" containing the value of the processing.
* @see copyRecord(), checkValue_flex_procInData_travDS()
* @see copyRecord()
* @see checkValue_flex_procInData_travDS()
*/
public function copyRecord_flexFormCallBack($pParams, $dsConf, $dataValue, $_1, $_2, $_3, $workspaceOptions)
{
......@@ -5288,7 +5290,8 @@ class DataHandler implements LoggerAwareInterface
* @param string $dataValue_ext1 Not used.
* @param string $dataValue_ext2 Not used.
* @param string $path Path in flexforms
* @see version_remapMMForVersionSwap(), checkValue_flex_procInData_travDS()
* @see version_remapMMForVersionSwap()
* @see checkValue_flex_procInData_travDS()
*/
public function version_remapMMForVersionSwap_flexFormCallBack($pParams, $dsConf, $dataValue, $dataValue_ext1, $dataValue_ext2, $path)
{
......@@ -5429,7 +5432,8 @@ class DataHandler implements LoggerAwareInterface
* @param string $dataValue_ext1 Not used
* @param string $dataValue_ext2 Not used
* @return array Array where the "value" key carries the value.
* @see checkValue_flex_procInData_travDS(), remapListedDBRecords()
* @see checkValue_flex_procInData_travDS()
* @see remapListedDBRecords()
*/
public function remapListedDBRecords_flexFormCallBack($pParams, $dsConf, $dataValue, $dataValue_ext1, $dataValue_ext2)
{
......@@ -6659,7 +6663,8 @@ class DataHandler implements LoggerAwareInterface
* @param array $fieldArray Array of field=>value pairs to insert/update
* @param string $action Action, for logging only.
* @return array|null Selected row
* @see insertDB(), updateDB()
* @see insertDB()
* @see updateDB()
*/
public function checkStoredRecord($table, $id, $fieldArray, $action)
{
......@@ -7236,7 +7241,8 @@ class DataHandler implements LoggerAwareInterface
*
* @param string $string List of pMap strings
* @return int Integer mask
* @see setTSconfigPermissions(), newFieldArray()
* @see setTSconfigPermissions()
* @see newFieldArray()
*/
public function assemblePermissions($string)
{
......
......@@ -24,7 +24,8 @@ use TYPO3\CMS\Core\Utility\MathUtility;
* process to create a new slug. Fields having `null` as value are ignored
* and can be used to by-pass implicit slug initialization.
*
* @see DataHandler::fillInFieldArray(), DataHandler::checkValueForSlug()
* @see DataHandler::fillInFieldArray()
* @see DataHandler::checkValueForSlug()
*/
class SlugEnricher
{
......
......@@ -62,7 +62,7 @@ class ExpressionBuilder
/**
* Creates a conjunction of the given boolean expressions
*
* @param mixed,... $expressions Optional clause. Requires at least one defined when converting to string.
* @param mixed $expressions Optional clause. Requires at least one defined when converting to string.
*
* @return CompositeExpression
*/
......@@ -74,7 +74,7 @@ class ExpressionBuilder
/**
* Creates a disjunction of the given boolean expressions.
*
* @param mixed,... $expressions Optional clause. Requires at least one defined when converting to string.
* @param mixed $expressions Optional clause. Requires at least one defined when converting to string.
*
* @return CompositeExpression
*/
......
......@@ -624,7 +624,7 @@ class QueryBuilder
* Specifies one or more restrictions to the query result.
* Replaces any previously specified restr