Improve naming style diagnostics when identifier begins with common prefixes (#32549)

* wip

* FIx for 18409

* Additional fix

* remove commented out code
Co-Authored-By: Nchborl <chborl@users.noreply.github.com>

* Fix spacing issue
Co-Authored-By: Nchborl <chborl@users.noreply.github.com>

* Use default instead of 0
Co-Authored-By: Nchborl <chborl@users.noreply.github.com>

* Respond to feedback

* Add test

src/EditorFeatures/CSharpTest/Diagnostics/NamingStyles/NamingStylesTests.cs

@@ -49,15 +49,15 @@ public async Task TestPascalCaseClass_NameGetsCapitalized()
         [InlineData("M_bar", "bar")]
         [InlineData("S_bar", "bar")]
         [InlineData("T_bar", "bar")]
-        [InlineData("_Bar", "bar", Skip = "https://github.com/dotnet/roslyn/issues/26588")]
-        [InlineData("__Bar", "bar", Skip = "https://github.com/dotnet/roslyn/issues/26588")]
+        [InlineData("_Bar", "bar")]
+        [InlineData("__Bar", "bar")]
         [InlineData("M_s__t_Bar", "bar")]
-        [InlineData("m_bar", "bar", Skip = "https://github.com/dotnet/roslyn/issues/26588")]
-        [InlineData("s_bar", "bar", Skip = "https://github.com/dotnet/roslyn/issues/26588")]
-        [InlineData("t_bar", "bar", Skip = "https://github.com/dotnet/roslyn/issues/26588")]
-        [InlineData("_bar", "bar", Skip = "https://github.com/dotnet/roslyn/issues/26588")]
-        [InlineData("__bar", "bar", Skip = "https://github.com/dotnet/roslyn/issues/26588")]
-        [InlineData("m_s__t_Bar", "bar", Skip = "https://github.com/dotnet/roslyn/issues/26588")]
+        [InlineData("m_bar", "bar")]
+        [InlineData("s_bar", "bar")]
+        [InlineData("t_bar", "bar")]
+        [InlineData("_bar", "bar")]
+        [InlineData("__bar", "bar")]
+        [InlineData("m_s__t_Bar", "bar")]
         // Special cases to ensure empty identifiers are not produced
         [InlineData("M_", "m_")]
         [InlineData("M__", "_")]
@@ -83,14 +83,14 @@ public async Task TestCamelCaseField_PrefixGetsStripped(string fieldName, string
         [InlineData("S_bar", "_bar")]
         [InlineData("T_bar", "_bar")]
         [InlineData("_Bar", "_bar")]
-        [InlineData("__Bar", "_bar", Skip = "https://github.com/dotnet/roslyn/issues/26588")]
+        [InlineData("__Bar", "_bar")]
         [InlineData("M_s__t_Bar", "_bar")]
         [InlineData("m_bar", "_bar")]
         [InlineData("s_bar", "_bar")]
         [InlineData("t_bar", "_bar")]
         [InlineData("bar", "_bar")]
-        [InlineData("__bar", "_bar", Skip = "https://github.com/dotnet/roslyn/issues/26588")]
-        [InlineData("__s_bar", "_bar", Skip = "https://github.com/dotnet/roslyn/issues/26588")]
+        [InlineData("__bar", "_bar")]
+        [InlineData("__s_bar", "_bar")]
         [InlineData("m_s__t_Bar", "_bar")]
         // Special cases to ensure empty identifiers are not produced
         [InlineData("M_", "_m_")]

src/EditorFeatures/Test2/Diagnostics/NamingStyles/NamingStyleTests.IdentifierCreation.Compliance.vb

@@ -32,7 +32,7 @@ Namespace Microsoft.CodeAnalysis.Editor.Implementation.Diagnostics.UnitTests
         <Fact, Trait(Traits.Feature, Traits.Features.NamingStyle)>
         Public Sub TestManyEmptyWords()
             Dim namingStyle = CreateNamingStyle(wordSeparator:="_", capitalizationScheme:=Capitalization.PascalCase)
-            TestNameCompliance(namingStyle, "_____")
+            TestNameNoncomplianceAndFixedNames(namingStyle, "_____", "_")
         End Sub
 
         <Fact, Trait(Traits.Feature, Traits.Features.NamingStyle)>
@@ -40,6 +40,18 @@ Namespace Microsoft.CodeAnalysis.Editor.Implementation.Diagnostics.UnitTests
             Dim namingStyle = CreateNamingStyle(prefix:="p_", suffix:="_s", capitalizationScheme:=Capitalization.PascalCase)
             TestNameNoncomplianceAndFixedNames(namingStyle, "_", "p_s", "p___s")
         End Sub
+
+        <Fact, Trait(Traits.Feature, Traits.Features.NamingStyle)>
+        Public Sub TestPrefixAndCommonPrefix()
+            Dim namingStyle = CreateNamingStyle(prefix:="Test_", suffix:="_z", capitalizationScheme:=Capitalization.PascalCase)
+            TestNameNoncomplianceAndFixedNames(namingStyle, "Test_m_BaseName", "Test_M_BaseName_z", "Test_BaseName_z")
+        End Sub
+
+        <Fact, Trait(Traits.Feature, Traits.Features.NamingStyle)>
+        Public Sub TestCommonPrefixAndPrefix()
+            Dim namingStyle = CreateNamingStyle(prefix:="Test_", suffix:="_z", capitalizationScheme:=Capitalization.PascalCase)
+            TestNameNoncomplianceAndFixedNames(namingStyle, "m_Test_BaseName", "Test_BaseName_z")
+        End Sub
 #End Region
 
 #Region "PascalCase"

src/Workspaces/Core/Portable/NamingStyles/NamingStyle.cs

@@ -139,11 +139,28 @@ public bool IsNameCompliant(string name, out string failureReason)
 
             if (name.Length <= Prefix.Length + Suffix.Length)
             {
+                // name consists of Prefix and Suffix and no base name
+                // Prefix and Suffix can overlap
+                // Example: Prefix = "s_", Suffix = "_t", name "s_t"
                 failureReason = null;
                 return true;
             }
 
-            var spanToCheck = TextSpan.FromBounds(Prefix.Length, name.Length - Suffix.Length);
+            // remove specified Prefix, then look for any other common prefixes
+            name = StripCommonPrefixes(name.Substring(Prefix.Length), out var prefix);
+
+            if (prefix != string.Empty)
+            {
+                // name started with specified prefix, but has at least one additional common prefix 
+                // Example: specified prefix "test_", actual prefix "test_m_"
+                failureReason = Prefix == string.Empty ?
+                    string.Format(WorkspacesResources.Prefix_0_is_not_expected, prefix) :
+                    string.Format(WorkspacesResources.Prefix_0_does_not_match_expected_prefix_1, prefix, Prefix);
+                return false;
+            }
+
+            // specified and common prefixes have been removed. Now see that the base name has correct capitalization
+            var spanToCheck = TextSpan.FromBounds(0, name.Length - Suffix.Length);
             Debug.Assert(spanToCheck.Length > 0);
 
             switch (CapitalizationScheme)
@@ -295,6 +312,10 @@ private bool CheckFirstUpper(string name, TextSpan nameSpan, out string reason)
 
         private string CreateCompliantNameDirectly(string name)
         {
+            // Example: for specified prefix = "Test_" and name = "Test_m_BaseName", we remove "Test_m_"
+            // "Test_" will be added back later in this method
+            name = StripCommonPrefixes(name.StartsWith(Prefix) ? name.Substring(Prefix.Length) : name, out _);
+
             var addPrefix = !name.StartsWith(Prefix);
             var addSuffix = !name.EndsWith(Suffix);
 
@@ -318,14 +339,14 @@ public IEnumerable<string> MakeCompliant(string name)
 
         private string CreateCompliantNameReusingPartialPrefixesAndSuffixes(string name)
         {
-            name = StripCommonPrefixes(name);
+            name = StripCommonPrefixes(name, out _);
             name = EnsurePrefix(name);
             name = EnsureSuffix(name);
 
             return FinishFixingName(name);
         }
 
-        private static string StripCommonPrefixes(string name)
+        private static string StripCommonPrefixes(string name, out string prefix)
         {
             var index = 0;
             while (index + 1 < name.Length)
@@ -355,6 +376,7 @@ private static string StripCommonPrefixes(string name)
                 break;
             }
 
+            prefix = name.Substring(0, index);
             return name.Substring(index);
         }
 
@@ -368,10 +390,17 @@ private string FinishFixingName(string name)
 
             name = name.Substring(Prefix.Length, name.Length - Suffix.Length - Prefix.Length);
             IEnumerable<string> words = new[] { name };
+
             if (!string.IsNullOrEmpty(WordSeparator))
             {
                 words = name.Split(new[] { WordSeparator }, StringSplitOptions.RemoveEmptyEntries);
 
+                // Edge case: the only character(s) in the name is(are) the WordSeparator
+                if (words.Count() == 0)
+                {
+                    return name;
+                }
+
                 if (words.Count() == 1) // Only Split if words have not been split before 
                 {
                     bool isWord = true;

src/Workspaces/Core/Portable/WorkspacesResources.Designer.cs

@@ -1276,6 +1276,24 @@ internal class WorkspacesResources {
             }
         }
         
+        /// <summary>
+        ///   Looks up a localized string similar to Prefix &apos;{0}&apos; does not match expected prefix &apos;{1}&apos;.
+        /// </summary>
+        internal static string Prefix_0_does_not_match_expected_prefix_1 {
+            get {
+                return ResourceManager.GetString("Prefix_0_does_not_match_expected_prefix_1", resourceCulture);
+            }
+        }
+        
+        /// <summary>
+        ///   Looks up a localized string similar to Prefix &apos;{0}&apos; is not expected.
+        /// </summary>
+        internal static string Prefix_0_is_not_expected {
+            get {
+                return ResourceManager.GetString("Prefix_0_is_not_expected", resourceCulture);
+            }
+        }
+        
         /// <summary>
         ///   Looks up a localized string similar to Private Method.
         /// </summary>

src/Workspaces/Core/Portable/WorkspacesResources.resx

@@ -807,6 +807,12 @@
   <data name="Parameter_preferences" xml:space="preserve">
     <value>Parameter preferences</value>
   </data>
+  <data name="Prefix_0_does_not_match_expected_prefix_1" xml:space="preserve">
+    <value>Prefix '{0}' does not match expected prefix '{1}'</value>
+  </data>
+  <data name="Prefix_0_is_not_expected" xml:space="preserve">
+    <value>Prefix '{0}' is not expected</value>
+  </data>
   <data name="DateTimeKind_must_be_Utc" xml:space="preserve">
     <value>DateTimeKind must be Utc</value>
   </data>

src/Workspaces/Core/Portable/xlf/WorkspacesResources.cs.xlf

@@ -72,6 +72,16 @@
         <target state="new">Parentheses preferences</target>
         <note />
       </trans-unit>
+      <trans-unit id="Prefix_0_does_not_match_expected_prefix_1">
+        <source>Prefix '{0}' does not match expected prefix '{1}'</source>
+        <target state="new">Prefix '{0}' does not match expected prefix '{1}'</target>
+        <note />
+      </trans-unit>
+      <trans-unit id="Prefix_0_is_not_expected">
+        <source>Prefix '{0}' is not expected</source>
+        <target state="new">Prefix '{0}' is not expected</target>
+        <note />
+      </trans-unit>
       <trans-unit id="Refactoring_Only">
         <source>Refactoring Only</source>
         <target state="translated">Pouze refaktoring</target>

src/Workspaces/Core/Portable/xlf/WorkspacesResources.de.xlf

@@ -72,6 +72,16 @@
         <target state="new">Parentheses preferences</target>
         <note />
       </trans-unit>
+      <trans-unit id="Prefix_0_does_not_match_expected_prefix_1">
+        <source>Prefix '{0}' does not match expected prefix '{1}'</source>
+        <target state="new">Prefix '{0}' does not match expected prefix '{1}'</target>
+        <note />
+      </trans-unit>
+      <trans-unit id="Prefix_0_is_not_expected">
+        <source>Prefix '{0}' is not expected</source>
+        <target state="new">Prefix '{0}' is not expected</target>
+        <note />
+      </trans-unit>
       <trans-unit id="Refactoring_Only">
         <source>Refactoring Only</source>
         <target state="translated">Nur Refactoring</target>

src/Workspaces/Core/Portable/xlf/WorkspacesResources.es.xlf

@@ -72,6 +72,16 @@
         <target state="new">Parentheses preferences</target>
         <note />
       </trans-unit>
+      <trans-unit id="Prefix_0_does_not_match_expected_prefix_1">
+        <source>Prefix '{0}' does not match expected prefix '{1}'</source>
+        <target state="new">Prefix '{0}' does not match expected prefix '{1}'</target>
+        <note />
+      </trans-unit>
+      <trans-unit id="Prefix_0_is_not_expected">
+        <source>Prefix '{0}' is not expected</source>
+        <target state="new">Prefix '{0}' is not expected</target>
+        <note />
+      </trans-unit>
       <trans-unit id="Refactoring_Only">
         <source>Refactoring Only</source>
         <target state="translated">Solo refactorización</target>

src/Workspaces/Core/Portable/xlf/WorkspacesResources.fr.xlf

@@ -72,6 +72,16 @@
         <target state="new">Parentheses preferences</target>
         <note />
       </trans-unit>
+      <trans-unit id="Prefix_0_does_not_match_expected_prefix_1">
+        <source>Prefix '{0}' does not match expected prefix '{1}'</source>
+        <target state="new">Prefix '{0}' does not match expected prefix '{1}'</target>
+        <note />
+      </trans-unit>
+      <trans-unit id="Prefix_0_is_not_expected">
+        <source>Prefix '{0}' is not expected</source>
+        <target state="new">Prefix '{0}' is not expected</target>
+        <note />
+      </trans-unit>
       <trans-unit id="Refactoring_Only">
         <source>Refactoring Only</source>
         <target state="translated">Refactorisation uniquement</target>

src/Workspaces/Core/Portable/xlf/WorkspacesResources.it.xlf

@@ -72,6 +72,16 @@
         <target state="new">Parentheses preferences</target>
         <note />
       </trans-unit>
+      <trans-unit id="Prefix_0_does_not_match_expected_prefix_1">
+        <source>Prefix '{0}' does not match expected prefix '{1}'</source>
+        <target state="new">Prefix '{0}' does not match expected prefix '{1}'</target>
+        <note />
+      </trans-unit>
+      <trans-unit id="Prefix_0_is_not_expected">
+        <source>Prefix '{0}' is not expected</source>
+        <target state="new">Prefix '{0}' is not expected</target>
+        <note />
+      </trans-unit>
       <trans-unit id="Refactoring_Only">
         <source>Refactoring Only</source>
         <target state="translated">Solo refactoring</target>

src/Workspaces/Core/Portable/xlf/WorkspacesResources.ja.xlf

@@ -72,6 +72,16 @@
         <target state="new">Parentheses preferences</target>
         <note />
       </trans-unit>
+      <trans-unit id="Prefix_0_does_not_match_expected_prefix_1">
+        <source>Prefix '{0}' does not match expected prefix '{1}'</source>
+        <target state="new">Prefix '{0}' does not match expected prefix '{1}'</target>
+        <note />
+      </trans-unit>
+      <trans-unit id="Prefix_0_is_not_expected">
+        <source>Prefix '{0}' is not expected</source>
+        <target state="new">Prefix '{0}' is not expected</target>
+        <note />
+      </trans-unit>
       <trans-unit id="Refactoring_Only">
         <source>Refactoring Only</source>
         <target state="translated">リファクタリングのみ</target>

src/Workspaces/Core/Portable/xlf/WorkspacesResources.ko.xlf

@@ -72,6 +72,16 @@
         <target state="new">Parentheses preferences</target>
         <note />
       </trans-unit>
+      <trans-unit id="Prefix_0_does_not_match_expected_prefix_1">
+        <source>Prefix '{0}' does not match expected prefix '{1}'</source>
+        <target state="new">Prefix '{0}' does not match expected prefix '{1}'</target>
+        <note />
+      </trans-unit>
+      <trans-unit id="Prefix_0_is_not_expected">
+        <source>Prefix '{0}' is not expected</source>
+        <target state="new">Prefix '{0}' is not expected</target>
+        <note />
+      </trans-unit>
       <trans-unit id="Refactoring_Only">
         <source>Refactoring Only</source>
         <target state="translated">리팩터링만</target>

src/Workspaces/Core/Portable/xlf/WorkspacesResources.pl.xlf

@@ -72,6 +72,16 @@
         <target state="new">Parentheses preferences</target>
         <note />
       </trans-unit>
+      <trans-unit id="Prefix_0_does_not_match_expected_prefix_1">
+        <source>Prefix '{0}' does not match expected prefix '{1}'</source>
+        <target state="new">Prefix '{0}' does not match expected prefix '{1}'</target>
+        <note />
+      </trans-unit>
+      <trans-unit id="Prefix_0_is_not_expected">
+        <source>Prefix '{0}' is not expected</source>
+        <target state="new">Prefix '{0}' is not expected</target>
+        <note />
+      </trans-unit>
       <trans-unit id="Refactoring_Only">
         <source>Refactoring Only</source>
         <target state="translated">Tylko refaktoryzacja</target>

src/Workspaces/Core/Portable/xlf/WorkspacesResources.pt-BR.xlf

@@ -72,6 +72,16 @@
         <target state="new">Parentheses preferences</target>
         <note />
       </trans-unit>
+      <trans-unit id="Prefix_0_does_not_match_expected_prefix_1">
+        <source>Prefix '{0}' does not match expected prefix '{1}'</source>
+        <target state="new">Prefix '{0}' does not match expected prefix '{1}'</target>
+        <note />
+      </trans-unit>
+      <trans-unit id="Prefix_0_is_not_expected">
+        <source>Prefix '{0}' is not expected</source>
+        <target state="new">Prefix '{0}' is not expected</target>
+        <note />
+      </trans-unit>
       <trans-unit id="Refactoring_Only">
         <source>Refactoring Only</source>
         <target state="translated">Somente Refatoração</target>

src/Workspaces/Core/Portable/xlf/WorkspacesResources.ru.xlf

@@ -72,6 +72,16 @@
         <target state="new">Parentheses preferences</target>
         <note />
       </trans-unit>
+      <trans-unit id="Prefix_0_does_not_match_expected_prefix_1">
+        <source>Prefix '{0}' does not match expected prefix '{1}'</source>
+        <target state="new">Prefix '{0}' does not match expected prefix '{1}'</target>
+        <note />
+      </trans-unit>
+      <trans-unit id="Prefix_0_is_not_expected">
+        <source>Prefix '{0}' is not expected</source>
+        <target state="new">Prefix '{0}' is not expected</target>
+        <note />
+      </trans-unit>
       <trans-unit id="Refactoring_Only">
         <source>Refactoring Only</source>
         <target state="translated">Только рефакторинг</target>

src/Workspaces/Core/Portable/xlf/WorkspacesResources.tr.xlf

@@ -72,6 +72,16 @@
         <target state="new">Parentheses preferences</target>
         <note />
       </trans-unit>
+      <trans-unit id="Prefix_0_does_not_match_expected_prefix_1">
+        <source>Prefix '{0}' does not match expected prefix '{1}'</source>
+        <target state="new">Prefix '{0}' does not match expected prefix '{1}'</target>
+        <note />
+      </trans-unit>
+      <trans-unit id="Prefix_0_is_not_expected">
+        <source>Prefix '{0}' is not expected</source>
+        <target state="new">Prefix '{0}' is not expected</target>
+        <note />
+      </trans-unit>
       <trans-unit id="Refactoring_Only">
         <source>Refactoring Only</source>
         <target state="translated">Sadece Yeniden Düzenlenme</target>

src/Workspaces/Core/Portable/xlf/WorkspacesResources.zh-Hans.xlf

@@ -72,6 +72,16 @@
         <target state="new">Parentheses preferences</target>
         <note />
       </trans-unit>
+      <trans-unit id="Prefix_0_does_not_match_expected_prefix_1">
+        <source>Prefix '{0}' does not match expected prefix '{1}'</source>
+        <target state="new">Prefix '{0}' does not match expected prefix '{1}'</target>
+        <note />
+      </trans-unit>
+      <trans-unit id="Prefix_0_is_not_expected">
+        <source>Prefix '{0}' is not expected</source>
+        <target state="new">Prefix '{0}' is not expected</target>
+        <note />
+      </trans-unit>
       <trans-unit id="Refactoring_Only">
         <source>Refactoring Only</source>
         <target state="translated">仅重构</target>

src/Workspaces/Core/Portable/xlf/WorkspacesResources.zh-Hant.xlf

@@ -72,6 +72,16 @@
         <target state="new">Parentheses preferences</target>
         <note />
       </trans-unit>
+      <trans-unit id="Prefix_0_does_not_match_expected_prefix_1">
+        <source>Prefix '{0}' does not match expected prefix '{1}'</source>
+        <target state="new">Prefix '{0}' does not match expected prefix '{1}'</target>
+        <note />
+      </trans-unit>
+      <trans-unit id="Prefix_0_is_not_expected">
+        <source>Prefix '{0}' is not expected</source>
+        <target state="new">Prefix '{0}' is not expected</target>
+        <note />
+      </trans-unit>
       <trans-unit id="Refactoring_Only">
         <source>Refactoring Only</source>
         <target state="translated">僅重構</target>