dotnet / roslyn

The Roslyn .NET compiler provides C# and Visual Basic languages with rich code analysis APIs.
https://docs.microsoft.com/dotnet/csharp/roslyn-sdk/
MIT License
18.95k stars 4.02k forks source link

ReplacementChangesSemantics fails to detect certain breaking changes #20698

Open sharwell opened 7 years ago

sharwell commented 7 years ago

Version Used: 15.3 Preview 3

As described in https://github.com/dotnet/roslyn/pull/20596#issuecomment-312662668, ReplacementChangesSemantics fails to detect certain semantic changes, leading to broken diagnostics and code fixes for some cases. @zaytsev-victor has identified a potential solution in https://github.com/dotnet/roslyn/pull/20596#issuecomment-312857730 which could fix several outstanding bugs in this area.

zaytsev-victor commented 7 years ago

Regarding to #20596. I've found some strange code (at least at my point of view) in src\compilers\csharp\portable\binder\Binder_Operators.cs file:

var best = this.UnaryOperatorOverloadResolution(kind, operand, node, diagnostics, out resultKind, out originalUserDefinedOperators);
            if (!best.HasValue)
            {
                ReportUnaryOperatorError(node, diagnostics, operatorText, operand, resultKind);
                return new BoundUnaryOperator(
                    node,
                    kind,
                    operand,
                    ConstantValue.NotAvailable,
                    null,
                    resultKind,
                    originalUserDefinedOperators,
                    GetSpecialType(SpecialType.System_Object, diagnostics, node),
                    hasErrors: true);
            }

Why SpecialType.System_Object used and not ErrorType since expression is invalid?

sharwell commented 7 years ago

💭 It seems like ideally the symbol returned would be the type of the operand, meaning it behaves in IntelliSense like the operand even if the operand (or operator) doesn't resolve. In almost all cases the unary operators do not change the type of the argument.