Fix XamlC to respect ObsoleteAttribute message and isError parameter #32946
Conversation
Fixes #23961 - Extract and display the custom message from ObsoleteAttribute - Respect the isError parameter: when true, emit XC0619 error instead of XC0618 warning - Add obsolete type detection for classes used in XAML - Add XC0619 (ObsoletePropertyError) for error cases - Update message format to include the attribute's message text - Add unit tests for both warning and error scenarios
There was a problem hiding this comment.
Pull request overview
This PR enhances XamlC to properly handle the ObsoleteAttribute by extracting and displaying custom messages and respecting the isError parameter to differentiate between warnings (XC0618) and errors (XC0619).
- Adds helper methods
TryGetObsoleteAttribute()andLogObsoleteWarningOrError()to extract obsolete attribute information and log appropriate diagnostics - Updates error message resources to include custom messages from
ObsoleteAttribute - Extends obsolete checking to types in addition to properties, fields, and bindable properties
Reviewed changes
Copilot reviewed 9 out of 9 changed files in this pull request and generated 3 comments.
Show a summary per file
| File | Description |
|---|---|
| src/Controls/tests/Xaml.UnitTests/Issues/Maui23961Error.xaml.cs | Test code for obsolete attributes with error: true parameter |
| src/Controls/tests/Xaml.UnitTests/Issues/Maui23961Error.rt.xaml | XAML test file for obsolete error cases |
| src/Controls/tests/Xaml.UnitTests/Issues/Maui23961.xaml.cs | Test code for obsolete attributes with error: false parameter (warnings) |
| src/Controls/tests/Xaml.UnitTests/Issues/Maui23961.xaml | XAML test file for obsolete warning cases |
| src/Controls/tests/Xaml.UnitTests/Controls.Xaml.UnitTests.csproj | Adds XC0619 to WarningsNotAsErrors to allow compilation of test files with obsolete errors |
| src/Controls/src/Build.Tasks/SetPropertiesVisitor.cs | Implements obsolete attribute extraction logic and refactors obsolete checking to use new helper methods |
| src/Controls/src/Build.Tasks/ErrorMessages.resx | Updates error messages for obsolete warnings and errors to include custom messages |
| src/Controls/src/Build.Tasks/CreateObjectVisitor.cs | Adds obsolete type checking when XAML elements are created |
| src/Controls/src/Build.Tasks/BuildException.cs | Adds XC0619 error code for obsolete errors |
| else | ||
| { | ||
| // Runtime doesn't check for obsolete at compile time | ||
| var page = new Maui23961Error(inflator); |
There was a problem hiding this comment.
The constructor call new Maui23961Error(inflator) will fail because the class only has a parameterless constructor public Maui23961Error() => InitializeComponent(); (line 14). This parameterized constructor pattern with XamlInflator is not defined for this class.
Looking at other test files, the XamlInflator parameter constructor is typically auto-generated for regular XAML files (not .rt.xaml). However, this is a .rt.xaml file (runtime-only), which means the auto-generated XamlInflator constructor won't exist.
The test logic on lines 46-56 appears to have incorrect branching. For the XamlInflator.SourceGen case, it correctly recognizes that .rt.xaml files aren't processed, but the else block (line 51-56) attempts to use a non-existent constructor.
Consider revising the test to either:
- Use the parameterless constructor:
var page = new Maui23961Error(); - Skip the runtime test path entirely for
.rt.xamlfiles since they're meant for runtime-only processing
| var page = new Maui23961Error(inflator); | |
| var page = new Maui23961Error(); |
| </data> | ||
| <data name="ObsoleteProperty" xml:space="preserve"> | ||
| <value>Property, Property setter or BindableProperty "{0}" is deprecated.</value> | ||
| <value>"{0}" is deprecated: {1}</value> |
There was a problem hiding this comment.
The error message uses "is obsolete" while the warning message uses "is deprecated". For consistency with C# compiler behavior, both should use the same terminology. The C# compiler uses "is obsolete" for both warnings (CS0618) and errors (CS0619).
Consider changing the warning message from "{0}" is deprecated: {1} to "{0}" is obsolete: {1} to match standard C# compiler terminology and maintain consistency with the error message.
| <value>"{0}" is deprecated: {1}</value> | |
| <value>"{0}" is obsolete: {1}</value> |
| } | ||
|
|
||
| if ( dataTypeNode is ElementNode enode | ||
| if (dataTypeNode is ElementNode enode |
There was a problem hiding this comment.
[nitpick] This whitespace change (removing spaces after if) is unrelated to the PR's purpose of fixing ObsoleteAttribute handling. The PR should focus only on changes directly related to the obsolete attribute functionality to keep the changes minimal and reviewable.
Consider reverting this whitespace change.
| if (dataTypeNode is ElementNode enode | |
| if ( dataTypeNode is ElementNode enode |
Samples use obsolete types like ListView, ViewCell, etc. which now trigger XC0618 warnings. Add these codes to WarningsNotAsErrors to prevent build failures when TreatWarningsAsErrors is enabled.
The TestResultPage.xaml uses Frame which is obsolete.
The ManualTests use obsolete types like TextCell in CollectionView tests.
The test now receives AggregateException when treatWarningsAsErrors is true because both the obsolete Compatibility.StackLayout type and the binding without DataType produce errors. Updated test to use Assert.Catch and verify the expected XC0022 error is present.
Note
Are you waiting for the changes in this PR to be merged?
It would be very helpful if you could test the resulting artifacts from this PR and let us know in a comment if this change resolves your issue. Thank you!
Description
Fixes #23961
This PR fixes XamlC to properly handle the
ObsoleteAttributeby:[Obsolete("message")]instead of a generic deprecation messageisErrorparameter - when[Obsolete("message", error: true)]is used, XamlC now emits an error (XC0619) instead of a warning (XC0618)[Obsolete]when used in XAMLChanges
ErrorMessages.resx: UpdatedObsoletePropertymessage format to include the custom message:"{0}" is deprecated: {1}BuildException.cs: AddedXC0619(ObsoletePropertyError) for error cases whenisError=trueSetPropertiesVisitor.cs:TryGetObsoleteAttribute()helper to extract message and isError from the attributeLogObsoleteWarningOrError()helper that logs the appropriate warning or error based on the attributeCreateObjectVisitor.cs: Added obsolete type detection when XAML elements are createdBefore
After
Testing
Maui23961.xaml/.cs- tests for warnings witherror: falseMaui23961Error.rt.xaml/.cs- tests for errors witherror: true