CA1806's doc fails to mention IDE0058, it's wrong about Linq, and should recommend the discard operator
- 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
Assessment
This issue has not been assessed yet.