Every `TagHelperDescriptorProviderContext` creates an `ItemCollection`
to hold onto, at most, two objects: a `Compilation` and a target
`ISymbol`. This is enormously wasteful because an `ItemCollection`
internally creates a `ConcurrentDictionary<object, object>`. So, I've
changed `TagHelperDescriptorProviderContext` to just hold onto a
compilation and a target symbol and avoid the `ItemCollection`
altogether.
I recommend reviewing commit-by-commit.
Also, I bumped into a long standing compiler bug in
`EventHandlerTagHelperDescriptorProvider` that was recently filed by a
customer: https://github.com/dotnet/razor/issues/10497. I opted to stay
on target and not fix this issue, but I made it painfully obvious where
the fix would go. 😄
* Move to central package pinning
This should make it much easier for us to respond to CG alerts in the
future. All that will need to be done is add an entry in
Directory.Packages.props and it will automatically impact all consumers
of it.
Consider this example in Roslyn for how to respond to a CG issue
https://github.com/dotnet/roslyn/pull/74653
because it's really easy to get one from the other.
While working on rename in cohosting, I noticed this unnecessary extra
bit of work while calling `GetPositionInfo`, so I fixed it. Then I
thought I'd see where else `GetSourceTextAsync` was used in an non-ideal
way, and that was clearly going to be too big for the rename PR so I
separated it out into it's own PR. Then I heard Dustin say he was
changing `DocumentSnapshot` so the unnecessary work will end up being
much less wasteful anyway, plus we'd probably just have conflicts, so I
pared the whole thing right back to just this minor removal of a couple
of unnecessary parameters. Enjoy.
- Enable nullability
- Mark all as sealed
- Use `Lazy<T>` for singleton `ITagHelperDescriptorProviders`
- Introduce `TagHelperDescriptorProviderBase`
- Inherit from `TagHelperDescriptorProviderBase`
- Remove unnecessary properties
- Remove unnecessary property setters
In addition, I spotted a bug in `EventHandlerTagDescriptorProvider` while addressing nullability warnings. The buggy code is right here:
fb84ae5d9b/src/Compiler/Microsoft.CodeAnalysis.Razor.Compiler/src/CSharp/EventHandlerTagHelperDescriptorProvider.cs (L70-L76)
When reading the constructor arguments of an `EventHandlerAttribute` it appears that we swap the last two arguments of the 4-argument variant. The constructor is defined like so:
```C#
EventHandlerAttribute(string attributeName, Type eventArgsType, bool enableStopPropagation, bool enablePreventDefault);
```
Unfortunately, the compiler reads the third parameter as `enablePreventDefaeult` and the fourth parameter as `enableStopPropagation`.
This has been broken since support for the 4-argument constructor variant was added in 7635bba6ef. Fixing this bug is tracked by https://github.com/dotnet/razor/issues/10497.
Every single `TagHelperDescriptorProviderContext` created in Razor compiler or tooling code adds a valid compilation. So, adding the compilation to the `ItemsCollection` is pure overhead and no `ITagHelperDescriptorProvider` needs to check it for null.
I removed a single test that verified the behavior if a compilation was never set. Now that a compilation is required, this test is no longer needed.
Adds a single telemetry point so that we if we receive feedback that
something unusual behavior occurs, we can determine if Fuse was engaged
or not engaged.
Dustin Campbell
dotnet-maestro[bot]
David Wengier
Jared Parsons
Fredric Silberberg
phil-allen-msft