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

[Console] Better error handling when misuse of ArgvInput with arrays #53838

Closed
wants to merge 8,156 commits into from

Conversation

symfonyaml
Copy link
Contributor

Q A
Branch? 5.4
Bug fix? yes
New feature? no
Deprecations? no
Issues Fix #53836
License MIT

Issue

When we don't use ArgvInput correclty, and use array in $argv values, it returns different PHP fatal errors.
See all details and how to reproduce it in the issue #53836

Solution

In this PR

  • Add some DX with an exception explaining the problem, to avoid PHP fatal errors
  • Add tests

fabpot and others added 30 commits February 3, 2024 15:09
* 6.4:
  [Console] Fix color support
* 7.0:
  fix tests
  fix tests
  [Console] Fix color support
…orp)

This PR was squashed before being merged into the 7.1 branch.

Discussion
----------

[Security] add CAS 2.0 AccessToken handler

| Q             | A
| ------------- | ---
| Branch?       | 6.3
| Bug fix?      | no
| New feature?  | yes
| Deprecations? | no
| Tickets       |
| License       | MIT
| Doc PR        | in progress

Hello,

Thanks to the new access token, I've added the [CAS](https://apereo.github.io/cas/6.6.x/protocol/CAS-Protocol-V2-Specification.html) one.

In order to make it work :

services.yaml

```yaml
    Symfony\Component\Security\Http\AccessToken\Handler\CasHandler:
        arguments:
            $validationUrl: '%env(CAS_SERVER_VALIDATION_URL)%'

    security.access_token_extractor.cas:
        class: Symfony\Component\Security\Http\AccessToken\QueryAccessTokenExtractor
        arguments:
            - 'ticket'
```

Thank you `@welcoMattic` for the conference at the SymfonyCon  and `@jeremyFreeAgent` for your support on my first PR on this project!

Commits
-------

a1be5df [Security] add CAS 2.0 AccessToken handler
This PR was merged into the 7.1 branch.

Discussion
----------

[Yaml] Allow to get all the enum cases

| Q             | A
| ------------- | ---
| Branch?       | 7.1
| Bug fix?      | no
| New feature?  | yes
| Deprecations? | no
| Issues        | n/a
| License       | MIT

<!--
Replace this notice by a description of your feature/bugfix.
This will help reviewers and should be a good start for the documentation.

Additionally (see https://symfony.com/releases):
 - Always add tests and ensure they pass.
 - Bug fixes must be submitted against the lowest maintained branch where they apply
   (lowest branches are regularly merged to upper ones so they get the fixes too).
 - Features and deprecations must be submitted against the latest branch.
 - For new features, provide some code snippets to help understand usage.
 - Changelog entry should follow https://symfony.com/doc/current/contributing/code/conventions.html#writing-a-changelog-entry
 - Never break backward compatibility (see https://symfony.com/bc).
-->

With this addition, the `!php/enum` syntax is allowed to expose an array with all the enum cases (the result from [`UnitEnum::cases()`](https://www.php.net/manual/en/unitenum.cases.php)). This is useful for cases like `choices` option from the `Choice` validation constraint:

**BEFORE**:
```yaml
App\Entity\User:
    properties:
        status:
            - Choice:
                choices:
                    - !php/enum 'App\Entity\Enum\UserStatus::Enabled'
                    - !php/enum 'App\Entity\Enum\UserStatus::Disabled'
                    - !php/enum 'App\Entity\Enum\UserStatus::Blocked'
```

**AFTER**:
```yaml
App\Entity\User:
    properties:
        status:
            - Choice:
                choices: !php/enum 'App\Entity\Enum\UserStatus'
```

Prior to the support for enumerations, this was allowed by array constants:

```yaml
App\Entity\User:
    properties:
        status:
            - Choice:
                choices: !php/const 'App\Entity\User::AVAILABLE_STATUSES'
```

Commits
-------

3286539 [Yaml] Allow Yaml component to get all the enum cases
…*_ONLY_RES`) in IP address & CIDR constraint
…IC`, `*_ONLY_PRIV` & `*_ONLY_RES`) in IP address & CIDR constraint (Ninos)

This PR was squashed before being merged into the 7.1 branch.

Discussion
----------

[Validator] Add additional versions  (`*_NO_PUBLIC`, `*_ONLY_PRIV` & `*_ONLY_RES`) in IP address & CIDR constraint

| Q             | A
| ------------- | ---
| Branch?       | 7.1
| Bug fix?      | no
| New feature?  | yes
| Deprecations? | no
| License       | MIT

Possibility to allow no public, only private or only reserved ips.

- Enhancement: Add `*_NO_PUBLIC`, `*_ONLY_PRIV` & `*_ONLY_RES` as possible versions in `Ip` constraint
- Enhancement: Possibility to use all `Ip` versions in `Cidr` constraint

See also old MR: symfony#51777

Commits
-------

291ef1c [Validator] Add additional versions  (`*_NO_PUBLIC`, `*_ONLY_PRIV` & `*_ONLY_RES`) in IP address & CIDR constraint
…lt groups (mtarld)

This PR was merged into the 7.1 branch.

Discussion
----------

[Serializer] Add Default and "class name" default groups

| Q             | A
| ------------- | ---
| Branch?       | 7.1
| Bug fix?      | no
| New feature?  | yes
| Deprecations? | no
| Tickets       | Fix symfony#32622
| License       | MIT
| Doc PR        | TODO

Add `Default` and "class name" groups to the (de)normalization context, following Validator's component naming convention.

Commits
-------

de58759 [Serializer] Add default groups
…cleanup (jnoordsij)

This PR was merged into the 7.1 branch.

Discussion
----------

[Console] `InputArgument` and `InputOption` code cleanup

| Q             | A
| ------------- | ---
| Branch?       | 7.1
| Bug fix?      | no
| New feature?  | no
| Deprecations? | no
| Tickets       | n/a
| License       | MIT

This updates the `InputOption` and `InputArgument` classes to be slightly more consistent. Moreover it adds some additional dockblocks that were not present yet.

Commits
-------

fdf207c [Console] `InputArgument` and `InputOption` code cleanup
…her` (alexandre-daubois)

This PR was merged into the 7.1 branch.

Discussion
----------

[HttpFoundation] Add `QueryParameterRequestMatcher`

| Q             | A
| ------------- | ---
| Branch?       | 7.1
| Bug fix?      | no
| New feature?  | yes
| Deprecations? | no
| Tickets       | _NA_
| License       | MIT
| Doc PR        | Todo

We know we need a `authorizationCode` query parameter to support a request in our authenticator. We would love to use only the `ChainRequestMatcher` to do so. I'd like to add this `QueryParameterRequestMatcher` in order to do the following:

```php
class OurAuthenticator extends AbstractAuthenticator
{
    public function supports(Request $request): ?bool
    {
        return (new ChainRequestMatcher([
            // ...
            new PathRequestMatcher('/sso/provider'),
            new QueryParameterRequestMatcher('authorizationCode')
        ]))->matches($request);
    }

    // ...
}
```

Which would match the following: `/sso/provider?authorizationCode=...`

Commits
-------

448c2b1 [HttpFoundation] Add `QueryParameterRequestMatcher`
…exandre-daubois)

This PR was merged into the 7.1 branch.

Discussion
----------

[HttpFoundation] Add `HeaderRequestMatcher`

| Q             | A
| ------------- | ---
| Branch?       | 7.1
| Bug fix?      | no
| New feature?  | yes
| Deprecations? | no
| Tickets       | _NA_
| License       | MIT
| Doc PR        | Todo

This a follow up to:
- symfony#51324

After `@norkunas`' [comment](symfony#51324 (comment))

Commits
-------

62b5a34 [HttpFoundation] Add `HeaderRequestMatcher`
This makes it possible to put the meta file in a different location.

One use case can be to write a file to `src` while keeping the meta file in `var`.
…eCheckerConfigCache` (ruudk)

This PR was merged into the 7.1 branch.

Discussion
----------

[Config] Allow custom meta location in `ResourceCheckerConfigCache`

| Q             | A
| ------------- | ---
| Branch?       | 7.1
| Bug fix?      | no
| New feature?  | yes
| Deprecations? |no
| Tickets       |
| License       | MIT

This makes it possible to put the meta file in a different location.

One use case can be to write a file to `src` while keeping the meta file in `var`.

Commits
-------

ae4f8e8 Allow custom meta location in `ResourceCheckerConfigCache`
…AMQP queue that is not configured (dbu)

This PR was squashed before being merged into the 7.1 branch.

Discussion
----------

[Messenger] helpful exception when requesting an AMQP queue that is not configured

without this check, running `messenger:consume --queue=notexisting` gives an "undefined array key" PHP error with no helpful context at all.

| Q             | A
| ------------- | ---
| Branch?       | 6.4
| Bug fix?      | no
| New feature?  | no
| BC breaks?    | no
| Deprecations? | no
| Tests pass?   | yes
| Fixed tickets | -
| License       | MIT
| Doc PR        | -

just struggled a bit to figure out what the problem was, until i realized my typo in the queue name on the cli...

this could also be added to older versions of symfony as its not a new feature but a sanity check that replaces a PHP error with a meaningful exception. happy to adjust my pull request if you want me to.

Commits
-------

0d0749e [Messenger] helpful exception when requesting an AMQP queue that is not configured
…buh)

This PR was merged into the 7.1 branch.

Discussion
----------

[Console] re-add accidentally removed property

| Q             | A
| ------------- | ---
| Branch?       | 7.1
| Bug fix?      | yes
| New feature?  | no
| Deprecations? | no
| Issues        |
| License       | MIT

Commits
-------

49625a1 re-add accidentally removed property
This PR was merged into the 7.1 branch.

Discussion
----------

[TypeInfo] Introduce component

| Q             | A
| ------------- | ---
| Branch?       | 7.1
| Bug fix?      | no
| New feature?  | yes
| Deprecations? | no
| Tickets       |
| License       | MIT
| Doc PR        | TODO

Introducing the brand new `TypeInfo` component

This work has been done with `@Korbeil`

## State of the art

### Scope of the current `Symfony\Component\PropertyInfo\Type` class
Nowadays, when we need to work with types within Symfony, we have to use the `Type` class of the `PropertyInfo` component.

But what if we want to represent the type of a function's return type?

We still can use that `Type` class, but it won't make much sense as the `Symfony\Component\PropertyInfo\Type` is closely related to a property (as the namespace suggests).

Plus, when we need to extract types, we must use the `PropertyTypeExtractorInterface` service:

```php
readonly class Person
{
  public function __construct(
    public string $firstName,
  ) {
  }
}

// will return an array with a single Type object representing a string
$types = $this->propertyTypeExtractor->getTypes(Person::class, 'firstName');
```

Therefore, type retrieval in Symfony is limited to properties only.

### `Symfony\Component\PropertyInfo\Type`'s conceptual limitations

On top of that, there is a clear limitation of the current `Type` class where unions, intersections or
even generics can't be properly described.

The actual workaround is that the `PropertyTypeExtractorInterface` is returning an array of `Type`, which can be interpreted as a union type.

## The `TypeInfo` component

All these reasons bring us to create the `TypeInfo` component.

The aim here is to address these issues and:

- Have a powerful `Type` definition that can handle union, intersections, and generics (and could be even more extended)
- Being able to get types from anything, such as properties, method arguments, return types, and raw strings (and can also be extended).

### `Type` classes

To ensure a powerful `Type` definition, we defined multiple classes:
![Type classes](https://i.imgur.com/Exs5gcY.png)

The base `Type` class is an abstract one, so you'll always need to use one of the classes that inherit it.
Other types of classes are kinda self-explanatory.

### Type resolving

In the `TypeInfo` component, we added a `TypeResolverInterface`, and several implementations which allow developers to get a `Type` from many things:
- `ReflectionParameterTypeResolver` to resolve a function/method parameter type thanks to reflection
- `ReflectionPropertyTypeResolver` to resolve a property type thanks to reflection
- `ReflectionReturnTypeResolver` to resolve a function/method return type thanks to reflection
- `ReflectionTypeResolver` to resolve a `ReflectionNamedType`
- `StringTypeResolver` to resolve a type string representation. This can greatly work in combination with PHP documentation.
  > That resolver will only be available if the `phpstan/phpdoc-parser` package is installed.
- `ChainTypeResolver` to iterate over resolvers and to return a type as soon as a nested resolver succeeds in resolving.

### Type Creation

We also improved a lot the DX for the type creation with factories:
```php
<?php

use Symfony\Component\TypeInfo\Type;

Type::int();
Type::nullable(Type::string());
Type::generic(Type::object(Collection::class), Type::int());
Type::list(Type::bool());
Type::intersection(Type::object(\Stringable::class), Type::object(\Iterator::class));

// Many others are available and can be
// found in Symfony\Component\TypeInfo\TypeFactoryTrait
```

### Upgrade path

This PR only introduces the `TypeInfo` component, but another one (which is already ready) will deprecate the `Symfony\Component\PropertyInfo\Type` in favor of `Symfony\Component\TypeInfo\Type`.

Commits
-------

6de7d7d [TypeInfo] Introduce component
…) (Jean-Beru)

This PR was squashed before being merged into the 7.1 branch.

Discussion
----------

[CssSelector] add support for :is() and :where()

| Q             | A
| ------------- | ---
| Branch?       | 7.1
| Bug fix?      | no
| New feature?  | yes
| Deprecations? | no
| Tickets       | Fix symfony#48772
| License       | MIT
| Doc PR        | symfony/symfony-docs#17628

This PR adds support for [:is()](https://developer.mozilla.org/en-US/docs/Web/CSS/:is) and  [:where()](https://developer.mozilla.org/en-US/docs/Web/CSS/:where) pseudo-classes.

Commits
-------

c083e1d [CssSelector] add support for :is() and :where()
…line (maxbeckers)

This PR was submitted for the 5.4 branch but it was merged into the 7.1 branch instead.

Discussion
----------

[Yaml] Fix Yaml Parser with quote end in a new line

| Q             | A
| ------------- | ---
| Branch?       | 5.4
| Bug fix?      | yes
| New feature?  | no
| Deprecations? | no
| Tickets       | Fix symfony#33082
| License       | MIT
| Doc PR        | N/A

This is a fix for issue symfony#33082.

The bug described in the ticket breaks on a ending quote in a new line:
```
foo:
  bar: 'baz

'
  baz: 'Lorem'
```
Before the fix:
`Symfony\Component\Yaml\Exception\ParseException: Malformed inline YAML string: 'baz at line 4.`

There was already a PR symfony#33119, which was closed because of problems.

Commits
-------

21cec3f [Yaml] Fix Yaml Parser with quote end in a new line
… (94noni)

This PR was merged into the 7.1 branch.

Discussion
----------

[TwigBridge] Allow `twig:lint` to excludes dirs

| Q             | A
| ------------- | ---
| Branch?       | 6.4
| Bug fix?      | no
| New feature?  | yes
| Deprecations? | no
| Tickets       | Read PR desc
| License       | MIT
| Doc PR        |

>**Note** if it is already possible somehow and I did not found, please feel free to answer and close PR

I added a twig file related to [data_collector](https://symfony.com/doc/current/profiler.html#creating-a-data-collector) and used a profiler_dump inside it (in dev env this function exists) ([PR doc pending](symfony/symfony-docs#18470))

I do have a CI checks running this `twig:lint` on my `templates`, and since I’ve added the twig file inside it for a dev profiler data collector, it crashs with `>> Unknown "profiler_dump" function.`

(FYI i do have severals directories in this `templates/` I do not want to list them all)

This PR (opened just to see how it goes) adds an optional option as array to excludes dirs like `--excludes=data_collector`

before:

`APP_DEBUG=0 APP_ENV=preprod symfony console lint:twig templates` 🔴

after:

`APP_DEBUG=0 APP_ENV=preprod symfony console lint:twig templates --excludes=data_collector` 🟢

Commits
-------

85e0c42 [TwigBridge] Allow `twig:lint` to excludes dirs
derrabus and others added 22 commits February 27, 2024 11:29
* 7.0:
  Bump Symfony version to 7.0.5
  Update VERSION for 7.0.4
  Update CHANGELOG for 7.0.4
  Bump Symfony version to 6.4.5
  Update VERSION for 6.4.4
  Update CHANGELOG for 6.4.4
  Bump Symfony version to 5.4.37
  Update VERSION for 5.4.36
  Update CONTRIBUTORS for 5.4.36
  Update CHANGELOG for 5.4.36
* 5.4:
  Safeguard dynamic access to Doctrine metadata properties
  Enhance error handling in StaticPrefixCollection for compatibility with libpcre2-10.43
* 6.4:
  Safeguard dynamic access to Doctrine metadata properties
  Enhance error handling in StaticPrefixCollection for compatibility with libpcre2-10.43
* 7.0:
  Safeguard dynamic access to Doctrine metadata properties
  Enhance error handling in StaticPrefixCollection for compatibility with libpcre2-10.43
…rface (mbabker)

This PR was merged into the 7.1 branch.

Discussion
----------

[Lock] Make NoLock implement the SharedLockInterface

| Q             | A
| ------------- | ---
| Branch?       | 7.1
| Bug fix?      | no
| New feature?  | yes
| Deprecations? | no
| Issues        | Fix symfony#53713
| License       | MIT

This PR updates the `Symfony\Component\Lock\NoLock` stub to implement `Symfony\Component\Lock\SharedLockInterface` and returns it to being compatible with the `Symfony\Component\Lock\LockFactory` return types.

Commits
-------

22600d0 Make NoLock implement the SharedLockInterface
…ator` (MatTheCat)

This PR was merged into the 7.0 branch.

Discussion
----------

[Validator] Simplify `NoSuspiciousCharactersValidator`

| Q             | A
| ------------- | ---
| Branch?       | 7.0
| Bug fix?      | no
| New feature?  | no
| Deprecations? | no
| Issues        | N/A
| License       | MIT

php/php-src#10647 has been fixed in PHP 8.1.17. Now that Symfony requires PHP ≥ 8.2, we can avoid calling `Spoofchecker::isSuspicious` for every check by leveraging its `$errorCode` parameter.

Commits
-------

c24e3e7 [Validator] Simplify `NoSuspiciousCharactersValidator`
'tags' in not mandatory part of an event
therefore it got removed from the validation process
…rguments (palgalik)

This PR was merged into the 6.4 branch.

Discussion
----------

[Mailer] [Brevo] Remove tags from mandatory event arguments

'tags' in not mandatory part of an event
therefore it got removed from the validation process

| Q             | A
| ------------- | ---
| Branch?       |  6.4
| Bug fix?      | yes
| New feature?  | no
| Deprecations? | no
| Issues        | Fix [54088](symfony#54088)
| License       | MIT

Commits
-------

dfb41cd [Mailer][Brevo] Remove tags from mandatory event arguments
This PR was merged into the 7.1 branch.

Discussion
----------

chore: simplify PHP CS Fixer config

requires ^3.50 of Fixer

| Q             | A
| ------------- | ---
| Branch?       | 7.1
| Bug fix?      | no
| New feature?  | no <!-- please update src/**/CHANGELOG.md files -->
| Deprecations? | no <!-- please update UPGRADE-*.md and src/**/CHANGELOG.md files -->
| Issues        | Fix complex config
| License       | MIT

<!--
Replace this notice by a description of your feature/bugfix.
This will help reviewers and should be a good start for the documentation.

Additionally (see https://symfony.com/releases):
 - Always add tests and ensure they pass.
 - Bug fixes must be submitted against the lowest maintained branch where they apply
   (lowest branches are regularly merged to upper ones so they get the fixes too).
 - Features and deprecations must be submitted against the latest branch.
 - For new features, provide some code snippets to help understand usage.
 - Changelog entry should follow https://symfony.com/doc/current/contributing/code/conventions.html#writing-a-changelog-entry
 - Never break backward compatibility (see https://symfony.com/bc).
-->

Commits
-------

7e62355 chore: simplify PHP CS Fixer config
…gression in regex (PhilETaylor)

This PR was squashed before being merged into the 6.4 branch.

Discussion
----------

[AssetMapper] Fix `JavaScriptImportPathCompiler` regression in regex

| Q             | A
| ------------- | ---
| Branch?       | 6.4
| Bug fix?      | yes
| New feature?  | no <!-- please update src/**/CHANGELOG.md files -->
| Deprecations? | no <!-- please update UPGRADE-*.md and src/**/CHANGELOG.md files -->
| Issues        | Fix symfony#54078
| License       | MIT

Fixes regression in regex, fix provided by `@nicolas`-grekas in symfony#54078

Commits
-------

ae16b2d [AssetMapper] Fix `JavaScriptImportPathCompiler` regression in regex
PHPUnit 11 will complain with the following deprecation:
```
Metadata in doc-comments is deprecated and will no longer be supported in PHPUnit 12.
Update your test code to use attributes instead.
```

If we add the attribute too, it stops complaining. So this most be a way to support PHPUnit 11 and
older versions.
…ruudk)

This PR was merged into the 6.4 branch.

Discussion
----------

[Clock] Add attributes to support PHPUnit 10 + 11

| Q             | A
| ------------- | ---
| Branch?       | 6.4
| Bug fix?      | yes
| New feature?  | no
| Deprecations? | no
| Issues        | Fix #... <!-- prefix each issue number with "Fix #", no need to create an issue if none exists, explain below instead -->
| License       | MIT

While upgrading to PHPUnit 11 I noticed the following deprecation:

> Metadata in doc-comments is deprecated and will no longer be supported in PHPUnit 12. Update your test code to use attributes instead.

If we add the attribute too, it stops complaining. So this most be a way to support PHPUnit 11 and older versions.

Commits
-------

0478d54 [Clock] Add PHPUnit 10 attributes
* 5.4:
  [Security][Tests] Update functional tests to better reflect end-user scenarios
  [HttpClient] Fix deprecation on PHP 8.3
* 6.4:
  [Security][Tests] Update functional tests to better reflect end-user scenarios
  [Clock] Add PHPUnit 10 attributes
  [AssetMapper] Fix `JavaScriptImportPathCompiler` regression in regex
  [HttpClient] Fix deprecation on PHP 8.3
  [Mailer][Brevo] Remove tags from mandatory event arguments
* 7.0:
  [Security][Tests] Update functional tests to better reflect end-user scenarios
  [Clock] Add PHPUnit 10 attributes
  [AssetMapper] Fix `JavaScriptImportPathCompiler` regression in regex
  [HttpClient] Fix deprecation on PHP 8.3
  [Mailer][Brevo] Remove tags from mandatory event arguments
  [Validator] Simplify `NoSuspiciousCharactersValidator`
…kageNameAndFilePath` (smnandre)

This PR was squashed before being merged into the 7.1 branch.

Discussion
----------

[AssetMapper] Deprecate unused method `splitPackageNameAndFilePath`

| Q             | A
| ------------- | ---
| Branch?       | 7.1
| Bug fix?      | no
| New feature?  | no
| Deprecations? | yes
| Issues        | Fix symfony#54123
| License       | MIT

This method is not used in the codebase, and duplicates` ImportMapEntry::splitPackageNameAndFilePath()`

Commits
-------

9af1f34 [AssetMapper] Deprecate unused method `splitPackageNameAndFilePath`
@symfonyaml
Copy link
Contributor Author

symfonyaml commented Mar 4, 2024

I rebased the wrong branch. I dont know how to revert it, so I created a new PR. Sorry

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