Skip to content

Do not report MA0192 when the HasFlag rewrite reorders an effectful operand #1442

New issue

Have a question about this project? Sign up for a free GitHub account to open an issue and contact its maintainers and the community.

Sign up for GitHub

By clicking “Sign up for GitHub”, you agree to our terms of service and privacy statement. We’ll occasionally send you account related emails.

Already on GitHub? Sign in to your account

Merged
meziantou merged 1 commit into from
Sep 8, 2026
Merged

Do not report MA0192 when the HasFlag rewrite reorders an effectful operand #1442

Show file tree
Hide file tree
Changes from all commits
Commits
Show all changes
1 commit
Select commit
File filter

Filter by extension

Filter by extension
Conversations
Failed to load comments.
Loading
Jump to
Jump to file
Failed to load files.
Loading
Diff view
Diff view
2 changes: 2 additions & 0 deletions docs/Rules/MA0192.md
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -23,6 +23,8 @@ The compared flag doesn't have to be a constant. `(value & flags) == flags` is r

Expressions that may return a different value on each evaluation, such as properties, method calls, or `volatile` fields, are not reported.

When the flag is not a constant, the rewrite must not change the order in which the operands are evaluated. `value.HasFlag(flags)` reads `value` before `flags` and drops the duplicated read of `flags`, so the check is only reported when `value` is already evaluated before every read of `flags`, or when evaluating `value` cannot change `flags`. For instance, `(flags & Compute()) == flags` is not reported, as `Compute()` could assign a new value to `flags`.

For comparisons against `0`, the enum member used in the bitwise `&` must be a single-bit value (for example `1`, `2`, `4`, `8`, ...). Combined values are ignored.

Zero-valued enum members are not reported by this rule. They are covered by [MA0201](https://github.com/meziantou/Meziantou.Analyzer/blob/main/docs/Rules/MA0201.md).
Expand Down
20 changes: 13 additions & 7 deletions src/Meziantou.Analyzer.CodeFixers/Rules/UseHasFlagMethodFixer.cs
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -96,7 +96,7 @@ private static bool TryGetHasFlagPattern(IOperation operation, [NotNullWhen(true
{
if (TryGetComparedOperand(patternOperation, out var comparedOperand, out var negate))
{
pattern = GetFromBitwiseAnd(andOperation, comparedOperand, operationExpression, negate);
pattern = GetFromBitwiseAnd(andOperation, comparedOperand, operationExpression, negate, comparedOperandEvaluatedFirst: false);
return pattern is not null;
}
}
Expand Down Expand Up @@ -137,37 +137,39 @@ private static bool TryGetComparedOperand(IPatternOperation patternOperation, [N

if (leftOperand is IBinaryOperation { OperatorKind: BinaryOperatorKind.And } leftBitwiseAnd)
{
var pattern = GetFromBitwiseAnd(leftBitwiseAnd, rightOperand, operationExpression, negate);
var pattern = GetFromBitwiseAnd(leftBitwiseAnd, rightOperand, operationExpression, negate, comparedOperandEvaluatedFirst: false);
if (pattern is not null)
return pattern;
}

if (rightOperand is IBinaryOperation { OperatorKind: BinaryOperatorKind.And } rightBitwiseAnd)
{
var pattern = GetFromBitwiseAnd(rightBitwiseAnd, leftOperand, operationExpression, negate);
var pattern = GetFromBitwiseAnd(rightBitwiseAnd, leftOperand, operationExpression, negate, comparedOperandEvaluatedFirst: true);
if (pattern is not null)
return pattern;
}

return null;
}

private static HasFlagPattern? GetFromBitwiseAnd(IBinaryOperation bitwiseAndOperation, IOperation comparedOperand, ExpressionSyntax operationExpression, bool negate)
private static HasFlagPattern? GetFromBitwiseAnd(IBinaryOperation bitwiseAndOperation, IOperation comparedOperand, ExpressionSyntax operationExpression, bool negate, bool comparedOperandEvaluatedFirst)
{
var leftOperand = bitwiseAndOperation.LeftOperand.UnwrapImplicitConversions();
var rightOperand = bitwiseAndOperation.RightOperand.UnwrapImplicitConversions();
comparedOperand = comparedOperand.UnwrapImplicitConversions();

if (TryGetEnumFlagReference(rightOperand, comparedOperand, out var flagOperation, out var comparedWithZero) &&
if (TryGetEnumFlagReference(rightOperand, comparedOperand, out var flagOperation, out var comparedWithZero, out var isConstantFlag) &&
IsValidPattern(leftOperand, flagOperation) &&
UseHasFlagMethodCommon.CanRewriteToHasFlag(leftOperand, isConstantFlag, preservesEvaluationOrder: !comparedOperandEvaluatedFirst) &&
leftOperand.Syntax is ExpressionSyntax enumValueExpression &&
flagOperation.Syntax is ExpressionSyntax flagExpression)
{
return new(operationExpression, enumValueExpression, flagExpression, comparedWithZero ? !negate : negate);
}

if (TryGetEnumFlagReference(leftOperand, comparedOperand, out flagOperation, out comparedWithZero) &&
if (TryGetEnumFlagReference(leftOperand, comparedOperand, out flagOperation, out comparedWithZero, out isConstantFlag) &&
IsValidPattern(rightOperand, flagOperation) &&
UseHasFlagMethodCommon.CanRewriteToHasFlag(rightOperand, isConstantFlag, preservesEvaluationOrder: false) &&
rightOperand.Syntax is ExpressionSyntax enumValueExpression2 &&
flagOperation.Syntax is ExpressionSyntax flagExpression2)
{
Expand All @@ -177,7 +179,7 @@ rightOperand.Syntax is ExpressionSyntax enumValueExpression2 &&
return null;
}

private static bool TryGetEnumFlagReference(IOperation potentialFlag, IOperation comparedOperand, [NotNullWhen(true)] out IOperation? flagOperation, out bool comparedWithZero)
private static bool TryGetEnumFlagReference(IOperation potentialFlag, IOperation comparedOperand, [NotNullWhen(true)] out IOperation? flagOperation, out bool comparedWithZero, out bool isConstantFlag)
{
potentialFlag = potentialFlag.UnwrapImplicitConversions();
comparedOperand = comparedOperand.UnwrapImplicitConversions();
Expand All @@ -192,13 +194,15 @@ private static bool TryGetEnumFlagReference(IOperation potentialFlag, IOperation
{
flagOperation = secondFieldReference;
comparedWithZero = false;
isConstantFlag = true;
return true;
}

if (comparedOperand.IsConstantZero() && IsSingleBitSet(firstFieldReference.Field.ConstantValue))
{
flagOperation = firstFieldReference;
comparedWithZero = true;
isConstantFlag = true;
return true;
}
}
Expand All @@ -207,11 +211,13 @@ private static bool TryGetEnumFlagReference(IOperation potentialFlag, IOperation
{
flagOperation = comparedOperand;
comparedWithZero = false;
isConstantFlag = comparedOperand.ConstantValue.HasValue;
return true;
}

flagOperation = null;
comparedWithZero = false;
isConstantFlag = false;
return false;
}

Expand Down
24 changes: 15 additions & 9 deletions src/Meziantou.Analyzer/Rules/UseHasFlagMethodAnalyzer.cs
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -252,7 +252,7 @@ private static bool TryGetHasFlagPattern(IOperation operation, [NotNullWhen(true
{
if (TryGetComparedOperand(patternOperation, out var comparedOperand, out _))
{
pattern = GetFromBitwiseAnd(andOperation, comparedOperand);
pattern = GetFromBitwiseAnd(andOperation, comparedOperand, comparedOperandEvaluatedFirst: false);
return pattern is not null;
}
}
Expand All @@ -268,43 +268,45 @@ private static bool TryGetHasFlagPattern(IOperation operation, [NotNullWhen(true

if (leftOperand is IBinaryOperation { OperatorKind: BinaryOperatorKind.And } leftBitwiseAnd)
{
var pattern = GetFromBitwiseAnd(leftBitwiseAnd, rightOperand);
var pattern = GetFromBitwiseAnd(leftBitwiseAnd, rightOperand, comparedOperandEvaluatedFirst: false);
if (pattern is not null)
return pattern;
}

if (rightOperand is IBinaryOperation { OperatorKind: BinaryOperatorKind.And } rightBitwiseAnd)
{
var pattern = GetFromBitwiseAnd(rightBitwiseAnd, leftOperand);
var pattern = GetFromBitwiseAnd(rightBitwiseAnd, leftOperand, comparedOperandEvaluatedFirst: true);
if (pattern is not null)
return pattern;
}

return null;
}

private static HasFlagPattern? GetFromBitwiseAnd(IBinaryOperation bitwiseAndOperation, IOperation comparedOperand)
private static HasFlagPattern? GetFromBitwiseAnd(IBinaryOperation bitwiseAndOperation, IOperation comparedOperand, bool comparedOperandEvaluatedFirst)
{
var leftOperand = bitwiseAndOperation.LeftOperand.UnwrapImplicitConversions();
var rightOperand = bitwiseAndOperation.RightOperand.UnwrapImplicitConversions();
comparedOperand = comparedOperand.UnwrapImplicitConversions();

if (TryGetEnumFlagReference(rightOperand, comparedOperand, out var flagOperation) &&
IsValidPattern(leftOperand, flagOperation))
if (TryGetEnumFlagReference(rightOperand, comparedOperand, out var flagOperation, out var isConstantFlag) &&
IsValidPattern(leftOperand, flagOperation) &&
UseHasFlagMethodCommon.CanRewriteToHasFlag(leftOperand, isConstantFlag, preservesEvaluationOrder: !comparedOperandEvaluatedFirst))
{
return new(leftOperand, flagOperation);
}

if (TryGetEnumFlagReference(leftOperand, comparedOperand, out flagOperation) &&
IsValidPattern(rightOperand, flagOperation))
if (TryGetEnumFlagReference(leftOperand, comparedOperand, out flagOperation, out isConstantFlag) &&
IsValidPattern(rightOperand, flagOperation) &&
UseHasFlagMethodCommon.CanRewriteToHasFlag(rightOperand, isConstantFlag, preservesEvaluationOrder: false))
{
return new(rightOperand, flagOperation);
}

return null;
}

private static bool TryGetEnumFlagReference(IOperation potentialFlag, IOperation comparedOperand, [NotNullWhen(true)] out IOperation? flagOperation)
private static bool TryGetEnumFlagReference(IOperation potentialFlag, IOperation comparedOperand, [NotNullWhen(true)] out IOperation? flagOperation, out bool isConstantFlag)
{
potentialFlag = potentialFlag.UnwrapImplicitConversions();
comparedOperand = comparedOperand.UnwrapImplicitConversions();
Expand All @@ -319,23 +321,27 @@ private static bool TryGetEnumFlagReference(IOperation potentialFlag, IOperation
!NumericHelpers.IsZero(firstFieldReference.Field.ConstantValue))
{
flagOperation = secondFieldReference;
isConstantFlag = true;
return true;
}

if (comparedOperand.IsConstantZero() && NumericHelpers.IsSingleBitSet(firstFieldReference.Field.ConstantValue))
{
flagOperation = firstFieldReference;
isConstantFlag = true;
return true;
}
}

if (!potentialFlag.IsConstantZero() && UseHasFlagMethodCommon.AreEquivalentOperands(potentialFlag, comparedOperand))
{
flagOperation = comparedOperand;
isConstantFlag = comparedOperand.ConstantValue.HasValue;
return true;
}

flagOperation = null;
isConstantFlag = false;
return false;
}

Expand Down
35 changes: 35 additions & 0 deletions src/Meziantou.Analyzer/Rules/UseHasFlagMethodCommon.cs
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -23,4 +23,39 @@ public static bool AreEquivalentOperands(IOperation? left, IOperation? right)
_ => false,
};
}

/// <summary>
/// Determines if replacing the flag check by <c>value.HasFlag(flag)</c> preserves the semantics of the check.
/// The rewrite evaluates the value operand before the flag operand and drops the duplicated read of the flag,
/// so it is only valid when evaluating the value operand cannot change the value of the flag.
/// </summary>
/// <param name="enumValueOperation">The operand the <c>HasFlag</c> method is called on.</param>
/// <param name="isConstantFlag">Whether the flag is a compile-time constant, in which case its reads are stable.</param>
/// <param name="preservesEvaluationOrder">Whether the value operand is already evaluated before every read of the flag.</param>
public static bool CanRewriteToHasFlag(IOperation enumValueOperation, bool isConstantFlag, bool preservesEvaluationOrder)
{
return isConstantFlag || preservesEvaluationOrder || IsSideEffectFree(enumValueOperation);
}

/// <summary>
/// Determines if evaluating the operation cannot have any observable side effect, so that it can be reordered with the read of the flag.
/// </summary>
private static bool IsSideEffectFree(IOperation? operation)
{
if (operation is null)
return false;

if (operation.ConstantValue.HasValue)
return true;

return operation switch
{
IParameterReferenceOperation or ILocalReferenceOperation or IInstanceReferenceOperation or ILiteralOperation => true,
IFieldReferenceOperation fieldReference => fieldReference.Field.IsStatic || IsSideEffectFree(fieldReference.Instance),
IConversionOperation conversion => conversion.OperatorMethod is null && IsSideEffectFree(conversion.Operand),
IUnaryOperation unary => unary.OperatorMethod is null && IsSideEffectFree(unary.Operand),
IBinaryOperation binary => binary.OperatorMethod is null && IsSideEffectFree(binary.LeftOperand) && IsSideEffectFree(binary.RightOperand),
_ => false,
};
}
}
Loading