Skip to content
New issue

Have a question about this project? Sign up for a free GitHub account to open an issue and contact its maintainers and the community.

By clicking “Sign up for GitHub”, you agree to our terms of service and privacy statement. We’ll occasionally send you account related emails.

Already on GitHub? Sign in to your account

TokenType: Tests and support for scoped type declarations #8061

Merged
merged 3 commits into from
Sep 25, 2023

Conversation

martin-strecker-sonarsource
Copy link
Contributor

Follow up to #7369

Copy link
Contributor

@antonioaversa antonioaversa left a comment

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

LGTM! Interesting C# feature, I was not aware of.
I proposed a couple of optional extensions to the UTs.

[DataRow("scoped ref [t:S] s", false)]
[DataRow("ref [t:S] s", false)]
[DataRow("scoped [t:S] s", false)]
public void IdentifierToken_Scoped_Parameter(string parameterDeclaration, bool allowSemanticModel) =>
Copy link
Contributor

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Optional: We could add a test combining scoped ref with readonly, when the reference is one to a readonly ref struct, just to make sure that readonly is interpreted as keyword, when following ref.

E.g. here, where GitHub fails at properly coloring it.

Copy link
Contributor Author

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

This does not compile (readonly parameter are at implemented proposal stage dotnet/csharplang#6010 for C#12). But for locals it works and I added a test case.

{
}
}
""", allowSemanticModel);
Copy link
Contributor

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Optional: I would also add tests with a parameter and/or variable named scope. Unlike other keyword, that require escaping with @, scope has been introduced recently, and as such, is not tokenized as a keyword when used after the type:

int scoped = 3;

I think it would make sense to assert that scoped is of type u, and not k.

@sonarqubecloud
Copy link

Kudos, SonarCloud Quality Gate passed!    Quality Gate passed

Bug A 0 Bugs
Vulnerability A 0 Vulnerabilities
Security Hotspot A 0 Security Hotspots
Code Smell A 0 Code Smells

No Coverage information No Coverage information
No Duplication information No Duplication information

@sonarqubecloud
Copy link

Kudos, SonarCloud Quality Gate passed!    Quality Gate passed

Bug A 0 Bugs
Vulnerability A 0 Vulnerabilities
Security Hotspot A 0 Security Hotspots
Code Smell A 0 Code Smells

100.0% 100.0% Coverage
0.0% 0.0% Duplication

@martin-strecker-sonarsource martin-strecker-sonarsource merged commit 6869c7c into master Sep 25, 2023
22 checks passed
@martin-strecker-sonarsource martin-strecker-sonarsource deleted the Martin/TokenType_Scoped branch September 25, 2023 15:39
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
None yet
Projects
None yet
Development

Successfully merging this pull request may close these issues.

2 participants