Enable rules for IDisposable and exceptions

.editorconfig

@@ -234,25 +234,32 @@ dotnet_diagnostic.IDE1006.severity = none       # These words must begin with up
 # https://learn.microsoft.com/en-us/dotnet/fundamentals/code-analysis/quality-rules/?view=vs-2022
 [*.{cs,vb}]
 
-dotnet_diagnostic.CA1507.severity = error   # Use nameof in place of string
-dotnet_diagnostic.CS1573.severity = silent  # Parameter has no matching param tag in the XML comment (but other parameters do). Justification: Don't require unneccessary comments. Only add comments for parameters that need clairification.
-dotnet_diagnostic.CA1508.severity = warning # Avoid dead conditional code
-dotnet_diagnostic.CA1700.severity = error   # Do not name enum values 'Reserved'
-dotnet_diagnostic.CA1707.severity = warning # Identifiers should not contain underscores
-dotnet_diagnostic.CA1708.severity = error   # Identifiers should differ by more than case
-dotnet_diagnostic.CA1710.severity = error   # Identifiers should have correct suffix
-dotnet_diagnostic.CA1712.severity = error   # Do not prefix enum values with type name
-dotnet_diagnostic.CA1720.severity = error   # Identifiers should not contain type names
-dotnet_diagnostic.CA1802.severity = error   # Use Literals Where Appropriate
-dotnet_diagnostic.CA1805.severity = warning # Do not initialize unnecessarily
-dotnet_diagnostic.CA1810.severity = error   # Initialize reference type static fields inline
-dotnet_diagnostic.CA1821.severity = error   # Remove empty finalizers
-dotnet_diagnostic.CA1822.severity = warning # Mark members as static
-dotnet_diagnostic.CA1823.severity = warning # Avoid unused private fields
-dotnet_diagnostic.CA1826.severity = warning # Use property instead of Linq Enumerable method
-dotnet_diagnostic.CA1827.severity = warning # Do not use Count()/LongCount() when Any() can be used
-dotnet_diagnostic.CA1849.severity = warning # Call async methods when in an async method
-dotnet_diagnostic.CA2000.severity = warning # Dispose objects before losing scope
+dotnet_diagnostic.CA1031.severity = error      # Do not catch general exception types
+dotnet_diagnostic.CA1063.severity = error      # Implement IDisposable correctly
+dotnet_diagnostic.CA1001.severity = error      # Types that own disposable fields should be disposable
+dotnet_diagnostic.CA1507.severity = error      # Use nameof in place of string
+dotnet_diagnostic.CS1573.severity = silent     # Parameter has no matching param tag in the XML comment (but other parameters do). Justification: Don't require unneccessary comments. Only add comments for parameters that need clairification.
+dotnet_diagnostic.CA1508.severity = warning    # Avoid dead conditional code
+dotnet_diagnostic.CA1700.severity = error      # Do not name enum values 'Reserved'
+dotnet_diagnostic.CA1707.severity = warning    # Identifiers should not contain underscores
+dotnet_diagnostic.CA1708.severity = error      # Identifiers should differ by more than case
+dotnet_diagnostic.CA1710.severity = error      # Identifiers should have correct suffix
+dotnet_diagnostic.CA1712.severity = error      # Do not prefix enum values with type name
+dotnet_diagnostic.CA1720.severity = error      # Identifiers should not contain type names
+dotnet_diagnostic.CA1802.severity = error      # Use Literals Where Appropriate
+dotnet_diagnostic.CA1805.severity = warning    # Do not initialize unnecessarily
+dotnet_diagnostic.CA1810.severity = error      # Initialize reference type static fields inline
+dotnet_diagnostic.CA1816.severity = error      # Call GC.SuppressFinalize correctly
+dotnet_diagnostic.CA1821.severity = error      # Remove empty finalizers
+dotnet_diagnostic.CA1822.severity = warning    # Mark members as static
+dotnet_diagnostic.CA1823.severity = warning    # Avoid unused private fields
+dotnet_diagnostic.CA1826.severity = warning    # Use property instead of Linq Enumerable method
+dotnet_diagnostic.CA1827.severity = warning    # Do not use Count()/LongCount() when Any() can be used
+dotnet_diagnostic.CA1849.severity = warning    # Call async methods when in an async method
+dotnet_diagnostic.CA2000.severity = error      # Dispose objects before losing scope
+dotnet_diagnostic.CA2213.severity = error      # Disposable fields should be disposed
+dotnet_diagnostic.CA2215.severity = error      # Dispose methods should call base class dispose
+dotnet_diagnostic.CA2216.severity = warning    # Disposable types should declare finalizer
 
 
 # *****************************************

samples/MauiMsApp/CreatePage.xaml.cs

@@ -1,7 +1,6 @@
 // Copyright (c) Microsoft Corporation.
 // Licensed under the MIT License.
 
-using Microsoft.PowerPlatform.PowerApps.Persistence.MsApp;
 using MSAppGenerator;
 
 namespace MauiMsApp;
@@ -13,9 +12,8 @@ public CreatePage()
         InitializeComponent();
     }
 
-#pragma warning disable CA1822 // Mark members as static
+    [System.Diagnostics.CodeAnalysis.SuppressMessage("Design", "CA1031:Do not catch general exception types", Justification = "REVIEW")]
     private async void OnCreateClicked(object sender, EventArgs e)
-#pragma warning restore CA1822 // Mark members as static
     {
         try
         {

samples/MauiMsApp/MainPage.xaml.cs

@@ -12,9 +12,8 @@ public MainPage()
         InitializeComponent();
     }
 
-#pragma warning disable CA1822 // Mark members as static
+    [System.Diagnostics.CodeAnalysis.SuppressMessage("Design", "CA1031:Do not catch general exception types", Justification = "REVIEW")]
     private async void OnOpenClicked(object sender, EventArgs e)
-#pragma warning restore CA1822 // Mark members as static
     {
         try
         {

samples/Test.AppWriter/InputProcessor.cs

@@ -43,6 +43,7 @@ private static bool ValidateFilePath(string filePath, out string error)
     /// <summary>
     /// Function to bind to the Create command and call App Creation code
     /// </summary>
+    [System.Diagnostics.CodeAnalysis.SuppressMessage("Design", "CA1031:Do not catch general exception types", Justification = "REVIEW")]
     private static void CreateFunction(bool interactive, string fullPathToMsApp, int numScreens, IList<string>? controlsinfo)
     {
         var creator = new AppCreator();

src/PAModel/ControlTemplates/ControlTemplateParser.cs

@@ -12,6 +12,7 @@ internal class ControlTemplateParser
 {
     internal static Regex _reservedIdentifierRegex = new(@"%([a-zA-Z]*)\.RESERVED%");
 
+    [System.Diagnostics.CodeAnalysis.SuppressMessage("Design", "CA1031:Do not catch general exception types", Justification = "<Pending>")]
     internal static bool TryParseTemplate(TemplateStore templateStore, string templateString, AppType type, Dictionary<string, ControlTemplate> loadedTemplates, out ControlTemplate template, out string name)
     {
         template = null;

src/PAModel/MsAppTest.cs

@@ -25,6 +25,7 @@ public static bool Compare(CanvasDocument doc1, CanvasDocument doc2, TextWriter
         }
     }
 
+    [System.Diagnostics.CodeAnalysis.SuppressMessage("Design", "CA1031:Do not catch general exception types", Justification = "<Pending>")]
     public static bool MergeStressTest(string pathToMsApp1, string pathToMsApp2)
     {
         try
@@ -142,6 +143,7 @@ public static CanvasDocument RemoveEntropy(string pathToMsApp)
     }
 
     // Given an msapp (original source of truth), stress test the conversions
+    [System.Diagnostics.CodeAnalysis.SuppressMessage("Design", "CA1031:Do not catch general exception types", Justification = "<Pending>")]
     public static bool StressTest(string pathToMsApp)
     {
         try
@@ -283,7 +285,7 @@ public static void CompareChecksums(string pathToZip, TextWriter log, Dictionary
                                     JsonDocument.Parse(originalContents);
                                     JsonDocument.Parse(newContents);
                                 }
-                                catch
+                                catch (JsonException)
                                 {
                                     isJson = false;
                                 }

src/PAModel/PAConvert/Parser/Parser.cs

@@ -9,11 +9,12 @@
 
 namespace Microsoft.PowerPlatform.Formulas.Tools.Parser;
 
-internal class Parser
+internal class Parser : IDisposable
 {
     public readonly ErrorContainer _errorContainer;
 
     private readonly YamlLexer _yaml;
+    private bool _isDisposed;
 
     public Parser(string fileName, string contents, ErrorContainer errors)
     {
@@ -345,4 +346,24 @@ private static bool IsControlStart(string line)
         line = line.Substring(length).TrimStart();
         return line.StartsWith("As");
     }
+
+    protected virtual void Dispose(bool disposing)
+    {
+        if (!_isDisposed)
+        {
+            if (disposing)
+            {
+                _yaml.Dispose();
+            }
+
+            _isDisposed = true;
+        }
+    }
+
+    public void Dispose()
+    {
+        // Do not change this code. Put cleanup code in 'Dispose(bool disposing)' method
+        Dispose(disposing: true);
+        GC.SuppressFinalize(this);
+    }
 }

src/PAModel/PAConvert/Yaml/YamlLexer.cs

@@ -651,7 +651,7 @@ protected virtual void Dispose(bool disposing)
         {
             if (disposing)
             {
-                _reader?.Dispose();
+                _reader.Dispose();
             }
 
             _isDisposed = true;

src/PAModel/Serializers/MsAppSerializer.cs

@@ -80,6 +80,7 @@ public static CanvasDocument Load(Stream streamToMsapp, ErrorContainer errors)
         var screenOrder = new Dictionary<string, double>();
 
         ZipArchive zipOpen;
+#pragma warning disable CA1031 // Do not catch general exception types
         try
         {
 #pragma warning disable CA2000 // Dispose objects before losing scope
@@ -92,6 +93,7 @@ public static CanvasDocument Load(Stream streamToMsapp, ErrorContainer errors)
             errors.MsAppFormatError(e.Message);
             return null;
         }
+#pragma warning restore CA1031 // Do not catch general exception types
 
         using (var z = zipOpen)
         {

src/PAModel/Serializers/SourceSerializer.cs

@@ -479,7 +479,7 @@ private static void AddControl(CanvasDocument app, string filePath, bool isCompo
         _ = Path.GetFileName(filePath);
         try
         {
-            var parser = new Parser.Parser(filePath, fileContents, errors);
+            using var parser = new Parser.Parser(filePath, fileContents, errors);
             var controlIR = parser.ParseControl(isComponent);
             if (controlIR == null)
             {
@@ -1061,6 +1061,7 @@ private static void AddDefaultTheme(CanvasDocument app)
 
     private static BuildVerJson GetBuildDetails()
     {
+#pragma warning disable CA1031 // Do not catch general exception types
         try
         {
             var assembly = Assembly.GetExecutingAssembly();
@@ -1078,6 +1079,7 @@ private static BuildVerJson GetBuildDetails()
         {
             return null;
         }
+#pragma warning restore CA1031 // Do not catch general exception types
     }
 
     /// <summary>

src/PAModelTests/RoundtripTests.cs

@@ -2,7 +2,7 @@
 // Licensed under the MIT License.
 
 using System.IO;
-using System.Threading.Tasks;
+using System.Linq;
 using Microsoft.PowerPlatform.Formulas.Tools;
 
 namespace PAModelTests;
@@ -11,27 +11,16 @@ namespace PAModelTests;
 [TestClass]
 public class RoundtripTests
 {
+    private static IEnumerable<object[]> TestAppFilePaths => Directory.GetFiles(Path.Combine(Environment.CurrentDirectory, "Apps"), "*.msapp").Select(p => new[] { p });
+
     // Apps live in the "Apps" folder, and should have a build action of "Copy to output"
     [TestMethod]
-    public void StressTestApps()
+    [DynamicData(nameof(TestAppFilePaths))]
+    public void StressTestApps(string msappPath)
     {
-        var directory = Path.Combine(Environment.CurrentDirectory, "Apps");
-
-        Parallel.ForEach(Directory.GetFiles(directory), root =>
-        {
-            try
-            {
-                var ok = MsAppTest.StressTest(root);
-                Assert.IsTrue(ok);
+        MsAppTest.StressTest(msappPath).Should().BeTrue();
 
-                var cloneOk = MsAppTest.TestClone(root);
-                // If this fails, to debug it, rerun and set a breakpoint in DebugChecksum().
-                cloneOk.Should().BeTrue(root);
-            }
-            catch (Exception ex)
-            {
-                Assert.Fail(ex.ToString());
-            }
-        });
+        // If this fails, to debug it, rerun and set a breakpoint in DebugChecksum().
+        MsAppTest.TestClone(msappPath).Should().BeTrue();
     }
 }

src/PASopa/Program.cs

@@ -48,7 +48,7 @@ private static void Main(string[] args)
             var msAppCommon = Path.Combine(msAppPathDir, "empty.msapp");
             foreach (var msAppPath in Directory.EnumerateFiles(msAppPathDir, "*.msapp", SearchOption.TopDirectoryOnly))
             {
-                // Merge test requires a 2nd app. Could do a full NxN matrix. But here, just pick the first item. 
+                // Merge test requires a 2nd app. Could do a full NxN matrix. But here, just pick the first item.
                 msAppCommon ??= msAppPath;
 
                 var sw = Stopwatch.StartNew();
@@ -267,6 +267,7 @@ private static void Usage()
                 ");
     }
 
+    [System.Diagnostics.CodeAnalysis.SuppressMessage("Design", "CA1031:Do not catch general exception types", Justification = "ByDesign - The exceptions are collected")]
     private static (CanvasDocument, ErrorContainer) TryOperation(Func<(CanvasDocument, ErrorContainer)> operation)
     {
         CanvasDocument app = null;
@@ -283,6 +284,7 @@ private static (CanvasDocument, ErrorContainer) TryOperation(Func<(CanvasDocumen
         return (app, errors);
     }
 
+    [System.Diagnostics.CodeAnalysis.SuppressMessage("Design", "CA1031:Do not catch general exception types", Justification = "ByDesign - The exceptions are collected")]
     private static ErrorContainer TryOperation(Func<ErrorContainer> operation)
     {
         var errors = new ErrorContainer();

targets/Program.cs

@@ -13,7 +13,7 @@
 
 namespace targets
 {
-    class Options
+    internal sealed class Options
     {
         [Value(0, MetaName = "target", HelpText = "build target to run; see available with: '--list", Default = "rebuild")]
         public string Target { get; set; }
@@ -25,7 +25,7 @@ class Options
         public IEnumerable<string> Projects { get; set; }
     }
 
-    class Program
+    public sealed class Program
     {
         static Options options;
 
@@ -34,6 +34,7 @@ static void Main(string[] args)
 
             string RootDir = "", gitHash = "";
             bool gitExists = true;
+#pragma warning disable CA1031 // Do not catch general exception types
             try
             {
                 RootDir = Read("git", "rev-parse --show-toplevel", noEcho: true).Trim();
@@ -45,6 +46,7 @@ static void Main(string[] args)
                 Console.WriteLine("Unable to find root directory using git, assuming this script is being run from root = " + RootDir);
                 gitExists = false;
             }
+#pragma warning restore CA1031 // Do not catch general exception types
 
             string BinDir = Path.Combine(RootDir, "bin");
             string ObjDir = Path.Combine(RootDir, "obj");
@@ -87,7 +89,7 @@ static void Main(string[] args)
             Target("pack",
                 () =>
                 {
-                    var projects = (options.Projects.Any()) ? options.Projects : new[]{ defaultPackProject };
+                    var projects = (options.Projects.Any()) ? options.Projects : new[] { defaultPackProject };
 
                     foreach (var project in projects)
                         RunDotnet("pack", $"{EscapePath(project)} --configuration {options.Configuration} --output {EscapePath(Path.Combine(PkgDir, "PackResult"))} --no-build -p:Packing=true", gitExists, LogDir);