Adjust sniffs to avoid false positive / breaking changes in Views code

[Crabcyborg]

These address some issues I'm seeing applying sniffs in Views.

Summary by CodeRabbit

• Bug Fixes

  • Safer empty-ternary simplification: only applied when the variable is definitely set.
  • Avoided applying isset/empty fixes when the checked expression uses array access.
  • Improved detection when moving variable assignments so closures/inline functions are skipped correctly.

• Behavior Change

  • Return-simplification now treats inline ternaries as complex, reducing automatic if→ternary conversions.

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

[coderabbitai]

📝 Walkthrough

Walkthrough

Adds defensive checks to multiple sniffs: ensure SimplifyEmptyTernary only rewrites when the variable is definitely set; avoid fixes when isset/empty involve array-access; and mark inline-ternary tokens as complex for SimplifyIfReturn to prevent unsafe simplifications.

Changes

Cohort / File(s)	Summary
SimplifyEmptyTernary & helpers phpcs-sniffs/Formidable/Sniffs/CodeAnalysis/SimplifyEmptyTernarySniff.php	Add isVariableDefinitelySet and helpers: findContainingFunction, isFunctionParameter, isUnconditionallyAssigned, isAtFunctionTopLevel. Process short-circuits when variable presence is uncertain.
Array-access guards phpcs-sniffs/Formidable/Sniffs/CodeAnalysis/RedundantEmptyAfterTypeCheckSniff.php, phpcs-sniffs/Formidable/Sniffs/CodeAnalysis/RedundantIssetBeforeNotEmptySniff.php	Add hasArrayAccess detection and early-return guards to avoid applying fixes when arguments contain array-index expressions (e.g., $arr['key']).
MoveVariableBelowEarlyReturn: assignment end finder phpcs-sniffs/Formidable/Sniffs/CodeAnalysis/MoveVariableBelowEarlyReturnSniff.php	Replace direct semicolon lookup with findAssignmentEnd(...) that skips closure/arrow-function bodies when locating the assignment terminator.
SimplifyIfReturn complexity tokens phpcs-sniffs/Formidable/Sniffs/CodeAnalysis/SimplifyIfReturnSniff.php	Treat T_INLINE_THEN and T_INLINE_ELSE as complex tokens so returns containing inline-ternaries are not simplified by this sniff.

Sequence Diagram(s)

(Skipped — changes are intra-sniff control-flow and helpers, not multi-component sequential flows.)

Estimated code review effort

🎯 3 (Moderate) | ⏱️ ~25 minutes

Possibly related PRs

• Add sniffs to catch redundant isset checks #2792: Updates RedundantIssetBeforeNotEmptySniff and related isset/empty sniffs with array-access guards — strongly related.
• New sniff to flip ternaries that use compare value on the right side … #2817: Prior changes to SimplifyEmptyTernarySniff addressing safety checks for variable definitude — strongly related.
• New sniff to simplify not empty checks after a type check #2821: Related modifications to RedundantEmptyAfterTypeCheckSniff adding array-access guards and helpers.

Poem

🐰 I hop through tokens, sniff each tiny part,

I check the params and where assigns start.
If arrays peek in, I step aside,
Safer hops now keep surprises denied. 🥕

🚥 Pre-merge checks | ✅ 3

✅ 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 clearly describes the main objective: adjusting sniffs to avoid false positives and breaking changes in Views code, which aligns with the PR objectives and all file changes.
Docstring Coverage	✅ Passed	Docstring coverage is 100.00% which is sufficient. The required threshold is 80.00%.

_{✏️ 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: 3

🤖 Fix all issues with AI agents

In `@phpcs-sniffs/Formidable/Sniffs/CodeAnalysis/SimplifyEmptyTernarySniff.php`:
- Around line 190-197: The phpdoc `@return` union ordering violates
phpdoc_types_order; update the return type order in the docblock for the method
that "Find the containing function for a token." by swapping the union from
"int|false" to "false|int" (i.e., change the `@return` annotation so false comes
before int) to satisfy php-cs-fixer.
- Around line 166-168: The local variable $tokens in the
isVariableDefinitelySet( File $phpcsFile, $stackPtr, $variableName ) method is
declared but never used; remove the unused assignment so the function no longer
creates $tokens from $phpcsFile->getTokens(), leaving the rest of the method
intact and relying on $phpcsFile directly where needed.
- Around line 198-213: The findContainingFunction method currently returns the
first function/closure in $tokens[$stackPtr]['conditions'], which picks the
outermost scope; change it to return the innermost function/closure instead by
selecting the last matching condition key in the $conditions array (e.g.,
iterate conditions in reverse or pick the last key where $conditionType ===
T_FUNCTION || $conditionType === T_CLOSURE) so nested closures are handled
correctly and outer parameters are not treated as defined inside inner closures.

[coderabbitai]

Actionable comments posted: 1

🤖 Fix all issues with AI agents

In `@phpcs-sniffs/Formidable/Sniffs/CodeAnalysis/SimplifyEmptyTernarySniff.php`:
- Around line 304-315: The current loop over $conditions (using
$conditionPtr/$conditionType) only rejects a hardcoded list and misses nested
closures/functions; instead determine the outer function's scope boundaries from
the $functionToken (use $tokens[$functionToken]['scope_opener'] and
['scope_closer'] or the file API to get the function scope) and inside the
foreach return false for any $conditionPtr !== $functionToken whose scope_opener
(or token position) lies strictly inside the function's scope (i.e. nested scope
start > function scope opener), which will reject T_CLOSURE/T_FUNCTION and any
other inner scopes rather than relying on a fixed token list.

♻️ Duplicate comments (2)

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

166-168: Remove unused $tokens.

Line 167 assigns $tokens but it’s never used, which triggers PHPMD.

🧹 Proposed fix

-		$tokens = $phpcsFile->getTokens();

---

198-213: Return the innermost function/closure.

Line 207-210 returns the first matching condition, which is the outermost scope. In nested closures this can incorrectly treat outer parameters as “definitely set.” Prefer the innermost match.

✅ Safer selection (innermost)

-		foreach ( $conditions as $conditionPtr => $conditionType ) {
-			if ( $conditionType === T_FUNCTION || $conditionType === T_CLOSURE ) {
-				return $conditionPtr;
-			}
-		}
-
-		return false;
+		$functionToken = false;
+		foreach ( $conditions as $conditionPtr => $conditionType ) {
+			if ( $conditionType === T_FUNCTION || $conditionType === T_CLOSURE ) {
+				$functionToken = $conditionPtr;
+			}
+		}
+
+		return $functionToken;