.IsRazorDocument() doesn't work in OOP scenarios, so as a workaround I added a check for .EndsWith(".g.cs") instead which seems to do the trick

@NTaylorMullen for thoughts.

note: This feels weird as well. somethign about the entire razor space here seems somewhat broken (necessitating these extra strange codepaths at the feature level). my preference would be that if razor is currently doing something different/weird, that we just not have this optimization/feature for them. and, if we can then get razor docs/scenarios to work like normal scenarios, then we reenable, with no specialized code in the feature to handle razor.

Most of this special casing is due to Razor not running the C# syntax classifier on their end. However, I believe adding support is a pretty big work item and could possibly also negatively impact Razor perf, so the effects would have to be investigated.

While this special casing doesn't look good, I think it will result in pretty good gains for Razor semantic tokens which imo might be an appropriate trade-off since the Razor LSP editor is on by default for users.

@dibarbet here as well...

interesting. why is this @allisonchou ? why not have the normal incremental analyzer handle this. note: if this is the right way to go, then this comment needs beefing up. Specifically, this is an example of a comment htat just repeats what the code is doing but doesn't provide clarity as to 'why'. I cannot look at this code/comment and say: ah, yes, that makes sense, we def should be doing things in this fashion.

The main issue with using the default logic is the method's call to document.IsOpen(). This call is problematic in LSP for two reasons:

1. It bases whether the document is open on the non-LSP workspace rather than the LSP workspace.
2. Razor back-end generated documents won't be considered open even if the front-end document is open. This means we would have to special case Razor (via exposing a new document service, workspace service or something else).

Putting the caching logic in the LSP handler addresses both 1) and 2) by ensuring we only cache open files in the LSP workspace, since we only get semantic token requests for the LSP file(s) currently open + we get requests for Razor generated documents. In an ideal world, I think we'd have the IsOpen check correspond to the LSP workspace and that workspace can then specially check for open Razor documents, but I believe that is a much bigger work item.

why this change?

I believe this is an existing bug in the logic related to the comment I added below:

                    // There may be multiple semantic parts corresponding to a single
                    // syntactic part, so we might need to go through a syntactic part
                    // multiple times to verify. For example, this is the case with
                    // verbatim string literals containing string escape characters.

Essentially, there's a chance the TextSpan of the semantic part will not exactly match the TextSpan of the syntactic part. This occurs with string literals containing string escape characters. The string escape character will not have the same exact span as the syntactic part and therefore will not be considered. I believe instead calling OverlapsWith mitigates the problem.

did this test go missing? I'm not seeing a new test that verifies the partial (1,0) -> (2,0) positions

Whoops, you're totally right. Thanks! Added it back.