.razor: implement the token type metrics

SonarSource / sonar-dotnet

Code analyzer for C# and VB.NET projects

https://redirect.sonarsource.com/plugins/csharp.html

GNU Lesser General Public License v3.0

787 stars 226 forks source link

.razor: implement the token type metrics #7895

Open costin-zaharia-sonarsource opened 1 year ago

costin-zaharia-sonarsource commented 1 year ago

Description

See the "Token type" section from the specification document. More details about the approach here.

Alternative

An alternative to this approach, which only focuses on mapping back tokens without altering the fundamental behavior of the Token Type Analyzer, is to use the RazorSyntaxTree parsing. That approach, that is more powerful but also more complex to implement, is captured by a dedicated issue.

Update

Moving back to "To do" waiting for the issue with location mapping for nested Razor expression to be fixed first, since wrong location mapping can produce wrong and overlapping tokens, and those break tokenization import on SonarQube/SonarCloud by the sensor.

Update 2024

Underlying PR closed for the reasons explained https://github.com/SonarSource/sonar-dotnet/pull/7924#issuecomment-2368879125. For a new implementation, please consider the analysis and the scenarios described in the description of the first PR.

antonioaversa commented 1 year ago

Handed over to @costin-zaharia-sonarsource. The underlying PR describes the current status and the next steps.

antonioaversa commented 1 year ago

Took it over from @csaba-sagi-sonarsource. After discussing with @csaba-sagi-sonarsource and @costin-zaharia-sonarsource, I am going to:

rebase onto the current master and fix conflicts
extend unit tests to include missing Razor features (e.g. nested components)
making sure everything works as expected and have a third round of review from @costin-zaharia-sonarsource
if time allows, check locally the performance delta (overall build time on mud-blazor) w.r.t. master (before master validation on peach)

antonioaversa commented 10 months ago

@costin-zaharia-sonarsource I am doing backlog cleanup, and I have a question about this issue and its related PR.

The issue has been hanging on the Kanban board, for a while now. At the time we decided not to proceed with razor tokenization, due to the issues with locations we had at the beginning. In the end, however, if I recall correctly, we went for our own custom mapping logic, so this task may make more sense now. There are still problems with location mapping in nested components, as explained in the description of the PR. As mentioned here, https://github.com/SonarSource/sonar-dotnet/issues/8040 should be addressed first.

There is quite a lot of code, and the PR went also for a round of review. However, the branch has become stale, and by now there are 10 conflicts with master.

How would you advise to proceed?

costin-zaharia-sonarsource commented 10 months ago

Since there are still problems with the mapping and the Roslyn APIs don't help us much currently, I would prefer to pause this effort for now. In my opinion, we should wait for the compiler team to finish the current effort in improving the API and continue when the new registrations are available.