Merge pull request #225 from WiseTechGlobal/MNJ/WI00764889/WTG1001_Exclude_NonVoid_Partial_Method

Cover WTG1001 on edge cases

Author: mj-wtg

src/WTG.Analyzers.Test/TestData/VisibilityAnalyzer/Partial/Diagnostics.xml

@@ -1,10 +1,17 @@
 <?xml version="1.0" encoding="utf-8" ?>
 <diagnostics severity="Hidden">
 	<languageVersion>9.0</languageVersion>
+	<suppressId>CS8799</suppressId>
 	<diagnostic id="WTG1001" message="Our convention is to omit the 'private' modifier where it is already the default.">
-		<location>Test0.cs: (15,3-10)</location>
+		<location>Test0.cs: (9,3-10)</location>
+	</diagnostic>
+	<diagnostic id="WTG1001" message="Our convention is to omit the 'private' modifier where it is already the default.">
+		<location>Test0.cs: (19,3-10)</location>
+	</diagnostic>
+	<diagnostic id="WTG1001" message="Our convention is to omit the 'private' modifier where it is already the default.">
+		<location>Test0.cs: (25,3-10)</location>
 	</diagnostic>
 	<diagnostic id="WTG1006" message="Our convention is to omit the 'internal' modifier on types where it is already the default." >
-		<location>Test0.cs: (20,2-10)</location>
+		<location>Test0.cs: (30,2-10)</location>
 	</diagnostic>
 </diagnostics>

src/WTG.Analyzers.Test/TestData/VisibilityAnalyzer/Partial/Result.cs

@@ -3,11 +3,21 @@ namespace ns
 	partial class Foo
 	{
 		private partial int Bar();
+		public partial void FooBar();
+		private partial int FooBarBaz(out int value);
+
+		partial void Qux();
+		private partial void Quux(out int value);
 	}
 
 	partial class Foo
 	{
 		private partial int Bar() { return default; }
+		public partial void FooBar() { }
+		private partial int FooBarBaz(out int value) => throw null;
+
+		partial void Qux() { }
+		private partial void Quux(out int value) { value = default; }
 	}
 
 	class Outer

src/WTG.Analyzers.Test/TestData/VisibilityAnalyzer/Partial/Source.cs

@@ -3,11 +3,21 @@ namespace ns
 	partial class Foo
 	{
 		private partial int Bar();
+		public partial void FooBar();
+		private partial int FooBarBaz(out int value);
+
+		private partial void Qux();
+		private partial void Quux(out int value);
 	}
 
 	partial class Foo
 	{
 		private partial int Bar() { return default; }
+		public partial void FooBar() { }
+		private partial int FooBarBaz(out int value) => throw null;
+
+		private partial void Qux() { }
+		private partial void Quux(out int value) { value = default; }
 	}
 
 	class Outer

src/WTG.Analyzers/Analyzers/Visibility/VisibilityAnalyzer.cs

@@ -1,6 +1,8 @@
 using System.Collections.Immutable;
+using System.Linq;
 using Microsoft.CodeAnalysis;
 using Microsoft.CodeAnalysis.CSharp;
+using Microsoft.CodeAnalysis.CSharp.Syntax;
 using Microsoft.CodeAnalysis.Diagnostics;
 using WTG.Analyzers.Utils;
 
@@ -32,7 +34,8 @@ static void Analyze(SyntaxNodeAnalysisContext context, FileDetailCache cache)
 				return;
 			}
 
-			var list = ModifierExtractionVisitor.Instance.Visit(context.Node);
+			var currentNode = context.Node;
+			var list = ModifierExtractionVisitor.Instance.Visit(currentNode);
 			var privateToken = default(SyntaxToken);
 
 			foreach (var modifier in list)
@@ -47,11 +50,10 @@ static void Analyze(SyntaxNodeAnalysisContext context, FileDetailCache cache)
 
 					case SyntaxKind.ProtectedKeyword:
 					case SyntaxKind.PublicKeyword:
-					case SyntaxKind.PartialKeyword when (context.Node.IsKind(SyntaxKind.MethodDeclaration)):
+					case SyntaxKind.PartialKeyword when (PartialMethodRequiresAccessibilityModifier(currentNode)):
 						return;
-
 					case SyntaxKind.InternalKeyword:
-						if (IsTopLevel(context.Node))
+						if (IsTopLevel(currentNode))
 						{
 							context.ReportDiagnostic(Rules.CreateDoNotUseTheInternalKeywordForTopLevelTypesDiagnostic(modifier.GetLocation()));
 						}
@@ -65,6 +67,31 @@ static void Analyze(SyntaxNodeAnalysisContext context, FileDetailCache cache)
 			}
 		}
 
+		static bool PartialMethodRequiresAccessibilityModifier(SyntaxNode node)
+		{
+			if (!node.IsKind(SyntaxKind.MethodDeclaration))
+			{
+				return false;
+			}
+
+			var methodNode = (MethodDeclarationSyntax)node;
+			if (methodNode.ReturnType.IsKind(SyntaxKind.PredefinedType))
+			{
+				var returnType = (PredefinedTypeSyntax)methodNode.ReturnType;
+				if (!returnType.Keyword.IsKind(SyntaxKind.VoidKeyword))
+				{
+					return true;
+				}
+			}
+
+			if (methodNode.ParameterList?.Parameters.Any(static p => p.Modifiers.Any(SyntaxKind.OutKeyword)) == true)
+			{
+				return true;
+			}
+
+			return false;
+		}
+
 		static bool IsTopLevel(SyntaxNode node)
 		{
 			var parentKind = node.Parent?.Kind() ?? SyntaxKind.None;