Merge pull request #102836 from raulsntos/dotnet/export-tool-button-no-storage

[.NET] Disallow `[ExportToolButton]` on members thay may store the Callable
This commit is contained in:
Rémi Verschelde 2025-02-14 22:58:25 +01:00
commit 7a0e659a80
No known key found for this signature in database
GPG Key ID: C3336907360768E1
14 changed files with 263 additions and 38 deletions

View File

@ -101,4 +101,13 @@ public class ExportDiagnosticsTests
new string[] { "ExportDiagnostics_GD0110_ScriptProperties.generated.cs" }
);
}
[Fact]
public async void ExportToolButtonStoringCallable()
{
await CSharpSourceGeneratorVerifier<ScriptPropertiesGenerator>.Verify(
new string[] { "ExportDiagnostics_GD0111.cs" },
new string[] { "ExportDiagnostics_GD0111_ScriptProperties.generated.cs" }
);
}
}

View File

@ -9,22 +9,12 @@ partial class ExportDiagnostics_GD0108
/// </summary>
public new class PropertyName : global::Godot.Node.PropertyName {
/// <summary>
/// Cached name for the 'MyButton' field.
/// Cached name for the 'MyButton' property.
/// </summary>
public new static readonly global::Godot.StringName @MyButton = "MyButton";
}
/// <inheritdoc/>
[global::System.ComponentModel.EditorBrowsable(global::System.ComponentModel.EditorBrowsableState.Never)]
protected override bool SetGodotClassPropertyValue(in godot_string_name name, in godot_variant value)
{
if (name == PropertyName.@MyButton) {
this.@MyButton = global::Godot.NativeInterop.VariantUtils.ConvertTo<global::Godot.Callable>(value);
return true;
}
return base.SetGodotClassPropertyValue(name, value);
}
/// <inheritdoc/>
[global::System.ComponentModel.EditorBrowsable(global::System.ComponentModel.EditorBrowsableState.Never)]
protected override bool GetGodotClassPropertyValue(in godot_string_name name, out godot_variant value)
{
if (name == PropertyName.@MyButton) {

View File

@ -9,22 +9,12 @@ partial class ExportDiagnostics_GD0109
/// </summary>
public new class PropertyName : global::Godot.Node.PropertyName {
/// <summary>
/// Cached name for the 'MyButton' field.
/// Cached name for the 'MyButton' property.
/// </summary>
public new static readonly global::Godot.StringName @MyButton = "MyButton";
}
/// <inheritdoc/>
[global::System.ComponentModel.EditorBrowsable(global::System.ComponentModel.EditorBrowsableState.Never)]
protected override bool SetGodotClassPropertyValue(in godot_string_name name, in godot_variant value)
{
if (name == PropertyName.@MyButton) {
this.@MyButton = global::Godot.NativeInterop.VariantUtils.ConvertTo<global::Godot.Callable>(value);
return true;
}
return base.SetGodotClassPropertyValue(name, value);
}
/// <inheritdoc/>
[global::System.ComponentModel.EditorBrowsable(global::System.ComponentModel.EditorBrowsableState.Never)]
protected override bool GetGodotClassPropertyValue(in godot_string_name name, out godot_variant value)
{
if (name == PropertyName.@MyButton) {

View File

@ -9,26 +9,16 @@ partial class ExportDiagnostics_GD0110
/// </summary>
public new class PropertyName : global::Godot.Node.PropertyName {
/// <summary>
/// Cached name for the 'MyButton' field.
/// Cached name for the 'MyButton' property.
/// </summary>
public new static readonly global::Godot.StringName @MyButton = "MyButton";
}
/// <inheritdoc/>
[global::System.ComponentModel.EditorBrowsable(global::System.ComponentModel.EditorBrowsableState.Never)]
protected override bool SetGodotClassPropertyValue(in godot_string_name name, in godot_variant value)
{
if (name == PropertyName.@MyButton) {
this.@MyButton = global::Godot.NativeInterop.VariantUtils.ConvertTo<string>(value);
return true;
}
return base.SetGodotClassPropertyValue(name, value);
}
/// <inheritdoc/>
[global::System.ComponentModel.EditorBrowsable(global::System.ComponentModel.EditorBrowsableState.Never)]
protected override bool GetGodotClassPropertyValue(in godot_string_name name, out godot_variant value)
{
if (name == PropertyName.@MyButton) {
value = global::Godot.NativeInterop.VariantUtils.CreateFrom<string>(this.@MyButton);
value = global::Godot.NativeInterop.VariantUtils.CreateFrom<int>(this.@MyButton);
return true;
}
return base.GetGodotClassPropertyValue(name, out value);

View File

@ -0,0 +1,116 @@
using Godot;
using Godot.NativeInterop;
partial class ExportDiagnostics_GD0111
{
#pragma warning disable CS0109 // Disable warning about redundant 'new' keyword
/// <summary>
/// Cached StringNames for the properties and fields contained in this class, for fast lookup.
/// </summary>
public new class PropertyName : global::Godot.Node.PropertyName {
/// <summary>
/// Cached name for the 'MyButtonGet' property.
/// </summary>
public new static readonly global::Godot.StringName @MyButtonGet = "MyButtonGet";
/// <summary>
/// Cached name for the 'MyButtonGetSet' property.
/// </summary>
public new static readonly global::Godot.StringName @MyButtonGetSet = "MyButtonGetSet";
/// <summary>
/// Cached name for the 'MyButtonGetWithBackingField' property.
/// </summary>
public new static readonly global::Godot.StringName @MyButtonGetWithBackingField = "MyButtonGetWithBackingField";
/// <summary>
/// Cached name for the 'MyButtonGetSetWithBackingField' property.
/// </summary>
public new static readonly global::Godot.StringName @MyButtonGetSetWithBackingField = "MyButtonGetSetWithBackingField";
/// <summary>
/// Cached name for the 'MyButtonOkWithCallableCreationExpression' property.
/// </summary>
public new static readonly global::Godot.StringName @MyButtonOkWithCallableCreationExpression = "MyButtonOkWithCallableCreationExpression";
/// <summary>
/// Cached name for the 'MyButtonOkWithImplicitCallableCreationExpression' property.
/// </summary>
public new static readonly global::Godot.StringName @MyButtonOkWithImplicitCallableCreationExpression = "MyButtonOkWithImplicitCallableCreationExpression";
/// <summary>
/// Cached name for the 'MyButtonOkWithCallableFromExpression' property.
/// </summary>
public new static readonly global::Godot.StringName @MyButtonOkWithCallableFromExpression = "MyButtonOkWithCallableFromExpression";
/// <summary>
/// Cached name for the '_backingField' field.
/// </summary>
public new static readonly global::Godot.StringName @_backingField = "_backingField";
}
/// <inheritdoc/>
[global::System.ComponentModel.EditorBrowsable(global::System.ComponentModel.EditorBrowsableState.Never)]
protected override bool SetGodotClassPropertyValue(in godot_string_name name, in godot_variant value)
{
if (name == PropertyName.@MyButtonGetSet) {
this.@MyButtonGetSet = global::Godot.NativeInterop.VariantUtils.ConvertTo<global::Godot.Callable>(value);
return true;
}
if (name == PropertyName.@MyButtonGetSetWithBackingField) {
this.@MyButtonGetSetWithBackingField = global::Godot.NativeInterop.VariantUtils.ConvertTo<global::Godot.Callable>(value);
return true;
}
if (name == PropertyName.@_backingField) {
this.@_backingField = global::Godot.NativeInterop.VariantUtils.ConvertTo<global::Godot.Callable>(value);
return true;
}
return base.SetGodotClassPropertyValue(name, value);
}
/// <inheritdoc/>
[global::System.ComponentModel.EditorBrowsable(global::System.ComponentModel.EditorBrowsableState.Never)]
protected override bool GetGodotClassPropertyValue(in godot_string_name name, out godot_variant value)
{
if (name == PropertyName.@MyButtonGet) {
value = global::Godot.NativeInterop.VariantUtils.CreateFrom<global::Godot.Callable>(this.@MyButtonGet);
return true;
}
if (name == PropertyName.@MyButtonGetSet) {
value = global::Godot.NativeInterop.VariantUtils.CreateFrom<global::Godot.Callable>(this.@MyButtonGetSet);
return true;
}
if (name == PropertyName.@MyButtonGetWithBackingField) {
value = global::Godot.NativeInterop.VariantUtils.CreateFrom<global::Godot.Callable>(this.@MyButtonGetWithBackingField);
return true;
}
if (name == PropertyName.@MyButtonGetSetWithBackingField) {
value = global::Godot.NativeInterop.VariantUtils.CreateFrom<global::Godot.Callable>(this.@MyButtonGetSetWithBackingField);
return true;
}
if (name == PropertyName.@MyButtonOkWithCallableCreationExpression) {
value = global::Godot.NativeInterop.VariantUtils.CreateFrom<global::Godot.Callable>(this.@MyButtonOkWithCallableCreationExpression);
return true;
}
if (name == PropertyName.@MyButtonOkWithImplicitCallableCreationExpression) {
value = global::Godot.NativeInterop.VariantUtils.CreateFrom<global::Godot.Callable>(this.@MyButtonOkWithImplicitCallableCreationExpression);
return true;
}
if (name == PropertyName.@MyButtonOkWithCallableFromExpression) {
value = global::Godot.NativeInterop.VariantUtils.CreateFrom<global::Godot.Callable>(this.@MyButtonOkWithCallableFromExpression);
return true;
}
if (name == PropertyName.@_backingField) {
value = global::Godot.NativeInterop.VariantUtils.CreateFrom<global::Godot.Callable>(this.@_backingField);
return true;
}
return base.GetGodotClassPropertyValue(name, out value);
}
/// <summary>
/// Get the property information for all the properties declared in this class.
/// This method is used by Godot to register the available properties in the editor.
/// Do not call this method.
/// </summary>
[global::System.ComponentModel.EditorBrowsable(global::System.ComponentModel.EditorBrowsableState.Never)]
internal new static global::System.Collections.Generic.List<global::Godot.Bridge.PropertyInfo> GetGodotPropertyList()
{
var properties = new global::System.Collections.Generic.List<global::Godot.Bridge.PropertyInfo>();
properties.Add(new(type: (global::Godot.Variant.Type)25, name: PropertyName.@_backingField, hint: (global::Godot.PropertyHint)0, hintString: "", usage: (global::Godot.PropertyUsageFlags)4096, exported: false));
properties.Add(new(type: (global::Godot.Variant.Type)25, name: PropertyName.@MyButtonOkWithCallableCreationExpression, hint: (global::Godot.PropertyHint)39, hintString: "", usage: (global::Godot.PropertyUsageFlags)4, exported: true));
properties.Add(new(type: (global::Godot.Variant.Type)25, name: PropertyName.@MyButtonOkWithImplicitCallableCreationExpression, hint: (global::Godot.PropertyHint)39, hintString: "", usage: (global::Godot.PropertyUsageFlags)4, exported: true));
properties.Add(new(type: (global::Godot.Variant.Type)25, name: PropertyName.@MyButtonOkWithCallableFromExpression, hint: (global::Godot.PropertyHint)39, hintString: "", usage: (global::Godot.PropertyUsageFlags)4, exported: true));
return properties;
}
#pragma warning restore CS0109
}

View File

@ -4,5 +4,5 @@ using Godot.Collections;
public partial class ExportDiagnostics_GD0108 : Node
{
[ExportToolButton("")]
public Callable {|GD0108:MyButton|};
public Callable {|GD0108:MyButton|} => new Callable();
}

View File

@ -5,5 +5,5 @@ using Godot.Collections;
public partial class ExportDiagnostics_GD0109 : Node
{
[Export, ExportToolButton("")]
public Callable {|GD0109:MyButton|};
public Callable {|GD0109:MyButton|} => new Callable();
}

View File

@ -5,5 +5,5 @@ using Godot.Collections;
public partial class ExportDiagnostics_GD0110 : Node
{
[ExportToolButton("")]
public string {|GD0110:MyButton|};
public int {|GD0110:MyButton|} => new();
}

View File

@ -0,0 +1,29 @@
using Godot;
using Godot.Collections;
[Tool]
public partial class ExportDiagnostics_GD0111 : Node
{
private Callable _backingField;
[ExportToolButton("")]
public Callable {|GD0111:MyButtonGet|} { get; }
[ExportToolButton("")]
public Callable {|GD0111:MyButtonGetSet|} { get; set; }
[ExportToolButton("")]
public Callable {|GD0111:MyButtonGetWithBackingField|} { get => _backingField; }
[ExportToolButton("")]
public Callable {|GD0111:MyButtonGetSetWithBackingField|} { get => _backingField; set => _backingField = value; }
[ExportToolButton("")]
public Callable MyButtonOkWithCallableCreationExpression => new Callable(this, "");
[ExportToolButton("")]
public Callable MyButtonOkWithImplicitCallableCreationExpression => new(this, "");
[ExportToolButton("")]
public Callable MyButtonOkWithCallableFromExpression => Callable.From(null);
}

View File

@ -6,3 +6,4 @@ GD0003 | Usage | Error | ScriptPathAttributeGenerator, [Documentation](ht
GD0108 | Usage | Error | ScriptPropertiesGenerator, [Documentation](https://docs.godotengine.org/en/latest/tutorials/scripting/c_sharp/diagnostics/GD0108.html)
GD0109 | Usage | Error | ScriptPropertiesGenerator, [Documentation](https://docs.godotengine.org/en/latest/tutorials/scripting/c_sharp/diagnostics/GD0109.html)
GD0110 | Usage | Error | ScriptPropertiesGenerator, [Documentation](https://docs.godotengine.org/en/latest/tutorials/scripting/c_sharp/diagnostics/GD0110.html)
GD0111 | Usage | Error | ScriptPropertiesGenerator, [Documentation](https://docs.godotengine.org/en/latest/tutorials/scripting/c_sharp/diagnostics/GD0111.html)

View File

@ -137,6 +137,16 @@ namespace Godot.SourceGenerators
"The exported tool button is not a Callable. The '[ExportToolButton]' attribute is only supported on members of type Callable.",
helpLinkUri: string.Format(_helpLinkFormat, "GD0110"));
public static readonly DiagnosticDescriptor ExportToolButtonMustBeExpressionBodiedProperty =
new DiagnosticDescriptor(id: "GD0111",
title: "The exported tool button must be an expression-bodied property",
messageFormat: "The exported tool button '{0}' must be an expression-bodied property",
category: "Usage",
DiagnosticSeverity.Error,
isEnabledByDefault: true,
"The exported tool button must be an expression-bodied property. The '[ExportToolButton]' attribute is only supported on expression-bodied properties with a 'new Callable(...)' or 'Callable.From(...)' expression.",
helpLinkUri: string.Format(_helpLinkFormat, "GD0111"));
public static readonly DiagnosticDescriptor SignalDelegateMissingSuffixRule =
new DiagnosticDescriptor(id: "GD0201",
title: "The name of the delegate must end with 'EventHandler'",

View File

@ -4,6 +4,7 @@ namespace Godot.SourceGenerators
{
public const string GodotObject = "Godot.GodotObject";
public const string Node = "Godot.Node";
public const string Callable = "Godot.Callable";
public const string AssemblyHasScriptsAttr = "Godot.AssemblyHasScriptsAttribute";
public const string ExportAttr = "Godot.ExportAttribute";
public const string ExportCategoryAttr = "Godot.ExportCategoryAttribute";

View File

@ -3,6 +3,7 @@ using System.Diagnostics;
using System.Linq;
using System.Text;
using Microsoft.CodeAnalysis;
using Microsoft.CodeAnalysis.CSharp;
using Microsoft.CodeAnalysis.CSharp.Syntax;
using Microsoft.CodeAnalysis.Text;
@ -465,6 +466,94 @@ namespace Godot.SourceGenerators
return null;
}
if (exportToolButtonAttr != null && propertySymbol != null)
{
if (!PropertyIsExpressionBodiedAndReturnsNewCallable(context.Compilation, propertySymbol))
{
context.ReportDiagnostic(Diagnostic.Create(
Common.ExportToolButtonMustBeExpressionBodiedProperty,
propertySymbol.Locations.FirstLocationWithSourceTreeOrDefault(),
propertySymbol.ToDisplayString()
));
return null;
}
static bool PropertyIsExpressionBodiedAndReturnsNewCallable(Compilation compilation, IPropertySymbol? propertySymbol)
{
if (propertySymbol == null)
{
return false;
}
var propertyDeclarationSyntax = propertySymbol.DeclaringSyntaxReferences
.Select(r => r.GetSyntax() as PropertyDeclarationSyntax).FirstOrDefault();
if (propertyDeclarationSyntax == null || propertyDeclarationSyntax.Initializer != null)
{
return false;
}
if (propertyDeclarationSyntax.AccessorList != null)
{
var accessors = propertyDeclarationSyntax.AccessorList.Accessors;
foreach (var accessor in accessors)
{
if (!accessor.IsKind(SyntaxKind.GetAccessorDeclaration))
{
// Only getters are allowed.
return false;
}
if (!ExpressionBodyReturnsNewCallable(compilation, accessor.ExpressionBody))
{
return false;
}
}
}
else if (!ExpressionBodyReturnsNewCallable(compilation, propertyDeclarationSyntax.ExpressionBody))
{
return false;
}
return true;
}
static bool ExpressionBodyReturnsNewCallable(Compilation compilation, ArrowExpressionClauseSyntax? expressionSyntax)
{
if (expressionSyntax == null)
{
return false;
}
var semanticModel = compilation.GetSemanticModel(expressionSyntax.SyntaxTree);
switch (expressionSyntax.Expression)
{
case ImplicitObjectCreationExpressionSyntax creationExpression:
// We already validate that the property type must be 'Callable'
// so we can assume this constructor is valid.
return true;
case ObjectCreationExpressionSyntax creationExpression:
var typeSymbol = semanticModel.GetSymbolInfo(creationExpression.Type).Symbol as ITypeSymbol;
if (typeSymbol != null)
{
return typeSymbol.FullQualifiedNameOmitGlobal() == GodotClasses.Callable;
}
break;
case InvocationExpressionSyntax invocationExpression:
var methodSymbol = semanticModel.GetSymbolInfo(invocationExpression).Symbol as IMethodSymbol;
if (methodSymbol != null && methodSymbol.Name == "From")
{
return methodSymbol.ContainingType.FullQualifiedNameOmitGlobal() == GodotClasses.Callable;
}
break;
}
return false;
}
}
var memberType = propertySymbol?.Type ?? fieldSymbol!.Type;
var memberVariantType = MarshalUtils.ConvertMarshalTypeToVariantType(marshalType)!.Value;

View File

@ -7,7 +7,7 @@ namespace Godot
/// <summary>
/// Exports the annotated <see cref="Callable"/> as a clickable button.
/// </summary>
[AttributeUsage(AttributeTargets.Field | AttributeTargets.Property)]
[AttributeUsage(AttributeTargets.Property)]
public sealed class ExportToolButtonAttribute : Attribute
{
/// <summary>