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

Tracking issue for all possible XAML compiler warnings #13707

Open
3 of 10 tasks
maxkatz6 opened this issue Nov 23, 2023 · 20 comments
Open
3 of 10 tasks

Tracking issue for all possible XAML compiler warnings #13707

maxkatz6 opened this issue Nov 23, 2023 · 20 comments
Labels
area-xaml enhancement help-wanted A contribution from the community would be most welcome.
Milestone

Comments

@maxkatz6
Copy link
Member

maxkatz6 commented Nov 23, 2023

Any other ideas?

@MrJul
Copy link
Member

MrJul commented Nov 23, 2023

Duplicate property, two cases:

Attribute + element:

<Border Background="Blue">
  <Border.Background>Red</Border.Background>
</Border>

Double elements:

<Border>
  <Border.Background>Blue</Border.Background>
  <Border.Background>Red</Border.Background>
</Border>

Double attributes is already a XML parsing error.

@MrJul
Copy link
Member

MrJul commented Nov 23, 2023

Possible null reference exception when adding Transition to the Transitions collection without initializing the collection first

It would be nice to have dependency properties able to instantiate collections lazily instead so users don't have to know whether they should instantiate the collection or not.

Currently we have two types:

  • Always null, e.g. the mentioned Animatable.Transitions. Pain: the user must always set the collection, or it's an exception.
  • Always instantiated, e.g. TextBlock.Inlines. Pain: allocation if unused (most textblocks), double allocations if set in XAML.

I'll open a separate proposal to discuss that.

@MrJul
Copy link
Member

MrJul commented Nov 23, 2023

TemplateBinding used outside of templates. I think I see this one in discussions or Telegram at least once a week :)

@timunie
Copy link
Contributor

timunie commented Nov 23, 2023

Used ContentPresenter and friends outside ControlTeplate, which is forbidden.

@stevemonaco
Copy link
Contributor

Opt-in warnings for {ReflectionBinding} usage so devs writing AOT apps can enforce avoiding its usage.

@maxkatz6 maxkatz6 added this to the 11.2 milestone Nov 29, 2023
@maxkatz6
Copy link
Member Author

XAML compiled should detect following patterns:

<TabControl>
    <TabControl.ItemTemplate>
          <DataTempalte>
                 <TabItem Header="{Binding Title}" /> <!-- WARNING:  -->

And recommend something like this (possibly, just a link to the documentation/samples):

<TabControl ItemsSource="{Binding ...}">
  <TabControl.Styles>
    <Style Selector="TabItem">
      <Setter Property="Header" Value="{Binding Title}"/>
    </Style>
  </TabControl.Styles>
</TabControl>

@Al-Dyachkov
Copy link
Contributor

Al-Dyachkov commented Dec 12, 2023

Is it possible to add a warning when base type declaration in XAML and code behind does not match ?
Common error when a person creates UserControl via template, changes it to ReactiveUserControl and does not immediately modify the code behind to match.

@maxkatz6
Copy link
Member Author

@Al-Dyachkov can you bring some example? If types of XAML and code behind are not compatible, there should already be an error (not even a warning). Unless I am missing something.

ReactiveUserControl in code behind and UserControl in XAML should also be fine.

@Al-Dyachkov
Copy link
Contributor

Al-Dyachkov commented Dec 12, 2023

@maxkatz6

xaml

<reactiveUi:ReactiveUserControl x:TypeArguments="VM" 
                                x:Class="View"
                                x:DataType="VM">
...
</reactiveUi:ReactiveUserControl>

code behind

public partial class View : UserControl
{ 
}

This compiles and even works to some extent because ReactiveUserControl<T> is derived from UserControl, but without Reactive UI features like activation, because it is actually a UserControl. This is a minor issue that is usually fixed right away but would be nice to have a warning anyway.

@workgroupengineering
Copy link
Contributor

What do you think about pinning this issue so it's easy to find?

@workgroupengineering
Copy link
Contributor

I would suggest adding a warning when a DynamicResource is reassigned. I wasted a whole day troubleshooting a problem due to this.

@timunie
Copy link
Contributor

timunie commented Feb 16, 2024

I would suggest adding a warning when a DynamicResource is reassigned. I wasted a whole day troubleshooting a problem due to this.

@workgroupengineering can you give an example?

@workgroupengineering
Copy link
Contributor

I would suggest adding a warning when a DynamicResource is reassigned. I wasted a whole day troubleshooting a problem due to this.

@workgroupengineering can you give an example?

A third-party control defines within it a DynamicResource with the same name as the one I created.

Application xmlns="https://github.com/avaloniaui"
             xmlns:x="http://schemas.microsoft.com/winfx/2006/xaml"
             xmlns:local="using:Dresses.Warehouse"
             x:Class="My.App"
             >
  <!-- "Default" ThemeVariant follows system theme variant. "Dark" or "Light" are other available options. -->

  <Application.Resources>
      <x:Double x:Key="SmallIconSize">18</x:Double>
  </Application.Resources>

  <Application.Styles>
    <FluentTheme />
  </Application.Styles>
</Application>
<UserControl xmlns="https://github.com/avaloniaui"
             xmlns:x="http://schemas.microsoft.com/winfx/2006/xaml"
             xmlns:d="http://schemas.microsoft.com/expression/blend/2008"
             xmlns:mc="http://schemas.openxmlformats.org/markup-compatibility/2006"
             x:Class="My.Views.MainView"
             x:ClassModifier="internal"
             >
 <UserContro.Resources>

    <ResourceDictionary>
      <ResourceDictionary.MergedDictionaries>
        <!-- internally redefines the resource SmallIconSize 24 -->
        <ResourceInclude Source="avares://third_party/third_party_Control.axaml"/>
      </ResourceDictionary.MergedDictionaries>
   </ResourceDictionary>
 </UserContro.Resources>
 <!-- The size is 24 instead of the expected 16, this is because I don't know Third_party_Control.axaml redefines SmallIconSize. -->
 <Rectangle Fill="Red"
             Width="{DynamicResource SmallIconSize}"
             Height="{DynamicResource SmallIconSize}"/>
</UserControl >

@workgroupengineering
Copy link
Contributor

workgroupengineering commented Feb 16, 2024

Add Warning suppression il xaml like

<!--#Suppress-Start: AVX11111 - Reason -->
  ...
<!--#Suppress-End: AVX11111-->

@timunie
Copy link
Contributor

timunie commented Feb 16, 2024

A third-party control defines within it a DynamicResource with the same name as the one I created.

That's the exactly what a dynamic resource should do imo 🤔

@workgroupengineering
Copy link
Contributor

A third-party control defines within it a DynamicResource with the same name as the one I created.

That's the exactly what a dynamic resource should do imo 🤔

but if I don't know that it was redefined by another library I will get unexpected behavior. It just needs to show a warning like SmallIconSize has been redefined in avares://third_party/third_party_Control.axaml

@maxkatz6
Copy link
Member Author

Sounds as an expected possibility.

It just needs to show a warning like SmallIconSize has been redefined

We currently don't have this information. As we don't store such metadata.

Add Warning suppression il xaml like

Yeah, we need something for that.

@timunie
Copy link
Contributor

timunie commented May 3, 2024

yet another one (maybe) #15593

@workgroupengineering
Copy link
Contributor

Add warning when local value override style selector

  <Window.Styles>
    <Style Selector=":is(StackPanel) Rectangle.Go">
      <Setter Property="Fill" Value="Red"/>
    </Style>
    <Style Selector=":is(StackPanel) Rectangle:not(.Go)">
      <Setter Property="Fill" Value="Blue"/>
    </Style>
  </Window.Styles>
  <StackPanel>
    <Rectangle Width="10" Height="10" Fill="Green"/>  <!-- Add  warning this line -->
  </StackPanel>

@marklam
Copy link

marklam commented Aug 21, 2024

Is it possible to add a warning when base type declaration in XAML and code behind does not match ? Common error when a person creates UserControl via template, changes it to ReactiveUserControl and does not immediately modify the code behind to match.

As raised in #16755, If the class in the code-behind is Control and the XAML is UserControl, it causes an Internal Compiler Error in the XAML compiler, so this mistake can be serious (and very hard to diagnose).

@maxkatz6 maxkatz6 added the help-wanted A contribution from the community would be most welcome. label Aug 22, 2024
@maxkatz6 maxkatz6 modified the milestones: 11.2, 11.x Oct 1, 2024
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
area-xaml enhancement help-wanted A contribution from the community would be most welcome.
Projects
None yet
Development

No branches or pull requests

7 participants