Skip to content

4.0 | File::addMessage(): do not ignore Internal errors when scanning selectively #98

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

Merged
merged 2 commits into from
Apr 15, 2025

Conversation

jrfnl
Copy link
Member

@jrfnl jrfnl commented Nov 11, 2023

Description

Recreation of upstream PR squizlabs/PHP_CodeSniffer#3915 + rebased against the 4.0 branch:

ErrorSuppressionTest: prevent Internal errors

... about a mismatch in line endings when the code "template" is defined using a heredoc with Linux line endings, while the code snippets being inserted into the code "template" were using line endings matching the OS on which the tests were being run.

File::addMessage(): do not ignore Internal errors when scanning selectively

When either the --sniffs=... CLI parameter is used, or the --exclude=... CLI parameter, the File::addMessage() method bows out when an error is passed which is not for one of the selected sniffs/is for one of the excluded sniffs.

Unfortunately, this "bowing out" did not take Internal errors into account, meaning those were now hidden, while those - IMO - should always be thrown as they generally inform the end-user of something wrong with the file which impacts the scan results (mixed line endings/no code found etc).

Fixed now.

Includes updating two test files to allow for seeing internal errors.

👉🏻 This could be considered a breaking change, though I'm not sure whether it should be. Opinions welcome.

Most typically, this change may impact tests for external standards which don't expect Internal errors.

👆🏻 Note: I've decided to err on the side of caution and have moved this PR to the 4.0 branch now.

Suggested changelog entry

Changed:

  • Internal errors will no longer be suppressed when the --sniffs CLI argument is used.

Types of changes

  • Bug fix (non-breaking change which fixes an issue)
  • New feature (non-breaking change which adds functionality)
  • Breaking change (fix or feature that would cause existing functionality to change)
    • This change is only breaking for integrators, not for external standards or end-users.
  • Documentation improvement

@jrfnl jrfnl added this to the 4.0.0 milestone Nov 11, 2023
@jrfnl jrfnl changed the title File::addMessage(): do not ignore Internal errors when scanning selectively 4.0 | File::addMessage(): do not ignore Internal errors when scanning selectively Dec 2, 2023
@jrfnl jrfnl force-pushed the feature/dont-ignore-internal-errors branch 5 times, most recently from 3cfc530 to 8ac4805 Compare December 7, 2023 16:05
... about a mismatch in line endings when the code "template" is defined using a heredoc with Linux line endings, while the code snippets being inserted into the code "template" were using line endings matching the OS on which the tests were being run.
@jrfnl jrfnl force-pushed the feature/dont-ignore-internal-errors branch from 8ac4805 to 61d18c1 Compare April 15, 2025 15:36
@jrfnl jrfnl changed the base branch from 4.0 to 4.x April 15, 2025 15:39
…ectively

When either the `--sniffs=...` CLI parameter is used, or the `--exclude=...` CLI parameter, the `File::addMessage()` method bows out when an error is passed which is not for one of the selected sniffs/is for one of the excluded sniffs.

Unfortunately, this "bowing out" did not take `Internal` errors into account, meaning those were now hidden, while those should _always_ be thrown as they generally inform the end-user of something seriously wrong (mixed line endings/no code found etc).

Fixed now.

Includes updating four test files to allow for seeing internal errors.

Also includes some dedicated tests to make sure that this doesn't interfere with the ability to silence `Internal` errors from within a ruleset file.
@jrfnl jrfnl force-pushed the feature/dont-ignore-internal-errors branch from 61d18c1 to 0451fd9 Compare April 15, 2025 15:40
@jrfnl jrfnl merged commit 2818c2e into 4.x Apr 15, 2025
52 checks passed
@jrfnl jrfnl deleted the feature/dont-ignore-internal-errors branch April 15, 2025 18:49
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.

4 participants