Movatterモバイル変換

[0]ホーム
URL:

Skip to content

Navigation Menu

Sign in
Appearance settings

Search code, repositories, users, issues, pull requests...

Provide feedback

We read every piece of feedback, and take your input very seriously.

Saved searches

Use saved searches to filter your results more quickly

 Sign up
Appearance settings

[Uid] Add@return non-empty-string annotations toAbstractUid and relevant functions#59075

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

Conversation

niravpateljoin
Copy link
Contributor

@niravpateljoinniravpateljoin commentedDec 3, 2024•
edited by derrabus
Loading

QA
Branch?7.3
Bug fix?no
New feature?yes
Deprecations?no
IssueFix#59076
LicenseMIT

This Merge Request introduces@return non-empty-string annotations to the AbstractUid class and other relevant functions in the Symfony UID component. By explicitly defining the return type as non-empty-string, this change improves type safety and ensures stricter static analysis when working with UIDs.

Rationale

  • Symfony 7.3 has adopted stricter type safety guidelines, including support for non-empty-string in interfaces like UserInterface::getUserIdentifier. Aligning the UID component with these standards improves consistency across the framework.
  • Adding these annotations benefits developers by providing enhanced autocompletion and error detection in IDEs and tools like Psalm and PHPStan.

Changes Made

  • Updated PHPDoc comments in AbstractUid and related classes to specify@return non-empty-string for methods where applicable.
  • Verified all usages of these methods to ensure compatibility with the stricter return type.

Backward Compatibility

  • This change does not affect backward compatibility, as it only updates PHPDoc annotations. The runtime behavior of the methods remains the same.

zmitic reacted with heart emoji
@carsonbot
Copy link

Hey!

I see that this is your first PR. That is great! Welcome!

Symfony has acontribution guide which I suggest you to read.

In short:

  • Always add tests
  • Keep backward compatibility (seehttps://symfony.com/bc).
  • Bug fixes must be submitted against the lowest maintained branch where they apply (seehttps://symfony.com/releases)
  • Features and deprecations must be submitted against the 7.3 branch.

Review the GitHub status checks of your pull request and try to solve the reported issues. If some tests are failing, try to see if they are failing because of this change.

When two Symfony core team members approve this change, it will be merged and you will become an official Symfony contributor!
If this PR is merged in a lower version branch, it will be merged up to all maintained branches within a few days.

I am going to sit back now and wait for the reviews.

Cheers!

Carsonbot

@xabbuhxabbuh added the Uid labelDec 3, 2024
@carsonbotcarsonbot changed the titleAdd @return non-empty-string annotations to AbstractUid and relevant …[Uid] Add @return non-empty-string annotations to AbstractUid and relevant …Dec 3, 2024
@derrabusderrabus changed the title[Uid] Add @return non-empty-string annotations to AbstractUid and relevant …[Uid] Add@return non-empty-string annotations to AbstractUid and relevant functionsDec 3, 2024
@xabbuhxabbuhforce-pushed thefeature/uid-non-empty-string-annotations-7.3 branch from20bd796 to41dacf7CompareDecember 7, 2024 08:13
@OskarStarkOskarStark changed the title[Uid] Add@return non-empty-string annotations to AbstractUid and relevant functions[Uid] Add@return non-empty-string annotations toAbstractUid and relevant functionsDec 7, 2024
@OskarStark
Copy link
Contributor

the error looks related:
CleanShot 2024-12-07 at 11 49 20@2x

any clue@derrabus ?

@alexandre-daubois
Copy link
Member

alexandre-daubois commentedDec 7, 2024•
edited
Loading

The diff patch file located ingithub/expected-missing-return-types.diff should be updated as well because the newly added annotation should appear in it I guess?

The command to generate the diff again should be at the top of the file

@OskarStark
Copy link
Contributor

Needs

alexandre-daubois reacted with thumbs up emoji

derrabus added a commit that referenced this pull requestDec 7, 2024
… when patching return types (xabbuh)This PR was merged into the 7.3 branch.Discussion----------[ErrorHandler] support non-empty-string/non-empty-list when patching return types| Q             | A| ------------- | ---| Branch?       | 7.3| Bug fix?      | no| New feature?  | yes| Deprecations? | no| Issues        || License       | MITsee the failures in#59075Commits-------01c78b3 support non-empty-string/non-empty-list when patching return types
@derrabus
Copy link
Member

Thank you@niravpateljoin.

@derrabusderrabus merged commit12ff1bf intosymfony:7.3Dec 7, 2024
7 of 10 checks passed
@fabpotfabpot mentioned this pull requestMay 2, 2025
Sign up for freeto join this conversation on GitHub. Already have an account?Sign in to comment
Reviewers

@alexandre-dauboisalexandre-dauboisalexandre-daubois left review comments

@OskarStarkOskarStarkOskarStark approved these changes

@derrabusderrabusderrabus approved these changes

@xabbuhxabbuhxabbuh approved these changes

@nicolas-grekasnicolas-grekasAwaiting requested review from nicolas-grekas

Assignees
No one assigned
Projects
None yet
Milestone
7.3
Development

Successfully merging this pull request may close these issues.

Add @return non-empty-string annotation to AbstractUid methods for better user identifier support
7 participants
@niravpateljoin@carsonbot@OskarStark@alexandre-daubois@derrabus@nicolas-grekas@xabbuh
[8]ページ先頭
©2009-2025 Movatter.jp