Skip to content
New issue

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

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

Already on GitHub? Sign in to your account

[XC] Add file info to exception reports #25359

Merged
merged 3 commits into from
Nov 28, 2024
Merged

[XC] Add file info to exception reports #25359

merged 3 commits into from
Nov 28, 2024

Conversation

StephaneDelcroix
Copy link
Contributor

@StephaneDelcroix StephaneDelcroix commented Oct 17, 2024

Description of Change

When the XamlParser throws an exception while executed from XamlC, the file info is missing from the log, and the whole XamlC process fails, so it only reports one issue at a time.

It now logs things like

/Users/sde/Projects/Microsoft/maui/src/Controls/tests/Xaml.UnitTests/Issues/Maui21038.xaml(8,10): XamlC error : 'Label.Text' is a duplicate property name. [/Users/sde/Projects/Microsoft/maui/src/Controls/tests/Xaml.UnitTests/Controls.Xaml.UnitTests.csproj]

Issues Fixed

Fixes #21038

@jsuarezruiz jsuarezruiz added the area-xaml XAML, CSS, Triggers, Behaviors label Oct 18, 2024
@StephaneDelcroix StephaneDelcroix marked this pull request as ready for review October 18, 2024 08:07
@StephaneDelcroix StephaneDelcroix requested a review from a team as a code owner October 18, 2024 08:07
@@ -76,7 +76,7 @@ static void ParseXamlElementFor(IElementNode node, XmlReader reader)
name = new XmlName(reader.NamespaceURI, reader.LocalName);

if (node.Properties.ContainsKey(name))
throw new XamlParseException($"'{reader.Name}' is a duplicate property name.", (IXmlLineInfo)reader);
throw new XamlParseException($"'{reader.Name}' is a duplicate property name.", ((IXmlLineInfo)reader).Clone());
Copy link
Contributor Author

Choose a reason for hiding this comment

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

reader is disposed before the message is logged, so the line info was missing

var rootnode = ParseXaml(resource.GetResourceStream(), typeDef);
if (rootnode == null)
ILRootNode rootnode = null;
try {
Copy link
Contributor Author

Choose a reason for hiding this comment

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

this try/catch is the main change, the rest is pulling #22808

@jsuarezruiz
Copy link
Contributor

@StephaneDelcroix Could you rebase? Thanks in advance.

@PureWeen PureWeen added the p/0 Work that we can't release without label Nov 27, 2024
@PureWeen PureWeen added this to the .NET 9 SR2 milestone Nov 27, 2024
@rmarinho rmarinho merged commit 489657c into main Nov 28, 2024
104 checks passed
@rmarinho rmarinho deleted the missingfileinfo branch November 28, 2024 12:39
@samhouts samhouts added fixed-in-9.0.21 fixed-in-net8.0-nightly This may be available in a nightly release! labels Dec 16, 2024
@github-actions github-actions bot locked and limited conversation to collaborators Jan 17, 2025
Sign up for free to subscribe to this conversation on GitHub. Already have an account? Sign in.
Labels
area-xaml XAML, CSS, Triggers, Behaviors fixed-in-9.0.21 fixed-in-net8.0-nightly This may be available in a nightly release! p/0 Work that we can't release without
Projects
Status: Done
Development

Successfully merging this pull request may close these issues.

XAML errors frequently do not reference the file with an issue
8 participants