Skip to content

Make constraint violation interfaces stringable #45484

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

Merged
merged 1 commit into from
Mar 9, 2022

Conversation

HypeMC
Copy link
Member

@HypeMC HypeMC commented Feb 19, 2022

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

Currently the ConstraintViolationInterface & ConstraintViolationListInterface don't have a __toString() method even though their only implementations do. Since the validate method returns a ConstraintViolationListInterface trying to cast the errors into a string doesn't sit well with static analysis tools.

Here I've used an example from the docs:
Screen

This PR adds a __toString() method to the interfaces via an @method tag. In Symfony 7 the Stringable interface can be implemented instead.

@carsonbot carsonbot added this to the 6.1 milestone Feb 19, 2022
@HypeMC HypeMC changed the title Make constraint violation interfaces stringable [Validator] Make constraint violation interfaces stringable Feb 19, 2022
@carsonbot
Copy link

Hey!

I think @jschaedl has recently worked with this code. Maybe they can help review this?

Cheers!

Carsonbot

@nicolas-grekas
Copy link
Member

What's the use case that made you open this?

@HypeMC
Copy link
Member Author

HypeMC commented Feb 21, 2022

What's the use case that made you open this?

The use case is casting the errors to a string for dev messages, which should be legit according to the docs, but then static analysis tools complain, which makes sense since there's no guarantee that the implementation being use is stringable. Adding it to the interface would assure that changing the implantation wont break any code and will satisfy static analysis tools.

@HypeMC HypeMC force-pushed the stringable-constraint-violations branch from 5416369 to 9c9408f Compare February 23, 2022 00:12
@ro0NL
Copy link
Contributor

ro0NL commented Feb 23, 2022

im not sure docs should rely on a debugging-string, but rather should show a serialized constraint violation list or some manually constructed payload

@HypeMC HypeMC force-pushed the stringable-constraint-violations branch from 9c9408f to 91bf1b1 Compare March 5, 2022 03:42
@HypeMC HypeMC force-pushed the stringable-constraint-violations branch from 91bf1b1 to 4cead0c Compare March 5, 2022 05: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.

About UPGRADE-7.0.md, it's already outdated. We need to clarify how we deal with this file at the policy level. Shouldn't be a blocker for this PR IMHO.

@carsonbot carsonbot changed the title [Validator] Make constraint violation interfaces stringable Make constraint violation interfaces stringable Mar 8, 2022
@fabpot
Copy link
Member

fabpot commented Mar 9, 2022

Thank you @HypeMC.

@fabpot fabpot merged commit 2ac4503 into symfony:6.1 Mar 9, 2022
@HypeMC HypeMC deleted the stringable-constraint-violations branch March 9, 2022 08:21
@fabpot fabpot mentioned this pull request Apr 15, 2022
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.

6 participants