.razor: implement the token type metrics

[antonioaversa]

Fixes #7895
Draft: don't merge, this PR is still based on #7916 by @costin-zaharia-sonarsource, due to dependencies on lines and location span mapping. DONE
Once #7916 is merged to master, this PR should be rebased. DONE
Also, IT should be implemented.

Issue with numeric tokens

The current implementation for the TokenTypeAnalyzer in this branch has an issue: it reports numeric tokens that don't exist in the original razor file. The issue comes from a bug in location mapping.

Take for example line 84 from the RazorTokens.razor file (defined here).

<p>@DateTime.Now</p>

The generated code for that line is the following:

            __builder.OpenElement(8, "p");
#nullable restore
#line (84,5)-(84,17) 24 "C:\dev\TestApp1\RazorClassLibrary1\Template.razor"
__builder.AddContent(9, DateTime.Now);

#line default
#line hidden
#nullable disable
            __builder.CloseElement();
            __builder.AddMarkupContent(10, "\r\n");

Because #line (84,5)-(84,17) 24 is not handled correctly when deriving the original location in the razor file from the location in the generated file, the numeric token 9 (right before DateTime.Now) is considered as coming from the original razor file, and not as a token in the generated code only.

Therefore, the following tokens are reported for line 84:

{ "tokenType": "NUMERIC_LITERAL", "textRange": { "startLine": 84, "endLine": 84, "startOffset": 4, "endOffset": 17 } }
{ "tokenType": "TYPE_NAME", "textRange": { "startLine": 84, "endLine": 84, "startOffset": 5, "endOffset": 13 } }

instead of

{ "tokenType": "TYPE_NAME", "textRange": { "startLine": 84, "endLine": 84, "startOffset": ..., "endOffset": ... } }

The issue basically affects all lines that generate enhanced line directives, and doesn't affect lines that:

• are entirely C# (e.g. within @code { ... } or @{ ... } etc.)
• or entirely Razor (e.g. @page "/razor", @namespace MyRazorNamespace, etc.)
• or entirely HTML (e.g. @@page "/razor" or <p>var x = 42;</p>).

Issue with tokens position asserting

Because:

• positions are off
• and wrong numeric tokens are reported, which don't exist in the original razor file, and overlap with other tokens
  we can't effectively use [x:...] syntax, introduced by @martin-strecker-sonarsource to assert tokens existence and positions.

The square brackets for all tokens whose location is mapped with enhanced line directives would need to be placed some columns off, to be reported correctly. Moreover, non-existing, numeric, overlapping tokens would break assertion.

So, for the time being, and while positions are fixed, this PR is only asserting how many tokens are defined per line, and of what type (here).

Performance impact of this change

Tested on local SonarQube Development 10.2.0.76792 on Dell laptop, on power network.
Project tested: https://github.com/MudBlazor/MudBlazor/tree/1e7677c70fefa1ac440e9fadbdf1aed64a336653
sonar-dotnet commit: 392d38b (feature)

Results (total build time):
00:01:28.33
00:01:58.35
00:02:02.40
00:01:50.90
00:01:59.53

sonar-dotnet commit: TBD (merge-base)
TBD

[antonioaversa]

@costin-zaharia-sonarsource The PR is still in draft and shouldn't be merged yet.
However, I am assigning this to you already as I would like to have early feedback on these first changes, before proceeding any further.
That will also make handover easier, shouldn't I be able to complete the work before the end of the week.
I have organized the history into 3 commits for an easier per-commit review.
Thanks!

[antonioaversa]

@costin-zaharia-sonarsource : Back to you for a second round of review.
UTs added, coverage increased from 93% of the 1st iteration to 100% of the 2nd iteration.
ITs are still missing, for the same reasons of the Symbol metric references and executable LOC metrics.

[antonioaversa]

@cristian-ambrosini-sonarsource As discussed offline, adding you for a first review of the second round of review. @costin-zaharia-sonarsource will then take over when I am off.

[sonarcloud]

Kudos, SonarCloud Quality Gate passed!   

0 Bugs
0 Vulnerabilities
0 Security Hotspots
0 Code Smells

100.0% Coverage
0.0% Duplication

[costin-zaharia-sonarsource]

Shouldn't return null if the filePath is null or whitespace?

[antonioaversa]

I made that choice for two reasons:

1. convenience: ClassifierTestHarness.AssertTokenTypes instantiates a TokenClassifier instance and a TriviaClassifier instance without having at hand the file path, which is implicitly created by ParseTokens
2. safety: while it seems indeed reasonable, to always return null in absence of filePath, I didn't know whether a SyntaxTree may have, for any reason, no file path (as it was somehow fully compiled in-memory), and I didn't want to alter the behavior the TokenTypeAnalyzer had before these changes

If we can/want to assume that the TokenTypeAnalyzer should always require filePath, we should do some refactoring to the ParseTokens, to basically return snippet0.cs, which would then be passed to the classifier.

[costin-zaharia-sonarsource]

Also here, I think we should ignore tokens if they don't have a file path. Do you have maybe an example of when this happens?

[antonioaversa]

Answered here

[costin-zaharia-sonarsource]

It looks good to me so far. As you mentioned, the test cases need to be finished, and we need to make some decisions about what we do with locations since they are wrongly mapped by Roslyn.

[cristian-ambrosini-sonarsource]

Good job! I left only one super nitpick comment

[antonioaversa]

@costin-zaharia-sonarsource @cristian-ambrosini-sonarsource I have addressed/replied to the comments made in the second round of review. I hand the PR over to @costin-zaharia-sonarsource for the next steps, which are described in the description of the PR.

[sonarcloud]

Kudos, SonarCloud Quality Gate passed!   

0 Bugs
0 Vulnerabilities
0 Security Hotspots
0 Code Smells

No Coverage information
No Duplication information

[antonioaversa]

Update: moved back to be a draft, since location mapping for nested Razor expressions should be fixed first, for the reasons expressed here.

[antonioaversa]

I am closing the PR, given that

• this effort was paused in 2023, waiting for the Roslyn team to change the mapping strategy
• the razor syntax highlighting has been de-prioritized afterward
• this PR has now a lot of conflicts, due to the many changes happened

I am adding a reference to the text in the description in the issue, so that a new implementation can benefit of the analysis done while implementing this PR.