diff --git a/.github/agent-pr-session/issue-33352.md b/.github/agent-pr-session/issue-33352.md new file mode 100644 index 000000000000..06ee3399d2f3 --- /dev/null +++ b/.github/agent-pr-session/issue-33352.md @@ -0,0 +1,136 @@ +# Issue #33352 - Fix Exploration Session + +**Issue:** Intermittent crash on exit on MacCatalyst - ObjectDisposedException in ShellSectionRootRenderer +**Platform:** MacCatalyst +**Test Filter:** Issue33352 +**Bug:** `TraitCollectionDidChange` is called on disposed ShellSectionRootRenderer after window scope disposed + +## Reproduction + +✅ **100% reproducible** with test: `TraitCollectionDidChangeAfterDisposeDoesNotCrash` + +**Error:** `ObjectDisposedException: Cannot access a disposed object. Object name: 'IServiceProvider'.` + +**Root Cause:** +- Window is closed, which disposes the service provider scope +- iOS/MacCatalyst then calls `TraitCollectionDidChange` on the ShellSectionRootRenderer +- The override tries to access `Context.Services.GetService()` +- The service provider has been disposed, throwing ObjectDisposedException + +**Affected Code:** +- `src/Controls/src/Core/Platform/iOS/ShellSectionRootRenderer.cs` - TraitCollectionDidChange override + +## Fix Candidates + +| # | Source | Approach | Test Result | Files Changed | Model | Notes | +|---|--------|----------|-------------|---------------|-------|-------| +| 1 | try-fix | Check `_isDisposed` before accessing services | ❌ FAIL | `ShellSectionRootRenderer.cs` (+5) | Claude Sonnet 4.5 | **Why failed:** `_isDisposed` flag not set when TraitCollectionDidChange called. Window scope disposed first, then iOS calls TraitCollectionDidChange, but renderer's Dispose() hasn't been called yet. The ServiceProvider is disposed before the renderer knows it's disposed. | +| 2 | try-fix | Store `IApplication` in `WeakReference` | ✅ PASS | `ShellSectionRootRenderer.cs` (+13) | Claude Sonnet 4.5 | **Why works:** Captures IApplication at construction time before service provider disposed. WeakReference allows GC to collect if needed. No ObjectDisposedException because we never touch the disposed service provider. | +| 3 | try-fix | Use `Application.Current` instead of service provider | ✅ PASS | `ShellSectionRootRenderer.cs` (+12) | Claude Sonnet 4.5 | **Why works:** `Application.Current` is a static property that doesn't go through the scoped service provider. Cast to `IApplication` to call `ThemeChanged()`. Simpler than WeakReference approach. **PREFERRED - simplest solution.** | +| 4 | try-fix | Try-catch `ObjectDisposedException` | ✅ PASS | `ShellSectionRootRenderer.cs` (+14) | Claude Sonnet 4.5 | **Why works:** Catches and swallows ObjectDisposedException. Theme change not critical during disposal. Works but less elegant than avoiding the exception. | +| 5 | try-fix | Remove override entirely | ❌ N/A | - | Claude Sonnet 4.5 | **Not applicable:** Method is in PublicAPI.Shipped.txt, removing it would be breaking change. Would require API removal process. | +| 6 | try-fix | Null-check `FindMauiContext()` + try-catch | ✅ PASS | `ShellSectionRootRenderer.cs` (+19) | Claude Sonnet 4.5 | **Why works:** Double protection - null check first, then try-catch. More defensive but verbose. | +| 7 | try-fix | Check if Shell's Window is null | ❌ FAIL | `ShellSectionRootRenderer.cs` (+15) | Claude Sonnet 4.5 | **Why failed:** Window property is still set when TraitCollectionDidChange called. Window.Parent disconnection happens after TraitCollectionDidChange, so checking Window is null doesn't help. | +| 8 | try-fix | Check if Window.Handler is null | ✅ PASS | `ShellSectionRootRenderer.cs` (+16) | Claude Sonnet 4.5 | **Why works:** Window.Handler is disconnected before service provider disposed. Checking `window?.Handler == null` catches the disconnection state. Good approach for detecting window closure. | +| 9 | try-fix | Check if Shell.Parent is null | ❌ FAIL | `ShellSectionRootRenderer.cs` (+15) | Claude Sonnet 4.5 | **Why failed:** Shell.Parent (Window) still set when TraitCollectionDidChange called. Shell remains attached to Window during disposal sequence. | +| 10 | try-fix | Combine `Application.Current` with Window.Handler check | ✅ PASS | `ShellSectionRootRenderer.cs` (+21) | Claude Sonnet 4.5 | **Why works:** Best of both: Window.Handler check catches disconnection early, Application.Current avoids service provider entirely. Most defensive approach. | +| 11 | try-fix | Check `Window.IsDestroyed` (internal flag) | ✅ PASS | `ShellSectionRootRenderer.cs` (+16) | Claude Sonnet 4.5 | **Why works:** `IsDestroyed` is set to true at line 540 of Window.Destroying(), BEFORE DisposeWindowScope() at line 558. Perfect timing! Checks the exact state user suggested. **EXCELLENT window-based solution.** | + +**Exhausted:** Yes (11 attempts completed) +**Selected Fix:** #3 - Use `Application.Current` - **Simplest** OR #11 - Check `Window.IsDestroyed` - **Most semantically correct** + +## Summary + +**Passing fixes (7 total):** +- ✅ #2: WeakReference +- ✅ #3: Application.Current (**SIMPLEST**) +- ✅ #4: Try-catch ObjectDisposedException +- ✅ #6: Null-check + try-catch +- ✅ #8: Check Window.Handler is null +- ✅ #10: Application.Current + Window.Handler check +- ✅ #11: Check Window.IsDestroyed (**SEMANTICALLY BEST - checks exact destroying state**) + +**Failed fixes (3 total):** +- ❌ #1: Check _isDisposed (flag not set yet) +- ❌ #7: Check Shell.Window is null (still set) +- ❌ #9: Check Shell.Parent is null (still set) + +**Not applicable (1 total):** +- ❌ #5: Remove override (breaking change) + +## Recommendation + +**Two best options:** + +1. **#3 - Application.Current** (simplest, 12 lines) + - Pros: Minimal code, no state tracking, works everywhere + - Cons: Doesn't check if window is actually closing + +2. **#11 - Window.IsDestroyed** (semantically correct, 16 lines) + - Pros: Checks the EXACT state that causes the bug, clear intent + - Cons: Slightly more code, relies on internal property (same assembly) + +User's suggestion of checking window destroying state was spot-on! + +--- + +## ACTUAL IMPLEMENTED FIX + +**Selected Fix:** Architectural improvement - Remove duplication + strengthen Core layer + +**What was implemented:** + +1. **REMOVED** `TraitCollectionDidChange` override from `ShellSectionRootRenderer` (Controls layer) + - Lines 144-151 deleted + - This was duplicate code that didn't belong in Controls + +2. **ENHANCED** `TraitCollectionDidChange` in `PageViewController` (Core layer) + - Added `window?.Handler == null` check (like attempt #8) + - Added try-catch safety net (like attempt #4) + - Uses `GetRequiredService` instead of `FindMauiContext` + - Combined approach: Window.Handler check + try-catch for race conditions + +**Why this wasn't discovered by try-fix:** + +1. **Tunnel vision** - Only looked at ShellSectionRootRenderer (where error appeared) +2. **Didn't search codebase** - Never found PageViewController also had TraitCollectionDidChange +3. **Didn't recognize duplication** - Both Core and Controls had the override +4. **Missed layer architecture** - Theme changes are CORE functionality, not Shell-specific + +**Key insight:** + +The bug existed because theme handling was **duplicated** across layers: +- Core (PageViewController) - Fundamental, applies to ALL pages +- Controls (ShellSectionRootRenderer) - Shell-specific override + +The proper fix was to **remove the Controls override** and **strengthen the Core implementation**, not patch the Controls one. + +**Files changed:** +- `src/Controls/src/Core/Compatibility/Handlers/Shell/iOS/ShellSectionRootRenderer.cs` (-10 lines) +- `src/Core/src/Platform/iOS/PageViewController.cs` (+29 lines) +- PublicAPI.Unshipped.txt (iOS/MacCatalyst) - document removal + +**Test verification:** +✅ TraitCollectionDidChangeAfterDisposeDoesNotCrash passes with new implementation + +## Test Command + +```bash +pwsh .github/scripts/BuildAndRunHostApp.ps1 -Platform catalyst -TestFilter "FullyQualifiedName~TraitCollectionDidChangeAfterDisposeDoesNotCrash" +``` + +## Lessons Learned + +**What would have helped discover this fix:** + +1. **Codebase-wide search** - `grep -r "TraitCollectionDidChange" src/` would have found both locations +2. **Layer analysis** - Ask "Does this belong in Core or Controls?" +3. **Duplication detection** - Recognize when the same override exists in multiple layers +4. **Remove vs patch** - Consider whether code should exist at all, not just how to fix it + +**Repository improvements needed:** + +1. Architecture documentation explaining Core vs Controls layer responsibility +2. Try-fix skill enhancement to search for duplicate implementations +3. Inline comments in key classes about layer responsibilities +4. Linting rule to detect duplicate iOS/Android method overrides across layers diff --git a/.github/agent-pr-session/pr-32289.md b/.github/agent-pr-session/pr-32289.md new file mode 100644 index 000000000000..33c9e80c1071 --- /dev/null +++ b/.github/agent-pr-session/pr-32289.md @@ -0,0 +1,202 @@ +# PR Review: #32289 - Fix handler not disconnected when removing non visible pages using RemovePage() + +**Date:** 2026-01-07 | **Issue:** [#32239](https://github.com/dotnet/maui/issues/32239) | **PR:** [#32289](https://github.com/dotnet/maui/pull/32289) + +## ✅ Status: COMPLETE + +| Phase | Status | +|-------|--------| +| Pre-Flight | ✅ COMPLETE | +| 🧪 Tests | ✅ COMPLETE | +| 🚦 Gate | ✅ PASSED | +| 🔧 Fix | ✅ COMPLETE | +| 📋 Report | ✅ COMPLETE | + +--- + +
+📋 Issue Summary + +**Problem:** When removing pages from a NavigationPage's navigation stack using `NavigationPage.Navigation.RemovePage()`, handlers are not properly disconnected from the removed pages. However, using `ContentPage.Navigation.RemovePage()` correctly disconnects handlers. + +**Root Cause (from PR):** The `RemovePage()` method removes the page from the navigation stack but does not explicitly disconnect its handler. + +**Regression:** Introduced in PR #24887, reproducible from MAUI 9.0.40+ + +**Steps to Reproduce:** +1. Push multiple pages onto a NavigationPage stack +2. Call `NavigationPage.Navigation.RemovePage()` on a non-visible page +3. Observe that the page's handler remains connected (no cleanup) + +**Workaround:** Manually call `.DisconnectHandlers()` after removing the page + +**Platforms Affected:** +- [x] iOS +- [x] Android +- [x] Windows +- [x] MacCatalyst + +
+ +
+📁 Files Changed + +| File | Type | Changes | +|------|------|---------| +| `src/Controls/src/Core/NavigationPage/NavigationPage.cs` | Fix | +4 lines | +| `src/Controls/src/Core/NavigationPage/NavigationPage.Legacy.cs` | Fix | +4 lines | +| `src/Controls/src/Core/NavigationProxy.cs` | Fix | -1 line (removed duplicate) | +| `src/Controls/src/Core/Shell/ShellSection.cs` | Fix | +1 line | +| `src/Controls/tests/Core.UnitTests/NavigationUnitTest.cs` | Unit Test | +63 lines | +| `src/Controls/tests/Core.UnitTests/ShellNavigatingTests.cs` | Unit Test | +25 lines | + +**Test Type:** Unit Tests + +
+ +
+💬 PR Discussion Summary + +**Key Comments:** +- Copilot flagged potential duplicate disconnection logic between NavigationProxy and NavigationPage +- Author responded by removing redundant logic from NavigationProxy and updating ShellSection +- StephaneDelcroix requested unit tests → Author added them +- rmarinho confirmed unit tests cover both `useMaui: true` and `useMaui: false` scenarios + +**Reviewer Feedback:** +- Comments about misleading code comments (fixed) +- Concern about duplicate `DisconnectHandlers()` calls (resolved by moving from NavigationProxy to implementations) +- StephaneDelcroix: Approved after unit tests added +- rmarinho: Approved - confirmed tests cover NavigationPage and Shell scenarios + +**Maintainer Approvals:** +- ✅ StephaneDelcroix (Jan 7, 2026) +- ✅ rmarinho (Jan 7, 2026) + +**Disagreements to Investigate:** +| File:Line | Reviewer Says | Author Says | Status | +|-----------|---------------|-------------|--------| +| NavigationPage.cs:914 | Duplicate disconnection with NavigationProxy | Removed from NavigationProxy, now only in NavigationPage | ✅ RESOLVED | +| NavigationPage.Legacy.cs:257 | Same duplicate concern | Same resolution | ✅ RESOLVED | + +**Author Uncertainty:** +- None noted + +
+ +
+🧪 Tests + +**Status**: ✅ COMPLETE + +- [x] PR includes unit tests +- [x] Tests follow naming convention +- [x] Unit tests cover both useMaui: true/false paths +- [x] Unit tests cover Shell navigation + +**Test Files (from PR - Unit Tests):** +- `src/Controls/tests/Core.UnitTests/NavigationUnitTest.cs` (+63 lines) + - `RemovePageDisconnectsHandlerForNonVisiblePage` - Tests removing middle page from 3-page stack (both useMaui: true/false) + - `RemovePageDisconnectsHandlerForRemovedRootPage` - Tests removing root page when another page is on top +- `src/Controls/tests/Core.UnitTests/ShellNavigatingTests.cs` (+25 lines) + - `RemovePageDisconnectsHandlerInShell` - Tests Shell navigation scenario + +**Unit Test Coverage Analysis:** +| Code Path | useMaui: true | useMaui: false | Shell | +|-----------|---------------|----------------|-------| +| Remove middle page | ✅ | ✅ | ✅ | +| Remove root page | ✅ | ✅ | - | + +Coverage is adequate - tests cover all modified code paths. + +
+ +
+🚦 Gate - Test Verification + +**Status**: ✅ PASSED + +- [x] Tests FAIL without fix (bug reproduced) +- [x] Tests PASS with fix + +**Result:** PASSED ✅ + +**Verification:** Unit tests from PR cover all code paths: +- `RemovePageDisconnectsHandlerForNonVisiblePage(true)` - Maui path (Android/Windows) +- `RemovePageDisconnectsHandlerForNonVisiblePage(false)` - Legacy path (iOS/MacCatalyst) +- `RemovePageDisconnectsHandlerForRemovedRootPage(true/false)` - Root page removal +- `RemovePageDisconnectsHandlerInShell` - Shell navigation + +
+ +
+🔧 Fix Candidates + +**Status**: ✅ COMPLETE + +| # | Source | Approach | Test Result | Files Changed | Notes | +|---|--------|----------|-------------|---------------|-------| +| 1 | try-fix | Add DisconnectHandlers() inside SendHandlerUpdateAsync callback | ❌ FAIL | `NavigationPage.cs` (+3) | **Why failed:** Timing issue - SendHandlerUpdateAsync uses FireAndForget(), so the callback with DisconnectHandlers() runs asynchronously. The test checks Handler immediately after RemovePage() returns, before the async callback executes. | +| 2 | try-fix | Add DisconnectHandlers() synchronously after SendHandlerUpdateAsync in MauiNavigationImpl | ❌ FAIL | `NavigationPage.cs` (+3) | **Why failed:** iOS uses `UseMauiHandler = false`, meaning it uses NavigationImpl (Legacy) NOT MauiNavigationImpl. My fix was in the wrong code path - iOS doesn't execute MauiNavigationImpl at all. | +| 3 | try-fix | Add DisconnectHandlers() at end of Legacy RemovePage method | ✅ PASS | `NavigationPage.Legacy.cs` (+3) | Works! iOS/MacCatalyst use Legacy path. Simpler fix - only 1 file needed for iOS. | +| 4 | try-fix | Approach 2+3 combined (both Maui and Legacy paths) | ✅ PASS (iOS) | `NavigationPage.cs`, `NavigationPage.Legacy.cs` (+6 total) | Works for NavigationPage on all platforms, BUT **misses Shell navigation** which has its own code path. | +| PR | PR #32289 | Add `DisconnectHandlers()` call in RemovePage for non-visible pages | ✅ PASS (Gate) | NavigationPage.cs, NavigationPage.Legacy.cs, NavigationProxy.cs, ShellSection.cs | Original PR - validated by Gate | + +**Note:** try-fix candidates (1, 2, 3...) are added during Phase 4. PR's fix is reference only. + +**Exhausted:** No (stopped after finding working alternative) +**Selected Fix:** PR's fix + +**Deep Analysis (Git History Research):** + +**Historical Timeline:** +1. **PR #24887** (Feb 2025): Fixed Android flickering by avoiding handler removal during PopAsync - inadvertently broke RemovePage scenarios +2. **PR #30049** (June 2025): Attempted fix by adding `page?.DisconnectHandlers()` to `NavigationProxy.OnRemovePage()` - **BUT THIS FIX WAS FUNDAMENTALLY FLAWED** +3. **PR #32289** (Current): Correctly fixes by adding DisconnectHandlers to the NavigationPage implementations + +**Why PR #30049's fix didn't work:** +- `MauiNavigationImpl` and `NavigationImpl` **override** `OnRemovePage()` +- The overrides do NOT call `base.OnRemovePage()` +- Therefore `NavigationProxy.OnRemovePage()` is **NEVER executed** for NavigationPage! +- ContentPage works because it doesn't override - uses the base NavigationProxy directly + +**Why calling base.OnRemovePage() won't work:** +- `MauiNavigationImpl.OnRemovePage()` is a **complete replacement** with its own validation, async flow, etc. +- Calling base would cause double removal and ordering issues + +**Conclusion:** The fix MUST be in the NavigationPage implementations themselves, not in NavigationProxy. PR #32289's approach is architecturally correct. + +**Comparison:** +- **My fix #3** works for iOS/MacCatalyst (Legacy path) - 1 file, 3 lines +- **PR's fix** works for ALL platforms (Legacy + Maui paths) - 3 files, ~10 lines +- **PR #30049's approach** ❌ Doesn't work - fix in NavigationProxy is bypassed by overrides + +**Rationale for selecting PR's fix:** +1. PR covers ALL platforms (iOS, MacCatalyst, Android, Windows) while my fix only covers iOS/MacCatalyst +2. PR also fixes ShellSection for Shell navigation scenarios +3. PR uses null-safety (`page?`) which is more defensive +4. PR correctly removes the ineffective DisconnectHandlers from NavigationProxy (cleanup) +5. My successful fix #3 is essentially a subset of the PR's approach + +**Independent validation:** My fix #3 independently arrived at the same solution as the PR for the Legacy path, which validates the PR's approach is correct. + +
+ +--- + +## ✅ Final Recommendation: APPROVE + +**Summary:** PR #32289 correctly fixes the handler disconnection issue when removing non-visible pages using `RemovePage()`. + +**Key Findings:** +1. ✅ **Root cause correctly identified** - NavigationPage overrides bypass NavigationProxy, requiring fix in implementations +2. ✅ **All code paths covered** - NavigationPage (Maui + Legacy) and ShellSection +3. ✅ **Unit tests adequate** - Cover both `useMaui: true/false` and Shell navigation +4. ✅ **Two maintainer approvals** - StephaneDelcroix and rmarinho +5. ✅ **Independent validation** - My try-fix #3 independently arrived at same solution for Legacy path + +**Alternative approaches tested:** +- Approach 2+3 (Maui + Legacy paths only) works but misses Shell navigation +- PR's fix is more complete and architecturally correct + +**No concerns identified.** diff --git a/.github/agent-pr-session/pr-33134.md b/.github/agent-pr-session/pr-33134.md new file mode 100644 index 000000000000..19b6325c46de --- /dev/null +++ b/.github/agent-pr-session/pr-33134.md @@ -0,0 +1,203 @@ +# PR Review: #33134 - [Android] EmptyView doesn't display when CollectionView is placed inside a VerticalStackLayout + +**Date:** 2026-01-09 | **Issue:** [#32932](https://github.com/dotnet/maui/issues/32932) | **PR:** [#33134](https://github.com/dotnet/maui/pull/33134) + +## ✅ Status: COMPLETE + +| Phase | Status | +|-------|--------| +| Pre-Flight | ✅ COMPLETE | +| 🧪 Tests | ✅ COMPLETE | +| 🚦 Gate | ✅ COMPLETE | +| 🔧 Fix | ✅ COMPLETE | +| 📋 Report | ✅ COMPLETE | + +--- + +
+📋 Issue Summary + +CollectionView has an EmptyView property for rendering a view when the ItemsSource is empty. This does not work when the CollectionView is placed inside a VerticalStackLayout. + +**Steps to Reproduce:** +1. Place a CollectionView inside a VerticalStackLayout +2. Set an empty ItemsSource +3. Set an EmptyView or EmptyViewTemplate +4. Run on Android - EmptyView does not display + +**Platforms Affected:** +- [ ] iOS +- [x] Android +- [ ] Windows +- [ ] MacCatalyst + +**Reproduction repo:** https://github.com/rrbabbb/EmptyViewNotWorkingRepro + +
+ +
+📁 Files Changed + +| File | Type | Changes | +|------|------|---------| +| `src/Controls/src/Core/Handlers/Items/Android/SizedItemContentView.cs` | Fix | +2 lines | +| `src/Controls/tests/TestCases.HostApp/Issues/Issue32932.cs` | Test | New file | +| `src/Controls/tests/TestCases.Shared.Tests/Tests/Issues/Issue32932.cs` | Test | New file | + +
+ +
+💬 PR Discussion Summary + +**Key Comments:** +- No significant reviewer feedback at time of review + +**Author:** NanthiniMahalingam (Syncfusion partner) + +**Labels:** platform/android, area-controls-collectionview, community ✨, partner/syncfusion + +
+ +
+🧪 Tests + +**Status**: ✅ COMPLETE + +- [x] PR includes UI tests +- [x] Tests reproduce the issue +- [x] Tests follow naming convention (`IssueXXXXX`) + +**Test Files:** +- HostApp: `src/Controls/tests/TestCases.HostApp/Issues/Issue32932.cs` +- NUnit: `src/Controls/tests/TestCases.Shared.Tests/Tests/Issues/Issue32932.cs` + +**Test Description:** Places a CollectionView with an empty ItemsSource inside a VerticalStackLayout and verifies that the EmptyView (a Label with AutomationId="EmptyView") is visible. + +**Verification:** +- Tests compile successfully +- Follow proper naming conventions (Issue32932) +- Use `CollectionView2` handler for Android +- Have correct `[Issue()]` attributes and categories + +
+ +
+🚦 Gate - Test Verification + +**Status**: ✅ COMPLETE + +- [x] Tests FAIL without fix (bug reproduced) +- [x] Tests PASS with fix (bug fixed) + +**Result:** VERIFIED ✅ + +**Test Execution Results:** + +| State | Result | Details | +|-------|--------|---------| +| **WITH fix** | ✅ PASS | EmptyView element found immediately (1 Appium query) | +| **WITHOUT fix** | ❌ FAIL (Expected) | EmptyView never appears - 25+ retries before timeout | + +**Verification Method:** +1. Ran test with PR fix applied → PASS +2. Reverted `SizedItemContentView.cs` to pre-fix version (before commit `b498b13ef6`) +3. Ran test without fix → FAIL (TimeoutException: `Timed out waiting for element...`) +4. Restored fix + +**Log Evidence:** +- WITH fix: `method: 'id', selector: 'com.microsoft.maui.uitests:id/EmptyView'` → found immediately +- WITHOUT fix: 25+ consecutive searches for `EmptyView` element, never found, test times out + +
+ +
+🔧 Fix Candidates + +**Status**: ✅ COMPLETE + +| # | Source | Approach | Test Result | Files Changed | Notes | +|---|--------|----------|-------------|---------------|-------| +| 1 | try-fix | Fix at source in `EmptyViewAdapter.GetHeight()/GetWidth()` - return `double`, check `IsInfinity()` before casting | ✅ PASS | `EmptyViewAdapter.cs` | Fixes bug at the source but changes method signatures | +| 2 | try-fix | Fix at source in `EmptyViewAdapter` - use `double` throughout + `Math.Abs` + infinity check | ✅ PASS | `EmptyViewAdapter.cs` | Cleaner version of #1 but still changes method signatures | +| 3 | try-fix | **Avoid int.MaxValue entirely:** keep `GetHeight/GetWidth` as `int`, but change the lambdas passed to `SizedItemContentView`/`SimpleViewHolder` to return `double.PositiveInfinity` when `RecyclerViewHeight/Width` is infinite | ✅ PASS | `EmptyViewAdapter.cs` (+4/-3) | Preserves infinity without signature changes and without magic-number checks; not defensive for other callers | +| 4 | try-fix | Check for infinity BEFORE casting in `GetHeight()`/`GetWidth()`, return 0 to trigger fallback | ❌ FAIL | `EmptyViewAdapter.cs` | **Why failed:** Fallback to `parent.MeasuredHeight` returns 0 during initial layout pass - doesn't provide valid dimensions | +| 5 | try-fix | Skip setting `RecyclerViewHeight/Width` when infinite | ❌ FAIL | `ItemsViewHandler.Android.cs` | **Why failed:** Same issue as #4 - fallback mechanism doesn't work when parent not yet measured | +| 6 | try-fix | Add `GetHeightAsDouble()`/`GetWidthAsDouble()` methods that return infinity when appropriate, use in `CreateEmptyViewHolder` lambdas | ✅ PASS | `EmptyViewAdapter.cs` (+18) | Cleaner version of #3 with dedicated helper methods; fixes at source, no heuristic, no signature changes | +| PR | PR #33134 | Add `NormalizeDimension()` helper in `SizedItemContentView` that converts `int.MaxValue` → `double.PositiveInfinity` | ✅ VERIFIED | `SizedItemContentView.cs` | **SELECTED** - Defensive/low-risk downstream patch | + +### Root Cause Analysis + +When a CollectionView is inside a VerticalStackLayout, the height constraint passed to the CollectionView is `double.PositiveInfinity` (unconstrained). This value propagates to `EmptyViewAdapter.RecyclerViewHeight`. + +In `EmptyViewAdapter.GetHeight()`, the code casts this to `int`: `int height = (int)RecyclerViewHeight;` + +In C#, `(int)double.PositiveInfinity` produces `int.MaxValue` (2147483647). This value is then passed via lambda to `SizedItemContentView.OnMeasure()`, where the code checks `double.IsInfinity(targetHeight)` - but `int.MaxValue` is not infinity, so the check fails and layout calculations break. + +### Key Insight from Failed Attempts + +Attempts #4 and #5 revealed that: +- `(int)double.PositiveInfinity` in C# actually produces `int.MaxValue` (verified empirically) +- The fallback to `parent.MeasuredHeight` doesn't work during initial layout when parent hasn't been measured yet +- The solution must **preserve infinity semantics** rather than trying to fall back to measured values + +### Comparison of Passing Fixes + +| Criteria | PR's Fix | Source-Level (#3/#6) | +|----------|----------|---------------------| +| **Correctness** | ✅ Handles `int.MaxValue` → `∞` | ✅ Preserves `∞` directly | +| **Defensive** | ✅ Protects ALL callers of `SizedItemContentView` | ❌ Only fixes EmptyView path | +| **Risk** | ✅ Minimal - 1 line addition | ⚠️ Adds new methods to adapter | +| **Heuristic concern** | ⚠️ Assumes `int.MaxValue` means infinity | ✅ No heuristic | + +**Why PR's heuristic is valid:** `(int)double.PositiveInfinity` produces `int.MaxValue` in C#, so the heuristic is actually semantically correct - `int.MaxValue` really does mean "this was infinity before the cast." + +**Exhausted:** Yes (6 try-fix attempts completed) +**Selected Fix:** PR #33134's fix (NormalizeDimension helper in SizedItemContentView) + +**Rationale:** The PR's fix is the best choice because: +1. **Defensive:** Protects ALL callers of `SizedItemContentView.OnMeasure()`, not just EmptyView +2. **Low-risk:** Minimal code change (1 helper method, 2 call sites) +3. **Semantically correct:** The heuristic `int.MaxValue → PositiveInfinity` is valid because `(int)double.PositiveInfinity` produces `int.MaxValue` in C# +4. **No signature changes:** Doesn't require changing method signatures in `EmptyViewAdapter` + +
+ +--- + +## 📋 Final Report + +### Recommendation: ✅ APPROVE + +**Summary:** PR #33134 correctly fixes the Android EmptyView visibility bug when CollectionView is inside a VerticalStackLayout. + +### Technical Analysis + +**Root Cause:** +- CollectionView inside VerticalStackLayout receives `double.PositiveInfinity` as height constraint +- `EmptyViewAdapter.GetHeight()` casts this to `int`: `(int)double.PositiveInfinity` → `int.MaxValue` +- `SizedItemContentView.OnMeasure()` checks `double.IsInfinity(targetHeight)` but `int.MaxValue` is not infinity +- Layout calculations fail, EmptyView doesn't display + +**Fix:** +- Adds `NormalizeDimension()` helper that converts `int.MaxValue` back to `double.PositiveInfinity` +- Applied at the point of use in `OnMeasure()`, making it defensive for all callers + +### Quality Assessment + +| Aspect | Rating | Notes | +|--------|--------|-------| +| **Correctness** | ✅ Excellent | Fix addresses root cause, tests verify behavior | +| **Test Coverage** | ✅ Good | UI test properly reproduces the bug | +| **Risk** | ✅ Low | Minimal, surgical change | +| **Code Quality** | ✅ Good | Clean helper method, proper naming | +| **Alternative Comparison** | ✅ Done | 6 alternatives explored, PR's approach is best | + +### Test Verification + +- ✅ Test PASSES with fix (EmptyView visible) +- ✅ Test FAILS without fix (TimeoutException - EmptyView never appears) + +### Minor Suggestions (Non-blocking) + +1. **Consider XML doc comment** for `NormalizeDimension()` explaining why the conversion is needed +2. **Consider adding regression test for Grid container** as reported in issue comments diff --git a/.github/agent-pr-session/pr-33406.md b/.github/agent-pr-session/pr-33406.md new file mode 100644 index 000000000000..e79aca2901f5 --- /dev/null +++ b/.github/agent-pr-session/pr-33406.md @@ -0,0 +1,247 @@ +# PR Review: #33406 - [iOS] Fixed Shell navigation on search handler suggestion selection + +**Date:** 2026-01-08 | **Issue:** [#33356](https://github.com/dotnet/maui/issues/33356) | **PR:** [#33406](https://github.com/dotnet/maui/pull/33406) + +**Related Prior Attempt:** [PR #33396](https://github.com/dotnet/maui/pull/33396) (closed - Copilot CLI attempt) + +## ⏳ Status: IN PROGRESS + +| Phase | Status | +|-------|--------| +| Pre-Flight | ✅ COMPLETE | +| 🧪 Tests | ⏳ PENDING | +| 🚦 Gate | ⏳ PENDING | +| 🔧 Fix | ⏳ PENDING | +| 📋 Report | ⏳ PENDING | + +--- + +
+📋 Issue Summary + +**Issue #33356**: [iOS] Clicking on search suggestions fails to navigate to detail page correctly + +**Bug Description**: Clicking on a search suggestion using NavigationBar/SearchBar/custom SearchHandler does not navigate to the detail page correctly on iOS 26.1 & 26.2 with MAUI 10. + +**Root Cause (from PR #33406)**: Navigation fails because `UISearchController` was dismissed (`Active = false`) BEFORE `ItemSelected` was called. This triggers a UIKit transition that deactivates the Shell navigation context and prevents the navigation from completing. + +**Reproduction App**: https://github.com/dotnet/maui-samples/tree/main/10.0/Fundamentals/Shell/Xaminals + +**Steps to Reproduce:** +1. Open the Xaminals sample app +2. Deploy to iPhone 17 Pro 26.2 simulator (Xcode 26.2) +3. Put focus on the search box +4. Type 'b' (note: search dropdown position is wrong - see Issue #32930) +5. Click on 'Bengal' in search suggestions +6. **Issue 1:** No navigation happens (expected: navigate to Bengal cat detail page) +7. Click on 'Bengal' from the main list - this works correctly +8. Click back button +9. **Issue 2:** Navigates to an empty page (expected: navigate back to list) +10. Click back button again - actually navigates back + +**Platforms Affected:** +- [ ] Android +- [x] iOS (26.1 & 26.2) +- [ ] Windows +- [ ] MacCatalyst + +**Regression Info:** +- **Confirmed regression** starting in version 9.0.90 +- Labels: `t/bug`, `platform/ios`, `s/verified`, `s/triaged`, `i/regression`, `shell-search-handler`, `regressed-in-9.0.90` +- Issue 2 (empty page on back navigation) specifically reproducible from 9.0.90 + +**Validated by:** TamilarasanSF4853 (Syncfusion partner) - Confirmed reproducible in VS Code 1.107.1 with MAUI versions 9.0.0, 9.0.82, 9.0.90, 9.0.120, 10.0.0, and 10.0.20 on iOS. + +
+ +
+📁 Files Changed - PR #33406 (Community PR) + +| File | Type | Changes | +|------|------|---------| +| `src/Controls/src/Core/Compatibility/Handlers/Shell/iOS/ShellPageRendererTracker.cs` | Fix | 2 lines (swap order) | +| `src/Controls/tests/TestCases.HostApp/Issues/Issue33356.cs` | Test (HostApp) | +261 lines | +| `src/Controls/tests/TestCases.Shared.Tests/Tests/Issues/Issue33356.cs` | Test (NUnit) | +46 lines | + +**PR #33406 Fix** (simpler approach - just swap order): +```diff + void OnSearchItemSelected(object? sender, object e) + { + if (_searchController is null) + return; + +- _searchController.Active = false; + (SearchHandler as ISearchHandlerController)?.ItemSelected(e); ++ _searchController.Active = false; + } +``` + +
+ +
+📁 Files Changed - PR #33396 (Prior Copilot Attempt - CLOSED) + +| File | Type | Changes | +|------|------|---------| +| `.github/agent-pr-session/pr-33396.md` | Session | +210 lines | +| `src/Controls/src/Core/Compatibility/Handlers/Shell/iOS/ShellPageRendererTracker.cs` | Fix | +17 lines | +| `src/Controls/tests/TestCases.HostApp/Issues/Issue33356.xaml` | Test (XAML) | +41 lines | +| `src/Controls/tests/TestCases.HostApp/Issues/Issue33356.xaml.cs` | Test (C#) | +138 lines | +| `src/Controls/tests/TestCases.Shared.Tests/Tests/Issues/Issue33356.cs` | Test (NUnit) | +70 lines | + +**PR #33396 Fix** (more defensive approach with BeginInvokeOnMainThread): +```diff + void OnSearchItemSelected(object? sender, object e) + { + if (_searchController is null) + return; + ++ // Store the search controller reference before any state changes ++ var searchController = _searchController; ++ ++ // Call ItemSelected first to trigger navigation before dismissing the search UI. ++ // On iOS 26+, setting Active = false before navigation can cause the navigation ++ // to be lost due to the search controller dismissal animation. + (SearchHandler as ISearchHandlerController)?.ItemSelected(e); +- _searchController.Active = false; ++ ++ // Deactivate the search controller after navigation has been initiated. ++ // Using BeginInvokeOnMainThread ensures this happens after the current run loop, ++ // allowing the navigation to proceed without interference from the dismissal animation. ++ ViewController?.BeginInvokeOnMainThread(() => ++ { ++ if (searchController is not null) ++ { ++ searchController.Active = false; ++ } ++ }); + } +``` + +
+ +
+💬 Discussion Summary + +**Key Comments from Issue #33356:** +- TamilarasanSF4853 (Syncfusion): Validated issue across multiple MAUI versions (9.0.0 through 10.0.20) +- Issue 2 (empty page on back) specifically regressed in 9.0.90 +- Issue 1 (no navigation on search suggestion tap) affects all tested versions on iOS + +**PR #33406 Review Comments:** +- Copilot PR reviewer caught typo: "searchHander" should be "searchHandler" (5 duplicate comments, all resolved/outdated now) +- Prior agent review by kubaflo marked it as ✅ APPROVE with comprehensive analysis +- PureWeen requested `/rebase` (latest comment) + +**PR #33396 Review Comments:** +- PureWeen asked to update state file to match PR number +- Copilot had firewall issues accessing GitHub API + +**Disagreements to Investigate:** +| File:Line | Reviewer Says | Author Says | Status | +|-----------|---------------|-------------|--------| +| N/A | N/A | N/A | No active disagreements | + +**Author Uncertainty:** +- None noted in either PR + +
+ +
+⚖️ Comparison: PR #33406 vs PR #33396 + +### Fix Approach Comparison + +| Aspect | PR #33406 (Community) | PR #33396 (Copilot) | +|--------|----------------------|---------------------| +| **Author** | SubhikshaSf4851 (Syncfusion) | Copilot | +| **Status** | Open | Closed (draft) | +| **Lines Changed** | 2 (swap order) | 17 (more defensive) | +| **Fix Strategy** | Simply swap order of operations | Swap order + dispatch to next run loop | +| **Test Style** | Code-only (no XAML) | XAML + code-behind | +| **Test Count** | 1 test method | 2 test methods | + +### Which Fix is Better? + +**PR #33406 (simpler approach):** +- ✅ Minimal change - just swaps two lines +- ✅ Addresses root cause: ItemSelected called while navigation context is valid +- ⚠️ Dismissal happens synchronously after ItemSelected +- ⚠️ Could theoretically still interfere if dismissal animation is fast + +**PR #33396 (defensive approach):** +- ✅ Uses BeginInvokeOnMainThread for explicit async deactivation +- ✅ Stores reference to search controller before state changes +- ✅ More detailed comments explaining the fix +- ⚠️ More code complexity +- ⚠️ Was closed/abandoned + +### Recommendation + +Both approaches should work. PR #33406 is simpler and has been reviewed/approved. The extra defensive measures in PR #33396 (BeginInvokeOnMainThread) may provide additional safety margin but add complexity. + +**Prior agent review on PR #33406** already verified: +- Tests FAIL without fix (bug reproduced - timeout) +- Tests PASS with fix (navigation successful) + +
+ +
+🧪 Tests + +**Status**: ⏳ PENDING (need to verify tests compile and reproduce issue) + +**PR #33406 Tests:** +- HostApp: `src/Controls/tests/TestCases.HostApp/Issues/Issue33356.cs` (code-only, no XAML) +- NUnit: `src/Controls/tests/TestCases.Shared.Tests/Tests/Issues/Issue33356.cs` +- 1 test: `Issue33356NavigateShouldOccur` - Tests search handler navigation AND back navigation + collection view navigation + +**PR #33396 Tests (for reference):** +- HostApp XAML: `src/Controls/tests/TestCases.HostApp/Issues/Issue33356.xaml` +- HostApp Code: `src/Controls/tests/TestCases.HostApp/Issues/Issue33356.xaml.cs` +- NUnit: `src/Controls/tests/TestCases.Shared.Tests/Tests/Issues/Issue33356.cs` +- 2 tests: `SearchSuggestionTapNavigatesToDetailPage`, `BackNavigationFromDetailPageWorks` + +**Test Checklist:** +- [ ] PR includes UI tests +- [ ] Tests reproduce the issue +- [ ] Tests follow naming convention (`Issue33356`) + +
+ +
+🚦 Gate - Test Verification + +**Status**: ⏳ PENDING + +- [ ] Tests FAIL without fix (bug reproduced) +- [ ] Tests PASS with fix (fix validated) + +**Prior Agent Review Result (kubaflo on PR #33406):** +``` +WITHOUT FIX: FAILED - System.TimeoutException: Timed out waiting for element "Issue33356CatNameLabel" +WITH FIX: PASSED - All 1 tests passed in 21.73 seconds +``` + +**Result:** [PENDING - needs re-verification] + +
+ +
+🔧 Fix Candidates + +**Status**: ⏳ PENDING + +| # | Source | Approach | Test Result | Files Changed | Notes | +|---|--------|----------|-------------|---------------|-------| +| PR | PR #33406 | Swap order: ItemSelected before Active=false | ⏳ PENDING (Gate) | `ShellPageRendererTracker.cs` (2 lines) | Current PR - simpler fix | +| Alt | PR #33396 | Swap order + BeginInvokeOnMainThread | ✅ VERIFIED (prior test) | `ShellPageRendererTracker.cs` (17 lines) | Prior attempt - more defensive | + +**Exhausted:** No +**Selected Fix:** [PENDING] + +
+ +--- + +**Next Step:** Verify PR #33406 tests compile and Gate passes. Read `.github/agents/pr/post-gate.md` after Gate passes. diff --git a/.github/instructions/uitests.instructions.md b/.github/instructions/uitests.instructions.md index 247792eecc55..c0778ec7bb33 100644 --- a/.github/instructions/uitests.instructions.md +++ b/.github/instructions/uitests.instructions.md @@ -171,9 +171,9 @@ Tests should run on all applicable platforms by default. The test infrastructure ### No Inline #if Directives in Test Methods -**Do NOT use `#if ANDROID`, `#if IOS`, etc. inside test method bodies.** Platform-specific behavior must be hidden behind extension methods for readability. +**Do NOT use `#if ANDROID`, `#if IOS`, etc. directly in test methods.** Platform-specific behavior must be hidden behind extension methods for readability. -**Note:** File-level `#if` (to exclude entire files) or wrapping entire test methods is acceptable. This rule targets inline conditionals within test logic that make code hard to read and maintain. +**Note:** This rule is about **code cleanliness**, not platform scope. Using `#if ANDROID ... #else ...` still compiles for all platforms - the issue is that inline directives make test logic hard to read and maintain. ```csharp // ❌ BAD - inline #if in test method (hard to read) diff --git a/src/Controls/src/BindingSourceGen/BindingSourceGenerator.cs b/src/Controls/src/BindingSourceGen/BindingSourceGenerator.cs index 66e1c66f6916..dd1e8e8aef8e 100644 --- a/src/Controls/src/BindingSourceGen/BindingSourceGenerator.cs +++ b/src/Controls/src/BindingSourceGen/BindingSourceGenerator.cs @@ -210,20 +210,30 @@ private static Result GetLambdaReturnType(LambdaExpressionSyntax la var lambdaResultType = semanticModel.GetTypeInfo(lambdaBody, t).Type; if (lambdaResultType == null || lambdaResultType is IErrorTypeSymbol) { - // Try to infer the type from known patterns (e.g., RelayCommand properties) + // Try to infer the type from known patterns (e.g., RelayCommand, ObservableProperty) if (lambdaBody is MemberAccessExpressionSyntax memberAccess) { var memberName = memberAccess.Name.Identifier.Text; var expressionType = semanticModel.GetTypeInfo(memberAccess.Expression).Type; - - if (expressionType != null && - expressionType.TryGetRelayCommandPropertyType(memberName, semanticModel.Compilation, out var commandType) && - commandType != null) + + if (expressionType != null) { - return Result.Success(commandType); + // Check for RelayCommand-generated properties + if (expressionType.TryGetRelayCommandPropertyType(memberName, semanticModel.Compilation, out var commandType) && + commandType != null) + { + return Result.Success(commandType); + } + + // Check for ObservableProperty-generated properties + if (expressionType.TryGetObservablePropertyType(memberName, semanticModel.Compilation, out var propertyType) && + propertyType != null) + { + return Result.Success(propertyType); + } } } - + return Result.Failure(DiagnosticsFactory.LambdaResultCannotBeResolved(lambdaBody.GetLocation())); } diff --git a/src/Controls/src/BindingSourceGen/ITypeSymbolExtensions.cs b/src/Controls/src/BindingSourceGen/ITypeSymbolExtensions.cs index fcefb561e33c..43ea23d060b6 100644 --- a/src/Controls/src/BindingSourceGen/ITypeSymbolExtensions.cs +++ b/src/Controls/src/BindingSourceGen/ITypeSymbolExtensions.cs @@ -67,10 +67,10 @@ public static bool TryGetRelayCommandPropertyType(this ITypeSymbol symbol, strin // Extract the method name (property name without "Command" suffix) var methodName = propertyName.Substring(0, propertyName.Length - "Command".Length); - + // Look for a method with the base name - search in the type and base types var methods = GetAllMethods(symbol, methodName); - + foreach (var method in methods) { // Check if the method has the RelayCommand attribute @@ -93,6 +93,54 @@ public static bool TryGetRelayCommandPropertyType(this ITypeSymbol symbol, strin return false; } + /// + /// Checks if a property name could be generated by CommunityToolkit.Mvvm's [ObservableProperty] attribute, + /// and returns the inferred property type if found. + /// + /// The type to search + /// The name of the property to find + /// The compilation (can be null) + /// The inferred property type if an ObservableProperty field is found + /// True if an ObservableProperty field was found that would generate this property + public static bool TryGetObservablePropertyType(this ITypeSymbol symbol, string propertyName, Compilation? compilation, out ITypeSymbol? propertyType) + { + propertyType = null; + + if (compilation == null || string.IsNullOrEmpty(propertyName)) + return false; + + // ObservableProperty generates a PascalCase property from a camelCase, _camelCase, or m_camelCase field + // Try common field naming patterns + var possibleFieldNames = new[] + { + char.ToLowerInvariant(propertyName[0]) + propertyName.Substring(1), // name from Name + "_" + char.ToLowerInvariant(propertyName[0]) + propertyName.Substring(1), // _name from Name + "m_" + char.ToLowerInvariant(propertyName[0]) + propertyName.Substring(1) // m_name from Name + }; + + // Look for a field with one of the possible names - search in the type and base types + foreach (var fieldName in possibleFieldNames) + { + var fields = GetAllFields(symbol, fieldName); + + foreach (var field in fields) + { + // Check if the field has the ObservableProperty attribute + var hasObservableProperty = field.GetAttributes().Any(attr => + attr.AttributeClass?.Name == "ObservablePropertyAttribute" || + attr.AttributeClass?.ToDisplayString() == "CommunityToolkit.Mvvm.ComponentModel.ObservablePropertyAttribute"); + + if (hasObservableProperty) + { + propertyType = field.Type; + return true; + } + } + } + + return false; + } + private static System.Collections.Generic.IEnumerable GetAllMethods(ITypeSymbol symbol, string name) { // Search in current type @@ -114,4 +162,17 @@ private static System.Collections.Generic.IEnumerable GetAllMetho baseType = baseType.BaseType; } } + + private static System.Collections.Generic.IEnumerable GetAllFields(ITypeSymbol? symbol, string name) + { + while (symbol != null) + { + foreach (var member in symbol.GetMembers(name)) + { + if (member is IFieldSymbol field) + yield return field; + } + symbol = symbol.BaseType; + } + } } diff --git a/src/Controls/src/BindingSourceGen/PathParser.cs b/src/Controls/src/BindingSourceGen/PathParser.cs index ce5c225aeda6..11a77e79c7c4 100644 --- a/src/Controls/src/BindingSourceGen/PathParser.cs +++ b/src/Controls/src/BindingSourceGen/PathParser.cs @@ -87,7 +87,7 @@ private bool TryHandleSpecialCases(string memberName, ITypeSymbol expressionType pathPart = null; // Check for RelayCommand-generated properties - if (expressionType.TryGetRelayCommandPropertyType(memberName, _context.SemanticModel.Compilation, out var commandType) + if (expressionType.TryGetRelayCommandPropertyType(memberName, _context.SemanticModel.Compilation, out var commandType) && commandType != null) { var memberType = commandType.CreateTypeDescription(_enabledNullable); @@ -105,6 +105,25 @@ private bool TryHandleSpecialCases(string memberName, ITypeSymbol expressionType return true; } + // Check for ObservableProperty-generated properties + if (expressionType.TryGetObservablePropertyType(memberName, _context.SemanticModel.Compilation, out var propertyType) + && propertyType != null) + { + var memberType = propertyType.CreateTypeDescription(_enabledNullable); + var containingType = expressionType.CreateTypeDescription(_enabledNullable); + + pathPart = new MemberAccess( + MemberName: memberName, + IsValueType: !propertyType.IsReferenceType, + ContainingType: containingType, + MemberType: memberType, + Kind: AccessorKind.Property, + IsGetterInaccessible: false, // Assume generated property is accessible + IsSetterInaccessible: false); // ObservableProperty properties have setters + + return true; + } + return false; } diff --git a/src/Controls/src/Core/Compatibility/Handlers/Shell/iOS/ShellItemRenderer.cs b/src/Controls/src/Core/Compatibility/Handlers/Shell/iOS/ShellItemRenderer.cs index a787ecf1dee5..345b042e9c63 100644 --- a/src/Controls/src/Core/Compatibility/Handlers/Shell/iOS/ShellItemRenderer.cs +++ b/src/Controls/src/Core/Compatibility/Handlers/Shell/iOS/ShellItemRenderer.cs @@ -246,6 +246,9 @@ protected virtual void OnItemsCollectionChanged(object sender, NotifyCollectionC ViewControllers = viewControllers; CustomizableViewControllers = Array.Empty(); + // Apply initial IsEnabled state for each tab item + SetTabItemsEnabledState(); + if (goTo) GoTo(ShellItem.CurrentItem); } @@ -290,6 +293,28 @@ void AddRenderer(IShellSectionRenderer renderer) renderer.ShellSection.PropertyChanged += OnShellSectionPropertyChanged; } + void SetTabItemsEnabledState() + { + if (TabBar?.Items is null) + { + return; + } + + var items = ShellItemController.GetItems(); + if (items is null) + { + return; + } + + if (TabBar.Items.Length >= items.Count) + { + for (int tabIndex = 0; tabIndex < items.Count; tabIndex++) + { + TabBar.Items[tabIndex].Enabled = items[tabIndex].IsEnabled; + } + } + } + void CreateTabRenderers() { if (ShellItem.CurrentItem == null) @@ -315,6 +340,9 @@ void CreateTabRenderers() ViewControllers = viewControllers; CustomizableViewControllers = Array.Empty(); + // Apply initial IsEnabled state for newly added tab items + SetTabItemsEnabledState(); + UpdateTabBarHidden(); // Make sure we are at the right item diff --git a/src/Controls/src/Core/Compatibility/Handlers/Shell/iOS/ShellNavBarAppearanceTracker.cs b/src/Controls/src/Core/Compatibility/Handlers/Shell/iOS/ShellNavBarAppearanceTracker.cs index 53483a802fdc..e31120c1b798 100644 --- a/src/Controls/src/Core/Compatibility/Handlers/Shell/iOS/ShellNavBarAppearanceTracker.cs +++ b/src/Controls/src/Core/Compatibility/Handlers/Shell/iOS/ShellNavBarAppearanceTracker.cs @@ -29,6 +29,9 @@ public void ResetAppearance(UINavigationController controller) navBar.BarTintColor = _defaultBarTint; navBar.TintColor = _defaultTint; navBar.TitleTextAttributes = _defaultTitleAttributes; + + if (OperatingSystem.IsIOSVersionAtLeast(13) || OperatingSystem.IsTvOSVersionAtLeast(13)) + UpdateiOS13NavigationBarAppearance(controller, null); } } @@ -95,37 +98,47 @@ void UpdateiOS13NavigationBarAppearance(UINavigationController controller, Shell var navigationBarAppearance = new UINavigationBarAppearance(); - // since we cannot set the Background Image directly, let's use the alpha in the background color to determine translucence - if (appearance.BackgroundColor?.Alpha < 1.0f) - { - navigationBarAppearance.ConfigureWithTransparentBackground(); - navBar.Translucent = true; - } - else + if (appearance is null) { navigationBarAppearance.ConfigureWithOpaqueBackground(); navBar.Translucent = false; + navBar.StandardAppearance = navBar.ScrollEdgeAppearance = navigationBarAppearance; } + else + { - // Set ForegroundColor - var foreground = appearance.ForegroundColor; + // since we cannot set the Background Image directly, let's use the alpha in the background color to determine translucence + if (appearance.BackgroundColor?.Alpha < 1.0f) + { + navigationBarAppearance.ConfigureWithTransparentBackground(); + navBar.Translucent = true; + } + else + { + navigationBarAppearance.ConfigureWithOpaqueBackground(); + navBar.Translucent = false; + } - if (foreground != null) - navBar.TintColor = foreground.ToPlatform(); + // Set ForegroundColor + var foreground = appearance.ForegroundColor; - // Set BackgroundColor - var background = appearance.BackgroundColor; + if (foreground != null) + navBar.TintColor = foreground.ToPlatform(); - if (background != null) - navigationBarAppearance.BackgroundColor = background.ToPlatform(); + // Set BackgroundColor + var background = appearance.BackgroundColor; - // Set TitleColor - var titleColor = appearance.TitleColor; + if (background != null) + navigationBarAppearance.BackgroundColor = background.ToPlatform(); - if (titleColor != null) - navigationBarAppearance.TitleTextAttributes = new UIStringAttributes() { ForegroundColor = titleColor.ToPlatform() }; + // Set TitleColor + var titleColor = appearance.TitleColor; - navBar.StandardAppearance = navBar.ScrollEdgeAppearance = navigationBarAppearance; + if (titleColor != null) + navigationBarAppearance.TitleTextAttributes = new UIStringAttributes() { ForegroundColor = titleColor.ToPlatform() }; + + navBar.StandardAppearance = navBar.ScrollEdgeAppearance = navigationBarAppearance; + } } void UpdateNavigationBarAppearance(UINavigationController controller, ShellAppearance appearance) diff --git a/src/Controls/src/Core/Compatibility/Handlers/Shell/iOS/ShellPageRendererTracker.cs b/src/Controls/src/Core/Compatibility/Handlers/Shell/iOS/ShellPageRendererTracker.cs index c96eb8b868ea..c18fdc433f46 100644 --- a/src/Controls/src/Core/Compatibility/Handlers/Shell/iOS/ShellPageRendererTracker.cs +++ b/src/Controls/src/Core/Compatibility/Handlers/Shell/iOS/ShellPageRendererTracker.cs @@ -1058,8 +1058,8 @@ void OnSearchItemSelected(object? sender, object e) return; } - _searchController.Active = false; (SearchHandler as ISearchHandlerController)?.ItemSelected(e); + _searchController.Active = false; } void SearchButtonClicked(object? sender, EventArgs e) diff --git a/src/Controls/src/Core/Compatibility/Handlers/Shell/iOS/ShellSectionRootRenderer.cs b/src/Controls/src/Core/Compatibility/Handlers/Shell/iOS/ShellSectionRootRenderer.cs index f71544b4bb63..6d880ec642a6 100644 --- a/src/Controls/src/Core/Compatibility/Handlers/Shell/iOS/ShellSectionRootRenderer.cs +++ b/src/Controls/src/Core/Compatibility/Handlers/Shell/iOS/ShellSectionRootRenderer.cs @@ -141,16 +141,6 @@ public override void ViewSafeAreaInsetsDidChange() LayoutHeader(); } - public override void TraitCollectionDidChange(UITraitCollection previousTraitCollection) - { -#pragma warning disable CA1422 // Validate platform compatibility - base.TraitCollectionDidChange(previousTraitCollection); -#pragma warning restore CA1422 // Validate platform compatibility - - var application = _shellContext?.Shell?.FindMauiContext().Services.GetService(); - application?.ThemeChanged(); - } - void IDisconnectable.Disconnect() { _pageAnimation?.StopAnimation(true); diff --git a/src/Controls/src/Core/Compatibility/Handlers/Shell/iOS/ShellTableViewController.cs b/src/Controls/src/Core/Compatibility/Handlers/Shell/iOS/ShellTableViewController.cs index 8edcda80b108..dac39312fcbc 100644 --- a/src/Controls/src/Core/Compatibility/Handlers/Shell/iOS/ShellTableViewController.cs +++ b/src/Controls/src/Core/Compatibility/Handlers/Shell/iOS/ShellTableViewController.cs @@ -98,6 +98,20 @@ public override void ViewDidLoad() ShellFlyoutContentManager.ViewDidLoad(); } + public override void LoadView() + { + base.LoadView(); + View = new AccessibilityNeutralTableView(); + } + + internal class AccessibilityNeutralTableView : UITableView, IUIAccessibilityContainer + { + public AccessibilityNeutralTableView() + { + this.SetAccessibilityContainerType(UIAccessibilityContainerType.None); + } + } + [System.Runtime.Versioning.SupportedOSPlatform("ios11.0")] [System.Runtime.Versioning.SupportedOSPlatform("tvos11.0")] public override void ViewSafeAreaInsetsDidChange() diff --git a/src/Controls/src/Core/Handlers/Items/Android/SizedItemContentView.cs b/src/Controls/src/Core/Handlers/Items/Android/SizedItemContentView.cs index 196d1725900f..6f9278355833 100644 --- a/src/Controls/src/Core/Handlers/Items/Android/SizedItemContentView.cs +++ b/src/Controls/src/Core/Handlers/Items/Android/SizedItemContentView.cs @@ -25,8 +25,8 @@ protected override void OnMeasure(int widthMeasureSpec, int heightMeasureSpec) return; } - double targetWidth = _width(); - double targetHeight = _height(); + double targetWidth = NormalizeDimension(_width()); + double targetHeight = NormalizeDimension(_height()); if (!double.IsInfinity(targetWidth)) targetWidth = Context.FromPixels(targetWidth); @@ -48,5 +48,7 @@ protected override void OnMeasure(int widthMeasureSpec, int heightMeasureSpec) SetMeasuredDimension((int)size.Width, (int)size.Height); } } + + static double NormalizeDimension(double value) => value == int.MaxValue ? double.PositiveInfinity : value; } } diff --git a/src/Controls/src/Core/NavigationPage/NavigationPage.Legacy.cs b/src/Controls/src/Core/NavigationPage/NavigationPage.Legacy.cs index 8d474aa3c689..cbdb009baea8 100644 --- a/src/Controls/src/Core/NavigationPage/NavigationPage.Legacy.cs +++ b/src/Controls/src/Core/NavigationPage/NavigationPage.Legacy.cs @@ -254,6 +254,10 @@ void RemovePage(Page page) _removePageRequested?.Invoke(this, new NavigationRequestedEventArgs(page, true)); RemoveFromInnerChildren(page); + // Disconnect handlers for the removed non-visible page. + // Note: When the current page is removed, PopAsync() is called instead. + page?.DisconnectHandlers(); + if (RootPage == page) RootPage = (Page)InternalChildren.First(); } diff --git a/src/Controls/src/Core/NavigationPage/NavigationPage.cs b/src/Controls/src/Core/NavigationPage/NavigationPage.cs index 0cbc1846d917..f6b6390c51e6 100644 --- a/src/Controls/src/Core/NavigationPage/NavigationPage.cs +++ b/src/Controls/src/Core/NavigationPage/NavigationPage.cs @@ -933,6 +933,10 @@ protected override void OnRemovePage(Page page) Owner.NavigationType = NavigationType.Remove; Owner.RemoveFromInnerChildren(page); + // Disconnect handlers for the removed non-visible page. + // Note: When the current page is removed, PopAsync() is called instead. + page?.DisconnectHandlers(); + if (Owner.RootPage == page) Owner.RootPage = (Page)Owner.InternalChildren[0]; }, diff --git a/src/Controls/src/Core/NavigationProxy.cs b/src/Controls/src/Core/NavigationProxy.cs index 8e374446304d..5d2f4806d17c 100644 --- a/src/Controls/src/Core/NavigationProxy.cs +++ b/src/Controls/src/Core/NavigationProxy.cs @@ -252,7 +252,6 @@ protected virtual void OnRemovePage(Page page) { currentInner.RemovePage(page); } - page?.DisconnectHandlers(); } Page Pop() diff --git a/src/Controls/src/Core/PublicAPI/net-ios/PublicAPI.Unshipped.txt b/src/Controls/src/Core/PublicAPI/net-ios/PublicAPI.Unshipped.txt index 7d0110928035..0c300d7e5d2e 100644 --- a/src/Controls/src/Core/PublicAPI/net-ios/PublicAPI.Unshipped.txt +++ b/src/Controls/src/Core/PublicAPI/net-ios/PublicAPI.Unshipped.txt @@ -1,2 +1,4 @@ #nullable enable ~override Microsoft.Maui.Controls.Platform.Compatibility.ShellFlyoutRenderer.ViewWillTransitionToSize(CoreGraphics.CGSize toSize, UIKit.IUIViewControllerTransitionCoordinator coordinator) -> void +override Microsoft.Maui.Controls.Platform.Compatibility.ShellTableViewController.LoadView() -> void +*REMOVED*~override Microsoft.Maui.Controls.Platform.Compatibility.ShellSectionRootRenderer.TraitCollectionDidChange(UIKit.UITraitCollection previousTraitCollection) -> void diff --git a/src/Controls/src/Core/PublicAPI/net-maccatalyst/PublicAPI.Unshipped.txt b/src/Controls/src/Core/PublicAPI/net-maccatalyst/PublicAPI.Unshipped.txt index 7d0110928035..0c300d7e5d2e 100644 --- a/src/Controls/src/Core/PublicAPI/net-maccatalyst/PublicAPI.Unshipped.txt +++ b/src/Controls/src/Core/PublicAPI/net-maccatalyst/PublicAPI.Unshipped.txt @@ -1,2 +1,4 @@ #nullable enable ~override Microsoft.Maui.Controls.Platform.Compatibility.ShellFlyoutRenderer.ViewWillTransitionToSize(CoreGraphics.CGSize toSize, UIKit.IUIViewControllerTransitionCoordinator coordinator) -> void +override Microsoft.Maui.Controls.Platform.Compatibility.ShellTableViewController.LoadView() -> void +*REMOVED*~override Microsoft.Maui.Controls.Platform.Compatibility.ShellSectionRootRenderer.TraitCollectionDidChange(UIKit.UITraitCollection previousTraitCollection) -> void diff --git a/src/Controls/src/Core/Shell/ShellSection.cs b/src/Controls/src/Core/Shell/ShellSection.cs index 4fdd786025f2..6ffd91cae3c5 100644 --- a/src/Controls/src/Core/Shell/ShellSection.cs +++ b/src/Controls/src/Core/Shell/ShellSection.cs @@ -946,6 +946,7 @@ protected virtual void OnRemovePage(Page page) PresentedPageAppearing(); RemovePage(page); + page?.DisconnectHandlers(); var args = new NavigationRequestedEventArgs(page, false) { RequestType = NavigationRequestType.Remove diff --git a/src/Controls/src/SourceGen/AnalyzerConfigOptionsExtensions.cs b/src/Controls/src/SourceGen/AnalyzerConfigOptionsExtensions.cs index f73f5b3e4b2c..aa355948e434 100644 --- a/src/Controls/src/SourceGen/AnalyzerConfigOptionsExtensions.cs +++ b/src/Controls/src/SourceGen/AnalyzerConfigOptionsExtensions.cs @@ -22,7 +22,7 @@ public static string GetValueOrDefault(this AnalyzerConfigOptions options, strin public static T GetValueOrDefault(this AnalyzerConfigOptions options, string key, T defaultValue, Func parse) => options.TryGetValue(key, out var value) ? parse(value) : defaultValue; - + public static string? GetValueOrNull(this AnalyzerConfigOptions options, string key) => options.TryGetValue(key, out var value) ? value : null; } diff --git a/src/Controls/src/SourceGen/AnalyzerReleases.Unshipped.md b/src/Controls/src/SourceGen/AnalyzerReleases.Unshipped.md index 803e753dbe5c..ca72bb590721 100644 --- a/src/Controls/src/SourceGen/AnalyzerReleases.Unshipped.md +++ b/src/Controls/src/SourceGen/AnalyzerReleases.Unshipped.md @@ -16,3 +16,9 @@ MAUIX2003 | XamlInflation | Error | Descriptors MAUIX2004 | XamlInflation | Error | Descriptors MAUIX2005 | XamlInflation | Warning | Descriptors MAUIX2006 | XamlParsing | Warning | PropertyElementWithAttribute +MAUIG2024 | XamlInflation | Warning | BindingWithXDataTypeFromOuterScope +MAUIG2041 | XamlInflation | Error | BindingIndexerNotClosed +MAUIG2042 | XamlInflation | Error | BindingIndexerEmpty +MAUIG2043 | XamlInflation | Error | BindingIndexerTypeUnsupported +MAUIG2045 | XamlInflation | Warning | BindingPropertyNotFound +MAUIG2064 | XamlInflation | Error | NamescopeDuplicate diff --git a/src/Controls/src/SourceGen/CodeBehindCodeWriter.cs b/src/Controls/src/SourceGen/CodeBehindCodeWriter.cs index 980902cf90bf..505eca34ff2a 100644 --- a/src/Controls/src/SourceGen/CodeBehindCodeWriter.cs +++ b/src/Controls/src/SourceGen/CodeBehindCodeWriter.cs @@ -98,7 +98,7 @@ public static string GenerateXamlCodeBehind(XamlProjectItemForCB? xamlItem, Comp var generateInflatorSwitch = compilation.AssemblyName == "Microsoft.Maui.Controls.Xaml.UnitTests" && !generateDefaultCtor; var xamlInflators = projItem.Inflator; - + //if there's only the XamlC inflator, prevent non-assigned errors if (xamlInflators == XamlInflator.XamlC) sb.AppendLine("#pragma warning disable CS0649"); @@ -174,7 +174,8 @@ public static string GenerateXamlCodeBehind(XamlProjectItemForCB? xamlItem, Comp InitComp("InitializeComponent"); else if ((xamlInflators & XamlInflator.XamlC) == XamlInflator.XamlC) InitComp("InitializeComponent"); - else if ((xamlInflators & XamlInflator.SourceGen) == XamlInflator.SourceGen) { + else if ((xamlInflators & XamlInflator.SourceGen) == XamlInflator.SourceGen) + { InitComp("InitializeComponent", partialsignature: true); //generate InitCompRuntime for HotReload fallback if (projItem.EnableDiagnostics) @@ -199,7 +200,7 @@ void InitComp(string methodName, bool empty = false, bool partialsignature = fal if (namedFields != null && namedFields.Any()) { sb.AppendLine($"#if NET5_0_OR_GREATER"); - foreach ((var fname, _, _) in namedFields) + foreach ((var fname, _, _) in namedFields) sb.AppendLine($"\t\t[global::System.Diagnostics.CodeAnalysis.MemberNotNullAttribute(nameof({EscapeIdentifier(fname)}))]"); sb.AppendLine($"#endif"); diff --git a/src/Controls/src/SourceGen/CompilationReferencesComparer.cs b/src/Controls/src/SourceGen/CompilationReferencesComparer.cs index 224044f18c41..2c67bbbeaab6 100644 --- a/src/Controls/src/SourceGen/CompilationReferencesComparer.cs +++ b/src/Controls/src/SourceGen/CompilationReferencesComparer.cs @@ -90,21 +90,26 @@ private static void AppendType(StringBuilder sb, INamedTypeSymbol type) { case IFieldSymbol f: sb.Append(f.DeclaredAccessibility).Append(' '); - if (f.IsStatic) sb.Append("static "); + if (f.IsStatic) + sb.Append("static "); sb.Append(f.Type.ToFQDisplayString()).Append(' ').Append(f.Name).Append(';'); break; case IPropertySymbol p: sb.Append(p.DeclaredAccessibility).Append(' '); - if (p.IsStatic) sb.Append("static "); + if (p.IsStatic) + sb.Append("static "); sb.Append(p.Type.ToFQDisplayString()).Append(' ').Append(p.Name); - if (p.GetMethod != null) sb.Append("{get;}"); - if (p.SetMethod != null) sb.Append("{set;}"); + if (p.GetMethod != null) + sb.Append("{get;}"); + if (p.SetMethod != null) + sb.Append("{set;}"); break; case IMethodSymbol m when m.MethodKind == MethodKind.Ordinary: sb.Append(m.DeclaredAccessibility).Append(' '); - if (m.IsStatic) sb.Append("static "); + if (m.IsStatic) + sb.Append("static "); sb.Append(m.ReturnType.ToFQDisplayString()).Append(' ').Append(m.Name); sb.Append('('); sb.Append(string.Join(",", m.Parameters.Select(p => p.Type.ToFQDisplayString()))); diff --git a/src/Controls/src/SourceGen/CompiledBindingMarkup.cs b/src/Controls/src/SourceGen/CompiledBindingMarkup.cs index bc941fc0558e..a7e8bffd4c21 100644 --- a/src/Controls/src/SourceGen/CompiledBindingMarkup.cs +++ b/src/Controls/src/SourceGen/CompiledBindingMarkup.cs @@ -26,8 +26,8 @@ public CompiledBindingMarkup(ElementNode node, string path, ILocalValue bindingE _bindingExtension = bindingExtension; } - private Location GetLocation(INode node) - => LocationHelpers.LocationCreate(_context.ProjectItem.RelativePath!, (IXmlLineInfo)node, "x:DataType"); + private Location GetLocation(INode node, string? context = null) + => LocationHelpers.LocationCreate(_context.ProjectItem.RelativePath!, (IXmlLineInfo)node, context ?? "Binding"); public bool TryCompileBinding(ITypeSymbol sourceType, bool isTemplateBinding, out string? newBindingExpression) { @@ -60,7 +60,9 @@ public bool TryCompileBinding(ITypeSymbol sourceType, bool isTemplateBinding, ou ILocalValue extVariable; if (!_context.Variables.TryGetValue(_node, out extVariable)) { - throw new Exception("BindingExtension not found"); // TODO report diagnostic + // This should not happen normally, but report a diagnostic instead of throwing + _context.ReportDiagnostic(Diagnostic.Create(Descriptors.XamlParserError, GetLocation(_node), "BindingExtension not found")); + return false; } var methodName = $"CreateTypedBindingFrom_{_bindingExtension.ValueAccessor}"; @@ -73,7 +75,7 @@ public bool TryCompileBinding(ITypeSymbol sourceType, bool isTemplateBinding, ou // Determine which properties were set in XAML var propertyFlags = BindingPropertyFlags.None; - + if (_node.HasProperty("Mode")) propertyFlags |= BindingPropertyFlags.Mode; if (_node.HasProperty("Converter")) @@ -92,11 +94,11 @@ public bool TryCompileBinding(ITypeSymbol sourceType, bool isTemplateBinding, ou //Generate the complete inline binding creation method using var stringWriter = new StringWriter(); using var code = new IndentedTextWriter(stringWriter, "\t"); - + code.WriteLine($"static global::Microsoft.Maui.Controls.BindingBase {methodName}({extensionTypeName} extension)"); code.WriteLine("{"); code.Indent++; - + // Setter initialization // If we can determine the exact binding mode at compile time, we can avoid generating the setter or avoid the ShouldUseSetter method. // If we cannot, we need to generate a ShouldUseSetter helper method. @@ -161,7 +163,7 @@ public bool TryCompileBinding(ITypeSymbol sourceType, bool isTemplateBinding, ou code.Indent--; code.WriteLine("};"); } - + if (generateShouldUseSetter) { code.Indent--; @@ -174,7 +176,7 @@ public bool TryCompileBinding(ITypeSymbol sourceType, bool isTemplateBinding, ou { code.WriteLine("null;"); } - + // TypedBinding creation code.WriteLine($"return new global::Microsoft.Maui.Controls.Internals.TypedBinding<{binding.SourceType}, {binding.PropertyType}>("); code.Indent++; @@ -185,14 +187,14 @@ public bool TryCompileBinding(ITypeSymbol sourceType, bool isTemplateBinding, ou AppendHandlersArray(code, binding); code.Write(")"); code.Indent--; - + // Object initializer if any properties are set if (propertyFlags != BindingPropertyFlags.None) { code.WriteLine(); code.WriteLine("{"); code.Indent++; - + if (propertyFlags.HasFlag(BindingPropertyFlags.Mode)) code.WriteLine("Mode = extension.Mode,"); if (propertyFlags.HasFlag(BindingPropertyFlags.Converter)) @@ -222,7 +224,7 @@ public bool TryCompileBinding(ITypeSymbol sourceType, bool isTemplateBinding, ou else code.WriteLine("TargetNullValue = extension.TargetNullValue,"); } - + code.Indent--; code.WriteLine("};"); } @@ -285,20 +287,18 @@ bool TryParsePath( var rbIndex = p.IndexOf(']', lbIndex); if (rbIndex == -1) { - return false; // TODO report diagnostic + _context.ReportDiagnostic(Diagnostic.Create(Descriptors.BindingIndexerNotClosed, GetLocation(_node))); + return false; } var argLength = rbIndex - lbIndex - 1; if (argLength == 0) { - return false; // TODO report diagnostic + _context.ReportDiagnostic(Diagnostic.Create(Descriptors.BindingIndexerEmpty, GetLocation(_node))); + return false; } indexArg = p.Substring(lbIndex + 1, argLength); - if (indexArg.Length == 0) - { - return false; // TODO report diagnostic - } p = p.Substring(0, lbIndex); p = p.Trim(); @@ -310,7 +310,11 @@ bool TryParsePath( if (!previousPartType.TryGetProperty(p, _context, out var property, out var currentPropertyType) || currentPropertyType is null) { - return false; // TODO report diagnostic + // Report as WARNING (not Error) because the property might be generated by another source generator + // that runs after the MAUI XAML source generator. The binding will fall back to reflection at runtime. + // This allows scenarios like CommunityToolkit.Mvvm's [ObservableProperty] to work correctly. + _context.ReportDiagnostic(Diagnostic.Create(Descriptors.BindingPropertyNotFound, GetLocation(_node), p, previousPartType.ToFQDisplayString())); + return false; } // TODO: detect if the type is annotated or not @@ -346,65 +350,130 @@ bool TryParsePath( if (indexArg != null) { - var defaultMemberAttribute = previousPartType.GetAttributes().FirstOrDefault(a => a.AttributeClass?.ToFQDisplayString() == "global::System.Reflection.DefaultMemberAttribute"); - var indexerName = defaultMemberAttribute?.ConstructorArguments.FirstOrDefault().Value as string ?? "Item"; - index = indexArg; - IPropertySymbol? indexer = null; - if (int.TryParse(indexArg, out int indexArgInt)) + // For arrays, use the element type directly without trying to find an indexer + if (previousPartType is IArrayTypeSymbol arrayType) { + if (!int.TryParse(indexArg, out int indexArgInt)) + { + _context.ReportDiagnostic(Diagnostic.Create(Descriptors.BindingIndexerTypeUnsupported, GetLocation(_node), previousPartType.ToFQDisplayString())); + return false; + } index = indexArgInt; - indexer = previousPartType - .GetAllProperties(indexerName, _context) - .FirstOrDefault(property => - property.GetMethod != null - && !property.GetMethod.IsStatic - && property.Parameters.Length == 1 - && property.Parameters[0].Type.SpecialType == SpecialType.System_Int32); - } - indexer ??= previousPartType - .GetAllProperties(indexerName, _context) - .FirstOrDefault(property => - property.GetMethod != null - && !property.GetMethod.IsStatic - && property.Parameters.Length == 1 - && property.Parameters[0].Type.SpecialType == SpecialType.System_String); - - indexer ??= previousPartType - .GetAllProperties(indexerName, _context) - .FirstOrDefault(property => - property.GetMethod != null - && !property.GetMethod.IsStatic - && property.Parameters.Length == 1 - && property.Parameters[0].Type.SpecialType == SpecialType.System_Object); - - if (indexer is not null) - { - var indexAccess = new IndexAccess(indexerName, index, indexer.Type.IsValueType); - bindingPathParts.Add(indexAccess); - - previousPartType = indexer.Type; - - setterOptions = new SetterOptions( - IsWritable: indexer.SetMethod != null && indexer.SetMethod.IsPublic() && !indexer.SetMethod.IsStatic, - AcceptsNullValue: previousPartType.IsTypeNullable(enabledNullable: true)); - } - else if (previousPartType is IArrayTypeSymbol arrayType) - { - var indexAccess = new IndexAccess("", index, arrayType.ElementType.IsValueType); + IPathPart indexAccess = new IndexAccess("", index, arrayType.ElementType.IsValueType); + if (previousPartIsNullable) + { + indexAccess = new ConditionalAccess(indexAccess); + } bindingPathParts.Add(indexAccess); previousPartType = arrayType.ElementType; setterOptions = new SetterOptions( - IsWritable: true, // TODO is this correct? + IsWritable: true, // arrays are writable by index AcceptsNullValue: previousPartType.IsTypeNullable(enabledNullable: true)); } else { - return false; // TODO report diagnostic + // For constructed generic types, we need to check the OriginalDefinition for the DefaultMemberAttribute + var typeToCheckForAttribute = previousPartType is INamedTypeSymbol namedType && namedType.IsGenericType + ? namedType.OriginalDefinition + : previousPartType; + var defaultMemberAttribute = typeToCheckForAttribute.GetAttributes().FirstOrDefault(a => a.AttributeClass?.ToFQDisplayString() == "global::System.Reflection.DefaultMemberAttribute"); + var indexerName = defaultMemberAttribute?.ConstructorArguments.FirstOrDefault().Value as string ?? "Item"; + + // For indexer lookup, we also need to search properties that are indexers (have parameters) + // The indexer may be declared directly on the type or inherited from interfaces + IPropertySymbol? indexer = null; + if (int.TryParse(indexArg, out int indexArgInt)) + { + index = indexArgInt; + indexer = previousPartType + .GetAllProperties(indexerName, _context) + .FirstOrDefault(property => + property.GetMethod != null + && !property.GetMethod.IsStatic + && property.Parameters.Length == 1 + && property.Parameters[0].Type.SpecialType == SpecialType.System_Int32); + + // If not found by name, try to find any indexer with int parameter (for cases where DefaultMemberAttribute is missing or different) + // But prefer indexers that are not from non-generic interfaces like IList (which returns object) + indexer ??= previousPartType + .GetAllProperties(_context) + .Where(property => + property.IsIndexer + && property.GetMethod != null + && !property.GetMethod.IsStatic + && property.Parameters.Length == 1 + && property.Parameters[0].Type.SpecialType == SpecialType.System_Int32) + .OrderByDescending(p => p.Type.SpecialType != SpecialType.System_Object) // Prefer non-object return types + .FirstOrDefault(); + } + + indexer ??= previousPartType + .GetAllProperties(indexerName, _context) + .FirstOrDefault(property => + property.GetMethod != null + && !property.GetMethod.IsStatic + && property.Parameters.Length == 1 + && property.Parameters[0].Type.SpecialType == SpecialType.System_String); + + // Fallback: try to find any indexer with string parameter, preferring non-object return types + indexer ??= previousPartType + .GetAllProperties(_context) + .Where(property => + property.IsIndexer + && property.GetMethod != null + && !property.GetMethod.IsStatic + && property.Parameters.Length == 1 + && property.Parameters[0].Type.SpecialType == SpecialType.System_String) + .OrderByDescending(p => p.Type.SpecialType != SpecialType.System_Object) + .FirstOrDefault(); + + indexer ??= previousPartType + .GetAllProperties(indexerName, _context) + .FirstOrDefault(property => + property.GetMethod != null + && !property.GetMethod.IsStatic + && property.Parameters.Length == 1 + && property.Parameters[0].Type.SpecialType == SpecialType.System_Object); + + // Fallback: try to find any indexer with object parameter + indexer ??= previousPartType + .GetAllProperties(_context) + .FirstOrDefault(property => + property.IsIndexer + && property.GetMethod != null + && !property.GetMethod.IsStatic + && property.Parameters.Length == 1 + && property.Parameters[0].Type.SpecialType == SpecialType.System_Object); + + if (indexer is not null) + { + // Use MetadataName because for indexers with [IndexerName("CustomName")], the + // Name property is "this[]" but MetadataName is "CustomName" which is what + // PropertyChanged events use (e.g., "CustomName[3]" not "this[][3]") + var actualIndexerName = indexer.MetadataName; + IPathPart indexAccess = new IndexAccess(actualIndexerName, index, indexer.Type.IsValueType); + if (previousPartIsNullable) + { + indexAccess = new ConditionalAccess(indexAccess); + } + bindingPathParts.Add(indexAccess); + + previousPartType = indexer.Type; + + setterOptions = new SetterOptions( + IsWritable: indexer.SetMethod != null && indexer.SetMethod.IsPublic() && !indexer.SetMethod.IsStatic, + AcceptsNullValue: previousPartType.IsTypeNullable(enabledNullable: true)); + } + else + { + _context.ReportDiagnostic(Diagnostic.Create(Descriptors.BindingIndexerTypeUnsupported, GetLocation(_node), previousPartType.ToFQDisplayString())); + return false; + } } } } @@ -514,7 +583,7 @@ void AppendSetterLambda(IndentedTextWriter code, BindingInvocationDescription bi if (setter.PatternMatchingExpressions.Length > 0) { code.Write("if ("); - + for (int i = 0; i < setter.PatternMatchingExpressions.Length; i++) { if (i > 0) @@ -524,7 +593,7 @@ void AppendSetterLambda(IndentedTextWriter code, BindingInvocationDescription bi } code.Write(setter.PatternMatchingExpressions[i]); } - + code.WriteLine(")"); code.WriteLine("{"); code.Indent++; @@ -536,7 +605,7 @@ void AppendSetterLambda(IndentedTextWriter code, BindingInvocationDescription bi { code.WriteLine(setter.AssignmentStatement); } - + code.Indent--; code.WriteLine("};"); } diff --git a/src/Controls/src/SourceGen/Descriptors.cs b/src/Controls/src/SourceGen/Descriptors.cs index f7748dd5fb5e..82c75d7b7af0 100644 --- a/src/Controls/src/SourceGen/Descriptors.cs +++ b/src/Controls/src/SourceGen/Descriptors.cs @@ -186,7 +186,7 @@ public static class Descriptors defaultSeverity: DiagnosticSeverity.Error, isEnabledByDefault: true); - public static DiagnosticDescriptor RequiredProperty = new DiagnosticDescriptor( + public static DiagnosticDescriptor RequiredProperty = new DiagnosticDescriptor( id: "MAUIX2005", title: new LocalizableResourceString(nameof(MauiGResources.RequiredPropertyTitle), MauiGResources.ResourceManager, typeof(MauiGResources)), messageFormat: new LocalizableResourceString(nameof(MauiGResources.RequiredPropertyMessage), MauiGResources.ResourceManager, typeof(MauiGResources)), @@ -194,6 +194,64 @@ public static class Descriptors defaultSeverity: DiagnosticSeverity.Warning, isEnabledByDefault: true); + // Binding-related diagnostics (MAUIG2041-MAUIG2045 matching XC0041-XC0045) + public static DiagnosticDescriptor BindingIndexerNotClosed = new DiagnosticDescriptor( + id: "MAUIG2041", + title: new LocalizableResourceString(nameof(MauiGResources.BindingCompilationFailed), MauiGResources.ResourceManager, typeof(MauiGResources)), + messageFormat: new LocalizableResourceString(nameof(MauiGResources.BindingIndexerNotClosed), MauiGResources.ResourceManager, typeof(MauiGResources)), + category: "XamlInflation", + defaultSeverity: DiagnosticSeverity.Error, + isEnabledByDefault: true); + + public static DiagnosticDescriptor BindingIndexerEmpty = new DiagnosticDescriptor( + id: "MAUIG2042", + title: new LocalizableResourceString(nameof(MauiGResources.BindingCompilationFailed), MauiGResources.ResourceManager, typeof(MauiGResources)), + messageFormat: new LocalizableResourceString(nameof(MauiGResources.BindingIndexerEmpty), MauiGResources.ResourceManager, typeof(MauiGResources)), + category: "XamlInflation", + defaultSeverity: DiagnosticSeverity.Error, + isEnabledByDefault: true); + + public static DiagnosticDescriptor BindingIndexerTypeUnsupported = new DiagnosticDescriptor( + id: "MAUIG2043", + title: new LocalizableResourceString(nameof(MauiGResources.BindingCompilationFailed), MauiGResources.ResourceManager, typeof(MauiGResources)), + messageFormat: new LocalizableResourceString(nameof(MauiGResources.BindingIndexerTypeUnsupported), MauiGResources.ResourceManager, typeof(MauiGResources)), + category: "XamlInflation", + defaultSeverity: DiagnosticSeverity.Error, + isEnabledByDefault: true); + + // MAUIG2045: BindingPropertyNotFound + // This is a WARNING (not Error) because the MAUI source generator cannot always see properties + // that are generated by other source generators (e.g., CommunityToolkit.Mvvm's [ObservableProperty]). + // While this might indicate a typo, it could also be a valid property that will exist at runtime. + // The binding will fall back to slower reflection-based binding, which will work if the property exists. + // See: https://github.com/dotnet/maui/issues/[issue-number] for the source generator visibility limitation. + public static DiagnosticDescriptor BindingPropertyNotFound = new DiagnosticDescriptor( + id: "MAUIG2045", + title: new LocalizableResourceString(nameof(MauiGResources.BindingCompilationWarning), MauiGResources.ResourceManager, typeof(MauiGResources)), + messageFormat: new LocalizableResourceString(nameof(MauiGResources.BindingPropertyNotFound), MauiGResources.ResourceManager, typeof(MauiGResources)), + category: "XamlInflation", + defaultSeverity: DiagnosticSeverity.Warning, + isEnabledByDefault: true); + + // Binding warnings (MAUIG2024 matching XC0024) + public static DiagnosticDescriptor BindingWithXDataTypeFromOuterScope = new DiagnosticDescriptor( + id: "MAUIG2024", + title: new LocalizableResourceString(nameof(MauiGResources.BindingCompilationWarning), MauiGResources.ResourceManager, typeof(MauiGResources)), + messageFormat: new LocalizableResourceString(nameof(MauiGResources.BindingWithXDataTypeFromOuterScope), MauiGResources.ResourceManager, typeof(MauiGResources)), + category: "XamlInflation", + defaultSeverity: DiagnosticSeverity.Warning, + isEnabledByDefault: true, + helpLinkUri: "https://learn.microsoft.com/dotnet/maui/fundamentals/data-binding/compiled-bindings"); + + // XAML issues (MAUIG2064 matching XC0064) + public static DiagnosticDescriptor NamescopeDuplicate = new DiagnosticDescriptor( + id: "MAUIG2064", + title: new LocalizableResourceString(nameof(MauiGResources.NamescopeError), MauiGResources.ResourceManager, typeof(MauiGResources)), + messageFormat: new LocalizableResourceString(nameof(MauiGResources.NamescopeDuplicate), MauiGResources.ResourceManager, typeof(MauiGResources)), + category: "XamlInflation", + defaultSeverity: DiagnosticSeverity.Error, + isEnabledByDefault: true); + public static DiagnosticDescriptor PropertyElementWithAttribute = new DiagnosticDescriptor( id: "MAUIX2006", title: "Property element has attributes", diff --git a/src/Controls/src/SourceGen/GeneratorHelpers.cs b/src/Controls/src/SourceGen/GeneratorHelpers.cs index b9aded9ba653..687d0711fe63 100644 --- a/src/Controls/src/SourceGen/GeneratorHelpers.cs +++ b/src/Controls/src/SourceGen/GeneratorHelpers.cs @@ -45,7 +45,7 @@ public static string EscapeIdentifier(string identifier) return projectItem.Kind == "None" ? null : projectItem; } - public static XamlProjectItemForIC? ComputeXamlProjectItemForIC((ProjectItem?, AssemblyAttributes) itemAndCaches, CancellationToken cancellationToken) + public static XamlProjectItemForIC? ComputeXamlProjectItemForIC((ProjectItem?, AssemblyAttributes) itemAndCaches, CancellationToken cancellationToken) { var (projectItem, assemblyCaches) = itemAndCaches; var text = projectItem?.AdditionalText.GetText(cancellationToken); @@ -204,9 +204,9 @@ public static AssemblyAttributes GetAssemblyAttributes(Compilation compilation, INamedTypeSymbol? allowImplicitXmlnsAttribute = compilation.GetTypesByMetadataName(typeof(Xaml.Internals.AllowImplicitXmlnsDeclarationAttribute).FullName) .FirstOrDefault(t => t.ContainingAssembly.Identity.Name == "Microsoft.Maui.Controls"); - if (xmlnsDefinitonAttribute is null || internalsVisibleToAttribute is null) + if (xmlnsDefinitonAttribute is null || internalsVisibleToAttribute is null) return AssemblyAttributes.Empty; - + var xmlnsDefinitions = new List(); var internalsVisible = new List(); @@ -389,18 +389,38 @@ public static (string? source, XamlProjectItemForCB? xamlItem, IList reportDiagnostic(Diagnostic.Create(Descriptors.XamlParserError, location, e.Message)); } return (code, xamlItem, diagnostics); - } + } /// /// Formats a value as a culture-independent C# literal for source generation. - /// Uses SymbolDisplay.FormatPrimitive to ensure proper handling of special values like NaN and Infinity - /// but also numeric types and makes sure they are formatted correctly. + /// Handles special floating-point values (NaN, Infinity) and uses SymbolDisplay.FormatPrimitive + /// for regular numeric types to ensure they are formatted correctly. /// /// The value to format /// Whether to include quotes around the formatted value /// A culture-independent string representation suitable for source generation public static string FormatInvariant(object value, bool quoted = false) { + // Handle special floating-point values that SymbolDisplay.FormatPrimitive doesn't prefix correctly + if (value is double d) + { + if (double.IsNaN(d)) + return "double.NaN"; + if (double.IsPositiveInfinity(d)) + return "double.PositiveInfinity"; + if (double.IsNegativeInfinity(d)) + return "double.NegativeInfinity"; + } + else if (value is float f) + { + if (float.IsNaN(f)) + return "float.NaN"; + if (float.IsPositiveInfinity(f)) + return "float.PositiveInfinity"; + if (float.IsNegativeInfinity(f)) + return "float.NegativeInfinity"; + } + return SymbolDisplay.FormatPrimitive(value, quoteStrings: quoted, useHexadecimalNumbers: false); } diff --git a/src/Controls/src/SourceGen/IMethodSymbolExtensions.cs b/src/Controls/src/SourceGen/IMethodSymbolExtensions.cs index e63a194f18f1..5dee60ac8dfd 100644 --- a/src/Controls/src/SourceGen/IMethodSymbolExtensions.cs +++ b/src/Controls/src/SourceGen/IMethodSymbolExtensions.cs @@ -39,7 +39,7 @@ public static bool MatchXArguments(this IMethodSymbol method, ElementNode enode, // } var argType = getNodeValue?.Invoke(nodeparameters[i], paramType)?.Type ?? context.Variables[nodeparameters[i]].Type; - + // Check interface implementation first (interfaces don't use inheritance) if (paramType.IsInterface()) { @@ -55,7 +55,7 @@ public static bool MatchXArguments(this IMethodSymbol method, ElementNode enode, parameters = null; return false; } - + parameters.Add((nodeparameters[i], paramType, null)); } return true; diff --git a/src/Controls/src/SourceGen/ITypeSymbolExtensions.cs b/src/Controls/src/SourceGen/ITypeSymbolExtensions.cs index 72c075f4b5c0..6067880c34ad 100644 --- a/src/Controls/src/SourceGen/ITypeSymbolExtensions.cs +++ b/src/Controls/src/SourceGen/ITypeSymbolExtensions.cs @@ -2,10 +2,9 @@ using System.Collections.Generic; using System.Collections.Immutable; using System.Linq; - using Microsoft.CodeAnalysis; -using Microsoft.Maui.Controls.Xaml; using Microsoft.Maui.Controls.BindingSourceGen; +using Microsoft.Maui.Controls.Xaml; namespace Microsoft.Maui.Controls.SourceGen; @@ -85,15 +84,15 @@ public static IEnumerable GetAllProperties(this ITypeSymbol sym => symbol.GetAllMembers(name, context).OfType(); /// - /// Tries to get a property by name, and if not found, checks if it could be inferred from a RelayCommand method. + /// Tries to get a property by name, and if not found, checks if it could be inferred from a RelayCommand method or ObservableProperty field. /// Returns the property type if found or inferred. /// /// The type to search /// The name of the property to find /// The source generation context - /// The found property symbol (null if inferred from RelayCommand) - /// The property type (either from the property or inferred from RelayCommand) - /// True if property exists or can be inferred from RelayCommand + /// The found property symbol (null if inferred from RelayCommand or ObservableProperty) + /// The property type (either from the property or inferred from RelayCommand/ObservableProperty) + /// True if property exists or can be inferred from RelayCommand or ObservableProperty public static bool TryGetProperty( this ITypeSymbol symbol, string propertyName, @@ -103,7 +102,7 @@ public static bool TryGetProperty( { property = symbol.GetAllProperties(propertyName, context) .FirstOrDefault(p => p.GetMethod != null && !p.GetMethod.IsStatic); - + if (property != null) { propertyType = property.Type; @@ -117,6 +116,13 @@ public static bool TryGetProperty( return true; } + // If property not found, check if it could be an ObservableProperty-generated property + // Call the BindingSourceGen extension method directly + if (symbol.TryGetObservablePropertyType(propertyName, context?.Compilation, out propertyType)) + { + return true; + } + propertyType = null; return false; } diff --git a/src/Controls/src/SourceGen/InitializeComponentCodeWriter.cs b/src/Controls/src/SourceGen/InitializeComponentCodeWriter.cs index cb225e2837e9..db94769a8e97 100644 --- a/src/Controls/src/SourceGen/InitializeComponentCodeWriter.cs +++ b/src/Controls/src/SourceGen/InitializeComponentCodeWriter.cs @@ -3,10 +3,10 @@ using System.Collections.Generic; using System.Globalization; using System.IO; +using System.Linq; using System.Runtime.InteropServices; using System.Xml; using Microsoft.CodeAnalysis; -using System.Linq; using Microsoft.Maui.Controls.Xaml; namespace Microsoft.Maui.Controls.SourceGen; @@ -102,7 +102,7 @@ PrePost newblock() => using (newblock()) { if (xamlItem.ProjectItem.EnableDiagnostics) - { + { codeWriter.WriteLine( $$""" // Fallback to Runtime inflation if the page was updated by HotReload diff --git a/src/Controls/src/SourceGen/KnownMarkups.cs b/src/Controls/src/SourceGen/KnownMarkups.cs index fc4dee30ed1b..4b01d252e166 100644 --- a/src/Controls/src/SourceGen/KnownMarkups.cs +++ b/src/Controls/src/SourceGen/KnownMarkups.cs @@ -446,10 +446,8 @@ static bool TryGetXDataType(ElementNode node, SourceGenContext context, out ITyp if (xDataTypeIsInOuterScope) { - // TODO - context.ReportDiagnostic(Diagnostic.Create(Descriptors.XamlParserError, location, "Binding with x:DataType from outer scope")); - // _context.LoggingHelper.LogWarningOrError(BuildExceptionCode.BindingWithXDataTypeFromOuterScope, context.XamlFilePath, node.LineNumber, node.LinePosition, 0, 0, null); - // continue compilation + context.ReportDiagnostic(Diagnostic.Create(Descriptors.BindingWithXDataTypeFromOuterScope, location)); + // continue compilation - this is a warning } if (dataTypeNode.RepresentsType(XamlParser.X2009Uri, "NullExtension")) diff --git a/src/Controls/src/SourceGen/MauiGResources.resx b/src/Controls/src/SourceGen/MauiGResources.resx index d3ff7b8a3a51..9e1ff7463dcb 100644 --- a/src/Controls/src/SourceGen/MauiGResources.resx +++ b/src/Controls/src/SourceGen/MauiGResources.resx @@ -203,4 +203,34 @@ {0} has the 'required' keyword. Support for this in XAML is limited at the moment, avoid using it. We're still doing a best effort to provide a value. 0 is a property identifier + + Binding compilation failed + + + Binding compilation warning + + + Binding: Indexer did not contain closing bracket. + + + Binding: Indexer did not contain arguments. + + + Binding: Unsupported indexer index type: "{0}". + 0 is indexer type name + + + Binding: Property "{0}" not found on "{1}". The binding will use slower reflection-based binding at runtime. If this property is generated by another source generator, consider reporting this at https://github.com/dotnet/maui/issues to help us improve source generator interoperability. + 0 is property name, 1 is type name + + + Binding might be compiled incorrectly since the x:DataType annotation comes from an outer scope. Make sure you annotate all DataTemplate XAML elements with the correct x:DataType. See https://learn.microsoft.com/dotnet/maui/fundamentals/data-binding/compiled-bindings for more information. + + + Namescope error + + + An element with the name "{0}" already exists in this NameScope. + 0 is the duplicated key + diff --git a/src/Controls/src/SourceGen/NamingHelpers.cs b/src/Controls/src/SourceGen/NamingHelpers.cs index fbd2c7827c46..93e8b338223b 100644 --- a/src/Controls/src/SourceGen/NamingHelpers.cs +++ b/src/Controls/src/SourceGen/NamingHelpers.cs @@ -20,7 +20,7 @@ public static string CreateUniqueTypeName(SourceGenContext context, string baseN { while (context.ParentContext != null) context = context.ParentContext; - + return CreateUniqueVariableNameImpl(context, baseName, lowFirst: false); } @@ -71,7 +71,7 @@ static string CamelCase(string name, bool lowFirst) if (string.IsNullOrEmpty(name)) return name; name = Regex.Replace(name, "([A-Z])([A-Z]+)($|[A-Z])", m => m.Groups[1].Value + m.Groups[2].Value.ToLowerInvariant() + m.Groups[3].Value); - + return lowFirst ? char.ToLowerInvariant(name[0]) + name.Substring(1) : name; } } \ No newline at end of file diff --git a/src/Controls/src/SourceGen/NodeSGExtensions.cs b/src/Controls/src/SourceGen/NodeSGExtensions.cs index 034c23953fa8..ef32f34549ef 100644 --- a/src/Controls/src/SourceGen/NodeSGExtensions.cs +++ b/src/Controls/src/SourceGen/NodeSGExtensions.cs @@ -8,8 +8,8 @@ using System.Xml; using Microsoft.CodeAnalysis; using Microsoft.CodeAnalysis.CSharp; -using Microsoft.Maui.Controls.Xaml; using Microsoft.Maui.Controls.SourceGen.TypeConverters; +using Microsoft.Maui.Controls.Xaml; namespace Microsoft.Maui.Controls.SourceGen; @@ -24,7 +24,7 @@ static class NodeSGExtensions // Lazy converter factory function static ConverterDelegate CreateLazyConverter() where T : ISGTypeConverter, new() => - (value, node, toType, writer, context, parentVar) => + (value, node, toType, writer, context, parentVar) => lazyConverters.GetOrAdd(typeof(T), _ => new T()).Convert(value, node, toType, writer, context, parentVar); static readonly ConcurrentDictionary lazyConverters = new(); @@ -32,7 +32,7 @@ static class NodeSGExtensions // Lazy registry-based converter function (for non-source-gen converters) static ConverterDelegate CreateLazyRegistryConverter(string typeName) => - (value, node, toType, writer, context, parentVar) => + (value, node, toType, writer, context, parentVar) => { var converter = lazyRegistryConverters.GetOrAdd(typeName, name => TypeConverterRegistry.GetConverter(name)!); return converter?.Convert(value, node, toType, writer, context, parentVar) ?? "default"; @@ -172,7 +172,7 @@ public static bool CanConvertTo(this ValueNode valueNode, ITypeSymbol toType, IT if (converter is not null && stringValue is not null) return true; - if ( toType.NullableAnnotation == NullableAnnotation.Annotated + if (toType.NullableAnnotation == NullableAnnotation.Annotated && toType.SpecialType == SpecialType.None && ((INamedTypeSymbol)toType).TypeArguments.Length == 1) { @@ -277,7 +277,7 @@ void reportDiagnostic() => context.ReportDiagnostic(Diagnostic.Create(Descriptors.ConversionFailed, LocationHelpers.LocationCreate(context.ProjectItem.RelativePath!, lineInfo, valueString), valueString, toType.ToDisplayString())); #pragma warning restore RS0030 // Do not use banned APIs - if ( toType.NullableAnnotation == NullableAnnotation.Annotated + if (toType.NullableAnnotation == NullableAnnotation.Annotated && toType.SpecialType == SpecialType.None && ((INamedTypeSymbol)toType).TypeArguments.Length == 1) { @@ -498,7 +498,7 @@ public static bool IsValueProvider(this INode node, SourceGenContext context, if (!context.Variables.TryGetValue(node, out var variable)) return false; - return variable.Type.IsValueProvider( context, out returnType, out iface, out acceptEmptyServiceProvider, out requiredServices); + return variable.Type.IsValueProvider(context, out returnType, out iface, out acceptEmptyServiceProvider, out requiredServices); } @@ -598,7 +598,7 @@ public static (IFieldSymbol?, IPropertySymbol?) GetFieldOrBP(ITypeSymbol owner, property = null; return (bpFieldSymbol, property); } - + public static IFieldSymbol GetBindableProperty(this ValueNode node, SourceGenContext context) { static ITypeSymbol? GetTargetTypeSymbol(INode node, SourceGenContext context) @@ -654,8 +654,8 @@ public static IFieldSymbol GetBindableProperty(this ValueNode node, SourceGenCon throw new Exception($"Expected VisualStateGroup but found {parent.Parent}"); //3. if the VSG is in a VSGL, skip that as it could be implicit - if ( target.Parent is ListNode - || (target.Parent as ElementNode)!.XmlType!.IsOfAnyType( "VisualStateGroupList")) + if (target.Parent is ListNode + || (target.Parent as ElementNode)!.XmlType!.IsOfAnyType("VisualStateGroupList")) target = (ElementNode)target.Parent.Parent; else target = (ElementNode)target.Parent; diff --git a/src/Controls/src/SourceGen/PrePost.cs b/src/Controls/src/SourceGen/PrePost.cs index d28aef7b36d2..16604ff72b47 100644 --- a/src/Controls/src/SourceGen/PrePost.cs +++ b/src/Controls/src/SourceGen/PrePost.cs @@ -48,7 +48,7 @@ public static PrePost NewConditional(IndentedTextWriter codeWriter, string condi public static PrePost NewDisableWarning(IndentedTextWriter codeWriter, string warning) => new(() => codeWriter.WriteLineNoTabs($"#pragma warning disable {warning}"), () => codeWriter.WriteLineNoTabs($"#pragma warning restore {warning}")); - + readonly Action post; PrePost(Action pre, Action post) { diff --git a/src/Controls/src/SourceGen/ServiceProviderExtensions.cs b/src/Controls/src/SourceGen/ServiceProviderExtensions.cs index 0503af9f66f2..c0283b895b00 100644 --- a/src/Controls/src/SourceGen/ServiceProviderExtensions.cs +++ b/src/Controls/src/SourceGen/ServiceProviderExtensions.cs @@ -44,18 +44,18 @@ static void AddServices(this INode node, IndentedTextWriter writer, string servi var parentObjects = node.ParentObjects(context).Select(v => v.ValueAccessor).ToArray(); if (parentObjects.Length == 0) parentObjects = ["null"]; - if ( createAllServices + if (createAllServices || requiredServices!.Value.Contains(context.Compilation.GetTypeByMetadataName("Microsoft.Maui.Controls.Xaml.IProvideParentValues")!, SymbolEqualityComparer.Default) || requiredServices!.Value.Contains(context.Compilation.GetTypeByMetadataName("Microsoft.Maui.Controls.Xaml.IProvideValueTarget")!, SymbolEqualityComparer.Default) || requiredServices!.Value.Contains(context.Compilation.GetTypeByMetadataName("Microsoft.Maui.Controls.Xaml.IReferenceProvider")!, SymbolEqualityComparer.Default)) - { - var simpleValueTargetProvider = NamingHelpers.CreateUniqueVariableName(context, context.Compilation.GetTypeByMetadataName("Microsoft.Maui.Controls.Xaml.IProvideValueTarget")!); - writer.WriteLine($"var {simpleValueTargetProvider} = new global::Microsoft.Maui.Controls.Xaml.Internals.SimpleValueTargetProvider("); - writer.Indent++; - writer.WriteLine($"new object?[] {{{string.Join(", ", parentObjects)}}},"); - var bpinfo = bpFieldSymbol?.ToFQDisplayString() ?? String.Empty; - var pinfo = $"typeof({propertySymbol?.ContainingSymbol.ToFQDisplayString()}).GetProperty(\"{propertySymbol?.Name}\")" ?? string.Empty; - writer.WriteLine($"{(bpFieldSymbol != null ? bpinfo : propertySymbol != null ? pinfo : "null")},"); + { + var simpleValueTargetProvider = NamingHelpers.CreateUniqueVariableName(context, context.Compilation.GetTypeByMetadataName("Microsoft.Maui.Controls.Xaml.IProvideValueTarget")!); + writer.WriteLine($"var {simpleValueTargetProvider} = new global::Microsoft.Maui.Controls.Xaml.Internals.SimpleValueTargetProvider("); + writer.Indent++; + writer.WriteLine($"new object?[] {{{string.Join(", ", parentObjects)}}},"); + var bpinfo = bpFieldSymbol?.ToFQDisplayString() ?? String.Empty; + var pinfo = $"typeof({propertySymbol?.ContainingSymbol.ToFQDisplayString()}).GetProperty(\"{propertySymbol?.Name}\")" ?? string.Empty; + writer.WriteLine($"{(bpFieldSymbol != null ? bpinfo : propertySymbol != null ? pinfo : "null")},"); if (context.Scopes.TryGetValue(node, out var scope)) { List scopes = [scope.namescope.ValueAccessor]; @@ -68,15 +68,15 @@ static void AddServices(this INode node, IndentedTextWriter writer, string servi } else writer.WriteLine($"null,"); - - var rootVar = context.Variables.FirstOrDefault(kvp => kvp.Key is SGRootNode).Value; - writer.WriteLine($"{rootVar?.ValueAccessor ?? "this"});"); - writer.Indent--; - writer.WriteLine($"{serviceProviderVariableName}.Add(typeof(global::Microsoft.Maui.Controls.Xaml.IReferenceProvider), {simpleValueTargetProvider});"); - writer.WriteLine($"{serviceProviderVariableName}.Add(typeof(global::Microsoft.Maui.Controls.Xaml.IProvideValueTarget), {simpleValueTargetProvider});"); - } - if ( createAllServices + var rootVar = context.Variables.FirstOrDefault(kvp => kvp.Key is SGRootNode).Value; + writer.WriteLine($"{rootVar?.ValueAccessor ?? "this"});"); + writer.Indent--; + writer.WriteLine($"{serviceProviderVariableName}.Add(typeof(global::Microsoft.Maui.Controls.Xaml.IReferenceProvider), {simpleValueTargetProvider});"); + writer.WriteLine($"{serviceProviderVariableName}.Add(typeof(global::Microsoft.Maui.Controls.Xaml.IProvideValueTarget), {simpleValueTargetProvider});"); + } + + if (createAllServices || requiredServices!.Value.Contains(context.Compilation.GetTypeByMetadataName("Microsoft.Maui.Controls.Xaml.IXamlTypeResolver")!, SymbolEqualityComparer.Default)) { var nsResolver = NamingHelpers.CreateUniqueVariableName(context, context.Compilation.GetTypeByMetadataName("Microsoft.Maui.Controls.Xaml.Internals.XmlNamespaceResolver")!); @@ -87,9 +87,9 @@ static void AddServices(this INode node, IndentedTextWriter writer, string servi writer.WriteLine($"{serviceProviderVariableName}.Add(typeof(global::Microsoft.Maui.Controls.Xaml.IXamlTypeResolver), new global::Microsoft.Maui.Controls.Xaml.Internals.XamlTypeResolver({nsResolver}, typeof({context.RootType.ToFQDisplayString()}).Assembly));"); } - if ( node is IXmlLineInfo xmlLineInfo - && ( createAllServices - || requiredServices!.Value.Contains(context.Compilation.GetTypeByMetadataName("Microsoft.Maui.Controls.Xaml.IXmlLineInfoProvider")!, SymbolEqualityComparer.Default))) + if (node is IXmlLineInfo xmlLineInfo + && (createAllServices + || requiredServices!.Value.Contains(context.Compilation.GetTypeByMetadataName("Microsoft.Maui.Controls.Xaml.IXmlLineInfoProvider")!, SymbolEqualityComparer.Default))) { writer.WriteLine($"{serviceProviderVariableName}.Add(typeof(global::Microsoft.Maui.Controls.Xaml.IXmlLineInfoProvider), new global::Microsoft.Maui.Controls.Xaml.Internals.XmlLineInfoProvider(new global::Microsoft.Maui.Controls.Xaml.XmlLineInfo({xmlLineInfo.LineNumber}, {xmlLineInfo.LinePosition})));"); diff --git a/src/Controls/src/SourceGen/SetterValueProvider.cs b/src/Controls/src/SourceGen/SetterValueProvider.cs index 7171e5e27ded..c17976218605 100644 --- a/src/Controls/src/SourceGen/SetterValueProvider.cs +++ b/src/Controls/src/SourceGen/SetterValueProvider.cs @@ -12,7 +12,7 @@ public bool CanProvideValue(ElementNode node, SourceGenContext context) { // Can only inline if all properties are simple ValueNodes (no markup extensions) // We need to check both the properties and any collection items - + // Get the value node (shared logic with TryProvideValue) var valueNode = GetValueNode(node); diff --git a/src/Controls/src/SourceGen/SourceGenContext.cs b/src/Controls/src/SourceGen/SourceGenContext.cs index 89c679f5d10c..58ed48d8112d 100644 --- a/src/Controls/src/SourceGen/SourceGenContext.cs +++ b/src/Controls/src/SourceGen/SourceGenContext.cs @@ -23,7 +23,7 @@ class SourceGenContext(IndentedTextWriter writer, Compilation compilation, Sourc public SourceProductionContext SourceProductionContext => sourceProductionContext; public IndentedTextWriter Writer => writer; - + public IndentedTextWriter? RefStructWriter { get; set; } public Compilation Compilation => compilation; @@ -32,7 +32,23 @@ class SourceGenContext(IndentedTextWriter writer, Compilation compilation, Sourc public IDictionary TypeCache => typeCache; public IDictionary Values { get; } = new Dictionary(); public IDictionary Variables { get; } = new Dictionary(); - public void ReportDiagnostic(Diagnostic diagnostic) => sourceProductionContext.ReportDiagnostic(diagnostic); + public void ReportDiagnostic(Diagnostic diagnostic) + { + // Check if this diagnostic should be suppressed based on NoWarn + var noWarn = ProjectItem?.NoWarn; + if (!string.IsNullOrEmpty(noWarn)) + { + var suppressedIds = noWarn!.Split(new[] { ',', ';' }, StringSplitOptions.RemoveEmptyEntries); + foreach (var id in suppressedIds) + { + if (diagnostic.Id.Equals(id.Trim(), StringComparison.OrdinalIgnoreCase)) + { + return; // Suppress this diagnostic + } + } + } + sourceProductionContext.ReportDiagnostic(diagnostic); + } public IDictionary ServiceProviders { get; } = new Dictionary(); public IDictionary namesInScope)> Scopes = new Dictionary)>(); public SourceGenContext? ParentContext { get; set; } diff --git a/src/Controls/src/SourceGen/TypeConverters/ColumnDefinitionCollectionConverter.cs b/src/Controls/src/SourceGen/TypeConverters/ColumnDefinitionCollectionConverter.cs index 4a68bb742b28..f724eacb372f 100644 --- a/src/Controls/src/SourceGen/TypeConverters/ColumnDefinitionCollectionConverter.cs +++ b/src/Controls/src/SourceGen/TypeConverters/ColumnDefinitionCollectionConverter.cs @@ -30,7 +30,7 @@ public string Convert(string value, BaseNode node, ITypeSymbol toType, IndentedT return $"new {columnDefinitionCollectionType.ToFQDisplayString()}([{string.Join(", ", columnDefinitions)}])"; } - context.ReportConversionFailed( xmlLineInfo, value, Descriptors.ColumnDefinitionCollectionConversionFailed); + context.ReportConversionFailed(xmlLineInfo, value, Descriptors.ColumnDefinitionCollectionConversionFailed); return "default"; } } \ No newline at end of file diff --git a/src/Controls/src/SourceGen/TypeConverters/ConstraintConverter.cs b/src/Controls/src/SourceGen/TypeConverters/ConstraintConverter.cs index f56d5f8aae98..aaf5aeeec616 100644 --- a/src/Controls/src/SourceGen/TypeConverters/ConstraintConverter.cs +++ b/src/Controls/src/SourceGen/TypeConverters/ConstraintConverter.cs @@ -27,7 +27,7 @@ public string Convert(string value, BaseNode node, ITypeSymbol toType, IndentedT } } - context.ReportConversionFailed( xmlLineInfo, value, toType, Descriptors.ConversionFailed); + context.ReportConversionFailed(xmlLineInfo, value, toType, Descriptors.ConversionFailed); return "default"; } } \ No newline at end of file diff --git a/src/Controls/src/SourceGen/TypeConverters/EasingConverter.cs b/src/Controls/src/SourceGen/TypeConverters/EasingConverter.cs index ec30044003e1..2ffeccfa6163 100644 --- a/src/Controls/src/SourceGen/TypeConverters/EasingConverter.cs +++ b/src/Controls/src/SourceGen/TypeConverters/EasingConverter.cs @@ -37,7 +37,7 @@ public string Convert(string value, BaseNode node, ITypeSymbol toType, IndentedT } } - context.ReportConversionFailed( xmlLineInfo, value, Descriptors.EasingConversionFailed); + context.ReportConversionFailed(xmlLineInfo, value, Descriptors.EasingConversionFailed); return "default"; } } \ No newline at end of file diff --git a/src/Controls/src/SourceGen/TypeConverters/EnumConverter.cs b/src/Controls/src/SourceGen/TypeConverters/EnumConverter.cs index bc45cc239b1b..ef0474b77ccc 100644 --- a/src/Controls/src/SourceGen/TypeConverters/EnumConverter.cs +++ b/src/Controls/src/SourceGen/TypeConverters/EnumConverter.cs @@ -26,7 +26,7 @@ public string Convert(string value, BaseNode node, ITypeSymbol toType, IndentedT } } - context.ReportConversionFailed( xmlLineInfo, value, value, toType, Descriptors.ConversionFailed); + context.ReportConversionFailed(xmlLineInfo, value, value, toType, Descriptors.ConversionFailed); return "default"; } } \ No newline at end of file diff --git a/src/Controls/src/SourceGen/TypeConverters/FlexBasisConverter.cs b/src/Controls/src/SourceGen/TypeConverters/FlexBasisConverter.cs index 1265b1fbc755..a1456b9529fd 100644 --- a/src/Controls/src/SourceGen/TypeConverters/FlexBasisConverter.cs +++ b/src/Controls/src/SourceGen/TypeConverters/FlexBasisConverter.cs @@ -40,7 +40,7 @@ public string Convert(string value, BaseNode node, ITypeSymbol toType, IndentedT } } - context.ReportConversionFailed( xmlLineInfo, value, Descriptors.FlexBasisConversionFailed); + context.ReportConversionFailed(xmlLineInfo, value, Descriptors.FlexBasisConversionFailed); return "default"; } } \ No newline at end of file diff --git a/src/Controls/src/SourceGen/TypeConverters/FlowDirectionConverter.cs b/src/Controls/src/SourceGen/TypeConverters/FlowDirectionConverter.cs index 9cc989fb5d08..b338827cfd86 100644 --- a/src/Controls/src/SourceGen/TypeConverters/FlowDirectionConverter.cs +++ b/src/Controls/src/SourceGen/TypeConverters/FlowDirectionConverter.cs @@ -28,7 +28,7 @@ public string Convert(string value, BaseNode node, ITypeSymbol toType, IndentedT { var flowDirectionRtlType = context.Compilation.GetTypeByMetadataName("Microsoft.Maui.FlowDirection")!; return $"{flowDirectionRtlType.ToFQDisplayString()}.RightToLeft"; - } + } if (value.Equals("inherit", StringComparison.OrdinalIgnoreCase)) { var flowDirectionMatchType = context.Compilation.GetTypeByMetadataName("Microsoft.Maui.FlowDirection")!; @@ -40,7 +40,7 @@ public string Convert(string value, BaseNode node, ITypeSymbol toType, IndentedT return enumConverter.Convert(value, node, toType, writer, context); } - context.ReportConversionFailed( xmlLineInfo, value, Descriptors.FlowDirectionConversionFailed); + context.ReportConversionFailed(xmlLineInfo, value, Descriptors.FlowDirectionConversionFailed); return "default"; } } \ No newline at end of file diff --git a/src/Controls/src/SourceGen/TypeConverters/FontSizeConverter.cs b/src/Controls/src/SourceGen/TypeConverters/FontSizeConverter.cs index 11d2bd8b6321..388ef08e81de 100644 --- a/src/Controls/src/SourceGen/TypeConverters/FontSizeConverter.cs +++ b/src/Controls/src/SourceGen/TypeConverters/FontSizeConverter.cs @@ -36,7 +36,7 @@ public string Convert(string value, BaseNode node, ITypeSymbol toType, IndentedT } } - context.ReportConversionFailed( xmlLineInfo, value, toType, Descriptors.ConversionFailed); + context.ReportConversionFailed(xmlLineInfo, value, toType, Descriptors.ConversionFailed); return "default"; } } \ No newline at end of file diff --git a/src/Controls/src/SourceGen/TypeConverters/GridLengthConverter.cs b/src/Controls/src/SourceGen/TypeConverters/GridLengthConverter.cs index 3c16a813b3e7..c5961ff267fb 100644 --- a/src/Controls/src/SourceGen/TypeConverters/GridLengthConverter.cs +++ b/src/Controls/src/SourceGen/TypeConverters/GridLengthConverter.cs @@ -47,7 +47,7 @@ public string Convert(string value, BaseNode node, ITypeSymbol toType, IndentedT } } - context.ReportConversionFailed( xmlLineInfo, value, Descriptors.GridLengthConversionFailed); + context.ReportConversionFailed(xmlLineInfo, value, Descriptors.GridLengthConversionFailed); return "default"; } } \ No newline at end of file diff --git a/src/Controls/src/SourceGen/TypeConverters/ImageSourceConverter.cs b/src/Controls/src/SourceGen/TypeConverters/ImageSourceConverter.cs index aa559dc1965f..548f1b9845e2 100644 --- a/src/Controls/src/SourceGen/TypeConverters/ImageSourceConverter.cs +++ b/src/Controls/src/SourceGen/TypeConverters/ImageSourceConverter.cs @@ -21,13 +21,13 @@ public string Convert(string value, BaseNode node, ITypeSymbol toType, IndentedT var imageSourceType = context.Compilation.GetTypeByMetadataName("Microsoft.Maui.Controls.ImageSource")!; var uriType = context.Compilation.GetTypeByMetadataName("System.Uri")!; - + return Uri.TryCreate(value, UriKind.Absolute, out Uri uri) && uri.Scheme != "file" ? - $"{imageSourceType.ToFQDisplayString()}.FromUri(new {uriType.ToFQDisplayString()}(\"{uri}\"))" : + $"{imageSourceType.ToFQDisplayString()}.FromUri(new {uriType.ToFQDisplayString()}(\"{uri}\"))" : $"{imageSourceType.ToFQDisplayString()}.FromFile(\"{value}\")"; } - context.ReportConversionFailed( xmlLineInfo, value, toType, Descriptors.ConversionFailed); + context.ReportConversionFailed(xmlLineInfo, value, toType, Descriptors.ConversionFailed); return "default"; } } \ No newline at end of file diff --git a/src/Controls/src/SourceGen/TypeConverters/LayoutOptionsConverter.cs b/src/Controls/src/SourceGen/TypeConverters/LayoutOptionsConverter.cs index 2c6562c7241f..0d8ac879d809 100644 --- a/src/Controls/src/SourceGen/TypeConverters/LayoutOptionsConverter.cs +++ b/src/Controls/src/SourceGen/TypeConverters/LayoutOptionsConverter.cs @@ -21,7 +21,7 @@ public string Convert(string value, BaseNode node, ITypeSymbol toType, IndentedT var parts = value.Split(['.']); if (parts.Length > 2 || (parts.Length == 2 && parts[0] != "LayoutOptions")) { - context.ReportConversionFailed( xmlLineInfo, value, Descriptors.LayoutOptionsConversionFailed); + context.ReportConversionFailed(xmlLineInfo, value, Descriptors.LayoutOptionsConversionFailed); return "default"; } @@ -40,15 +40,15 @@ public string Convert(string value, BaseNode node, ITypeSymbol toType, IndentedT { "CenterAndExpand", $"{layoutOptionsType.ToFQDisplayString()}.CenterAndExpand" }, { "EndAndExpand", $"{layoutOptionsType.ToFQDisplayString()}.EndAndExpand" }, { "FillAndExpand", $"{layoutOptionsType.ToFQDisplayString()}.FillAndExpand" } - }; - + }; + if (layoutOptionsMap.TryGetValue(value, out var layoutOption)) { return layoutOption; } } - context.ReportConversionFailed( xmlLineInfo, value, Descriptors.LayoutOptionsConversionFailed); + context.ReportConversionFailed(xmlLineInfo, value, Descriptors.LayoutOptionsConversionFailed); return "default"; } } \ No newline at end of file diff --git a/src/Controls/src/SourceGen/TypeConverters/ListStringConverter.cs b/src/Controls/src/SourceGen/TypeConverters/ListStringConverter.cs index 4a543a8f9dc7..ae333d8ddf2f 100644 --- a/src/Controls/src/SourceGen/TypeConverters/ListStringConverter.cs +++ b/src/Controls/src/SourceGen/TypeConverters/ListStringConverter.cs @@ -25,7 +25,7 @@ public string Convert(string value, BaseNode node, ITypeSymbol toType, IndentedT return $"new {constructedListType.ToFQDisplayString()} {{ {string.Join(", ", value.Split([','], StringSplitOptions.RemoveEmptyEntries).Select(v => $"\"{v.Trim()}\""))} }}"; } - context.ReportConversionFailed( xmlLineInfo, value, Descriptors.ListStringConversionFailed); + context.ReportConversionFailed(xmlLineInfo, value, Descriptors.ListStringConversionFailed); return "default"; } } \ No newline at end of file diff --git a/src/Controls/src/SourceGen/TypeConverters/PathGeometryConverter.cs b/src/Controls/src/SourceGen/TypeConverters/PathGeometryConverter.cs index 16433d7ad43b..3aef7dd78ee8 100644 --- a/src/Controls/src/SourceGen/TypeConverters/PathGeometryConverter.cs +++ b/src/Controls/src/SourceGen/TypeConverters/PathGeometryConverter.cs @@ -5,7 +5,7 @@ using Microsoft.Maui.Controls.Xaml; namespace Microsoft.Maui.Controls.SourceGen.TypeConverters; - + class PathGeometryConverter : ISGTypeConverter { public IEnumerable SupportedTypes => new[] { "PathGeometry", "Microsoft.Maui.Controls.Shapes.PathGeometry" }; @@ -22,7 +22,7 @@ public string Convert(string value, BaseNode node, ITypeSymbol toType, IndentedT return $"new {pathGeometryType.ToFQDisplayString()}()"; } - context.ReportConversionFailed( xmlLineInfo, value, toType, Descriptors.ConversionFailed); + context.ReportConversionFailed(xmlLineInfo, value, toType, Descriptors.ConversionFailed); return "default"; } } \ No newline at end of file diff --git a/src/Controls/src/SourceGen/TypeConverters/PointCollectionConverter.cs b/src/Controls/src/SourceGen/TypeConverters/PointCollectionConverter.cs index f863d97a3d7a..ab11d581de0b 100644 --- a/src/Controls/src/SourceGen/TypeConverters/PointCollectionConverter.cs +++ b/src/Controls/src/SourceGen/TypeConverters/PointCollectionConverter.cs @@ -43,23 +43,23 @@ public string Convert(string value, BaseNode node, ITypeSymbol toType, IndentedT } else { - context.ReportConversionFailed( xmlLineInfo, value, toType, Descriptors.ConversionFailed); + context.ReportConversionFailed(xmlLineInfo, value, toType, Descriptors.ConversionFailed); return "default"; } } if (hasX) { - context.ReportConversionFailed( xmlLineInfo, value, toType, Descriptors.ConversionFailed); + context.ReportConversionFailed(xmlLineInfo, value, toType, Descriptors.ConversionFailed); return "default"; } var pointCollectionType = context.Compilation.GetTypeByMetadataName("Microsoft.Maui.Controls.PointCollection")!; - + return $"new {pointCollectionType.ToFQDisplayString()}(new[] {{ {string.Join(", ", pointCollection)} }})"; } - context.ReportConversionFailed( xmlLineInfo, value, toType, Descriptors.ConversionFailed); + context.ReportConversionFailed(xmlLineInfo, value, toType, Descriptors.ConversionFailed); return "default"; } } \ No newline at end of file diff --git a/src/Controls/src/SourceGen/TypeConverters/PointConverter.cs b/src/Controls/src/SourceGen/TypeConverters/PointConverter.cs index 7fc805582977..040fdfe3d1de 100644 --- a/src/Controls/src/SourceGen/TypeConverters/PointConverter.cs +++ b/src/Controls/src/SourceGen/TypeConverters/PointConverter.cs @@ -27,7 +27,7 @@ public string Convert(string value, BaseNode node, ITypeSymbol toType, IndentedT } } - context.ReportConversionFailed( xmlLineInfo, value, Descriptors.PointConversionFailed); + context.ReportConversionFailed(xmlLineInfo, value, Descriptors.PointConversionFailed); return "default"; } } \ No newline at end of file diff --git a/src/Controls/src/SourceGen/TypeConverters/RowDefinitionCollectionConverter.cs b/src/Controls/src/SourceGen/TypeConverters/RowDefinitionCollectionConverter.cs index 3952565d8b09..cce12570b7ec 100644 --- a/src/Controls/src/SourceGen/TypeConverters/RowDefinitionCollectionConverter.cs +++ b/src/Controls/src/SourceGen/TypeConverters/RowDefinitionCollectionConverter.cs @@ -30,7 +30,7 @@ public string Convert(string value, BaseNode node, ITypeSymbol toType, IndentedT return $"new {rowDefinitionCollectionType.ToFQDisplayString()}([{string.Join(", ", rowDefinitions)}])"; } - context.ReportConversionFailed( xmlLineInfo, value, Descriptors.RowDefinitionCollectionConversionFailed); + context.ReportConversionFailed(xmlLineInfo, value, Descriptors.RowDefinitionCollectionConversionFailed); return "default"; } } \ No newline at end of file diff --git a/src/Controls/src/SourceGen/TypeConverters/StrokeShapeConverter.cs b/src/Controls/src/SourceGen/TypeConverters/StrokeShapeConverter.cs index 4f02f5b90358..cf0e61e907b1 100644 --- a/src/Controls/src/SourceGen/TypeConverters/StrokeShapeConverter.cs +++ b/src/Controls/src/SourceGen/TypeConverters/StrokeShapeConverter.cs @@ -145,7 +145,7 @@ public string Convert(string value, BaseNode node, ITypeSymbol toType, IndentedT } } - context.ReportConversionFailed( xmlLineInfo, value, Descriptors.StrokeShapeConversionFailed); + context.ReportConversionFailed(xmlLineInfo, value, Descriptors.StrokeShapeConversionFailed); return "default"; } } \ No newline at end of file diff --git a/src/Controls/src/SourceGen/TypeConverters/ThicknessConverter.cs b/src/Controls/src/SourceGen/TypeConverters/ThicknessConverter.cs index 2fe71822e423..c4573604e484 100644 --- a/src/Controls/src/SourceGen/TypeConverters/ThicknessConverter.cs +++ b/src/Controls/src/SourceGen/TypeConverters/ThicknessConverter.cs @@ -55,7 +55,7 @@ public string Convert(string value, BaseNode node, ITypeSymbol toType, IndentedT } } - context.ReportConversionFailed( xmlLineInfo, value, Descriptors.ThicknessConversionFailed); + context.ReportConversionFailed(xmlLineInfo, value, Descriptors.ThicknessConversionFailed); return "default"; } } \ No newline at end of file diff --git a/src/Controls/src/SourceGen/TypeConverters/TypeTypeConverter.cs b/src/Controls/src/SourceGen/TypeConverters/TypeTypeConverter.cs index ca5c1c8b2198..a7bc574dab2d 100644 --- a/src/Controls/src/SourceGen/TypeConverters/TypeTypeConverter.cs +++ b/src/Controls/src/SourceGen/TypeConverters/TypeTypeConverter.cs @@ -28,7 +28,7 @@ public string Convert(string value, BaseNode node, ITypeSymbol toType, IndentedT if (typeRef != null) return $"typeof({typeRef.ToFQDisplayString()})"; error: - context.ReportConversionFailed((IXmlLineInfo)node, value, Descriptors.TypeResolution); - return "default"; + context.ReportConversionFailed((IXmlLineInfo)node, value, Descriptors.TypeResolution); + return "default"; } } \ No newline at end of file diff --git a/src/Controls/src/SourceGen/Visitors/SetNamescopesAndRegisterNames.cs b/src/Controls/src/SourceGen/Visitors/SetNamescopesAndRegisterNames.cs index d1d6b8ac350d..c8b949660cda 100644 --- a/src/Controls/src/SourceGen/Visitors/SetNamescopesAndRegisterNames.cs +++ b/src/Controls/src/SourceGen/Visitors/SetNamescopesAndRegisterNames.cs @@ -28,8 +28,11 @@ public void Visit(ValueNode node, INode parentNode) return; var name = (string)node.Value; if (namescope.namesInScope.ContainsKey(name)) - //TODO send diagnostic instead - throw new Exception("dup x:Name"); + { + var location = LocationHelpers.LocationCreate(Context.ProjectItem.RelativePath!, (System.Xml.IXmlLineInfo)node, "x:Name"); + Context.ReportDiagnostic(Microsoft.CodeAnalysis.Diagnostic.Create(Descriptors.NamescopeDuplicate, location, name)); + return; + } namescope.namesInScope.Add(name, Context.Variables[(ElementNode)parentNode]); using (PrePost.NewConditional(Writer, "!_MAUIXAML_SG_NAMESCOPE_DISABLE")) { @@ -113,7 +116,7 @@ ILocalValue GetOrCreateNameScope(ElementNode node) return new LocalVariable(Context.Compilation.GetTypeByMetadataName("Microsoft.Maui.Controls.Internals.NameScope")!, namescope); } - public static ILocalValue CreateNamescope(IndentedTextWriter writer, SourceGenContext context, string? accessor = null) + public static ILocalValue CreateNamescope(IndentedTextWriter writer, SourceGenContext context, string? accessor = null) { var namescope = NamingHelpers.CreateUniqueVariableName(context, context.Compilation.GetTypeByMetadataName("Microsoft.Maui.Controls.Internals.INameScope")!); using (PrePost.NewConditional(writer, "!_MAUIXAML_SG_NAMESCOPE_DISABLE")) diff --git a/src/Controls/src/SourceGen/XmlTypeExtensions.cs b/src/Controls/src/SourceGen/XmlTypeExtensions.cs index 279644781e3b..0726a8a4d5ea 100644 --- a/src/Controls/src/SourceGen/XmlTypeExtensions.cs +++ b/src/Controls/src/SourceGen/XmlTypeExtensions.cs @@ -40,7 +40,7 @@ static class XmlTypeExtensions public static bool TryResolveTypeSymbol(this XmlType xmlType, Action? reportDiagnostic, Compilation compilation, AssemblyAttributes xmlnsCache, IDictionary typeCache, out INamedTypeSymbol? symbol) { - if (typeCache.TryGetValue(xmlType, out symbol)) + if (typeCache.TryGetValue(xmlType, out symbol)) return true; var name = xmlType.Name.Split(':').Last(); //strip prefix @@ -49,12 +49,12 @@ public static bool TryResolveTypeSymbol(this XmlType xmlType, Action if (!xmlnsCache.ClrNamespacesForXmlns.TryGetValue(xmlType.NamespaceUri, out var namespaces)) { XmlnsHelper.ParseXmlns(xmlType.NamespaceUri, out _, out var ns, out _, out _); - namespaces = [ns!]; + namespaces = [ns!]; } - var extsuffixes = (name != "DataTemplate" && !name.EndsWith("Extension", StringComparison.Ordinal)) ? new [] {"Extension", string.Empty} : [string.Empty]; + var extsuffixes = (name != "DataTemplate" && !name.EndsWith("Extension", StringComparison.Ordinal)) ? new[] { "Extension", string.Empty } : [string.Empty]; foreach (var suffix in extsuffixes) - { + { var types = namespaces.Select(ns => $"{ns}.{name}{suffix}{genericSuffix}").SelectMany(typeName => compilation.GetTypesByMetadataName(typeName)).Where(ts => ts.IsPublicOrVisibleInternal(xmlnsCache.InternalsVisible)).Distinct(SymbolEqualityComparer.Default).Cast().ToArray(); if (types.Length > 1) @@ -75,7 +75,7 @@ public static bool TryResolveTypeSymbol(this XmlType xmlType, Action typeCache[xmlType] = symbol; return true; } - } + } symbol = null; return false; } diff --git a/src/Controls/src/SourceGen/compat.cs b/src/Controls/src/SourceGen/compat.cs index 49832e9e3e1c..9de2b73abb0f 100644 --- a/src/Controls/src/SourceGen/compat.cs +++ b/src/Controls/src/SourceGen/compat.cs @@ -10,30 +10,30 @@ namespace System.Runtime.CompilerServices { #if !NET5_0_OR_GREATER - [EditorBrowsable(EditorBrowsableState.Never)] - internal static class IsExternalInit {} + [EditorBrowsable(EditorBrowsableState.Never)] + internal static class IsExternalInit { } #endif // !NET5_0_OR_GREATER #if !NET7_0_OR_GREATER - [AttributeUsage(AttributeTargets.Class | AttributeTargets.Struct | AttributeTargets.Field | AttributeTargets.Property, AllowMultiple = false, Inherited = false)] - internal sealed class RequiredMemberAttribute : Attribute {} + [AttributeUsage(AttributeTargets.Class | AttributeTargets.Struct | AttributeTargets.Field | AttributeTargets.Property, AllowMultiple = false, Inherited = false)] + internal sealed class RequiredMemberAttribute : Attribute { } - [AttributeUsage(AttributeTargets.All, AllowMultiple = true, Inherited = false)] - internal sealed class CompilerFeatureRequiredAttribute : Attribute - { - public CompilerFeatureRequiredAttribute(string featureName) - { - FeatureName = featureName; - } + [AttributeUsage(AttributeTargets.All, AllowMultiple = true, Inherited = false)] + internal sealed class CompilerFeatureRequiredAttribute : Attribute + { + public CompilerFeatureRequiredAttribute(string featureName) + { + FeatureName = featureName; + } - public string FeatureName { get; } - public bool IsOptional { get; init; } + public string FeatureName { get; } + public bool IsOptional { get; init; } - public const string RefStructs = nameof(RefStructs); - public const string RequiredMembers = nameof(RequiredMembers); - } + public const string RefStructs = nameof(RefStructs); + public const string RequiredMembers = nameof(RequiredMembers); + } #endif // !NET7_0_OR_GREATER } @@ -41,7 +41,7 @@ public CompilerFeatureRequiredAttribute(string featureName) namespace System.Diagnostics.CodeAnalysis { #if !NET7_0_OR_GREATER - [AttributeUsage(AttributeTargets.Constructor, AllowMultiple = false, Inherited = false)] - internal sealed class SetsRequiredMembersAttribute : Attribute {} + [AttributeUsage(AttributeTargets.Constructor, AllowMultiple = false, Inherited = false)] + internal sealed class SetsRequiredMembersAttribute : Attribute { } #endif } \ No newline at end of file diff --git a/src/Controls/src/SourceGen/xlf/MauiGResources.Designer.cs b/src/Controls/src/SourceGen/xlf/MauiGResources.Designer.cs index f3864def33c5..3dba5901e54e 100644 --- a/src/Controls/src/SourceGen/xlf/MauiGResources.Designer.cs +++ b/src/Controls/src/SourceGen/xlf/MauiGResources.Designer.cs @@ -254,5 +254,18 @@ internal static string DuplicateTypeError internal static string AmbiguousTypeMessage => ResourceManager.GetString("AmbiguousTypeMessage", resourceCulture); internal static string RequiredPropertyTitle => ResourceManager.GetString("RequiredPropertyTitle", resourceCulture); internal static string RequiredPropertyMessage => ResourceManager.GetString("RequiredPropertyMessage", resourceCulture); + + // Binding-related diagnostics + internal static string BindingCompilationFailed => ResourceManager.GetString("BindingCompilationFailed", resourceCulture); + internal static string BindingCompilationWarning => ResourceManager.GetString("BindingCompilationWarning", resourceCulture); + internal static string BindingIndexerNotClosed => ResourceManager.GetString("BindingIndexerNotClosed", resourceCulture); + internal static string BindingIndexerEmpty => ResourceManager.GetString("BindingIndexerEmpty", resourceCulture); + internal static string BindingIndexerTypeUnsupported => ResourceManager.GetString("BindingIndexerTypeUnsupported", resourceCulture); + internal static string BindingPropertyNotFound => ResourceManager.GetString("BindingPropertyNotFound", resourceCulture); + internal static string BindingWithXDataTypeFromOuterScope => ResourceManager.GetString("BindingWithXDataTypeFromOuterScope", resourceCulture); + + // Namescope diagnostics + internal static string NamescopeError => ResourceManager.GetString("NamescopeError", resourceCulture); + internal static string NamescopeDuplicate => ResourceManager.GetString("NamescopeDuplicate", resourceCulture); } } diff --git a/src/Controls/src/Xaml/SimplifyOnPlatformVisitor.cs b/src/Controls/src/Xaml/SimplifyOnPlatformVisitor.cs index 005af624ced1..789ea0099a60 100644 --- a/src/Controls/src/Xaml/SimplifyOnPlatformVisitor.cs +++ b/src/Controls/src/Xaml/SimplifyOnPlatformVisitor.cs @@ -7,7 +7,8 @@ namespace Microsoft.Maui.Controls.Xaml; class SimplifyOnPlatformVisitor : IXamlNodeVisitor -{ public SimplifyOnPlatformVisitor(string targetFramework) +{ + public SimplifyOnPlatformVisitor(string targetFramework) { if (string.IsNullOrEmpty(targetFramework)) @@ -49,7 +50,7 @@ public void Visit(ElementNode node, INode parentNode) //`{OnPlatform}` markup extension if (node.XmlType.IsOfAnyType("OnPlatformExtension")) { - if ( node.Properties.TryGetValue(new XmlName("", Target), out INode targetNode) + if (node.Properties.TryGetValue(new XmlName("", Target), out INode targetNode) || node.Properties.TryGetValue(new XmlName(null, Target), out targetNode) || node.Properties.TryGetValue(new XmlName("", "Default"), out targetNode) || node.Properties.TryGetValue(new XmlName(null, "Default"), out targetNode)) @@ -111,10 +112,10 @@ public void Visit(ElementNode node, INode parentNode) // Create a new ElementNode with the type from x:TypeArguments var typeArg = node.XmlType.TypeArguments[0]; var elementNode = new ElementNode(typeArg, typeArg.NamespaceUri, node.NamespaceResolver, node.LineNumber, node.LinePosition); - + // Set the value as the collection item (for types like Color, this is how they're represented) elementNode.CollectionItems.Add(valueNode); - + // Replace the ValueNode with the new ElementNode onNode = elementNode; } diff --git a/src/Controls/src/Xaml/XamlNode.cs b/src/Controls/src/Xaml/XamlNode.cs index 65c65469daae..7c54f37fb98d 100644 --- a/src/Controls/src/Xaml/XamlNode.cs +++ b/src/Controls/src/Xaml/XamlNode.cs @@ -56,9 +56,9 @@ public override void Accept(IXamlNodeVisitor visitor, INode parentNode) public override INode Clone() => new ValueNode(Value, NamespaceResolver, LineNumber, LinePosition) - { - IgnorablePrefixes = IgnorablePrefixes - }; + { + IgnorablePrefixes = IgnorablePrefixes + }; } [DebuggerDisplay("{MarkupString}")] @@ -71,13 +71,13 @@ public override void Accept(IXamlNodeVisitor visitor, INode parentNode) public override INode Clone() => new MarkupNode(MarkupString, NamespaceResolver, LineNumber, LinePosition) - { - IgnorablePrefixes = IgnorablePrefixes - }; + { + IgnorablePrefixes = IgnorablePrefixes + }; } static class XmlNameExtensions -{ +{ public static bool TryGetValue(this Dictionary properties, string name, out INode node, out XmlName xmlName) { xmlName = new XmlName("", name); diff --git a/src/Controls/src/Xaml/XmlType.cs b/src/Controls/src/Xaml/XmlType.cs index da31542b754e..71902a5e4462 100644 --- a/src/Controls/src/Xaml/XmlType.cs +++ b/src/Controls/src/Xaml/XmlType.cs @@ -15,12 +15,12 @@ class XmlType(string namespaceUri, string name, IList typeArguments) public override bool Equals(object obj) { - if (obj is not XmlType other) + if (obj is not XmlType other) return false; - + return NamespaceUri == other.NamespaceUri && Name == other.Name - && ( TypeArguments == null && other.TypeArguments == null + && (TypeArguments == null && other.TypeArguments == null || TypeArguments != null && other.TypeArguments != null && TypeArguments.SequenceEqual(other.TypeArguments)); } diff --git a/src/Controls/tests/BindingSourceGen.UnitTests/ObservablePropertyTests.cs b/src/Controls/tests/BindingSourceGen.UnitTests/ObservablePropertyTests.cs new file mode 100644 index 000000000000..b3e82c0d320b --- /dev/null +++ b/src/Controls/tests/BindingSourceGen.UnitTests/ObservablePropertyTests.cs @@ -0,0 +1,160 @@ +using System.Linq; +using Microsoft.CodeAnalysis; +using Microsoft.CodeAnalysis.CSharp; +using Microsoft.Maui.Controls.BindingSourceGen; +using Xunit; + +namespace BindingSourceGen.UnitTests; + +public class ObservablePropertyTests +{ + // Note: Expression-based bindings work with ObservableProperty through type inference in + // GetLambdaReturnType. The generated interceptor code will be correct. In tests, + // we'll see compilation errors because the actual generated property doesn't exist + // (CommunityToolkit.Mvvm's source generator isn't running in the test environment), + // but the interceptor code itself is generated correctly. + + [Fact] + public void GenerateBindingToObservablePropertyFromCamelCaseField() + { + var source = """ + using Microsoft.Maui.Controls; + using System.Collections.ObjectModel; + + namespace CommunityToolkit.Mvvm.ComponentModel + { + [System.AttributeUsage(System.AttributeTargets.Field)] + public class ObservablePropertyAttribute : System.Attribute { } + } + + namespace TestApp + { + public class MyViewModel + { + [CommunityToolkit.Mvvm.ComponentModel.ObservableProperty] + private string? name; + // Name property will be generated by CommunityToolkit.Mvvm + } + + public class TestCode + { + public void Test() + { + var label = new Label(); + label.SetBinding(Label.TextProperty, static (MyViewModel vm) => vm.Name); + } + } + } + """; + + var result = SourceGenHelpers.Run(source); + + // The binding should be generated successfully with ObservableProperty inference + Assert.NotNull(result.Binding); + + // Verify the generated interceptor code contains the correct getter and setter references + var allGeneratedCode = string.Join("\n\n", result.GeneratedFiles.Values); + // Check that the handler contains the property access + Assert.Contains("new(static source => source, \"Name\")", allGeneratedCode, System.StringComparison.Ordinal); + // Check that setter assigns to .Name + Assert.Contains("source.Name = value;", allGeneratedCode, System.StringComparison.Ordinal); + + // Note: There will be compilation errors because Name doesn't actually exist, + // but the interceptor code itself is generated correctly. In real usage with + // CommunityToolkit.Mvvm, the property would exist and compile successfully. + } + + [Fact] + public void GenerateBindingToObservablePropertyFromUnderscorePrefixedField() + { + var source = """ + using Microsoft.Maui.Controls; + using System.Collections.ObjectModel; + + namespace CommunityToolkit.Mvvm.ComponentModel + { + [System.AttributeUsage(System.AttributeTargets.Field)] + public class ObservablePropertyAttribute : System.Attribute { } + } + + namespace TestApp + { + public class MyViewModel + { + [CommunityToolkit.Mvvm.ComponentModel.ObservableProperty] + private string? _title; + // Title property will be generated by CommunityToolkit.Mvvm + } + + public class TestCode + { + public void Test() + { + var label = new Label(); + label.SetBinding(Label.TextProperty, static (MyViewModel vm) => vm.Title); + } + } + } + """; + + var result = SourceGenHelpers.Run(source); + + // The binding should be generated successfully with ObservableProperty inference + Assert.NotNull(result.Binding); + + // Verify the generated interceptor code contains the correct getter and setter references + var allGeneratedCode = string.Join("\n\n", result.GeneratedFiles.Values); + // Check that the handler contains the property access + Assert.Contains("new(static source => source, \"Title\")", allGeneratedCode, System.StringComparison.Ordinal); + // Check that setter assigns to .Title + Assert.Contains("source.Title = value;", allGeneratedCode, System.StringComparison.Ordinal); + } + + [Fact] + public void GenerateBindingToObservablePropertyCollection() + { + var source = """ + using Microsoft.Maui.Controls; + using System.Collections.ObjectModel; + + namespace CommunityToolkit.Mvvm.ComponentModel + { + [System.AttributeUsage(System.AttributeTargets.Field)] + public class ObservablePropertyAttribute : System.Attribute { } + } + + namespace TestApp + { + public class Tag { } + + public class MyViewModel + { + [CommunityToolkit.Mvvm.ComponentModel.ObservableProperty] + private ObservableCollection _tags = new(); + // Tags property will be generated by CommunityToolkit.Mvvm + } + + public class TestCode + { + public void Test() + { + var label = new Label(); + label.SetBinding(Label.BindingContextProperty, static (MyViewModel vm) => vm.Tags); + } + } + } + """; + + var result = SourceGenHelpers.Run(source); + + // The binding should be generated successfully with ObservableProperty inference + Assert.NotNull(result.Binding); + + // Verify the generated interceptor code contains the correct getter and setter references + var allGeneratedCode = string.Join("\n\n", result.GeneratedFiles.Values); + // Check that the handler contains the property access + Assert.Contains("new(static source => source, \"Tags\")", allGeneratedCode, System.StringComparison.Ordinal); + // Check that setter assigns to .Tags + Assert.Contains("source.Tags = value;", allGeneratedCode, System.StringComparison.Ordinal); + } +} diff --git a/src/Controls/tests/Core.UnitTests/NavigationUnitTest.cs b/src/Controls/tests/Core.UnitTests/NavigationUnitTest.cs index 787bc5abfb3b..745ea0c8a3da 100644 --- a/src/Controls/tests/Core.UnitTests/NavigationUnitTest.cs +++ b/src/Controls/tests/Core.UnitTests/NavigationUnitTest.cs @@ -679,6 +679,69 @@ public async Task TestRemovePage(bool useMaui) }); } + [Theory] + [InlineData(true)] + [InlineData(false)] + public async Task RemovePageDisconnectsHandlerForNonVisiblePage(bool useMaui) + { + // Arrange: Create a navigation stack with 3 pages + var root = new ContentPage { Title = "Root" }; + var middlePage = new ContentPage { Title = "Middle" }; + var topPage = new ContentPage { Title = "Top" }; + var navPage = new TestNavigationPage(useMaui, root); + _ = new TestWindow(navPage); + + await navPage.PushAsync(middlePage); + await navPage.PushAsync(topPage); + + // Assign a handler to the middle page to verify it gets disconnected + var mockHandler = new HandlerStub(); + middlePage.Handler = mockHandler; + + // Act: Remove the middle (non-visible) page from the stack + navPage.Navigation.RemovePage(middlePage); + await navPage.NavigatingTask; + + // Assert: Verify the page was removed from the navigation stack + Assert.Equal(2, navPage.Navigation.NavigationStack.Count); + Assert.Same(root, navPage.RootPage); + Assert.Same(topPage, navPage.CurrentPage); + Assert.DoesNotContain(middlePage, navPage.Navigation.NavigationStack); + + // Verify the page is no longer a logical child of the NavigationPage + Assert.DoesNotContain(middlePage, navPage.InternalChildren.Cast()); + + // Verify the handler was disconnected + Assert.Null(middlePage.Handler); + } + + [Theory] + [InlineData(true)] + [InlineData(false)] + public async Task RemovePageDisconnectsHandlerForRemovedRootPage(bool useMaui) + { + // Arrange: Create a navigation stack with 2 pages where the root page has a handler + var root = new ContentPage { Title = "Root" }; + var topPage = new ContentPage { Title = "Top" }; + var navPage = new TestNavigationPage(useMaui, root); + var window = new TestWindow(navPage); + + // Manually assign a mock handler to track disconnection + var mockHandler = new HandlerStub(); + root.Handler = mockHandler; + + await navPage.PushAsync(topPage); + + // Act: Remove the root page (non-visible) from the stack + navPage.Navigation.RemovePage(root); + await navPage.NavigatingTask; + + // Assert: Handler should be disconnected (set to null on the page) + Assert.Null(root.Handler); + Assert.Same(topPage, navPage.RootPage); + Assert.Same(topPage, navPage.CurrentPage); + } + [Fact] public async Task CurrentPageUpdatesOnPopBeforeAsyncCompletes() { diff --git a/src/Controls/tests/Core.UnitTests/ShellNavigatingTests.cs b/src/Controls/tests/Core.UnitTests/ShellNavigatingTests.cs index 1451bf322a6b..3042ef90f915 100644 --- a/src/Controls/tests/Core.UnitTests/ShellNavigatingTests.cs +++ b/src/Controls/tests/Core.UnitTests/ShellNavigatingTests.cs @@ -994,6 +994,31 @@ protected override Task OnPushModal(Page modal, bool animated) } } + [Fact] + public async Task RemovePageDisconnectsHandlerInShell() + { + Routing.RegisterRoute("page1", typeof(TestPage1)); + Routing.RegisterRoute("page2", typeof(TestPage2)); + var shell = new TestShell( + CreateShellItem(shellContentRoute: "root", shellItemRoute: "main") + ); + + await shell.GoToAsync("//main/root/page1"); + await shell.GoToAsync("page2"); + + // Get the middle page and assign a handler + var middlePage = shell.Navigation.NavigationStack[1]; + var mockHandler = new HandlerStub(); + middlePage.Handler = mockHandler; + + // Act: Remove the middle page + shell.Navigation.RemovePage(middlePage); + + // Assert: Handler should be disconnected + Assert.Null(middlePage.Handler); + Assert.Equal(2, shell.Navigation.NavigationStack.Count); + } + ShellNavigatingEventArgs CreateShellNavigatedEventArgs() => new ShellNavigatingEventArgs("..", "../newstate", ShellNavigationSource.Push, true); } diff --git a/src/Controls/tests/SourceGen.UnitTests/BindingDiagnosticsTests.cs b/src/Controls/tests/SourceGen.UnitTests/BindingDiagnosticsTests.cs new file mode 100644 index 000000000000..42ef7c303189 --- /dev/null +++ b/src/Controls/tests/SourceGen.UnitTests/BindingDiagnosticsTests.cs @@ -0,0 +1,253 @@ +using System.Linq; +using Microsoft.CodeAnalysis; +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 BindingDiagnosticsTests : 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 BindingPropertyNotFound_ReportsCorrectDiagnostic() + { + var xaml = +""" + + + +"""; + + var csharp = +""" +namespace Test; + +public partial class TestPage : Microsoft.Maui.Controls.ContentPage { } + +public class ViewModel +{ + public string Name { get; set; } +} +"""; + + var compilation = CreateMauiCompilation() + .AddSyntaxTrees(Microsoft.CodeAnalysis.CSharp.CSharpSyntaxTree.ParseText(csharp)); + var result = RunGenerator(compilation, new AdditionalXamlFile("Test.xaml", xaml), assertNoCompilationErrors: false); + + // Should have a diagnostic for property not found + var diagnostic = result.Diagnostics.FirstOrDefault(d => d.Id == "MAUIG2045"); + Assert.NotNull(diagnostic); + Assert.Equal(DiagnosticSeverity.Warning, diagnostic.Severity); + + var message = diagnostic.GetMessage(); + Assert.Contains("NonExistentProperty", message, System.StringComparison.Ordinal); + Assert.Contains("ViewModel", message, System.StringComparison.Ordinal); + } + + [Fact] + public void BindingIndexerNotClosed_ReportsCorrectDiagnostic() + { + var xaml = +""" + + + +"""; + + var csharp = +""" +namespace Test; + +public partial class TestPage : Microsoft.Maui.Controls.ContentPage { } + +public class ViewModel +{ + public string[] Items { get; set; } +} +"""; + + var compilation = CreateMauiCompilation() + .AddSyntaxTrees(Microsoft.CodeAnalysis.CSharp.CSharpSyntaxTree.ParseText(csharp)); + var result = RunGenerator(compilation, new AdditionalXamlFile("Test.xaml", xaml), assertNoCompilationErrors: false); + + // Should have a diagnostic for indexer not closed + var diagnostic = result.Diagnostics.FirstOrDefault(d => d.Id == "MAUIG2041"); + Assert.NotNull(diagnostic); + Assert.Equal(DiagnosticSeverity.Error, diagnostic.Severity); + + var message = diagnostic.GetMessage(); + Assert.Contains("closing bracket", message, System.StringComparison.Ordinal); + } + + [Fact] + public void BindingIndexerEmpty_ReportsCorrectDiagnostic() + { + var xaml = +""" + + + +"""; + + var csharp = +""" +namespace Test; + +public partial class TestPage : Microsoft.Maui.Controls.ContentPage { } + +public class ViewModel +{ + public string[] Items { get; set; } +} +"""; + + var compilation = CreateMauiCompilation() + .AddSyntaxTrees(Microsoft.CodeAnalysis.CSharp.CSharpSyntaxTree.ParseText(csharp)); + var result = RunGenerator(compilation, new AdditionalXamlFile("Test.xaml", xaml), assertNoCompilationErrors: false); + + // Should have a diagnostic for indexer empty + var diagnostic = result.Diagnostics.FirstOrDefault(d => d.Id == "MAUIG2042"); + Assert.NotNull(diagnostic); + Assert.Equal(DiagnosticSeverity.Error, diagnostic.Severity); + + var message = diagnostic.GetMessage(); + Assert.Contains("did not contain arguments", message, System.StringComparison.Ordinal); + } + + [Fact] + public void DuplicateXName_ReportsCorrectDiagnostic() + { + var xaml = +""" + + + + + +"""; + + var compilation = CreateMauiCompilation(); + var result = RunGenerator(compilation, new AdditionalXamlFile("Test.xaml", xaml), assertNoCompilationErrors: false); + + // Should have a diagnostic for duplicate name + var diagnostic = result.Diagnostics.FirstOrDefault(d => d.Id == "MAUIG2064"); + Assert.NotNull(diagnostic); + Assert.Equal(DiagnosticSeverity.Error, diagnostic.Severity); + + var message = diagnostic.GetMessage(); + Assert.Contains("MyLabel", message, System.StringComparison.Ordinal); + Assert.Contains("already exists", message, System.StringComparison.Ordinal); + } + + [Fact] + public void BindingWithXDataTypeFromOuterScope_ReportsWarning() + { + var xaml = +""" + + + + + + + +"""; + + var csharp = +""" +namespace Test; + +public partial class TestPage : Microsoft.Maui.Controls.ContentPage { } + +public class ViewModel +{ + public string Name { get; set; } +} +"""; + + var compilation = CreateMauiCompilation() + .AddSyntaxTrees(Microsoft.CodeAnalysis.CSharp.CSharpSyntaxTree.ParseText(csharp)); + var result = RunGenerator(compilation, new AdditionalXamlFile("Test.xaml", xaml), assertNoCompilationErrors: false); + + // Should have a warning for x:DataType from outer scope + var diagnostic = result.Diagnostics.FirstOrDefault(d => d.Id == "MAUIG2024"); + Assert.NotNull(diagnostic); + Assert.Equal(DiagnosticSeverity.Warning, diagnostic.Severity); + } + + [Fact] + public void BindingIndexerTypeUnsupported_ReportsCorrectDiagnostic() + { + var xaml = +""" + + + +"""; + + var csharp = +""" +namespace Test; + +public partial class TestPage : Microsoft.Maui.Controls.ContentPage { } + +public class ViewModel +{ + public User User { get; set; } +} + +public class User +{ + public string Name { get; set; } +} +"""; + + var compilation = CreateMauiCompilation() + .AddSyntaxTrees(Microsoft.CodeAnalysis.CSharp.CSharpSyntaxTree.ParseText(csharp)); + var result = RunGenerator(compilation, new AdditionalXamlFile("Test.xaml", xaml), assertNoCompilationErrors: false); + + // Should have a diagnostic for unsupported indexer type + var diagnostic = result.Diagnostics.FirstOrDefault(d => d.Id == "MAUIG2043"); + Assert.NotNull(diagnostic); + Assert.Equal(DiagnosticSeverity.Error, diagnostic.Severity); + } +} diff --git a/src/Controls/tests/SourceGen.UnitTests/InitializeComponent/IndexerBindings.cs b/src/Controls/tests/SourceGen.UnitTests/InitializeComponent/IndexerBindings.cs new file mode 100644 index 000000000000..64a4e6ff2ff4 --- /dev/null +++ b/src/Controls/tests/SourceGen.UnitTests/InitializeComponent/IndexerBindings.cs @@ -0,0 +1,609 @@ +using System; +using System.IO; +using System.Linq; +using Xunit; + +namespace Microsoft.Maui.Controls.SourceGen.UnitTests; + +public class IndexerBindings : SourceGenXamlInitializeComponentTestBase +{ + [Fact] + public void BindingToCustomIndexerWithIndexerNameAttribute() + { + // Test that indexers with [IndexerName("CustomName")] attribute generate correct handlers + // The PropertyChanged events use "CustomName[index]" format, not "Item[index]" + var xaml = +""" + + + +"""; + + var code = +""" +#nullable enable +using System; +using System.Runtime.CompilerServices; +using Microsoft.Maui.Controls; +using Microsoft.Maui.Controls.Xaml; + +namespace Test; + +[XamlProcessing(XamlInflator.SourceGen)] +public partial class TestPage : ContentPage +{ + public TestPage() + { + InitializeComponent(); + } +} + +public class TestViewModel +{ + public IndexedModel Model { get; set; } = new IndexedModel(); +} + +public class IndexedModel +{ + private string[] _values = new string[5]; + + [IndexerName("Indexer")] + public string this[int index] + { + get => _values[index]; + set => _values[index] = value; + } +} +"""; + + var (result, generated) = RunGenerator(xaml, code); + + // Verify no errors + Assert.Empty(result.Diagnostics.Where(d => d.Severity == Microsoft.CodeAnalysis.DiagnosticSeverity.Error)); + Assert.NotNull(generated); + + // Verify the generated code uses "Indexer" (from IndexerName attribute), not "Item" + Assert.Contains("\"Indexer\"", generated, StringComparison.Ordinal); + Assert.Contains("\"Indexer[3]\"", generated, StringComparison.Ordinal); + Assert.DoesNotContain("\"Item\"", generated, StringComparison.Ordinal); + Assert.DoesNotContain("\"Item[3]\"", generated, StringComparison.Ordinal); + + // Verify the getter and setter use the indexer correctly + Assert.Contains("source.Model[3]", generated, StringComparison.Ordinal); + } + + [Fact] + public void BindingToDefaultIndexer() + { + // Test that indexers without [IndexerName] attribute use the default "Item" name + var xaml = +""" + + + +"""; + + var code = +""" +#nullable enable +using System; +using Microsoft.Maui.Controls; +using Microsoft.Maui.Controls.Xaml; + +namespace Test; + +[XamlProcessing(XamlInflator.SourceGen)] +public partial class TestPage : ContentPage +{ + public TestPage() + { + InitializeComponent(); + } +} + +public class TestViewModel +{ + public IndexedModel Model { get; set; } = new IndexedModel(); +} + +public class IndexedModel +{ + private string[] _values = new string[5]; + + public string this[int index] + { + get => _values[index]; + set => _values[index] = value; + } +} +"""; + + var (result, generated) = RunGenerator(xaml, code); + + // Verify no errors + Assert.Empty(result.Diagnostics.Where(d => d.Severity == Microsoft.CodeAnalysis.DiagnosticSeverity.Error)); + Assert.NotNull(generated); + + // Verify the generated code uses "Item" (default indexer name) + Assert.Contains("\"Item\"", generated, StringComparison.Ordinal); + Assert.Contains("\"Item[2]\"", generated, StringComparison.Ordinal); + + // Verify the getter and setter use the indexer correctly + Assert.Contains("source.Model[2]", generated, StringComparison.Ordinal); + } + + [Fact] + public void BindingToListIndexer() + { + // Test binding to List indexer - should use the typed indexer returning T, not object + var xaml = +""" + + + +"""; + + var code = +""" +#nullable enable +using System; +using System.Collections.Generic; +using Microsoft.Maui.Controls; +using Microsoft.Maui.Controls.Xaml; + +namespace Test; + +[XamlProcessing(XamlInflator.SourceGen)] +public partial class TestPage : ContentPage +{ + public TestPage() + { + InitializeComponent(); + } +} + +public class TestViewModel +{ + public List Items { get; set; } = new List { "Item1", "Item2" }; +} +"""; + + var (result, generated) = RunGenerator(xaml, code); + + // Verify no errors + Assert.Empty(result.Diagnostics.Where(d => d.Severity == Microsoft.CodeAnalysis.DiagnosticSeverity.Error)); + Assert.NotNull(generated); + + // Verify the generated TypedBinding uses string, not object (from typed List indexer) + Assert.Contains("TypedBinding", generated, StringComparison.Ordinal); + + // Verify the getter uses the indexer + Assert.Contains("source.Items[0]", generated, StringComparison.Ordinal); + } + + [Fact] + public void BindingToDictionaryIndexer() + { + // Test binding to Dictionary indexer with string key + var xaml = +""" + + + +"""; + + var code = +""" +#nullable enable +using System; +using System.Collections.Generic; +using Microsoft.Maui.Controls; +using Microsoft.Maui.Controls.Xaml; + +namespace Test; + +[XamlProcessing(XamlInflator.SourceGen)] +public partial class TestPage : ContentPage +{ + public TestPage() + { + InitializeComponent(); + } +} + +public class TestViewModel +{ + public Dictionary Data { get; set; } = new Dictionary { ["key1"] = 42 }; +} +"""; + + var (result, generated) = RunGenerator(xaml, code); + + // Verify no errors + Assert.Empty(result.Diagnostics.Where(d => d.Severity == Microsoft.CodeAnalysis.DiagnosticSeverity.Error)); + Assert.NotNull(generated); + + // Verify the generated TypedBinding uses int (the value type of Dictionary) + Assert.Contains("TypedBinding", generated, StringComparison.Ordinal); + + // Verify the getter uses the indexer with string key + Assert.Contains("source.Data[\"key1\"]", generated, StringComparison.Ordinal); + } + + [Fact] + public void BindingToArrayElement() + { + // Test binding to array element - arrays don't have named indexers + var xaml = +""" + + + +"""; + + var code = +""" +#nullable enable +using System; +using Microsoft.Maui.Controls; +using Microsoft.Maui.Controls.Xaml; + +namespace Test; + +[XamlProcessing(XamlInflator.SourceGen)] +public partial class TestPage : ContentPage +{ + public TestPage() + { + InitializeComponent(); + } +} + +public class TestViewModel +{ + public string[] Names { get; set; } = new[] { "Alice", "Bob", "Charlie" }; +} +"""; + + var (result, generated) = RunGenerator(xaml, code); + + // Verify no errors + Assert.Empty(result.Diagnostics.Where(d => d.Severity == Microsoft.CodeAnalysis.DiagnosticSeverity.Error)); + Assert.NotNull(generated); + + // Verify the generated TypedBinding uses string (element type of string[]) + Assert.Contains("TypedBinding", generated, StringComparison.Ordinal); + + // Verify the getter uses array indexer + Assert.Contains("source.Names[1]", generated, StringComparison.Ordinal); + + // For arrays, the handler should use empty string for indexer name (no property to listen to on the array itself) + // The array itself can't notify about element changes - only the containing property can + Assert.Contains("new(static source => source, \"Names\")", generated, StringComparison.Ordinal); + } + + [Fact] + public void BindingToNestedIndexer() + { + // Test binding through multiple levels with indexer + var xaml = +""" + + + +"""; + + var code = +""" +#nullable enable +using System; +using System.Collections.Generic; +using Microsoft.Maui.Controls; +using Microsoft.Maui.Controls.Xaml; + +namespace Test; + +[XamlProcessing(XamlInflator.SourceGen)] +public partial class TestPage : ContentPage +{ + public TestPage() + { + InitializeComponent(); + } +} + +public class TestViewModel +{ + public Container Model { get; set; } = new Container(); +} + +public class Container +{ + public List Items { get; set; } = new List { new Item { Name = "First" } }; +} + +public class Item +{ + public string Name { get; set; } = ""; +} +"""; + + var (result, generated) = RunGenerator(xaml, code); + + // Verify no errors + Assert.Empty(result.Diagnostics.Where(d => d.Severity == Microsoft.CodeAnalysis.DiagnosticSeverity.Error)); + Assert.NotNull(generated); + + // Verify the generated TypedBinding uses string + Assert.Contains("TypedBinding", generated, StringComparison.Ordinal); + + // Verify the getter navigates through the path correctly + Assert.Contains("source.Model.Items[0].Name", generated, StringComparison.Ordinal); + + // Verify handlers for each part of the path + Assert.Contains("\"Model\"", generated, StringComparison.Ordinal); + Assert.Contains("\"Items\"", generated, StringComparison.Ordinal); + Assert.Contains("\"Item\"", generated, StringComparison.Ordinal); + Assert.Contains("\"Item[0]\"", generated, StringComparison.Ordinal); + Assert.Contains("\"Name\"", generated, StringComparison.Ordinal); + } + + [Fact] + public void BindingToIndexerWithTwoWayMode() + { + // Test that two-way binding to indexer generates correct setter + var xaml = +""" + + + + +"""; + + var code = +""" +#nullable enable +using System; +using System.Collections.Generic; +using Microsoft.Maui.Controls; +using Microsoft.Maui.Controls.Xaml; + +namespace Test; + +[XamlProcessing(XamlInflator.SourceGen)] +public partial class TestPage : ContentPage +{ + public TestPage() + { + InitializeComponent(); + } +} + +public class TestViewModel +{ + public List Values { get; set; } = new List { "Initial" }; +} +"""; + + var (result, generated) = RunGenerator(xaml, code); + + // Verify no errors + Assert.Empty(result.Diagnostics.Where(d => d.Severity == Microsoft.CodeAnalysis.DiagnosticSeverity.Error)); + Assert.NotNull(generated); + + // Verify setter is generated for two-way binding + Assert.Contains("source.Values[0] = value", generated, StringComparison.Ordinal); + } + + [Fact] + public void BindingToNullableIndexer() + { + // Test binding to indexer on nullable property - should use conditional access + var xaml = +""" + + + +"""; + + var code = +""" +#nullable enable +using System; +using System.Collections.Generic; +using Microsoft.Maui.Controls; +using Microsoft.Maui.Controls.Xaml; + +namespace Test; + +[XamlProcessing(XamlInflator.SourceGen)] +public partial class TestPage : ContentPage +{ + public TestPage() + { + InitializeComponent(); + } +} + +public class TestViewModel +{ + public List? Model { get; set; } = null; +} +"""; + + var (result, generated) = RunGenerator(xaml, code); + + // Verify no errors + Assert.Empty(result.Diagnostics.Where(d => d.Severity == Microsoft.CodeAnalysis.DiagnosticSeverity.Error)); + Assert.NotNull(generated); + + // Verify the getter uses conditional access for nullable Model + Assert.Contains("source.Model?[0]", generated, StringComparison.Ordinal); + } + + [Fact] + public void BindingToStringIndexer() + { + // Test binding with string indexer key + var xaml = +""" + + + +"""; + + var code = +""" +#nullable enable +using System; +using Microsoft.Maui.Controls; +using Microsoft.Maui.Controls.Xaml; + +namespace Test; + +[XamlProcessing(XamlInflator.SourceGen)] +public partial class TestPage : ContentPage +{ + public TestPage() + { + InitializeComponent(); + } +} + +public class TestViewModel +{ + public SettingsCollection Settings { get; set; } = new SettingsCollection(); +} + +public class SettingsCollection +{ + public string this[string key] + { + get => key == "theme" ? "dark" : "unknown"; + set { } + } +} +"""; + + var (result, generated) = RunGenerator(xaml, code); + + // Verify no errors + Assert.Empty(result.Diagnostics.Where(d => d.Severity == Microsoft.CodeAnalysis.DiagnosticSeverity.Error)); + Assert.NotNull(generated); + + // Verify the getter uses string indexer + Assert.Contains("source.Settings[\"theme\"]", generated, StringComparison.Ordinal); + + // Verify handlers include the indexer with string key + Assert.Contains("\"Item[theme]\"", generated, StringComparison.Ordinal); + } + + [Fact] + public void BindingToReadOnlyIndexer() + { + // Test binding to indexer without setter - should not generate setter for TwoWay + var xaml = +""" + + + + +"""; + + var code = +""" +#nullable enable +using System; +using Microsoft.Maui.Controls; +using Microsoft.Maui.Controls.Xaml; + +namespace Test; + +[XamlProcessing(XamlInflator.SourceGen)] +public partial class TestPage : ContentPage +{ + public TestPage() + { + InitializeComponent(); + } +} + +public class TestViewModel +{ + public ReadOnlyIndexer Values { get; set; } = new ReadOnlyIndexer(); +} + +public class ReadOnlyIndexer +{ + public string this[int index] => index.ToString(); +} +"""; + + var (result, generated) = RunGenerator(xaml, code); + + // Verify no errors + Assert.Empty(result.Diagnostics.Where(d => d.Severity == Microsoft.CodeAnalysis.DiagnosticSeverity.Error)); + Assert.NotNull(generated); + + // Verify setter throws because indexer is read-only + Assert.Contains("throw new global::System.InvalidOperationException", generated, StringComparison.Ordinal); + } +} diff --git a/src/Controls/tests/SourceGen.UnitTests/Maui33532Tests.cs b/src/Controls/tests/SourceGen.UnitTests/Maui33532Tests.cs new file mode 100644 index 000000000000..07241eec92a0 --- /dev/null +++ b/src/Controls/tests/SourceGen.UnitTests/Maui33532Tests.cs @@ -0,0 +1,212 @@ +using System; +using System.Linq; +using Xunit; + +namespace Microsoft.Maui.Controls.SourceGen.UnitTests; + +/// +/// Tests for issue #33532: NaN value in XAML generates invalid code +/// When Padding="NaN" is used, the source generator was generating bare "NaN" instead of "double.NaN" +/// +public class Maui33532Tests : SourceGenXamlInitializeComponentTestBase +{ + [Fact] + public void ThicknessWithSingleNaNValue() + { + // Issue #33532: Padding="NaN" generates invalid code with bare "NaN" instead of "double.NaN" + var xaml = +""" + + +