IDE0078: Code Cleanup applied IDE0078 and broke the code

[shpaass]

Version Used: Microsoft Visual Studio Community 2022 (64-bit) - Current Version 17.11.2

Steps to Reproduce:

1. git clone https://github.com/shpaass/yafc-ce.git
2. git checkout cd421ef319f9dd354757b5898500bfb13c72f140
3. Open yafc-ce/FactorioCalc.sln in Visual Studio.
4. Navigate to Yafc/Windows/ProjectPageSettingsPanel.cs#L222.
5. Click on the line. See the lightbulb on the left. Click on it. See the first option: Use pattern matching (may change code meaning).
6. Close the lightbulb without appying. Apply Code Cleanup instead.
7. See that the target line was changed into a broken code.

From

if (DataUtils.ReadLine(bytes, ref index) != "YAFC" || DataUtils.ReadLine(bytes, ref index) != "ProjectPage") {
    throw new InvalidDataException();
}

To

if (DataUtils.ReadLine(bytes, ref index) is not "YAFC" or not "ProjectPage") {
    throw new InvalidDataException();
}

The expression now has one read operation instead of two, and is always true, which would lead to a guaranteed exception.

Diagnostic Id: "IDE0078: Use pattern matching"

Expected Behavior: The line is not changed or changed without breaking the code.

Actual Behavior: The line is changed. The change broke the code.

[CyrusNajmabadi]

We should probably disable the pattern matching logic if there is a variable passed by-ref. It strongly indicates a mutation across calls that can't be elided.

[shpaass]

@CyrusNajmabadi thank you for the triage! I'm currently searching for a way to disable all inspections that "may change code meaning", but I haven't found a list on the internet. MSDN articles of the inspections (IDE0078 for instance) also don't mention this aspect.

Is there a list of such inspections somewhere? If not, then what can I do to compile such a list?

[CyrusNajmabadi]

@CyrusNajmabadi thank you for the triage! I'm currently searching for a way to disable all inspections that "may change code meaning", but I haven't found a list on the internet. MSDN articles of the inspections (IDE0078 for instance) also don't mention this aspect.

Is there a list of such inspections somewhere? If not, then what can I do to compile such a list?

All fixers can change code meaning. (Not joking)

[CyrusNajmabadi]

@sharwell Can you make it so that "Apply Code Cleanup" does not automatically apply the CSharpUsePatternCombinatorsDiagnosticAnalyzer fixes to diagnostics that do not have hte "safe" property attached to them. I do not know how 'code cleanup' makes its determinations. Thanks.

[shpaass]

On an unrelated note, apologies for the commit spam. I was rebasing after squashes.