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

Update console & classlib project templates to opt-in to ImplicitUsings feature #3619

Closed
Tracked by #19521
DamianEdwards opened this issue Aug 6, 2021 · 17 comments
Closed
Tracked by #19521
Assignees
Labels
triaged The issue was evaluated by the triage team, placed on correct area, next action defined.
Milestone

Comments

@DamianEdwards
Copy link
Member

To prepare for the changes coming in dotnet/sdk#19521 we need to update the .NET 6 project templates to include the <ImplicitUsings>enable</ImplicitUsings> property setup in their project files.

@DamianEdwards
Copy link
Member Author

FYI @vlada-shubina

@vlada-shubina
Copy link
Member

@bekir-ozturk @DavidKarlas could you please take it over whilst I'm on leave? Thanks

@DavidKarlas
Copy link
Contributor

I looked into making this change... We need SDK to make switch first so our integration tests can pass...

@bekir-ozturk bekir-ozturk added the triaged The issue was evaluated by the triage team, placed on correct area, next action defined. label Aug 9, 2021
@bekir-ozturk bekir-ozturk added this to the August Sprint milestone Aug 9, 2021
@DamianEdwards
Copy link
Member Author

@DavidKarlas why is that? The property is benign right now so I think the opposite is true isn't it? Add the property now so that when the SDK change lands the templates continue to get implicit usings applied.

@DavidKarlas
Copy link
Contributor

Problem is that if user specify following parameters:

dotnet new classlib -o MyProject --framework net6.0 --langVersion 9.0

and tries to build project using dotnet build this is result:

error CS8023: Feature 'global using directive' is not available in C# 9.0. Please use language version 10.0 or greater.

@AraHaan
Copy link
Member

AraHaan commented Aug 10, 2021

I think the issue is that the templates should be made dependent on language versions set.

However I think that with .NET 6 one should not be able to downgrade to C# 9 or lower due to the issue pointed above about global usings not being in C# 9.

@DamianEdwards
Copy link
Member Author

The templates that support specifying version as an option (like classlib and console) could decide to condition the addition of <ImplicitUsings>enable</ImplicitUsings> based on language version, i.e. if --langVersion is specified and is less than 10 then do not emit the property.

@DavidKarlas
Copy link
Contributor

I have that change ready, problem is that integration tests are failing with error message from previous comment with current SDK, hence need to wait for SDK change... I guess I could disable those tests temporary... I will open PR

@DamianEdwards
Copy link
Member Author

@DavidKarlas ah sorry I understand now. I keep forgetting the classlib and console templates are "special" in that they support defining language version and thus you'd have tests for that 😄

Do the needful to coordinate the changes.

@AraHaan
Copy link
Member

AraHaan commented Aug 10, 2021

Perhaps what should be done is default the templates to disabled but have a line that overrides it like so:

<ImplicitUsings Condition="'$(LangVersion)' == '10' OR '$(LangVersion)' == 'preview' OR '$(LangVersion)' == 'latest'">enable</ImplicitUsings>

I think with the .NET 6 SDK that technically all 3 values would mean the same thing?

Also I think with this it would make it a minimal yet also non-breaking change for tests?

@DamianEdwards
Copy link
Member Author

@AraHaan as long as the conditions are evaluated by the template engine at project creation time, rather than staying as MSBuild conditions in the project file, I think it makes sense to default the inclusion of <ImplicitUsings>enabled</ImplicitUsings> based on the template's --langVersion option.

@AraHaan
Copy link
Member

AraHaan commented Aug 10, 2021

Alternatively why not have the .NET Sdk enable automatically it for the user if they select a language version of 10 or newer?

@DamianEdwards
Copy link
Member Author

@AraHaan that's effectively what I'm saying and the PR does AFAICT.

@DamianEdwards
Copy link
Member Author

@DavidKarlas can this issue be closed now that your PR is merged?

@pranavkm
Copy link
Contributor

@DavidKarlas a build of the installer with the SDK changes are available in case you needed to update your tests.

@DavidKarlas
Copy link
Contributor

I noticed, thank you, I'm working on making changes, tnx!

@DavidKarlas
Copy link
Contributor

DavidKarlas commented Aug 17, 2021

With removal of <DisableImplicitUsings>... This change is now complete...

Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
triaged The issue was evaluated by the triage team, placed on correct area, next action defined.
Projects
None yet
Development

No branches or pull requests

6 participants