Skip to content

Navigation Menu

Sign in

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
Appearance settings

[Uid] Add @return non-empty-string annotations to AbstractUid 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.

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
Dec 7, 2024
Merged

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

merged 1 commit into from
Dec 7, 2024

Conversation

niravpateljoin
Copy link
Contributor

@niravpateljoin niravpateljoin commented Dec 3, 2024 •
edited by derrabus
Loading

Q A
Branch? 7.3
Bug fix? no
New feature? yes
Deprecations? no
Issue Fix #59076
License MIT

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.

@carsonbot carsonbot added this to the 7.3 milestone Dec 3, 2024
@carsonbot
Copy link

Hey!

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

Symfony has a contribution guide which I suggest you to read.

In short:

  • Always add tests
  • Keep backward compatibility (see https://symfony.com/bc).
  • Bug fixes must be submitted against the lowest maintained branch where they apply (see https://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

@xabbuh xabbuh added the Uid label Dec 3, 2024
@carsonbot carsonbot changed the title Add @return non-empty-string annotations to AbstractUid and relevant … [Uid] Add @return non-empty-string annotations to AbstractUid and relevant … Dec 3, 2024
@derrabus derrabus 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 functions Dec 3, 2024
@niravpateljoin niravpateljoin requested a review from xabbuh December 5, 2024 03:30
@xabbuh xabbuh force-pushed the feature/uid-non-empty-string-annotations-7.3 branch from 20bd796 to 41dacf7 Compare December 7, 2024 08:13
@xabbuh xabbuh mentioned this pull request Dec 7, 2024
Merged
@OskarStark OskarStark changed the title [Uid] Add @return non-empty-string annotations to AbstractUid and relevant functions [Uid] Add @return non-empty-string annotations to AbstractUid and relevant functions Dec 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 commented Dec 7, 2024 •
edited
Loading

The diff patch file located in github/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

derrabus added a commit that referenced this pull request Dec 7, 2024
@derrabus
feature #59114 [ErrorHandler] support non-empty-string/non-empty-list…
b7ed0a4 
… 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       | MIT

see the failures in #59075

Commits
-------

01c78b3 support non-empty-string/non-empty-list when patching return types
@derrabus
Copy link
Member

derrabus commented Dec 7, 2024

Thank you @niravpateljoin.

@derrabus derrabus merged commit 12ff1bf into symfony:7.3 Dec 7, 2024
7 of 10 checks passed
@fabpot fabpot mentioned this pull request May 2, 2025
Merged
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Reviewers

@alexandre-daubois alexandre-daubois alexandre-daubois left review comments

@OskarStark OskarStark OskarStark approved these changes

@derrabus derrabus derrabus approved these changes

@xabbuh xabbuh xabbuh approved these changes

@nicolas-grekas nicolas-grekas Awaiting requested review from nicolas-grekas

Assignees
No one assigned
Labels
Feature Status: Reviewed Uid
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
Morty Proxy This is a proxified and sanitized view of the page, visit original site.