From e0cbf321b227d2e4ff06fe2c0bf367f3352031de Mon Sep 17 00:00:00 2001 From: Cyrus Najmabadi Date: Tue, 5 Nov 2024 13:33:29 -0800 Subject: [PATCH 1/2] Improve pattern matching when user is likely providing camel humps --- .../Test/Utilities/PatternMatcherTests.cs | 2 + .../CSharpCompletionCommandHandlerTests.vb | 71 ++++++++++++++++++- .../PatternMatcher.TextChunk.cs | 2 + .../PatternMatching/PatternMatcher.cs | 68 +++++++++++++----- 4 files changed, 125 insertions(+), 18 deletions(-) diff --git a/src/EditorFeatures/Test/Utilities/PatternMatcherTests.cs b/src/EditorFeatures/Test/Utilities/PatternMatcherTests.cs index 70f63b4c7c1b6..56bd915658eea 100644 --- a/src/EditorFeatures/Test/Utilities/PatternMatcherTests.cs +++ b/src/EditorFeatures/Test/Utilities/PatternMatcherTests.cs @@ -193,8 +193,10 @@ private static void VerifyBreakIntoCharacterParts(string original, params string [InlineData("Combine[|Bin|]ary", "bin", PatternMatchKind.StartOfWordSubstring, CaseInsensitive)] [InlineData("_ABC_[|Abc|]_", "Abc", PatternMatchKind.StartOfWordSubstring, CaseSensitive)] + [InlineData("[|C|]reate[|R|]ange", "CR", PatternMatchKind.CamelCaseExact, CaseSensitive)] [WorkItem("https://github.com/dotnet/roslyn/issues/51029")] + [WorkItem("https://github.com/dotnet/roslyn/issues/17275")] internal void TestNonFuzzyMatch( string candidate, string pattern, PatternMatchKind matchKind, bool isCaseSensitive) { diff --git a/src/EditorFeatures/Test2/IntelliSense/CSharpCompletionCommandHandlerTests.vb b/src/EditorFeatures/Test2/IntelliSense/CSharpCompletionCommandHandlerTests.vb index 6cf7e7776f03c..c7058982685ed 100644 --- a/src/EditorFeatures/Test2/IntelliSense/CSharpCompletionCommandHandlerTests.vb +++ b/src/EditorFeatures/Test2/IntelliSense/CSharpCompletionCommandHandlerTests.vb @@ -16,7 +16,6 @@ Imports Microsoft.CodeAnalysis.Editor.Implementation.IntelliSense.AsyncCompletio Imports Microsoft.CodeAnalysis.Editor.Shared.Options Imports Microsoft.CodeAnalysis.Editor.[Shared].Utilities Imports Microsoft.CodeAnalysis.Editor.UnitTests.Extensions -Imports Microsoft.CodeAnalysis.Editor.UnitTests.Workspaces Imports Microsoft.CodeAnalysis.Host Imports Microsoft.CodeAnalysis.Host.Mef Imports Microsoft.CodeAnalysis.Options @@ -34,7 +33,7 @@ Imports Roslyn.Test.Utilities.TestGenerators Namespace Microsoft.CodeAnalysis.Editor.UnitTests.IntelliSense - Public Class CSharpCompletionCommandHandlerTests + Public NotInheritable Class CSharpCompletionCommandHandlerTests Public Async Function CompletionOnFileType_SameFile_NonQualified(showCompletionInArgumentLists As Boolean) As Task Using state = TestStateFactory.CreateCSharpTestState( @@ -12682,5 +12681,73 @@ internal class Program Await state.AssertCompletionItemsContain("Id", displayTextSuffix:="") End Using End Function + + + Public Async Function PreferCamelCasedExactMatchOverPrefixCaseInsensitiveMatch1(showCompletionInArgumentLists As Boolean) As Task + Using state = TestStateFactory.CreateCSharpTestState( + + , + showCompletionInArgumentLists:=showCompletionInArgumentLists, languageVersion:=LanguageVersion.CSharp12) + + state.SendTypeChars("CR") + Await state.AssertSelectedCompletionItem("CreateRange") + End Using + End Function + + + Public Async Function PreferCamelCasedExactMatchOverPrefixCaseInsensitiveMatch2(showCompletionInArgumentLists As Boolean) As Task + Using state = TestStateFactory.CreateCSharpTestState( + + , + showCompletionInArgumentLists:=showCompletionInArgumentLists, languageVersion:=LanguageVersion.CSharp12) + + state.SendTypeChars("cr") + Await state.AssertSelectedCompletionItem("Create") + End Using + End Function + + + Public Async Function PreferCamelCasedExactMatchOverPrefixCaseInsensitiveMatch3(showCompletionInArgumentLists As Boolean) As Task + Using state = TestStateFactory.CreateCSharpTestState( + + class C + { + void Create() { } + void CreateRange() { } + + void M() + { + this.$$ + } + } + , + showCompletionInArgumentLists:=showCompletionInArgumentLists, languageVersion:=LanguageVersion.CSharp12) + + state.SendTypeChars("Cr") + Await state.AssertSelectedCompletionItem("Create") + End Using + End Function End Class End Namespace diff --git a/src/Workspaces/Core/Portable/PatternMatching/PatternMatcher.TextChunk.cs b/src/Workspaces/Core/Portable/PatternMatching/PatternMatcher.TextChunk.cs index 23b15fec6d7e4..01fbfb1ab3bef 100644 --- a/src/Workspaces/Core/Portable/PatternMatching/PatternMatcher.TextChunk.cs +++ b/src/Workspaces/Core/Portable/PatternMatching/PatternMatcher.TextChunk.cs @@ -38,6 +38,7 @@ private struct TextChunk : IDisposable public WordSimilarityChecker SimilarityChecker; public readonly bool IsLowercase; + public readonly bool IsUppercase; public TextChunk(string text, bool allowFuzzingMatching) { @@ -50,6 +51,7 @@ public TextChunk(string text, bool allowFuzzingMatching) : default; IsLowercase = !ContainsUpperCaseLetter(text); + IsUppercase = !ContainsLowerCaseLetter(text); } public void Dispose() diff --git a/src/Workspaces/Core/Portable/PatternMatching/PatternMatcher.cs b/src/Workspaces/Core/Portable/PatternMatching/PatternMatcher.cs index 40d789da6019f..96c74e0f71db9 100644 --- a/src/Workspaces/Core/Portable/PatternMatching/PatternMatcher.cs +++ b/src/Workspaces/Core/Portable/PatternMatching/PatternMatcher.cs @@ -114,9 +114,19 @@ private static bool ContainsUpperCaseLetter(string pattern) for (var i = 0; i < pattern.Length; i++) { if (char.IsUpper(pattern[i])) - { return true; - } + } + + return false; + } + + private static bool ContainsLowerCaseLetter(string pattern) + { + // Expansion of "foreach(char ch in pattern)" to avoid a CharEnumerator allocation + for (var i = 0; i < pattern.Length; i++) + { + if (char.IsLower(pattern[i])) + return true; } return false; @@ -153,6 +163,8 @@ private static bool ContainsUpperCaseLetter(string pattern) in TextChunk patternChunk, bool punctuationStripped) { + using var candidateHumps = TemporaryArray.Empty; + var candidateLength = candidate.Length; var caseInsensitiveIndex = _compareInfo.IndexOf(candidate, patternChunk.Text, CompareOptions.IgnoreCase); @@ -170,15 +182,31 @@ private static bool ContainsUpperCaseLetter(string pattern) } else { + var isCaseSensitive = _compareInfo.IsPrefix(candidate, patternChunk.Text); + + if (!isCaseSensitive && patternChunk.IsUppercase) + { + // The user wrote something all upper case, but happened to match the prefix of the word. For + // example, matching `CR` against both `Create` (a case insensitive prefix match) and `CreateRange` + // (a camel hump match). We want to prefer the latter in this case as the all upper case string + // is a strong signal they wanted camel hump matching here. + PopulateCandidateHumps(); + + // Note: ensure that we match here case sensitively as well. We only want to take this if their + // camel humps actually matched real word starts. + var match = TryCamelCaseMatch(candidate, patternChunk, punctuationStripped, isLowercase: false, candidateHumps); + if (match is { IsCaseSensitive: true }) + return match; + + // Deliberately fall through. + } + // Lengths were the same, this is either a case insensitive or sensitive prefix match. return new PatternMatch( - PatternMatchKind.Prefix, punctuationStripped, isCaseSensitive: _compareInfo.IsPrefix(candidate, patternChunk.Text), - matchedSpan: GetMatchedSpan(0, patternChunk.Text.Length)); + PatternMatchKind.Prefix, punctuationStripped, isCaseSensitive, matchedSpan: GetMatchedSpan(0, patternChunk.Text.Length)); } } - using var candidateHumps = TemporaryArray.Empty; - var patternIsLowercase = patternChunk.IsLowercase; if (caseInsensitiveIndex > 0) { @@ -221,7 +249,8 @@ private static bool ContainsUpperCaseLetter(string pattern) // Now do the more expensive check to see if we're at the start of a word. This is to catch // word matches like CombineBinary. We want to find the hit against '[|Bin|]ary' not // 'Com[|bin|]e' - StringBreaker.AddWordParts(candidate, ref candidateHumps.AsRef()); + PopulateCandidateHumps(); + for (int i = 0, n = candidateHumps.Count; i < n; i++) { var hump = TextSpan.FromBounds(candidateHumps[i].Start, candidateLength); @@ -235,16 +264,17 @@ private static bool ContainsUpperCaseLetter(string pattern) } } - // Didn't have an exact/prefix match, or a high enough quality substring match. - // See if we can find a camel case match. - if (candidateHumps.Count == 0) - StringBreaker.AddWordParts(candidate, ref candidateHumps.AsRef()); + { + // Didn't have an exact/prefix match, or a high enough quality substring match. + // See if we can find a camel case match. + PopulateCandidateHumps(); - // Didn't have an exact/prefix match, or a high enough quality substring match. - // See if we can find a camel case match. - var match = TryCamelCaseMatch(candidate, patternChunk, punctuationStripped, patternIsLowercase, candidateHumps); - if (match != null) - return match; + // Didn't have an exact/prefix match, or a high enough quality substring match. + // See if we can find a camel case match. + var match = TryCamelCaseMatch(candidate, patternChunk, punctuationStripped, patternIsLowercase, candidateHumps); + if (match != null) + return match; + } // If pattern was all lowercase, we allow it to match an all lowercase section of the candidate. But // only after we've tried all other forms first. This is the weakest of all matches. For example, if @@ -265,6 +295,12 @@ private static bool ContainsUpperCaseLetter(string pattern) } return null; + + void PopulateCandidateHumps() + { + if (candidateHumps.Count == 0) + StringBreaker.AddWordParts(candidate, ref candidateHumps.AsRef()); + } } private TextSpan? GetMatchedSpan(int start, int length) From 414a61e1c2b7aa4c7f125c166f846fc0a051ef3e Mon Sep 17 00:00:00 2001 From: Cyrus Najmabadi Date: Tue, 5 Nov 2024 13:58:01 -0800 Subject: [PATCH 2/2] Improve pattern matching when user is likely providing camel humps --- .../CSharpCompletionCommandHandlerTests.vb | 4 ++++ .../Portable/PatternMatching/PatternMatch.cs | 24 +++++++++++++++---- .../PatternMatching/PatternMatchKind.cs | 6 +++++ 3 files changed, 30 insertions(+), 4 deletions(-) diff --git a/src/EditorFeatures/Test2/IntelliSense/CSharpCompletionCommandHandlerTests.vb b/src/EditorFeatures/Test2/IntelliSense/CSharpCompletionCommandHandlerTests.vb index c7058982685ed..93e3d6a1af72e 100644 --- a/src/EditorFeatures/Test2/IntelliSense/CSharpCompletionCommandHandlerTests.vb +++ b/src/EditorFeatures/Test2/IntelliSense/CSharpCompletionCommandHandlerTests.vb @@ -12638,6 +12638,7 @@ $$ End Using End Function + Public Async Function CompletionInsideImplicitObjectCreationInsideCollectionExpression(showCompletionInArgumentLists As Boolean) As Task Using state = TestStateFactory.CreateCSharpTestState( @@ -12682,6 +12683,7 @@ internal class Program End Using End Function + Public Async Function PreferCamelCasedExactMatchOverPrefixCaseInsensitiveMatch1(showCompletionInArgumentLists As Boolean) As Task Using state = TestStateFactory.CreateCSharpTestState( @@ -12705,6 +12707,7 @@ internal class Program End Using End Function + Public Async Function PreferCamelCasedExactMatchOverPrefixCaseInsensitiveMatch2(showCompletionInArgumentLists As Boolean) As Task Using state = TestStateFactory.CreateCSharpTestState( @@ -12728,6 +12731,7 @@ internal class Program End Using End Function + Public Async Function PreferCamelCasedExactMatchOverPrefixCaseInsensitiveMatch3(showCompletionInArgumentLists As Boolean) As Task Using state = TestStateFactory.CreateCSharpTestState( diff --git a/src/Workspaces/Core/Portable/PatternMatching/PatternMatch.cs b/src/Workspaces/Core/Portable/PatternMatching/PatternMatch.cs index 76af1d62ffc13..ad069f7feb922 100644 --- a/src/Workspaces/Core/Portable/PatternMatching/PatternMatch.cs +++ b/src/Workspaces/Core/Portable/PatternMatching/PatternMatch.cs @@ -67,17 +67,33 @@ public int CompareTo(PatternMatch? other, bool ignoreCase) public int CompareTo(PatternMatch other, bool ignoreCase) { + // 1. In all scenarios, An case sensitive camel match (like CR against CreateRange) should beat a case + // insensitive match (like 'CR' against 'Create'). + // + // Other cases can be added here as necessary. + + switch (this.IsCaseSensitive, this.Kind.IsCamelCaseKind(), other.IsCaseSensitive, other.Kind.IsCamelCaseKind()) + { + case (true, true, false, false): + return -1; + case (false, false, true, true): + return 1; + } + // Compare types var comparison = this.Kind - other.Kind; if (comparison != 0) return comparison; - // Compare cases if (!ignoreCase) { - comparison = (!this.IsCaseSensitive).CompareTo(!other.IsCaseSensitive); - if (comparison != 0) - return comparison; + switch (this.IsCaseSensitive, other.IsCaseSensitive) + { + case (true, false): + return -1; + case (false, true): + return 1; + } } // Consider a match to be better if it was successful without stripping punctuation diff --git a/src/Workspaces/Core/Portable/PatternMatching/PatternMatchKind.cs b/src/Workspaces/Core/Portable/PatternMatching/PatternMatchKind.cs index e3c524344b7f5..d5fefcb9143b4 100644 --- a/src/Workspaces/Core/Portable/PatternMatching/PatternMatchKind.cs +++ b/src/Workspaces/Core/Portable/PatternMatching/PatternMatchKind.cs @@ -107,3 +107,9 @@ internal enum PatternMatchKind /// LowercaseSubstring, } + +internal static class PatternMatchKindExtensions +{ + public static bool IsCamelCaseKind(this PatternMatchKind kind) + => kind is PatternMatchKind.CamelCaseExact or PatternMatchKind.CamelCasePrefix or PatternMatchKind.CamelCaseNonContiguousPrefix or PatternMatchKind.CamelCaseSubstring; +}