TokenType: Tests and optimization for pointer types

[martin-strecker-sonarsource]

Follow up to #7369

[antonioaversa]

Requesting additional tests. I have also made a few educational questions that are lower priority, not blocking the merge of the PR, and can be addressed at a later stage.

[antonioaversa]

Educational: could you please briefly explain when a token is of an unknown type (i.e. TokenType.UnknownTokentype), and why the method parameter dateTimePointer is of such type?

[martin-strecker-sonarsource]

Unknown is the default tokentype and is used for all tokens that are not of any of the other types:

  public enum TokenType {
    [pbr::OriginalName("UNKNOWN_TOKENTYPE")] UnknownTokentype = 0,
    [pbr::OriginalName("TYPE_NAME")] TypeName = 1,
    [pbr::OriginalName("NUMERIC_LITERAL")] NumericLiteral = 2,
    [pbr::OriginalName("STRING_LITERAL")] StringLiteral = 3,
    [pbr::OriginalName("KEYWORD")] Keyword = 4,
    [pbr::OriginalName("COMMENT")] Comment = 5,
  }

"Unknown" tokens are skipped when the info is written to the disc because there is no difference.

[antonioaversa]

Educational: I guess Date is of unknown type because dateTimePointer is of unknown type. Is that correct?

[martin-strecker-sonarsource]

UnknownTokentype here means the classification of the token isn't any of the other TokenTypes. It doesn't have anything to do with the actual .Net types involved. The enum is badly named. It should be called TokenClassification instead of TokenType.

Also:
For all identifier tokens, there can only be two possible TokenType outcomes: "TypeName" or "UnknownTokentype" *). If we can conclude that the token can never bind to a type, we are done. A pointer can never be a type. (On a normal member access, we can not be sure: Math.Pi here Math can bind to a type or a property. Here we need to consult the semantic model for Math and see what it binds to.

*) One exception is "value" in property setters, which is classified as a keyword.

[antonioaversa]

Educational: [u:dateTimePointer] and [u:Date] could be assessed in the same test, correct? Decorating the token dateTimePointer with the token type assertion syntax doesn't impact the token Date.

[martin-strecker-sonarsource]

Sure. I try to assert only single tokens as it eases the test debugging. Also, the allowSemanticModel value may differ between the tokens. I just prefer the test isolation here (test a single thing).

[antonioaversa]

There are some additional test cases that come to mind:

int?* x1; // Pointer of a nullable value type ("?" shouldn't be part of the token, right?
Nullable<int>* x2; // Nullable is of type t, whereas int is of type k, both are recognized
int** x3; // ** is not included in the token int, which is of type k

And relative assignments:

_ = &x1; // x1 of type u
_ = (*(&x1))->Value; // x1 and Value of type u
_ = (*x1).Value; // x1 and Value of type u

There are also ways of mixing @ and &, and we may want to make sure we only highlight the right thing:

var @int = (int*)null; // 2nd int and null of type k, 1st int of type u (?) with @ included
_ = *@int; // @int of type u (?), * not included, @ included
_ = &@int; // @int of type u (?), & not included, @ included

You can also similarly mix [] with * and &.

Hereafter is my understanding of one of the cases mentioned above (to be verified):

[DataRow("var [u:@int] = ([k:int]*)[k:null];_ = *[u:@int];", false)]

Would it make sense to cover (some of) them?

[martin-strecker-sonarsource]

I added a couple of tests. The @ tests are a bit unrelated and I separated those. The @ is just parsed as part of the identifier token, so it doesn't really change anything.

[antonioaversa]

Educational: this assertion still holds when using using unsafe from C# 12, right?

using System;
using unsafe Ptr = int*;

internal class Program
{
    private unsafe static void Main(string[] args)
    {
        int x = 3;
        Ptr y = &x;
        Console.WriteLine($"x = {y->ToString()}");
    }
}

Ptr should still be considered TokenType.UnknownTokentype, and not a type. Correct?

[martin-strecker-sonarsource]

It just works. Ptr is in a type position and is therefore classified as type (There is a distinguishing happening before ClassifyMemberAccess is called. An IdentfierSymtax can either be in an expression or a type context. In a type context, a single IdentfierName resolves to TokenType.TypeName and in an expression context to UnknownTokentype).

[sonarqubecloud]

Kudos, SonarCloud Quality Gate passed!   

0 Bugs
0 Vulnerabilities
0 Security Hotspots
0 Code Smells

No Coverage information
No Duplication information

[sonarqubecloud]

Kudos, SonarCloud Quality Gate passed!   

0 Bugs
0 Vulnerabilities
0 Security Hotspots
0 Code Smells

100.0% Coverage
0.0% Duplication

[antonioaversa]

LGTM, thanks a lot for your explanations, they made the code much clearer to me!

[antonioaversa]

@martin-strecker-sonarsource Converted to draft temporarily, to avoid merging by mistake, while the CI runs.
The latest failure of UTs doesn't seem to have anything to do with the changes of this PR, and they seem to be related to Azure machines provisioning. I am rerunning the UTs. As soon as they are green, I am going to undraft and merge the PR.