Skip to content

Conversation

@AlexSkrypnyk
Copy link
Member

@AlexSkrypnyk AlexSkrypnyk commented Dec 18, 2025

Variables like $_static_value are now completely skipped from naming convention checks as they are commonly used for internal/special variables.

Summary by CodeRabbit

  • New Features

    • Variables and parameters that start with a leading underscore are now excluded from naming-convention validation for local variables and function parameters.
  • Tests

    • Added unit tests and fixture cases covering underscore-prefixed parameters and local variables to confirm the new skip behavior.

✏️ Tip: You can customize this high-level summary in your review settings.

@coderabbitai
Copy link

coderabbitai bot commented Dec 18, 2025

📝 Walkthrough

Walkthrough

Adds a helper to detect leading-underscore variable names and updates local-variable and parameter sniff processing to skip variables that start with an underscore; tests and fixtures updated to cover underscore-prefixed parameters and locals.

Changes

Cohort / File(s) Summary
Naming sniff implementations
src/DrevOps/Sniffs/NamingConventions/AbstractVariableNamingSniff.php, src/DrevOps/Sniffs/NamingConventions/LocalVariableNamingSniff.php, src/DrevOps/Sniffs/NamingConventions/ParameterNamingSniff.php
Added protected hasLeadingUnderscore(string $name): bool to the abstract sniff. Added early-exit guards in LocalVariableNamingSniff and ParameterNamingSniff to skip processing for variables/parameters that start with _.
Test fixtures
tests/Fixtures/Valid.php
Added functions and methods using underscore-prefixed parameters and local variables to reflect the new skip behavior.
Unit tests
tests/Unit/AbstractVariableNamingSniffTest.php, tests/Unit/LocalVariableNamingSniffTest.php, tests/Unit/ParameterNamingSniffTest.php
Added testHasLeadingUnderscore and provider in AbstractVariableNamingSniffTest; added cases for underscore-prefixed local variables and parameters in the respective sniff tests.

Estimated code review effort

🎯 3 (Moderate) | ⏱️ ~20–30 minutes

  • Check correct placement of the guard relative to reserved-variable and other existing checks in LocalVariableNamingSniff and ParameterNamingSniff.
  • Verify hasLeadingUnderscore() handles edge cases (single _, multiple leading underscores) and matches test data.
  • Confirm unit tests and fixtures reflect intended skip behavior and do not inadvertently change other naming validations.

Possibly related PRs

Pre-merge checks and finishing touches

✅ Passed checks (3 passed)
Check name Status Explanation
Description Check ✅ Passed Check skipped - CodeRabbit’s high-level summary is enabled.
Title check ✅ Passed The title accurately reflects the main change: adding support to skip variables with leading underscores from naming convention checks.
Docstring Coverage ✅ Passed Docstring coverage is 84.62% which is sufficient. The required threshold is 80.00%.
✨ Finishing touches
  • 📝 Generate docstrings
🧪 Generate unit tests (beta)
  • Create PR with unit tests
  • Post copyable unit tests in a comment
  • Commit unit tests in branch feature/15-skip-underscore-vars

📜 Recent review details

Configuration used: Organization UI

Review profile: ASSERTIVE

Plan: Pro

📥 Commits

Reviewing files that changed from the base of the PR and between bebe37e and bb762db.

📒 Files selected for processing (7)
  • src/DrevOps/Sniffs/NamingConventions/AbstractVariableNamingSniff.php (1 hunks)
  • src/DrevOps/Sniffs/NamingConventions/LocalVariableNamingSniff.php (1 hunks)
  • src/DrevOps/Sniffs/NamingConventions/ParameterNamingSniff.php (1 hunks)
  • tests/Fixtures/Valid.php (2 hunks)
  • tests/Unit/AbstractVariableNamingSniffTest.php (1 hunks)
  • tests/Unit/LocalVariableNamingSniffTest.php (1 hunks)
  • tests/Unit/ParameterNamingSniffTest.php (1 hunks)
🧰 Additional context used
📓 Path-based instructions (3)
**/*.php

📄 CodeRabbit inference engine (CLAUDE.md)

**/*.php: Use declare(strict_types=1); at the top of all PHP files
Use snake_case for local variables and method parameters
Use camelCase for method names and class properties
Use single quotes for strings (double quotes when containing single quotes) in PHP files
Declare strict_types=1 at the top of all PHP files in the project

Files:

  • tests/Unit/ParameterNamingSniffTest.php
  • src/DrevOps/Sniffs/NamingConventions/LocalVariableNamingSniff.php
  • tests/Fixtures/Valid.php
  • tests/Unit/LocalVariableNamingSniffTest.php
  • src/DrevOps/Sniffs/NamingConventions/AbstractVariableNamingSniff.php
  • tests/Unit/AbstractVariableNamingSniffTest.php
  • src/DrevOps/Sniffs/NamingConventions/ParameterNamingSniff.php
**/*

📄 CodeRabbit inference engine (CLAUDE.md)

Files must end with a newline character

Files:

  • tests/Unit/ParameterNamingSniffTest.php
  • src/DrevOps/Sniffs/NamingConventions/LocalVariableNamingSniff.php
  • tests/Fixtures/Valid.php
  • tests/Unit/LocalVariableNamingSniffTest.php
  • src/DrevOps/Sniffs/NamingConventions/AbstractVariableNamingSniff.php
  • tests/Unit/AbstractVariableNamingSniffTest.php
  • src/DrevOps/Sniffs/NamingConventions/ParameterNamingSniff.php
src/DrevOps/Sniffs/**/*Sniff.php

📄 CodeRabbit inference engine (CLAUDE.md)

src/DrevOps/Sniffs/**/*Sniff.php: Place sniff classes in src/DrevOps/Sniffs/ following PHPCS naming conventions (Format: CategoryName/SniffNameSniff.php, e.g., NamingConventions/LocalVariableNamingSniff.php)
Implement the Sniff interface from PHP_CodeSniffer\Sniffs\Sniff in sniff classes
Mark auto-fix code blocks with @codeCoverageIgnore (not testable in unit tests) in sniff classes

Files:

  • src/DrevOps/Sniffs/NamingConventions/LocalVariableNamingSniff.php
  • src/DrevOps/Sniffs/NamingConventions/AbstractVariableNamingSniff.php
  • src/DrevOps/Sniffs/NamingConventions/ParameterNamingSniff.php
🧠 Learnings (11)
📚 Learning: 2025-12-16T22:47:02.675Z
Learnt from: CR
Repo: drevops/phpcs-standard PR: 0
File: CLAUDE.md:0-0
Timestamp: 2025-12-16T22:47:02.675Z
Learning: Create both unit tests and functional tests for sniffs: unit tests for internal method logic using reflection, functional tests for complete PHPCS integration, organized by class hierarchy

Applied to files:

  • tests/Unit/ParameterNamingSniffTest.php
  • tests/Fixtures/Valid.php
  • tests/Unit/LocalVariableNamingSniffTest.php
  • tests/Unit/AbstractVariableNamingSniffTest.php
📚 Learning: 2025-12-16T22:47:02.675Z
Learnt from: CR
Repo: drevops/phpcs-standard PR: 0
File: CLAUDE.md:0-0
Timestamp: 2025-12-16T22:47:02.675Z
Learning: Applies to src/DrevOps/Sniffs/**/*Sniff.php : Mark auto-fix code blocks with `codeCoverageIgnore` (not testable in unit tests) in sniff classes

Applied to files:

  • tests/Unit/ParameterNamingSniffTest.php
  • src/DrevOps/Sniffs/NamingConventions/LocalVariableNamingSniff.php
  • tests/Fixtures/Valid.php
  • tests/Unit/LocalVariableNamingSniffTest.php
  • tests/Unit/AbstractVariableNamingSniffTest.php
  • src/DrevOps/Sniffs/NamingConventions/ParameterNamingSniff.php
📚 Learning: 2025-12-16T22:47:02.675Z
Learnt from: CR
Repo: drevops/phpcs-standard PR: 0
File: CLAUDE.md:0-0
Timestamp: 2025-12-16T22:47:02.675Z
Learning: Create fixture files in `tests/Fixtures/` with intentional violations for testing sniffs

Applied to files:

  • tests/Unit/ParameterNamingSniffTest.php
  • tests/Unit/LocalVariableNamingSniffTest.php
  • tests/Unit/AbstractVariableNamingSniffTest.php
📚 Learning: 2025-12-16T22:47:02.675Z
Learnt from: CR
Repo: drevops/phpcs-standard PR: 0
File: CLAUDE.md:0-0
Timestamp: 2025-12-16T22:47:02.675Z
Learning: Applies to src/DrevOps/Sniffs/**/*Sniff.php : Place sniff classes in `src/DrevOps/Sniffs/` following PHPCS naming conventions (Format: `CategoryName/SniffNameSniff.php`, e.g., `NamingConventions/LocalVariableNamingSniff.php`)

Applied to files:

  • tests/Unit/ParameterNamingSniffTest.php
  • src/DrevOps/Sniffs/NamingConventions/LocalVariableNamingSniff.php
  • tests/Fixtures/Valid.php
  • tests/Unit/LocalVariableNamingSniffTest.php
  • src/DrevOps/Sniffs/NamingConventions/AbstractVariableNamingSniff.php
  • tests/Unit/AbstractVariableNamingSniffTest.php
  • src/DrevOps/Sniffs/NamingConventions/ParameterNamingSniff.php
📚 Learning: 2025-12-16T22:47:02.675Z
Learnt from: CR
Repo: drevops/phpcs-standard PR: 0
File: CLAUDE.md:0-0
Timestamp: 2025-12-16T22:47:02.675Z
Learning: Tests must use PHP reflection to test protected methods in sniff classes

Applied to files:

  • tests/Unit/ParameterNamingSniffTest.php
  • tests/Unit/AbstractVariableNamingSniffTest.php
📚 Learning: 2025-12-16T22:47:02.675Z
Learnt from: CR
Repo: drevops/phpcs-standard PR: 0
File: CLAUDE.md:0-0
Timestamp: 2025-12-16T22:47:02.675Z
Learning: Applies to **/*.php : Use snake_case for local variables and method parameters

Applied to files:

  • tests/Unit/ParameterNamingSniffTest.php
  • src/DrevOps/Sniffs/NamingConventions/LocalVariableNamingSniff.php
  • tests/Fixtures/Valid.php
  • tests/Unit/LocalVariableNamingSniffTest.php
  • src/DrevOps/Sniffs/NamingConventions/AbstractVariableNamingSniff.php
  • tests/Unit/AbstractVariableNamingSniffTest.php
  • src/DrevOps/Sniffs/NamingConventions/ParameterNamingSniff.php
📚 Learning: 2025-12-16T22:47:02.675Z
Learnt from: CR
Repo: drevops/phpcs-standard PR: 0
File: CLAUDE.md:0-0
Timestamp: 2025-12-16T22:47:02.675Z
Learning: Follow error code naming convention: `StandardName.Category.SniffName.ErrorName` (e.g., `DrevOps.NamingConventions.LocalVariableNaming.NotSnakeCase`)

Applied to files:

  • src/DrevOps/Sniffs/NamingConventions/LocalVariableNamingSniff.php
📚 Learning: 2025-12-16T22:47:02.675Z
Learnt from: CR
Repo: drevops/phpcs-standard PR: 0
File: CLAUDE.md:0-0
Timestamp: 2025-12-16T22:47:02.675Z
Learning: Applies to **/*.php : Use camelCase for method names and class properties

Applied to files:

  • tests/Fixtures/Valid.php
  • src/DrevOps/Sniffs/NamingConventions/AbstractVariableNamingSniff.php
📚 Learning: 2025-12-16T22:47:02.675Z
Learnt from: CR
Repo: drevops/phpcs-standard PR: 0
File: CLAUDE.md:0-0
Timestamp: 2025-12-16T22:47:02.675Z
Learning: Use PSR-4 autoloading for both source code and tests with namespace `DrevOps\` for sniff classes and `DrevOps\PhpcsStandard\Tests\` for tests

Applied to files:

  • tests/Fixtures/Valid.php
📚 Learning: 2025-12-16T22:47:02.675Z
Learnt from: CR
Repo: drevops/phpcs-standard PR: 0
File: CLAUDE.md:0-0
Timestamp: 2025-12-16T22:47:02.675Z
Learning: Test files must use `processCode()` helper to simulate PHPCS token processing

Applied to files:

  • tests/Unit/LocalVariableNamingSniffTest.php
📚 Learning: 2025-12-16T22:47:02.675Z
Learnt from: CR
Repo: drevops/phpcs-standard PR: 0
File: CLAUDE.md:0-0
Timestamp: 2025-12-16T22:47:02.675Z
Learning: Applies to src/DrevOps/Sniffs/**/*Sniff.php : Implement the `Sniff` interface from `PHP_CodeSniffer\Sniffs\Sniff` in sniff classes

Applied to files:

  • src/DrevOps/Sniffs/NamingConventions/AbstractVariableNamingSniff.php
🧬 Code graph analysis (3)
src/DrevOps/Sniffs/NamingConventions/LocalVariableNamingSniff.php (1)
src/DrevOps/Sniffs/NamingConventions/AbstractVariableNamingSniff.php (1)
  • hasLeadingUnderscore (80-82)
tests/Unit/AbstractVariableNamingSniffTest.php (1)
src/DrevOps/Sniffs/NamingConventions/LocalVariableNamingSniff.php (1)
  • LocalVariableNamingSniff (16-89)
src/DrevOps/Sniffs/NamingConventions/ParameterNamingSniff.php (1)
src/DrevOps/Sniffs/NamingConventions/AbstractVariableNamingSniff.php (1)
  • hasLeadingUnderscore (80-82)
🪛 PHPMD (2.15.0)
tests/Fixtures/Valid.php

11-11: The variable $_static_value is not named in camelCase. (undefined)

(CamelCaseVariableName)


12-12: The variable $_internalVar is not named in camelCase. (undefined)

(CamelCaseVariableName)


20-20: The variable $_prefixed_param is not named in camelCase. (undefined)

(CamelCaseVariableName)


30-30: The variable $_internal_cache is not named in camelCase. (undefined)

(CamelCaseVariableName)


30-30: Avoid unused local variables such as '$_internal_cache'. (undefined)

(UnusedLocalVariable)


31-31: The variable $_tempValue is not named in camelCase. (undefined)

(CamelCaseVariableName)


31-31: Avoid unused local variables such as '$_tempValue'. (undefined)

(UnusedLocalVariable)


38-40: The parameter $_prefixed_param is not named in camelCase. (undefined)

(CamelCaseParameterName)


39-39: The variable $_prefixed_param is not named in camelCase. (undefined)

(CamelCaseVariableName)

tests/Unit/AbstractVariableNamingSniffTest.php

697-697: Missing class import via use statement (line '697', column '23'). (undefined)

(MissingImport)

🔇 Additional comments (10)
src/DrevOps/Sniffs/NamingConventions/LocalVariableNamingSniff.php (1)

40-43: LGTM! Leading underscore guard correctly implemented.

The guard is properly placed after the reserved variable check and correctly uses the hasLeadingUnderscore helper method to skip variables prefixed with underscores from naming convention validation.

tests/Unit/ParameterNamingSniffTest.php (1)

115-122: LGTM! Comprehensive test coverage for underscore-prefixed parameters.

The test cases correctly verify that parameters with leading underscores (both snake_case and camelCase variants) are skipped from validation and do not raise errors.

src/DrevOps/Sniffs/NamingConventions/ParameterNamingSniff.php (1)

41-44: LGTM! Consistent implementation of leading underscore guard.

The guard correctly skips underscore-prefixed parameters from validation and is consistently placed with the same logic as in LocalVariableNamingSniff.

src/DrevOps/Sniffs/NamingConventions/AbstractVariableNamingSniff.php (1)

67-82: LGTM! Clean and well-documented helper method.

The hasLeadingUnderscore method is correctly implemented using str_starts_with and provides a clear abstraction for detecting underscore-prefixed variables. The comprehensive docblock explains the rationale and use case.

tests/Unit/LocalVariableNamingSniffTest.php (1)

121-128: LGTM! Thorough test coverage for underscore-prefixed local variables.

The test cases properly verify that local variables with leading underscores (both snake_case and camelCase) are skipped from naming convention validation.

tests/Unit/AbstractVariableNamingSniffTest.php (1)

686-722: LGTM! Comprehensive test coverage for leading underscore detection.

The test method and data provider thoroughly exercise the hasLeadingUnderscore helper with a wide range of scenarios including edge cases like single underscore, double underscore, and various positions of underscores.

tests/Fixtures/Valid.php (4)

10-14: LGTM! Fixture correctly demonstrates underscore-prefixed local variables.

The additions properly test that underscore-prefixed local variables are skipped from naming convention checks. The comment clearly explains the intent.


16-21: LGTM! Fixture correctly demonstrates underscore-prefixed parameters.

The new function properly tests that parameters with leading underscores are excluded from naming validation. The docblock clearly documents the expected behavior.


29-32: LGTM! Additional examples for class method contexts.

The underscore-prefixed local variables in the class method demonstrate that the skip behavior works consistently across different scopes.


35-40: LGTM! Class method with underscore-prefixed parameter.

The new method properly demonstrates that underscore-prefixed parameters are skipped in class methods as well as standalone functions.


Comment @coderabbitai help to get the list of available commands and usage tips.

@codecov
Copy link

codecov bot commented Dec 18, 2025

Codecov Report

✅ All modified and coverable lines are covered by tests.
✅ Project coverage is 100.00%. Comparing base (dc77cbf) to head (bb762db).
⚠️ Report is 1 commits behind head on main.
✅ All tests successful. No failed tests found.

Additional details and impacted files
@@            Coverage Diff            @@
##              main       #16   +/-   ##
=========================================
  Coverage   100.00%   100.00%           
=========================================
  Files            6         6           
  Lines          415       420    +5     
=========================================
+ Hits           415       420    +5     

☔ View full report in Codecov by Sentry.
📢 Have feedback on the report? Share it here.

Variables like `$_static_value` are now completely skipped from naming
convention checks as they are commonly used for internal/special variables.
@AlexSkrypnyk AlexSkrypnyk force-pushed the feature/15-skip-underscore-vars branch from bebe37e to bb762db Compare December 18, 2025 04:17
@AlexSkrypnyk AlexSkrypnyk merged commit c710039 into main Dec 18, 2025
11 checks passed
@AlexSkrypnyk AlexSkrypnyk deleted the feature/15-skip-underscore-vars branch December 18, 2025 04:22
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