fix(model): apply a leading ! in validation conditions to its own clause only (#3662)

$evaluateConditionString() stripped a leading `!` and inverted the result of the
whole remaining string before the top-level `||`/`&&` split ran, so
`!a || b` was evaluated as `!(a || b)` and `!a && b` as `!(a && b)`. CFML
binds `!` tighter than `&&` and `||`.

Only negate when the rest after the `!` is a single clause or a fully wrapped
parenthesised group (no top-level `||`/`&&`); otherwise fall through to the
compound split, where each operand recurses and negates just itself.
`!(a || b)` still negates the whole group.


Claude-Session: https://claude.ai/code/session_018RdXpVKos1AZ24dM4X8wL1

Signed-off-by: Claude <noreply@anthropic.com>
Co-authored-by: Claude <noreply@anthropic.com>

Author: bpamiri

changelog.d/validation-negation.fixed.md

@@ -0,0 +1 @@
+- Model validations: a leading `!` in a compound `condition`/`unless` expression now negates only the clause it prefixes, matching CFML precedence (`!` binds tighter than `&&`, which binds tighter than `||`). Previously `$evaluateConditionString()` stripped the `!` and inverted the whole remaining expression, so `!isNew() || this.force eq 1` was evaluated as `!(isNew() || this.force eq 1)` and the validation ran or was skipped on the wrong records. A negated parenthesised group such as `!(a || b)` still negates the whole group

vendor/wheels/model/validations.cfm

@@ -854,17 +854,28 @@
 	public any function $evaluateConditionString(required string condition) {
 		local.normalized = $normalizeConditionOperators(arguments.condition);
 
-		// A leading `!` negates the whole clause (matching CFML's reading of
-		// `!x`). It cannot be handled further down: `$splitConditionOnOperator()`
+		// A leading `!` negates the single clause (or parenthesised group) it
+		// prefixes. It cannot be handled further down: `$splitConditionOnOperator()`
 		// splits on whitespace, so a negated call with arguments would be torn
 		// into `!StructKeyExists(this,` / `'x')`. Strip it here, evaluate the rest
 		// as a unit, and invert the result.
 		//
+		// `!` binds tighter than `&&`/`||`, so when the rest still has a top-level
+		// `||` or `&&` (`!a || b`) the `!` belongs to the first operand only: fall
+		// through and let the compound split run first — each side recurses back
+		// here and negates just its own clause. `!(a || b)` has no top-level
+		// operator after the `!`, so the whole group is still negated.
+		//
 		// `!=` is already rewritten to ` neq ` by the normalizer above, so any
 		// `!` reaching this point is negation.
-		local.negate = (Left(local.normalized, 1) == "!");
-		if (local.negate) {
-			local.normalized = Trim(Mid(local.normalized, 2, Len(local.normalized)));
+		if (Left(local.normalized, 1) == "!") {
+			local.rest = Trim(Mid(local.normalized, 2, Len(local.normalized)));
+			if (
+				ArrayLen($splitTopLevelCondition(local.rest, "||")) == 1
+				&& ArrayLen($splitTopLevelCondition(local.rest, "&&")) == 1
+			) {
+				return !$evaluateConditionString(local.rest);
+			}
 		}
 
 		// Parenthesised group, e.g. `(a || b) && c`. Unwrap only when the parens
@@ -877,14 +888,13 @@
 			}
 		}
 
-		local.result = $evaluateSingleConditionString(local.normalized);
-		return local.negate ? !local.result : local.result;
+		return $evaluateSingleConditionString(local.normalized);
 	}
 
 	/**
-	 * Evaluates one clause — no leading negation, no surrounding group. Split out
-	 * of `$evaluateConditionString()` so the negation wrapper above can invert the
-	 * finished result instead of the first operand.
+	 * Evaluates one clause or compound expression — no negated single clause, no
+	 * surrounding group. Split out of `$evaluateConditionString()` so the negation
+	 * wrapper above can invert a finished clause instead of its first token.
 	 */
 	public any function $evaluateSingleConditionString(required string condition) {
 		local.normalized = arguments.condition;

vendor/wheels/tests/specs/model/validationsSpec.cfc

@@ -430,6 +430,32 @@ component extends="wheels.WheelsTest" {
 				expect(user.$evaluateConditionString("1 eq 0 || !ListFind('a,b,c', 'z')")).toBeTrue()
 			})
 
+			// `!` binds tighter than `&&`/`||`: a leading `!` negates only the first
+			// operand, not the whole compound. It used to invert the entire
+			// expression, so `!a || b` was evaluated as `!(a || b)`.
+			it("applies a leading ! only to the first operand of an || compound", () => {
+				expect(user.$evaluateConditionString("!StructKeyExists(this, 'username') || 1 eq 1")).toBeTrue()
+				expect(user.$evaluateConditionString("!StructKeyExists(this, 'nonexistent') || 1 eq 0")).toBeTrue()
+				expect(user.$evaluateConditionString("!StructKeyExists(this, 'username') || 1 eq 0")).toBeFalse()
+			})
+
+			it("applies a leading ! only to the first operand of an && compound", () => {
+				expect(user.$evaluateConditionString("!StructKeyExists(this, 'nonexistent') && 1 eq 0")).toBeFalse()
+				expect(user.$evaluateConditionString("!StructKeyExists(this, 'username') && 1 eq 0")).toBeFalse()
+				expect(user.$evaluateConditionString("!StructKeyExists(this, 'nonexistent') && 1 eq 1")).toBeTrue()
+			})
+
+			it("negates a whole parenthesised group", () => {
+				expect(user.$evaluateConditionString("!(StructKeyExists(this, 'username') || 1 eq 0)")).toBeFalse()
+				expect(user.$evaluateConditionString("!(StructKeyExists(this, 'nonexistent') || 1 eq 0)")).toBeTrue()
+				expect(user.$evaluateConditionString("!(1 eq 1) || (1 eq 1)")).toBeTrue()
+			})
+
+			it("negates a mid-expression operand only", () => {
+				expect(user.$evaluateConditionString("1 eq 1 && !StructKeyExists(this, 'username')")).toBeFalse()
+				expect(user.$evaluateConditionString("StructKeyExists(this, 'username') && !ListFind('a,b,c', 'z')")).toBeTrue()
+			})
+
 			// An argument that is not present on the instance resolves to an
 			// empty value rather than throwing, so `||` short-circuits the way
 			// it reads. Property absence is normal on a model instance, and the