Merge pull request #2596 from BZngr/next

retailcoder · web-flow · commit 82c9151110a2 · 2017-01-30T16:46:33.000-05:00
#2260 Meaningful names inspection doesn't catch consecutive vowels
diff --git a/RetailCoder.VBE/Inspections/UseMeaningfulNameInspection.cs b/RetailCoder.VBE/Inspections/UseMeaningfulNameInspection.cs
@@ -62,7 +62,8 @@ private static bool IsBadIdentifier(string identifier)
         {
             return identifier.Length < 3 ||
                    char.IsDigit(identifier.Last()) ||
-                   !HasVowels(identifier);
+                   !HasVowels(identifier) ||
+                    NameIsASingleRepeatedLetter(identifier);
         }
 
         private static bool HasVowels(string identifier)
@@ -71,5 +72,11 @@ private static bool HasVowels(string identifier)
             return  identifier.Any(character => vowels.Any(vowel => 
                     string.Compare(vowel.ToString(), character.ToString(), StringComparison.OrdinalIgnoreCase) == 0));
         }
+        private static bool NameIsASingleRepeatedLetter(string identifierName)
+        {
+            string firstLetter = identifierName.First().ToString();
+            return identifierName.All(a => string.Compare(a.ToString(), firstLetter,
+                StringComparison.OrdinalIgnoreCase) == 0);
+        }
     }
 }
diff --git a/RubberduckTests/Inspections/UseMeaningfulNameInspectionTests.cs b/RubberduckTests/Inspections/UseMeaningfulNameInspectionTests.cs
@@ -20,30 +20,36 @@ public class UseMeaningfulNameInspectionTests
     {
         [TestMethod]
         [TestCategory("Inspections")]
-        public void UseMeaningfulName_ReturnsResult_NameWithoutVowels()
+        public void UseMeaningfulName_ReturnsResult_NameWithAllTheSameLetters()
         {
-            const string inputCode = 
-@"Sub Ffffff()
+            const string inputCode =
+@"
+Private aaa As String
+Private bbb As String 
+Private ccc As String
+Private ddd As String
+Private eee As String
+Private iii As String
+Private ooo As String
+Private uuu As String
+
+Sub Eeeeee()
+Dim a2z as String       'This is the only declaration that should pass
+Dim gGGG as String
 End Sub";
 
-            //Arrange
-            var builder = new MockVbeBuilder();
-            var project = builder.ProjectBuilder("VBAProject", ProjectProtection.Unprotected)
-                .AddComponent("MyClass", ComponentType.ClassModule, inputCode)
-                .Build();
-            var vbe = builder.AddProject(project).Build();
-
-            var mockHost = new Mock<IHostApplication>();
-            mockHost.SetupAllProperties();
-            var parser = MockParser.Create(vbe.Object, new RubberduckParserState(new Mock<ISinks>().Object));
-
-            parser.Parse(new CancellationTokenSource());
-            if (parser.State.Status >= ParserState.Error) { Assert.Inconclusive("Parser Error"); }
+            AssertVbaFragmentYieldsExpectedInspectionResultCount(inputCode, 10);
+        }
 
-            var inspection = new UseMeaningfulNameInspection(null, parser.State, GetInspectionSettings().Object);
-            var inspectionResults = inspection.GetInspectionResults();
 
-            Assert.AreEqual(1, inspectionResults.Count());
+        [TestMethod]
+        [TestCategory("Inspections")]
+        public void UseMeaningfulName_ReturnsResult_NameWithoutVowels()
+        {
+            const string inputCode = 
+@"Sub Ffffff()
+End Sub";
+            AssertVbaFragmentYieldsExpectedInspectionResultCount(inputCode, 1);
         }
 
         [TestMethod]
@@ -53,25 +59,7 @@ public void UseMeaningfulName_ReturnsResult_NameUnderThreeLetters()
             const string inputCode =
 @"Sub Oo()
 End Sub";
-
-            //Arrange
-            var builder = new MockVbeBuilder();
-            var project = builder.ProjectBuilder("VBAProject", ProjectProtection.Unprotected)
-                .AddComponent("MyClass", ComponentType.ClassModule, inputCode)
-                .Build();
-            var vbe = builder.AddProject(project).Build();
-
-            var mockHost = new Mock<IHostApplication>();
-            mockHost.SetupAllProperties();
-            var parser = MockParser.Create(vbe.Object, new RubberduckParserState(new Mock<ISinks>().Object));
-
-            parser.Parse(new CancellationTokenSource());
-            if (parser.State.Status >= ParserState.Error) { Assert.Inconclusive("Parser Error"); }
-
-            var inspection = new UseMeaningfulNameInspection(null, parser.State, GetInspectionSettings().Object);
-            var inspectionResults = inspection.GetInspectionResults();
-
-            Assert.AreEqual(1, inspectionResults.Count());
+            AssertVbaFragmentYieldsExpectedInspectionResultCount(inputCode, 1);
         }
 
         [TestMethod]
@@ -82,24 +70,7 @@ public void UseMeaningfulName_ReturnsResult_NameEndsWithDigit()
 @"Sub Foo1()
 End Sub";
 
-            //Arrange
-            var builder = new MockVbeBuilder();
-            var project = builder.ProjectBuilder("VBAProject", ProjectProtection.Unprotected)
-                .AddComponent("MyClass", ComponentType.ClassModule, inputCode)
-                .Build();
-            var vbe = builder.AddProject(project).Build();
-
-            var mockHost = new Mock<IHostApplication>();
-            mockHost.SetupAllProperties();
-            var parser = MockParser.Create(vbe.Object, new RubberduckParserState(new Mock<ISinks>().Object));
-
-            parser.Parse(new CancellationTokenSource());
-            if (parser.State.Status >= ParserState.Error) { Assert.Inconclusive("Parser Error"); }
-
-            var inspection = new UseMeaningfulNameInspection(null, parser.State, GetInspectionSettings().Object);
-            var inspectionResults = inspection.GetInspectionResults();
-
-            Assert.AreEqual(1, inspectionResults.Count());
+            AssertVbaFragmentYieldsExpectedInspectionResultCount(inputCode, 1);
         }
 
         [TestMethod]
@@ -110,24 +81,7 @@ public void UseMeaningfulName_DoesNotReturnsResult_GoodName_LowerCaseVowels()
 @"Sub FooBar()
 End Sub";
 
-            //Arrange
-            var builder = new MockVbeBuilder();
-            var project = builder.ProjectBuilder("VBAProject", ProjectProtection.Unprotected)
-                .AddComponent("MyClass", ComponentType.ClassModule, inputCode)
-                .Build();
-            var vbe = builder.AddProject(project).Build();
-
-            var mockHost = new Mock<IHostApplication>();
-            mockHost.SetupAllProperties();
-            var parser = MockParser.Create(vbe.Object, new RubberduckParserState(new Mock<ISinks>().Object));
-
-            parser.Parse(new CancellationTokenSource());
-            if (parser.State.Status >= ParserState.Error) { Assert.Inconclusive("Parser Error"); }
-
-            var inspection = new UseMeaningfulNameInspection(null, parser.State, GetInspectionSettings().Object);
-            var inspectionResults = inspection.GetInspectionResults();
-
-            Assert.AreEqual(0, inspectionResults.Count());
+            AssertVbaFragmentYieldsExpectedInspectionResultCount(inputCode, 0);
         }
 
         [TestMethod]
@@ -138,24 +92,7 @@ public void UseMeaningfulName_DoesNotReturnsResult_GoodName_UpperCaseVowels()
 @"Sub FOOBAR()
 End Sub";
 
-            //Arrange
-            var builder = new MockVbeBuilder();
-            var project = builder.ProjectBuilder("VBAProject", ProjectProtection.Unprotected)
-                .AddComponent("MyClass", ComponentType.ClassModule, inputCode)
-                .Build();
-            var vbe = builder.AddProject(project).Build();
-
-            var mockHost = new Mock<IHostApplication>();
-            mockHost.SetupAllProperties();
-            var parser = MockParser.Create(vbe.Object, new RubberduckParserState(new Mock<ISinks>().Object));
-
-            parser.Parse(new CancellationTokenSource());
-            if (parser.State.Status >= ParserState.Error) { Assert.Inconclusive("Parser Error"); }
-
-            var inspection = new UseMeaningfulNameInspection(null, parser.State, GetInspectionSettings().Object);
-            var inspectionResults = inspection.GetInspectionResults();
-
-            Assert.AreEqual(0, inspectionResults.Count());
+            AssertVbaFragmentYieldsExpectedInspectionResultCount(inputCode, 0);
         }
 
         [TestMethod]
@@ -165,24 +102,7 @@ public void UseMeaningfulName_DoesNotReturnsResult_OptionBase()
             const string inputCode =
 @"Option Base 1";
 
-            //Arrange
-            var builder = new MockVbeBuilder();
-            var project = builder.ProjectBuilder("VBAProject", ProjectProtection.Unprotected)
-                .AddComponent("MyClass", ComponentType.ClassModule, inputCode)
-                .Build();
-            var vbe = builder.AddProject(project).Build();
-
-            var mockHost = new Mock<IHostApplication>();
-            mockHost.SetupAllProperties();
-            var parser = MockParser.Create(vbe.Object, new RubberduckParserState(new Mock<ISinks>().Object));
-
-            parser.Parse(new CancellationTokenSource());
-            if (parser.State.Status >= ParserState.Error) { Assert.Inconclusive("Parser Error"); }
-
-            var inspection = new UseMeaningfulNameInspection(null, parser.State, GetInspectionSettings().Object);
-            var inspectionResults = inspection.GetInspectionResults();
-
-            Assert.AreEqual(0, inspectionResults.Count());
+            AssertVbaFragmentYieldsExpectedInspectionResultCount(inputCode, 0);
         }
 
         [TestMethod]
@@ -193,24 +113,7 @@ public void UseMeaningfulName_DoesNotReturnResult_NameWithoutVowels_NameIsInWhit
 @"Sub sss()
 End Sub";
 
-            //Arrange
-            var builder = new MockVbeBuilder();
-            var project = builder.ProjectBuilder("VBAProject", ProjectProtection.Unprotected)
-                .AddComponent("MyClass", ComponentType.ClassModule, inputCode)
-                .Build();
-            var vbe = builder.AddProject(project).Build();
-
-            var mockHost = new Mock<IHostApplication>();
-            mockHost.SetupAllProperties();
-            var parser = MockParser.Create(vbe.Object, new RubberduckParserState(new Mock<ISinks>().Object));
-
-            parser.Parse(new CancellationTokenSource());
-            if (parser.State.Status >= ParserState.Error) { Assert.Inconclusive("Parser Error"); }
-
-            var inspection = new UseMeaningfulNameInspection(null, parser.State, GetInspectionSettings().Object);
-            var inspectionResults = inspection.GetInspectionResults();
-
-            Assert.IsFalse(inspectionResults.Any());
+            AssertVbaFragmentYieldsExpectedInspectionResultCount(inputCode, 0);
         }
 
         [TestMethod]
@@ -222,24 +125,7 @@ public void UseMeaningfulName_Ignored_DoesNotReturnResult()
 Sub Ffffff()
 End Sub";
 
-            //Arrange
-            var builder = new MockVbeBuilder();
-            var project = builder.ProjectBuilder("VBAProject", ProjectProtection.Unprotected)
-                .AddComponent("MyClass", ComponentType.ClassModule, inputCode)
-                .Build();
-            var vbe = builder.AddProject(project).Build();
-
-            var mockHost = new Mock<IHostApplication>();
-            mockHost.SetupAllProperties();
-            var parser = MockParser.Create(vbe.Object, new RubberduckParserState(new Mock<ISinks>().Object));
-
-            parser.Parse(new CancellationTokenSource());
-            if (parser.State.Status >= ParserState.Error) { Assert.Inconclusive("Parser Error"); }
-
-            var inspection = new UseMeaningfulNameInspection(null, parser.State, GetInspectionSettings().Object);
-            var inspectionResults = inspection.GetInspectionResults();
-
-            Assert.IsFalse(inspectionResults.Any());
+            AssertVbaFragmentYieldsExpectedInspectionResultCount(inputCode, 0);
         }
 
         [TestMethod]
@@ -272,7 +158,6 @@ Sub Ffffff()
 
             var inspection = new UseMeaningfulNameInspection(null, parser.State, GetInspectionSettings().Object);
             var inspectionResults = inspection.GetInspectionResults();
-
             inspectionResults.First().QuickFixes.Single(s => s is IgnoreOnceQuickFix).Fix();
 
             Assert.AreEqual(expectedCode, module.Content());
@@ -296,6 +181,26 @@ public void InspectionName()
             Assert.AreEqual(inspectionName, inspection.Name);
         }
 
+        private void AssertVbaFragmentYieldsExpectedInspectionResultCount(string inputCode, int expectedCount)
+        {
+            var builder = new MockVbeBuilder();
+            var project = builder.ProjectBuilder("VBAProject", ProjectProtection.Unprotected)
+                .AddComponent("MyClass", ComponentType.ClassModule, inputCode)
+                .Build();
+            var vbe = builder.AddProject(project).Build();
+
+            var mockHost = new Mock<IHostApplication>();
+            mockHost.SetupAllProperties();
+            var parser = MockParser.Create(vbe.Object, new RubberduckParserState(new Mock<ISinks>().Object));
+
+            parser.Parse(new CancellationTokenSource());
+            if (parser.State.Status >= ParserState.Error) { Assert.Inconclusive("Parser Error"); }
+
+            var inspection = new UseMeaningfulNameInspection(null, parser.State, GetInspectionSettings().Object);
+            var inspectionResults = inspection.GetInspectionResults();
+            Assert.AreEqual(expectedCount, inspectionResults.Count());
+        }
+
         internal static Mock<IPersistanceService<CodeInspectionSettings>> GetInspectionSettings()
         {
             var settings = new Mock<IPersistanceService<CodeInspectionSettings>>();

Original file line number	Diff line number	Diff line change
`@@ -62,7 +62,8 @@ private static bool IsBadIdentifier(string identifier)`
`62`	`62`	`{`
`63`	`63`	`return identifier.Length < 3 \|\|`
`64`	`64`	`char.IsDigit(identifier.Last()) \|\|`
`65`		`- !HasVowels(identifier);`
	`65`	`+ !HasVowels(identifier) \|\|`
	`66`	`+ NameIsASingleRepeatedLetter(identifier);`
`66`	`67`	`}`
`67`	`68`
`68`	`69`	`private static bool HasVowels(string identifier)`
`@@ -71,5 +72,11 @@ private static bool HasVowels(string identifier)`
`71`	`72`	`return identifier.Any(character => vowels.Any(vowel =>`
`72`	`73`	`string.Compare(vowel.ToString(), character.ToString(), StringComparison.OrdinalIgnoreCase) == 0));`
`73`	`74`	`}`
	`75`	`+ private static bool NameIsASingleRepeatedLetter(string identifierName)`
	`76`	`+ {`
	`77`	`+ string firstLetter = identifierName.First().ToString();`
	`78`	`+ return identifierName.All(a => string.Compare(a.ToString(), firstLetter,`
	`79`	`+ StringComparison.OrdinalIgnoreCase) == 0);`
	`80`	`+ }`
`74`	`81`	`}`
`75`	`82`	`}`