Skip to content

[ObjectMapper] CS fixes #60031

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
Merged
merged 1 commit into from
Mar 25, 2025
Merged

[ObjectMapper] CS fixes #60031

merged 1 commit into from
Mar 25, 2025

Conversation

OskarStark
Copy link
Contributor

Q A
Branch? 7.3
Bug fix? no
New feature? no
Deprecations? no
Issues --
License MIT

@carsonbot carsonbot added this to the 7.3 milestone Mar 24, 2025
@OskarStark OskarStark mentioned this pull request Mar 24, 2025
Merged
alexandre-daubois
alexandre-daubois approved these changes Mar 25, 2025
View reviewed changes
Copy link
Member

@alexandre-daubois alexandre-daubois left a comment

Choose a reason for hiding this comment

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

After Jérôme's comment

OskarStark
OskarStark commented Mar 25, 2025
View reviewed changes
src/Symfony/Component/ObjectMapper/Tests/ObjectMapperTest.php
@@ -100,7 +100,7 @@ public function testHasNothingToMapTo()
{
$this->expectException(MappingException::class);
$this->expectExceptionMessage('Mapping target not found for source "class@anonymous".');
(new ObjectMapper())->map(new class () {});
(new ObjectMapper())->map(new class {});
Copy link
Contributor Author

Choose a reason for hiding this comment

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

Shall we enable:
https://cs.symfony.com/doc/rules/operator/new_with_parentheses.html#example-2

OskarStark
OskarStark commented Mar 25, 2025
View reviewed changes
src/Symfony/Component/ObjectMapper/Tests/ObjectMapperTest.php
{
$this->expectException(MappingTransformException::class);
$this->expectExceptionMessage('Cannot map "stdClass" to a non-object target of type "string".');

$u = new \stdClass;
$u = new \stdClass();
Copy link
Contributor Author

Choose a reason for hiding this comment

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

Shall we enable:
https://cs.symfony.com/doc/rules/operator/new_with_parentheses.html#example-1

OskarStark
OskarStark commented Mar 25, 2025
View reviewed changes
src/Symfony/Component/ObjectMapper/Tests/ObjectMapperTest.php
{
$this->expectException(MappingException::class);
$this->expectExceptionMessage(sprintf('Expected the mapped object to be an instance of "%s" but got "stdClass".', ClassWithoutTarget::class));
$this->expectExceptionMessage(\sprintf('Expected the mapped object to be an instance of "%s" but got "stdClass".', ClassWithoutTarget::class));
Copy link
Contributor Author

Choose a reason for hiding this comment

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

There should also be a CS fixer for this

Copy link
Member

Choose a reason for hiding this comment

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

There is native_function_invocation

Copy link
Contributor Author

Choose a reason for hiding this comment

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

Thanks, didn't found it on the go

@OskarStark
Copy link
Contributor Author

cc @nicolas-grekas and @fabpot regarding the CS settings

@GromNaN
Copy link
Member

GromNaN commented Mar 25, 2025

Thank you @OskarStark.

@GromNaN GromNaN merged commit f803dc1 into symfony:7.3 Mar 25, 2025
4 of 11 checks passed
@OskarStark OskarStark deleted the fix/object-mapper branch March 25, 2025 12:05
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Reviewers

@GromNaN GromNaN GromNaN left review comments

@soyuka soyuka soyuka approved these changes

@xabbuh xabbuh xabbuh approved these changes

@alexandre-daubois alexandre-daubois alexandre-daubois approved these changes

Assignees
No one assigned
Labels
ObjectMapper Status: Reviewed
Projects
None yet
Milestone
7.3
Development

Successfully merging this pull request may close these issues.
6 participants
@OskarStark @GromNaN @soyuka @xabbuh @alexandre-daubois @carsonbot