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

[Feature] Add/update type information for source files #68

Merged
merged 1 commit into from
Mar 27, 2021
Merged

[Feature] Add/update type information for source files #68

merged 1 commit into from
Mar 27, 2021

Conversation

lisachenko
Copy link
Contributor

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

Description

This PR adds/updates type information for source files to fix static analysis errors in dependent libraries. Types has been added to properties and phpDocs only to keep existing method signatures. So nothing should change at all.

@lisachenko lisachenko changed the title Add/update type information for source files [WIP ]Add/update type information for source files Jan 9, 2021
@lisachenko lisachenko force-pushed the fix/invalid-type-hint branch 2 times, most recently from 5f17ebc to 972c121 Compare January 10, 2021 09:50
@lisachenko lisachenko changed the title [WIP ]Add/update type information for source files [Feature] Add/update type information for source files Jan 10, 2021
lisachenko referenced this pull request Jan 10, 2021
@Ocramius
Overhauled #53 to support all known (so far) combinations of new type…
9f2dc66 
…s possible with PHP 8.0.0 in codegen

Signed-Off-By: Marco Pivetta <ocramius@gmail.com>
@lisachenko lisachenko force-pushed the fix/invalid-type-hint branch 2 times, most recently from 4d9c78d to 38f5560 Compare January 11, 2021 09:14
Ocramius
Ocramius approved these changes Jan 24, 2021
View reviewed changes
Copy link
Member

@Ocramius Ocramius left a comment

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Looking good: some minor issues (suspicious potential BC issues), and needs a rebase, but otherwise a really good improvement 👍

test/Generator/ParameterGeneratorTest.php Outdated Show resolved Hide resolved
test/Generator/PropertyGeneratorTest.php Outdated Show resolved Hide resolved
test/Generator/PropertyGeneratorTest.php Outdated Show resolved Hide resolved
test/Generator/PropertyGeneratorTest.php Outdated Show resolved Hide resolved
@lisachenko
Add/update type information for source files
d64a325 
Signed-off-by: Lisachenko Alexander <lisachenko.it@gmail.com>
@lisachenko lisachenko force-pushed the fix/invalid-type-hint branch from 38f5560 to d64a325 Compare March 19, 2021 22:15
@lisachenko
Copy link
Contributor Author

Hey, @Ocramius, could you please check this PR again?

My next PR with changes for probable next major version #69 will depend on this PR, so want to be sure that everything is ok with current PR.

Ocramius
Ocramius approved these changes Mar 27, 2021
View reviewed changes
Copy link
Member

@Ocramius Ocramius left a comment

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

LGTM 👍

@Ocramius Ocramius self-assigned this Mar 27, 2021
@Ocramius Ocramius added this to the 4.1.0 milestone Mar 27, 2021
@Ocramius
Copy link
Member

Overall, the change in protected properties could be considered a BC break, but since the parent class depends on the type being respected, I'm OK with saying that subtypes have to respect it.

@Ocramius Ocramius merged commit a5a506c into laminas:4.1.x Mar 27, 2021
@Ocramius
Copy link
Member

Thanks @lisachenko!

@lisachenko lisachenko deleted the fix/invalid-type-hint branch April 3, 2021 12:26
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Reviewers

@Ocramius Ocramius Ocramius approved these changes

Assignees

@Ocramius Ocramius

Labels
Enhancement
Projects
None yet
Milestone
4.1.0
Development

Successfully merging this pull request may close these issues.
2 participants
@lisachenko @Ocramius