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

Odd behaviour with async method declaration: bad colorization #60339

Closed
DoctorKrolic opened this issue Mar 23, 2022 · 5 comments · Fixed by #60558
Closed

Odd behaviour with async method declaration: bad colorization #60339

DoctorKrolic opened this issue Mar 23, 2022 · 5 comments · Fixed by #60558
Labels
4 - In Review A fix for the issue is submitted for review. Area-IDE Concept-Continuous Improvement
Milestone

Comments

@DoctorKrolic
Copy link
Contributor

Version Used:
VS 2022 v.17.2 Preview 2.1

Steps to Reproduce:
Start code:

class Test
{
    public async
}

Expected Behavior:
async is colorized as a keyword

Actual Behavior:
devenv_FQBeKlVXzM

@CyrusNajmabadi
Copy link
Member

At this point, we don't know if async is going to be a keyword, or is just a return type for a member that it's in the middle of being written. We could colorize as keyword in that case, but it's very minor.

@CyrusNajmabadi
Copy link
Member

It's called a 'contextual' keyword. Meaning we allow it to be an identifier as well for compat reasons. e.g. it's entirely legal (and has happened) for someone to write: class async { } ... public async GetAsync() { ... }.

Note that while rare, we do support this as this code existed prior to us adding async as an actual language feature.

@DoctorKrolic
Copy link
Contributor Author

Yeah, I know about contextual keywords, but in this case it doesn't seem reasonable for me. You can't do every new keyword in language contextual (or you can, but it will complicate compiler and IDE and in the end result in many developer's hours wasted to support backward compatability for 0.00...01% of users, who followed bad naming practices). value contextual keyword is reasonable, because it is a very common name for local names and method parameters and it has a very limited scope, in which it is legal to use it. async is just a method modifier, like public, abstract etc. and scope of usage for it as a lot bigger. There is list of breaking changes for every realese of .NET, which contains several times more than 2 or 3 items. Having a breaking change "async is no longer a valid type name" will apply to 0.0000...01% of users, but in the end will result in simplification of compiler and IDE code.

@CyrusNajmabadi
Copy link
Member

You can't do every new keyword in language contextual (or you can, but it will complicate compiler and IDE and in the end result in many developer's hours wasted to support backward compatability for 0.00...01% of users, who followed bad naming practices)

We don't want to break people. So we take that burden to keep that compat bar. And we have done that for every single lang version we have :)

but in the end will result in simplification of compiler and IDE code.

Here's the thing though. It's not that complex for us to support it, and we're totally fine with taking that pain ourselves instead of causing pain to customers. We may change things in the future, but we're not going to do it haphazardly.

@CyrusNajmabadi
Copy link
Member

who followed bad naming practices)

This is not true fwiw. And developers have many reasons they may end up here. Notably because they were mapping an existing API from some other system, or even auto-generating code based on schemas and whatnot. People do this and their code works and continues to work. We do make the determination at times to break things, but only when it's really really critical and the break is worth it. That's not the case here. It's not a big problem to just support this, and it means no pain for customers. Our objective is not to avoid pain on our side by pushing onto customers.

@jinujoseph jinujoseph added 4 - In Review A fix for the issue is submitted for review. Concept-Continuous Improvement and removed untriaged Issues and PRs which have not yet been triaged by a lead labels Apr 5, 2022
@jinujoseph jinujoseph added this to the 17.3 milestone Apr 5, 2022
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
4 - In Review A fix for the issue is submitted for review. Area-IDE Concept-Continuous Improvement
Projects
None yet
3 participants