8000 OptionsResolver error message with string[] as allowedTypes · Issue #30432 · symfony/symfony · GitHub
[go: up one dir, main page]
Skip to content

OptionsResolver error message with string[] as allowedTypes #30432

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.

Sign up for GitHub

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

Jump to bottom
Closed
steevanb opened this issue Mar 4, 2019 · 7 comments
Closed

OptionsResolver error message with string[] as allowedTypes #30432

steevanb opened this issue Mar 4, 2019 · 7 comments
Labels
Bug OptionsResolver Status: Needs Review

Comments

@steevanb
Copy link
steevanb commented Mar 4, 2019

Symfony version(s) affected: 3.4, maybe other versions too (not tested)

Description
Error message when allowedTypes is an array contains [] but should not:
The option "testme" with value array is expected to be of type "string[]", but one of the elements is of type "integer[]".
It should be:
The option "testme" with value array is expected to be of type "string[]", but one of the elements is of type "integer".

How to reproduce

$resolver = (new OptionsResolver())
    ->setDefault('testme', [])
    ->setAllowedTypes('testme', ['string[]'])
    ->resolve(['testme' => ['test', 12]]);
@xabbuh
Copy link
Member
xabbuh commented Mar 4, 2019

Good catch, would you like to send a PR?

@dimabory
Copy link
Contributor
dimabory commented Mar 4, 2019

@xabbuh I took a look at OptionsResolver component. Can you or someone else confirm how an error message should look like?

For instance, the current implementation implies that this code

(new OptionsResolver())
	->setDefined('foo');
	->setAllowedTypes('foo', 'int[][]');
	->resolve([
		'foo' => [
			[1, true, 'str', [2, 3]],
		],
	]);

will throw an exception with message The option "foo" with value array is expected to be of type "int[][]", but one of the elements is of type "boolean[][]".
If we are good to go with ..., but one of the elements is of type "boolean". I can open PR right now as much work already done.

@xabbuh
Copy link
Member
xabbuh commented Mar 4, 2019

For the example from the PR description the correct error message IMO indeed should be:

The option "testme" with value array is expected to be of type "string[]", but one of the elements is of type "integer".

@dimabory
Copy link
Contributor
dimabory commented Mar 4, 2019

I got it. Confirm case when allowed types consist multidimensional array notation (e.g. string[][]) and message should look like:

The option "testme" with value array is expected to be of type "string[][]", but one of the elements is of type "integer".

@xabbuh
Copy link
Member
xabbuh commented Mar 4, 2019

I would have expected an error message containing all invalid types:

[...] is expected to be of type "integer[][]", but is of type "integer|boolean|string".

@dimabory
Copy link
Contributor
dimabory commented Mar 4, 2019

Hm, seems like it requires a huge effort. But I will see what I can do with it.

dimabory added a commit to dimabory/symfony that referenced this issue Mar 4, 2019
@dimabory
symfony#30432 fix an error message
e949563
@dimabory dimabory mentioned this issue Mar 4, 2019
Merged
dimabory added a commit to dimabory/symfony that referenced this issue Mar 4, 2019
@dimabory
symfony#30432 fix an error message
4f822e5
dimabory added a commit to dimabory/symfony that referenced this issue Mar 4, 2019
@dimabory
symfony#30432 fix an error message
8cd409d
dimabory added a commit to dimabory/symfony that referenced this issue Mar 4, 2019
@dimabory
symfony#30432 fix an error message
26fc322
dimabory added a commit to dimabory/symfony that referenced this issue Mar 4, 2019
@dimabory
symfony#30432 fix an error message
3b6a98e
dimabory added a commit to dimabory/symfony that referenced this issue Mar 4, 2019
@dimabory
symfony#30432 fix an error message
4b22c46
@steevanb
Copy link
Author
steevanb commented Mar 6, 2019

Thanks!

dimabory added a commit to dimabory/symfony that referenced this issue Mar 6, 2019
@dimabory
symfony#30432 provide an accurate error message for multiple allowed …
ce249ba 
…types

symfony#30442 (comment)
dimabory added a commit to dimabory/symfony that referenced this issue Mar 6, 2019
@dimabory
symfony#30432 provide an accurate error message for multiple allowed …
16286c7 
…types

symfony#30442 (comment)
dimabory added a commit to dimabory/symfony that referenced this issue Mar 7, 2019
@dimabory
symfony#30432 provide an accurate error message for multiple allowed …
308ed6e 
…types

symfony#30442 (comment)
dimabory added a commit to dimabory/symfony that referenced this issue Mar 11, 2019
@dimabory
symfony#30432 fix an error message
fd67b95
dimabory added a commit to dimabory/symfony that referenced this issue Mar 11, 2019
@dimabory
symfony#30432 fix an error message
2f4ff2d
dimabory added a commit to dimabory/symfony that referenced this issue Mar 11, 2019
@dimabory
symfony#30432 fix an error message
b2721b3
dimabory added a commit to dimabory/symfony that referenced this issue Mar 11, 2019
@dimabory
symfony#30432 fix an error message
dad7b4d
dimabory added a commit to dimabory/symfony that referenced this issue Mar 13, 2019
@dimabory
symfony#30432 fix an error message
dc4383d
xabbuh pushed a commit to dimabory/symfony that referenced this issue Mar 27, 2019
@dimabory @xabbuh
symfony#30432 fix an error message
d1fb955
dimabory added a commit to dimabory/symfony that referenced this issue Oct 15, 2019
@dimabory
symfony#30432 fix an error message
6addfa5
yceruto pushed a commit to dimabory/symfony that referenced this issue Oct 26, 2019
@dimabory @yceruto
symfony#30432 fix an error message
7fa2fc2
yceruto added a commit that referenced this issue Oct 26, 2019
@yceruto
minor #30442 [OptionsResolver] Fix an error message to be more accura…
fb70e0a 
…te (dimabory)

This PR was merged into the 3.4 branch.

Discussion
----------

[OptionsResolver] Fix an error message to be more accurate

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

See #30432 for more details:
> **Symfony version(s) affected**: 3.4, maybe other versions too (not tested)
>
> **Description**
> Error message when allowedTypes is an array contains `[]` but should not:
> `The option "testme" with value array is expected to be of type "string[]", but one of the elements is of type "integer[]".`
> It should be:
> `The option "testme" with value array is expected to be of type "string[]", but one of the elements is of type "integer".`
>
> **How to reproduce**
>
> ```
> $resolver = (new OptionsResolver())
>     ->setDefault('testme', [])
>     ->setAllowedTypes('testme', ['string[]'])
>     ->resolve(['testme' => ['test', 12]]);
> ```

In addition I changed an error message to be more
accurate if provided more than one incorrect value:
> [...] is expected to be of type "integer[][]", but is of type "integer|boolean|string".

Commits
-------

7fa2fc2 #30432 fix an error message
nicolas-grekas added a commit that referenced this issue Oct 28, 2019
@nicolas-grekas
Merge branch '3.4' into 4.3
6d5c91b 
* 3.4:
  #30432 fix an error message
  fix paths to detect code owners
  [Validator] Ensure numeric subpaths do not cause errors on PHP 7.4
  Remove unused local variables in tests
  Make sure to collect child forms created on *_SET_DATA events
  do not render errors for checkboxes twice
@yceruto yceruto mentioned this issue Oct 28, 2019
Merged
nicolas-grekas added a commit that referenced this issue Oct 28, 2019
@nicolas-grekas
Merge branch '4.3' into 4.4
05f7f4e 
* 4.3:
  [OptionsResolve] Revert change in tests for a not-merged change in code
  [HttpClient] fix handling of 3xx with no Location header - ignore Content-Length when no body is expected
  [Workflow] Made the configuration more robust for the 'property' key
  [Security/Core] make NativePasswordEncoder use sodium to validate passwords when possible
  #30432 fix an error message
  fix paths to detect code owners
  [HttpClient] ignore the body of responses to HEAD requests
  [Validator] Ensure numeric subpaths do not cause errors on PHP 7.4
  [SecurityBundle] Fix wrong assertion
  Remove unused local variables in tests
  [Yaml][Parser] Remove the getLastLineNumberBeforeDeprecation() internal unused method
  Make sure to collect child forms created on *_SET_DATA events
  [WebProfilerBundle] Improve display in Email panel for dark theme
  do not render errors for checkboxes twice
nicolas-grekas added a commit that referenced this issue Oct 28, 2019
@nicolas-grekas
Merge branch '4.4'
5a85540 
* 4.4:
  [OptionsResolve] Revert change in tests for a not-merged change in code
  [HttpClient] fix handling of 3xx with no Location header - ignore Content-Length when no body is expected
  [Workflow] Made the configuration more robust for the 'property' key
  [Security/Core] make NativePasswordEncoder use sodium to validate passwords when possible
  [FrameworkBundle] make SodiumVault report bad decryption key accurately
  cs fix
  [Security] Allow to set a fixed algorithm
  [Security/Core] make encodedLength computation more generic
  [Security/Core] add fast path when encoded password cannot match anything
  #30432 fix an error message
  fix paths to detect code owners
  [HttpClient] ignore the body of responses to HEAD requests
  [Validator] Ensure numeric subpaths do not cause errors on PHP 7.4
  [SecurityBundle] Fix wrong assertion
  Remove unused local variables in tests
  [Yaml][Parser] Remove the getLastLineNumberBeforeDeprecation() internal unused method
  Make sure to collect child forms created on *_SET_DATA events
  [WebProfilerBundle] Improve display in Email panel for dark theme
  do not render errors for checkboxes twice
PhilETaylor pushed a commit to PhilETaylor/symfony that referenced this issue Sep 6, 2023 
* 4.4:
  [OptionsResolve] Revert change in tests for a not-merged change in code
  [HttpClient] fix handling of 3xx with no Location header - ignore Content-Length when no body is expected
  [Workflow] Made the configuration more robust for the 'property' key
  [Security/Core] make NativePasswordEncoder use sodium to validate passwords when possible
  [FrameworkBundle] make SodiumVault report bad decryption key accurately
  cs fix
  [Security] Allow to set a fixed algorithm
  [Security/Core] make encodedLength computation more generic
  [Security/Core] add fast path when encoded password cannot match anything
  symfony#30432 fix an error message
  fix paths to detect code owners
  [HttpClient] ignore the body of responses to HEAD requests
  [Validator] Ensure numeric subpaths do not cause errors on PHP 7.4
  [SecurityBundle] Fix wrong assertion
  Remove unused local variables in tests
  [Yaml][Parser] Remove the getLastLineNumberBeforeDeprecation() internal unused method
  Make sure to collect child forms created on *_SET_DATA events
  [WebProfilerBundle] Improve display in Email panel for dark theme
  do not render errors for checkboxes twice
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Assignees
No one assigned
Labels
Bug OptionsResolver Status: Needs Review
Projects
None yet
Milestone
No milestone
Development

No branches or pull requests
4 participants
@xabbuh @steevanb @dimabory @carsonbot
0