Skip to content

Conversation

@Kocal
Copy link
Owner

@Kocal Kocal commented Nov 23, 2025

Q A
Bug fix? no
New feature? yes
Tests pass? yes
Fixed tickets Close #...

@Kocal Kocal self-assigned this Nov 23, 2025
@Kocal Kocal requested a review from Copilot November 23, 2025 12:52
@Kocal Kocal force-pushed the ExposePublicPropsShouldBeFalseRule branch from 05bb612 to 25f0c74 Compare November 23, 2025 12:53
Copilot finished reviewing on behalf of Kocal November 23, 2025 12:55
Copy link

Copilot AI left a comment

Choose a reason for hiding this comment

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

Pull request overview

This pull request adds a new PHPStan rule ExposePublicPropsShouldBeFalseRule to enforce that Twig components explicitly set exposePublicProps: false in the #[AsTwigComponent] attribute. This promotes explicit control over which properties are exposed to Twig templates, improving security and maintainability of Symfony UX Twig components.

Key Changes:

  • Adds new PHPStan rule that validates the exposePublicProps parameter in #[AsTwigComponent] attributes
  • Includes comprehensive test coverage with fixtures for various scenarios
  • Adds documentation to README following the established pattern for rule documentation

Reviewed changes

Copilot reviewed 8 out of 8 changed files in this pull request and generated 3 comments.

Show a summary per file
File Description
src/Rules/TwigComponent/ExposePublicPropsShouldBeFalseRule.php New rule implementation that checks if exposePublicProps is explicitly set to false in #[AsTwigComponent] attributes
tests/Rules/TwigComponent/ExposePublicPropsShouldBeFalseRule/ExposePublicPropsShouldBeFalseRuleTest.php Test class with separate methods for violations and no-violations scenarios
tests/Rules/TwigComponent/ExposePublicPropsShouldBeFalseRule/config/configured_rule.neon Configuration file for test setup
tests/Rules/TwigComponent/ExposePublicPropsShouldBeFalseRule/Fixture/NotAComponent.php Fixture for testing that non-component classes are ignored
tests/Rules/TwigComponent/ExposePublicPropsShouldBeFalseRule/Fixture/ComponentWithoutExposePublicProps.php Fixture for testing violation when exposePublicProps is not set
tests/Rules/TwigComponent/ExposePublicPropsShouldBeFalseRule/Fixture/ComponentWithExposePublicPropsTrue.php Fixture for testing violation when exposePublicProps is set to true
tests/Rules/TwigComponent/ExposePublicPropsShouldBeFalseRule/Fixture/ComponentWithExposePublicPropsFalse.php Fixture for testing valid case where exposePublicProps is set to false
README.md Documentation for the new rule with examples showing violations and correct usage

💡 Add Copilot custom instructions for smarter, more guided reviews. Learn how to get started.

RuleErrorBuilder::message('The #[AsTwigComponent] attribute must have its "exposePublicProps" parameter set to false.')
->identifier('symfonyUX.twigComponent.exposePublicPropsShouldBeFalse')
->line($asTwigComponent->getLine())
->tip('Add "exposePublicProps: false" to the #[AsTwigComponent] attribute.')
Copy link

Copilot AI Nov 23, 2025

Choose a reason for hiding this comment

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

The tip message "Add" is misleading when exposePublicProps is already set to true. Consider using "Set" instead of "Add" to cover both cases (when the parameter is missing and when it's set to true).

Suggested change:

->tip('Set "exposePublicProps" to false in the #[AsTwigComponent] attribute.')
Suggested change
->tip('Add "exposePublicProps: false" to the #[AsTwigComponent] attribute.')
->tip('Set "exposePublicProps" to false in the #[AsTwigComponent] attribute.')

Copilot uses AI. Check for mistakes.
@Kocal Kocal force-pushed the ExposePublicPropsShouldBeFalseRule branch from 25f0c74 to 8dad96c Compare November 23, 2025 13:03
@Kocal Kocal merged commit 82ed246 into main Nov 23, 2025
8 checks passed
@Kocal Kocal deleted the ExposePublicPropsShouldBeFalseRule branch November 23, 2025 13:04
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.

2 participants