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

Jump to bottom

FullyQualifiedStrictTypesFixer - fix same classname is imported from … #6197

Merged
merged 1 commit into from Jan 22, 2022
Merged

FullyQualifiedStrictTypesFixer - fix same classname is imported from … #6197

merged 1 commit into from Jan 22, 2022

Conversation

SpacePossum
Copy link
Contributor

@SpacePossum SpacePossum commented Dec 20, 2021 •
edited

…global and non-global namespace

closes #4726

there are multiple issues with this fixer, please have a look at the new logic and tests

@SpacePossum SpacePossum marked this pull request as draft December 20, 2021 10:34
@SpacePossum SpacePossum mentioned this pull request Jan 2, 2022
Closed
@SpacePossum SpacePossum marked this pull request as ready for review January 5, 2022 12:32
SpacePossum
SpacePossum commented Jan 5, 2022
View reviewed changes
tests/Fixer/Import/FullyQualifiedStrictTypesFixerTest.php
Pang\Pung $pangpung,
Pyng\Pung\Pong $pongpyngpangpang,
Bar\Baz\Buz $bazbuz
\Ping\Something $something,
Copy link
Contributor Author

Choose a reason for hiding this comment

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

I think the current fixer is wrong here, removing the leader \ within the current namespace changes the type that is supposedly used here

SpacePossum
SpacePossum commented Jan 5, 2022
View reviewed changes
tests/Fixer/Import/FullyQualifiedStrictTypesFixerTest.php
Pung $other1,
Pang\Pung $other2,
Pyng\Pung\Pong $other3,
\Foo\Bar\Baz\Buz $other4
Copy link
Contributor Author

Choose a reason for hiding this comment

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

same here

@coveralls
Copy link

coveralls commented Jan 5, 2022 •
edited

Coverage Status

Coverage decreased (-0.0008%) to 93.17% when pulling 2e57360 on SpacePossum:master_4726 into 5a463a5 on FriendsOfPHP:master.

@SpacePossum
Copy link
Contributor Author

note to self; make coverage report of the fixer to see if all paths are covered

kubawerlos
kubawerlos requested changes Jan 21, 2022
View reviewed changes
Copy link
Contributor

@kubawerlos kubawerlos left a comment

Choose a reason for hiding this comment

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

Let's add a test case for:

<?php
namespace Foo\Bar;
function test(\Foo\Bar $x) {}

@SpacePossum
Copy link
Contributor Author

nice edge-case, thanks 👍 , patched

@SpacePossum SpacePossum merged commit d0d54d7 into PHP-CS-Fixer:master Jan 22, 2022
@SpacePossum SpacePossum deleted the master_4726 branch January 22, 2022 20:41
@Wirone Wirone mentioned this pull request Aug 20, 2023
Closed
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Reviewers

@kubawerlos kubawerlos kubawerlos approved these changes

Assignees
No one assigned
Labels
kind/bug
Projects
None yet
Milestone
No milestone
Development

Successfully merging this pull request may close these issues.

fully_qualified_strict_types messes up when same classname is imported from global and non-global namespace
3 participants
@SpacePossum @coveralls @kubawerlos