Fix some analyzer warnings (#2386)

sbomer · vitek-karas · web-flow · commit 284b7ac6ecb7 · 2021-11-22T06:36:32.000-08:00
These were causing dotnet format to produce warnings because
they didn't have code fixes available. There are still some IDE0060
rule violations left over that I am not fixing here because they occur
in test projects (and I didn't want to silence all such warnings in the
test projects).

Co-authored-by: vitek-karas &lt;10670590+vitek-karas@users.noreply.github.com&gt;
diff --git a/.editorconfig b/.editorconfig
@@ -140,6 +140,9 @@ dotnet_diagnostic.CA2252.severity = none
 # CA1822: Mark members as static
 dotnet_diagnostic.CA1822.severity = none
 
+# IDE0060: Remove unused parameter
+dotnet_diagnostic.IDE0060.severity = none
+
 [external**]
 dotnet_analyzer_diagnostic.severity = none
 generated_code = true
diff --git a/lint.cmd b/lint.cmd
@@ -1,2 +1,2 @@
 @echo off
-powershell -ExecutionPolicy ByPass -NoProfile -command "Set-Location %~dp0; & """%~dp0eng\dotnet.ps1""" ""format illink.sln --exclude src/analyzer src/tuner external %*"""
+powershell -ExecutionPolicy ByPass -NoProfile -command "Set-Location %~dp0; & """%~dp0eng\dotnet.ps1""" ""format illink.sln --exclude external %*"""
diff --git a/lint.sh b/lint.sh
@@ -13,4 +13,4 @@ while [[ -h $source ]]; do
 done
 
 scriptroot="$( cd -P "$( dirname "$source" )" && pwd )"
-"$scriptroot/eng/dotnet.sh" format illink.sln --exclude src/analyzer src/tuner external $@
+"$scriptroot/eng/dotnet.sh" format illink.sln --exclude external $@
diff --git a/src/analyzer/ConsoleDependencyGraph.cs b/src/analyzer/ConsoleDependencyGraph.cs
@@ -96,7 +96,7 @@ public void ShowDependencies (VertexData vertex)
 						int pi = 0, childIdx;
 
 						do {
-							childIdx = childVertex.parentIndexes [pi];
+							childIdx = childVertex.parentIndexes[pi];
 							pi++;
 						} while (visited.Contains (childIdx) && pi < childVertex.parentIndexes.Count);
 
@@ -148,13 +148,13 @@ public void ShowStat (bool verbose = false)
 			Header ("Statistics");
 			if (verbose) {
 				foreach (var key in counts.Keys)
-					Console.WriteLine ("Vertex type:\t{0}{1}count:{2}", key, Tabs (key), counts [key]);
+					Console.WriteLine ("Vertex type:\t{0}{1}count:{2}", key, Tabs (key), counts[key]);
 			} else {
-				Console.WriteLine ("Assemblies:\t{0}", counts ["Assembly"]);
-				Console.WriteLine ("Modules:\t{0}", counts ["Module"]);
-				Console.WriteLine ("Types:\t\t{0}", counts ["TypeDef"]);
-				Console.WriteLine ("Fields:\t\t{0}", counts ["Field"]);
-				Console.WriteLine ("Methods:\t{0}", counts ["Method"]);
+				Console.WriteLine ("Assemblies:\t{0}", counts["Assembly"]);
+				Console.WriteLine ("Modules:\t{0}", counts["Module"]);
+				Console.WriteLine ("Types:\t\t{0}", counts["TypeDef"]);
+				Console.WriteLine ("Fields:\t\t{0}", counts["Field"]);
+				Console.WriteLine ("Methods:\t{0}", counts["Method"]);
 			}
 
 			Console.WriteLine ();
diff --git a/src/analyzer/LinkerAnalyzerCore/DependencyGraph.cs b/src/analyzer/LinkerAnalyzerCore/DependencyGraph.cs
@@ -14,7 +14,8 @@
 
 namespace LinkerAnalyzer.Core
 {
-	public class VertexData {
+	public class VertexData
+	{
 		public string value;
 		public List<int> parentIndexes;
 		public int index;
@@ -51,7 +52,8 @@ public void Load (string filename)
 			}
 		}
 
-		void Load (GZipStream zipStream) {
+		void Load (GZipStream zipStream)
+		{
 			using (XmlReader reader = XmlReader.Create (zipStream)) {
 				while (reader.Read ()) {
 					switch (reader.NodeType) {
@@ -88,7 +90,7 @@ public VertexData Vertex (string vertexName, bool create = false)
 			VertexData vertex;
 
 			try {
-				vertex = vertices [indexes [vertexName]];
+				vertex = vertices[indexes[vertexName]];
 			} catch (KeyNotFoundException) {
 				if (create) {
 					int index = vertices.Count;
@@ -97,9 +99,9 @@ public VertexData Vertex (string vertexName, bool create = false)
 					indexes.Add (vertexName, index);
 					string prefix = vertexName.Substring (0, vertexName.IndexOf (':'));
 					if (counts.ContainsKey (prefix))
-						counts [prefix]++;
+						counts[prefix]++;
 					else
-						counts [prefix] = 1;
+						counts[prefix] = 1;
 					//Console.WriteLine ("prefix " + prefix + " count " + counts[prefix]);
 					if (prefix == "TypeDef") {
 						Types.Add (vertex);
@@ -113,7 +115,7 @@ public VertexData Vertex (string vertexName, bool create = false)
 
 		public VertexData Vertex (int index)
 		{
-			return vertices [index];
+			return vertices[index];
 		}
 
 		IEnumerable<Tuple<VertexData, int>> AddDependencies (VertexData vertex, HashSet<int> reachedVertices, int depth)
diff --git a/src/analyzer/LinkerAnalyzerCore/SpaceAnalyzer.cs b/src/analyzer/LinkerAnalyzerCore/SpaceAnalyzer.cs
@@ -75,7 +75,7 @@ int GetMethodSize (MethodDefinition method)
 			var key = GetKey (method);
 
 			if (sizes.ContainsKey (key))
-				return sizes [key];
+				return sizes[key];
 
 			var msize = method.Body.CodeSize;
 			msize += method.Name.Length;
@@ -125,7 +125,7 @@ public void LoadAssemblies (bool verbose = true)
 				else
 					Console.Write (".");
 
-				ReaderParameters parameters = new ReaderParameters () { ReadingMode = ReadingMode.Immediate, AssemblyResolver = resolver};
+				ReaderParameters parameters = new ReaderParameters () { ReadingMode = ReadingMode.Immediate, AssemblyResolver = resolver };
 				var assembly = AssemblyDefinition.ReadAssembly (file, parameters);
 				assemblies.Add (assembly);
 				foreach (var module in assembly.Modules) {
@@ -146,7 +146,7 @@ public void LoadAssemblies (bool verbose = true)
 		public int GetSize (VertexData vertex)
 		{
 			if (sizes.ContainsKey (vertex.value))
-				return sizes [vertex.value];
+				return sizes[vertex.value];
 			return 0;
 		}
 	}
diff --git a/src/analyzer/Main.cs b/src/analyzer/Main.cs
@@ -7,8 +7,8 @@
 // Copyright 2015 Xamarin Inc (http://www.xamarin.com).
 //
 using System;
-using Mono.Options;
 using LinkerAnalyzer.Core;
+using Mono.Options;
 
 namespace LinkerAnalyzer
 {
@@ -56,7 +56,7 @@ static void Main (string[] args)
 				return;
 			}
 
-			string dependencyFile = args [args.Length - 1];
+			string dependencyFile = args[args.Length - 1];
 
 			ConsoleDependencyGraph deps = new ConsoleDependencyGraph () { Tree = reduceToTree, FlatDeps = flatDeps };
 			deps.Load (dependencyFile);
diff --git a/test/ILLink.RoslynAnalyzer.Tests/CompilationExtensions.cs b/test/ILLink.RoslynAnalyzer.Tests/CompilationExtensions.cs
@@ -18,17 +18,15 @@ public static MetadataReference EmitToImageReference (
 			this Compilation comp,
 			EmitOptions? options = null,
 			bool embedInteropTypes = false,
-			ImmutableArray<string> aliases = default,
-			DiagnosticDescriptor[]? expectedWarnings = null) => EmitToPortableExecutableReference (comp, options, embedInteropTypes, aliases, expectedWarnings);
+			ImmutableArray<string> aliases = default) => EmitToPortableExecutableReference (comp, options, embedInteropTypes, aliases);
 
 		public static PortableExecutableReference EmitToPortableExecutableReference (
 			this Compilation comp,
 			EmitOptions? options = null,
 			bool embedInteropTypes = false,
-			ImmutableArray<string> aliases = default,
-			DiagnosticDescriptor[]? expectedWarnings = null)
+			ImmutableArray<string> aliases = default)
 		{
-			var image = comp.EmitToArray (options, expectedWarnings: expectedWarnings);
+			var image = comp.EmitToArray (options);
 			if (comp.Options.OutputKind == OutputKind.NetModule) {
 				return ModuleMetadata.CreateFromImage (image).GetReference (display: comp.MakeSourceModuleName ());
 			} else {
@@ -39,7 +37,6 @@ public static PortableExecutableReference EmitToPortableExecutableReference (
 		internal static ImmutableArray<byte> EmitToArray (
 			this Compilation compilation,
 			EmitOptions? options = null,
-			DiagnosticDescriptor[]? expectedWarnings = null,
 			Stream? pdbStream = null,
 			IMethodSymbol? debugEntryPoint = null,
 			Stream? sourceLinkStream = null,
diff --git a/test/ILLink.RoslynAnalyzer.Tests/TestCaseCompilation.cs b/test/ILLink.RoslynAnalyzer.Tests/TestCaseCompilation.cs
@@ -56,7 +56,7 @@ internal static class TestCaseCompilation
 			return (new CompilationWithAnalyzers (comp, SupportedDiagnosticAnalyzers, compWithAnalyzerOptions), comp.GetSemanticModel (src));
 		}
 
-		public static async Task<Compilation> GetCompilation (string source, IEnumerable<MetadataReference>? additionalReferences = null, IEnumerable<SyntaxTree>? additionalSources = null)
+		public static async Task<Compilation> GetCompilation (string source, IEnumerable<MetadataReference>? additionalReferences = null)
 			=> (await CreateCompilation (source, additionalReferences: additionalReferences ?? Array.Empty<MetadataReference> ())).Compilation.Compilation;
 
 		class SimpleAnalyzerOptions : AnalyzerConfigOptionsProvider
diff --git a/test/ILLink.RoslynAnalyzer.Tests/TestChecker.cs b/test/ILLink.RoslynAnalyzer.Tests/TestChecker.cs
@@ -10,7 +10,6 @@
 using Microsoft.CodeAnalysis;
 using Microsoft.CodeAnalysis.CSharp;
 using Microsoft.CodeAnalysis.CSharp.Syntax;
-using Microsoft.CodeAnalysis.Diagnostics;
 using Microsoft.CodeAnalysis.Text;
 using Mono.Linker.Tests.Cases.Expectations.Assertions;
 using Xunit;
@@ -182,13 +181,15 @@ static ProducedBy GetProducedBy (ExpressionSyntax expression)
 				var producedBy = (ProducedBy) 0x0;
 				switch (expression) {
 				case BinaryExpressionSyntax binaryExpressionSyntax:
-					Enum.TryParse<ProducedBy> ((binaryExpressionSyntax.Left as MemberAccessExpressionSyntax)!.Name.Identifier.ValueText, out var besProducedBy);
+					if (!Enum.TryParse<ProducedBy> ((binaryExpressionSyntax.Left as MemberAccessExpressionSyntax)!.Name.Identifier.ValueText, out var besProducedBy))
+						throw new ArgumentException ("Expression must be a ProducedBy value", nameof (expression));
 					producedBy |= besProducedBy;
 					producedBy |= GetProducedBy (binaryExpressionSyntax.Right);
 					break;
 
 				case MemberAccessExpressionSyntax memberAccessExpressionSyntax:
-					Enum.TryParse<ProducedBy> (memberAccessExpressionSyntax.Name.Identifier.ValueText, out var maeProducedBy);
+					if (!Enum.TryParse<ProducedBy> (memberAccessExpressionSyntax.Name.Identifier.ValueText, out var maeProducedBy))
+						throw new ArgumentException ("Expression must be a ProducedBy value", nameof (expression));
 					producedBy |= maeProducedBy;
 					break;
 
@@ -282,15 +283,15 @@ private bool TryValidateLogContainsAttribute (AttributeSyntax attribute, List<Di
 				}
 			}
 
-			missingDiagnosticMessage = $"Could not find text:\n{text}\nIn diagnostics:\n{(string.Join (Environment.NewLine, _diagnostics))}";
+			missingDiagnosticMessage = $"Could not find text:\n{text}\nIn diagnostics:\n{string.Join (Environment.NewLine, _diagnostics)}";
 			return false;
 		}
 
-		private void ValidateLogDoesNotContainAttribute (AttributeSyntax attribute, IReadOnlyList<Diagnostic> diagnosticMessages)
+		private static void ValidateLogDoesNotContainAttribute (AttributeSyntax attribute, IReadOnlyList<Diagnostic> diagnosticMessages)
 		{
 			var arg = Assert.Single (LinkerTestBase.GetAttributeArguments (attribute));
 			var text = LinkerTestBase.GetStringFromExpression (arg.Value);
-			foreach (var diagnostic in _diagnostics)
+			foreach (var diagnostic in diagnosticMessages)
 				Assert.DoesNotContain (text, diagnostic.GetMessage ());
 		}
 
diff --git a/test/ILLink.Tasks.Tests/Mock.cs b/test/ILLink.Tasks.Tests/Mock.cs
@@ -179,7 +179,7 @@ protected override List<BaseStep> CreateDefaultResolvers ()
 
 	public class MockXmlDependencyRecorder : IDependencyRecorder
 	{
-		public static MockXmlDependencyRecorder Singleton = new MockXmlDependencyRecorder ();
+		public static MockXmlDependencyRecorder Singleton { get; } = new MockXmlDependencyRecorder ();
 		public void RecordDependency (object source, object arget, bool marked) { }
 		public void RecordDependency (object target, in DependencyInfo reason, bool marked) { }
 	}
diff --git a/test/Mono.Linker.Tests/Extensions/CecilExtensions.cs b/test/Mono.Linker.Tests/Extensions/CecilExtensions.cs
@@ -115,7 +115,7 @@ public static bool DerivesFrom (this TypeDefinition type, string baseTypeName)
 		public static PropertyDefinition GetPropertyDefinition (this MethodDefinition method)
 		{
 			if (!method.IsSetter && !method.IsGetter)
-				throw new ArgumentException ();
+				throw new ArgumentException ("Method must be a property getter or setter", nameof (method));
 
 			var propertyName = method.Name.Substring (4);
 			return method.DeclaringType.Properties.First (p => p.Name == propertyName);
diff --git a/test/Mono.Linker.Tests/Extensions/NiceIO.cs b/test/Mono.Linker.Tests/Extensions/NiceIO.cs
@@ -45,7 +45,7 @@ public class NPath : IEquatable<NPath>, IComparable
 		public NPath (string path)
 		{
 			if (path == null)
-				throw new ArgumentNullException ();
+				throw new ArgumentNullException (nameof (path));
 
 			path = ParseDriveLetter (path, out _driveLetter);
 
diff --git a/test/Mono.Linker.Tests/TestCasesRunner/ResultChecker.cs b/test/Mono.Linker.Tests/TestCasesRunner/ResultChecker.cs
@@ -578,7 +578,7 @@ protected virtual bool TryVerifyKeptMemberInAssemblyAsProperty (string memberNam
 
 		protected virtual bool TryVerifyKeptMemberInAssemblyAsMethod (string memberName, TypeDefinition originalType, TypeDefinition linkedType)
 		{
-			return TryVerifyKeptMemberInAssemblyAsMethod (memberName, originalType, linkedType, out MethodDefinition _originalMethod, out MethodDefinition _linkedMethod);
+			return TryVerifyKeptMemberInAssemblyAsMethod (memberName, originalType, linkedType, out _, out _);
 		}
 
 		protected virtual bool TryVerifyKeptMemberInAssemblyAsMethod (string memberName, TypeDefinition originalType, TypeDefinition linkedType, out MethodDefinition originalMethod, out MethodDefinition linkedMethod)
@@ -1222,7 +1222,7 @@ static string[] GetMessagePartsFromReflectionAccessPatternAttribute (CustomAttri
 
 		static string GetFullMemberNameFromDefinition (IMetadataTokenProvider member)
 		{
-			return GetFullMemberNameFromDefinition (member, out string genericMember);
+			return GetFullMemberNameFromDefinition (member, out _);
 		}
 
 		static string GetFullMemberNameFromDefinition (IMetadataTokenProvider member, out string genericMember)
diff --git a/test/Mono.Linker.Tests/Tests/DocumentationSignatureParserTests.cs b/test/Mono.Linker.Tests/Tests/DocumentationSignatureParserTests.cs
@@ -131,7 +131,7 @@ public static void MIn (in int a)
 			{
 			}
 
-			public static int i;
+			static int i;
 
 			[ExpectGeneratedDocumentationSignature ("M:Mono.Linker.Tests.DocumentationSignatureParserTests.A.MRefReturn")]
 			[ExpectExactlyResolvedDocumentationSignature ("M:Mono.Linker.Tests.DocumentationSignatureParserTests.A.MRefReturn")]

Original file line number	Diff line number	Diff line change
`@@ -1,2 +1,2 @@`
`1`	`1`	`@echo off`
`2`		`-powershell -ExecutionPolicy ByPass -NoProfile -command "Set-Location %~dp0; & """%~dp0eng\dotnet.ps1""" ""format illink.sln --exclude src/analyzer src/tuner external %*"""`
	`2`	`+powershell -ExecutionPolicy ByPass -NoProfile -command "Set-Location %~dp0; & """%~dp0eng\dotnet.ps1""" ""format illink.sln --exclude external %*"""`
Original file line number	Diff line number	Diff line change
`@@ -56,7 +56,7 @@ internal static class TestCaseCompilation`
`56`	`56`	`return (new CompilationWithAnalyzers (comp, SupportedDiagnosticAnalyzers, compWithAnalyzerOptions), comp.GetSemanticModel (src));`
`57`	`57`	`}`
`58`	`58`
`59`		`- public static async Task<Compilation> GetCompilation (string source, IEnumerable<MetadataReference>? additionalReferences = null, IEnumerable<SyntaxTree>? additionalSources = null)`
	`59`	`+ public static async Task<Compilation> GetCompilation (string source, IEnumerable<MetadataReference>? additionalReferences = null)`
`60`	`60`	`=> (await CreateCompilation (source, additionalReferences: additionalReferences ?? Array.Empty<MetadataReference> ())).Compilation.Compilation;`
`61`	`61`
`62`	`62`	`class SimpleAnalyzerOptions : AnalyzerConfigOptionsProvider`