[#13] Fixed nullable class properties incorrectly triggering LocalVariableNaming.NotSnakeCase.

[AlexSkrypnyk]

Closes #13

Summary by CodeRabbit

• Bug Fixes

  • Improved detection and handling of PHP 8+ namespaced type tokens in property contexts, including support for nullable properties with fully qualified and namespace-qualified types.

• Tests

  • Expanded test coverage for typed and nullable property detection scenarios.

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

[coderabbitai]

📝 Walkthrough

Walkthrough

The PR addresses a false positive where nullable class properties with fully-qualified namespaced types incorrectly triggered the LocalVariableNaming.NotSnakeCase sniff. The fix expands token recognition in property detection to include PHP 8+ namespaced type tokens (T_NAME_FULLY_QUALIFIED, T_NAME_QUALIFIED, T_NAME_RELATIVE), accompanied by corresponding test fixtures and unit test cases.

Changes

Cohort / File(s)	Summary
Sniff token handling src/DrevOps/Sniffs/NamingConventions/AbstractVariableNamingSniff.php	Expanded token whitelisting in isProperty and isPromotedProperty methods to recognize PHP 8+ namespaced type tokens for proper property context identification
Test fixtures tests/Fixtures/VariableNaming.php	Added three nullable properties with typed declarations (?string, ?\DOMDocument, ?\DateTime) to ClassWithMixedVariableNaming; added promoted nullable property (?\DOMDocument $promotedNullableProperty) to ClassWithPromotedProperties constructor
Unit tests tests/Unit/AbstractVariableNamingSniffTest.php	Expanded isProperty and isPromotedProperty data providers with test cases for typed and nullable properties (simple types, fully qualified, and namespace-qualified forms)

Estimated code review effort

🎯 2 (Simple) | ⏱️ ~10 minutes

• Token additions are straightforward and follow a consistent pattern
• Test additions are homogeneous and predictable
• Primary review focus should be verifying correct token constants are used and no control flow regressions are introduced

Possibly related PRs

• Refactored case sniffs to allow choosing snake or camel case. #12: Modifies the same AbstractVariableNamingSniff class with related changes to property-detection logic
• Revert "Refactored case sniffs to allow choosing snake or camel case." #11: Refactors the same base sniff's property-detection helper methods that this PR extends
• Refactored case sniffs to allow choosing snake or camel case. #10: Also modifies AbstractVariableNamingSniff with structural changes to token and property handling

Pre-merge checks and finishing touches

✅ Passed checks (5 passed)

Check name	Status	Explanation
Description Check	✅ Passed	Check skipped - CodeRabbit’s high-level summary is enabled.
Title check	✅ Passed	The title accurately describes the main fix: addressing false positives for nullable class properties triggering LocalVariableNaming.NotSnakeCase error.
Linked Issues check	✅ Passed	Changes to AbstractVariableNamingSniff correctly expand token whitelisting for PHP 8+ namespaced types in property/promoted property detection [#13], addressing the root cause of the false positive.
Out of Scope Changes check	✅ Passed	All changes are scoped to fixing the nullable property false positive: sniff logic update, test fixtures, and unit tests validating the fix with typed and nullable properties.
Docstring Coverage	✅ Passed	No functions found in the changed files to evaluate docstring coverage. Skipping docstring coverage check.

✨ 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/13-nullable-property-fix

---

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

[codecov]

Codecov Report

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

Additional details and impacted files

@@            Coverage Diff            @@
##              main       #14   +/-   ##
=========================================
  Coverage   100.00%   100.00%           
=========================================
  Files            6         6           
  Lines          409       415    +6     
=========================================
+ Hits           409       415    +6     

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

[coderabbitai]

Actionable comments posted: 1

📜 Review details

Configuration used: Organization UI

Review profile: ASSERTIVE

Plan: Pro

📥 Commits

Reviewing files that changed from the base of the PR and between 08ceac6 and 7312aef.

📒 Files selected for processing (3)

• src/DrevOps/Sniffs/NamingConventions/AbstractVariableNamingSniff.php (2 hunks)
• tests/Fixtures/VariableNaming.php (2 hunks)
• tests/Unit/AbstractVariableNamingSniffTest.php (2 hunks)

🧰 Additional context used

🪛 PHPMD (2.15.0)

tests/Fixtures/VariableNaming.php

74-74: Avoid excessively long variable names like $promotedNullableProperty. Keep variable name length under 20. (undefined)

(LongVariable)

🔇 Additional comments (4)

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

314-317: LGTM! Correct fix for namespaced type handling.

The addition of PHP 8+ namespaced type tokens (T_NAME_FULLY_QUALIFIED, T_NAME_QUALIFIED, T_NAME_RELATIVE) properly extends property detection to handle fully-qualified and namespace-qualified types like \DOMDocument or Some\Type. This directly addresses the false positive reported in issue #13.

---

408-411: LGTM! Consistent token handling for promoted properties.

The same token additions applied to isPromotedProperty ensure consistent behavior for PHP 8.0+ constructor property promotion with namespaced types. This maintains parity with the isProperty fix.

tests/Unit/AbstractVariableNamingSniffTest.php (1)

264-283: LGTM! Comprehensive test coverage for the fix.

The new test cases thoroughly validate property detection for typed and nullable properties across multiple scenarios:

• Simple types with nullable (?string)
• Fully-qualified namespaced types (?\DOMDocument)
• Namespace-qualified types (?Some\Type)
• Promoted property variants

This coverage directly validates the fix for issue #13 and guards against regressions.

Also applies to: 351-360

tests/Fixtures/VariableNaming.php (1)

17-21: LGTM! Fixtures validate nullable property scenarios.

The three new nullable property declarations effectively test the fix for issue #13, covering both simple nullable types (?string) and fully-qualified namespaced nullable types (?\DOMDocument, ?\DateTime).

[coderabbitai]

🧹 Nitpick | 🔵 Trivial

Consider shortening the variable name.

PHPMD flags $promotedNullableProperty as exceeding the recommended 20-character limit (26 characters). While this is test fixture code and the name is descriptive, consider a shorter alternative like $promotedNullableDom or $nullableDom for better readability.

Based on learnings from static analysis hints.

🧰 Tools

🪛 PHPMD (2.15.0)

74-74: Avoid excessively long variable names like $promotedNullableProperty. Keep variable name length under 20. (undefined)

(LongVariable)

🤖 Prompt for AI Agents

In tests/Fixtures/VariableNaming.php around line 74, the property name
$promotedNullableProperty is too long per PHPMD; shorten it (for example to
$promotedNullableDom or $nullableDom) and update every occurrence in this file
(constructor promotion, any references, docblocks) so the type and nullability
remain the same (\DOMDocument|null). Ensure consistency by renaming usages in
related tests/fixtures if referenced elsewhere.