Skip to content

Commit 56f2265

Browse files
rmarinhoCopilot
rmarinho
and
Copilot
committed
Review improvements: centralized formatter, arg sanitization, IDisposable, NuGet cleanup
- I1: Replace all 27+ direct JsonOutputFormatter/SpectreOutputFormatter constructions with centralized Program.GetFormatter(context) across all command files - I2: Use static Option references instead of string-based lookups in DeviceCommand - I4: IAndroidProvider extends IDisposable, AndroidProvider.Dispose() chains to SdkManager - I5: Remove LOCAL_MACIOS_DEVTOOLS_PLACEHOLDER from NuGet.config, clean disabled sources - I6: Add ProcessRunner.SanitizeArg() to validate/quote process arguments, applied to all Adb.cs and Simctl.cs interpolated arguments Co-authored-by: Copilot <223556219+Copilot@users.noreply.github.com>
1 parent 0510768 commit 56f2265

File tree

12 files changed

+67
-108
lines changed

12 files changed

+67
-108
lines changed

‎NuGet.config‎

Lines changed: 2 additions & 2 deletions
Original file line numberDiff line numberDiff line change
@@ -15,7 +15,8 @@
1515
<add key="dotnet11" value="https://pkgs.dev.azure.com/dnceng/public/_packaging/dotnet11/nuget/v3/index.json" />
1616
<!-- End: Package sources from dotnet-android-tools -->
1717
<!-- Begin: Package sources from dotnet-macios-devtools (local until CI feed is set up) -->
18-
<add key="macios-devtools-local" value="LOCAL_MACIOS_DEVTOOLS_PLACEHOLDER" />
18+
<!-- Uncomment and set path to use locally-built macios-devtools packages: -->
19+
<!-- <add key="macios-devtools-local" value="/path/to/macios-devtools/nupkgs" /> -->
1920
<!-- End: Package sources from dotnet-macios-devtools -->
2021
<!-- Begin: Package sources from dotnet-dotnet -->
2122
<add key="darc-pub-dotnet-dotnet-e17b0d0" value="https://pkgs.dev.azure.com/dnceng/public/_packaging/darc-pub-dotnet-dotnet-e17b0d08/nuget/v3/index.json" />
@@ -35,6 +36,5 @@
3536
<disabledPackageSources>
3637
<add key="local" value="true" />
3738
<add key="nuget-only" value="true" />
38-
<add key="macios-devtools-local" value="true" />
3939
</disabledPackageSources>
4040
</configuration>

‎src/Tools/Microsoft.Maui.Client/Commands/AndroidCommands.cs‎

Lines changed: 15 additions & 59 deletions
Original file line numberDiff line numberDiff line change
@@ -64,11 +64,7 @@ private static Command CreateInstallCommand()
6464
// Support comma-separated packages: "pkg1,pkg2,pkg3" becomes ["pkg1", "pkg2", "pkg3"]
6565
var packages = rawPackages?
6666
.SelectMany(p => p.Split(',', StringSplitOptions.RemoveEmptyEntries | StringSplitOptions.TrimEntries))
67-
.ToArray();
68-
69-
var formatter = useJson
70-
? (IOutputFormatter)new JsonOutputFormatter(Console.Out)
71-
: new SpectreOutputFormatter();
67+
.ToArray(); var formatter = Program.GetFormatter(context);
7268

7369
try
7470
{
@@ -227,9 +223,7 @@ private static Command CreateJdkCommand()
227223
var jdkManager = Program.JdkManager;
228224

229225
var useJson = context.ParseResult.GetValueForOption(GlobalOptions.JsonOption);
230-
var formatter = useJson
231-
? (IOutputFormatter)new JsonOutputFormatter(Console.Out)
232-
: new SpectreOutputFormatter();
226+
var formatter = Program.GetFormatter(context);
233227

234228
var healthCheck = await jdkManager.CheckHealthAsync(context.GetCancellationToken());
235229

@@ -263,11 +257,7 @@ private static Command CreateJdkCommand()
263257
var version = context.ParseResult.GetValueForOption(
264258
(Option<int>)context.ParseResult.CommandResult.Command.Options.First(o => o.Name == "version"));
265259
var path = context.ParseResult.GetValueForOption(
266-
(Option<string>)context.ParseResult.CommandResult.Command.Options.First(o => o.Name == "path"));
267-
268-
var formatter = useJson
269-
? (IOutputFormatter)new JsonOutputFormatter(Console.Out)
270-
: new SpectreOutputFormatter();
260+
(Option<string>)context.ParseResult.CommandResult.Command.Options.First(o => o.Name == "path")); var formatter = Program.GetFormatter(context);
271261

272262
try
273263
{
@@ -307,12 +297,12 @@ private static Command CreateJdkCommand()
307297

308298
if (useJson)
309299
{
310-
var formatter = new JsonOutputFormatter(Console.Out);
300+
var formatter = Program.GetFormatter(context);
311301
formatter.Write(new { versions = versions });
312302
}
313303
else
314304
{
315-
var formatter = new SpectreOutputFormatter();
305+
var formatter = Program.GetFormatter(context);
316306
formatter.WriteTable(
317307
versions,
318308
("Version", v => v.ToString()),
@@ -338,9 +328,7 @@ private static Command CreateSdkCommand()
338328
var androidProvider = Program.AndroidProvider;
339329

340330
var useJson = context.ParseResult.GetValueForOption(GlobalOptions.JsonOption);
341-
var formatter = useJson
342-
? (IOutputFormatter)new JsonOutputFormatter(Console.Out)
343-
: new SpectreOutputFormatter();
331+
var formatter = Program.GetFormatter(context);
344332

345333
var checks = await androidProvider.CheckHealthAsync(context.GetCancellationToken());
346334
var sdkCheck = checks.FirstOrDefault(c => c.Name == "Android SDK");
@@ -377,11 +365,7 @@ private static Command CreateSdkCommand()
377365

378366
var useJson = context.ParseResult.GetValueForOption(GlobalOptions.JsonOption);
379367
var dryRun = context.ParseResult.GetValueForOption(GlobalOptions.DryRunOption);
380-
var packages = context.ParseResult.GetValueForArgument(sdkInstallPkgsArg);
381-
382-
var formatter = useJson
383-
? (IOutputFormatter)new JsonOutputFormatter(Console.Out)
384-
: new SpectreOutputFormatter();
368+
var packages = context.ParseResult.GetValueForArgument(sdkInstallPkgsArg); var formatter = Program.GetFormatter(context);
385369

386370
try
387371
{
@@ -581,11 +565,7 @@ await spectreInstall.Console.Status()
581565
var showAvailable = context.ParseResult.GetValueForOption(
582566
(Option<bool>)context.ParseResult.CommandResult.Command.Options.First(o => o.Name == "available"));
583567
var showAll = context.ParseResult.GetValueForOption(
584-
(Option<bool>)context.ParseResult.CommandResult.Command.Options.First(o => o.Name == "all"));
585-
586-
var formatter = useJson
587-
? (IOutputFormatter)new JsonOutputFormatter(Console.Out)
588-
: new SpectreOutputFormatter();
568+
(Option<bool>)context.ParseResult.CommandResult.Command.Options.First(o => o.Name == "all")); var formatter = Program.GetFormatter(context);
589569

590570
try
591571
{
@@ -701,9 +681,7 @@ void WriteGroupedPackages(IEnumerable<SdkPackage> pkgs, string title)
701681
var androidProvider = Program.AndroidProvider;
702682

703683
var useJson = context.ParseResult.GetValueForOption(GlobalOptions.JsonOption);
704-
var formatter = useJson
705-
? (IOutputFormatter)new JsonOutputFormatter(Console.Out)
706-
: new SpectreOutputFormatter();
684+
var formatter = Program.GetFormatter(context);
707685

708686
try
709687
{
@@ -830,11 +808,7 @@ private static Command CreateSdkUninstallCommand()
830808
var useJson = context.ParseResult.GetValueForOption(GlobalOptions.JsonOption);
831809
var dryRun = context.ParseResult.GetValueForOption(GlobalOptions.DryRunOption);
832810
var packages = context.ParseResult.GetValueForArgument(
833-
(Argument<string[]>)context.ParseResult.CommandResult.Command.Arguments.First());
834-
835-
var formatter = useJson
836-
? (IOutputFormatter)new JsonOutputFormatter(Console.Out)
837-
: new SpectreOutputFormatter();
811+
(Argument<string[]>)context.ParseResult.CommandResult.Command.Arguments.First()); var formatter = Program.GetFormatter(context);
838812

839813
try
840814
{
@@ -876,9 +850,7 @@ private static Command CreateEmulatorCommand()
876850
var androidProvider = Program.AndroidProvider;
877851

878852
var useJson = context.ParseResult.GetValueForOption(GlobalOptions.JsonOption);
879-
var formatter = useJson
880-
? (IOutputFormatter)new JsonOutputFormatter(Console.Out)
881-
: new SpectreOutputFormatter();
853+
var formatter = Program.GetFormatter(context);
882854

883855
try
884856
{
@@ -933,11 +905,7 @@ private static Command CreateEmulatorCommand()
933905
var device = context.ParseResult.GetValueForOption(
934906
(Option<string>)context.ParseResult.CommandResult.Command.Options.First(o => o.Name == "device"));
935907
var force = context.ParseResult.GetValueForOption(
936-
(Option<bool>)context.ParseResult.CommandResult.Command.Options.First(o => o.Name == "force"));
937-
938-
var formatter = useJson
939-
? (IOutputFormatter)new JsonOutputFormatter(Console.Out)
940-
: new SpectreOutputFormatter();
908+
(Option<bool>)context.ParseResult.CommandResult.Command.Options.First(o => o.Name == "force")); var formatter = Program.GetFormatter(context);
941909

942910
try
943911
{
@@ -1081,11 +1049,7 @@ private static Command CreateEmulatorCommand()
10811049
var coldBoot = context.ParseResult.GetValueForOption(
10821050
(Option<bool>)context.ParseResult.CommandResult.Command.Options.First(o => o.Name == "cold-boot"));
10831051
var wait = context.ParseResult.GetValueForOption(
1084-
(Option<bool>)context.ParseResult.CommandResult.Command.Options.First(o => o.Name == "wait"));
1085-
1086-
var formatter = useJson
1087-
? (IOutputFormatter)new JsonOutputFormatter(Console.Out)
1088-
: new SpectreOutputFormatter();
1052+
(Option<bool>)context.ParseResult.CommandResult.Command.Options.First(o => o.Name == "wait")); var formatter = Program.GetFormatter(context);
10891053

10901054
try
10911055
{
@@ -1174,11 +1138,7 @@ await spectre.StatusAsync($"Starting {name}...", async () =>
11741138

11751139
var useJson = context.ParseResult.GetValueForOption(GlobalOptions.JsonOption);
11761140
var dryRun = context.ParseResult.GetValueForOption(GlobalOptions.DryRunOption);
1177-
var name = context.ParseResult.GetValueForArgument(stopNameArg);
1178-
1179-
var formatter = useJson
1180-
? (IOutputFormatter)new JsonOutputFormatter(Console.Out)
1181-
: new SpectreOutputFormatter();
1141+
var name = context.ParseResult.GetValueForArgument(stopNameArg); var formatter = Program.GetFormatter(context);
11821142

11831143
try
11841144
{
@@ -1289,11 +1249,7 @@ await spectreStop2.StatusAsync($"Stopping {name}...", async () =>
12891249

12901250
var useJson = context.ParseResult.GetValueForOption(GlobalOptions.JsonOption);
12911251
var dryRun = context.ParseResult.GetValueForOption(GlobalOptions.DryRunOption);
1292-
var name = context.ParseResult.GetValueForArgument(deleteNameArg);
1293-
1294-
var formatter = useJson
1295-
? (IOutputFormatter)new JsonOutputFormatter(Console.Out)
1296-
: new SpectreOutputFormatter();
1252+
var name = context.ParseResult.GetValueForArgument(deleteNameArg); var formatter = Program.GetFormatter(context);
12971253

12981254
try
12991255
{

‎src/Tools/Microsoft.Maui.Client/Commands/DeployCommand.cs‎

Lines changed: 1 addition & 5 deletions
Original file line numberDiff line numberDiff line change
@@ -37,11 +37,7 @@ public static Command Create()
3737
var framework = context.ParseResult.GetValueForOption(
3838
(Option<string>)context.ParseResult.CommandResult.Command.Options.First(o => o.Name == "framework"));
3939
var noBuild = context.ParseResult.GetValueForOption(
40-
(Option<bool>)context.ParseResult.CommandResult.Command.Options.First(o => o.Name == "no-build"));
41-
42-
var formatter = useJson
43-
? (IOutputFormatter)new JsonOutputFormatter(Console.Out)
44-
: new SpectreOutputFormatter();
40+
(Option<bool>)context.ParseResult.CommandResult.Command.Options.First(o => o.Name == "no-build")); var formatter = Program.GetFormatter(context);
4541

4642
try
4743
{

‎src/Tools/Microsoft.Maui.Client/Commands/DeviceCommand.cs‎

Lines changed: 10 additions & 19 deletions
Original file line numberDiff line numberDiff line change
@@ -28,22 +28,18 @@ public static Command Create()
2828

2929
private static Command CreateListCommand()
3030
{
31+
var platformOption = new Option<string>("--platform", () => "all", "Filter by platform (android, apple, windows, all)");
3132
var command = new Command("list", "List available devices")
3233
{
33-
new Option<string>("--platform", () => "all", "Filter by platform (android, apple, windows, all)")
34+
platformOption
3435
};
3536

3637
command.SetHandler(async (InvocationContext context) =>
3738
{
3839
var deviceManager = Program.DeviceManager;
39-
40+
var formatter = Program.GetFormatter(context);
4041
var useJson = context.ParseResult.GetValueForOption(GlobalOptions.JsonOption);
41-
var platform = context.ParseResult.GetValueForOption(
42-
(Option<string>)context.ParseResult.CommandResult.Command.Options.First(o => o.Name == "platform"));
43-
44-
var formatter = useJson
45-
? (IOutputFormatter)new JsonOutputFormatter(Console.Out)
46-
: new SpectreOutputFormatter();
42+
var platform = context.ParseResult.GetValueForOption(platformOption);
4743

4844
try
4945
{
@@ -78,25 +74,20 @@ private static Command CreateListCommand()
7874

7975
private static Command CreateScreenshotCommand()
8076
{
77+
var deviceIdArg = new Argument<string>("device-id", "Device ID to capture");
78+
var outputOption = new Option<string>("--output", "Output file path");
8179
var command = new Command("screenshot", "Capture device screenshot")
8280
{
83-
new Argument<string>("device-id", "Device ID to capture"),
84-
new Option<string>("--output", "Output file path")
81+
deviceIdArg, outputOption
8582
};
8683

8784
command.SetHandler(async (InvocationContext context) =>
8885
{
8986
var deviceManager = Program.DeviceManager;
90-
87+
var formatter = Program.GetFormatter(context);
9188
var useJson = context.ParseResult.GetValueForOption(GlobalOptions.JsonOption);
92-
var deviceId = context.ParseResult.GetValueForArgument(
93-
(Argument<string>)context.ParseResult.CommandResult.Command.Arguments.First());
94-
var outputPath = context.ParseResult.GetValueForOption(
95-
(Option<string>)context.ParseResult.CommandResult.Command.Options.First(o => o.Name == "output"));
96-
97-
var formatter = useJson
98-
? (IOutputFormatter)new JsonOutputFormatter(Console.Out)
99-
: new SpectreOutputFormatter();
89+
var deviceId = context.ParseResult.GetValueForArgument(deviceIdArg);
90+
var outputPath = context.ParseResult.GetValueForOption(outputOption);
10091

10192
try
10293
{

‎src/Tools/Microsoft.Maui.Client/Commands/DoctorCommand.cs‎

Lines changed: 1 addition & 5 deletions
Original file line numberDiff line numberDiff line change
@@ -31,11 +31,7 @@ public static Command Create()
3131
var fix = context.ParseResult.GetValueForOption(
3232
(Option<bool>)context.ParseResult.CommandResult.Command.Options.First(o => o.Name == "fix"));
3333
var platform = context.ParseResult.GetValueForOption(
34-
(Option<string>)context.ParseResult.CommandResult.Command.Options.First(o => o.Name == "platform"));
35-
36-
var formatter = useJson
37-
? (IOutputFormatter)new JsonOutputFormatter(Console.Out)
38-
: new SpectreOutputFormatter();
34+
(Option<string>)context.ParseResult.CommandResult.Command.Options.First(o => o.Name == "platform")); var formatter = Program.GetFormatter(context);
3935

4036
try
4137
{

‎src/Tools/Microsoft.Maui.Client/Commands/VersionCommand.cs‎

Lines changed: 2 additions & 2 deletions
Original file line numberDiff line numberDiff line change
@@ -28,7 +28,7 @@ public static Command Create()
2828

2929
if (useJson)
3030
{
31-
var formatter = new JsonOutputFormatter(Console.Out);
31+
var formatter = Program.GetFormatter(context);
3232
formatter.Write(new
3333
{
3434
version = version,
@@ -38,7 +38,7 @@ public static Command Create()
3838
}
3939
else
4040
{
41-
var formatter = new SpectreOutputFormatter();
41+
var formatter = Program.GetFormatter(context);
4242
formatter.WriteVersion(version, $".NET {Environment.Version}", Environment.OSVersion.ToString());
4343
}
4444
});

‎src/Tools/Microsoft.Maui.Client/Program.cs‎

Lines changed: 1 addition & 6 deletions
Original file line numberDiff line numberDiff line change
@@ -55,14 +55,9 @@ public static async Task<int> Main(string[] args)
5555
.UseDefaults()
5656
.UseExceptionHandler((exception, context) =>
5757
{
58-
var useJson = context.ParseResult.FindResultFor(GlobalOptions.JsonOption) is not null &&
59-
context.ParseResult.GetValueForOption(GlobalOptions.JsonOption);
58+
var formatter = GetFormatter(context);
6059
var isCi = context.ParseResult.GetValueForOption(GlobalOptions.CiOption);
6160

62-
var formatter = useJson
63-
? (IOutputFormatter)new JsonOutputFormatter(Console.Out)
64-
: new SpectreOutputFormatter();
65-
6661
formatter.WriteError(exception);
6762
context.ExitCode = 1;
6863

‎src/Tools/Microsoft.Maui.Client/Providers/Android/Adb.cs‎

Lines changed: 3 additions & 3 deletions
Original file line numberDiff line numberDiff line change
@@ -80,7 +80,7 @@ public async Task<List<Device>> GetDevicesAsync(CancellationToken cancellationTo
8080
// Try adb emu avd name first
8181
try
8282
{
83-
var result = await ProcessRunner.RunAsync(AdbPath, $"-s {serial} emu avd name",
83+
var result = await ProcessRunner.RunAsync(AdbPath, $"-s {ProcessRunner.SanitizeArg(serial)} emu avd name",
8484
timeout: TimeSpan.FromSeconds(5), cancellationToken: cancellationToken);
8585
if (result.Success && !string.IsNullOrWhiteSpace(result.StandardOutput))
8686
{
@@ -216,14 +216,14 @@ public async Task<string> TakeScreenshotAsync(string deviceSerial, string output
216216
var remotePath = "/sdcard/screenshot.png";
217217

218218
// Capture screenshot on device
219-
var captureResult = await ProcessRunner.RunAsync(AdbPath!, $"-s {deviceSerial} shell screencap -p {remotePath}", cancellationToken: cancellationToken);
219+
var captureResult = await ProcessRunner.RunAsync(AdbPath!, $"-s {ProcessRunner.SanitizeArg(deviceSerial)} shell screencap -p {remotePath}", cancellationToken: cancellationToken);
220220
if (!captureResult.Success)
221221
throw new MauiToolException(ErrorCodes.AndroidDeviceNotFound,
222222
$"Failed to capture screenshot: {captureResult.StandardError}",
223223
nativeError: captureResult.StandardError);
224224

225225
// Pull screenshot to local path
226-
var pullResult = await ProcessRunner.RunAsync(AdbPath!, $"-s {deviceSerial} pull {remotePath} {outputPath}", cancellationToken: cancellationToken);
226+
var pullResult = await ProcessRunner.RunAsync(AdbPath!, $"-s {ProcessRunner.SanitizeArg(deviceSerial)} pull {remotePath} {ProcessRunner.SanitizeArg(outputPath)}", cancellationToken: cancellationToken);
227227
if (!pullResult.Success)
228228
throw new MauiToolException(ErrorCodes.AndroidDeviceNotFound,
229229
$"Failed to pull screenshot: {pullResult.StandardError}",

‎src/Tools/Microsoft.Maui.Client/Providers/Android/AndroidProvider.cs‎

Lines changed: 6 additions & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -44,6 +44,12 @@ public AndroidProvider(IJdkManager jdkManager, SdkManager? sdkManager = null, Ad
4444
_avdManager = avdManager ?? new AvdManager(() => SdkPath, () => JdkPath, _adb);
4545
}
4646

47+
public void Dispose()
48+
{
49+
_sdkManager.Dispose();
50+
GC.SuppressFinalize(this);
51+
}
52+
4753
public async Task<List<HealthCheck>> CheckHealthAsync(CancellationToken cancellationToken = default)
4854
{
4955
var checks = new List<HealthCheck>();

‎src/Tools/Microsoft.Maui.Client/Providers/Android/IAndroidProvider.cs‎

Lines changed: 1 addition & 1 deletion
Original file line numberDiff line numberDiff line change
@@ -8,7 +8,7 @@ namespace Microsoft.Maui.Client.Providers.Android;
88
/// <summary>
99
/// Interface for Android SDK and device operations.
1010
/// </summary>
11-
public interface IAndroidProvider
11+
public interface IAndroidProvider : IDisposable
1212
{
1313
/// <summary>
1414
/// Gets the Android SDK path.

0 commit comments

Comments
 (0)