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.

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

FullyQualifiedStrictTypesFixer - Ignore partial class names which look like FQCNs #4630

Merged
merged 1 commit into from
Jan 7, 2020
Merged

FullyQualifiedStrictTypesFixer - Ignore partial class names which look like FQCNs #4630

merged 1 commit into from
Jan 7, 2020

Conversation

localheinz
Copy link
Member

@localheinz localheinz commented Nov 8, 2019 •
edited

This PR

  • adds a failing test case
  • ignores types when they are not references relative to the root namespace

@localheinz localheinz force-pushed the fix/fully-qualified-strict-types branch 2 times, most recently from 8a1dd4f to f09f402 Compare November 8, 2019 09:40
@localheinz localheinz marked this pull request as ready for review November 8, 2019 09:43
@GrahamCampbell
Copy link
Contributor

Is this ready to be merged?

@localheinz localheinz force-pushed the fix/fully-qualified-strict-types branch from c7d23ab to cbfa444 Compare November 29, 2019 13:49
@SpacePossum
Copy link
Contributor

This looks like a solid fix to me 👍

Just wondering, maybe we could apply it to the TypeShortNameResolver::resolve itself?
So ->resolve($tokens, "\Foo") just returns "\Foo" ?

@localheinz localheinz force-pushed the fix/fully-qualified-strict-types branch from cbfa444 to ad2dca6 Compare December 31, 2019 14:08
@localheinz
Copy link
Member Author

@SpacePossum

Can you explain what you mean?

@localheinz localheinz force-pushed the fix/fully-qualified-strict-types branch 3 times, most recently from fb29fdc to 4a368f8 Compare December 31, 2019 18:33
@SpacePossum
Copy link
Contributor

checking the code again Im not sure what I was thinking 😅

@julienfalque julienfalque added the RTM Ready To Merge label Jan 6, 2020
@SpacePossum SpacePossum added this to the 2.15.6 milestone Jan 6, 2020
@SpacePossum SpacePossum force-pushed the fix/fully-qualified-strict-types branch from bd58838 to 0785928 Compare January 7, 2020 10:44
@SpacePossum
Copy link
Contributor

Thank you @localheinz.

SpacePossum added a commit that referenced this pull request Jan 7, 2020
@SpacePossum
bug #4630 FullyQualifiedStrictTypesFixer - Ignore partial class names…
a8604fe 
… which look like FQCNs (localheinz, SpacePossum)

This PR was squashed before being merged into the 2.15 branch (closes #4630).

Discussion
----------

FullyQualifiedStrictTypesFixer - Ignore partial class names which look like FQCNs

This PR

* [x] adds a failing test case
* [x] ignores types when they are not references relative to the root namespace

Commits
-------

0785928 FullyQualifiedStrictTypesFixer - Ignore partial class names which look like FQCNs
@SpacePossum SpacePossum merged commit 0785928 into PHP-CS-Fixer:2.15 Jan 7, 2020
@localheinz localheinz deleted the fix/fully-qualified-strict-types branch January 7, 2020 10:45
@SpacePossum SpacePossum removed the RTM Ready To Merge label Jan 7, 2020
@localheinz
Copy link
Member Author

Thank you, @GrahamCampbell, @julienfalque, and @SpacePossum!

Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Reviewers

@julienfalque julienfalque julienfalque approved these changes

@GrahamCampbell GrahamCampbell GrahamCampbell approved these changes

@SpacePossum SpacePossum SpacePossum approved these changes

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

Successfully merging this pull request may close these issues.

None yet
4 participants
@localheinz @GrahamCampbell @SpacePossum @julienfalque