Add a Roslyn analyzer to detect parameter/attribute issues

[peterthorpe81]

This PR adds an analyzer to MudBlazor to detect issues with v7 parameters at compile time (unless its dynamic code). It also adds a similar analyzer to specifically not allow unknown attributes. Its originally based on https://github.com/meziantou/Meziantou.Analyzer.

The analyzer has been added to the projects in a specific way so that it applies to MudBlazor.Docs and transitively to any project that uses MudBlazor nuget package There is also the potential to have it as a separate nuget project if deemed more appropriate.

Description

The analyzers will emit two warnings: MUD0001 - Detects parameters in the specific V7 list of removed/renamed parameters MUD0002 - Identifies parameters/attributes that don't appear to be appropriate for the component e.g. a typo

The Analyzers can be configured within a .csproj by adding this section:

<PropertyGroup>
    <MudAllowedAttributePattern>LowerCase</MudAllowedAttributePattern>
    <MudIllegalParameters>V7CaseSensitive</MudIllegalParameters>
</PropertyGroup>

<ItemGroup>
   <CompilerVisibleProperty Include="MudAllowedAttributePattern" />
   <CompilerVisibleProperty Include="MudIllegalParameters" />
</ItemGroup>

MudAllowedAttributePattern supports the options:

• LowerCase - Only allow lower case attributes
• DataAndAria - Only allow data-* and aria-* attributes
• None - Don't allow any extra attributes
• Any - Allow any extra attributes

MudIllegalParameters support the options:

• V7CaseSensitive - Match the v7 parameter list with case sensitivity
• V7IgnoreCase - Match the v7 parameter list ignoring case
• None - Don't run this check

Notes

• I have specifically configured MudBlazor.Docs with the attribute pattern LowerCase to demonstrate detection of potential attribute issues. This will mean the project won't build unless that is changed.
• Inherited controls should be correctly handled
• Visual studio sometimes seems to cache the error list window and sometimes results are only visible on "Build only" not on "Build Only + Intellisense"
• CompilerVisibleProperty tag should be able to be embedded in the MudBlazor.props file rather than a consumer csproj but I don't seem to be able to get that to be added without manually editing the nuget output

How Has This Been Tested?

Visually tested with MudBlazor docs and personal projects. Unit tests may be possible as https://github.com/meziantou/Meziantou.Analyzer has some using XUnit but it does appear to be quite complicated.

Type of Changes

• [ ] Bug fix (non-breaking change which fixes an issue)
• [X] New feature (non-breaking change which adds functionality)
• [ ] Breaking change (fix or feature that would cause existing functionality to not work as expected)
• [ ] Documentation (fix or improvement to the website or code docs)

Checklist

• [X] The PR is submitted to the correct branch (dev).
• [X] My code follows the code style of this project.
• [ ] I've added relevant tests.

[ScarletKuro]

Hi. Excellent work! It seems like a lot of hard work went into this. It already detects the illegal parameters that we have: https://github.com/MudBlazor/MudBlazor/blob/24687738e712c2d2a4733d543a3b1ea33d80051a/src/MudBlazor.Docs/Pages/Components/DropZone/Examples/DropZoneItemSelectorOnlyZoneExample.razor#L3 The Item after the Items or the Inline in MudText.

I have a couple of questions:

1. Since it's added transitively to any project that uses the MudBlazor nuget package, is there a property for the .csproj file to completely disable this whole thing? Some people might want to disable it for IDE performance reasons.
2. Are tests really that hard? You mentioned that Meziantou's analyzer has tests and it seems complicated. Can we explore other analyzers? I am slightly concerned about not having tests, because it will be easy to break if we want to add something new. In that case, nobody will want to touch it because there are no tests, making it scary to potentially break something.

[danielchalmers]

The illegal parameter detection for first-party components is a neat evolution of the current one that throws at runtime.

I think that detecting possibly bad attributes on MudBlazor components is shaky, but detecting them on other controls as well way oversteps the bounds of the library. Maybe if it was opt-in but then I imagine very few people would and there's a large maintenance cost. Same issue with a separate package. I might have misunderstood the proposal though.

Unit tests are crucial when it's so potentially disruptive to a user's build.

Other risk I can think of is whether or not anyone who is active on MudBlazor is proficient in analyzers to be able to fix things that come up.

[henon]

Other risk I can think of is whether or not anyone who is active on MudBlazor is proficient in analyzers to be able to fix things that come up.

Maybe @peterthorpe81 wants to join the team? Then this risk would be smaller, but we have this with everything. Team members have been known to abandon us. Others will come and replace them eventually. Of course this speaks for the need of tests.

[henon]

Actually, the tests in meziantou's repo don't look complicated at all. Basically he builds a c# source snippet, or (in our case) a razor component and executes the analyzer on it.

    [Theory]
    [InlineData("Param1")]
    [InlineData("Param2")]
    public async Task ValidParameterName(string parameterName)
    {
        var sourceCode = $$"""
class TypeName : ComponentBase
{
    protected override void BuildRenderTree(Microsoft.AspNetCore.Components.Rendering.RenderTreeBuilder __builder)
    {
        __builder.OpenComponent<SampleComponent>(0);
        __builder.AddAttribute(1, "{{parameterName}}", "test");
        __builder.CloseComponent();
    }
}
""";
        await CreateProjectBuilder()
              .WithSourceCode(Usings + sourceCode + ComponentWithParameters)
              .ValidateAsync();
    }

But maybe I don't understand yet where the difficulties really lie.

[peterthorpe81]

Hi. Excellent work! It seems like a lot of hard work went into this. It already detects the illegal parameters that we have:

https://github.com/MudBlazor/MudBlazor/blob/24687738e712c2d2a4733d543a3b1ea33d80051a/src/MudBlazor.Docs/Pages/Components/DropZone/Examples/DropZoneItemSelectorOnlyZoneExample.razor#L3

The Item after the Items or the Inline in MudText. I have a couple of questions:

1. Since it's added transitively to any project that uses the MudBlazor nuget package, is there a property for the .csproj file to completely disable this whole thing? Some people might want to disable it for IDE performance reasons.
2. Are tests really that hard? You mentioned that Meziantou's analyzer has tests and it seems complicated. Can we explore other analyzers? I am slightly concerned about not having tests, because it will be easy to break if we want to add something new. In that case, nobody will want to touch it because there are no tests, making it scary to potentially break something.

Its been quite an interesting little project to get an analyzer working. Razor source gen adds some complexity.

1. Good point, it would have still run some looping logic. I have just added a commit for an earlier exit so it only hits checking the config and exits if its disabled (Any attribute + None Illegal Parameters). The example below would just run on a Release build:

<PropertyGroup Condition=" '$(Configuration)' == 'Release' ">
  <MudAllowedAttributePattern>LowerCase</MudAllowedAttributePattern>
  <MudIllegalParameters>V7IgnoreCase</MudIllegalParameters>
</PropertyGroup>

<PropertyGroup Condition=" '$(Configuration)' == 'Debug' ">
  <MudAllowedAttributePattern>Any</MudAllowedAttributePattern>
  <MudIllegalParameters>None</MudIllegalParameters>
</PropertyGroup>

2. Its probably doable I will have another look. I'm not particularly experienced with BUnit or XUnit.

[peterthorpe81]

The illegal parameter detection for first-party components is a neat evolution of the current one that throws at runtime.

I think that detecting possibly bad attributes on MudBlazor components is shaky, but detecting them on other controls as well way oversteps the bounds of the library. Maybe if it was opt-in but then I imagine very few people would and there's a large maintenance cost. Same issue with a separate package. I might have misunderstood the proposal though.

Unit tests are crucial when it's so potentially disruptive to a user's build.

Other risk I can think of is whether or not anyone who is active on MudBlazor is proficient in analyzers to be able to fix things that come up.

I probably didn't explain bad attributes well. Its only applied to inheritors of MudComponentBase so should be the core library and then I guess if anyone inherits from a component which I guess is questionable. The main point for me was to detect silly mistakes like typos in parameters which then just go in as html attributes with splatting enabled.

You can configure it so the current default is to allow through any lower case (first letter) attributes. That would seem to make sense as html attributes should be lower case and parameters normally begin with upper case. You can currently configure it to allow Any, DataAndAria, None extra attributes.

I will take another look at tests

[peterthorpe81]

Actually, the tests in meziantou's repo don't look complicated at all. Basically he builds a c# source snippet, or (in our case) a razor component and executes the analyzer on it.

    [Theory]
    [InlineData("Param1")]
    [InlineData("Param2")]
    public async Task ValidParameterName(string parameterName)
    {
        var sourceCode = $$"""
class TypeName : ComponentBase
{
    protected override void BuildRenderTree(Microsoft.AspNetCore.Components.Rendering.RenderTreeBuilder __builder)
    {
        __builder.OpenComponent<SampleComponent>(0);
        __builder.AddAttribute(1, "{{parameterName}}", "test");
        __builder.CloseComponent();
    }
}
""";
        await CreateProjectBuilder()
              .WithSourceCode(Usings + sourceCode + ComponentWithParameters)
              .ValidateAsync();
    }

But maybe I don't understand yet where the difficulties really lie.

I will take another look at tests i'm not particularly experienced with BUnit so perhaps was a bit quick to dismiss testability.

[ScarletKuro]

I'm not particularly experienced with bUnit.

bUnit is not used there. And I don't think it's necessary. bUnit is for parsing components and rendering them. Here, you just need to create the final generated Razor code and run it through the analyzer, just like in @henon's example.

The main point for me was to detect silly mistakes like typos in parameters, which then just go in as HTML attributes with splatting enabled.

I heard Meziantou got this support recently as well, so you might want to double-check.

[ScarletKuro]

Hi, sorry for pushing this, but do you have any ETA for when you can look at the testing? We plan to make RC1 soon, and it would be nice if this could be shipped with RC.

[Nickztar]

Hi! I really like working with analyzers/source generators and thought it would be a fun project to write some tests for this. So i've decided to pick it up. Not sure if Peter is still working on this but i've implemented the testing framework, hugely based on Meziantou framework for testing their analyzers. It is simple enough to validate that the analyzers generate the correct diagnostics, (Both in IDE and tests). Not sure how to best work together on this. Should I create a new fork and new PR or try and contribute to this PR?

[peterthorpe81]

Hi, sorry for pushing this, but do you have any ETA for when you can look at the testing? We plan to make RC1 soon, and it would be nice if this could be shipped with RC.

Yeah sorry I have been a bit quite about it.

I had a good look into testing. The thing about Meziantou's I wasn't keen on for Blazor was it creates a project in code adds references to nugets... adds the source generated razor component from a string version and then compiles it and runs the analyzers.

It's quite code heavy and would need quite a bit of maintenance as things change. Blazor wise that also means it bypasses the actual source generator. This also makes complex tests cumbersome as things like inheritance mean adding lots of stringified classes.

Sonarsource actually creates a physical project with the test components in and compiles it and analyzes it meaning you can actually use .razor files. Something I am doing (or attempting to) that Meziantou's doesn't is map the source error back to the .razor file so this seems easier to maintain and test. The test files could technically sit in MudBlazor.UnitTests but will be faster in their own small project.

When I got to testing I found my line mapping (source -> .razor) didn't work in all cases as ##line indicators aren't what I expected for certain circumstances (so its good to test :-)). I am looking at a different way to do this at the moment but if I don't crack it by the weekend I will release a version with tests minus the line mapping. The generator could either report to the source file or the top of the .razor file.

[Nickztar]

That sounds like an interesting solution. I have made some tests using the Meziantou's version in my repo. I can agree that is is somewhat cumbersome to have the razor files as strings, but they aren't really dependent on MudBlazor code in the current form, so changes should be somewhat decoupled from the base repo (Good/Bad?).

https://github.com/Nickztar/MudBlazor/tree/feature/parameter-analyzer-rebase

[henon]

@peterthorpe81 to get rid of the code quality check failure execute dotnet format in VS developer command prompt

[codecov[bot]]

Codecov Report

All modified and coverable lines are covered by tests :white_check_mark:

Project coverage is 90.82%. Comparing base (28bc599) to head (8201bd5). Report is 285 commits behind head on dev.

Additional details and impacted files

@@            Coverage Diff             @@
##              dev    #9031      +/-   ##
==========================================
+ Coverage   89.82%   90.82%   +0.99%     
==========================================
  Files         412      401      -11     
  Lines       11878    12513     +635     
  Branches     2364     2434      +70     
==========================================
+ Hits        10670    11365     +695     
+ Misses        681      608      -73     
- Partials      527      540      +13     

:umbrella: View full report in Codecov by Sentry.
:loudspeaker: Have feedback on the report? Share it here.

[peterthorpe81]

I have updated this with how I think tests should work so essentially MudBlazor.Analyzers.TestComponents contains the relevant .razor files. This project is compiled and analyzed several times to try the different settings you can use in your .csproj file.

The tests check if the source generated error messages match what I expect for the different configs.

I have for now stopped trying to map the .razor location from the source generator line. The diagnostic error on click will instead just go to the top of the .razor file which I think is at least a little more useful than indicating the source generated location.

The original mapping using ##line numbers in the source file didn't work 100% of the time. I did experiment with a lexer/parser for the .razor file that would look for the nth occurrence of a tag and attribute but it got too complicated with all the possible escape characters in razor syntax.

I have set MudBlazor and MudBlazor.Docs to this config:

  <PropertyGroup Condition=" '$(Configuration)' == 'Release' ">
    <MudAllowedAttributePattern>Any</MudAllowedAttributePattern>
    <MudIllegalParameters>None</MudIllegalParameters>
  </PropertyGroup>

  <PropertyGroup Condition=" '$(Configuration)' == 'Debug' ">
    <MudAllowedAttributePattern>LowerCase</MudAllowedAttributePattern>
    <MudIllegalParameters>V7IgnoreCase</MudIllegalParameters>
    <MudDebugAnalyzer>false</MudDebugAnalyzer>
  </PropertyGroup>

Release builds are successful and debug builds indicate a few issues which would need correcting if this seems like a good default. I didn't add them in the PR but can if you want, couple of examples:

1. MudDataGrid is using MudIconButton with a Title attribute. This was until recently a Parameter so its just a case of chanigng the casing to title to match the enforced pattern.
2. DatePickerInternationalizationExample page is indicating a use of UseShortNames on MudDatePicker. This may have been a parameter in the past? Its flagging it because of the casing but probably needs removing.

[ScarletKuro]

Few questions and then its good to go: @peterthorpe81 Did you align with the latest illegal parameters https://github.com/MudBlazor/MudBlazor/blob/dev/src/MudBlazor/Base/MudComponentBase.cs#L111 I'm asking to be sure since this was started a month ago.

@henon should we remove DetectIllegalRazorParametersV7 from MudComponentBase in this PR?

There were some illegal parameters detected in our docs:

Error: /home/runner/work/MudBlazor/MudBlazor/src/MudBlazor.Docs/Pages/Components/DataGrid/Examples/DataGridCustomSortingExample.razor(4,46): error MUD0002: Illegal Attribute 'Sortable' detected on component 'MudDataGrid' using pattern 'LowerCase' (https://mudblazor.com/features/analyzers) [/home/runner/work/MudBlazor/MudBlazor/src/MudBlazor.Docs/MudBlazor.Docs.csproj]
Error: /home/runner/work/MudBlazor/MudBlazor/src/MudBlazor.Docs/Pages/Components/Table/Examples/TableMultiSelectExample.razor(12,61): error MUD0002: Illegal Attribute 'SelectOnRowClickChanged' detected on component 'MudTable' using pattern 'LowerCase' (https://mudblazor.com/features/analyzers) [/home/runner/work/MudBlazor/MudBlazor/src/MudBlazor.Docs/MudBlazor.Docs.csproj]
Error: /home/runner/work/MudBlazor/MudBlazor/src/MudBlazor.Docs/Pages/Components/FileUpload/Examples/FileUploadSelectedTemplateExample.razor(22,48): error MUD0002: Illegal Attribute 'Multiple' detected on component 'MudFileUpload' using pattern 'LowerCase' (https://mudblazor.com/features/analyzers) [/home/runner/work/MudBlazor/MudBlazor/src/MudBlazor.Docs/MudBlazor.Docs.csproj]
Error: /home/runner/work/MudBlazor/MudBlazor/src/MudBlazor.Docs/Pages/Components/DropZone/Examples/DropZoneItemSelectorOnlyZoneExample.razor(3,47): error MUD0002: Illegal Attribute 'Item' detected on component 'MudDropContainer' using pattern 'LowerCase' (https://mudblazor.com/features/analyzers) [/home/runner/work/MudBlazor/MudBlazor/src/MudBlazor.Docs/MudBlazor.Docs.csproj]
Error: /home/runner/work/MudBlazor/MudBlazor/src/MudBlazor.Docs/Pages/Components/DropZone/Examples/DropZoneDraggingStyleExample.razor(3,47): error MUD0002: Illegal Attribute 'Item' detected on component 'MudDropContainer' using pattern 'LowerCase' (https://mudblazor.com/features/analyzers) [/home/runner/work/MudBlazor/MudBlazor/src/MudBlazor.Docs/MudBlazor.Docs.csproj]
Error: /home/runner/work/MudBlazor/MudBlazor/src/MudBlazor.Docs/Pages/Components/DatePicker/Examples/DatePickerInternationalizationExample.razor(6,82): error MUD0002: Illegal Attribute 'UseShortNames' detected on component 'MudDatePicker' using pattern 'LowerCase' (https://mudblazor.com/features/analyzers) [/home/runner/work/MudBlazor/MudBlazor/src/MudBlazor.Docs/MudBlazor.Docs.csproj]
Error: /home/runner/work/MudBlazor/MudBlazor/src/MudBlazor.Docs/Pages/Components/ChipSet/Examples/ChipSetBasicExample.razor(15,67): error MUD0002: Illegal Attribute 'Inline' detected on component 'MudText' using pattern 'LowerCase' (https://mudblazor.com/features/analyzers) [/home/runner/work/MudBlazor/MudBlazor/src/MudBlazor.Docs/MudBlazor.Docs.csproj]

Should we just fix them here?

[peterthorpe81]

Few questions and then its good to go: @peterthorpe81 Did you align with the latest illegal parameters https://github.com/MudBlazor/MudBlazor/blob/dev/src/MudBlazor/Base/MudComponentBase.cs#L111 I'm asking to be sure since this was started a month ago.

@henon should we remove DetectIllegalRazorParametersV7 from MudComponentBase in this PR?

There were some illegal parameters detected in our docs:

Error: /home/runner/work/MudBlazor/MudBlazor/src/MudBlazor.Docs/Pages/Components/DataGrid/Examples/DataGridCustomSortingExample.razor(4,46): error MUD0002: Illegal Attribute 'Sortable' detected on component 'MudDataGrid' using pattern 'LowerCase' (https://mudblazor.com/features/analyzers) [/home/runner/work/MudBlazor/MudBlazor/src/MudBlazor.Docs/MudBlazor.Docs.csproj]
Error: /home/runner/work/MudBlazor/MudBlazor/src/MudBlazor.Docs/Pages/Components/Table/Examples/TableMultiSelectExample.razor(12,61): error MUD0002: Illegal Attribute 'SelectOnRowClickChanged' detected on component 'MudTable' using pattern 'LowerCase' (https://mudblazor.com/features/analyzers) [/home/runner/work/MudBlazor/MudBlazor/src/MudBlazor.Docs/MudBlazor.Docs.csproj]
Error: /home/runner/work/MudBlazor/MudBlazor/src/MudBlazor.Docs/Pages/Components/FileUpload/Examples/FileUploadSelectedTemplateExample.razor(22,48): error MUD0002: Illegal Attribute 'Multiple' detected on component 'MudFileUpload' using pattern 'LowerCase' (https://mudblazor.com/features/analyzers) [/home/runner/work/MudBlazor/MudBlazor/src/MudBlazor.Docs/MudBlazor.Docs.csproj]
Error: /home/runner/work/MudBlazor/MudBlazor/src/MudBlazor.Docs/Pages/Components/DropZone/Examples/DropZoneItemSelectorOnlyZoneExample.razor(3,47): error MUD0002: Illegal Attribute 'Item' detected on component 'MudDropContainer' using pattern 'LowerCase' (https://mudblazor.com/features/analyzers) [/home/runner/work/MudBlazor/MudBlazor/src/MudBlazor.Docs/MudBlazor.Docs.csproj]
Error: /home/runner/work/MudBlazor/MudBlazor/src/MudBlazor.Docs/Pages/Components/DropZone/Examples/DropZoneDraggingStyleExample.razor(3,47): error MUD0002: Illegal Attribute 'Item' detected on component 'MudDropContainer' using pattern 'LowerCase' (https://mudblazor.com/features/analyzers) [/home/runner/work/MudBlazor/MudBlazor/src/MudBlazor.Docs/MudBlazor.Docs.csproj]
Error: /home/runner/work/MudBlazor/MudBlazor/src/MudBlazor.Docs/Pages/Components/DatePicker/Examples/DatePickerInternationalizationExample.razor(6,82): error MUD0002: Illegal Attribute 'UseShortNames' detected on component 'MudDatePicker' using pattern 'LowerCase' (https://mudblazor.com/features/analyzers) [/home/runner/work/MudBlazor/MudBlazor/src/MudBlazor.Docs/MudBlazor.Docs.csproj]
Error: /home/runner/work/MudBlazor/MudBlazor/src/MudBlazor.Docs/Pages/Components/ChipSet/Examples/ChipSetBasicExample.razor(15,67): error MUD0002: Illegal Attribute 'Inline' detected on component 'MudText' using pattern 'LowerCase' (https://mudblazor.com/features/analyzers) [/home/runner/work/MudBlazor/MudBlazor/src/MudBlazor.Docs/MudBlazor.Docs.csproj]

Should we just fix them here?

Yes, illegal parameters should be aligned. I'm not 100% on removing DetectIllegalRazorParametersV7 as I think it will cover some dynamic scenarios that the analyzer won't. If it's a performance hit maybe make it opt in? The methods could potentially share the detection list too.

I haven't looked at @xC0dex review yet but all seem like good edits from a quick scan.

Also do you think its going to be clear enough to consumers when they start getting MUD0001-2 warnings at build? Again the default is LowerCase attributes which matches W3C suggestion and then the illegal parameters list ignoring case. I am just a little worried about breaking people builds if they are using something like TreatWarningsAsErrors.

[peterthorpe81]

Just added a commit to cover @xC0dex review.

[henon]

There were some illegal parameters detected in our docs:

Error: /home/runner/work/MudBlazor/MudBlazor/src/MudBlazor.Docs/Pages/Components/DataGrid/Examples/DataGridCustomSortingExample.razor(4,46): error MUD0002: Illegal Attribute 'Sortable' detected on component 'MudDataGrid' using pattern 'LowerCase' (https://mudblazor.com/features/analyzers) [/home/runner/work/MudBlazor/MudBlazor/src/MudBlazor.Docs/MudBlazor.Docs.csproj]
Error: /home/runner/work/MudBlazor/MudBlazor/src/MudBlazor.Docs/Pages/Components/Table/Examples/TableMultiSelectExample.razor(12,61): error MUD0002: Illegal Attribute 'SelectOnRowClickChanged' detected on component 'MudTable' using pattern 'LowerCase' (https://mudblazor.com/features/analyzers) [/home/runner/work/MudBlazor/MudBlazor/src/MudBlazor.Docs/MudBlazor.Docs.csproj]
Error: /home/runner/work/MudBlazor/MudBlazor/src/MudBlazor.Docs/Pages/Components/FileUpload/Examples/FileUploadSelectedTemplateExample.razor(22,48): error MUD0002: Illegal Attribute 'Multiple' detected on component 'MudFileUpload' using pattern 'LowerCase' (https://mudblazor.com/features/analyzers) [/home/runner/work/MudBlazor/MudBlazor/src/MudBlazor.Docs/MudBlazor.Docs.csproj]
Error: /home/runner/work/MudBlazor/MudBlazor/src/MudBlazor.Docs/Pages/Components/DropZone/Examples/DropZoneItemSelectorOnlyZoneExample.razor(3,47): error MUD0002: Illegal Attribute 'Item' detected on component 'MudDropContainer' using pattern 'LowerCase' (https://mudblazor.com/features/analyzers) [/home/runner/work/MudBlazor/MudBlazor/src/MudBlazor.Docs/MudBlazor.Docs.csproj]
Error: /home/runner/work/MudBlazor/MudBlazor/src/MudBlazor.Docs/Pages/Components/DropZone/Examples/DropZoneDraggingStyleExample.razor(3,47): error MUD0002: Illegal Attribute 'Item' detected on component 'MudDropContainer' using pattern 'LowerCase' (https://mudblazor.com/features/analyzers) [/home/runner/work/MudBlazor/MudBlazor/src/MudBlazor.Docs/MudBlazor.Docs.csproj]
Error: /home/runner/work/MudBlazor/MudBlazor/src/MudBlazor.Docs/Pages/Components/DatePicker/Examples/DatePickerInternationalizationExample.razor(6,82): error MUD0002: Illegal Attribute 'UseShortNames' detected on component 'MudDatePicker' using pattern 'LowerCase' (https://mudblazor.com/features/analyzers) [/home/runner/work/MudBlazor/MudBlazor/src/MudBlazor.Docs/MudBlazor.Docs.csproj]
Error: /home/runner/work/MudBlazor/MudBlazor/src/MudBlazor.Docs/Pages/Components/ChipSet/Examples/ChipSetBasicExample.razor(15,67): error MUD0002: Illegal Attribute 'Inline' detected on component 'MudText' using pattern 'LowerCase' (https://mudblazor.com/features/analyzers) [/home/runner/work/MudBlazor/MudBlazor/src/MudBlazor.Docs/MudBlazor.Docs.csproj]

Should we just fix them here?

@peterthorpe81 These have not been fixed right? Also, the attribute Inline is no longer illegal, we re-added it.

[ScarletKuro]

@peterthorpe81 These have not been fixed right? Also, the attribute Inline is no longer illegal, we re-added it.

No, it wasn't. As far as I see it was removed from the list, i just took the log from the old pipeline execution.

[henon]

So if we don't fix the errors the build will fail for everyone after merging, right?

[peterthorpe81]

Yes it will fail on debug build but not release as I configured it for MudBlazor and MudBlazor.Docs to only run on debug build.

I left the warnings in originally as it was a good demonstration of what its doing. Do you want me to add the fixes to the PR for these warnings? They appear as errors because of TreatWarningsAsErrors setting.

MudDataGrid.razor(1,1,1,1): error MUD0002: Illegal Attribute 'Title' on 'MudIconButton' using pattern 'LowerCase' source location '(1224,32)-(1232,34)' (https://mudblazor.com/features/analyzers)
MudDataGrid.razor(1,1,1,1): error MUD0002: Illegal Attribute 'Title' on 'MudIconButton' using pattern 'LowerCase' source location '(857,36)-(865,38)' (https://mudblazor.com/features/analyzers)
MudDataGrid.razor(1,1,1,1): error MUD0002: Illegal Attribute 'Title' on 'MudIconButton' using pattern 'LowerCase' source location '(930,36)-(938,38)' (https://mudblazor.com/features/analyzers)
MudDataGrid.razor(1,1,1,1): error MUD0002: Illegal Attribute 'Title' on 'MudIconButton' using pattern 'LowerCase' source location '(1014,32)-(1022,34)' (https://mudblazor.com/features/analyzers)
MudDataGrid.razor(1,1,1,1): error MUD0002: Illegal Attribute 'Title' on 'MudIconButton' using pattern 'LowerCase' source location '(1080,32)-(1088,34)' (https://mudblazor.com/features/analyzers)
MudDataGrid.razor(1,1,1,1): error MUD0002: Illegal Attribute 'Title' on 'MudIconButton' using pattern 'LowerCase' source location '(1345,32)-(1353,34)' (https://mudblazor.com/features/analyzers)
MudDataGrid.razor(1,1,1,1): error MUD0002: Illegal Attribute 'Title' on 'MudIconButton' using pattern 'LowerCase' source location '(1412,32)-(1420,34)' (https://mudblazor.com/features/analyzers)


ToggleIconButtonTwoWayBindingExample.razor(1,1,1,1): error MUD0002: Illegal Attribute 'Title' on 'MudToggleIconButton' using pattern 'LowerCase' source location '(125,12)-(125,62)' (https://mudblazor.com/features/analyzers)
ToggleIconButtonTwoWayBindingExample.razor(1,1,1,1): error MUD0002: Illegal Attribute 'ToggledTitle' on 'MudToggleIconButton' using pattern 'LowerCase' source location '(144,12)-(144,68)' (https://mudblazor.com/features/analyzers)
ToggleIconButtonEventCallbackExample.razor(1,1,1,1): error MUD0002: Illegal Attribute 'Title' on 'MudToggleIconButton' using pattern 'LowerCase' source location '(143,12)-(143,62)' (https://mudblazor.com/features/analyzers)
ToggleIconButtonEventCallbackExample.razor(1,1,1,1): error MUD0002: Illegal Attribute 'ToggledTitle' on 'MudToggleIconButton' using pattern 'LowerCase' source location '(162,12)-(162,68)' (https://mudblazor.com/features/analyzers)
TabsHeadersWrapExample.razor(1,1,1,1): error MUD0002: Illegal Attribute 'TabHeadersInline' on 'MudTabs' using pattern 'LowerCase' source location '(264,12)-(264,75)' (https://mudblazor.com/features/analyzers)
TableMultiSelectExample.razor(1,1,1,1): error MUD0002: Illegal Attribute 'SelectOnRowClickChanged' on 'MudTable' using pattern 'LowerCase' source location '(239,12)-(239,240)' (https://mudblazor.com/features/analyzers)
DataGridCustomSortingExample.razor(1,1,1,1): error MUD0002: Illegal Attribute 'Sortable' on 'MudDataGrid' using pattern 'LowerCase' source location '(294,8)-(294,67)' (https://mudblazor.com/features/analyzers)
DatePickerInternationalizationExample.razor(1,1,1,1): error MUD0002: Illegal Attribute 'UseShortNames' on 'MudDatePicker' using pattern 'LowerCase' source location '(143,12)-(143,72)' (https://mudblazor.com/features/analyzers)
DropZoneDraggingStyleExample.razor(1,1,1,1): error MUD0002: Illegal Attribute 'Item' on 'MudDropContainer' using pattern 'LowerCase' source location '(116,12)-(116,60)' (https://mudblazor.com/features/analyzers)
DropZoneItemSelectorOnlyZoneExample.razor(1,1,1,1): error MUD0002: Illegal Attribute 'Item' on 'MudDropContainer' using pattern 'LowerCase' source location '(116,12)-(116,60)' (https://mudblazor.com/features/analyzers)
FileUploadSelectedTemplateExample.razor(1,1,1,1): error MUD0002: Illegal Attribute 'Multiple' on 'MudFileUpload' using pattern 'LowerCase' source location '(187,12)-(187,65)' (https://mudblazor.com/features/analyzers)

The bulk of the fixes are changing the casing so attributes are lower case e.g. Title (formerly a parameter) to title. There are a couple of old/renamed parameters in the mix too like Sortable on MudDataGrid.

I'm not getting an error on Inline locally is this an old result? The line in question does look odd though so the Inline parameter is there but value less: <MudText Color="@SelectedColor" Inline>@SelectedColor.ToDescriptionString()</MudText> Source gen appears to add it as true which I didn't know blazor would do: __builder2.AddComponentParameter(62, "Inline", true); I guess that's so it matches how html attributes work but perhaps not the best practice for a parameter?

[henon]

I'm not getting an error on Inline locally is this an old result? The line in question does look odd though so the Inline parameter is there but value less: <MudText Color="@SelectedColor" Inline>@SelectedColor.ToDescriptionString()</MudText>

Yes, this is HTML logic, parameters without value are set to true

perhaps not the best practice for a parameter?

I don't think so. We use it in many locations in the source base and the examples. Does it pose a problem to the analyzer?

I left the warnings in originally as it was a good demonstration of what its doing. Do you want me to add the fixes to the PR for these warnings? They appear as errors because of TreatWarningsAsErrors setting.

When this is merged everyone working on a PR in debug mode will get the errors. That would cause issues and questions to pop up how to deal with them, so in my opinion we'll have to fix them as part of this PR. What do you think @ScarletKuro ?

[ScarletKuro]

I'm not getting an error on Inline locally is this an old result?

Yes, as I said I posted a month old log. Inline was re-added in https://github.com/MudBlazor/MudBlazor/pull/9065

That would cause issues and questions to pop up how to deal with them, so in my opinion we'll have to fix them as part of this PR. What do you think @ScarletKuro ?

Yes, I think it's better if we fix it in this PR. Maybe we could even enable it for all modes, but up to you @henon

[henon]

I resently saw an example of how to use conditional compilation in razor using @code { ... } blocks. That would be one argument for all modes. Another argument would be that it then also fails on the CI server which would actually be important. So yes, I agree, we should also enable it in Release.

@peterthorpe81 Is there an easy way to disable it for people who don't want it or have issues with it?

[ScarletKuro]

@peterthorpe81 Is there an easy way to disable it for people who don't want it or have issues with it?

We discussed it and I think @peterthorpe81 added this feature and there even a doc page that explains the settings? Would be nice to post the screenshot of it here.

[peterthorpe81]

Added the fixes to the PR.

The analyzer will now run for release and debug builds of MudBlazor.

This is the page in the docs about how to configure/disable the analyzer. The diagnostics include this as the link url like when you get erros that point to Microsoft pages.

[ScarletKuro]

I noticed that after this PR, the bin and obj etc folders are forcibly shown, which is not good unless I explicitly set this in VS. This is caused by these lines:

<ItemGroup>
  <None Include="$(OutputPath)\net7.0\$(AssemblyName).Analyzers.dll" Pack="true" PackagePath="analyzers/dotnet/cs/" Visible="true" />
  <Content Include="..\Directory.Build.targets" Pack="true" PackagePath="build\$(PackageId).targets" Visible="true" />
</ItemGroup>

I guess this is a similar problem: Stack Overflow link.

@peterthorpe81, it would be great if you could fix this.

[ScarletKuro]

~~It also makes the .razor.cs not to be nested under the corresponding .razor file.~~

Never mind, this .razor.cs was a false alarm. I accidentally disabled it while trying to figure out why the bin and obj folders were appearing.

[peterthorpe81]

I noticed that after this PR, the bin and obj etc folders are forcibly shown, which is not good unless I explicitly set this in VS. This is caused by these lines:

<ItemGroup>
  <None Include="$(OutputPath)\net7.0\$(AssemblyName).Analyzers.dll" Pack="true" PackagePath="analyzers/dotnet/cs/" Visible="true" />
  <Content Include="..\Directory.Build.targets" Pack="true" PackagePath="build\$(PackageId).targets" Visible="true" />
</ItemGroup>

I guess this is a similar problem: Stack Overflow link.

@peterthorpe81, it would be great if you could fix this.

I hadn't noticed that, I have changed the way its made transitive with this PR https://github.com/MudBlazor/MudBlazor/pull/9229

Its similar to the stackoverflow fix using TargetsForTfmSpecificContentInPackage. Because MudBlazor is multi targeted it would try to copy the files in for each framework version which errors because the analyzer needs to sit as a .net standard 2.0 dll in analyzers/dotnet/cs/. I have limited the item group to .net 8.0 so it only does one copy.