Skip to content

SONARPHP-1552 Do not raise S3011 on test files - #1798

Merged
malte-skoruppa-sonarsource merged 4 commits into
masterfrom
gb/SONARPHP-1552
Sep 10, 2026
Merged

SONARPHP-1552 Do not raise S3011 on test files#1798
malte-skoruppa-sonarsource merged 4 commits into
masterfrom
gb/SONARPHP-1552

Conversation

@guillem-bartrina-sonarsource

@guillem-bartrina-sonarsource guillem-bartrina-sonarsource commented Sep 8, 2026

Copy link
Copy Markdown
Contributor

Summary by Gitar

  • Rule updates:
    • Exclude test files from raising S3011 warnings

This will update automatically on new commits.

@hashicorp-vault-sonar-prod

hashicorp-vault-sonar-prod Bot commented Sep 8, 2026

Copy link
Copy Markdown
Contributor

SONARPHP-1552

Comment thread sonar-php-plugin/src/test/java/org/sonar/plugins/php/PHPSensorTest.java Outdated
@gitar-bot

gitar-bot Bot commented Sep 9, 2026

Copy link
Copy Markdown
Code Review ✅ Approved 2 resolved / 2 findings

Excludes test files from raising S3011 warnings. Test coverage confirms the rule still triggers outside test-like paths. No issues found.

✅ 2 resolved
Bug: Ruling expected issues under test paths not updated for S3011

📄 php-checks/src/main/java/org/sonar/php/checks/security/ChangingAccessibilityCheck.java:31
AnalysisScanner filters issues from checks implementing TestFileExcludedCheck for files matching /test/, /tests/, /Tests/ and /spec/ (AnalysisScanner.java:92,226-233), and the ruling ITs run without sonar.tests so the heuristic is active (RulingHelper.java:90). Five expected-issue files still list S3011 issues on test-like paths — PHPMailer:test/, PHPWord:tests/PhpWordTests/Style/AbstractStyleTest.php:107, PHP_CodeSniffer:tests/Core/Config/ReportWidthTest.php:297-299, PhpSpreadsheet:tests/PhpSpreadsheetTests/Writer/Xls/WorkbookTest.php:47-78, psysh:test/Input/ShellInputTest.php:104-105 and psysh:test/ShellTest.php:632 — so LITS will report these as missing issues and the ruling integration test will fail. Remove those entries (deleting php-S3011.json for PHPMailer and PHP_CodeSniffer, which become empty) and keep only non-test paths such as PHPWord:src/, PhpSpreadsheet:infra/, psysh:src/, RubixML:src/** and flysystem:src/Ftp/FtpAdapterTest.php.

Quality: No test asserts S3011 still raises outside test-like paths

📄 sonar-php-plugin/src/test/java/org/sonar/plugins/php/PHPSensorTest.java:485-491 📄 sonar-php-plugin/src/test/resources/testFileExcludedCheck/tests/ChangingAccessibility.php:10
The new test only asserts that no issue is produced under tests/, which would also pass if the resource never triggered the rule at all (e.g. after a future change to the matcher or the resource). The S2068 case pairs its suppression test with s2068ShouldRaiseIssueOnNonTestPath (PHPSensorTest.java:470-475); add the same positive counterpart for S3011 using a copy of the resource at testFileExcludedCheck/ChangingAccessibility.php. Also, the new resource reflects on Subway::class while declaring MyClass, so the constant it asserts on does not exist in the declared class — use MyClass::class to keep the sample coherent.

Implementation Status ✅ 1 of 1 objectives covered
SONARPHP-1552 - 1 of 1 objectives covered

This PR covers the objective of preventing rule S3011 from being raised on test files.

✅ 1 covered here
  • ✅ Do not raise rule S3011 on test files
Options

Auto-apply is off → Gitar will not commit updates to this branch.
Display: compact → Counting what did not apply, without listing it.

Comment with these commands to change the behavior for this request:

Auto-apply Compact
gitar auto-apply:on         
gitar display:verbose         

Was this helpful? React with 👍 / 👎 | Gitar

@sonarqube-next

sonarqube-next Bot commented Sep 9, 2026

Copy link
Copy Markdown

@asya-vorobeva asya-vorobeva left a comment

Copy link
Copy Markdown

Choose a reason for hiding this comment

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

💯

@malte-skoruppa-sonarsource
malte-skoruppa-sonarsource merged commit 037032e into master Sep 10, 2026
22 checks passed
@malte-skoruppa-sonarsource
malte-skoruppa-sonarsource deleted the gb/SONARPHP-1552 branch September 10, 2026 09:16
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