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

Remove some redundant code #884

Merged
merged 1 commit into from
Mar 17, 2025
Merged

Conversation

jrfnl
Copy link
Member

@jrfnl jrfnl commented Mar 17, 2025

Description

With the MessageCollector being used in the Ruleset class since PR #857, some guard code as introduced in #99 is no longer needed.

The guard code was in place to prevent the following situation:

  • File under test contained an invalid // phpcs:set annotation which tried to set a non-existent property on a sniff.
  • The File::process() method would see this annotation and call the Ruleset::setSniffProperty() method to set the property.
  • The Ruleset::setSniffProperty() method would throw an exception for the non-existent property.
  • The exception would cause the File class to stop scanning the file, potentially leading to missing errors/warnings.

As the error for setting a non-existent property now no longer leads to an exception on a direct call to the Ruleset::setSniffProperty() method, we also no longer need to catch the exception and handle it from within the File::process() method.

In practice, this means that setting a non-existent property on a sniff via an inline // phpcs:set annotation will now be silently ignored (again) and will not affect the scan of the rest of the file.

This commit reverts #99.

Suggested changelog entry

N/A

With the `MessageCollector` being used in the `Ruleset` class since PR 857, some guard code as introduced in 99 is no longer needed.

The guard code was in place to prevent the following situation:
* File under test contained an invalid `// phpcs:set` annotation which tried to set a non-existent property on a sniff.
* The  `File::process()` method would see this annotation and call the `Ruleset::setSniffProperty()` method to set the property.
* The `Ruleset::setSniffProperty()` method would throw an exception for the non-existent property.
* The exception would cause the `File` class to stop scanning the file, potentially leading to missing errors/warnings.

As the error for setting a non-existent property now no longer leads to an exception on a direct call to the `Ruleset::setSniffProperty()` method, we also no longer need to catch the exception and handle it from within the `File::process()` method.

In practice, this means that setting a non-existent property on a sniff via an inline `// phpcs:set` annotation will now be silently ignored (again) and will not affect the scan of the rest of the file.

This commit reverts 99.
@jrfnl jrfnl merged commit c6a579e into master Mar 17, 2025
61 checks passed
@jrfnl jrfnl deleted the feature/messagecollector-follow-up branch March 17, 2025 14:39
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.

None yet

2 participants