Skip to content

Make the return-type of ClassReflection#getStartLine() explicit#161

Merged
Ocramius merged 1 commit intolaminas:4.8.xfrom
nicolas-grekas:patch-1
Nov 25, 2022
Merged

Make the return-type of ClassReflection#getStartLine() explicit#161
Ocramius merged 1 commit intolaminas:4.8.xfrom
nicolas-grekas:patch-1

Conversation

@nicolas-grekas
Copy link
Contributor

Q A
Documentation no
Bugfix no
BC Break no
New Feature no
RFC no
QA no

This was removed in #154 but this phpdoc is useful to some static analyzers, so that they can know what the future return-type is going to be.

This was removed in laminas#154 but this phpdoc is useful to some static analyzers, so that they can know what the future return-type is going to be.

Signed-off-by: Nicolas Grekas <nicolas.grekas@gmail.com>
@IonBazan
Copy link
Contributor

If we are aiming for ~8.1.0 || ~8.2.0 for v4.8.0, I think we don't really need to use #[ReturnTypeWillChange] and replace it with native union type there, right?

@Ocramius
Copy link
Member

@IonBazan changing a return type in a non-final class is still a BC break

@Ocramius Ocramius self-assigned this Nov 25, 2022
@Ocramius Ocramius merged commit 84532c3 into laminas:4.8.x Nov 25, 2022
@Ocramius Ocramius changed the title Make the return-type of ClassReflection::getStartLine() explicit Make the return-type of ClassReflection#getStartLine() explicit Nov 25, 2022
@IonBazan
Copy link
Contributor

Noted, let's do that for v5, if you don't mind

@nicolas-grekas nicolas-grekas deleted the patch-1 branch November 25, 2022 09:24
@Ocramius
Copy link
Member

@IonBazan I'd be really happy to make a lot of this stuff final for v5.

Once it's final, it is much much easier to maintain and change internals.

@nicolas-grekas
Copy link
Contributor Author

Thanks for merging quickly!

@Ocramius
Copy link
Member

No problem: just beware that I won't release until @IonBazan's #159 is also merged.

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

Projects

None yet

Development

Successfully merging this pull request may close these issues.

3 participants