-
Notifications
You must be signed in to change notification settings - Fork 1.9k
[C] Fix NullReferenceException when binding to Dictionary<Enum, object> with x:DataType #32916
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.
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
base: main
Are you sure you want to change the base?
Changes from all commits
File filter
Filter by extension
Conversations
Jump to
Diff view
Diff view
There are no files selected for viewing
| Original file line number | Diff line number | Diff line change |
|---|---|---|
| @@ -0,0 +1,87 @@ | ||
| using System.Linq; | ||
| using Microsoft.CodeAnalysis; | ||
| using Microsoft.CodeAnalysis.CSharp; | ||
| using Microsoft.Maui.Controls.SourceGen; | ||
| using Xunit; | ||
|
|
||
| using static Microsoft.Maui.Controls.Xaml.UnitTests.SourceGen.SourceGeneratorDriver; | ||
|
|
||
| namespace Microsoft.Maui.Controls.Xaml.UnitTests.SourceGen; | ||
|
|
||
| public class Maui13856Tests : SourceGenTestsBase | ||
| { | ||
| private record AdditionalXamlFile(string Path, string Content, string? RelativePath = null, string? TargetPath = null, string? ManifestResourceName = null, string? TargetFramework = null, string? NoWarn = null) | ||
| : AdditionalFile(Text: SourceGeneratorDriver.ToAdditionalText(Path, Content), Kind: "Xaml", RelativePath: RelativePath ?? Path, TargetPath: TargetPath, ManifestResourceName: ManifestResourceName, TargetFramework: TargetFramework, NoWarn: NoWarn); | ||
|
|
||
| [Fact] | ||
| public void DictionaryWithEnumKeyBindingDoesNotCauseErrors() | ||
| { | ||
| // https://github.com/dotnet/maui/issues/13856 | ||
| // Binding to Dictionary<CustomEnum, object> with x:DataType should not cause generator errors | ||
| // Note: SourceGen currently falls back to runtime binding for dictionary indexers (both string and enum keys) | ||
| // This test verifies that enum key bindings don't cause errors in the generator | ||
|
|
||
| var codeBehind = | ||
| """ | ||
| using System.Collections.Generic; | ||
| using Microsoft.Maui.Controls; | ||
| namespace Test | ||
| { | ||
| public enum UserSetting | ||
| { | ||
| BrowserInvisible, | ||
| GlobalWaitForElementsInBrowserInSek, | ||
| TBD, | ||
| } | ||
| public partial class TestPage : ContentPage | ||
| { | ||
| public TestPage() | ||
| { | ||
| UserSettings = new Dictionary<UserSetting, object> | ||
| { | ||
| { UserSetting.TBD, "test value" } | ||
| }; | ||
| InitializeComponent(); | ||
| } | ||
| public Dictionary<UserSetting, object> UserSettings { get; set; } | ||
| } | ||
| } | ||
| """; | ||
|
|
||
| var xaml = | ||
| """ | ||
| <?xml version="1.0" encoding="UTF-8"?> | ||
| <ContentPage | ||
| xmlns="http://schemas.microsoft.com/dotnet/2021/maui" | ||
| xmlns:x="http://schemas.microsoft.com/winfx/2009/xaml" | ||
| xmlns:local="clr-namespace:Test" | ||
| x:Class="Test.TestPage" | ||
| x:DataType="local:TestPage"> | ||
| <Entry x:Name="entry" Text="{Binding UserSettings[TBD]}" /> | ||
| </ContentPage> | ||
| """; | ||
|
|
||
| var compilation = CreateMauiCompilation(); | ||
| compilation = compilation.AddSyntaxTrees(CSharpSyntaxTree.ParseText(codeBehind)); | ||
|
|
||
| var result = RunGenerator<XamlGenerator>(compilation, new AdditionalXamlFile("Test.xaml", xaml)); | ||
|
|
||
| // The generator should not produce any errors | ||
| var errors = result.Diagnostics.Where(d => d.Severity == DiagnosticSeverity.Error).ToList(); | ||
| Assert.Empty(errors); | ||
|
|
||
| // Verify that a source file was generated | ||
| var generatedSource = result.Results | ||
| .SelectMany(r => r.GeneratedSources) | ||
| .FirstOrDefault(s => s.HintName.Contains("xsg.cs", System.StringComparison.Ordinal)); | ||
|
|
||
| Assert.True(generatedSource.SourceText != null, "Expected generated source file with xsg.cs extension"); | ||
| var generatedCode = generatedSource.SourceText.ToString(); | ||
|
|
||
| // Verify the binding path is in the generated code (even if using runtime binding fallback) | ||
| Assert.Contains("UserSettings[TBD]", generatedCode, System.StringComparison.Ordinal); | ||
|
Comment on lines
+84
to
+85
Member
There was a problem hiding this comment. Choose a reason for hiding this commentThe reason will be displayed to describe this comment to others. Learn more. I don't think this assert should be here. Ideally we'll drop the unnecessary markup extension instantiation at some point (#31614). I think this assert should look for
Member
There was a problem hiding this comment. Choose a reason for hiding this commentThe reason will be displayed to describe this comment to others. Learn more. (although that might currently fail due to #32905) |
||
| } | ||
| } | ||
| Original file line number | Diff line number | Diff line change |
|---|---|---|
| @@ -0,0 +1,9 @@ | ||
| <?xml version="1.0" encoding="UTF-8"?> | ||
| <ContentPage | ||
| xmlns="http://schemas.microsoft.com/dotnet/2021/maui" | ||
| xmlns:x="http://schemas.microsoft.com/winfx/2009/xaml" | ||
| xmlns:local="using:Microsoft.Maui.Controls.Xaml.UnitTests" | ||
| x:Class="Microsoft.Maui.Controls.Xaml.UnitTests.Maui13856" | ||
| x:DataType="local:Maui13856"> | ||
| <Entry x:Name="entry" Text="{Binding UserSettings[TBD]}" /> | ||
| </ContentPage> |
| Original file line number | Diff line number | Diff line change |
|---|---|---|
| @@ -0,0 +1,39 @@ | ||
| using System.Collections.Generic; | ||
| using NUnit.Framework; | ||
|
|
||
| namespace Microsoft.Maui.Controls.Xaml.UnitTests; | ||
|
|
||
| public enum Maui13856UserSetting | ||
| { | ||
| BrowserInvisible, | ||
| GlobalWaitForElementsInBrowserInSek, | ||
| TBD, | ||
| } | ||
|
|
||
| public partial class Maui13856 : ContentPage | ||
| { | ||
| public Maui13856() | ||
| { | ||
| InitializeComponent(); | ||
| } | ||
|
|
||
| public Dictionary<Maui13856UserSetting, object> UserSettings { get; set; } = new Dictionary<Maui13856UserSetting, object> | ||
| { | ||
| { Maui13856UserSetting.TBD, "test value" } | ||
| }; | ||
|
|
||
| [TestFixture] | ||
| class Tests | ||
| { | ||
| [Test] | ||
| public void DictionaryWithEnumKeyBinding([Values] XamlInflator inflator) | ||
| { | ||
| // https://github.com/dotnet/maui/issues/13856 | ||
| // Binding to Dictionary<CustomEnum, object> with x:DataType should compile and work | ||
| var page = new Maui13856(inflator); | ||
| page.BindingContext = page; | ||
|
|
||
| Assert.That(page.entry.Text, Is.EqualTo("test value")); | ||
| } | ||
| } | ||
| } |
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
When an enum indexer is found but the enum member doesn't exist (enumMember is null), the code silently continues without updating the
indexvariable. This means it will use the originalindexArgstring value instead of a fully qualified enum member reference, which could lead to incorrect code generation.Consider adding an else clause to handle the case when the enum member is not found: