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.

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

Variable sniffs: minor performance tweak #374

Open
wants to merge 1 commit into
base: master
Choose a base branch
from

Conversation

jrfnl
Copy link
Member

@jrfnl jrfnl commented Mar 1, 2024

Description

The AbstractVariableSniff by default listens to all T_VARIABLE, T_DOUBLE_QUOTED_STRING and T_HEREDOC tokens.

The majority of the sniffs extending the AbstractVariableSniff, however, only handle T_VARIABLE tokens and in particular, only handle T_VARIABLE tokens when in an OO scope and nothing more.

This small tweak means these sniffs will no longer "listen" to T_DOUBLE_QUOTED_STRING and T_HEREDOC tokens, which should make them marginally faster, in particular for code bases containing lots of T_DOUBLE_QUOTED_STRING and T_HEREDOC tokens.

It also means that these sniff will no longer be triggered for T_VARIABLE tokens outside of an OO context.

Suggested changelog entry

Small performance improvement for five sniffs.

@jrfnl jrfnl added this to the 3.9.1 milestone Mar 1, 2024
@jrfnl jrfnl force-pushed the feature/variable-sniffs-minor-performance-tweak branch 2 times, most recently from 8e5fb89 to 580a850 Compare March 5, 2024 09:12
@jrfnl
Copy link
Member Author

jrfnl commented Mar 5, 2024

Rebased without changes. Merging once the build passes.

@jrfnl jrfnl modified the milestones: 3.9.1, 3.10.0 Mar 5, 2024
@jrfnl
Copy link
Member Author

jrfnl commented Mar 5, 2024

On second thought, I will delay merging this PR until the 3.10.0 release as there is a, albeit small, chance that this change could break external sniffs which extend one of the sniffs involved in this PR (as they would inherit the change, which may not be the desired behaviour for their sniff).

@jrfnl jrfnl modified the milestones: 3.10.0, 4.0.0 Apr 22, 2024
The `AbstractVariableSniff` by default listens to all `T_VARIABLE`, `T_DOUBLE_QUOTED_STRING` and `T_HEREDOC` tokens.

The majority of the sniffs extending the `AbstractVariableSniff`, however, only handle `T_VARIABLE` tokens and in particular, only handle `T_VARIABLE` tokens when in an OO scope and nothing more.

This small tweak means these sniffs will no longer "listen" to `T_DOUBLE_QUOTED_STRING` and `T_HEREDOC` tokens, which should make them marginally faster, in particular for code bases containing lots of `T_DOUBLE_QUOTED_STRING` and `T_HEREDOC` tokens.

It also means that these sniff will no longer be triggered for `T_VARIABLE` tokens outside of an OO context.
@jrfnl jrfnl force-pushed the feature/variable-sniffs-minor-performance-tweak branch from 580a850 to d42dedd Compare April 23, 2024 21:21
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.

1 participant