Fix NRE when invoking completion in empty document

[alexgav]

In empty document completion context will have RazorDocumentSyntax as Owner. RazorDoucmentSyntax has null as the parent, which makes sense. However compiler thinks we can't have a null here, while obviously we do.

I'm adding a null check and a test for this case. We would probably further discuss the implications here. I tried to track down how we get null in a non-nullable field, but it's a bit confusing since the class gets generated.

### Summary of the changes

• Fix NRE thrown if completion is invoked in an empty razor document

[alexgav]

Thinking more about this though... Seems like we should get HTML tags and tag helpers when invoking completion in an empty document. Legacy editor has that -

LSP Razor editor returns neither HTML tags nor tag helpers.

Things to work better if you type in either @ or < instead of hitting Ctrl+Space or Ctrl+J to invoke completion, which is probably 99% case.

I think we should get this fix in, and if we decided it's worth pursing, also log a bug for a better fix that will get tag names and tag helpers in the completion.

[davidwengier]

I think we should get this fix in

What is the user impact of the null ref? I would imagine nothing other than telemetry or logs, so I don't think there is any point trying to get this in over just fixing completion so it works in this situation as expected.

[alexgav]

I think we should get this fix in

What is the user impact of the null ref? I would imagine nothing other than telemetry or logs, so I don't think there is any point trying to get this in over just fixing completion so it works in this situation as expected.

[alexgav]

I think we should get this fix in

What is the user impact of the null ref? I would imagine nothing other than telemetry or logs, so I don't think there is any point trying to get this in over just fixing completion so it works in this situation as expected.

I investigated this more in terms of what it would take to "just fix the issue" 😄 and unfortunately it looks fairly complicated as I anticipated.

1. Completely empty Razor document (zero length) creates generates HTML document with no spans (we don't create a mapping span for zero-length tokens, see

https://github.com/dotnet/razor/blob/7b511df1710c54ab1a36ea5e422c7edd7bb29d20/src/Compiler/Microsoft.CodeAnalysis.Razor.Compiler/src/Language/RazorHtmlWriter.cs#L198C13-L198C35

we'd need to change that (or at least special-case this for a completely empty document). A bit of a scary change (and would probably cause performance implications unless we special-cased it for the 0-length document case)

Without that, position doesn't map to HTML (and completion request wouldn't get sent to HTML server)

2. If we have an empty document of non-zero length (e.g. a single space, or a blank line), things are slightly better - we do get a mapping span, we do send request to HTML language server, but HTML language server doesn't produce completion items in that case. I verified it with plain HTML file (switching HTML editor to LSP mode). So we'd need to send a bug to Vlad to address that. That's a regression from non-LSP editor.

We do get HTML snippets in that case, but not tags in the completion list in Razor. in HTML (LSP mode) we get nothing.

3. TagHelperCompletionProvider would need to get further modified to handle RazorDocumentSyntax context propertly. Essentially it should handle it the same as the start tag context I think. Not a completely trivial change.

Given all of the above, I'd still say we should get this simple fix in and file bugs for the rest of the work (with some work done in WebTools by Vlad or his team). Or we could leave it as is (after fixing NRE) since we have no user feedback, and if the user actually starts typing (e.g. < or @) instead of using Ctrl+Space or Ctrl+J, things actually work fine - so I am not sure if all the work (especially changing RazorHtmlWriter, yes, I am a coward 😄 ) is justified.

[davidwengier]

Thanks for the screenshot. Totally happy to take this fix since there is actual user impact.

A real fix to have tooling handle empty documents (though my gut feeling is that we wouldn't want to change the compiler for that?) can come later if its complicated. I was hoping that simply using the owner, rather than its parent, in this case would just magically work :)

[alexgav]

Thanks for the screenshot. Totally happy to take this fix since there is actual user impact.

A real fix to have tooling handle empty documents (though my gut feeling is that we wouldn't want to change the compiler for that?) can come later if its complicated. I was hoping that simply using the owner, rather than its parent, in this case would just magically work :)

Sadly no :( HtmlFacts has special cases as well, we'd need to add one more for RazorDocumentSyntax which isn't completely obvious (the existing ones go off of the Tag properties, and document is not a tag of course 😄)

razor/src/Razor/src/Microsoft.CodeAnalysis.Razor.Workspaces/HtmlFacts.cs

Line 152 in 7b511df

case MarkupStartTagSyntax startTag:

Anyway - anything else to address in this one that we are waiting on or is it good to go?

[davidwengier]

It just needs to compile 😛

[alexgav]

It just needs to compile 😛

Oh c'mon, it's a minor detail 😄 (fixed, local commit on my box wasn't pushed, oops)

[alexgav]

• Filed No completion items are returned if invoking completion in an empty Razor document #10613 for issue # 1 (completely empty document)
• Filed (Microsoft internal only) bug https://devdiv.visualstudio.com/DevDiv/_workitems/edit/2136052 for issue # 2 (no HTML completions in an empty HTML document)
• Filed No tag helpers are added to completion in an emtpy Razor Document #10614 for issue # 3 (tag helpers should be merged in after issues # 1 and # 2 above are fixed)