Update sniffs to avoid false positive issues in pro

[Crabcyborg]

I'm planning to pull these sniffs into our other plugins so they get checked for every PR, but Pro is changing on some lines that it shouldn't.

Summary by CodeRabbit

• Bug Fixes
  • Enhanced variable assignment detection to accurately distinguish between conditional and unconditional assignments, reducing false positives in redundant empty() call detection
  • Improved code analysis accuracy by preventing false simplification suggestions for ternary operators when statements already contain ternary expressions

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

[coderabbitai]

📝 Walkthrough

Walkthrough

Updates two PHPCS sniffs. One refines detection of unconditional variable assignments by distinguishing conditional contexts and adds helper methods for block ownership/conditional checks. The other adds a pre-check to skip processing when a ternary already exists in a statement, introducing a helper to detect ternaries.

Changes

Cohort / File(s)	Summary
Unconditional assignment detection refinement phpcs-sniffs/Formidable/Sniffs/CodeAnalysis/RedundantEmptyOnAssignedVariableSniff.php	Reworks assignment evaluation to distinguish unconditional vs conditional assignments; adds isInsideConditionalBlock(...) and findBraceOwner(...); adjusts traversal to match statement/assignment levels; returns false when only conditional assignments exist.
Ternary pre-check in dual-condition sniff phpcs-sniffs/Formidable/Sniffs/CodeAnalysis/SimplifyDualConditionToTernarySniff.php	Adds pre-check to skip when statement already contains a ternary; introduces statementContainsTernary(...) to locate ternary tokens within the statement bounds.

Estimated code review effort

🎯 4 (Complex) | ⏱️ ~55 minutes

Possibly related PRs

• Update redundant empty sniff to also catch conditions with && and || #2758 — Also modifies RedundantEmptyOnAssignedVariableSniff for conditional/boolean contexts, overlapping with assignment context handling.
• New sniff to simplify ternaries that check the opposite thing in the … #2796 — Introduces/adjusts SimplifyDualConditionToTernarySniff; directly related to the new ternary pre-check logic.

Suggested reviewers

• AbdiTolesa

Poem

I nibble through code with whiskered delight,
Sniffs now discern what’s wrong and what’s right.
Ifs and elses, I trace every trail,
Ternaries spotted—no duplicate tale.
Ears up, paws down, I hop through the diff—
Clean paths ahead, a tidy code sniff! 🐇✨

🚥 Pre-merge checks | ✅ 2 | ❌ 1

❌ Failed checks (1 inconclusive)

Check name	Status	Explanation	Resolution
Title check	❓ Inconclusive	The title is vague and uses non-descriptive phrasing ('update sniffs', 'avoid false positive issues') that doesn't clearly convey which sniffs are being updated or what specific false positives are being fixed.	Make the title more specific by mentioning the affected sniffs (e.g., 'Fix conditional assignment checks in RedundantEmpty and SimplifyDualCondition sniffs') or the specific issues being resolved.

✅ Passed checks (2 passed)

Check name	Status	Explanation
Docstring Coverage	✅ Passed	Docstring coverage is 100.00% which is sufficient. The required threshold is 80.00%.
Description Check	✅ Passed	Check skipped - CodeRabbit’s high-level summary is enabled.

_{✏️ Tip: You can configure your own custom pre-merge checks in the settings.}

✨ Finishing touches

• 📝 Generate docstrings

---

Thanks for using CodeRabbit! It's free for OSS, and your support helps us grow. If you like it, consider giving us a shout-out.

❤️ Share

• X
• Mastodon
• Reddit
• LinkedIn

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

[coderabbitai]

Actionable comments posted: 0

Caution

Some comments are outside the diff and can’t be posted inline due to platform limitations.

⚠️ Outside diff range comments (1)

phpcs-sniffs/Formidable/Sniffs/CodeAnalysis/RedundantEmptyOnAssignedVariableSniff.php (1)

366-407: Remove unused variable $hasAnyAssignment.

The variable is set at line 386 but never read. The return statement at line 407 only uses $hasUnconditionalAssignment. The comment suggests intended usage, but the implementation doesn't use it.

🧹 Proposed fix

-		$hasUnconditionalAssignment = false;
-		$hasAnyAssignment           = false;
+		$hasUnconditionalAssignment = false;

 		// Search from the function start to the statement.
 		for ( $i = $scopeOpener + 1; $i < $statementToken; $i++ ) {
 			if ( $tokens[ $i ]['code'] !== T_VARIABLE ) {
 				continue;
 			}

 			if ( $tokens[ $i ]['content'] !== $variableName ) {
 				continue;
 			}

 			// Check if this variable is being assigned (has = after it).
 			$nextToken = $phpcsFile->findNext( T_WHITESPACE, $i + 1, null, true );

 			if ( false === $nextToken || $tokens[ $nextToken ]['code'] !== T_EQUAL ) {
 				continue;
 			}

-			$hasAnyAssignment = true;
-
 			// Check if the assignment is at the same scope level as the statement.

📜 Review details

Configuration used: Path: .coderabbit.yaml

Review profile: CHILL

Plan: Pro

📥 Commits

Reviewing files that changed from the base of the PR and between 75c6fbc and 976424a.

📒 Files selected for processing (2)

• phpcs-sniffs/Formidable/Sniffs/CodeAnalysis/RedundantEmptyOnAssignedVariableSniff.php
• phpcs-sniffs/Formidable/Sniffs/CodeAnalysis/SimplifyDualConditionToTernarySniff.php

🧰 Additional context used

📓 Path-based instructions (1)

**/*.php

⚙️ CodeRabbit configuration file

**/*.php: - Do not make suggestions when we use x.x for a version placeholder. This is used like "@SInCE x.x" in a PHP doc comment, but also like 'x.x' when functions like _deprecated_function() are used.

• Treat these placeholders as acceptable in this codebase.

Files:

• phpcs-sniffs/Formidable/Sniffs/CodeAnalysis/SimplifyDualConditionToTernarySniff.php
• phpcs-sniffs/Formidable/Sniffs/CodeAnalysis/RedundantEmptyOnAssignedVariableSniff.php

🪛 PHPMD (2.15.0)

phpcs-sniffs/Formidable/Sniffs/CodeAnalysis/RedundantEmptyOnAssignedVariableSniff.php

367-367: Avoid unused local variables such as '$hasAnyAssignment'. (undefined)

(UnusedLocalVariable)

⏰ Context from checks skipped due to timeout of 90000ms. You can increase the timeout in your CodeRabbit configuration to a maximum of 15 minutes (900000ms). (6)

• GitHub Check: Cypress
• GitHub Check: PHP 7.4 tests in WP 6.9
• GitHub Check: PHP 8 tests in WP 6.9
• GitHub Check: PHP 8 tests in WP 6.9
• GitHub Check: PHP 7.4 tests in WP 6.9
• GitHub Check: Cypress

🔇 Additional comments (4)

phpcs-sniffs/Formidable/Sniffs/CodeAnalysis/SimplifyDualConditionToTernarySniff.php (2)

113-116: LGTM!

Good addition. The early return when a ternary already exists in the statement prevents the sniff from suggesting a transformation that would create nested or redundant ternaries.

---

487-530: LGTM!

The statementContainsTernary helper correctly identifies statement boundaries and scans for T_INLINE_THEN. The fallback to count( $tokens ) - 1 when no semicolon is found handles edge cases gracefully.

phpcs-sniffs/Formidable/Sniffs/CodeAnalysis/RedundantEmptyOnAssignedVariableSniff.php (2)

419-458: LGTM!

The isInsideConditionalBlock method correctly walks backwards to find if a token is inside an if/else/elseif block. The containment check at line 436 properly handles brace boundaries.

---

460-496: LGTM!

The findBraceOwner method handles brace ownership detection well:

• First checks the standard PHPCS scope_condition attribute
• Falls back to backwards traversal for direct owners (else, do, try, finally)
• Handles parenthesis-based owners via parenthesis_owner

_{✏️ Tip: You can disable this entire section by setting review_details to false in your review settings.}