Some OOB assemblies are not copying intellisense extracted from docs package

[carlossanlop]

Some of our assemblies are not showing intellisense in VS.

With our recent intellisense changes, I checked if we were overwriting the built intellisense xml with the one from the docs nuget package, and noticed it's not. I think we should, if the UseIntellisensePackageDocXmlFile property is not explicitly set to false in the csproj.

This problem seems to be somewhat related to having the GeneratePlatformNotSupportedAssemblyMessage property specified in the csproj, but I'm not entirely confident, because there are some assemblies without this problem. The IsPartialFacadeAssembly property does not seem to be affecting, although it could also be a combination of the two properties.

Here's a list I collected:

IsPackable assembly	Generates PNSE?	Is Façade?	Intellisense in VS?
System.Security.Cryptography.ProtectedData	No	Yes	No
System.Net.Http.WinHttpHandler	Yes	No	No
System.Security.Cryptography.Pkcs	Yes	No	No
System.Speech	Yes	No	No
System.Diagnostics.EventLog	Yes	Yes	No
System.IO.Ports	Yes	Yes	No
Microsoft.Extensions.Hosting	No	No	Yes
System.Reflection.MetadataLoadContext	No	No	Yes
System.Resources.Extensions	No	No	Yes
System.Text.Encodings.Web	No	No	Yes
System.ComponentModel.Composition	Yes	No	Yes
System.Reflection.Context	Yes	No	Yes
System.Data.Odbc	Yes	Yes	Yes

Related to #79030

[ghost]

Tagging subscribers to this area: @dotnet/area-infrastructure-libraries
See info in area-owners.md if you want to be subscribed.

Issue Details

---

Some of our assemblies are not showing intellisense in VS.

With our recent intellisense changes, I checked if we were overwriting the built intellisense xml with the one from the docs nuget package, and noticed it's not. I think we should, if the UseIntellisensePackageDocXmlFile property is not explicitly set to false in the csproj.

This problem seems to be somewhat related to having the GeneratePlatformNotSupportedAssemblyMessage property specified in the csproj, but I'm not entirely confident, because there are some assemblies without this problem. The IsPartialFacadeAssembly property does not seem to be affecting.

Here's a list I collected:

IsPackable assembly	Generates PNSE?	Is Façade?	Intellisense in VS?
System.Security.Cryptography.ProtectedData	No	Yes	No
System.Net.Http.WinHttpHandler	Yes	No	No
System.Security.Cryptography.Pkcs	Yes	No	No
System.Speech	Yes	No	No
System.Diagnostics.EventLog	Yes	Yes	No
System.IO.Ports	Yes	Yes	No
Microsoft.Extensions.Hosting	No	No	Yes
System.Reflection.MetadataLoadContext	No	No	Yes
System.Resources.Extensions	No	No	Yes
System.Text.Encodings.Web	No	No	Yes
System.ComponentModel.Composition	Yes	No	Yes
System.Reflection.Context	Yes	No	Yes
System.Data.Odbc	Yes	Yes	Yes

Related to #79030

Author:	carlossanlop
Assignees:	-
Labels:	documentation, area-Infrastructure-libraries
Milestone:	-

[carlossanlop]

Looking at this a bit more with a binlog, it seems the property IsNetCoreAppSrc is not set when running dotnet pack on the src csproj of an OOB assembly. This causes the CopyDocumentationFileToXmlDocDir target to get skipped.

runtime/eng/intellisense.targets

Lines 41 to 43 in 2ba2396

	<Target Name="CopyDocumentationFileToXmlDocDir"
	AfterTargets="CopyFilesToOutputDirectory"
	Condition="'$(IsNetCoreAppSrc)' == 'true' and '$(TargetFramework)' == '$(NetCoreAppCurrent)'"

This might be naive to ask, but I wonder if that condition could be extended to check if either IsNetCoreAppSrc is true or if IsPackable is true.

[ViktorHofer]

it seems the property IsNetCoreAppSrc is not set when running dotnet pack on the src csproj of an OOB assembly

Correct. IsNetCoreAppSrc is true if an assembly is part of the shared framework which isn't true for OOB assemblies. So that's the correct behavior.

This causes the CopyDocumentationFileToXmlDocDir target to get skipped.

We discussed that in your PR. We don't need xml files from OOB assemblies to be copied into the xml doc directory. That directory is expected to only contain shared framework xml doc files.

[carlossanlop]

What we discussed it in my PR was the placement of xml files of OOB packages in the artifacts/bin/docs folder, which is useless for OOB packages. What I am trying to do in this issue is to understand why the documentation of OOB packages regressed from 5.0: they used to have their full documentation, but now they only show SR strings. I posted some additional findings, specific to System.IO.Ports, in #79030.

I think we could add logic that swaps the built xml file of OOB packages with the one from dotnet-api-docs. I assume we would have to do this somewhere in the packaging.targets file. The logic would be very similar to what we have in intellisense.targets but the target path of the xml would be different.

[ViktorHofer]

I think we could add logic that swaps the built xml file of OOB packages with the one from dotnet-api-docs. I assume we would have to do this somewhere in the packaging.targets file. The logic would be very similar to what we have in intellisense.targets but the target path of the xml would be different.

What you describe is what this target already does:

runtime/eng/intellisense.targets

Lines 30 to 39 in 65f5ad0

	<!-- TODO: Remove this target when no library relies on the intellisense documentation file anymore.-->
	<!-- Replace the default xml file generated in the obj folder with the one that comes from the docs feed. -->
	<Target Name="ChangeDocumentationFileForPackaging"
	AfterTargets="DocumentationProjectOutputGroup"
	Condition="'$(UseIntellisensePackageDocXmlFile)' == 'true'">
	<ItemGroup>
	<DocFileItem Remove="@(DocFileItem)" />
	<DocFileItem Include="$(IntellisensePackageXmlFilePath)" />
	</ItemGroup>
	</Target>

How does Visual Studio fetch intellisense documentation from projects? Is that happening inside a design time build? cc @ericstj

[carlossanlop]

What you describe is what this target already does

I tested running the dotnet pack command inside src/libraries/System.IO.Ports, and Unfortunately, the ChangeDocumentationFileForPackaging target is not getting called at all. That target only gets invoked in System.Text.Encoding.CodePages, which seems to be a referenced project of System.IO.Ports.

If it worked for that command, I am not sure if the file would be swapped on time, because it looks like packaging.targets gets included before intellisense.targets. Unless I'm missing something, I think that means the nupkg gets generated first.

[ViktorHofer]

If it worked for that command, I am not sure if the file would be swapped on time, because it looks like packaging.targets gets included before intellisense.targets. Unless I'm missing something, I think that means the nupkg gets generated first.

That's not how msbuild works. The order of target file imports doesn't define the order in which targets run.

I tested running the dotnet pack command inside src/libraries/System.IO.Ports, and Unfortunately, the ChangeDocumentationFileForPackaging target is not getting called at all. That target only gets invoked in System.Text.Encoding.CodePages, which seems to be a referenced project of System.IO.Ports.

That's because you missed @ericstj's comment here: #79134 (comment). He suggested to use a different hook point as the current one runs too late. It's not clear to me how this actually worked to begin with and I'm nervous that our current packages in main contain the wrong intellisense file. Can you please double check that that's not the case?

[carlossanlop]

It seems that we've been including the wrong intellisense file in OOB packages since .NET 6.0 RC2.

I created a console app targeting net6.0, I added the System.IO.Ports nuget package as a dependency, and manually changes versions, until I found that VS was still showing the correct intellisense in the nuget package version 6.0.0-rc.1.21451.13 (published on Sept 14 2021), and intellisense completely disappeared starting with version 6.0.0-rc.2.21480.5 (published on Oct 12 2021).

The RC1 package contained the xml file that included all the APIs, and the RC2 only contained exceptions with SR strings.

The .NET 6.0 RC1 branch was snapped on August 17th and shipped on Sept 14th, while RC2 was snapped on Sept 14th and shipped on Oct 12th. Both RC1 and RC2 got snapped from main. Calendar.

I found a couple of big refactoring that were merged to main before the RC2 snap and touched the packaging infra (Sept 14th):

• Simplify .NETFramework tfms #58558 (Sept 10th).
• Upgrade to net7.0 and remove target frameworks #58011 (Sept 8th).

I also found a change that might be relevant only to the System.IO.Ports case, but it went into RC1, so chances are it didn't cause this regression:

• Fix analyzer, docs file packaging and don't overbuild runtime.native IO.Ports packages. #57118 (Aug 10th)

[ViktorHofer]

Simplify .NETFramework tfms #58558

That change isn't part of the release/6.0 branch (it can't be - otherwise we would have shipped net7.0 assemblies in our 6.0 packages):

Upgrade to net7.0 and remove target frameworks #58011 (Sept 8th).

That change isn't either in 6.0:

I agree that the intellisense files are broken since .NET 6 but the above listed changes aren't the culprit.

[ViktorHofer]

Reason for that is that the packaging logic tries to find the documentation file here (from the release/7.0 branch): C:\Users\vikto\.nuget\packages\microsoft.private.intellisense\7.0.0-preview-20221010.1\IntellisenseFiles\net\1033\System.Drawing.Common.xml. I think we already talked about this and noticed that the wrong intellisense files were picked up for some time.

This regressed with 688206b which changed the lookup code for the intellisense files. Previously, all files under the netcoreapp folder were considered. The package layout presumably then changed to net and net-plat-extensions. Unfortunately, the change only considered the former but not the latter folder.

Fortunately this is already fixed in main:

runtime/eng/intellisense.targets

Lines 20 to 21 in 986abec

	<IntellisensePackageXmlFilePathFromNetFolder>$([MSBuild]::NormalizePath('$(IntellisensePackageXmlRootFolder)', 'net', '1033', '$(AssemblyName).xml'))</IntellisensePackageXmlFilePathFromNetFolder>
	<IntellisensePackageXmlFilePathFromDotNetPlatExtFolder>$([MSBuild]::NormalizePath('$(IntellisensePackageXmlRootFolder)', 'dotnet-plat-ext', '1033', '$(AssemblyName).xml'))</IntellisensePackageXmlFilePathFromDotNetPlatExtFolder>

Meanwhile, as this is entirely unrelated to what this issue is tracking, @carlossanlop can you please work on the fix that @ericstj suggested in #79134 (comment)?