Skip to content

feat(interface): php_impl_interface macro#621

Merged
ptondereau merged 1 commit intoextphprs:masterfrom
kakserpom:590_interfaces
Jan 29, 2026
Merged

feat(interface): php_impl_interface macro#621
ptondereau merged 1 commit intoextphprs:masterfrom
kakserpom:590_interfaces

Conversation

@kakserpom
Copy link
Copy Markdown
Contributor

@kakserpom kakserpom commented Dec 20, 2025

Implements #590 and #308

@coveralls
Copy link
Copy Markdown

coveralls commented Dec 20, 2025

Pull Request Test Coverage Report for Build 21479162825

Details

  • 16 of 130 (12.31%) changed or added relevant lines in 8 files are covered.
  • 1 unchanged line in 1 file lost coverage.
  • Overall coverage decreased (-0.5%) to 34.529%

Changes Missing Coverage Covered Lines Changed/Added Lines %
src/internal/class.rs 0 2 0.0%
src/builders/class.rs 3 7 42.86%
src/class.rs 0 4 0.0%
src/builders/module.rs 1 6 16.67%
crates/macros/src/lib.rs 0 11 0.0%
crates/macros/src/interface.rs 5 17 29.41%
crates/macros/src/function.rs 7 26 26.92%
crates/macros/src/impl_interface.rs 0 57 0.0%
Files with Coverage Reduction New Missed Lines %
src/builders/class.rs 1 57.82%
Totals Coverage Status
Change from base Build 21478368156: -0.5%
Covered Lines: 1916
Relevant Lines: 5549

💛 - Coveralls

@kakserpom kakserpom force-pushed the 590_interfaces branch 8 times, most recently from e5c61bb to 7febcb0 Compare December 21, 2025 20:22
@kakserpom kakserpom force-pushed the 590_interfaces branch 5 times, most recently from f47b1a1 to 174c959 Compare December 24, 2025 05:31
@kakserpom

This comment was marked as outdated.

@kakserpom kakserpom force-pushed the 590_interfaces branch 2 times, most recently from 4cb1cc0 to 240b4fb Compare December 24, 2025 08:25
@kakserpom kakserpom requested a review from ptondereau January 3, 2026 13:33
@kakserpom
Copy link
Copy Markdown
Contributor Author

@ptondereau @Xenira

Please review/merge.

@kakserpom kakserpom force-pushed the 590_interfaces branch 4 times, most recently from 3d0cdd2 to 1a82750 Compare January 25, 2026 08:29
@kakserpom
Copy link
Copy Markdown
Contributor Author

@ptondereau Rebased and green. This one next?

Copy link
Copy Markdown
Member

@ptondereau ptondereau left a comment

Choose a reason for hiding this comment

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

Nice first draft.

But let me add some suggestions:

  • Can we add a compile-time error if #[php_impl_interface] is used on a non-trait impl?
  • Maybe throw a warning if trait methods aren't also in #[php_impl]
  • We should add an example in the documentation showing cross-crate interface implementation

@kakserpom kakserpom force-pushed the 590_interfaces branch 2 times, most recently from 7cb7add to d3c2b8c Compare January 25, 2026 14:04
ptondereau
ptondereau previously approved these changes Jan 25, 2026
Copy link
Copy Markdown
Member

@ptondereau ptondereau left a comment

Choose a reason for hiding this comment

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

I'll do a final review tomorrow but for now LGTM

@kakserpom kakserpom requested a review from ptondereau January 26, 2026 11:28
Copy link
Copy Markdown
Member

@ptondereau ptondereau left a comment

Choose a reason for hiding this comment

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

One more things 😅

@kakserpom kakserpom force-pushed the 590_interfaces branch 7 times, most recently from f9e0e55 to f733620 Compare January 26, 2026 15:16
@kakserpom
Copy link
Copy Markdown
Contributor Author

One more things 😅

All done, Mr Colombo, but CI is acting weird

@ptondereau
Copy link
Copy Markdown
Member

One more things 😅

All done, Mr Colombo, but CI is acting weird

thank you!
it seems that GH is having some trouble... Let's wait, and I will merge once feedback from CI is green

@kakserpom kakserpom force-pushed the 590_interfaces branch 2 times, most recently from 98e4300 to 9d51536 Compare January 27, 2026 03:02
@kakserpom kakserpom requested a review from ptondereau January 27, 2026 03:20
@ptondereau
Copy link
Copy Markdown
Member

@kakserpom Maybe you might be interested in this #653

@kakserpom
Copy link
Copy Markdown
Contributor Author

@kakserpom Maybe you might be interested in this #653

How is it related?

@kakserpom
Copy link
Copy Markdown
Contributor Author

@ptondereau

image

ptondereau
ptondereau previously approved these changes Jan 29, 2026
Copy link
Copy Markdown
Member

@ptondereau ptondereau left a comment

Choose a reason for hiding this comment

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

LGTM, nitpicks again

@ptondereau
Copy link
Copy Markdown
Member

also 1 rebase and i'll merge ;)

@kakserpom kakserpom requested a review from ptondereau January 29, 2026 13:30
@ptondereau ptondereau merged commit a4a2814 into extphprs:master Jan 29, 2026
65 checks passed
@Xenira Xenira mentioned this pull request Jan 29, 2026
kakserpom added a commit to kakserpom/ext-php-rs that referenced this pull request Feb 5, 2026
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