Skip to content
New issue

Have a question about this project? Sign up for a free GitHub account to open an issue and contact its maintainers and the community.

By clicking “Sign up for GitHub”, you agree to our terms of service and privacy statement. We’ll occasionally send you account related emails.

Already on GitHub? Sign in to your account

Add remaining missing return types to safe methods #50121

Merged
merged 1 commit into from
Apr 24, 2023

Conversation

wouterj
Copy link
Member

@wouterj wouterj commented Apr 22, 2023

Q A
Branch? 6.3
Bug fix? no
New feature? no
Deprecations? no
Tickets Ref #47551
License MIT
Doc PR -

This adds return types to remaining methods that are safe to receive a return type (final, internal or private ones), except from methods of DataCollector classes (which are missing quite a lot of types). I couldn't figure out whether they all return Data, the actual value or a union of both, so I've left them out for now.

Asides from these, we're missing 121 more return types on "non-safe" methods. If I have time, I'll create some smaller PRs for them to allow easier reviews.

Full list

Symfony\Bridge\Doctrine\DependencyInjection\CompilerPass\RegisterEventListenersAndSubscribersPass::process
Symfony\Bridge\Doctrine\Form\DoctrineOrmTypeGuesser::getMetadata
Symfony\Bridge\Monolog\Handler\ConsoleHandler::setOutput
Symfony\Bridge\Monolog\Handler\ConsoleHandler::onCommand
Symfony\Bridge\Monolog\Handler\ConsoleHandler::onTerminate
Symfony\Bridge\Monolog\Handler\ServerLogHandler::createSocket
Symfony\Bundle\FrameworkBundle\Command\AbstractConfigCommand::listBundles
Symfony\Bundle\FrameworkBundle\Command\AbstractConfigCommand::validateConfiguration
Symfony\Bundle\FrameworkBundle\DependencyInjection\Configuration::addHttpClientRetrySection
Symfony\Bundle\FrameworkBundle\DependencyInjection\Configuration::addRemoteEventSection
Symfony\Bundle\FrameworkBundle\DependencyInjection\FrameworkExtension::registerWebhookConfiguration
Symfony\Bundle\FrameworkBundle\DependencyInjection\FrameworkExtension::registerRemoteEventConfiguration
Symfony\Bundle\SecurityBundle\DependencyInjection\SecurityExtension::createContextListener
Symfony\Component\BrowserKit\AbstractBrowser::getScript
Symfony\Component\Cache\Adapter\ArrayAdapter::freeze
Symfony\Component\Cache\Adapter\ArrayAdapter::unfreeze
Symfony\Component\Cache\Adapter\MemcachedAdapter::checkResultCode
Symfony\Component\Cache\DependencyInjection\CachePoolPass::process
Symfony\Component\Config\Definition\BaseNode::validateType
Symfony\Component\Config\Definition\PrototypedArrayNode::addChild
Symfony\Component\Config\FileLocator::locate
Symfony\Component\Config\Loader\FileLoader::setCurrentDir
Symfony\Component\Config\Loader\FileLoader::import
Symfony\Component\Config\Loader\FileLoader::doImport
Symfony\Component\Config\ResourceCheckerConfigCache::safelyUnserialize
Symfony\Component\Console\Command\LockableTrait::release
Symfony\Component\Console\Formatter\OutputFormatter::formatAndWrap
Symfony\Component\Console\Helper\Helper::formatTime
Symfony\Component\Console\Helper\Table::setStyleDefinition
Symfony\Component\Console\Helper\Table::render
Symfony\Component\Console\Helper\Table::renderRowSeparator
Symfony\Component\Console\Helper\Table::renderRow
Symfony\Component\Console\Helper\Table::calculateNumberOfColumns
Symfony\Component\Console\Helper\Table::calculateColumnsWidth
Symfony\Component\Console\Helper\Table::cleanup
Symfony\Component\Console\Input\Input::parse
Symfony\Component\Console\Input\Input::getStream
Symfony\Component\Console\Output\Output::doWrite
Symfony\Component\DependencyInjection\Argument\ReferenceSetArgumentTrait::setValues
Symfony\Component\DependencyInjection\Container::make
Symfony\Component\DependencyInjection\Container::load
Symfony\Component\DependencyInjection\Extension\Extension::getXsdValidationBasePath
Symfony\Component\DependencyInjection\Extension\Extension::getNamespace
Symfony\Component\DependencyInjection\Extension\Extension::getConfiguration
Symfony\Component\DependencyInjection\Loader\YamlFileLoader::parseDefinition
Symfony\Component\DependencyInjection\ParameterBag\FrozenParameterBag::clear
Symfony\Component\DependencyInjection\ParameterBag\FrozenParameterBag::add
Symfony\Component\DependencyInjection\ParameterBag\FrozenParameterBag::set
Symfony\Component\DependencyInjection\ParameterBag\FrozenParameterBag::deprecate
Symfony\Component\DependencyInjection\ParameterBag\FrozenParameterBag::remove
Symfony\Component\DependencyInjection\ParameterBag\ParameterBag::isResolved
Symfony\Component\DomCrawler\AbstractUriElement::setNode
Symfony\Component\DomCrawler\Field\FormField::initialize
Symfony\Component\ExpressionLanguage\Compiler::getFunction
Symfony\Component\ExpressionLanguage\Node\Node::toArray
Symfony\Component\ExpressionLanguage\Parser::parseExpression
Symfony\Component\ExpressionLanguage\Parser::getPrimary
Symfony\Component\ExpressionLanguage\SerializedParsedExpression::getNodes
Symfony\Component\Filesystem\Filesystem::linkException
Symfony\Component\Form\AbstractType::getBlockPrefix
Symfony\Component\Form\AbstractType::getParent
Symfony\Component\Form\ButtonBuilder::setFormFactory
Symfony\Component\Form\Extension\Core\DataAccessor\PropertyPathAccessor::getPropertyValue
Symfony\Component\Form\Extension\Core\Type\ChoiceType::buildForm
Symfony\Component\Form\Extension\Core\Type\ChoiceType::buildView
Symfony\Component\Form\Extension\Core\Type\ChoiceType::finishView
Symfony\Component\Form\Extension\Core\Type\ChoiceType::configureOptions
Symfony\Component\Form\Extension\Core\Type\ChoiceType::addSubForms
Symfony\Component\Form\Extension\Core\Type\ChoiceType::addSubForm
Symfony\Component\Form\Extension\Core\Type\DateType::buildForm
Symfony\Component\Form\Extension\Core\Type\DateType::finishView
Symfony\Component\Form\Extension\Core\Type\DateType::configureOptions
Symfony\Component\Form\Extension\Core\Type\FileType::buildForm
Symfony\Component\Form\Extension\Core\Type\FileType::buildView
Symfony\Component\Form\Extension\Core\Type\FileType::finishView
Symfony\Component\Form\Extension\Core\Type\FileType::configureOptions
Symfony\Component\Form\Extension\Core\Type\MoneyType::getPattern
Symfony\Component\Form\Extension\Validator\ViolationMapper\ViolationPathIterator::mapsForm
Symfony\Component\HttpClient\Response\StreamWrapper::stream_cast
Symfony\Component\HttpFoundation\Request::prepareRequestUri
Symfony\Component\HttpKernel\DependencyInjection\ConfigurableExtension::loadInternal
Symfony\Component\Ldap\Security\LdapUserProvider::getAttributeValue
Symfony\Component\Mailer\Bridge\Mailjet\Transport\MailjetApiTransport::castCustomHeader
Symfony\Component\Mime\Email::ensureValidity
Symfony\Component\Process\Exception\ProcessFailedException::getProcess
Symfony\Component\Process\Exception\ProcessTimedOutException::getProcess
Symfony\Component\Process\Exception\ProcessTimedOutException::getExceededTimeout
Symfony\Component\Process\Process::start
Symfony\Component\Process\Process::checkTimeout
Symfony\Component\Process\Process::setOptions
Symfony\Component\Process\Process::updateStatus
Symfony\Component\Process\Process::readPipesForOutput
Symfony\Component\Process\Process::readPipes
Symfony\Component\Process\Process::resetProcessData
Symfony\Component\Process\Process::requireProcessIsStarted
Symfony\Component\Process\Process::requireProcessIsTerminated
Symfony\Component\Routing\Loader\AnnotationClassLoader::configureRoute
Symfony\Component\Routing\Matcher\UrlMatcher::getExpressionLanguage
Symfony\Component\Routing\Router::getRouteCollection
Symfony\Component\Security\Core\User\InMemoryUserProvider::createUser
Symfony\Component\Security\Http\Firewall::getSubscribedEvents
Symfony\Component\Serializer\Normalizer\AbstractObjectNormalizer::supportsNormalization
Symfony\Component\Serializer\Normalizer\AbstractObjectNormalizer::normalize
Symfony\Component\Serializer\Normalizer\AbstractObjectNormalizer::instantiateObject
Symfony\Component\Serializer\Normalizer\AbstractObjectNormalizer::supportsDenormalization
Symfony\Component\Serializer\Normalizer\AbstractObjectNormalizer::denormalize
Symfony\Component\Serializer\Normalizer\AbstractObjectNormalizer::setAttributeValue
Symfony\Component\Translation\Catalogue\AbstractOperation::processDomain
Symfony\Component\Translation\Command\XliffLintCommand::display
Symfony\Component\Translation\Command\XliffLintCommand::displayJson
Symfony\Component\Translation\Command\XliffLintCommand::getDirectoryIterator
Symfony\Component\Translation\Command\XliffLintCommand::isReadable
Symfony\Component\Translation\Extractor\PhpExtractor::seekToNextRelevantToken
Symfony\Component\Translation\Extractor\PhpExtractor::skipMethodArgument
Symfony\Component\Translation\Extractor\PhpExtractor::parseTokens
Symfony\Component\Translation\Util\ArrayConverter::getElementByPath
Symfony\Component\Validator\Constraints\IsbnValidator::getMessage
Symfony\Component\Validator\Exception\ValidationFailedException::getValue
Symfony\Component\VarDumper\Test\VarDumperTestTrait::assertDumpEquals
Symfony\Component\VarDumper\Test\VarDumperTestTrait::assertDumpMatchesFormat

@carsonbot carsonbot added this to the 6.3 milestone Apr 22, 2023
@wouterj wouterj changed the title Add remaining missing return types to safe methods [WIP] Add remaining missing return types to safe methods Apr 22, 2023
@wouterj wouterj changed the title [WIP] Add remaining missing return types to safe methods Add remaining missing return types to safe methods Apr 23, 2023
@nicolas-grekas nicolas-grekas force-pushed the missing-safe-return-types branch 3 times, most recently from 978e623 to 1da4528 Compare April 24, 2023 14:14
Copy link
Member

@nicolas-grekas nicolas-grekas left a comment

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

As always, mocks are desynchronized from the real implementation and make use move forward with closed eyes :(
I wish we could stop using them as much as possible, and use e.g. an sqlite DB here.

@nicolas-grekas
Copy link
Member

Thank you @wouterj.

@nicolas-grekas nicolas-grekas merged commit 73dc02d into symfony:6.3 Apr 24, 2023
5 of 7 checks passed
@@ -71,7 +71,7 @@ private function notify(array $records): void
$this->notifier->send($notification, ...$this->notifier->getAdminRecipients());
}

private function getHighestRecord(array $records)
private function getHighestRecord(array $records): array

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

I think return must be : LogRecord|array for monolog 3+ compatibility

@wouterj wouterj deleted the missing-safe-return-types branch June 8, 2023 05:34
nicolas-grekas added a commit that referenced this pull request Jun 8, 2023
…ity (xabbuh)

This PR was merged into the 6.3 branch.

Discussion
----------

[MonologBridge] widen return type for Monolog 3 compatibility

| Q             | A
| ------------- | ---
| Branch?       | 6.3
| Bug fix?      | yes
| New feature?  | no
| Deprecations? | no
| Tickets       | Fix #50121 (comment)
| License       | MIT
| Doc PR        |

Commits
-------

40a8561 widen return type for Monolog 3 compatibility
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Projects
None yet
Development

Successfully merging this pull request may close these issues.

None yet

4 participants