Skip to content

Add nullable annotations #101

New issue

Have a question about this project? Sign up for a free GitHub account to open an issue and contact its maintainers and the community.

Sign up for GitHub

By clicking “Sign up for GitHub”, you agree to our terms of service and privacy statement. We’ll occasionally send you account related emails.

Already on GitHub? Sign in to your account

Closed
wants to merge 5 commits into from
Closed

Add nullable annotations #101

Show file tree
Hide file tree
Changes from 4 commits
Commits
Show all changes
5 commits
Select commit Hold shift + click to select a range
770992b
Add nullable annotations
Joy-less Feb 11, 2025
70e9fd8
Update Microsoft.CodeAnalysis.CSharp
Joy-less Feb 11, 2025
3bc96df
Merge branch 'Cat-Lips:main' into add-nullable-annotations
Joy-less Feb 11, 2025
c2dd68a
Merge branch 'main' into add-nullable-annotations
Cat-Lips Jun 6, 2025
ee868e0
TESTS PASS OK
Cat-Lips Jun 6, 2025
File filter

Filter by extension

Filter by extension
Conversations
Failed to load comments.
Loading
Jump to
Jump to file
Failed to load files.
Loading
Diff view
Diff view
2 changes: 1 addition & 1 deletion SourceGenerators/AutoloadExtensions/AutoloadAttribute.cs
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -3,7 +3,7 @@
namespace Godot;

[AttributeUsage(AttributeTargets.Class)]
public class AutoloadAttribute([CallerFilePath] string ClassPath = null) : Attribute
public class AutoloadAttribute([CallerFilePath] string ClassPath = null!) : Attribute
{
public string ClassPath { get; } = ClassPath;
}
Expand Down
4 changes: 2 additions & 2 deletions SourceGenerators/AutoloadExtensions/AutoloadDataModel.cs
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -4,11 +4,11 @@ namespace GodotSharp.SourceGenerators.AutoloadExtensions;

internal class AutoloadDataModel : ClassDataModel
{
public record AutoloadData(string Type, string GodotName, string DisplayName);
public record AutoloadData(string Type, string? GodotName, string? DisplayName);

public IList<AutoloadData> Autoloads { get; }

public AutoloadDataModel(Compilation compilation, INamedTypeSymbol symbol, string csPath, string gdRoot, IDictionary<string, string> lookup)
public AutoloadDataModel(Compilation compilation, INamedTypeSymbol symbol, string csPath, string? gdRoot, IDictionary<string, string> lookup)
: base(symbol)
{
Autoloads = AutoloadScraper.GetAutoloads(compilation, csPath, gdRoot)
Expand Down
33 changes: 17 additions & 16 deletions SourceGenerators/AutoloadExtensions/AutoloadScraper.cs
View file
Open in desktop
Original file line number Diff line number Diff line change
@@ -1,4 +1,5 @@
using System.Diagnostics;
using System.Diagnostics.CodeAnalysis;
using System.Text.RegularExpressions;
using Microsoft.CodeAnalysis;

Expand All @@ -11,7 +12,7 @@ internal static class AutoloadScraper
private const string AutoloadRegexStr = @"^(?<Name>.+?)=""\*res:\/\/(?<Path>.+?)""$";
private static readonly Regex AutoloadRegex = new(AutoloadRegexStr, RegexOptions.Compiled | RegexOptions.ExplicitCapture);

public static IEnumerable<Autoload> GetAutoloads(Compilation compilation, string csFile, string gdRoot)
public static IEnumerable<Autoload> GetAutoloads(Compilation compilation, string csFile, string? gdRoot)
{
var gdFile = GD.GetProjectFile(csFile, gdRoot);
Log.Debug($"Scraping {gdFile} [Compiling {csFile}]");
Expand Down Expand Up @@ -42,7 +43,7 @@ IEnumerable<Autoload> MatchAutoloads(string gdFile)
}
}

bool TryMatchAutoload(string line, out string name, out string path)
bool TryMatchAutoload(string line, [NotNullWhen(true)] out string? name, [NotNullWhen(true)] out string? path)
{
var match = AutoloadRegex.Match(line);
if (match.Success)
Expand All @@ -58,13 +59,13 @@ bool TryMatchAutoload(string line, out string name, out string path)
return false;
}

string TryGetType(string path)
string? TryGetType(string? path)
{
return Path.GetExtension(path) switch
{
".gd" => MiniGdScraper.TryGetType(compilation, gdRoot, path),
".cs" => MiniCsScraper.TryGetType(compilation, gdRoot, path),
".tscn" => MiniTscnScraper.TryGetType(compilation, gdRoot, path),
".gd" => MiniGdScraper.TryGetType(compilation, gdRoot, path!),
".cs" => MiniCsScraper.TryGetType(compilation, gdRoot, path!),
".tscn" => MiniTscnScraper.TryGetType(compilation, gdRoot, path!),
_ => null,
};
}
Expand All @@ -84,7 +85,7 @@ public static string TryGetType(Compilation compilation, string gdRoot, string p
var file = Path.Combine(gdRoot, path);
Log.Debug($">>> Scraping type from gd: {file}");

string type = null;
string? type = null;
foreach (var line in File.ReadLines(file)
.Where(line => line is not "" && !line.StartsWith("#")))
{
Expand All @@ -100,7 +101,7 @@ public static string TryGetType(Compilation compilation, string gdRoot, string p
Log.Debug($"<<< {type}");
return type;

bool TryMatchExtends(string line, ref string type)
bool TryMatchExtends(string line, ref string? type)
{
var match = ExtendsRegex1.Match(line);
if (match.Success)
Expand All @@ -124,7 +125,7 @@ bool TryMatchExtends(string line, ref string type)
return false;
}

string GetValidType(string type)
string GetValidType(string? type)
{
if (type is null)
{
Expand Down Expand Up @@ -164,7 +165,7 @@ public static string TryGetType(Compilation compilation, string gdRoot, string p
Log.Debug($"<<< {type}");
return type;

static string GetValidType(string type)
static string GetValidType(string? type)
{
if (type is null)
{
Expand Down Expand Up @@ -201,7 +202,7 @@ public static string TryGetType(Compilation compilation, string gdRoot, string t
var file = Path.Combine(gdRoot, tscn);
Log.Debug($">>> Scraping type from tscn: {file}");

string type = null;
string? type = null;
var doneRes = false;
var doneRoot = false;
var doneValues = false;
Expand Down Expand Up @@ -254,7 +255,7 @@ bool TryMatchRes(string line)
return false;
}

bool TryMatchRoot(string line, ref string type)
bool TryMatchRoot(string line, ref string? type)
{
if (doneRoot) return false;

Expand Down Expand Up @@ -298,7 +299,7 @@ bool TryMatchRoot(string line, ref string type)
return true;
}

bool TryMatchValues(string line, ref string type)
bool TryMatchValues(string line, ref string? type)
{
if (doneValues) return false;

Expand All @@ -316,7 +317,7 @@ bool TryMatchValues(string line, ref string type)

return true;

bool TryMatchScript(string line, ref string type)
bool TryMatchScript(string line, ref string? type)
{
var match = ScriptRegexGD4.Match(line);
if (match.Success)
Expand All @@ -338,7 +339,7 @@ bool TryMatchScript(string line, ref string type)

return false;

string TryGetType(string path)
string? TryGetType(string path)
{
return Path.GetExtension(path) switch
{
Expand All @@ -350,7 +351,7 @@ string TryGetType(string path)
}
}

string GetValidType(string type)
string GetValidType(string? type)
{
if (type is null)
{
Expand Down
14 changes: 8 additions & 6 deletions SourceGenerators/AutoloadExtensions/AutoloadSourceGenerator.cs
View file
Open in desktop
Original file line number Diff line number Diff line change
@@ -1,4 +1,5 @@
using Microsoft.CodeAnalysis;
using System.Diagnostics.CodeAnalysis;
using Microsoft.CodeAnalysis;
using Microsoft.CodeAnalysis.Diagnostics;
using Scriban;

Expand All @@ -7,6 +8,7 @@ namespace GodotSharp.SourceGenerators.AutoloadExtensions;
[Generator]
internal class AutoloadSourceGenerator : SourceGeneratorForDeclaredTypeWithAttribute<Godot.AutoloadAttribute>
{
[field: MaybeNull]
private static Template AutoloadTemplate => field ??= Template.Parse(Resources.AutoloadTemplate);

protected override IEnumerable<(string Name, string Source)> StaticSources
Expand All @@ -17,7 +19,7 @@ internal class AutoloadSourceGenerator : SourceGeneratorForDeclaredTypeWithAttri
}
}

protected override (string GeneratedCode, DiagnosticDetail Error) GenerateCode(Compilation compilation, SyntaxNode _, INamedTypeSymbol symbol, AttributeData attribute, AnalyzerConfigOptions options)
protected override (string? GeneratedCode, DiagnosticDetail? Error) GenerateCode(Compilation compilation, SyntaxNode _, INamedTypeSymbol symbol, AttributeData attribute, AnalyzerConfigOptions options)
{
var model = new AutoloadDataModel(compilation, symbol, ReconstructAttribute().ClassPath, options.TryGetGodotProjectDir(), GetAutoloadRenameLookup());
Log.Debug($"--- MODEL ---\n{model}\n");
Expand All @@ -28,15 +30,15 @@ protected override (string GeneratedCode, DiagnosticDetail Error) GenerateCode(C
return (output, null);

Godot.AutoloadAttribute ReconstructAttribute()
=> new((string)attribute.ConstructorArguments[0].Value);
=> new((string)attribute.ConstructorArguments[0].Value!);
Copy link

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

🛠️ Refactor suggestion

Potentially unsafe attribute value casting.

Multiple uses of null-forgiving operators when casting attribute constructor arguments assume these values are never null, which could cause runtime exceptions if attributes are malformed.

Consider safer attribute value extraction:

-=> new((string)attribute.ConstructorArguments[0].Value!);
+=> new(attribute.ConstructorArguments[0].Value as string ?? throw new InvalidOperationException("Autoload attribute requires a string argument"));

-.Where(x => x.AttributeClass?.Name == RenameAttribute)
+.Where(x => x.AttributeClass?.Name == RenameAttribute)
 .ToDictionary(
-    x => (string)x.ConstructorArguments[1].Value!,   // GodotName
-    x => (string)x.ConstructorArguments[0].Value!);  // DisplayName
+    x => x.ConstructorArguments[1].Value as string ?? throw new InvalidOperationException("Rename attribute GodotName must be a string"),
+    x => x.ConstructorArguments[0].Value as string ?? throw new InvalidOperationException("Rename attribute DisplayName must be a string"));

Also applies to: 38-42

🤖 Prompt for AI Agents 
In SourceGenerators/AutoloadExtensions/AutoloadSourceGenerator.cs at lines 33
and 38-42, the code uses null-forgiving operators when casting attribute
constructor arguments, which can cause runtime exceptions if the attribute
values are null. To fix this, add null checks before casting or use safe casting
methods like 'as' with null checks, and handle the null case appropriately to
avoid exceptions from malformed attributes.


IDictionary<string, string> GetAutoloadRenameLookup()
{
return symbol.GetAttributes()
.Where(x => x.AttributeClass.Name == RenameAttribute)
.Where(x => x.AttributeClass?.Name == RenameAttribute)
.ToDictionary(
x => (string)x.ConstructorArguments[1].Value, // GodotName
x => (string)x.ConstructorArguments[0].Value); // DisplayName
x => (string)x.ConstructorArguments[1].Value!, // GodotName
x => (string)x.ConstructorArguments[0].Value!); // DisplayName
}
}

Expand Down
8 changes: 5 additions & 3 deletions SourceGenerators/CodeCommentsExtensions/CodeCommentsSourceGenerator.cs
View file
Open in desktop
Original file line number Diff line number Diff line change
@@ -1,4 +1,5 @@
using Microsoft.CodeAnalysis;
using System.Diagnostics.CodeAnalysis;
using Microsoft.CodeAnalysis;
using Microsoft.CodeAnalysis.Diagnostics;
using Scriban;

Expand All @@ -7,9 +8,10 @@ namespace GodotSharp.SourceGenerators.CodeCommentsExtensions;
[Generator]
internal class CodeCommentsSourceGenerator : SourceGeneratorForDeclaredTypeWithAttribute<Godot.CodeCommentsAttribute>
{
[field: MaybeNull]
private static Template CodeCommentsTemplate => field ??= Template.Parse(Resources.CodeCommentsTemplate);

protected override (string GeneratedCode, DiagnosticDetail Error) GenerateCode(Compilation compilation, SyntaxNode node, INamedTypeSymbol symbol, AttributeData attribute, AnalyzerConfigOptions options)
protected override (string? GeneratedCode, DiagnosticDetail? Error) GenerateCode(Compilation compilation, SyntaxNode node, INamedTypeSymbol symbol, AttributeData attribute, AnalyzerConfigOptions options)
{
var model = new CodeCommentsDataModel(symbol, node, ReconstructAttribute().Strip);
Log.Debug($"--- MODEL ---\n{model}\n");
Expand All @@ -20,6 +22,6 @@ protected override (string GeneratedCode, DiagnosticDetail Error) GenerateCode(C
return (output, null);

Godot.CodeCommentsAttribute ReconstructAttribute()
=> new((string)attribute.ConstructorArguments[0].Value);
=> new((string)attribute.ConstructorArguments[0].Value!);
Copy link

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

🛠️ Refactor suggestion

Potentially unsafe cast with null-forgiving operator.

Using (string)attribute.ConstructorArguments[0].Value! assumes the constructor argument value is never null, but this could fail if the attribute is malformed.

Consider safer casting:

-=> new((string)attribute.ConstructorArguments[0].Value!);
+=> new(attribute.ConstructorArguments[0].Value as string ?? throw new InvalidOperationException("CodeComments attribute requires a string argument"));
📝 Committable suggestion

‼️ IMPORTANT
Carefully review the code before committing. Ensure that it accurately replaces the highlighted code, contains no missing lines, and has no issues with indentation. Thoroughly test & benchmark the code to ensure it meets the requirements.

Suggested change
=> new((string)attribute.ConstructorArguments[0].Value!);
=> new(attribute.ConstructorArguments[0].Value as string
?? throw new InvalidOperationException("CodeComments attribute requires a string argument"));
🤖 Prompt for AI Agents 
In SourceGenerators/CodeCommentsExtensions/CodeCommentsSourceGenerator.cs at
line 25, the code uses a potentially unsafe cast with a null-forgiving operator
on attribute.ConstructorArguments[0].Value. To fix this, replace the direct cast
with a safer approach that checks for null before casting, such as using a
conditional cast or null-coalescing operator to handle possible null values
gracefully and avoid runtime exceptions.

}
}
8 changes: 4 additions & 4 deletions SourceGenerators/DiagnosticDetail.cs
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -2,8 +2,8 @@

public record DiagnosticDetail
{
public string Id { get; init; }
public string Category { get; init; }
public string Title { get; init; }
public string Message { get; init; }
public string? Id { get; init; }
public string? Category { get; init; }
public required string Title { get; init; }
public required string Message { get; init; }
}
6 changes: 3 additions & 3 deletions SourceGenerators/GodotNotifyExtensions/GodotNotifyDataModel.cs
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -10,8 +10,8 @@ internal class GodotNotifyDataModel : MemberDataModel
public string Field { get; }

public string Modifiers { get; }
public string GetAccess { get; }
public string SetAccess { get; }
public string? GetAccess { get; }
public string? SetAccess { get; }

public bool ClassIsResource { get; }
public bool ValueIsResource { get; }
Expand All @@ -36,7 +36,7 @@ public GodotNotifyDataModel(IPropertySymbol symbol, SyntaxNode node)
static bool IsResource(ITypeSymbol type)
=> type.InheritsFrom("Resource");

static string GetAccessibility(IMethodSymbol accessor, string @default)
static string? GetAccessibility(IMethodSymbol? accessor, string @default)
{
var accessibility = accessor?.GetDeclaredAccessibility();
return accessibility is null ? null
Expand Down
6 changes: 4 additions & 2 deletions SourceGenerators/GodotNotifyExtensions/GodotNotifySourceGenerator.cs
View file
Open in desktop
Original file line number Diff line number Diff line change
@@ -1,4 +1,5 @@
using Microsoft.CodeAnalysis;
using System.Diagnostics.CodeAnalysis;
using Microsoft.CodeAnalysis;
using Microsoft.CodeAnalysis.Diagnostics;
using Scriban;

Expand All @@ -7,9 +8,10 @@ namespace GodotSharp.SourceGenerators.GodotNotifyExtensions;
[Generator]
internal class GodotNotifySourceGenerator : SourceGeneratorForDeclaredPropertyWithAttribute<Godot.NotifyAttribute>
{
[field: MaybeNull]
private static Template GodotNotifyTemplate => field ??= Template.Parse(Resources.GodotNotifyTemplate);

protected override (string GeneratedCode, DiagnosticDetail Error) GenerateCode(Compilation compilation, SyntaxNode node, IPropertySymbol symbol, AttributeData attribute, AnalyzerConfigOptions options)
protected override (string? GeneratedCode, DiagnosticDetail? Error) GenerateCode(Compilation compilation, SyntaxNode node, IPropertySymbol symbol, AttributeData attribute, AnalyzerConfigOptions options)
{
var model = new GodotNotifyDataModel(symbol, node);
Log.Debug($"--- MODEL ---\n{model}\n");
Expand Down
8 changes: 5 additions & 3 deletions SourceGenerators/GodotOverrideExtensions/GodotOverrideSourceGenerator.cs
View file
Open in desktop
Original file line number Diff line number Diff line change
@@ -1,4 +1,5 @@
using Microsoft.CodeAnalysis;
using System.Diagnostics.CodeAnalysis;
using Microsoft.CodeAnalysis;
using Microsoft.CodeAnalysis.Diagnostics;
using Scriban;

Expand All @@ -7,9 +8,10 @@ namespace GodotSharp.SourceGenerators.GodotOverrideExtensions;
[Generator]
internal class GodotOverrideSourceGenerator : SourceGeneratorForDeclaredMethodWithAttribute<Godot.GodotOverrideAttribute>
{
[field: MaybeNull]
private static Template GodotOverrideTemplate => field ??= Template.Parse(Resources.GodotOverrideTemplate);

protected override (string GeneratedCode, DiagnosticDetail Error) GenerateCode(Compilation compilation, SyntaxNode node, IMethodSymbol symbol, AttributeData attribute, AnalyzerConfigOptions options)
protected override (string? GeneratedCode, DiagnosticDetail? Error) GenerateCode(Compilation compilation, SyntaxNode node, IMethodSymbol symbol, AttributeData attribute, AnalyzerConfigOptions options)
{
var model = new GodotOverrideDataModel(symbol, ReconstructAttribute().Replace);
Log.Debug($"--- MODEL ---\n{model}\n");
Expand All @@ -20,6 +22,6 @@ protected override (string GeneratedCode, DiagnosticDetail Error) GenerateCode(C
return (output, null);

Godot.GodotOverrideAttribute ReconstructAttribute()
=> new((bool)attribute.ConstructorArguments[0].Value);
=> new((bool)attribute.ConstructorArguments[0].Value!);
Copy link

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

🛠️ Refactor suggestion

Potentially unsafe cast with null-forgiving operator.

Similar to the previous file, using (bool)attribute.ConstructorArguments[0].Value! assumes the constructor argument is never null.

Consider safer casting:

-=> new((bool)attribute.ConstructorArguments[0].Value!);
+=> new(attribute.ConstructorArguments[0].Value as bool? ?? throw new InvalidOperationException("GodotOverride attribute requires a boolean argument"));
📝 Committable suggestion

‼️ IMPORTANT
Carefully review the code before committing. Ensure that it accurately replaces the highlighted code, contains no missing lines, and has no issues with indentation. Thoroughly test & benchmark the code to ensure it meets the requirements.

Suggested change
=> new((bool)attribute.ConstructorArguments[0].Value!);
=> new(attribute.ConstructorArguments[0].Value as bool?
?? throw new InvalidOperationException("GodotOverride attribute requires a boolean argument"));
🤖 Prompt for AI Agents 
In SourceGenerators/GodotOverrideExtensions/GodotOverrideSourceGenerator.cs at
line 25, the cast (bool)attribute.ConstructorArguments[0].Value! uses the
null-forgiving operator which assumes the value is never null and can cause
runtime exceptions. To fix this, replace the cast with a safer approach such as
checking if the value is null before casting or using a nullable bool and
handling the null case appropriately to avoid invalid casts.

}
}
2 changes: 1 addition & 1 deletion SourceGenerators/InputMapExtensions/InputMapAttribute.cs
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -5,7 +5,7 @@ namespace Godot;
[AttributeUsage(AttributeTargets.Class)]
public sealed class InputMapAttribute : Attribute
{
public InputMapAttribute([CallerFilePath] string classPath = null)
public InputMapAttribute([CallerFilePath] string classPath = null!)
Copy link

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

🛠️ Refactor suggestion

Reconsider using null-forgiving operator as default value.

Using null! as a default value for a non-nullable parameter is an anti-pattern that undermines the benefits of nullable reference types. While [CallerFilePath] typically provides a value, this approach declares the parameter as non-nullable while providing a null default.

Consider reverting to the nullable approach:

-public InputMapAttribute([CallerFilePath] string classPath = null!)
+public InputMapAttribute([CallerFilePath] string? classPath = null)

Or if you want to enforce non-null, use a proper default:

-public InputMapAttribute([CallerFilePath] string classPath = null!)
+public InputMapAttribute([CallerFilePath] string classPath = "")
📝 Committable suggestion

‼️ IMPORTANT
Carefully review the code before committing. Ensure that it accurately replaces the highlighted code, contains no missing lines, and has no issues with indentation. Thoroughly test & benchmark the code to ensure it meets the requirements.

Suggested change
public InputMapAttribute([CallerFilePath] string classPath = null!)
public InputMapAttribute([CallerFilePath] string? classPath = null)
🤖 Prompt for AI Agents 
In SourceGenerators/InputMapExtensions/InputMapAttribute.cs at line 8, avoid
using the null-forgiving operator 'null!' as the default value for the
non-nullable parameter classPath. Instead, change the parameter type to a
nullable string and set the default to null, or provide a meaningful non-null
default value. This aligns with nullable reference type best practices and
ensures the parameter's nullability is correctly represented.

=> ClassPath = classPath;

public string ClassPath { get; }
Expand Down
2 changes: 1 addition & 1 deletion SourceGenerators/InputMapExtensions/InputMapDataModel.cs
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -9,7 +9,7 @@ internal class InputMapDataModel : ClassDataModel
public IList<InputAction> Actions { get; }
public ILookup<string, InputAction> NestedActions { get; }

public InputMapDataModel(INamedTypeSymbol symbol, string csPath, string gdRoot) : base(symbol)
public InputMapDataModel(INamedTypeSymbol symbol, string csPath, string? gdRoot) : base(symbol)
{
var actions = InputMapScraper
.GetInputActions(csPath, gdRoot)
Expand Down
7 changes: 4 additions & 3 deletions SourceGenerators/InputMapExtensions/InputMapScraper.cs
View file
Open in desktop
Original file line number Diff line number Diff line change
@@ -1,4 +1,5 @@
using System.Text.RegularExpressions;
using System.Diagnostics.CodeAnalysis;
using System.Text.RegularExpressions;

namespace GodotSharp.SourceGenerators.InputMapExtensions;

Expand All @@ -7,7 +8,7 @@ internal static class InputMapScraper
private const string InputRegexStr = @"^""?(?<Input>.+?)""?=.*$";
private static readonly Regex InputRegex = new(InputRegexStr, RegexOptions.Compiled | RegexOptions.ExplicitCapture);

public static IEnumerable<string> GetInputActions(string csFile, string gdRoot)
public static IEnumerable<string> GetInputActions(string csFile, string? gdRoot)
{
var gdFile = GD.GetProjectFile(csFile, gdRoot);
Log.Debug($"Scraping {gdFile} [Compiling {csFile}]");
Expand Down Expand Up @@ -36,7 +37,7 @@ static IEnumerable<string> MatchInputActions(string gdFile)
}
}

static bool TryMatchInput(string line, out string action)
static bool TryMatchInput(string line, [NotNullWhen(true)] out string? action)
{
var match = InputRegex.Match(line);
if (match.Success)
Expand Down
8 changes: 5 additions & 3 deletions SourceGenerators/InputMapExtensions/InputMapSourceGenerator.cs
View file
Open in desktop
Original file line number Diff line number Diff line change
@@ -1,4 +1,5 @@
using Microsoft.CodeAnalysis;
using System.Diagnostics.CodeAnalysis;
using Microsoft.CodeAnalysis;
using Microsoft.CodeAnalysis.Diagnostics;
using Scriban;

Expand All @@ -7,9 +8,10 @@ namespace GodotSharp.SourceGenerators.InputMapExtensions;
[Generator]
internal class InputMapSourceGenerator : SourceGeneratorForDeclaredTypeWithAttribute<Godot.InputMapAttribute>
{
[field: MaybeNull]
private static Template InputMapTemplate => field ??= Template.Parse(Resources.InputMapTemplate);

protected override (string GeneratedCode, DiagnosticDetail Error) GenerateCode(Compilation compilation, SyntaxNode node, INamedTypeSymbol symbol, AttributeData attribute, AnalyzerConfigOptions options)
protected override (string? GeneratedCode, DiagnosticDetail? Error) GenerateCode(Compilation compilation, SyntaxNode node, INamedTypeSymbol symbol, AttributeData attribute, AnalyzerConfigOptions options)
{
var model = new InputMapDataModel(symbol, ReconstructAttribute().ClassPath, options.TryGetGodotProjectDir());
Log.Debug($"--- MODEL ---\n{model}\n");
Expand All @@ -20,6 +22,6 @@ protected override (string GeneratedCode, DiagnosticDetail Error) GenerateCode(C
return (output, null);

Godot.InputMapAttribute ReconstructAttribute()
=> new((string)attribute.ConstructorArguments[0].Value);
=> new((string)attribute.ConstructorArguments[0].Value!);
}
}
2 changes: 1 addition & 1 deletion SourceGenerators/LayerNamesExtensions/LayerNamesAttribute.cs
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -5,7 +5,7 @@ namespace Godot;
[AttributeUsage(AttributeTargets.Class)]
public sealed class LayerNamesAttribute : Attribute
{
public LayerNamesAttribute([CallerFilePath] string classPath = null)
public LayerNamesAttribute([CallerFilePath] string classPath = null!)
Copy link

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

🛠️ Refactor suggestion

Same issue as InputMapAttribute - avoid null-forgiving operator as default.

This has the same problematic pattern as InputMapAttribute where a non-nullable parameter uses null! as the default value, which is an anti-pattern for nullable reference types.

Apply the same fix as suggested for InputMapAttribute:

-public LayerNamesAttribute([CallerFilePath] string classPath = null!)
+public LayerNamesAttribute([CallerFilePath] string? classPath = null)

Or use a proper non-null default if you want to enforce non-nullability.

📝 Committable suggestion

‼️ IMPORTANT
Carefully review the code before committing. Ensure that it accurately replaces the highlighted code, contains no missing lines, and has no issues with indentation. Thoroughly test & benchmark the code to ensure it meets the requirements.

Suggested change
public LayerNamesAttribute([CallerFilePath] string classPath = null!)
public LayerNamesAttribute([CallerFilePath] string? classPath = null)
🤖 Prompt for AI Agents 
In SourceGenerators/LayerNamesExtensions/LayerNamesAttribute.cs at line 8, the
constructor parameter classPath is declared as non-nullable but uses the
null-forgiving operator with a default null value, which is an anti-pattern. To
fix this, change the parameter to be nullable by adding a question mark (string?
classPath) and handle the null case inside the constructor, or provide a proper
non-null default value instead of null!. This will align with nullable reference
type best practices and avoid misleading nullability annotations.

💡 Verification agent

🧩 Analysis chain

Consistent but semantically confusing pattern with null! default.

This follows the same pattern as other attribute classes in the PR, but maintains the semantic inconsistency of declaring a non-nullable parameter while defaulting to null!. While [CallerFilePath] should provide a non-null value at compile time, the pattern creates confusion.

Consider establishing a consistent approach across all attribute classes - either maintain nullable parameters or use non-null defaults like empty strings.

Let me verify this pattern across other attribute classes:

🏁 Script executed:

#!/bin/bash
# Description: Check the pattern usage across all attribute classes in the codebase
# Expected: Find similar patterns in other attribute constructors

rg -A 2 -B 2 "\[CallerFilePath\].*classPath.*null!" --type cs

Length of output: 1458

Standardize CallerFilePath Parameter Defaults Across Attribute Constructors

All three attribute constructors in this PR use the pattern:

public XAttribute([CallerFilePath] string classPath = null!)

– declaring a non‐nullable parameter with a null! default. While the compiler will supply a non‐null file path at call sites, defaulting to null! is semantically confusing and bypasses null safety checks.

Consider one of these two consistent approaches for all attribute classes:

  • Default to an empty string and drop the null‐forgiving operator: 
    

    • 

  • public XAttribute([CallerFilePath] string classPath = null!)
    • 



    

  • public XAttribute([CallerFilePath] string classPath = "")

    

    • 



    

  • Or mark the parameter as nullable and keep null as the default:

    

    • 

  • public XAttribute([CallerFilePath] string classPath = null!)
    • 



    

  • public XAttribute([CallerFilePath] string? classPath = null)

    

    

    • 



Affected files:


    

  • SourceGenerators/InputMapExtensions/InputMapAttribute.cs
    • 

  • SourceGenerators/LayerNamesExtensions/LayerNamesAttribute.cs
    • 

  • SourceGenerators/SceneTreeExtensions/SceneTreeAttribute.cs
    • 




🤖 Prompt for AI Agents

In SourceGenerators/LayerNamesExtensions/LayerNamesAttribute.cs at line 8, the
constructor parameter classPath is declared as non-nullable but defaults to
null! which is semantically confusing and bypasses null safety. To fix this,
standardize the parameter by either making classPath nullable and defaulting to
null, or keep it non-nullable and default it to an empty string without the
null-forgiving operator. Apply the same consistent approach across all attribute
classes in the PR.





    

  
  




    


        

      
  
  

    
  
    
    

  

  

  



          

      
    








    


  









  


    


  		



    

    

    

  
            => ClassPath = classPath;






    

    

    

  
    







    

    

    

  
        public string ClassPath { get; }






  

    
          
            
          
          Expand Down
    
    
  



                
              


          


    

  



    
  

    

      

        


          

            
            
          


          



          
            2 changes: 1 addition & 1 deletion
          
          

        

  SourceGenerators/LayerNamesExtensions/LayerNamesDataModel.cs

  
  
      
      
  





        
      


      

        











            


          

          

            
              

                
    

              

            
            
                            

                            


                              
   View file



                                    


                                  


                            


                            

                            

    Open in desktop



                
            
          

        

      

    



    



          

            




              
      
    
              

                
                  

                      Original file line number
                      Diff line number
                      Diff line change
                  

                
                
                      
      

    
          
            
          
          Expand Up
    
    @@ -7,7 +7,7 @@ internal class LayerNamesDataModel : ClassDataModel
  


    

    

    

  
    {






    

    

    

  
        public ILookup<string, LayerNameValue> Layers { get; }






    

    

    

  
    







    

    

    

  
        public LayerNamesDataModel(INamedTypeSymbol symbol, string csPath, string gdRoot) : base(symbol)






    

    

    

  
        public LayerNamesDataModel(INamedTypeSymbol symbol, string csPath, string? gdRoot) : base(symbol)






    

    

    

  
        {






    

    

    

  
            Layers = LayerNamesScraper






    

    

    

  
                .GetLayerNames(csPath, gdRoot)






  

    
          
            
          
          Expand Down
    
    
  



                
              


          


    

  



  


    

  
  
    
      Loading