-
Notifications
You must be signed in to change notification settings - Fork 4.7k
New issue
Have a question about this project? Sign up for a free GitHub account to open an issue and contact its maintainers and the community.
By clicking “Sign up for GitHub”, you agree to our terms of service and privacy statement. We’ll occasionally send you account related emails.
Already on GitHub? Sign in to your account
ConsoleKeyInfo does not implement IEquatable #2127
Comments
What are you trying to achieve? |
Well, I think the question is that runtime/src/libraries/System.Console/src/System/ConsoleKeyInfo.cs Lines 55 to 58 in cb1fc80
I think then this is an API proposal: namespace System
{
- public readonly struct ConsoleKeyInfo
+ public readonly struct ConsoleKeyInfo : IEquatable<ConsoleKeyInfo>
{ |
A common scenario could be implementing a TUI and maintaining a dictionary of |
@terrajobst -public readonly struct ConsoleKeyInfo
+public readonly struct ConsoleKeyInfo : IEquatable<ConsoleKeyInfo> @jeffhandley it looks like a good candidate for an analyzer (#31996 is another example) |
@adamsitnik Yeah that seems like a cool analyzer idea. Feel free to submit an issue for that and tag it with |
Yes..hopefully quick and easy one though |
It actually already exists: CA1066 |
@stephentoub thanks, I did not know about it! Is there any reason why it's disabled in this repo? runtime/eng/CodeAnalysis.ruleset Line 54 in 3c9fcce
|
Most if not all of the API-focused rules are disabled because they're not actionable on their own: we have a dedicated API review process for any new public APIs, so a developer can't just go in and fix a violation on their own. There are currently dozens of warnings from the rule on public surface area, mostly APIs that have been around for a very long time. If someone did the work to catalogue all of the public APIs that would need to be augmented, run them all through API review, and get all the APIs added, then we could enable the rule, assuming we wanted to enforce it in all public APIs shipped from this repo moving forward. For reference, when I build locally, these are the warnings I get from it:
Note that some of these highlight that just because a type overrides Equals doesn't mean it should implement IEquatable. Take Hashcode, for example, which overrides Equals to throw so as to prevent comparison; implementing IEquatable would be a step backwards. |
namespace System
{
public partial readonly struct ConsoleKeyInfo : IEquatable<ConsoleKeyInfo>
{
}
} |
System.ConsoleKeyInfo
does not implementSystem.IEquatable<System.ConsoleKeyInfo>
. Is this intended?The text was updated successfully, but these errors were encountered: