dotnet / dotnet/winforms

Conditional order of precedence (possible) bug in `AxHost.CanAccessProperties`

Open
#1,989 4 comments 0 reactions 0 assignees View on GitHub
champion-required code cleanup
Dominant language
C#
Stars
4.9k
Forks
1.1k
Avg merge
20h 23m
Merged PRs (30d)
103

Description

* .NET Core Version: (e.g. 3.0 Preview1, or daily build number, use `dotnet --info`)
* Have you experienced this same bug with .NET Framework?: Yes/No

**Problem description:**

Discovered as part of a review of https://github.com/dotnet/winforms/issues/1955.
Having enabled [SA1408](https://github.com/DotNetAnalyzers/StyleCopAnalyzers/blob/master/documentation/SA1408.md) the analyser flagged the following issue:

```cs
// NB: the snippet is re-formatted to increase readability

internal override bool CanAccessProperties
{
get
{
int ocState = GetOcState();
return axState[fOwnWindow] && (ocState > OC_RUNNING || (IsUserMode() && ocState >= OC_RUNNING)) || ocState >= OC_INPLACE;
}
}
```
https://github.com/dotnet/winforms/blob/7019e1414309d820bc377e987cd1efcf9c17a12d/src/System.Windows.Forms/src/System/Windows/Forms/AxHost.cs#L526-L535

**Actual behavior:**

The intent is unclear.

* Is the following a correct expectation: `ocState >= OC_INPLACE` - OR - `axState[fOwnWindow] && (ocState > OC_RUNNING || (IsUserMode() && ocState >= OC_RUNNING))`?
* In `ocState > OC_RUNNING || (IsUserMode() && ocState >= OC_RUNNING)` condition the second `>` is redundant, as if `ocState > OC_RUNNING` the condition will be `true` on the first operand

**Expected behavior:**

The code is written in a manner that clearly communicates the intent and a desired outcome.

Contributor guide

Open the contributing guide

Assessment

This issue has not been assessed yet.

Get new issues in your inbox

A short digest of beginner-friendly GitHub issues.