Skip to content

make CompletionItem implement IEquatable<CompletionItem> #1991

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

Merged
merged 1 commit into from
Dec 14, 2022
Merged
Show file tree
Hide file tree
Changes from all commits
Commits
File filter

Filter by extension

Filter by extension

Conversations
Failed to load comments.
Loading
Jump to
Jump to file
Failed to load files.
Loading
Diff view
Diff view
Original file line number Diff line number Diff line change
Expand Up @@ -277,15 +277,15 @@ System.CommandLine.Completions
public abstract class CompletionContext
public System.CommandLine.ParseResult ParseResult { get; }
public System.String WordToComplete { get; }
public class CompletionItem
public class CompletionItem, System.IEquatable<CompletionItem>
.ctor(System.String label, System.String kind = Value, System.String sortText = null, System.String insertText = null, System.String documentation = null, System.String detail = null)
public System.String Detail { get; }
public System.String Documentation { get; set; }
public System.String InsertText { get; }
public System.String Kind { get; }
public System.String Label { get; }
public System.String SortText { get; }
protected System.Boolean Equals(CompletionItem other)
public System.Boolean Equals(CompletionItem other)
public System.Boolean Equals(System.Object obj)
public System.Int32 GetHashCode()
public System.String ToString()
Expand Down
26 changes: 4 additions & 22 deletions src/System.CommandLine/Completions/CompletionItem.cs
Original file line number Diff line number Diff line change
Expand Up @@ -6,7 +6,7 @@ namespace System.CommandLine.Completions
/// <summary>
/// Provides details about a command line completion item.
/// </summary>
public class CompletionItem
public class CompletionItem : IEquatable<CompletionItem>
{
/// <param name="label">The label value, which is the text displayed to users and, unless <paramref name="insertText"/> is set, is also used to populate the <see cref="InsertText"/> property.</param>
/// <param name="kind">The kind of completion item.</param>
Expand Down Expand Up @@ -66,31 +66,13 @@ public CompletionItem(
/// <summary>
/// Determines whether two completion items are equal.
/// </summary>
protected bool Equals(CompletionItem other)
public bool Equals(CompletionItem? other)
{
return Label == other.Label && Kind == other.Kind;
return other is not null && Label == other.Label && Kind == other.Kind;
}

/// <inheritdoc />
public override bool Equals(object? obj)
{
if (ReferenceEquals(null, obj))
{
return false;
}

if (ReferenceEquals(this, obj))
{
return true;
}

if (obj.GetType() != GetType())

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

The new implementation does not check obj.GetType() != GetType(). That makes it difficult for derived classes to override the Equals methods correctly. CompletionItem is not a sealed class and should not be, as noted in #1962 (comment).

{
return false;
}

return Equals((CompletionItem)obj);
}
public override bool Equals(object? obj) => Equals(obj as CompletionItem);

/// <inheritdoc />
public override int GetHashCode()
Expand Down