Allow components in global namespace

[jjonescz]

Fixes https://devdiv.visualstudio.com/DevDiv/_workitems/edit/1954771.
Fixes #10356.

[jjonescz]

_type.ContainingNamespace.ToDisplayString() would be "<global namespace>"; with SymbolExtensions.FullNameTypeDisplayFormat we get an empty string instead (this format was already used in other places, just not everywhere)

[jjonescz]

MVC scenarios still call this overload. They already kinda work - you can see that in the added test - they use a constant fallback namespace name, but do not append hash to class name, so users could be referencing them. Razor components in global namespace, on the other hand, used also fallback class name with appended hash so it would be nearly impossible for users to reference them.

[davidwengier]

Not sure how common this is, but I would expect that formatting of Razor components in the global namespace would not work. With on-type formatting specifically, editing these files could be a pretty annoying experience.

[jjonescz]

I would expect that formatting of Razor components in the global namespace would not work.

Is there something I should change to make it work?

[davidwengier]

I think expecting you to fix the formatting engine is unreasonable. Maybe if we're really lucky, then we'd just need to change the numbers in this line from 2 and 3 to 1 and 2:
https://github.com/dotnet/razor/blob/main/src/Razor/src/Microsoft.AspNetCore.Razor.LanguageServer/Formatting/FormattingSpan.cs#L52

I guess it would be nice to ensure there is some way we can find out if a particular Razor document is in the global namespace? Either a bool, or maybe a "base indent" we can use? That way at least we're set up for success. Personally I don't expect this will be too common that I'd block merging it on formatting being correct. Though having said that, I've no idea what the experience for users would actually be like :)

[jjonescz]

Oh, so the concern is about indentation? Components in the global namespace are actually still indented as if they were emitted inside namespace block: https://github.com/dotnet/razor/pull/10086/files#diff-38349d3b6effd9eb09b5b2da2152117586045b6e2171fda332b593a705269ea5

[davidwengier]

That's how they're indented before we ask Roslyn to format the generated code, and then interpret the results.

[jjonescz]

@davidwengier

I've tried testing formatting on component in global namespace (debugged through it to verify the generated C# code really doesn't contain the namespace block). It works. But I'm not sure if this is the correct place or correct way to test this.

I've also tried this in VS - and the design-time doc is using namespace __GeneratedComponent (my guess is RootNamespace is set to null) which is wrong but at least formatting should work. I will try to see if that wrong namespace can be fixed as well (without breaking formatting).

[davidwengier]

If the test is generating the code you expect, then great. Perhaps my fears are unfounded, or its only certain edge cases that would be affected. Either way, adding the infrastructure for us to test it, like you have, will be very valuable so thank you very much for that.

[jjonescz]

I ended up doing what you suggested to fix formatting. Thanks!

[jjonescz]

@dotnet/razor-compiler @dotnet/razor-tooling this is ready for review; thanks

[davidwengier]

Tooling LGTM. Thanks for the extra test coverage.

[jjonescz]

@dotnet/razor-compiler for reviews, thanks

[333fred]

Compiler changes LGTM. Didn't review the IDE layer changes.

[333fred]

Would prefer if we were more direct with the condition here:

Suggested change

	var containingNamespace = type.ContainingNamespace.ToDisplayString(SymbolExtensions.FullNameTypeDisplayFormat);
	var fullName = string.IsNullOrEmpty(containingNamespace)
	? type.Name
	: $"{containingNamespace}.{type.Name}";
	var fullName = type.ContainingNamespace.IsGlobalNamespace
	? type.Name
	: $"{type.ContainingNamespace.ToDisplayString(SymbolExtensions.FullNameTypeDisplayFormat)}.{type.Name}";

[333fred]

Also, is it possible to have a component in a nested type? If so, will this handle that correctly?

[jjonescz]

Also, is it possible to have a component in a nested type? If so, will this handle that correctly?

I don't think it's possible. (You've already asked me this previously in #9689 (comment) :D)

In any case, the previous code did "{namespace}.{typeName}" and I've just changed it to omit the "{namespace}." prefix when needed, so nothing should change at this front, right?