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

Console/Class lib project templates use latest C# language idioms #3360

Closed
8 tasks done
Tracked by #3359
DamianEdwards opened this issue Jun 29, 2021 · 14 comments
Closed
8 tasks done
Tracked by #3359
Assignees
Labels
area: template-content The issue is related to content of template packages managed in this repo (/template_feed) triaged The issue was evaluated by the triage team, placed on correct area, next action defined. User Story A single user-facing feature. Can be grouped under an epic.
Milestone

Comments

@DamianEdwards
Copy link
Member

DamianEdwards commented Jun 29, 2021

@DamianEdwards DamianEdwards added the User Story A single user-facing feature. Can be grouped under an epic. label Jun 29, 2021
@vlada-shubina
Copy link
Member

vlada-shubina commented Jun 30, 2021

@DamianEdwards

Couple things here:

  1. Recently we created new app template (create new console-app template #3248). This template suppose to replace console template. It already has most of the features in and we are adding global usings in Add global using's for console and classlib templates #3344. I can add async Main in same time.

  2. Nullables were already added to classlib template: Change C# templates to have #nullable enabled by default #2979. I can add file scoped templates and global usings if needed.

  3. Just double checking - any changes needed for other languages (F#, VB)?

@KathleenDollard
Copy link

@DamianEdwards There is an enormous amount of tutorials, instructions, books, etc in the wild that has been created over the years. This includes everything from high school curriculum to SO posts and is used by the people that are introducing .NET for us. Given the amount of this information and the fact it fundamentally not working affects our newest customers as they are just trying .NET, we decided that a new template was least disruptive. In addition, the name "console" has meaning only from the perspective of the tech, not naïve intuition. We decided to "freeze" Console, and thus did not add nullable.

For these reasons we added a new "app" template with the expectation that all beginner instructions etc. will change to this template over (hopefully short) time allowing us to make this change without disrupting our new users and have a better name. In addition to the scenario where a teacher hands out instructions that make no sense, someone could rebuild their laptop a week before finals to do some end semester work and be unable to do it. We can instead teach teachers about using these new features and allow them to introduce namespaces and class at the time they think appropriate. Of course we update things that can be disruptive, but this massive change to Console would disrupt the wrong people at the wrong time.

Something else has taken a great deal of time over the last several days, and thus the blog post for templates in Preview 6 was delayed.

@KathleenDollard
Copy link

@vlada-shubina There is another change we are working on that will affect global usings overall. So hold off on that change to the app template please.

@DamianEdwards
Copy link
Member Author

@KathleenDollard thanks for the details. Given that context, are we planning on removing console from the default list shown when dotnet new is invoked without arguments? The request here is that all our templates are updated to reflect the latest and greatest language idioms while acknowledging that this could cause friction in some cases. Given the specific concerns RE console and education however, perhaps a compromise of not showing it in the default set is acceptable.

@vlada-shubina no need to add async Main unless the template is already calling Task-returning methods, or calling methods that have Task-returning equivalents.

@vlada-shubina vlada-shubina added this to the July Sprint milestone Jul 5, 2021
@vlada-shubina vlada-shubina added the triaged The issue was evaluated by the triage team, placed on correct area, next action defined. label Jul 5, 2021
vlada-shubina added a commit to vlada-shubina/templating that referenced this issue Jul 6, 2021
@vlada-shubina
Copy link
Member

@DamianEdwards @KathleenDollard File-scoped namespaces were added in #3395, however these templates cannot be built at the moment, therefore we cannot merge it until it is supported for build.

@DamianEdwards
Copy link
Member Author

@vlada-shubina thanks, this is the compiler issue to watch for them landing the file-scoped namespaces feature in preview.7

vlada-shubina added a commit to vlada-shubina/templating that referenced this issue Jul 7, 2021
vlada-shubina added a commit to vlada-shubina/templating that referenced this issue Jul 13, 2021
@vlada-shubina
Copy link
Member

vlada-shubina commented Jul 13, 2021

@DamianEdwards on this thread: we are removing app template #3423 now and porting all features of it to console net6.0 template.

@DamianEdwards @KathleenDollard what should be behavior or the features with regards to language version: we have a parameter langVersion that sets C# language version to use.

  • global usings are not available before 10.0
  • top-level statements are not available before 9.0
  • nullable is not available before 8.0

Should we make a condition on those or should the template just fail in case lang version is different from 10.0?

How other repos are handling langVersion (winforms, wpf)?

vlada-shubina added a commit to vlada-shubina/templating that referenced this issue Jul 13, 2021
@vlada-shubina
Copy link
Member

vlada-shubina commented Jul 13, 2021

@DamianEdwards @KathleenDollard another issue blocking PRs with global usings: global usings are not working for language version other than 10.0. The project on language version below 10.0 cannot be compiled.
This blocks number of tests.

Example:

> dotnet new console --langVersion 9.0
(succeeds)

> dotnet build
    Microsoft (R) Build Engine version 17.0.0-preview-21359-04+d150e93ff for .NET
Copyright (C) Microsoft Corporation. All rights reserved.

  Determining projects to restore...
  All projects are up-to-date for restore.
  You are using a preview version of .NET. See: https://aka.ms/dotnet-core-preview
C:\Users\vshubina\AppData\Local\Temp\TemplateEngine.Tests\258e9877-0085-44ec-91bb-419c11a5f7a3\obj\Debug\net6.0\258e9877-0085-44ec-91bb-419c11a5f7a3.ImplicitNamespaceImports.cs(2,1): error CS8773: Feature 'global using directive' is not available in C# 9.0. Please use language version 10.0 or greater. [C:\Users\vshubina\AppData\Local\Temp\TemplateEngine.Tests\258e9877-0085-44ec-91bb-419c11a5f7a3\258e9877-0085-44ec-91bb-419c11a5f7a3.csproj]
C:\Users\vshubina\AppData\Local\Temp\TemplateEngine.Tests\258e9877-0085-44ec-91bb-419c11a5f7a3\obj\Debug\net6.0\258e9877-0085-44ec-91bb-419c11a5f7a3.ImplicitNamespaceImports.cs(3,1): error CS8773: Feature 'global using directive' is not available in C# 9.0. Please use language version 10.0 or greater. [C:\Users\vshubina\AppData\Local\Temp\TemplateEngine.Tests\258e9877-0085-44ec-91bb-419c11a5f7a3\258e9877-0085-44ec-91bb-419c11a5f7a3.csproj]
C:\Users\vshubina\AppData\Local\Temp\TemplateEngine.Tests\258e9877-0085-44ec-91bb-419c11a5f7a3\obj\Debug\net6.0\258e9877-0085-44ec-91bb-419c11a5f7a3.ImplicitNamespaceImports.cs(4,1): error CS8773: Feature 'global using directive' is not available in C# 9.0. Please use language version 10.0 or greater. [C:\Users\vshubina\AppData\Local\Temp\TemplateEngine.Tests\258e9877-0085-44ec-91bb-419c11a5f7a3\258e9877-0085-44ec-91bb-419c11a5f7a3.csproj]
C:\Users\vshubina\AppData\Local\Temp\TemplateEngine.Tests\258e9877-0085-44ec-91bb-419c11a5f7a3\obj\Debug\net6.0\258e9877-0085-44ec-91bb-419c11a5f7a3.ImplicitNamespaceImports.cs(5,1): error CS8773: Feature 'global using directive' is not available in C# 9.0. Please use language version 10.0 or greater. [C:\Users\vshubina\AppData\Local\Temp\TemplateEngine.Tests\258e9877-0085-44ec-91bb-419c11a5f7a3\258e9877-0085-44ec-91bb-419c11a5f7a3.csproj]
C:\Users\vshubina\AppData\Local\Temp\TemplateEngine.Tests\258e9877-0085-44ec-91bb-419c11a5f7a3\obj\Debug\net6.0\258e9877-0085-44ec-91bb-419c11a5f7a3.ImplicitNamespaceImports.cs(6,1): error CS8773: Feature 'global using directive' is not available in C# 9.0. Please use language version 10.0 or greater. [C:\Users\vshubina\AppData\Local\Temp\TemplateEngine.Tests\258e9877-0085-44ec-91bb-419c11a5f7a3\258e9877-0085-44ec-91bb-419c11a5f7a3.csproj]
C:\Users\vshubina\AppData\Local\Temp\TemplateEngine.Tests\258e9877-0085-44ec-91bb-419c11a5f7a3\obj\Debug\net6.0\258e9877-0085-44ec-91bb-419c11a5f7a3.ImplicitNamespaceImports.cs(7,1): error CS8773: Feature 'global using directive' is not available in C# 9.0. Please use language version 10.0 or greater. [C:\Users\vshubina\AppData\Local\Temp\TemplateEngine.Tests\258e9877-0085-44ec-91bb-419c11a5f7a3\258e9877-0085-44ec-91bb-419c11a5f7a3.csproj]
C:\Users\vshubina\AppData\Local\Temp\TemplateEngine.Tests\258e9877-0085-44ec-91bb-419c11a5f7a3\obj\Debug\net6.0\258e9877-0085-44ec-91bb-419c11a5f7a3.ImplicitNamespaceImports.cs(8,1): error CS8773: Feature 'global using directive' is not available in C# 9.0. Please use language version 10.0 or greater. [C:\Users\vshubina\AppData\Local\Temp\TemplateEngine.Tests\258e9877-0085-44ec-91bb-419c11a5f7a3\258e9877-0085-44ec-91bb-419c11a5f7a3.csproj]

Build FAILED.

C:\Users\vshubina\AppData\Local\Temp\TemplateEngine.Tests\258e9877-0085-44ec-91bb-419c11a5f7a3\obj\Debug\net6.0\258e9877-0085-44ec-91bb-419c11a5f7a3.ImplicitNamespaceImports.cs(2,1): error CS8773: Feature 'global using directive' is not available in C# 9.0. Please use language version 10.0 or greater. [C:\Users\vshubina\AppData\Local\Temp\TemplateEngine.Tests\258e9877-0085-44ec-91bb-419c11a5f7a3\258e9877-0085-44ec-91bb-419c11a5f7a3.csproj]
C:\Users\vshubina\AppData\Local\Temp\TemplateEngine.Tests\258e9877-0085-44ec-91bb-419c11a5f7a3\obj\Debug\net6.0\258e9877-0085-44ec-91bb-419c11a5f7a3.ImplicitNamespaceImports.cs(3,1): error CS8773: Feature 'global using directive' is not available in C# 9.0. Please use language version 10.0 or greater. [C:\Users\vshubina\AppData\Local\Temp\TemplateEngine.Tests\258e9877-0085-44ec-91bb-419c11a5f7a3\258e9877-0085-44ec-91bb-419c11a5f7a3.csproj]
C:\Users\vshubina\AppData\Local\Temp\TemplateEngine.Tests\258e9877-0085-44ec-91bb-419c11a5f7a3\obj\Debug\net6.0\258e9877-0085-44ec-91bb-419c11a5f7a3.ImplicitNamespaceImports.cs(4,1): error CS8773: Feature 'global using directive' is not available in C# 9.0. Please use language version 10.0 or greater. [C:\Users\vshubina\AppData\Local\Temp\TemplateEngine.Tests\258e9877-0085-44ec-91bb-419c11a5f7a3\258e9877-0085-44ec-91bb-419c11a5f7a3.csproj]
C:\Users\vshubina\AppData\Local\Temp\TemplateEngine.Tests\258e9877-0085-44ec-91bb-419c11a5f7a3\obj\Debug\net6.0\258e9877-0085-44ec-91bb-419c11a5f7a3.ImplicitNamespaceImports.cs(5,1): error CS8773: Feature 'global using directive' is not available in C# 9.0. Please use language version 10.0 or greater. [C:\Users\vshubina\AppData\Local\Temp\TemplateEngine.Tests\258e9877-0085-44ec-91bb-419c11a5f7a3\258e9877-0085-44ec-91bb-419c11a5f7a3.csproj]
C:\Users\vshubina\AppData\Local\Temp\TemplateEngine.Tests\258e9877-0085-44ec-91bb-419c11a5f7a3\obj\Debug\net6.0\258e9877-0085-44ec-91bb-419c11a5f7a3.ImplicitNamespaceImports.cs(6,1): error CS8773: Feature 'global using directive' is not available in C# 9.0. Please use language version 10.0 or greater. [C:\Users\vshubina\AppData\Local\Temp\TemplateEngine.Tests\258e9877-0085-44ec-91bb-419c11a5f7a3\258e9877-0085-44ec-91bb-419c11a5f7a3.csproj]
C:\Users\vshubina\AppData\Local\Temp\TemplateEngine.Tests\258e9877-0085-44ec-91bb-419c11a5f7a3\obj\Debug\net6.0\258e9877-0085-44ec-91bb-419c11a5f7a3.ImplicitNamespaceImports.cs(7,1): error CS8773: Feature 'global using directive' is not available in C# 9.0. Please use language version 10.0 or greater. [C:\Users\vshubina\AppData\Local\Temp\TemplateEngine.Tests\258e9877-0085-44ec-91bb-419c11a5f7a3\258e9877-0085-44ec-91bb-419c11a5f7a3.csproj]
C:\Users\vshubina\AppData\Local\Temp\TemplateEngine.Tests\258e9877-0085-44ec-91bb-419c11a5f7a3\obj\Debug\net6.0\258e9877-0085-44ec-91bb-419c11a5f7a3.ImplicitNamespaceImports.cs(8,1): error CS8773: Feature 'global using directive' is not available in C# 9.0. Please use language version 10.0 or greater. [C:\Users\vshubina\AppData\Local\Temp\TemplateEngine.Tests\258e9877-0085-44ec-91bb-419c11a5f7a3\258e9877-0085-44ec-91bb-419c11a5f7a3.csproj]
    0 Warning(s)
    7 Error(s)

Time Elapsed 00:00:01.96

Is it expected failure? How we should deal with other language versions in net6.0 templates?

@DamianEdwards
Copy link
Member Author

@vlada-shubina yes global usings requires C# 10. @JunTaoLuo do the implicit namespace conditions in the SDK not include a check of LangVersion if it's defined?

@JunTaoLuo
Copy link
Contributor

JunTaoLuo commented Jul 13, 2021

I'm hesitant about a LangVersion check. Specifically, I couldn't find a good way of expressing the logic "LangVersion >= 10.0".

LangVersion can be set to non-numerical values such as latest, latestMajor, default, preview each of which may resolve as different numerical versions. I think all this logic is encapsulated in the compiler at the moment and if the resolution can be made available in an MSBuild task, I wouldn't mind a LangVersion check in the SDK. As it stands now, I'm not sure it's a great idea to replicate the compiler logic for resolving the raw LangVersion since it may drift from the compiler and hard to ensure parity.

As for the tests being blocked, could we disable the feature with DisableImplicitNamespaceImports before building? That is what we envisioned for projects that have the new TFM (>net6.0) but specifically sets the LangVersion to a lower level. I'm not sure how we can do that in the template though since it would also require us to understand LangVersion semantically.

Another idea is that we should output a better error message somehow that indicates if CS8773 occurs, DisableImplicitNamespaceImports can be set to disable this feature.

@vlada-shubina
Copy link
Member

vlada-shubina commented Jul 13, 2021

@JunTaoLuo I see, we have the similar problem here. We have a --langVersion option, which allows setting the value of <LangVersion> property in csproj file when running dotnet new. We also don't have a good way to manage conditions on that for the new features.
The draft was here: 4a16adf however it doesn't take into account latest, preview etc values which have to be evaluated as mentioned above.

@DamianEdwards have you discussed this issue before? dotnet/templating is not the only repo using langVersion option, at least wpf and winforms are using it as well. It would be good to align on strategy so we do same thing across the repos.

As example:

  • if the user uses --langVersion 9.0 or below we should fallback template to not use global usings as the project won't be built.
  • if the user uses --langVersion 8.0 or below we should fallback template to not use top-level programs.
  • if the user uses --langVersion 7.3 or below we should fallback template to not use nullable.

@DamianEdwards
Copy link
Member Author

Yeah seems we'll have to condition this in the templates that have a --langVersion option (the web templates do not).

Your enumeration seems like the correct approach.

vlada-shubina added a commit to vlada-shubina/templating that referenced this issue Jul 14, 2021
@vlada-shubina vlada-shubina added the area: template-content The issue is related to content of template packages managed in this repo (/template_feed) label Jul 14, 2021
vlada-shubina added a commit to vlada-shubina/templating that referenced this issue Jul 15, 2021
vlada-shubina added a commit to vlada-shubina/templating that referenced this issue Jul 15, 2021
vlada-shubina added a commit to vlada-shubina/templating that referenced this issue Jul 15, 2021
vlada-shubina added a commit to vlada-shubina/templating that referenced this issue Jul 15, 2021
vlada-shubina added a commit to vlada-shubina/templating that referenced this issue Jul 15, 2021
vlada-shubina added a commit to vlada-shubina/templating that referenced this issue Jul 15, 2021
vlada-shubina added a commit that referenced this issue Jul 15, 2021
@vlada-shubina
Copy link
Member

@DamianEdwards all tasks in this issue have been completed, closing. Feel free to reopen if needed.

@jcouv
Copy link
Member

jcouv commented Jul 24, 2021

The file-scoped namespaces portion of this change is causing some pain, as the IDE experience is not yet fully baked for this feature. Added you to the email thread to discuss temporarily reverting that change. Thanks

Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
area: template-content The issue is related to content of template packages managed in this repo (/template_feed) triaged The issue was evaluated by the triage team, placed on correct area, next action defined. User Story A single user-facing feature. Can be grouped under an epic.
Projects
None yet
Development

No branches or pull requests

5 participants