dotnet / razor

Compiler and tooling experience for Razor ASP.NET Core apps in Visual Studio, Visual Studio for Mac, and VS Code.
https://asp.net
MIT License
493 stars 191 forks source link

Remove IDocumentMappingService.GetLanguageKind(...) and make it an extension method on RazorCodeDocument #10851

Closed DustinCampbell closed 2 weeks ago

DustinCampbell commented 3 weeks ago

For a long while, the GetLanguageKind(...) method that determines whether an index into a document falls within Razor, C# or HTML has been a bit of a wart on the IDocumentMappingService. It really isn't part of document mapping, and its implementation is completely distinct. In fact, making the actual change is quite simple, so why hadn't it been done yet? The answer is mocking.

There are several tests that mock IDocumentMappingService.GetLanguageKind(...) to lie about test inputs. In my not-so-humble opinion, this represents an abuse of mocking. Instead of setting up tests to have the necessary inputs that ensure GetLanguageKind(...) would return a real and correct result, the inputs would often be garbage and an IDocumentMappingService mock would lie about the GetLanguageKind(...) result at a particular location. This makes moving GetLanguageKind(...) off of IDocumentMappingService a much larger change than it needs to be. This is why there are substantial test changes in this PR.

Don't misunderstand me as a mocking hater! Mocking libraries are definitely useful! In fact, there are new mocks used in this very PR! However, mocks should be used judiciously and thoughtfully, and in this case, a mock was used to write lazy tests.

Fixes https://github.com/dotnet/razor/issues/8774