Improvements to Content management for consistency

Prevent users from using `contentFiles` as a relative dir in their
@content items, since that would be a user error since the folder
is already added to Content items with no CopyToOutputDirectory.

Keep the original behavior where Content is by default:
- CodeLanguage=any
- TargetFramework=current TFM.

Meaning that Content is by default %(FrameworkSpecific)=true. This
can obviously be overriden by either:

- Adding an explicit %(TargetFramework) to target (i.e. 'net46' or 'any')
- Adding %(FrameworkSpecific)=false

Turned off inclusion of None items by default, since they basically serve
the same purpose as raw PackageFile. Turning it on by default could mean
inadvertently including stuff that we shouldn't.

Included additional tests for the metadata overrides for lang/tfm.

NOTE: we're not defaulting %(Content.TargetFramework) via an
ItemDefinitionGroup to avoid potentially colliding with other targets in
what seems like a super generic metadata name. So instead, we process
the default in AssignPackagePath and assign that default value to the
resulting items, so the calculated/inferred value can be inspected if
necessary.

Also, added the PackageFile as an AvailableItemName so that users can
trivially select it as the Build Action for an item, and cause it to
be included in the package by default. This would make using None
completely unnecessary.

Author: kzu

src/Build/NuGet.Build.Packaging.Tasks/AssignPackagePath.cs

@@ -117,38 +117,62 @@ ITaskItem EnsurePackagePath(ITaskItem file, IDictionary<string, ITaskItem> kindM
 			// Special case for contentFiles, since they can also provide a codeLanguage metadata
 			if (packageFolder == PackagingConstants.Folders.ContentFiles)
 			{
-				/// See https://docs.nuget.org/create/nuspec-reference#contentfiles-with-visual-studio-2015-update-1-and-later
+				if (file.GetMetadata("TargetPath").StartsWith(packageFolder, StringComparison.OrdinalIgnoreCase))
+				{
+					Log.LogErrorCode(nameof(ErrorCode.NG0013), ErrorCode.NG0013(file.GetMetadata("TargetPath")));
+					// We return the file anyway, since the task result will still be false.
+					return file;
+				}
+
+				// See https://docs.nuget.org/create/nuspec-reference#contentfiles-with-visual-studio-2015-update-1-and-later
 				var codeLanguage = file.GetMetadata(MetadataName.ContentFile.CodeLanguage);
 				if (string.IsNullOrEmpty(codeLanguage))
+				{
 					codeLanguage = PackagingConstants.AnyFramework;
+					output.SetMetadata(MetadataName.ContentFile.CodeLanguage, codeLanguage);
+				}
 
 				packageFolder = Path.Combine(packageFolder, codeLanguage);
 
 				// And they also cannot have an empty framework, at most, it will be "any"
 				if (string.IsNullOrEmpty(targetFramework))
 					targetFramework = PackagingConstants.AnyFramework;
+
+				// Once TF is defaulted, a content file is actually always framework-specific, 
+				// although the framework may be 'any'.
+				frameworkSpecific = true;
+
+				// At this point we have the correct target framework for a content file, so persist it.
+				output.SetMetadata(MetadataName.TargetFramework, targetFramework);
 			}
 
+			var targetPath = file.GetMetadata("TargetPath");
 			// NOTE: TargetPath allows a framework-specific file to still specify its relative 
 			// location without hardcoding the target framework (useful for multi-targetting and 
-			// P2P references)
-			var targetPath = file.GetMetadata("TargetPath");
+			// P2P references).
 			if (string.IsNullOrEmpty(targetPath))
 			{
-				targetPath = kind == PackageItemKind.None ? 
-					// For None, preserve the relative dir. 
-					file.GetMetadata("RelativeDir") + file.GetMetadata("FileName") + file.GetMetadata("Extension") :
+				targetPath = string.IsNullOrEmpty(packageFolder) ?
+					Path.Combine(file.GetMetadata("RelativeDir"), file.GetMetadata("FileName") + file.GetMetadata("Extension")) :
+					// Well-known folders only get root-level files by default. Can be overriden with PackagePath or TargetPath 
+					// explicitly, of course
 					file.GetMetadata("FileName") + file.GetMetadata("Extension");
 			}
 
-			// None files or those for which we know no mapping, go straight to the root folder of the package, 
-			// respecting their RelativeDir.
+			if (!string.IsNullOrEmpty(packageFolder) && 
+				targetPath.StartsWith(packageFolder + Path.DirectorySeparatorChar, StringComparison.OrdinalIgnoreCase))
+			{
+				// Avoid duplicating already determined package folder in package path later on.
+				targetPath = targetPath.Substring(packageFolder.Length + 1);
+			}
+
+			// If we have no known package folder, files go to their RelativeDir location.
 			// This allows custom packaging paths such as "workbooks", "docs" or whatever, which aren't prohibited by 
 			// the format.
 			var packagePath = string.IsNullOrEmpty(packageFolder) ?
 				// File goes to the determined target path (or the root of the package), such as a readme.txt
 				targetPath :
-				frameworkSpecific ? 
+				frameworkSpecific ?
 					// Otherwise, it goes to a framework-specific folder.
 					Path.Combine(new[] { packageFolder, targetFramework }.Concat(targetPath.Split(Path.DirectorySeparatorChar)).ToArray()) :
 					// Except if frameworkSpecific is false, such as for build, tools, runtimes

src/Build/NuGet.Build.Packaging.Tasks/NuGet.Build.Packaging.Inference.targets

@@ -25,7 +25,7 @@ Copyright (c) .NET Foundation. All rights reserved.
 		<!-- Whether to include @(Content) items with CopyToOutputDirectory != '' in the package -->
 		<IncludeContentInPackage Condition="'$(IncludeContentInPackage)' == ''">true</IncludeContentInPackage>
 		<!-- Whether to include @(None) items with CopyToOutputDirectory != '' in the package -->
-		<IncludeNoneInPackage Condition="'$(IncludeNoneInPackage)' == ''">true</IncludeNoneInPackage>
+		<IncludeNoneInPackage Condition="'$(IncludeNoneInPackage)' == ''">false</IncludeNoneInPackage>
 		<!-- Whether to include @(BuiltProjectOutputGroupOutput), @(DocumentationProjectOutputGroupOutput) and @(SatelliteDllsProjectOutputGroupOutput) items in the package -->
 		<IncludeOutputsInPackage Condition="'$(IncludeOutputsInPackage)' == ''">true</IncludeOutputsInPackage>
 		<!-- Whether to include @(DebugSymbolsProjectOutputGroupOutput) items in the package -->
@@ -88,7 +88,6 @@ Copyright (c) .NET Foundation. All rights reserved.
 			</_InferredPackageFile>
 
 			<!-- NOTE: Content is opt-out (must have Pack=false to exclude if IncludeContentInPackage=true) -->
-			<!-- @ContentFilesProjectOutputGroupOutput = @(ContentWithTargetPath -> '%(FullPath)') -->
 			<_InferredPackageFile Include="@(ContentWithTargetPath->'%(FullPath)')"
 							   Condition="
 									'%(ContentWithTargetPath.Pack)' == 'true' or 
@@ -98,13 +97,14 @@ Copyright (c) .NET Foundation. All rights reserved.
 				<Kind Condition="'%(ContentWithTargetPath.Kind)' == '' and '%(ContentWithTargetPath.CopyToOutputDirectory)' == ''">Content</Kind>
 			</_InferredPackageFile>
 
-			<!-- NOTE: None is also opt-out (must have Pack=false to exclude if IncludeNoneInPackage=true, but only for those with CopyToOutput) -->
+			<!-- NOTE: None is also opt-out (must have Pack=false to exclude if IncludeNoneInPackage=true, but this property defaults to false) -->
 			<_InferredPackageFile Include="@(_NoneWithTargetPath->'%(FullPath)')"
 							   Condition="
 									'%(_NoneWithTargetPath.Pack)' == 'true' or 
 									'%(_NoneWithTargetPath.Kind)' != '' or 
-									('$(IncludeNoneInPackage)' == 'true' and '%(_NoneWithTargetPath.CopyToOutputDirectory)' != '' and '%(_NoneWithTargetPath.Pack)' != 'false')">
+									('$(IncludeNoneInPackage)' == 'true' and '%(_NoneWithTargetPath.Pack)' != 'false')">
 				<Kind Condition="'%(_NoneWithTargetPath.Kind)' == '' and '%(_NoneWithTargetPath.CopyToOutputDirectory)' != ''">$(PrimaryOutputKind)</Kind>
+				<Kind Condition="'%(_NoneWithTargetPath.Kind)' == '' and '%(_NoneWithTargetPath.CopyToOutputDirectory)' == ''">None</Kind>
 			</_InferredPackageFile>
 
 			<_InferredPackageFile Include="@(PackageReference)" Condition="'%(Identity)' != 'NuGet.Build.Packaging'">

src/Build/NuGet.Build.Packaging.Tasks/NuGet.Build.Packaging.ReferenceAssembly.targets

@@ -47,6 +47,7 @@ Copyright (c) .NET Foundation. All rights reserved.
 		<ItemGroup>
 			<PackageFile Include="@(_ReferenceAssemblyTargetPath)">
 				<Kind>Lib</Kind>
+				<Source>Implicit</Source>
 				<TargetFrameworkMoniker>%(_ReferenceAssemblyTargetPath.TargetFrameworkMoniker)</TargetFrameworkMoniker>
 			</PackageFile>
 		</ItemGroup>

src/Build/NuGet.Build.Packaging.Tasks/NuGet.Build.Packaging.Tasks.csproj

@@ -48,9 +48,9 @@
     <Content Include="NuGet.Build.Packaging.Authoring.targets">
       <CopyToOutputDirectory>PreserveNewest</CopyToOutputDirectory>
     </Content>
-    <None Include="NuGet.Build.Packaging.Compatibility.props">
+    <Content Include="NuGet.Build.Packaging.Compatibility.props">
       <CopyToOutputDirectory>PreserveNewest</CopyToOutputDirectory>
-    </None>
+    </Content>
     <Content Include="NuGet.Build.Packaging.ReferenceAssembly.targets">
       <CopyToOutputDirectory>PreserveNewest</CopyToOutputDirectory>
     </Content>

src/Build/NuGet.Build.Packaging.Tasks/NuGet.Build.Packaging.props

@@ -26,6 +26,10 @@ Copyright (c) .NET Foundation. All rights reserved.
 		<ApiIntersectToolExe Condition=" '$(ApiIntersectToolExe)' == '' ">ApiIntersect.exe</ApiIntersectToolExe>
 	</PropertyGroup>
 
+	<ItemGroup>
+		<AvailableItemName Include="PackageFile" />
+	</ItemGroup>
+
 	<ItemDefinitionGroup>
 		<PackageFile>
 			<!-- See @(PackageItemKind) below -->

src/Build/NuGet.Build.Packaging.Tasks/Properties/Resources.Designer.cs

@@ -126,4 +126,7 @@
   <data name="ErrorCode_NG0012" xml:space="preserve">
     <value>Duplicate package source files detected for target package file path '{0}'.</value>
   </data>
+  <data name="ErrorCode_NG0013" xml:space="preserve">
+    <value>Content files cannot specify the reserved 'contentFiles' relative directory. Please use the 'CodeLanguage' and 'TargetFramework' item metadata for '{0}' to control its relative path within 'contentFiles'.</value>
+  </data>
 </root>

src/Build/NuGet.Build.Packaging.Tasks/Properties/Resources.resx

@@ -10,6 +10,7 @@
 using static NuGet.Build.Packaging.Properties.Strings;
 using Metadata = System.Collections.Generic.Dictionary<string, string>;
 using System;
+using NuGet.Build.Packaging.Properties;
 
 namespace NuGet.Build.Packaging
 {
@@ -278,6 +279,34 @@ public void when_file_has_target_framework_and_tfm_then_existing_value_is_preser
 			}));
 		}
 
+		[Fact]
+		public void when_content_is_not_framework_specific_then_has_any_lang_and_tfm()
+		{
+			var task = new AssignPackagePath
+			{
+				BuildEngine = engine,
+				Kinds = kinds,
+				Files = new ITaskItem[]
+				{
+					new TaskItem("readme.txt", new Metadata
+					{
+						{ "PackageId", "A" },
+						{ "Kind", "Content" },
+						{ "FrameworkSpecific", "false" },
+						{ "TargetFrameworkMoniker", "MonoAndroid,Version=v2.5" },
+					})
+				}
+			};
+
+			Assert.True(task.Execute());
+
+			Assert.Contains(task.AssignedFiles, item => item.Matches(new
+			{
+				CodeLanguage = "any",
+				TargetFramework = "any",
+				PackagePath = @"contentFiles\any\any\readme.txt"
+			}));
+		}
 
 		// TODO: these all end up in all lowercase, but MonoAndroid, Xamarin.iOS are usually properly 
 		// cased in nupkgs out in the wild (i.e. Rx)
@@ -440,6 +469,28 @@ public void when_assigning_content_file_then_applies_tfm_and_language(string tfm
 		}
 
 		[Fact]
+		public void when_assigning_content_file_with_reserved_dir_then_fails()
+		{
+			var task = new AssignPackagePath
+			{
+				BuildEngine = engine,
+				Kinds = kinds,
+				Files = new ITaskItem[]
+				{
+					new TaskItem(@"contentFiles\cs\monodroid\content.cs", new Metadata
+					{
+						{ "PackageId", "A" },
+						{ "TargetFrameworkMoniker", ".NETFramework,Version=v4.5" },
+						{ "TargetPath", @"contentFiles\cs\monodroid\content.cs" },
+						{ "Kind", "Content" },
+					})
+				}
+			};
+
+			Assert.False(task.Execute());
+			Assert.True(engine.LoggedErrorEvents.Any(e => e.Code == nameof(Strings.ErrorCode.NG0013)));
+		}
+
 		public void when_assigning_content_file_with_additional_metadata_then_preserves_metadata()
 		{
 			var task = new AssignPackagePath

src/Build/NuGet.Build.Packaging.Tests/AssignPackagePathTests.cs

@@ -42,10 +42,15 @@
     <Content Include="Scenarios\given_a_library_with_content\none-copy-with-include-true.txt" />
     <Content Include="Scenarios\given_a_library_with_content\none-copy-with-kind.txt" />
     <Content Include="Scenarios\given_a_library_with_content\none-with-kind.txt" />
+    <Content Include="Scenarios\given_a_library_with_content\quickstart\any-non-tfm.txt" />
+    <Content Include="Scenarios\given_a_library_with_content\quickstart\fs-tfm.txt" />
+    <Content Include="Scenarios\given_a_library_with_content\quickstart\any-any.txt" />
     <Content Include="Scenarios\given_a_library_with_content\relative\content-copy-with-kind.txt" />
     <Content Include="Scenarios\given_a_library_with_content\relative\content-copy.txt" />
     <Content Include="Scenarios\given_a_library_with_content\relative\none-copy-with-kind.txt" />
     <Content Include="Scenarios\given_a_library_with_content\relative\none-copy.txt" />
+    <None Include="Scenarios\given_a_multi_platform_solution\android\quickstart\sample.fs" />
+    <None Include="Scenarios\given_a_multi_platform_solution\android\quickstart\sample.vb" />
     <Content Include="Scenarios\given_duplicate_package_files\a.csproj" />
     <Content Include="Scenarios\given_duplicate_package_files\b.csproj" />
     <Content Include="Scenarios\given_duplicate_package_files\content\content.txt" />
@@ -132,13 +137,13 @@
     <Compile Include="given_a_library_with_json_dependencies.cs" />
     <Compile Include="given_an_empty_library_project.cs" />
     <Compile Include="given_a_library_with_project_reference.cs" />
-    <Content Include="Scenarios\given_a_library_with_content\contentFiles\cs\monoandroid\sample.cs" />
-    <Content Include="Scenarios\given_a_multi_platform_solution\android\quickstart\sample.cs" />
-    <Content Include="Scenarios\given_a_multi_platform_solution\android\quickstart\sample.fs" />
-    <Content Include="Scenarios\given_a_multi_platform_solution\android\quickstart\sample.vb" />
     <Compile Include="ReadLegacyJsonDependenciesTests.cs" />
     <Compile Include="ReadLegacyConfigDependenciesTests.cs" />
     <Content Include="Scenarios\given_a_packaging_project_with_reference_assembly\b\MyClass.cs" />
+    <Content Include="Scenarios\given_a_library_with_content\contentFiles\cs\monoandroid\content.cs" />
+    <Content Include="Scenarios\given_a_library_with_content\contentFiles\cs\monoandroid\none.cs" />
+    <None Include="Scenarios\given_a_multi_platform_solution\android\quickstart\sample.cs" />
+    <Content Include="Scenarios\given_a_library_with_content\quickstart\cs-any.txt" />
     <Compile Include="TargetsTests.cs" />
     <Compile Include="UtilitiesTests.cs" />
     <Compile Include="Utilities\FrameworkAssemblyReferenceComparer.cs" />