raise error when missing tpv and platform present (#3691)

Fixes: NuGet/Home#9441

src/NuGet.Core/NuGet.Commands/RestoreCommand/RestoreCommand.cs

@@ -181,6 +181,8 @@ public async Task<RestoreResult> ExecuteAsync(CancellationToken token)
                     _success = false;
                 }
 
+                _success &= HasValidPlatformVersions();
+
                 // evaluate packages.lock.json file
                 var packagesLockFilePath = PackagesLockFileUtilities.GetNuGetLockFilePath(_request.Project);
                 var isLockFileValid = false;
@@ -404,6 +406,26 @@ public async Task<RestoreResult> ExecuteAsync(CancellationToken token)
             }
         }
 
+        private bool HasValidPlatformVersions()
+        {
+            IEnumerable<NuGetFramework> badPlatforms = _request.Project.TargetFrameworks
+                .Select(frameworkInfo => frameworkInfo.FrameworkName)
+                .Where(framework => !string.IsNullOrEmpty(framework.Platform) && (framework.PlatformVersion == FrameworkConstants.EmptyVersion));
+
+            if (badPlatforms.Any())
+            {
+                _logger.Log(RestoreLogMessage.CreateError(
+                    NuGetLogCode.NU1012,
+                    string.Format(CultureInfo.CurrentCulture, Strings.Error_PlatformVersionNotPresent, string.Join(", ", badPlatforms))
+                ));
+                return false;
+            }
+            else
+            {
+                return true;
+            }
+        }
+
         private async Task<bool> AreCentralVersionRequirementsSatisfiedAsync()
         {
             // The dependencies should not have versions explicitelly defined if cpvm is enabled.
@@ -627,7 +649,7 @@ private bool ValidatePackagesSha512(PackagesLockFile lockFile, LockFile assetsFi
             {
                 if (!noOp)
                 {
-                    // Clean up to preserve the pre no-op behavior. This should not be used, but we want to be cautious. 
+                    // Clean up to preserve the pre no-op behavior. This should not be used, but we want to be cautious.
                     _request.LockFilePath = null;
                     _request.Project.RestoreMetadata.CacheFilePath = null;
                 }
@@ -727,7 +749,7 @@ private LockFile BuildAssetsFile(
         /// <summary>
         /// Check if the given graphs are valid and log errors/warnings.
         /// If fatal errors are encountered the rest of the errors/warnings
-        /// are not logged. This is to avoid flooding the log with long 
+        /// are not logged. This is to avoid flooding the log with long
         /// dependency chains for every package.
         /// </summary>
         private async Task<bool> ValidateRestoreGraphsAsync(IEnumerable<RestoreTargetGraph> graphs, ILogger logger)
@@ -1108,7 +1130,7 @@ private List<ExternalProjectReference> GetProjectReferences(RemoteWalkContext co
             else
             {
                 // External references were passed, but the top level project wasn't found.
-                // This is always due to an internal issue and typically caused by errors 
+                // This is always due to an internal issue and typically caused by errors
                 // building the project closure.
                 Debug.Fail("RestoreRequest.ExternalProjects contains references, but does not contain the top level references. Add the project we are restoring for.");
                 throw new InvalidOperationException($"Missing external reference metadata for {_request.Project.Name}");

src/NuGet.Core/NuGet.Commands/Strings.resx

@@ -1114,4 +1114,8 @@ For more information, visit https://docs.nuget.org/docs/reference/command-line-r
   <data name="Error_MultiplePackagePaths" xml:space="preserve">
     <value>{0} contains more than one path</value>
   </data>
+  <data name="Error_PlatformVersionNotPresent" xml:space="preserve">
+    <value>Platform version is not present for one or more target frameworks, even though they have specified a platform: {0}</value>
+    <comment>0 - comma-separated list of TFMs missing a platform version</comment>
+  </data>
 </root>

src/NuGet.Core/NuGet.Common/Errors/NuGetLogCode.cs

@@ -4,18 +4,18 @@
 namespace NuGet.Common
 {
     /// <summary>
-    /// This enum is used to quantify NuGet error and warning codes. 
+    /// This enum is used to quantify NuGet error and warning codes.
     /// Format - NUxyzw where NU is the prefix indicating NuGet and xyzw is a 4 digit code
     ///
     /// Numbers - xyzw
     ///     x - 'x' is the largest digit and should be used to quantify a set of errors.
     ///         For example 1yzw are set of restore related errors and no other code path should use the range 1000 to 1999 for errors or warnings.
-    ///         
+    ///
     ///     y - 'y' is the second largest digit and should be used for sub sections withing a broad category.
-    ///     
+    ///
     ///         For example 12zw cvould be http related errors.
     ///         Further 'y' = 0-4 should be used for errors and 'y' = 5-9 should be warnings.
-    ///         
+    ///
     ///     zw - 'zw' are the least two digit.
     ///         These could be used for different errors or warnings within the broad categories set by digits 'xy'.
     ///
@@ -31,9 +31,9 @@ namespace NuGet.Common
     /// 1200/1700     - Compat
     /// 1300/1800     - Feed
     /// 1400/1900     - Package
-    /// 
+    ///
     /// All new codes need a corresponding MarkDown file under https://github.com/NuGet/docs.microsoft.com-nuget/tree/master/docs/reference/errors-and-warnings.
-    /// 
+    ///
     /// </summary>
     public enum NuGetLogCode
     {
@@ -102,6 +102,11 @@ public enum NuGetLogCode
         /// </summary>
         NU1011 = 1011,
 
+        /// <summary>
+        /// Platform version not found.
+        /// </summary>
+        NU1012 = 1012,
+
         /// <summary>
         /// Unable to resolve package, generic message for unknown type constraints.
         /// </summary>

src/NuGet.Core/NuGet.Common/PublicAPI/net45/PublicAPI.Unshipped.txt

@@ -1,3 +1,4 @@
 NuGet.Common.NuGetLogCode.NU1010 = 1010 -> NuGet.Common.NuGetLogCode
 NuGet.Common.NuGetLogCode.NU1011 = 1011 -> NuGet.Common.NuGetLogCode
 NuGet.Common.NuGetLogCode.NU5501 = 5501 -> NuGet.Common.NuGetLogCode
+NuGet.Common.NuGetLogCode.NU1012 = 1012 -> NuGet.Common.NuGetLogCode

src/NuGet.Core/NuGet.Common/PublicAPI/net472/PublicAPI.Unshipped.txt

@@ -1,3 +1,4 @@
 NuGet.Common.NuGetLogCode.NU1010 = 1010 -> NuGet.Common.NuGetLogCode
 NuGet.Common.NuGetLogCode.NU1011 = 1011 -> NuGet.Common.NuGetLogCode
 NuGet.Common.NuGetLogCode.NU5501 = 5501 -> NuGet.Common.NuGetLogCode
+NuGet.Common.NuGetLogCode.NU1012 = 1012 -> NuGet.Common.NuGetLogCode

src/NuGet.Core/NuGet.Common/PublicAPI/netstandard2.0/PublicAPI.Unshipped.txt

@@ -1,3 +1,4 @@
 NuGet.Common.NuGetLogCode.NU1010 = 1010 -> NuGet.Common.NuGetLogCode
 NuGet.Common.NuGetLogCode.NU1011 = 1011 -> NuGet.Common.NuGetLogCode
 NuGet.Common.NuGetLogCode.NU5501 = 5501 -> NuGet.Common.NuGetLogCode
+NuGet.Common.NuGetLogCode.NU1012 = 1012 -> NuGet.Common.NuGetLogCode

src/NuGet.Core/NuGet.Packaging/PackageCreation/Authoring/PackageBuilder.cs

@@ -12,11 +12,14 @@
 using System.Reflection;
 using System.Reflection.Emit;
 using System.Xml.Linq;
+using NuGet.Client;
 using NuGet.Common;
+using NuGet.ContentModel;
 using NuGet.Frameworks;
 using NuGet.Packaging.Core;
 using NuGet.Packaging.PackageCreation.Resources;
 using NuGet.Packaging.Rules;
+using NuGet.RuntimeModel;
 using NuGet.Versioning;
 
 namespace NuGet.Packaging
@@ -226,7 +229,7 @@ public ISet<string> Tags
         }
 
         /// <summary>
-        /// Exposes the additional properties extracted by the metadata 
+        /// Exposes the additional properties extracted by the metadata
         /// extractor or received from the command line.
         /// </summary>
         public Dictionary<string, string> Properties
@@ -377,8 +380,10 @@ public void Save(Stream stream)
 
             ValidateDependencies(Version, DependencyGroups);
             ValidateReferenceAssemblies(Files, PackageAssemblyReferences);
+            ValidateFrameworkAssemblies(FrameworkReferences, FrameworkReferenceGroups);
             ValidateLicenseFile(Files, LicenseMetadata);
             ValidateIconFile(Files, Icon);
+            ValidateFileFrameworks(Files);
 
             using (var package = new ZipArchive(stream, ZipArchiveMode.Create, leaveOpen: true))
             {
@@ -543,6 +548,15 @@ private static bool HasXdtTransformFile(ICollection<IPackageFile> contentFiles)
         private static void ValidateDependencies(SemanticVersion version,
             IEnumerable<PackageDependencyGroup> dependencies)
         {
+            var frameworksMissingPlatformVersion = new HashSet<string>(dependencies
+                .Select(group => group.TargetFramework)
+                .Where(groupFramework => groupFramework.HasPlatform && groupFramework.PlatformVersion == FrameworkConstants.EmptyVersion)
+                .Select(framework => framework.GetShortFolderName()));
+            if (frameworksMissingPlatformVersion.Any())
+            {
+                throw new PackagingException(NuGetLogCode.NU1012, String.Format(CultureInfo.CurrentCulture, Strings.MissingTargetPlatformVersionsFromDependencyGroups, string.Join(", ", frameworksMissingPlatformVersion.OrderBy(str => str))));
+            }
+
             if (version == null)
             {
                 // We have independent validation for null-versions.
@@ -557,6 +571,15 @@ private static void ValidateDependencies(SemanticVersion version,
 
         public static void ValidateReferenceAssemblies(IEnumerable<IPackageFile> files, IEnumerable<PackageReferenceSet> packageAssemblyReferences)
         {
+            var frameworksMissingPlatformVersion = new HashSet<string>(packageAssemblyReferences
+                .Select(group => group.TargetFramework)
+                .Where(groupFramework => groupFramework.HasPlatform && groupFramework.PlatformVersion == FrameworkConstants.EmptyVersion)
+                .Select(framework => framework.GetShortFolderName()));
+            if (frameworksMissingPlatformVersion.Any())
+            {
+                throw new PackagingException(NuGetLogCode.NU1012, string.Format(CultureInfo.CurrentCulture, Strings.MissingTargetPlatformVersionsFromReferenceGroups, string.Join(", ", frameworksMissingPlatformVersion.OrderBy(str => str))));
+            }
+
             var libFiles = new HashSet<string>(from file in files
                                                where !String.IsNullOrEmpty(file.Path) && file.Path.StartsWith("lib", StringComparison.OrdinalIgnoreCase)
                                                select Path.GetFileName(file.Path), StringComparer.OrdinalIgnoreCase);
@@ -573,6 +596,30 @@ public static void ValidateReferenceAssemblies(IEnumerable<IPackageFile> files,
             }
         }
 
+        private static void ValidateFrameworkAssemblies(IEnumerable<FrameworkAssemblyReference> references, IEnumerable<FrameworkReferenceGroup> referenceGroups)
+        {
+            // Check standalone references
+            var frameworksMissingPlatformVersion = new HashSet<string>(references
+                .SelectMany(reference => reference.SupportedFrameworks)
+                .Where(framework => framework.HasPlatform && framework.PlatformVersion == FrameworkConstants.EmptyVersion)
+                .Select(framework => framework.GetShortFolderName())
+            );
+            if (frameworksMissingPlatformVersion.Any())
+            {
+                throw new PackagingException(NuGetLogCode.NU1012, string.Format(CultureInfo.CurrentCulture, Strings.MissingTargetPlatformVersionsFromFrameworkAssemblyReferences, string.Join(", ", frameworksMissingPlatformVersion.OrderBy(str => str))));
+            }
+
+            // Check reference groups too
+            frameworksMissingPlatformVersion = new HashSet<string>(referenceGroups
+                .Select(group => group.TargetFramework)
+                .Where(groupFramework => groupFramework.HasPlatform && groupFramework.PlatformVersion == FrameworkConstants.EmptyVersion)
+                .Select(framework => framework.GetShortFolderName()));
+            if (frameworksMissingPlatformVersion.Any())
+            {
+                throw new PackagingException(NuGetLogCode.NU1012, string.Format(CultureInfo.CurrentCulture, Strings.MissingTargetPlatformVersionsFromFrameworkAssemblyGroups, string.Join(", ", frameworksMissingPlatformVersion.OrderBy(str => str))));
+            }
+        }
+
         private void ValidateLicenseFile(IEnumerable<IPackageFile> files, LicenseMetadata licenseMetadata)
         {
             if (!PackageTypes.Contains(PackageType.SymbolsPackage) && licenseMetadata?.Type == LicenseType.File)
@@ -649,6 +696,63 @@ private void ValidateIconFile(IEnumerable<IPackageFile> files, string iconPath)
             }
         }
 
+        private static void ValidateFileFrameworks(IEnumerable<IPackageFile> files)
+        {
+            var set = new HashSet<string>(StringComparer.OrdinalIgnoreCase);
+            foreach (var file in files.Where(t => t.Path != null).Select(t => PathUtility.GetPathWithDirectorySeparator(t.Path)))
+            {
+                set.Add(file);
+            }
+
+            var managedCodeConventions = new ManagedCodeConventions(new RuntimeGraph());
+            var collection = new ContentItemCollection();
+            collection.Load(set.Select(path => path.Replace('\\', '/')).ToArray());
+
+            var patterns = managedCodeConventions.Patterns;
+
+            var frameworkPatterns = new List<PatternSet>()
+            {
+                patterns.RuntimeAssemblies,
+                patterns.CompileRefAssemblies,
+                patterns.CompileLibAssemblies,
+                patterns.NativeLibraries,
+                patterns.ResourceAssemblies,
+                patterns.MSBuildFiles,
+                patterns.ContentFiles,
+                patterns.ToolsAssemblies,
+                patterns.EmbedAssemblies,
+                patterns.MSBuildTransitiveFiles
+            };
+            var warnPaths = new HashSet<string>();
+
+            var frameworksMissingPlatformVersion = new HashSet<string>();
+            foreach (var pattern in frameworkPatterns)
+            {
+                IEnumerable<ContentItemGroup> targetedItemGroups = ContentExtractor.GetContentForPattern(collection, pattern);
+                foreach (ContentItemGroup group in targetedItemGroups)
+                {
+                    foreach (ContentItem item in group.Items)
+                    {
+                        var framework = (NuGetFramework)item.Properties["tfm"];
+                        if (framework == null)
+                        {
+                            continue;
+                        }
+
+                        if (framework.HasPlatform && framework.PlatformVersion == FrameworkConstants.EmptyVersion)
+                        {
+                            frameworksMissingPlatformVersion.Add(framework.GetShortFolderName());
+                        }
+                    }
+                }
+            }
+
+            if (frameworksMissingPlatformVersion.Any())
+            {
+                throw new PackagingException(NuGetLogCode.NU1012, string.Format(CultureInfo.CurrentCulture, Strings.MissingTargetPlatformVersionsFromIncludedFiles, string.Join(", ", frameworksMissingPlatformVersion.OrderBy(str => str))));
+            }
+        }
+
         private void ReadManifest(Stream stream, string basePath, Func<string, string> propertyProvider)
         {
             // Deserialize the document and extract the metadata

src/NuGet.Core/NuGet.Packaging/Strings.resx

@@ -839,4 +839,24 @@ Valid from:</comment>
     <value>The timestamp service responded with HTTP status code '{0}' ('{1}').</value>
     <comment>{0} is the httpResponse.StatusCode, {1} is the httpResponse.ReasonPhrase.</comment>
   </data>
-</root>
+  <data name="MissingTargetPlatformVersionsFromDependencyGroups" xml:space="preserve">
+    <value>Some dependency group TFMs are missing a platform version: {0}</value>
+    <comment>0 - comma-separated list of dependency group TFMs</comment>
+  </data>
+  <data name="MissingTargetPlatformVersionsFromFrameworkAssemblyGroups" xml:space="preserve">
+    <value>Some reference assembly group TFMs are missing a platform version: {0}</value>
+    <comment>0 - comma-separated list of reference assembly group TFMs</comment>
+  </data>
+  <data name="MissingTargetPlatformVersionsFromIncludedFiles" xml:space="preserve">
+    <value>Some included files are included under TFMs which are missing a platform version: {0}</value>
+    <comment>0 - comma-separated list of files with bad TFMs</comment>
+  </data>
+  <data name="MissingTargetPlatformVersionsFromReferenceGroups" xml:space="preserve">
+    <value>Some reference group TFMs are missing a platform version: {0}</value>
+    <comment>0 - comma-separated list of reference group TFMs</comment>
+  </data>
+  <data name="MissingTargetPlatformVersionsFromFrameworkAssemblyReferences" xml:space="preserve">
+    <value>Some framework assembly reference TFMs are missing a platform version: {0}</value>
+    <comment>0 - comma-separated list of reference group TFMs</comment>
+  </data>
+</root>