Skip to content

Commit 1d1bf33

Browse files
jonathanpeppersjonathanpeppers
authored
[Xamarin.Android.Build.Tasks] use copilot to opt into NRT (#9770)
Context: https://code.visualstudio.com/docs/copilot/copilot-customization We have various MSBuild tasks that have not enabled C# nullable reference types. This is a bit tedious to do, so I tried VS Code Insiders and GitHub Copilot "Edits" to do this work. I did this for just a few files for now. I introduced a new file `.github/copilot-instructions.md` to keep copilot from going off the rails! I mostly had to fight it to respect our code formatting, not delete comments, etc. I would recommend the settings: "github.copilot.chat.codeGeneration.useInstructionFiles": true, "github.copilot.chat.codeGeneration.instructions": [ { "file": ".editorconfig" }, { "file": "README.md" } ], * `github.copilot.chat.codeGeneration.useInstructionFiles`: enables the use of `.github/copilot-instructions.md` * `github.copilot.chat.codeGeneration.instructions`: attaches `.editorconfig` and `README.md` so copilot has general context about this repo. In the end it did a decent job, but I had to manually fix a few things.
1 parent 3822f2b commit 1d1bf33

10 files changed

+282
-142
lines changed

‎.github/copilot-instructions.md

Lines changed: 107 additions & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -0,0 +1,107 @@
1+
# Instructions for AIs
2+
3+
This repository is .NET for Android.
4+
5+
This is the main branch targeting .NET 10.
6+
7+
## Nullable Reference Types
8+
9+
When opting C# code into nullable reference types:
10+
11+
* Add `#nullable enable` at the top of the file.
12+
13+
* Don't *ever* use `!` to handle `null`!
14+
15+
* Declare variables non-nullable, and check for `null` at entry points.
16+
17+
* Use `throw new ArgumentNullException (nameof (parameter))` in `netstandard2.0` projects.
18+
19+
* Use `ArgumentNullException.ThrowIfNull (parameter)` in Android projects that will be .NET 10+.
20+
21+
* `[Required]` properties in MSBuild task classes should always be non-nullable with a default value.
22+
23+
* Non-`[Required]` properties should be nullable and have null-checks in C# code using them.
24+
25+
* For MSBuild task properties like:
26+
27+
```csharp
28+
public string NonRequiredProperty { get; set; }
29+
public ITaskItem [] NonRequiredItemGroup { get; set; }
30+
31+
[Output]
32+
public string OutputProperty { get; set; }
33+
[Output]
34+
public ITaskItem [] OutputItemGroup { get; set; }
35+
36+
[Required]
37+
public string RequiredProperty { get; set; }
38+
[Required]
39+
public ITaskItem [] RequiredItemGroup { get; set; }
40+
```
41+
42+
Fix them such as:
43+
44+
```csharp
45+
public string? NonRequiredProperty { get; set; }
46+
public ITaskItem []? NonRequiredItemGroup { get; set; }
47+
48+
[Output]
49+
public string? OutputProperty { get; set; }
50+
[Output]
51+
public ITaskItem []? OutputItemGroup { get; set; }
52+
53+
[Required]
54+
public string RequiredProperty { get; set; } = "";
55+
[Required]
56+
public ITaskItem [] RequiredItemGroup { get; set; } = [];
57+
```
58+
59+
If you see a `string.IsNullOrEmpty()` check:
60+
61+
```csharp
62+
if (!string.IsNullOrEmpty (NonRequiredProperty)) {
63+
// Code here
64+
}
65+
```
66+
67+
Convert this to:
68+
69+
```csharp
70+
if (NonRequiredProperty is { Length: > 0 }) {
71+
// Code here
72+
}
73+
```
74+
75+
## Formatting
76+
77+
C# code uses tabs (not spaces) and the Mono code-formatting style defined in `.editorconfig`
78+
79+
* Your mission is to make diffs as absolutely as small as possible, preserving existing code formatting.
80+
81+
* If you encounter additional spaces or formatting within existing code blocks, LEAVE THEM AS-IS.
82+
83+
* If you encounter code comments, LEAVE THEM AS-IS.
84+
85+
* Place a space prior to any parentheses `(` or `[`
86+
87+
* Use `""` for empty string and *not* `string.Empty`
88+
89+
* Use `[]` for empty arrays and *not* `Array.Empty<T>()`
90+
91+
Examples of properly formatted code:
92+
93+
```csharp
94+
Foo ();
95+
Bar (1, 2, "test");
96+
myarray [0] = 1;
97+
98+
if (someValue) {
99+
// Code here
100+
}
101+
102+
try {
103+
// Code here
104+
} catch (Exception e) {
105+
// Code here
106+
}
107+
```

‎src/Xamarin.Android.Build.Tasks/Tasks/D8.cs

Lines changed: 17 additions & 15 deletions
Original file line numberDiff line numberDiff line change
@@ -1,3 +1,5 @@
1+
#nullable enable
2+
13
using Microsoft.Build.Framework;
24
using Microsoft.Build.Utilities;
35
using System.Collections.Generic;
@@ -15,32 +17,32 @@ public class D8 : JavaToolTask
1517
public override string TaskPrefix => "DX8";
1618

1719
[Required]
18-
public string JarPath { get; set; }
20+
public string JarPath { get; set; } = "";
1921

2022
/// <summary>
2123
/// Output for *.dex files. R8 can be invoked for just --main-dex-list-output, so this is not [Required]
2224
/// </summary>
23-
public string OutputDirectory { get; set; }
25+
public string? OutputDirectory { get; set; }
2426

2527
/// <summary>
2628
/// It is loaded to calculate --min-api, which is used by desugaring part to determine which levels of desugaring it performs.
2729
/// </summary>
28-
public string AndroidManifestFile { get; set; }
30+
public string? AndroidManifestFile { get; set; }
2931

3032
// general d8 feature options.
3133
public bool Debug { get; set; }
3234
public bool EnableDesugar { get; set; } = true;
3335

3436
// Java libraries to embed or reference
35-
public string ClassesZip { get; set; }
37+
public string? ClassesZip { get; set; }
3638
[Required]
37-
public string JavaPlatformJarPath { get; set; }
38-
public ITaskItem [] JavaLibrariesToEmbed { get; set; }
39-
public ITaskItem [] AlternativeJarLibrariesToEmbed { get; set; }
40-
public ITaskItem [] JavaLibrariesToReference { get; set; }
41-
public ITaskItem [] MapDiagnostics { get; set; }
39+
public string JavaPlatformJarPath { get; set; } = "";
40+
public ITaskItem []? JavaLibrariesToEmbed { get; set; }
41+
public ITaskItem []? AlternativeJarLibrariesToEmbed { get; set; }
42+
public ITaskItem []? JavaLibrariesToReference { get; set; }
43+
public ITaskItem []? MapDiagnostics { get; set; }
4244

43-
public string ExtraArguments { get; set; }
45+
public string? ExtraArguments { get; set; }
4446

4547
protected override string GenerateCommandLineCommands ()
4648
{
@@ -55,22 +57,22 @@ protected virtual CommandLineBuilder GetCommandLineBuilder ()
5557
{
5658
var cmd = new CommandLineBuilder ();
5759

58-
if (!string.IsNullOrEmpty (JavaOptions)) {
60+
if (JavaOptions is { Length: > 0 }) {
5961
cmd.AppendSwitch (JavaOptions);
6062
}
6163
cmd.AppendSwitchIfNotNull ("-Xmx", JavaMaximumHeapSize);
6264
cmd.AppendSwitchIfNotNull ("-classpath ", JarPath);
6365
cmd.AppendSwitch (MainClass);
6466

65-
if (!string.IsNullOrEmpty (ExtraArguments))
67+
if (ExtraArguments is { Length: > 0 })
6668
cmd.AppendSwitch (ExtraArguments); // it should contain "--dex".
6769
if (Debug)
6870
cmd.AppendSwitch ("--debug");
6971
else
7072
cmd.AppendSwitch ("--release");
7173

7274
//NOTE: if this is blank, we can omit --min-api in this call
73-
if (!string.IsNullOrEmpty (AndroidManifestFile)) {
75+
if (AndroidManifestFile is { Length: > 0 }) {
7476
var doc = AndroidAppManifest.Load (AndroidManifestFile, MonoAndroidHelper.SupportedVersions);
7577
if (doc.MinSdkVersion.HasValue) {
7678
MinSdkVersion = doc.MinSdkVersion.Value;
@@ -90,7 +92,7 @@ protected virtual CommandLineBuilder GetCommandLineBuilder ()
9092
}
9193
} else if (JavaLibrariesToEmbed != null) {
9294
Log.LogDebugMessage (" processing ClassesZip, JavaLibrariesToEmbed...");
93-
if (!string.IsNullOrEmpty (ClassesZip) && File.Exists (ClassesZip)) {
95+
if (ClassesZip is { Length: > 0 } && File.Exists (ClassesZip)) {
9496
injars.Add (ClassesZip);
9597
}
9698
foreach (var jar in JavaLibrariesToEmbed) {
@@ -114,7 +116,7 @@ protected virtual CommandLineBuilder GetCommandLineBuilder ()
114116
foreach (var diagnostic in MapDiagnostics) {
115117
var from = diagnostic.ItemSpec;
116118
var to = diagnostic.GetMetadata ("To");
117-
if (string.IsNullOrEmpty (from) || string.IsNullOrEmpty (to))
119+
if (from is not { Length: > 0 } || to is not { Length: > 0 })
118120
continue;
119121
cmd.AppendSwitch ("--map-diagnostics");
120122
cmd.AppendSwitch (from);

‎src/Xamarin.Android.Build.Tasks/Tasks/R8.cs

Lines changed: 11 additions & 9 deletions
Original file line numberDiff line numberDiff line change
@@ -1,3 +1,5 @@
1+
#nullable enable
2+
13
using System;
24
using Microsoft.Build.Framework;
35
using Microsoft.Build.Utilities;
@@ -17,22 +19,22 @@ public class R8 : D8
1719
public override string TaskPrefix => "R8S";
1820

1921
[Required]
20-
public string AndroidSdkBuildToolsPath { get; set; }
22+
public string AndroidSdkBuildToolsPath { get; set; } = "";
2123

2224
// multidex
2325
public bool EnableMultiDex { get; set; }
24-
public ITaskItem [] CustomMainDexListFiles { get; set; }
25-
public string MultiDexMainDexListFile { get; set; }
26+
public ITaskItem []? CustomMainDexListFiles { get; set; }
27+
public string? MultiDexMainDexListFile { get; set; }
2628

2729
// proguard-like configuration settings
2830
public bool EnableShrinking { get; set; } = true;
2931
public bool IgnoreWarnings { get; set; }
30-
public string AcwMapFile { get; set; }
31-
public string ProguardGeneratedReferenceConfiguration { get; set; }
32-
public string ProguardGeneratedApplicationConfiguration { get; set; }
33-
public string ProguardCommonXamarinConfiguration { get; set; }
34-
public string ProguardMappingFileOutput { get; set; }
35-
public string [] ProguardConfigurationFiles { get; set; }
32+
public string? AcwMapFile { get; set; }
33+
public string? ProguardGeneratedReferenceConfiguration { get; set; }
34+
public string? ProguardGeneratedApplicationConfiguration { get; set; }
35+
public string? ProguardCommonXamarinConfiguration { get; set; }
36+
public string? ProguardMappingFileOutput { get; set; }
37+
public string []? ProguardConfigurationFiles { get; set; }
3638

3739
protected override string MainClass => "com.android.tools.r8.R8";
3840

‎src/Xamarin.Android.Build.Tasks/Tasks/ResolveAndroidTooling.cs

Lines changed: 25 additions & 20 deletions
Original file line numberDiff line numberDiff line change
@@ -1,3 +1,5 @@
1+
#nullable enable
2+
13
using Microsoft.Build.Framework;
24
using Microsoft.Build.Utilities;
35
using System;
@@ -20,54 +22,54 @@ public class ResolveAndroidTooling : AndroidTask
2022
{
2123
public override string TaskPrefix => "RAT";
2224

23-
public string TargetPlatformVersion { get; set; }
25+
public string? TargetPlatformVersion { get; set; }
2426

25-
public string AndroidSdkPath { get; set; }
27+
public string? AndroidSdkPath { get; set; }
2628

27-
public string AndroidSdkBuildToolsVersion { get; set; }
29+
public string? AndroidSdkBuildToolsVersion { get; set; }
2830

29-
public string CommandLineToolsVersion { get; set; }
31+
public string? CommandLineToolsVersion { get; set; }
3032

31-
public string ProjectFilePath { get; set; }
33+
public string? ProjectFilePath { get; set; }
3234

33-
public string SequencePointsMode { get; set; }
35+
public string? SequencePointsMode { get; set; }
3436

3537
public bool AotAssemblies { get; set; }
3638

3739
public bool AndroidApplication { get; set; } = true;
3840

3941
[Output]
40-
public string AndroidApiLevel { get; set; }
42+
public string? AndroidApiLevel { get; set; }
4143

4244
[Output]
43-
public string AndroidApiLevelName { get; set; }
45+
public string? AndroidApiLevelName { get; set; }
4446

4547
[Output]
46-
public string AndroidSdkBuildToolsPath { get; set; }
48+
public string? AndroidSdkBuildToolsPath { get; set; }
4749

4850
[Output]
49-
public string AndroidSdkBuildToolsBinPath { get; set; }
51+
public string? AndroidSdkBuildToolsBinPath { get; set; }
5052

5153
[Output]
52-
public string ZipAlignPath { get; set; }
54+
public string? ZipAlignPath { get; set; }
5355

5456
[Output]
55-
public string AndroidSequencePointsMode { get; set; }
57+
public string? AndroidSequencePointsMode { get; set; }
5658

5759
[Output]
58-
public string LintToolPath { get; set; }
60+
public string? LintToolPath { get; set; }
5961

6062
[Output]
61-
public string ApkSignerJar { get; set; }
63+
public string? ApkSignerJar { get; set; }
6264

6365
[Output]
6466
public bool AndroidUseApkSigner { get; set; }
6567

6668
[Output]
67-
public string Aapt2Version { get; set; }
69+
public string? Aapt2Version { get; set; }
6870

6971
[Output]
70-
public string Aapt2ToolPath { get; set; }
72+
public string? Aapt2ToolPath { get; set; }
7173

7274
protected static readonly bool IsWindows = Path.DirectorySeparatorChar == '\\';
7375
protected static readonly string ZipAlign = IsWindows ? "zipalign.exe" : "zipalign";
@@ -88,7 +90,7 @@ public override bool RunTask ()
8890
string toolsZipAlignPath = Path.Combine (AndroidSdkPath, "tools", ZipAlign);
8991
bool findZipAlign = (string.IsNullOrEmpty (ZipAlignPath) || !Directory.Exists (ZipAlignPath)) && !File.Exists (toolsZipAlignPath);
9092

91-
var lintPaths = MonoAndroidHelper.AndroidSdk.GetCommandLineToolsPaths (CommandLineToolsVersion)
93+
var lintPaths = MonoAndroidHelper.AndroidSdk.GetCommandLineToolsPaths (CommandLineToolsVersion ?? "")
9294
.SelectMany (p => new[]{
9395
p,
9496
Path.Combine (p, "bin"),
@@ -102,7 +104,7 @@ public override bool RunTask ()
102104
}
103105
}
104106

105-
foreach (var dir in MonoAndroidHelper.AndroidSdk.GetBuildToolsPaths (AndroidSdkBuildToolsVersion)) {
107+
foreach (var dir in MonoAndroidHelper.AndroidSdk.GetBuildToolsPaths (AndroidSdkBuildToolsVersion ?? "")) {
106108
Log.LogDebugMessage ("Trying build-tools path: {0}", dir);
107109
if (dir == null || !Directory.Exists (dir))
108110
continue;
@@ -222,7 +224,7 @@ bool GetAapt2Version (string aapt2Exe)
222224
// because the path to aapt2 is in the key and the value is a string.
223225
var key = ($"{nameof (ResolveAndroidTooling)}.{nameof (Aapt2Version)}", aapt2Tool);
224226
var cached = BuildEngine4.GetRegisteredTaskObject (key, RegisteredTaskObjectLifetime.AppDomain) as string;
225-
if (!string.IsNullOrEmpty (cached)) {
227+
if (cached is { Length: > 0 }) {
226228
Log.LogDebugMessage ($"Using cached value for {nameof (Aapt2Version)}: {cached}");
227229
Aapt2Version = cached;
228230
return true;
@@ -254,7 +256,10 @@ bool GetAapt2Version (string aapt2Exe)
254256

255257
protected int GetMaxStableApiLevel ()
256258
{
257-
return MonoAndroidHelper.SupportedVersions.MaxStableVersion.ApiLevel;
259+
var stableVersion = MonoAndroidHelper.SupportedVersions.MaxStableVersion;
260+
if (stableVersion is null)
261+
throw new ArgumentNullException ("MaxStableVersion");
262+
return stableVersion.ApiLevel;
258263
}
259264
}
260265
}

0 commit comments

Comments
 (0)