Skip to content

feat: Add constructor visability #542

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 our terms of service and privacy statement. We’ll occasionally send you account related emails.

Already on GitHub? Sign in to your account

Open
wants to merge 2 commits into
base: master
Choose a base branch
from

Conversation

Norbytus
Copy link
Contributor

@Norbytus Norbytus commented Aug 4, 2025

Add visability in constructor meta, and feature mark constructor as private and protected

Description

Checklist

Check the boxes that apply (put an x in the brackets, like [x]). You can also check boxes after the PR is created.

❤️ Thank you for your contribution!

@Norbytus Norbytus force-pushed the feat/modify_constructor_visability branch from 51bb4ad to 8308d26 Compare August 4, 2025 09:33
@Norbytus
Copy link
Contributor Author

Norbytus commented Aug 4, 2025

@Xenira how i can update guide there wrong doc about private,protected, public
Now

#[php(public)]`, `#[php(protected)]` and `#[php(private)]` - Sets the

Should be

#[php(vis = "public")]`, `#[php(vis = "protected")]` and `#[php(vis = "private")]` - Sets the

@Norbytus Norbytus force-pushed the feat/modify_constructor_visability branch 3 times, most recently from d633599 to e360e76 Compare August 4, 2025 14:41
@coveralls
Copy link

coveralls commented Aug 4, 2025

Pull Request Test Coverage Report for Build 16824767628

Details

  • 5 of 21 (23.81%) changed or added relevant lines in 4 files are covered.
  • 1 unchanged line in 1 file lost coverage.
  • Overall coverage decreased (-0.03%) to 27.748%

Changes Missing Coverage Covered Lines Changed/Added Lines %
crates/macros/src/function.rs 0 3 0.0%
src/builders/class.rs 5 8 62.5%
crates/macros/src/impl_.rs 0 4 0.0%
crates/macros/src/parsing.rs 0 6 0.0%
Files with Coverage Reduction New Missed Lines %
crates/macros/src/impl_.rs 1 0.0%
Totals Coverage Status
Change from base Build 16813466135: -0.03%
Covered Lines: 1156
Relevant Lines: 4166

💛 - Coveralls

@Xenira
Copy link
Collaborator

Xenira commented Aug 5, 2025

@Norbytus see here:

- `#[php(public)]`, `#[php(protected)]` and `#[php(private)]` - Sets the visibility of the

@Norbytus
Copy link
Contributor Author

Norbytus commented Aug 5, 2025

@Norbytus see here:

- `#[php(public)]`, `#[php(protected)]` and `#[php(private)]` - Sets the visibility of the

I see that variant, but it's not working)

error: Unknown field: `private`
   --> tests/src/integration/class/mod.rs:152:11
    |
152 |     #[php(private)]
    |           ^^^^^^^

So i think, maybe we update dock.
But anyway, main thing for this PR it's declare __construct as public, private, protected

@Xenira
Copy link
Collaborator

Xenira commented Aug 7, 2025

Ye, docs are probably wrong there.

Copy link
Collaborator

@Xenira Xenira left a comment

Choose a reason for hiding this comment

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

Thanks, looks good. Just some minor code style suggestions.

@Norbytus Norbytus force-pushed the feat/modify_constructor_visability branch 3 times, most recently from 6c25cac to 3adf8ff Compare August 7, 2025 19:55
Add visability in constructor meta, and feature mark constructor as
private and protected
@Norbytus Norbytus force-pushed the feat/modify_constructor_visability branch from 3adf8ff to a676026 Compare August 8, 2025 07:13
@Norbytus Norbytus requested a review from Xenira August 12, 2025 08:47
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
None yet
Projects
None yet
Development

Successfully merging this pull request may close these issues.

3 participants