Uh oh!
There was an error while loading.Please reload this page.
- Notifications
You must be signed in to change notification settings - Fork9.6k
[TypeInfo] use an EOL-agnostic approach to parse class uses#60909
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 ourterms of service andprivacy statement. We’ll occasionally send you account related emails.
Already on GitHub?Sign in to your account
base:7.2
Are you sure you want to change the base?
Conversation
xabbuh commentedJun 26, 2025
| Q | A |
|---|---|
| Branch? | 7.2 |
| Bug fix? | yes |
| New feature? | no |
| Deprecations? | no |
| Issues | Fix#60906 |
| License | MIT |
This comment was marked as outdated.
This comment was marked as outdated.
| @@ -82,6 +83,22 @@ public function testCollectUses() | |||
| $this->assertEquals($uses,$this->typeContextFactory->createFromReflection(new \ReflectionParameter([DummyWithUses::class,'setCreatedAt'],'createdAt'))->uses); | |||
| } | |||
| publicfunctiontestCollectUsesWindowsLineEndings() | |||
| { | |||
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others.Learn more.
I suggest adding an assertion that theDummyWithUsesWindowsLineEndings.php file contains\r\n, to avoid having someone changing the line endings (by mistake or intentionally) and making this test useless.
This could happen for instance for contributors on Windows using git's newline conversion features (maybe we could even avoid that specific case with a.gitattributes telling git we want Windows line endings in this specific file)
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others.Learn more.
I added both the assertion inside the test as well as entries to the relevant.gitattributes files
e803276 to76ad82bCompare.gitattributes Outdated
| @@ -7,5 +7,6 @@ | |||
| /src/Symfony/Component/Translation/Bridgeexport-ignore | |||
| /src/Symfony/Component/Emoji/Resources/data/*linguist-generated=true | |||
| /src/Symfony/Component/Intl/Resources/data/*/*linguist-generated=true | |||
| /src/Symfony/Component/TypeInfo/Tests/Fixtures/DummyWithUsesWindowsLineEndings.phptexteol=crlf | |||
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others.Learn more.
I don't think we need this ifsrc/Symfony/Component/TypeInfo/.gitattributes already defines it
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others.Learn more.
Are you sure thatsrc/Symfony/Component/TypeInfo/.gitattributes is evaluated when the mono-repo is checked out?
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others.Learn more.
yes. git respects all.gitattributes files, not just the file at the root of the repo (same for.gitignore files)
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others.Learn more.
alright 👍 I reverted this change