Remove design-time __TypeConstraints functions
#8825
Conversation
There was a problem hiding this comment.
This is generally looking ok, but I'd like to have @davidwengier take a look at this change as well to make sure there's no gotchas we're missing from the IDE side. Also, if you could do a smoke test in the IDE and make sure everything appears to be working Jan, that would be appreciated.
| @@ -123,10 +123,12 @@ protected override CodeTarget CreateTarget(RazorCodeDocument codeDocument, Razor | |||
| continue; | |||
| } | |||
|
|
|||
| var constraints = typeParamNode.Tokens.Skip(1).FirstOrDefault(); | |||
There was a problem hiding this comment.
What is this parameter we're skipping? Please leave a comment.
Side note: at some point we need to remove all of these IEnumerables and make them ImmutableArrays/Spans for this type of thing.
There was a problem hiding this comment.
This is just extracted variable from existing code. But will add a comment.
There was a problem hiding this comment.
From looking at the new baselines, this looks good from a tooling perspecitve. Would appreciate a smoke test, for sure. I would just make sure that if you hover over an interface name in a constraint, the hover shows correctly. I had a quick play and it seems formatting doesn't work currently anyway (logged #8827)
It is surprising to me that there is no padding on the output, ie this generates:
where T : IFoo
rather than
where
T : IFoo
but the previous code didnt' do it either, so its fine. Normally this means that certain Roslyn code actions may not apply well (the padding is so that the C# code ends up at the same column positions as the original user written Razor code). It's also possible there just arent any problematic code actions issued by roslyn for type constraints.
In general though, I think this change is perfectly in line with the direction we need to be going, regardless of bug fixing. Having the "real" C# code source mapped, instead of generating design time helpers, should provide a better experience for users, and gets us ever so slightly closer to runtime and designtime being the same.
Yeah, I also noticed, but didn't want to change the code more than necessary in one PR. Also I'm wondering - padding is equivalent to using enhanced line pragma, right?
Everything seems to be working fine (except for sometimes hitting this issue #8572).
|
That is also my understanding, and I don't see why we can't move to enhanced line pragmas across the board at some point. The source mappings we get directly from the compiler already have that level of fidelity anyway. Definitely something for another PR though :) |
|
@chsienki @dotnet/razor-compiler for a second review, thanks |
Fixes #8711.
Fixes #8558.