Merge pull request #29 from microsoft/alzollin/logRefactor

Switched to use dotnet's ILogger instead of Console.Out/Error.

Author: azchohfi

.github/codeql/codeql-config.yml

@@ -0,0 +1,4 @@
+paths:
+  - src
+paths-ignore:
+  - '**/*Tests/*.cs'

.github/workflows/codeql.yml

@@ -0,0 +1,64 @@
+# For most projects, this workflow file will not need changing; you simply need
+# to commit it to your repository.
+#
+# You may wish to alter this file to override the set of languages analyzed,
+# or to provide custom queries or build logic.
+#
+# ******** NOTE ********
+# We have attempted to detect the languages in your repository. Please check
+# the `language` matrix defined below to confirm you have the correct set of
+# supported CodeQL languages.
+#
+name: "CodeQL Advanced"
+
+on:
+  push:
+    branches: [ "main" ]
+  pull_request:
+    branches: [ "main" ]
+  schedule:
+    - cron: '35 20 * * 0'
+
+jobs:
+  analyze:
+    name: Analyze (${{ matrix.language }})
+    runs-on: ${{ (matrix.language == 'swift' && 'macos-latest') || 'ubuntu-latest' }}
+    permissions:
+      # required for all workflows
+      security-events: write
+
+      # required to fetch internal or private CodeQL packs
+      packages: read
+
+      # only required for workflows in private repositories
+      actions: read
+      contents: read
+
+    strategy:
+      fail-fast: false
+      matrix:
+        include:
+        - language: actions
+          build-mode: none
+        - language: c-cpp
+          build-mode: none
+        - language: csharp
+          build-mode: none
+        - language: javascript-typescript
+          build-mode: none
+    steps:
+    - name: Checkout repository
+      uses: actions/checkout@v5
+
+    # Initializes the CodeQL tools for scanning.
+    - name: Initialize CodeQL
+      uses: github/codeql-action/init@v4
+      with:
+        languages: ${{ matrix.language }}
+        build-mode: ${{ matrix.build-mode }}
+        config-file: ./.github/codeql/codeql-config.yml
+
+    - name: Perform CodeQL Analysis
+      uses: github/codeql-action/analyze@v4
+      with:
+        category: "/language:${{matrix.language}}"

src/winsdk-CLI/.editorconfig

@@ -8,15 +8,15 @@ indent_style = space
 indent_size = 4
 trim_trailing_whitespace = true
 
-[*.{cs,csx,vb,vbx}]
-indent_size = 4
-
 [*.{csproj,vbproj,vcxproj,vcxproj.filters,proj,projitems,shproj}]
 indent_size = 2
 
 [*.{json,yml,yaml}]
 indent_size = 2
 
+[*.{cs,csx,vb,vbx}]
+indent_size = 4
+
 # C# Code Style Rules
 
 # Async method naming
@@ -71,7 +71,7 @@ dotnet_diagnostic.CA1515.severity = none
 dotnet_diagnostic.CA1031.severity = suggestion
 
 # CA1848: Use the LoggerMessage delegates
-dotnet_diagnostic.CA1848.severity = suggestion
+dotnet_diagnostic.CA1848.severity = none
 
 # CA2249: Consider using String.Contains instead of String.IndexOf
 dotnet_diagnostic.CA2249.severity = suggestion
@@ -89,7 +89,7 @@ dotnet_diagnostic.CA1016.severity = none
 dotnet_diagnostic.CA1031.severity = suggestion
 
 # CA1032: Implement standard exception constructors
-dotnet_diagnostic.CA1032.severity = warning
+dotnet_diagnostic.CA1032.severity = suggestion
 
 # CA1303: Do not pass literals as localized parameters
 dotnet_diagnostic.CA1303.severity = none

src/winsdk-CLI/Winsdk.Cli.Tests/BaseCommandTests.cs

@@ -1,24 +1,35 @@
 using Microsoft.Extensions.DependencyInjection;
+using Microsoft.Extensions.Logging;
 using Winsdk.Cli.Helpers;
 
 namespace Winsdk.Cli.Tests;
 
 public class BaseCommandTests : IDisposable
 {
     private ServiceProvider _serviceProvider;
+    protected StringWriter ConsoleStdOut { get; } = new StringWriter();
+    protected StringWriter ConsoleStdErr { get; } = new StringWriter();
 
     public BaseCommandTests()
     {
         var services = new ServiceCollection()
             .ConfigureServices()
-            .ConfigureCommands();
+            .ConfigureCommands()
+            .AddLogging(b =>
+            {
+                b.ClearProviders();
+                b.AddTextWriterLogger(ConsoleStdOut, ConsoleStdErr);
+                b.SetMinimumLevel(LogLevel.Debug);
+            });
 
         _serviceProvider = services.BuildServiceProvider();
     }
 
     public void Dispose()
     {
         _serviceProvider?.Dispose();
+        ConsoleStdOut?.Dispose();
+        ConsoleStdErr?.Dispose();
         GC.SuppressFinalize(this);
     }
 

src/winsdk-CLI/Winsdk.Cli.Tests/BuildToolsServiceTests.cs

@@ -214,7 +214,7 @@ public async Task RunBuildToolAsync_WithNonExistentTool_ThrowsFileNotFoundExcept
         // Act & Assert
         await Assert.ThrowsExactlyAsync<FileNotFoundException>(async () =>
         {
-            await _buildToolsService.RunBuildToolAsync("nonexistent.exe", "", quiet: true);
+            await _buildToolsService.RunBuildToolAsync("nonexistent.exe", "");
         });
     }
 
@@ -225,7 +225,7 @@ public async Task EnsureBuildToolsAsync_WithNoExistingPackage_ShouldAttemptInsta
         // since we can't easily mock the package installation service in this test setup
 
         // Act
-        var result = await _buildToolsService.EnsureBuildToolsAsync(quiet: true);
+        var result = await _buildToolsService.EnsureBuildToolsAsync();
 
         // Assert - Result can be either null (if installation fails) or a path (if successful)
         // The important part is that the method completes without throwing
@@ -243,7 +243,7 @@ public async Task EnsureBuildToolsAsync_WithExistingPackage_ReturnsExistingPath(
         Directory.CreateDirectory(binDir);
 
         // Act
-        var result = await _buildToolsService.EnsureBuildToolsAsync(quiet: true);
+        var result = await _buildToolsService.EnsureBuildToolsAsync();
 
         // Assert - Should find and return the existing bin path
         Assert.AreEqual(binDir, result);
@@ -259,7 +259,7 @@ public async Task EnsureBuildToolsAsync_WithForceLatest_ShouldAttemptReinstallat
         Directory.CreateDirectory(binDir);
 
         // Act - Force latest should attempt reinstallation even with existing package
-        var result = await _buildToolsService.EnsureBuildToolsAsync(quiet: true, forceLatest: true);
+        var result = await _buildToolsService.EnsureBuildToolsAsync(forceLatest: true);
 
         // Assert - Result can be either null (if installation fails) or a path (if successful)
         // The important part is that the method completes and attempts reinstallation
@@ -279,7 +279,7 @@ public async Task EnsureBuildToolAvailableAsync_WithExistingTool_ReturnsToolPath
         File.WriteAllText(toolPath, "fake mt.exe");
 
         // Act
-        var result = await _buildToolsService.EnsureBuildToolAvailableAsync("mt.exe", quiet: true);
+        var result = await _buildToolsService.EnsureBuildToolAvailableAsync("mt.exe");
 
         // Assert
         Assert.AreEqual(toolPath, result);
@@ -298,7 +298,7 @@ public async Task EnsureBuildToolAvailableAsync_WithToolNameWithoutExtension_Add
         File.WriteAllText(toolPath, "fake mt.exe");
 
         // Act - Request tool without .exe extension
-        var result = await _buildToolsService.EnsureBuildToolAvailableAsync("mt", quiet: true);
+        var result = await _buildToolsService.EnsureBuildToolAvailableAsync("mt");
 
         // Assert
         Assert.AreEqual(toolPath, result);
@@ -313,7 +313,7 @@ public async Task EnsureBuildToolAvailableAsync_WithNoExistingPackageAndInstallS
         // Act
         try 
         {
-            var result = await _buildToolsService.EnsureBuildToolAvailableAsync("mt.exe", quiet: true);
+            var result = await _buildToolsService.EnsureBuildToolAvailableAsync("mt.exe");
 
             // Assert - If we get here, installation was successful and we got a path
             Assert.IsNotNull(result);
@@ -347,7 +347,7 @@ public async Task EnsureBuildToolAvailableAsync_WithNonExistentTool_ThrowsFileNo
         // Act & Assert
         await Assert.ThrowsExactlyAsync<FileNotFoundException>(async () =>
         {
-            await _buildToolsService.EnsureBuildToolAvailableAsync("nonexistent.exe", quiet: true);
+            await _buildToolsService.EnsureBuildToolAvailableAsync("nonexistent.exe");
         });
     }
 
@@ -361,7 +361,7 @@ public async Task RunBuildToolAsync_WithNoExistingPackage_AutoInstallsAndRuns()
         {
             // Create a simple batch command that outputs something
             // This will either succeed (if BuildTools installs successfully) or throw an exception
-            await _buildToolsService.RunBuildToolAsync("echo.cmd", "test", verbose: false, quiet: true);
+            await _buildToolsService.RunBuildToolAsync("echo.cmd", "test");
 
             // If we reach here, the auto-installation worked - test passes
         }
@@ -388,7 +388,7 @@ public async Task RunBuildToolAsync_WithExistingTool_RunsDirectly()
         File.WriteAllText(batchFile, "@echo Hello from test tool");
 
         // Act
-        var (stdout, stderr) = await _buildToolsService.RunBuildToolAsync("test.cmd", "", verbose: false, quiet: true);
+        var (stdout, stderr) = await _buildToolsService.RunBuildToolAsync("test.cmd", "");
 
         // Assert
         Assert.Contains("Hello from test tool", stdout);

src/winsdk-CLI/Winsdk.Cli.Tests/ManifestCommandTests.cs

@@ -267,28 +267,14 @@ public async Task ManifestGenerateCommandWithVerboseOptionShouldProduceOutput()
             "--yes" // Skip interactive prompts
         };
 
-        // Capture console output
-        using var consoleOutput = new StringWriter();
-        var originalConsoleOut = Console.Out;
-        Console.SetOut(consoleOutput);
-
-        try
-        {
-            // Act
-            var parseResult = generateCommand.Parse(args);
-            var exitCode = await parseResult.InvokeAsync();
+        var parseResult = generateCommand.Parse(args);
+        var exitCode = await parseResult.InvokeAsync();
 
-            // Assert
-            Assert.AreEqual(0, exitCode, "Generate command should complete successfully");
+        // Assert
+        Assert.AreEqual(0, exitCode, "Generate command should complete successfully");
 
-            var output = consoleOutput.ToString();
-            Assert.Contains("Generating manifest", output, "Verbose output should contain generation message");
-        }
-        finally
-        {
-            // Restore console output
-            Console.SetOut(originalConsoleOut);
-        }
+        var output = ConsoleStdOut.ToString();
+        Assert.Contains("Generating manifest", output, "Verbose output should contain generation message");
     }
 
     [TestMethod]

src/winsdk-CLI/Winsdk.Cli.Tests/PackageCommandTests.cs

@@ -184,7 +184,7 @@ public async Task PackageCommand_ToolDiscovery_FindsCommonBuildTools()
         var missingTools = new List<string>();
 
         // Ensure BuildTools are installed
-        var buildToolsPath = await _buildToolsService.EnsureBuildToolsAsync(quiet: true);
+        var buildToolsPath = await _buildToolsService.EnsureBuildToolsAsync();
         if (buildToolsPath == null)
         {
             Assert.Fail("Cannot run test - BuildTools installation failed.");
@@ -249,7 +249,6 @@ public async Task CreateMsixPackageAsync_OutputPathHandling_WorksCorrectly(strin
             packageName: "TestPackage",
             skipPri: true,
             autoSign: false,
-            verbose: true,
             cancellationToken: CancellationToken.None
         );
 
@@ -273,7 +272,6 @@ await _msixService.CreateMsixPackageAsync(
                 packageName: "TestPackage",
                 skipPri: true,
                 autoSign: false,
-                verbose: true,
                 cancellationToken: CancellationToken.None
             );
         }, "Expected DirectoryNotFoundException when input folder does not exist.");
@@ -298,7 +296,6 @@ await _msixService.CreateMsixPackageAsync(
                 packageName: "TestPackage",
                 skipPri: true,
                 autoSign: false,
-                verbose: true,
                 cancellationToken: CancellationToken.None
             );
         }, "Expected FileNotFoundException when manifest file is missing.");
@@ -340,7 +337,6 @@ public async Task CreateMsixPackageAsync_ExternalManifestWithAssets_CopiesManife
             skipPri: true,
             autoSign: false,
             manifestPath: externalManifestPath,
-            verbose: true,
             cancellationToken: CancellationToken.None
         );
 
@@ -368,7 +364,7 @@ public async Task CreateMsixPackageAsync_WithSigningAndMatchingPublishers_Should
         const string testPublisher = "CN=TestPublisher"; // This matches StandardTestManifestContent
 
         var certResult = await _certificateService.GenerateDevCertificateAsync(
-            testPublisher, certPath, testPassword, verbose: true);
+            testPublisher, certPath, testPassword);
 
         // Create minimal winsdk.yaml
         await File.WriteAllTextAsync(_configService.ConfigPath, "packages: []");
@@ -382,7 +378,6 @@ public async Task CreateMsixPackageAsync_WithSigningAndMatchingPublishers_Should
             autoSign: true,
             certificatePath: certPath,
             certificatePassword: testPassword,
-            verbose: true,
             cancellationToken: CancellationToken.None
         );
 
@@ -405,7 +400,7 @@ public async Task CreateMsixPackageAsync_WithSigningAndMismatchedPublishers_Shou
         const string wrongPublisher = "CN=WrongPublisher"; // This does NOT match StandardTestManifestContent
 
         var certResult = await _certificateService.GenerateDevCertificateAsync(
-            wrongPublisher, certPath, testPassword, verbose: true);
+            wrongPublisher, certPath, testPassword);
 
         // Create minimal winsdk.yaml
         await File.WriteAllTextAsync(_configService.ConfigPath, "packages: []");
@@ -421,7 +416,6 @@ await _msixService.CreateMsixPackageAsync(
                 autoSign: true,
                 certificatePath: certPath,
                 certificatePassword: testPassword,
-                verbose: true,
                 cancellationToken: CancellationToken.None
             );
         });
@@ -453,7 +447,7 @@ public async Task CreateMsixPackageAsync_WithExternalManifestAndMismatchedCertif
         const string wrongPublisher = "CN=DifferentPublisher";
 
         await _certificateService.GenerateDevCertificateAsync(
-            wrongPublisher, certPath, testPassword, verbose: true);
+            wrongPublisher, certPath, testPassword);
 
         // Create minimal winsdk.yaml
         await File.WriteAllTextAsync(_configService.ConfigPath, "packages: []");
@@ -470,7 +464,6 @@ await _msixService.CreateMsixPackageAsync(
                 certificatePath: certPath,
                 certificatePassword: testPassword,
                 manifestPath: externalManifestPath,
-                verbose: true,
                 cancellationToken: CancellationToken.None
             );
         });
@@ -494,7 +487,7 @@ public void CertificateService_ExtractPublisherFromCertificate_ShouldReturnCorre
 
         // Create a test certificate using the existing certificate service
         var certResult = _certificateService.GenerateDevCertificateAsync(
-            testPublisherCN, certPath, testPassword, verbose: true).GetAwaiter().GetResult();
+            testPublisherCN, certPath, testPassword).GetAwaiter().GetResult();
 
         // Act
         var extractedPublisher = CertificateService.ExtractPublisherFromCertificate(certPath, testPassword);
@@ -525,7 +518,7 @@ public void CertificateService_ExtractPublisherFromCertificate_WithWrongPassword
         const string wrongPassword = "wrong123";
 
         _certificateService.GenerateDevCertificateAsync(
-            "CN=PasswordTestPublisher", certPath, correctPassword, verbose: true).GetAwaiter().GetResult();
+            "CN=PasswordTestPublisher", certPath, correctPassword).GetAwaiter().GetResult();
 
         // Act & Assert
         Assert.ThrowsExactly<InvalidOperationException>(() =>
@@ -545,7 +538,7 @@ public async Task CertificateService_ValidatePublisherMatch_WithMatchingPublishe
 
         // Create certificate
         await _certificateService.GenerateDevCertificateAsync(
-            commonPublisher, certPath, testPassword, verbose: true);
+            commonPublisher, certPath, testPassword);
 
         // Create manifest with same publisher
         var manifestContent = StandardTestManifestContent.Replace(
@@ -566,7 +559,7 @@ public async Task CertificateService_ValidatePublisherMatch_WithMismatchedPublis
 
         // Create certificate with one publisher
         await _certificateService.GenerateDevCertificateAsync(
-            "CN=CertificatePublisher", certPath, testPassword, verbose: true);
+            "CN=CertificatePublisher", certPath, testPassword);
 
         // Create manifest with different publisher
         var manifestContent = StandardTestManifestContent.Replace(