Skip to content
Open
Changes from all commits
Commits
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
123 changes: 123 additions & 0 deletions docs/design/hresult-analyzer.md
Original file line number Diff line number Diff line change
@@ -0,0 +1,123 @@
**Analyzer goal:** prevent callers from accidentally ignoring the result of any method that returns the CsWin32 `HRESULT` type.
# Scope

Analyze every method invocation whose compile-time return type is the CsWin32 `HRESULT` type.

The method does not need to be generated by CsWin32 or marked PreserveSig. This includes helper and wrapper methods that return an `HRESULT`.
# Recognized consumption

1. Throw on failure: `obj.DoWork().ThrowOnFailure();`
2. Test success: `if (obj.DoWork().Succeeded) { ... }`
3. Test failure: `if (obj.DoWork().Failed) return;`
4. Compare with an HRESULT: `if (hr == HRESULT.S_FALSE) { ... }`
5. Switch on outcomes: `switch (hr) { case HRESULT.S_OK: ... }`
6. Explicitly discard the result: `_ = obj.DoWork();`
7. Pass the result to another method: `Log(obj.DoWork());`
8. Store the result in a field or collection: `this.lastResult = obj.DoWork();`
9. Return the result to the caller: `return obj.DoWork();`

`COMHR001` is conservative. It accepts any explicit use of the result because the caller did not accidentally ignore it.

Reading `HRESULT.Value` does not by itself count as use. It transfers the tracking obligation to the resulting integer. Storing that integer in a local variable is insufficient until the value is read or otherwise used according to the same rules as an `HRESULT`.

Some accepted uses have clearer alternatives:

1. Prefer `hr.Failed` to `hr.Value < 0`.
2. Prefer `hr.ThrowOnFailure()` to `Marshal.ThrowExceptionForHR(hr.Value)`.

Use may occur through a local variable:

```csharp
HRESULT hr = obj.DoWork();

if (hr.Failed)
{
return;
}
```
## Switch requirements

Any switch that consumes the `HRESULT` counts as intentional use. For example:

```csharp
switch (hr)
{
case HRESULT.S_OK:
UseResult();
break;

case HRESULT.S_FALSE:
HandleNoResult();
break;

default:
hr.ThrowOnFailure();
break;
}
```

`COMHR001` does not require the switch to distinguish a specific HRESULT or success/failure category. This is intentional to avoid excessively restricting handling patterns while still preventing consumers from accidentally ignoring HRESULTs.
# Control-flow requirement

Track each `HRESULT` through local variables. Every reachable path must consume the value before it is overwritten or the method exits.

This is insufficient because the false path does not consume the HRESULT:

```csharp
HRESULT hr = obj.DoWork();

if (condition)
{
hr.ThrowOnFailure();
}
```

# Diagnostic conditions

Report `COMHR001` if an HRESULT:

1. Is ignored as an unused invocation result.
2. Is assigned to a local that is never read.
3. Is overwritten before its value is read.
4. Has its `Value` stored in a local that is never read.
5. Reaches a normal method exit without being consumed.
6. Is not consumed on all reachable paths.
# Insufficient handling

```csharp
obj.DoWork();
HRESULT hr = obj.DoWork(); // Never checked
int value = obj.DoWork().Value; // Never used
```

Logging, storing outside a local, returning, or passing the HRESULT or its numeric value counts as intentional use and does not produce `COMHR001`.
# Propagation through wrappers

Consider a wrapper around a CsWin32 method:

```csharp
HRESULT Initialize()
{
return native.Initialize();
}
```

Returning the HRESULT transfers responsibility. It does not handle the result.

Because the analyzer checks every method that returns `HRESULT`, it also enforces handling at this call site:

```csharp
Initialize();
```

This supports transparent HRESULT wrappers without requiring explicit annotation.
# Diagnostics

| Rule | Condition |
| ---------- | ------------------------------------------------- |
| `COMHR001` | HRESULT invocation result is accidentally ignored |
# Analysis boundary

Start analysis at every invocation whose compile-time return type is the CsWin32 `HRESULT` type. Track the value through local variables and control flow until every reachable path consumes it, overwrites it, or reaches a method exit.

When analysis reaches an `HRESULT.Value` access, continue tracking the resulting integer. Apply the same control-flow and consumption rules to that integer as to the original `HRESULT`.
Loading