docs/rules/SER302.md
The condition asks exactly what the queued command already tells you through its return value, so the transaction buys nothing but a round-trip and the risk of aborting.
// flagged
var tran = db.CreateTransaction();
tran.AddCondition(Condition.SetContains(key, member));
_ = tran.SetRemoveAsync(key, member);
await tran.ExecuteAsync();
// suggested
bool removed = await db.SetRemoveAsync(key, member);
SetRemove returns false when the member was not there - which is what the condition was checking.
Applies to SetNotContains + SetAdd, SetContains + SetRemove, SortedSetContains +
SortedSetRemove, HashExists + HashDelete, KeyExists + KeyDelete, and KeyExists + KeyExpire
(EXPIRE already returns false for a missing key). No particular server version is involved: these commands
have always reported this.
This is a bigger change than SER300, which is why it has its own ID: the fix deletes the transaction rather than moving an argument, and the result changes meaning.
tran.Execute() returning false means "the guard did not hold, so nothing ran".false means "it ran, and had no effect".Those usually amount to the same decision, but not always - code that logs, retries, or reports differently
between "someone beat me to it" and "there was nothing to do" needs a second look. The queued Task<T> also
disappears, and CommandFlags must be carried over verbatim.
"a" does not guard a write to member "b"; that
transaction is doing real work, and the rule stays quiet even though the key matches.ListIndexExists + ListSetByIndex. LSET reports an out-of-range index by failing, not by returning
false (ListSetByIndex returns Task, not Task<bool>), so dropping the condition would turn an aborted
transaction into an exception. That is a change in behaviour, not a simplification.Plus everything under when these rules stay quiet - except the one about arguments, which does not apply here. This rule keeps the command exactly as you wrote it and deletes only the condition, so there is nothing it could drop.
See also Transactions.
This rule is a heuristic. It reads your source text - it cannot see your keys, your server, or what you know about the code - so it is deliberately conservative and stays quiet wherever it is unsure. Everything it flags still works, and will keep working: this is a suggestion, not a defect report.
That conservatism is meant to make a false positive rare, not impossible. If you think the rule has flagged something it should not have, please report it, including the transaction as written. A rule that fires on correct code is a bug in the rule - and one that reaches every consumer of the package - so it is worth fixing rather than quietly suppressing.
Reported as a warning, so TreatWarningsAsErrors builds fail until you act on it or turn it down.
<NoWarn>$(NoWarn);SER302</NoWarn>
or locally:
#pragma warning disable SER302