Update implicit global usings feature #19599

pranavkm · 2021-08-09T19:17:23Z

Undo changes to VB projects
For C#, projects use the Using itemgroup to generate global usings. Support aliasing and static imports with attributes.
Use ImplicitUsings to determine if SDK-specific namespaces are imported by default.

Contributes to #19521

DamianEdwards · 2021-08-09T19:33:47Z

Can we get the schema files updated as part of this too please?

pranavkm · 2021-08-09T20:25:07Z

Can we get the schema files updated as part of this too please?

Is it in this repo?

DamianEdwards · 2021-08-09T20:33:44Z

@pranavkm ah I think you're right and that it's over at https://github.com/dotnet/msbuild/blob/main/src/MSBuild/MSBuild/Microsoft.Build.CommonTypes.xsd

DamianEdwards · 2021-08-09T20:39:04Z

@pranavkm logged dotnet/msbuild#6745 to track the schema updates

DamianEdwards · 2021-08-09T22:25:15Z

@pranavkm when you merge this please change it to "Contributes to #19521" as it doesn't fix everything in that issue yet.

AraHaan · 2021-08-10T06:11:10Z

In this case cant all projects generate usings for System, System.Collections.Generic, System.Threading.Tasks, System.Linq, etc by default?

There might be a few others as well that even my projects use commonly as well too (exposed in Elskom/Sdk#248). In which case I can change some of them to the new <Using and nuke the other more common ones for the ones the .NET SDK would automatically add then.

src/BlazorWasmSdk/Sdk/Sdk.props

src/Tests/Microsoft.NET.Build.Tests/GivenThatWeWantToGenerateGlobalUsings_BlazorWasm.cs

dsplaisted · 2021-08-10T19:56:27Z

src/Tasks/Microsoft.NET.Build.Tasks/GenerateGlobalUsings.cs

+                var existing = File.ReadAllText(outputFilePath);
+
+                if (string.Equals(existing, contentToWrite, StringComparison.Ordinal))
+                {
+                     Log.LogMessage(MessageImportance.Low, Strings.SkippingUnchangedFile, OutputFile.ItemSpec);
+                    return;
+                }


The WriteLinesToFile task has a WriteOnlyWhenDifferent which handles this. Rather than duplicate the logic here (and have to add an additional string), I think we should change it so that the GenerateGlobalUsings task outputs the generated file contents to a property, and then we use the WriteLinesToFile task to write it with WriteOnlyWhenDifferent set to true.

I tried that before on my failed PR and it would result in test failures, I also tried consuming that task directly in a task like this and it would pass when ran on it's own on the specific test assembly for this dll, but then fail on other tests or at least the same tests in CI vs when ran locally.

dsplaisted · 2021-08-10T20:06:59Z

src/Tasks/Microsoft.NET.Build.Tasks/GenerateGlobalUsings.cs

+                if (@using.Static)
+                {
+                    builder.Append("static ");
+                }
+
+                if (!string.IsNullOrEmpty(@using.Alias))
+                {
+                    builder.Append(@using.Alias)
+                        .Append(" = ");
+                }


What if a Using item has both Static and Alias metadata. This will generate C# code that won't compile, right? Should we error here in that case?

Yeah we should error as global using static Foo = global::My.Namespace.Value isn't valid AFAIK.

My plan was to let the compiler complain (it produces an error saying that you cannot alias a static using). It felt less complicated to do this in case a newer langversion started supporting this in the future.

Letting the compiler generate the error seems less diagnosable to me. It will take you to the generated source file, but not indicate that the issue is with Import items.

I don't mind either way, it's just something to consider.

src/Tasks/Microsoft.NET.Build.Tasks/GenerateGlobalUsings.cs

src/Tests/Microsoft.NET.Build.Tests/GivenThatWeWantToGenerateGlobalUsings_DotNet.cs

src/Tests/Microsoft.NET.Build.Tests/GivenThatWeWantToGenerateGlobalUsings_WebApp.cs

src/Tests/Microsoft.NET.Build.Tests/GivenThatWeWantToGenerateGlobalUsings_Worker.cs

src/Tasks/Microsoft.NET.Build.Tasks/GenerateGlobalUsings.cs

src/Tasks/Microsoft.NET.Build.Tasks/targets/Microsoft.NET.Sdk.targets

src/Tests/Microsoft.NET.Build.Tests/GivenThatWeWantToGenerateGlobalUsings_DotNet.cs

src/Tests/Microsoft.NET.Build.Tests/GivenThatWeWantToGenerateGlobalUsings_WebApp.cs

* Undo changes to VB projects * For C#, projects use the `Using` itemgroup to generate global usings. Support aliasing and static imports with attributes. * Use ImplicitUsings to determine if SDK-specific namespaces are imported by default.

RussKie

👍

AraHaan · 2021-08-13T03:57:15Z

Thanks.

Relates to dotnet/sdk#19521 Relates to dotnet/sdk#19599

RussKie · 2021-08-18T03:01:10Z

src/Tasks/Microsoft.NET.Build.Tasks/targets/Microsoft.NET.Sdk.VisualBasic.targets

@@ -10,6 +10,30 @@ Copyright (c) .NET Foundation. All rights reserved.
 ***********************************************************************************************
 -->
 <Project ToolsVersion="14.0" xmlns="http://schemas.microsoft.com/developer/msbuild/2003">
+  <ItemGroup Condition=" '$(DisableImplicitNamespaceImports)' != 'true' and '$(TargetFrameworkIdentifier)' == '.NETFramework'">


May I please get a clarification why VB's implicit imports are now hidden behind DisableImplicitNamespaceImports, which is on by default? VB always had implicit imports on by default AFAIK.

This has broken dotnet/winforms#5437

/cc: @KathleenDollard @KlausLoeffelmann @DamianEdwards

I think this may be the culprit, which needs to be undone:

For a test I built a .NET 5.0 VB app, and neither DisableImplicitNamespaceImports nor DisableImplicitFrameworkReferences are defined.

E.g.

dotnet new winforms --language vb -f net5.0 dotnet build /bl start msbuild.binlog

And it has all imports:

Looks like this might have been missed. @pranavkm?

Why does the screen shot show "net6.0-windows" if this is a " .NET 5.0 VB app"?

The issue that @RussKie is running in to is because of an Arcade change - dotnet/arcade#7745. That said, I missed undoing one of the VB changes that is covered as part of #19840

dotnet-issue-labeler bot added the Area-NetSDK label Aug 9, 2021

pranavkm marked this pull request as ready for review August 9, 2021 20:25

pranavkm requested review from dsplaisted and davidfowl August 9, 2021 20:25

KalleOlaviNiemitalo mentioned this pull request Aug 10, 2021

ImplicitNamespaceImports generated codefile generates an invalid import if you try to import as an alias. #19446

Closed

dsplaisted reviewed Aug 10, 2021

View reviewed changes

KalleOlaviNiemitalo reviewed Aug 11, 2021

View reviewed changes

src/Tasks/Microsoft.NET.Build.Tasks/GenerateGlobalUsings.cs Show resolved Hide resolved

AraHaan suggested changes Aug 11, 2021

View reviewed changes

src/Tasks/Microsoft.NET.Build.Tasks/targets/Microsoft.NET.Sdk.targets Show resolved Hide resolved

src/Tests/Microsoft.NET.Build.Tests/GivenThatWeWantToGenerateGlobalUsings_DotNet.cs Outdated Show resolved Hide resolved

dsplaisted mentioned this pull request Aug 12, 2021

Update implicit global usings feature to address issues #19521

Closed

12 tasks

RussKie reviewed Aug 12, 2021

View reviewed changes

src/Tests/Microsoft.NET.Build.Tests/GivenThatWeWantToGenerateGlobalUsings_WebApp.cs Show resolved Hide resolved

Update implicit global usings feature

08db8d7

* Undo changes to VB projects * For C#, projects use the `Using` itemgroup to generate global usings. Support aliasing and static imports with attributes. * Use ImplicitUsings to determine if SDK-specific namespaces are imported by default.

pranavkm force-pushed the prkrishn/sdk-implicitusings branch from e64e990 to 08db8d7 Compare August 12, 2021 15:57

pranavkm added 3 commits August 12, 2021 11:17

Fixup test

87baea3

One more test

e2f1577

Fixup

dc5f247

markwilkie mentioned this pull request Aug 12, 2021

Work around implicit usings breaking change dotnet/arcade#7745

Merged

dsplaisted approved these changes Aug 13, 2021

View reviewed changes

RussKie reviewed Aug 13, 2021

View reviewed changes

pranavkm merged commit f2cc77b into main Aug 13, 2021

pranavkm deleted the prkrishn/sdk-implicitusings branch August 13, 2021 01:40

KalleOlaviNiemitalo mentioned this pull request Aug 13, 2021

.NET SDK 6 Preview 7 generated file ImplicitNamespaceImports.cs contains error CS8773 #19702

Closed

RussKie mentioned this pull request Aug 16, 2021

Update implicit global usings feature dotnet/winforms#5409

Closed

RussKie added a commit to RussKie/winforms that referenced this pull request Aug 17, 2021

Update import of implicit usings

cb9a032

Relates to dotnet/sdk#19521 Relates to dotnet/sdk#19599

RussKie mentioned this pull request Aug 17, 2021

Update import of implicit usings dotnet/winforms#5432

Merged

RussKie added a commit to RussKie/winforms that referenced this pull request Aug 17, 2021

Update SDK and opt-in implicit usings

4899baa

Relates to dotnet/sdk#19521 Relates to dotnet/sdk#19599

RussKie mentioned this pull request Aug 17, 2021

Update SDK and opt-in implicit usings dotnet/winforms#5433

Merged

RussKie added a commit to dotnet/winforms that referenced this pull request Aug 17, 2021

Update SDK and opt-in implicit usings (#5433)

ede63c8

Relates to dotnet/sdk#19521 Relates to dotnet/sdk#19599

RussKie added a commit to dotnet/wpf that referenced this pull request Aug 17, 2021

Update SDK

eebb035

Relates to dotnet/sdk#19521 Relates to dotnet/sdk#19599

RussKie mentioned this pull request Aug 17, 2021

Update WindowsDesktop SDK to support recent changes to implicit global usings feature dotnet/wpf#5084

Merged

ryalanms pushed a commit to dotnet/wpf that referenced this pull request Aug 18, 2021

Update SDK (#5084)

123b057

Relates to dotnet/sdk#19521 Relates to dotnet/sdk#19599

RussKie mentioned this pull request Aug 18, 2021

[main] Update dependencies from dotnet/arcade dotnet/winforms#5437

Merged

RussKie reviewed Aug 18, 2021

View reviewed changes

RussKie mentioned this pull request Aug 19, 2021

Revert "Work around implicit usings breaking change (#7745)" dotnet/arcade#7766

Merged

1 task

alexrp mentioned this pull request Aug 23, 2021

Automatically provide global using spectreconsole/spectre.console#516

Closed

jonathanpeppers mentioned this pull request Oct 19, 2021

Implement .NET 6 features for templates. dotnet/maui#3018

Merged

Provide feedback

Saved searches

Use saved searches to filter your results more quickly

Update implicit global usings feature #19599

Update implicit global usings feature #19599

pranavkm commented Aug 9, 2021 •

edited

Loading

DamianEdwards commented Aug 9, 2021

pranavkm commented Aug 9, 2021

DamianEdwards commented Aug 9, 2021

DamianEdwards commented Aug 9, 2021

DamianEdwards commented Aug 9, 2021

AraHaan commented Aug 10, 2021 •

edited

Loading

dsplaisted Aug 10, 2021

AraHaan Aug 11, 2021

dsplaisted Aug 10, 2021

DamianEdwards Aug 10, 2021

pranavkm Aug 10, 2021

dsplaisted Aug 11, 2021

RussKie left a comment

AraHaan commented Aug 13, 2021

RussKie Aug 18, 2021

RussKie Aug 18, 2021

RussKie Aug 18, 2021

DamianEdwards Aug 18, 2021

KalleOlaviNiemitalo Aug 18, 2021

pranavkm Aug 18, 2021

Update implicit global usings feature #19599

Update implicit global usings feature #19599

Conversation

pranavkm commented Aug 9, 2021 • edited Loading

DamianEdwards commented Aug 9, 2021

pranavkm commented Aug 9, 2021

DamianEdwards commented Aug 9, 2021

DamianEdwards commented Aug 9, 2021

DamianEdwards commented Aug 9, 2021

AraHaan commented Aug 10, 2021 • edited Loading

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

RussKie left a comment

Choose a reason for hiding this comment

AraHaan commented Aug 13, 2021

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

pranavkm commented Aug 9, 2021 •

edited

Loading

AraHaan commented Aug 10, 2021 •

edited

Loading