Fix MSTEST0036 is shown for cases where no shadowing happens (#3881)

microsoft · Sep 30, 2024 · 491b2d6 · 491b2d6
1 parent 8d3545b
commit 491b2d6
Show file tree

Hide file tree

Showing 17 changed files with 118 additions and 29 deletions.
diff --git a/src/Analyzers/MSTest.Analyzers/DoNotUseShadowingAnalyzer.cs b/src/Analyzers/MSTest.Analyzers/DoNotUseShadowingAnalyzer.cs
@@ -112,7 +112,8 @@ private static bool IsMemberShadowing(ISymbol member, ISymbol baseMember)
         }
 
         // Compare methods
-        if (member is IMethodSymbol methodSymbol && baseMember is IMethodSymbol baseMethodSymbol)
+        if (member is IMethodSymbol methodSymbol && baseMember is IMethodSymbol baseMethodSymbol && methodSymbol.IsGenericMethod == baseMethodSymbol.IsGenericMethod
+            && !(methodSymbol.DeclaredAccessibility == Accessibility.Private && baseMethodSymbol.DeclaredAccessibility == Accessibility.Private))
         {
             return methodSymbol.Name == baseMethodSymbol.Name &&
                    methodSymbol.Parameters.Length == baseMethodSymbol.Parameters.Length &&

diff --git a/src/Analyzers/MSTest.Analyzers/Resources.Designer.cs b/src/Analyzers/MSTest.Analyzers/Resources.Designer.cs
diff --git a/src/Analyzers/MSTest.Analyzers/Resources.resx b/src/Analyzers/MSTest.Analyzers/Resources.resx
@@ -524,7 +524,7 @@ The type declaring these methods should also respect the following rules:
     <value>Shadowing test members could cause testing issues (such as NRE).</value>
   </data>
   <data name="DoNotUseShadowingMessageFormat" xml:space="preserve">
-    <value>Member '{0}' is already exist in the base class</value>
+    <value>Member '{0}' already exists in the base class</value>
   </data>
   <data name="DoNotUseShadowingTitle" xml:space="preserve">
     <value>Do not use shadowing</value>

diff --git a/src/Analyzers/MSTest.Analyzers/xlf/Resources.cs.xlf b/src/Analyzers/MSTest.Analyzers/xlf/Resources.cs.xlf
@@ -287,8 +287,8 @@ Typ deklarující tyto metody by měl také respektovat následující pravidla:
         <note />
       </trans-unit>
       <trans-unit id="DoNotUseShadowingMessageFormat">
-        <source>Member '{0}' is already exist in the base class</source>
-        <target state="translated">Člen {0} už v základní (kořenové) třídě existuje.</target>
+        <source>Member '{0}' already exists in the base class</source>
+        <target state="needs-review-translation">Člen {0} už v základní (kořenové) třídě existuje.</target>
         <note />
       </trans-unit>
       <trans-unit id="DoNotUseShadowingTitle">

diff --git a/src/Analyzers/MSTest.Analyzers/xlf/Resources.de.xlf b/src/Analyzers/MSTest.Analyzers/xlf/Resources.de.xlf
@@ -289,8 +289,8 @@ Der Typ, der diese Methoden deklariert, sollte auch die folgenden Regeln beachte
         <note />
       </trans-unit>
       <trans-unit id="DoNotUseShadowingMessageFormat">
-        <source>Member '{0}' is already exist in the base class</source>
-        <target state="translated">Der Member "{0}" ist bereits in der Basisklasse vorhanden.</target>
+        <source>Member '{0}' already exists in the base class</source>
+        <target state="needs-review-translation">Der Member "{0}" ist bereits in der Basisklasse vorhanden.</target>
         <note />
       </trans-unit>
       <trans-unit id="DoNotUseShadowingTitle">

diff --git a/src/Analyzers/MSTest.Analyzers/xlf/Resources.es.xlf b/src/Analyzers/MSTest.Analyzers/xlf/Resources.es.xlf
@@ -287,8 +287,8 @@ El tipo que declara estos métodos también debe respetar las reglas siguientes:
         <note />
       </trans-unit>
       <trans-unit id="DoNotUseShadowingMessageFormat">
-        <source>Member '{0}' is already exist in the base class</source>
-        <target state="translated">El miembro ''{0}'' ya existe en la clase base</target>
+        <source>Member '{0}' already exists in the base class</source>
+        <target state="needs-review-translation">El miembro ''{0}'' ya existe en la clase base</target>
         <note />
       </trans-unit>
       <trans-unit id="DoNotUseShadowingTitle">

diff --git a/src/Analyzers/MSTest.Analyzers/xlf/Resources.fr.xlf b/src/Analyzers/MSTest.Analyzers/xlf/Resources.fr.xlf
@@ -287,8 +287,8 @@ Le type doit être une classe
         <note />
       </trans-unit>
       <trans-unit id="DoNotUseShadowingMessageFormat">
-        <source>Member '{0}' is already exist in the base class</source>
-        <target state="translated">Le membre '{0}' existe déjà dans la classe de base</target>
+        <source>Member '{0}' already exists in the base class</source>
+        <target state="needs-review-translation">Le membre '{0}' existe déjà dans la classe de base</target>
         <note />
       </trans-unit>
       <trans-unit id="DoNotUseShadowingTitle">

diff --git a/src/Analyzers/MSTest.Analyzers/xlf/Resources.it.xlf b/src/Analyzers/MSTest.Analyzers/xlf/Resources.it.xlf
@@ -287,8 +287,8 @@ Anche il tipo che dichiara questi metodi deve rispettare le regole seguenti:
         <note />
       </trans-unit>
       <trans-unit id="DoNotUseShadowingMessageFormat">
-        <source>Member '{0}' is already exist in the base class</source>
-        <target state="translated">Il membro '{0}' esiste già nella classe di base</target>
+        <source>Member '{0}' already exists in the base class</source>
+        <target state="needs-review-translation">Il membro '{0}' esiste già nella classe di base</target>
         <note />
       </trans-unit>
       <trans-unit id="DoNotUseShadowingTitle">

diff --git a/src/Analyzers/MSTest.Analyzers/xlf/Resources.ja.xlf b/src/Analyzers/MSTest.Analyzers/xlf/Resources.ja.xlf
@@ -287,8 +287,8 @@ The type declaring these methods should also respect the following rules:
         <note />
       </trans-unit>
       <trans-unit id="DoNotUseShadowingMessageFormat">
-        <source>Member '{0}' is already exist in the base class</source>
-        <target state="translated">メンバー '{0}' は既に基本クラスに存在します</target>
+        <source>Member '{0}' already exists in the base class</source>
+        <target state="needs-review-translation">メンバー '{0}' は既に基本クラスに存在します</target>
         <note />
       </trans-unit>
       <trans-unit id="DoNotUseShadowingTitle">

diff --git a/src/Analyzers/MSTest.Analyzers/xlf/Resources.ko.xlf b/src/Analyzers/MSTest.Analyzers/xlf/Resources.ko.xlf
@@ -287,8 +287,8 @@ The type declaring these methods should also respect the following rules:
         <note />
       </trans-unit>
       <trans-unit id="DoNotUseShadowingMessageFormat">
-        <source>Member '{0}' is already exist in the base class</source>
-        <target state="translated">멤버 '{0}'이(가) 기본 클래스에 이미 있습니다.</target>
+        <source>Member '{0}' already exists in the base class</source>
+        <target state="needs-review-translation">멤버 '{0}'이(가) 기본 클래스에 이미 있습니다.</target>
         <note />
       </trans-unit>
       <trans-unit id="DoNotUseShadowingTitle">

diff --git a/src/Analyzers/MSTest.Analyzers/xlf/Resources.pl.xlf b/src/Analyzers/MSTest.Analyzers/xlf/Resources.pl.xlf
@@ -287,8 +287,8 @@ Typ deklarujący te metody powinien również przestrzegać następujących regu
         <note />
       </trans-unit>
       <trans-unit id="DoNotUseShadowingMessageFormat">
-        <source>Member '{0}' is already exist in the base class</source>
-        <target state="translated">Element członkowski „{0}” już istnieje w klasie bazowej</target>
+        <source>Member '{0}' already exists in the base class</source>
+        <target state="needs-review-translation">Element członkowski „{0}” już istnieje w klasie bazowej</target>
         <note />
       </trans-unit>
       <trans-unit id="DoNotUseShadowingTitle">

diff --git a/src/Analyzers/MSTest.Analyzers/xlf/Resources.pt-BR.xlf b/src/Analyzers/MSTest.Analyzers/xlf/Resources.pt-BR.xlf
@@ -287,8 +287,8 @@ O tipo que declara esses métodos também deve respeitar as seguintes regras:
         <note />
       </trans-unit>
       <trans-unit id="DoNotUseShadowingMessageFormat">
-        <source>Member '{0}' is already exist in the base class</source>
-        <target state="translated">O membro "{0}" já existe na classe base</target>
+        <source>Member '{0}' already exists in the base class</source>
+        <target state="needs-review-translation">O membro "{0}" já existe na classe base</target>
         <note />
       </trans-unit>
       <trans-unit id="DoNotUseShadowingTitle">

diff --git a/src/Analyzers/MSTest.Analyzers/xlf/Resources.ru.xlf b/src/Analyzers/MSTest.Analyzers/xlf/Resources.ru.xlf
@@ -291,8 +291,8 @@ The type declaring these methods should also respect the following rules:
         <note />
       </trans-unit>
       <trans-unit id="DoNotUseShadowingMessageFormat">
-        <source>Member '{0}' is already exist in the base class</source>
-        <target state="translated">Элемент "{0}" уже существует в базовом классе</target>
+        <source>Member '{0}' already exists in the base class</source>
+        <target state="needs-review-translation">Элемент "{0}" уже существует в базовом классе</target>
         <note />
       </trans-unit>
       <trans-unit id="DoNotUseShadowingTitle">

diff --git a/src/Analyzers/MSTest.Analyzers/xlf/Resources.tr.xlf b/src/Analyzers/MSTest.Analyzers/xlf/Resources.tr.xlf
@@ -287,8 +287,8 @@ Bu yöntemleri bildiren tipin ayrıca aşağıdaki kurallara uyması gerekir:
         <note />
       </trans-unit>
       <trans-unit id="DoNotUseShadowingMessageFormat">
-        <source>Member '{0}' is already exist in the base class</source>
-        <target state="translated">Üye '{0}' zaten temel sınıfta var</target>
+        <source>Member '{0}' already exists in the base class</source>
+        <target state="needs-review-translation">Üye '{0}' zaten temel sınıfta var</target>
         <note />
       </trans-unit>
       <trans-unit id="DoNotUseShadowingTitle">

diff --git a/src/Analyzers/MSTest.Analyzers/xlf/Resources.zh-Hans.xlf b/src/Analyzers/MSTest.Analyzers/xlf/Resources.zh-Hans.xlf
@@ -287,8 +287,8 @@ The type declaring these methods should also respect the following rules:
         <note />
       </trans-unit>
       <trans-unit id="DoNotUseShadowingMessageFormat">
-        <source>Member '{0}' is already exist in the base class</source>
-        <target state="translated">基类中已存在成员“{0}”</target>
+        <source>Member '{0}' already exists in the base class</source>
+        <target state="needs-review-translation">基类中已存在成员“{0}”</target>
         <note />
       </trans-unit>
       <trans-unit id="DoNotUseShadowingTitle">

diff --git a/src/Analyzers/MSTest.Analyzers/xlf/Resources.zh-Hant.xlf b/src/Analyzers/MSTest.Analyzers/xlf/Resources.zh-Hant.xlf
@@ -287,8 +287,8 @@ The type declaring these methods should also respect the following rules:
         <note />
       </trans-unit>
       <trans-unit id="DoNotUseShadowingMessageFormat">
-        <source>Member '{0}' is already exist in the base class</source>
-        <target state="translated">成員 '{0}' 已存在於基類中</target>
+        <source>Member '{0}' already exists in the base class</source>
+        <target state="needs-review-translation">成員 '{0}' 已存在於基類中</target>
         <note />
       </trans-unit>
       <trans-unit id="DoNotUseShadowingTitle">

diff --git a/test/UnitTests/MSTest.Analyzers.UnitTests/DoNotUseShadowingAnalyzerTests.cs b/test/UnitTests/MSTest.Analyzers.UnitTests/DoNotUseShadowingAnalyzerTests.cs
@@ -48,6 +48,94 @@ public void Method() { }
         await VerifyCS.VerifyAnalyzerAsync(code);
     }
 
+    public async Task WhenTestClassHaveSameMethodAsBaseClassMethod_ButBothArePrivate_NoDiagnostic()
+    {
+        string code = """
+            using Microsoft.VisualStudio.TestTools.UnitTesting;
+            public class BaseClass
+            {
+                private void Method() { }
+            }
+
+            [TestClass]
+            public class DerivedClass : BaseClass
+            {
+                private void Method() { }
+            }
+            """;
+
+        await VerifyCS.VerifyAnalyzerAsync(code);
+    }
+
+    public async Task WhenTestClassHaveSameMethodAsBaseClassMethod_ButBaseIsPrivate_Diagnostic()
+    {
+        string code = """
+            using Microsoft.VisualStudio.TestTools.UnitTesting;
+            public class BaseClass
+            {
+                private void Method() { }
+            }
+
+            [TestClass]
+            public class DerivedClass : BaseClass
+            {
+                public void [|Method|]() { }
+            }
+            """;
+
+        await VerifyCS.VerifyAnalyzerAsync(code);
+    }
+
+    public async Task WhenTestClassHaveSameMethodAsBaseClassMethod_ButDerivedClassIsPrivate_Diagnostic()
+    {
+        string code = """
+            using Microsoft.VisualStudio.TestTools.UnitTesting;
+            public class BaseClass
+            {
+                public void Method() { }
+            }
+
+            [TestClass]
+            public class DerivedClass : BaseClass
+            {
+                private void [|Method|]() { }
+            }
+            """;
+
+        await VerifyCS.VerifyAnalyzerAsync(code);
+    }
+
+    public async Task WhenTestClassHaveSameMethodAsBaseClassMethod_ButOneIsGeneric_NoDiagnostic()
+    {
+        string code = """
+            using Microsoft.VisualStudio.TestTools.UnitTesting;
+            using System;
+            [TestClass]
+            public class BaseTest
+            {
+                protected TObject CreateObject<TObject>()
+                {
+                    throw new NotImplementedException();
+                }
+            }
+
+            [TestClass]
+            public class ExtendedTest : BaseTest
+            {
+                private SomeType CreateObject()
+                {
+                    throw new NotImplementedException();
+                }
+            }
+
+            public class SomeType
+            {
+            }
+            """;
+
+        await VerifyCS.VerifyAnalyzerAsync(code);
+    }
+
     public async Task WhenTestClassHaveSameMethodAsBaseClassMethod_WithParameters_Diagnostic()
     {
         string code = """