From 98cf91c3853ad8935db98f6d7ab201e9983e9b84 Mon Sep 17 00:00:00 2001 From: Max Katz Date: Wed, 30 Nov 2022 02:25:48 -0500 Subject: [PATCH 1/5] Properly support relative Uris in StyleInclude/ResourceInclude --- .../AvaloniaXamlIlLanguageParseIntrinsics.cs | 2 +- .../XamlIncludeGroupTransformer.cs | 21 +++-- .../Converters/AvaloniaUriTypeConverter.cs | 2 +- .../Xaml/StyleIncludeTests.cs | 88 ++++++++++++++++++- 4 files changed, 100 insertions(+), 13 deletions(-) diff --git a/src/Markup/Avalonia.Markup.Xaml.Loader/CompilerExtensions/AvaloniaXamlIlLanguageParseIntrinsics.cs b/src/Markup/Avalonia.Markup.Xaml.Loader/CompilerExtensions/AvaloniaXamlIlLanguageParseIntrinsics.cs index 64fdfe155d..925bf0a4fa 100644 --- a/src/Markup/Avalonia.Markup.Xaml.Loader/CompilerExtensions/AvaloniaXamlIlLanguageParseIntrinsics.cs +++ b/src/Markup/Avalonia.Markup.Xaml.Loader/CompilerExtensions/AvaloniaXamlIlLanguageParseIntrinsics.cs @@ -274,7 +274,7 @@ namespace Avalonia.Markup.Xaml.XamlIl.CompilerExtensions { var uriText = text.Trim(); - var kind = ((!uriText?.StartsWith("/") == true) ? UriKind.Absolute : UriKind.Relative); + var kind = ((!uriText?.StartsWith("/") == true) ? UriKind.RelativeOrAbsolute : UriKind.Relative); if (string.IsNullOrWhiteSpace(uriText) || !Uri.TryCreate(uriText, kind, out var _)) { diff --git a/src/Markup/Avalonia.Markup.Xaml.Loader/CompilerExtensions/GroupTransformers/XamlIncludeGroupTransformer.cs b/src/Markup/Avalonia.Markup.Xaml.Loader/CompilerExtensions/GroupTransformers/XamlIncludeGroupTransformer.cs index 09a65c9d64..f0895de5e2 100644 --- a/src/Markup/Avalonia.Markup.Xaml.Loader/CompilerExtensions/GroupTransformers/XamlIncludeGroupTransformer.cs +++ b/src/Markup/Avalonia.Markup.Xaml.Loader/CompilerExtensions/GroupTransformers/XamlIncludeGroupTransformer.cs @@ -36,34 +36,37 @@ internal class AvaloniaXamlIncludeTransformer : IXamlAstGroupTransformer return context.ParseError($"Source property must be set on the \"{nodeTypeName}\" node.", node); } + // We expect that AvaloniaXamlIlLanguageParseIntrinsics has already parsed the Uri and created node like: `new Uri(assetPath, uriKind)`. if (sourceProperty.Values.OfType().FirstOrDefault() is not { } sourceUriNode || sourceUriNode.Type.GetClrType() != context.GetAvaloniaTypes().Uri - || sourceUriNode.Arguments.FirstOrDefault() is not XamlConstantNode { Constant: string originalAssetPath }) + || sourceUriNode.Arguments.FirstOrDefault() is not XamlConstantNode { Constant: string originalAssetPath } + || sourceUriNode.Arguments.Skip(1).FirstOrDefault() is not XamlConstantNode { Constant: int uriKind }) { // TODO: make it a compiler warning // Source value can be set with markup extension instead of the Uri object node, we don't support it here yet. return node; } - if (originalAssetPath.StartsWith("/")) + var uriPath = new Uri(originalAssetPath, (UriKind)uriKind); + if (!uriPath.IsAbsoluteUri) { var baseUrl = context.CurrentDocument.Uri ?? throw new InvalidOperationException("CurrentDocument URI is null."); - originalAssetPath = baseUrl.Substring(0, baseUrl.LastIndexOf('/')) + originalAssetPath; + uriPath = new Uri(new Uri(baseUrl, UriKind.Absolute), uriPath); } - else if (!originalAssetPath.StartsWith("avares://")) + else if (!uriPath.Scheme.Equals("avares", StringComparison.CurrentCultureIgnoreCase)) { return context.ParseError( $"Avalonia supports only \"avares://\" sources or relative sources starting with \"/\" on the \"{nodeTypeName}\" node.", node); } - originalAssetPath = Uri.UnescapeDataString(new Uri(originalAssetPath).AbsoluteUri); - var assetPath = originalAssetPath.Replace("avares://", ""); + var assetPathUri = Uri.UnescapeDataString(uriPath.AbsoluteUri); + var assetPath = assetPathUri.Replace("avares://", ""); var assemblyNameSeparator = assetPath.IndexOf('/'); var assembly = assetPath.Substring(0, assemblyNameSeparator); var fullTypeName = Path.GetFileNameWithoutExtension(assetPath.Replace('/', '.')); - if (context.Documents.FirstOrDefault(d => string.Equals(d.Uri, originalAssetPath, StringComparison.InvariantCultureIgnoreCase)) is {} targetDocument) + if (context.Documents.FirstOrDefault(d => string.Equals(d.Uri, assetPathUri, StringComparison.InvariantCultureIgnoreCase)) is {} targetDocument) { if (targetDocument.ClassType is not null) { @@ -72,7 +75,7 @@ internal class AvaloniaXamlIncludeTransformer : IXamlAstGroupTransformer if (targetDocument.BuildMethod is null) { - return context.ParseError($"\"{originalAssetPath}\" cannot be instantiated.", node); + return context.ParseError($"\"{assetPathUri}\" cannot be instantiated.", node); } return FromMethod(context, targetDocument.BuildMethod, node); @@ -81,7 +84,7 @@ internal class AvaloniaXamlIncludeTransformer : IXamlAstGroupTransformer if (context.Configuration.TypeSystem.FindAssembly(assembly) is not { } assetAssembly) { - return context.ParseError($"Assembly \"{assembly}\" was not found from the \"{originalAssetPath}\" source.", node); + return context.ParseError($"Assembly \"{assembly}\" was not found from the \"{assetPathUri}\" source.", node); } if (assetAssembly.FindType(fullTypeName) is { } type diff --git a/src/Markup/Avalonia.Markup.Xaml/Converters/AvaloniaUriTypeConverter.cs b/src/Markup/Avalonia.Markup.Xaml/Converters/AvaloniaUriTypeConverter.cs index 3c3a718213..54c57d8b7b 100644 --- a/src/Markup/Avalonia.Markup.Xaml/Converters/AvaloniaUriTypeConverter.cs +++ b/src/Markup/Avalonia.Markup.Xaml/Converters/AvaloniaUriTypeConverter.cs @@ -17,7 +17,7 @@ namespace Avalonia.Markup.Xaml.Converters if (s == null) return null; //On Unix Uri tries to interpret paths starting with "/" as file Uris - var kind = s.StartsWith("/") ? UriKind.Relative : UriKind.Absolute; + var kind = s.StartsWith("/") ? UriKind.Relative : UriKind.RelativeOrAbsolute; if (!Uri.TryCreate(s, kind, out var res)) throw new ArgumentException("Unable to parse URI: " + s); return res; diff --git a/tests/Avalonia.Markup.Xaml.UnitTests/Xaml/StyleIncludeTests.cs b/tests/Avalonia.Markup.Xaml.UnitTests/Xaml/StyleIncludeTests.cs index 4ff4405109..8eed5013a2 100644 --- a/tests/Avalonia.Markup.Xaml.UnitTests/Xaml/StyleIncludeTests.cs +++ b/tests/Avalonia.Markup.Xaml.UnitTests/Xaml/StyleIncludeTests.cs @@ -79,7 +79,35 @@ public class StyleIncludeTests } [Fact] - public void Relative_StyleInclude_Is_Resolved_With_Two_Files() + public void Relative_Back_StyleInclude_Is_Resolved_With_Two_Files() + { + var documents = new[] + { + new RuntimeXamlLoaderDocument(new Uri("avares://Tests/Subfolder/Style.xaml"), @" +"), + new RuntimeXamlLoaderDocument(new Uri("avares://Tests/Subfolder/Folder/Root.xaml"), @" + + + + +") + }; + + var objects = AvaloniaRuntimeXamlLoader.LoadGroup(documents); + var style = Assert.IsType"), + new RuntimeXamlLoaderDocument(new Uri("avares://Tests/Folder/Root.xaml"), @" + + + + +") + }; + + var objects = AvaloniaRuntimeXamlLoader.LoadGroup(documents); + var style = Assert.IsType"), + new RuntimeXamlLoaderDocument(new Uri("avares://Tests/Folder/Root.xaml"), @" + + + ") }; From 2e6fc861941f635be9ac47be7943472b08c10133 Mon Sep 17 00:00:00 2001 From: Max Katz Date: Wed, 30 Nov 2022 02:29:32 -0500 Subject: [PATCH 2/5] Make error clear when Source is not set on xaml include (#3788) --- src/Markup/Avalonia.Markup.Xaml/Styling/ResourceInclude.cs | 3 ++- src/Markup/Avalonia.Markup.Xaml/Styling/StyleInclude.cs | 3 ++- 2 files changed, 4 insertions(+), 2 deletions(-) diff --git a/src/Markup/Avalonia.Markup.Xaml/Styling/ResourceInclude.cs b/src/Markup/Avalonia.Markup.Xaml/Styling/ResourceInclude.cs index 01db2c081f..d7eb328715 100644 --- a/src/Markup/Avalonia.Markup.Xaml/Styling/ResourceInclude.cs +++ b/src/Markup/Avalonia.Markup.Xaml/Styling/ResourceInclude.cs @@ -42,7 +42,8 @@ namespace Avalonia.Markup.Xaml.Styling if (_loaded == null) { _isLoading = true; - _loaded = (IResourceDictionary)AvaloniaXamlLoader.Load(Source, _baseUri); + var source = Source ?? throw new InvalidOperationException("ResourceInclude.Source must be set."); + _loaded = (IResourceDictionary)AvaloniaXamlLoader.Load(source, _baseUri); _isLoading = false; } diff --git a/src/Markup/Avalonia.Markup.Xaml/Styling/StyleInclude.cs b/src/Markup/Avalonia.Markup.Xaml/Styling/StyleInclude.cs index 8af49b5480..2b75e0dd54 100644 --- a/src/Markup/Avalonia.Markup.Xaml/Styling/StyleInclude.cs +++ b/src/Markup/Avalonia.Markup.Xaml/Styling/StyleInclude.cs @@ -51,7 +51,8 @@ namespace Avalonia.Markup.Xaml.Styling if (_loaded == null) { _isLoading = true; - var loaded = (IStyle)AvaloniaXamlLoader.Load(Source, _baseUri); + var source = Source ?? throw new InvalidOperationException("StyleInclude.Source must be set."); + var loaded = (IStyle)AvaloniaXamlLoader.Load(source, _baseUri); _loaded = new[] { loaded }; _isLoading = false; } From 5e95e07ce29fbc475bc0324d2a83ed2837826ff7 Mon Sep 17 00:00:00 2001 From: Max Katz Date: Wed, 30 Nov 2022 02:33:37 -0500 Subject: [PATCH 3/5] Run xaml after CoreCompile --- packages/Avalonia/AvaloniaBuildTasks.targets | 2 +- 1 file changed, 1 insertion(+), 1 deletion(-) diff --git a/packages/Avalonia/AvaloniaBuildTasks.targets b/packages/Avalonia/AvaloniaBuildTasks.targets index 1b822c14cf..5da76e414c 100644 --- a/packages/Avalonia/AvaloniaBuildTasks.targets +++ b/packages/Avalonia/AvaloniaBuildTasks.targets @@ -80,7 +80,7 @@ From dc5aa99d7aac152c0eec2666aacea51aab0a9968 Mon Sep 17 00:00:00 2001 From: Max Katz Date: Wed, 30 Nov 2022 03:21:22 -0500 Subject: [PATCH 4/5] Revert "Run xaml after CoreCompile" This reverts commit 5e95e07ce29fbc475bc0324d2a83ed2837826ff7. --- packages/Avalonia/AvaloniaBuildTasks.targets | 2 +- 1 file changed, 1 insertion(+), 1 deletion(-) diff --git a/packages/Avalonia/AvaloniaBuildTasks.targets b/packages/Avalonia/AvaloniaBuildTasks.targets index 5da76e414c..1b822c14cf 100644 --- a/packages/Avalonia/AvaloniaBuildTasks.targets +++ b/packages/Avalonia/AvaloniaBuildTasks.targets @@ -80,7 +80,7 @@ From c0c67d8bdd9445622b833e323606a434d5e3a2c5 Mon Sep 17 00:00:00 2001 From: Max Katz Date: Wed, 30 Nov 2022 03:46:39 -0500 Subject: [PATCH 5/5] Improve error messages and add type checking --- .../XamlIncludeGroupTransformer.cs | 88 ++++++++++++------- 1 file changed, 55 insertions(+), 33 deletions(-) diff --git a/src/Markup/Avalonia.Markup.Xaml.Loader/CompilerExtensions/GroupTransformers/XamlIncludeGroupTransformer.cs b/src/Markup/Avalonia.Markup.Xaml.Loader/CompilerExtensions/GroupTransformers/XamlIncludeGroupTransformer.cs index f0895de5e2..cc29d5ccb5 100644 --- a/src/Markup/Avalonia.Markup.Xaml.Loader/CompilerExtensions/GroupTransformers/XamlIncludeGroupTransformer.cs +++ b/src/Markup/Avalonia.Markup.Xaml.Loader/CompilerExtensions/GroupTransformers/XamlIncludeGroupTransformer.cs @@ -28,6 +28,13 @@ internal class AvaloniaXamlIncludeTransformer : IXamlAstGroupTransformer } var nodeTypeName = objectNode.Type.GetClrType().Name; + var expectedLoadedType = objectNode.Type.GetClrType().GetAllProperties() + .FirstOrDefault(p => p.Name == "Loaded")?.PropertyType; + if (expectedLoadedType is null) + { + throw new InvalidOperationException($"\"{nodeTypeName}\".Loaded property is expected to be defined"); + } + if (valueNode.Manipulation is not XamlObjectInitializationNode { Manipulation: XamlPropertyAssignmentNode { Property: { Name: "Source" } } sourceProperty @@ -56,8 +63,8 @@ internal class AvaloniaXamlIncludeTransformer : IXamlAstGroupTransformer else if (!uriPath.Scheme.Equals("avares", StringComparison.CurrentCultureIgnoreCase)) { return context.ParseError( - $"Avalonia supports only \"avares://\" sources or relative sources starting with \"/\" on the \"{nodeTypeName}\" node.", - node); + $"\"{nodeTypeName}.Source\" supports only \"avares://\" absolute or relative uri.", + sourceUriNode, node); } var assetPathUri = Uri.UnescapeDataString(uriPath.AbsoluteUri); @@ -66,64 +73,79 @@ internal class AvaloniaXamlIncludeTransformer : IXamlAstGroupTransformer var assembly = assetPath.Substring(0, assemblyNameSeparator); var fullTypeName = Path.GetFileNameWithoutExtension(assetPath.Replace('/', '.')); + // Search file in the current assembly among other XAML resources. if (context.Documents.FirstOrDefault(d => string.Equals(d.Uri, assetPathUri, StringComparison.InvariantCultureIgnoreCase)) is {} targetDocument) { - if (targetDocument.ClassType is not null) + if (targetDocument.BuildMethod is not null) { - return FromType(context, targetDocument.ClassType, node); + return FromMethod(context, targetDocument.BuildMethod, sourceUriNode, expectedLoadedType, node, assetPathUri, assembly); } - if (targetDocument.BuildMethod is null) + if (targetDocument.ClassType is not null) { - return context.ParseError($"\"{assetPathUri}\" cannot be instantiated.", node); + return FromType(context, targetDocument.ClassType, sourceUriNode, expectedLoadedType, node, assetPathUri, assembly); } - return FromMethod(context, targetDocument.BuildMethod, node); + return context.ParseError( + $"Unable to resolve XAML resource \"{assetPathUri}\" in the current assembly.", + sourceUriNode, node); } - + // If resource wasn't found in the current assembly, search in the others. if (context.Configuration.TypeSystem.FindAssembly(assembly) is not { } assetAssembly) { - return context.ParseError($"Assembly \"{assembly}\" was not found from the \"{assetPathUri}\" source.", node); + return context.ParseError($"Assembly \"{assembly}\" was not found from the \"{assetPathUri}\" source.", sourceUriNode, node); } - if (assetAssembly.FindType(fullTypeName) is { } type - && type.FindMethod(m => m.Name == "!XamlIlPopulate") is not null) + var avaResType = assetAssembly.FindType("CompiledAvaloniaXaml.!AvaloniaResources"); + if (avaResType is null) { - return FromType(context, type, node); + return context.ParseError( + $"Unable to resolve \"!AvaloniaResources\" type on \"{assembly}\" assembly.", sourceUriNode, node); } - else - { - var avaResType = assetAssembly.FindType("CompiledAvaloniaXaml.!AvaloniaResources"); - if (avaResType is null) - { - return context.ParseError( - $"Unable to resolve \"!AvaloniaResources\" type on \"{assembly}\" assembly.", node); - } - - var relativeName = "Build:" + assetPath.Substring(assemblyNameSeparator); - var buildMethod = avaResType.FindMethod(m => m.Name == relativeName); - if (buildMethod is null) - { - return context.ParseError( - $"Unable to resolve build method \"{relativeName}\" resource on the \"{assembly}\" assembly.", - node); - } - return FromMethod(context, buildMethod, node); + var relativeName = "Build:" + assetPath.Substring(assemblyNameSeparator); + var buildMethod = avaResType.FindMethod(m => m.Name == relativeName); + if (buildMethod is not null) + { + return FromMethod(context, buildMethod, sourceUriNode, expectedLoadedType, node, assetPathUri, assembly); + } + else if (assetAssembly.FindType(fullTypeName) is { } type) + { + return FromType(context, type, sourceUriNode, expectedLoadedType, node, assetPathUri, assembly); } - } - private static IXamlAstNode FromType(AstTransformationContext context, IXamlType type, IXamlLineInfo li) + return context.ParseError( + $"Unable to resolve XAML resource \"{assetPathUri}\" in the \"{assembly}\" assembly.", + sourceUriNode, node); + } + + private static IXamlAstNode FromType(AstTransformationContext context, IXamlType type, IXamlAstNode li, + IXamlType expectedLoadedType, IXamlAstNode fallbackNode, string assetPathUri, string assembly) { + if (!expectedLoadedType.IsAssignableFrom(type)) + { + return context.ParseError( + $"Resource \"{assetPathUri}\" is defined as \"{type}\" type in the \"{assembly}\" assembly, but expected \"{expectedLoadedType}\".", + li, fallbackNode); + } + IXamlAstNode newObjNode = new XamlAstObjectNode(li, new XamlAstClrTypeReference(li, type, false)); newObjNode = new AvaloniaXamlIlConstructorServiceProviderTransformer().Transform(context, newObjNode); newObjNode = new ConstructableObjectTransformer().Transform(context, newObjNode); return new NewObjectTransformer().Transform(context, newObjNode); } - private static IXamlAstNode FromMethod(AstTransformationContext context, IXamlMethod method, IXamlLineInfo li) + private static IXamlAstNode FromMethod(AstTransformationContext context, IXamlMethod method, IXamlAstNode li, + IXamlType expectedLoadedType, IXamlAstNode fallbackNode, string assetPathUri, string assembly) { + if (!expectedLoadedType.IsAssignableFrom(method.ReturnType)) + { + return context.ParseError( + $"Resource \"{assetPathUri}\" is defined as \"{method.ReturnType}\" type in the \"{assembly}\" assembly, but expected \"{expectedLoadedType}\".", + li, fallbackNode); + } + var sp = context.Configuration.TypeMappings.ServiceProvider; return new XamlStaticOrTargetedReturnMethodCallNode(li, method, new[] { new NewServiceProviderNode(sp, li) });