Better Illegal Razor Parameter Runtime Detection for v7

[henon]

Description

Follow-up to Illegal Razor Parameter Runtime Detection for v7 #8771

I noticed that the detection didn't work for generic types. After fixing that the detector finds at least 26 more illegal usages in our docs and tests. Naturally this PR fails until I fix the old params.

I think we probably should not remove this detector, ever. It is just too useful. We can use it to detect illegal usage of obsolete API in our docs and tests when we enter the next stable development phase. Of course then we'd change it to opt-in instead of opt-out.

How Has This Been Tested?

unit

Type of Changes

• Bug fix (non-breaking change which fixes an issue)
• 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

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

[codecov]

Codecov Report

All modified and coverable lines are covered by tests ✅

Project coverage is 90.10%. Comparing base (28bc599) to head (51da8f6).
Report is 103 commits behind head on dev.

Additional details and impacted files

@@            Coverage Diff             @@
##              dev    #8777      +/-   ##
==========================================
+ Coverage   89.82%   90.10%   +0.27%     
==========================================
  Files         412      419       +7     
  Lines       11878    12183     +305     
  Branches     2364     2399      +35     
==========================================
+ Hits        10670    10978     +308     
+ Misses        681      661      -20     
- Partials      527      544      +17     

☔ View full report in Codecov by Sentry.
📢 Have feedback on the report? Share it here.

[henon]

All errors fixed. Ready to merge

[ScarletKuro]

I think we probably should not remove this detector, ever.

Not sure about having it permanent as the reflection hurts performance

Also posting code in case we want to lower amount of ifs.

Code

protected void DetectIllegalRazorParametersV7()
{
  var illegalParameters = new Dictionary<Type, string[]>
  {
      { typeof(MudBadge), new[] { "Bottom", "Left", "Start" } },
      { typeof(MudProgressCircular), new[] { "Minimum", "Maximum" } },
      { typeof(MudProgressLinear), new[] { "Minimum", "Maximum" } },
      { typeof(MudRadio<>), new[] { "Option" } },
      { typeof(MudFab), new[] { "Icon" } },
      { typeof(MudCheckBox<>), new[] { "Checked" } },
      { typeof(MudSwitch<>), new[] { "Checked" } },
      { typeof(MudPopover), new[] { "Direction", "OffsetX", "OffsetY" } },
      { typeof(MudAutocomplete<>), new[] { "Direction", "OffsetX", "OffsetY" } },
      { typeof(MudSelect<>), new[] { "Direction", "OffsetX", "OffsetY" } },
      { typeof(MudToggleGroup<>), new[] { "Outline" } },
      { typeof(MudAvatar), new[] { "Image", "Alt" } },
      { typeof(MudSlider<>), new[] { "Text" } },
      { typeof(MudRadioGroup<>), new[] { "SelectedOption", "SelectedOptionChanged" } },
      { typeof(MudSwipeArea), new[] { "OnSwipe" } },
      { typeof(MudChip<>), new[] { "Avatar", "AvatarClass" } },
      { typeof(MudChipSet<>), new[] { "Filter", "MultiSelection", "Mandatory", "SelectedChip", "SelectedChipChanged", "SelectedChips", "SelectedChipsChanged" } },
      { typeof(MudList<>), new[] { "SelectedItem", "SelectedItemChanged", "Clickable", "Avatar", "AvatarClass" } },
      { typeof(MudListItem<>), new[] { "SelectedItem", "SelectedItemChanged", "Clickable", "Avatar", "AvatarClass" } },
      { typeof(MudTreeView<>), new[] { "CanActivate", "CanHover", "CanSelect", "ActivatedValue", "ActivatedValueChanged", "Multiselection", "Activated", "ExpandedIcon", "SelectedItem" } },
      { typeof(MudTreeViewItem<>), new[] { "CanActivate", "CanHover", "CanSelect", "ActivatedValue", "ActivatedValueChanged", "Multiselection", "Activated", "ExpandedIcon", "SelectedItem" } },
      { typeof(MudMenu), new[] { "Link", "Target", "HtmlTag", "ButtonType" } },
      { typeof(MudMenuItem), new[] { "Link", "Target", "HtmlTag", "ButtonType" } },
  };

  foreach (var parameter in UserAttributes.Keys)
  {
      foreach (var entry in illegalParameters)
      {
          if (MatchTypes(entry.Key))
          {
              if (entry.Value.Contains(parameter))
              {
                  NotifyIllegalParameter(parameter);
                  break;
              }
          }
      }

      // Additional illegal parameters not mapped to specific types
      var additionalIllegalParameters = new[]
      {
          "UnCheckedColor", "Command", "CommandParameter", "IsEnabled", "ClassAction", "InputIcon", "InputVariant",
          "AllowKeyboardInput", "ClassActions", "DisableRipple", "DisableGutters", "DisablePadding", "DisableElevation",
          "DisableUnderLine", "DisableRowsPerPage", "Link", "Delayed", "AlertTextPosition", "ShowDelimiters",
          "DelimitersColor", "DrawerWidth", "DrawerHeightTop", "DrawerHeightBottom", "AppbarMinHeight", "ClassBackground",
          "Cancelled", "ClassContent", "IsExpanded", "IsExpandedChanged", "IsInitiallyExpanded", "InitiallyExpanded",
          "RightAlignSmall", "IsExpandable"
      };

      if (additionalIllegalParameters.Contains(parameter))
      {
          NotifyIllegalParameter(parameter);
      }
  }
}

internal bool MatchTypes(params Type[] types)
{
  var self = GetType().IsGenericType ? GetType().GetGenericTypeDefinition() : GetType();
  return types.Any(type => self == type);
}

[ExcludeFromCodeCoverage]
private void NotifyIllegalParameter(string parameter)
{
  throw new ArgumentException($"Illegal parameter '{parameter}' in component of type '{GetType().Name}'. This was removed in v7.0.0, see Migration Guide for more info https://github.com/MudBlazor/MudBlazor/issues/8447");
}

[henon]

Not sure about having it permanent as the reflection hurts performance

With active opt-in the cost would be a single if

[henon]

wouldn't this perform a linear search?

      // Additional illegal parameters not mapped to specific types
      var additionalIllegalParameters = new[]
      {
          "UnCheckedColor", "Command", "CommandParameter", "IsEnabled", "ClassAction", "InputIcon", "InputVariant",
          "AllowKeyboardInput", "ClassActions", "DisableRipple", "DisableGutters", "DisablePadding", "DisableElevation",
          "DisableUnderLine", "DisableRowsPerPage", "Link", "Delayed", "AlertTextPosition", "ShowDelimiters",
          "DelimitersColor", "DrawerWidth", "DrawerHeightTop", "DrawerHeightBottom", "AppbarMinHeight", "ClassBackground",
          "Cancelled", "ClassContent", "IsExpanded", "IsExpandedChanged", "IsInitiallyExpanded", "InitiallyExpanded",
          "RightAlignSmall", "IsExpandable"
      };

      if (additionalIllegalParameters.Contains(parameter))
      {
          NotifyIllegalParameter(parameter);
      }

[ScarletKuro]

With active opt-in the cost would be a single if

oh, right