dotnet / dotnet/docs

CA1806's doc fails to mention IDE0058, it's wrong about Linq, and should recommend the discard operator

Open
#24,435 1 comment 0 reactions 0 assignees View on GitHub
dotnet-fundamentals/svc okr-quality waiting-on-feedback
Dominant language
No language data
Stars
4.8k
Forks
6.1k
Avg merge
19h 10m
Merged PRs (30d)
268

Description

https://docs.microsoft.com/en-us/dotnet/fundamentals/code-analysis/quality-rules/ca1806

## 1: No mention of IDE0058

* What purpose [does IDE0058](https://github.com/dotnet/roslyn-analyzers/issues/1101) serve when CA1806 does the same thing?
* Is there any difference? If so, then what? And why isn't the difference documented?
* Why does the documentation for both [CA1806](https://docs.microsoft.com/en-us/dotnet/fundamentals/code-analysis/quality-rules/ca1806) and [IDE0058](https://docs.microsoft.com/en-us/dotnet/fundamentals/code-analysis/style-rules/ide0058) completely ignore each other?

It feels to me like both analysis rules were written by independent teams completely ignorant of each other's work.

## 2: The doc is wrong about Linq:

This statement is untrue and potentially harmful:

> LINQ methods are known to not have side effects, and the result should not be ignored.

Linq methods _can_ and _do_ have side-effects depending on the background Linq provider, such as Linq-to-Entities (and Linq-to-SQL) where _materialization_ of a Linq query can happen unintentionally even without a final `ToList`/`ToDictionary`/`ToArray` call. Another risk is that a Linq expression invokes a database procedure or trigger (as some databases, like [Informix](https://www.ibm.com/docs/en/informix-servers/14.10?topic=statements-create-trigger-statement), do support `SELECT`-triggers).

## 3: It should recommend using the discard operator:

The article does not mention the C# discard operator at all, despite it being available since C# 7.0 and because the CA1806 rule _does_ recognize the operator:

> ### When to suppress warnings
> Do not suppress a warning from this rule unless the act of creating the object serves some purpose.

Instead it should read:

> ### When to suppress warnings
> Instead of suppressing the rule when a method's return value serves no purpose, use the discard operator to signify intent that the return value is unneeded:
>
> ```
> _ = MethodWithIgnoredReturnValue();
> ```

I would also suggest an example including `StringBuilder`. For example, this code triggers CA1806:

```
StringBuilder sb = new StringBuilder("foo");
sb.Append("bar");
sb.Append("baz");
```

This does not:

```
StringBuilder sb = new StringBuilder("foo");
_ = sb.Append("bar");
_ = sb.Append("baz");
```

(Can we customize CA1806 (or IDE0058) to ignore `StringBuilder.Append` yet?)

----

I did start writing changes in my own fork of the docs but I realised my changes would be too extensive for a simple PR and would likely be rejected - hence me filing this issue first.

-------

**Target framework**

- [x] .NET Core
- [x] .NET Framework
- [x] .NET Standard

[Edited below by gewarren]

---
#### Document Details

⚠ *Do not edit this section. It is required for docs.microsoft.com ➟ GitHub issue linking.*

* ID: 0f51f0a0-29e7-be15-ffdc-ee210cf7ef1c
* Version Independent ID: 59d0b85d-87f3-c4e4-6891-3002e2f60158
* Content: [CA1806: Do not ignore method results (code analysis) - .NET](https://docs.microsoft.com/en-us/dotnet/fundamentals/code-analysis/quality-rules/ca1806)
* Content Source: [docs/fundamentals/code-analysis/quality-rules/ca1806.md](https://github.com/dotnet/docs/blob/main/docs/fundamentals/code-analysis/quality-rules/ca1806.md)
* Product: **dotnet-fundamentals**
* GitHub Login: @gewarren
* Microsoft Alias: **gewarren**

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.