tweak new code

3 files changed, 51 insertions, 49 deletions

diff --git a/src/SMAPI.ModBuildConfig/DeployModTask.cs b/src/SMAPI.ModBuildConfig/DeployModTask.cs
index 1581b282..3508a6db 100644
--- a/src/SMAPI.ModBuildConfig/DeployModTask.cs
+++ b/src/SMAPI.ModBuildConfig/DeployModTask.cs
@@ -85,33 +85,30 @@ namespace StardewModdingAPI.ModBuildConfig
                 return true;
 
             // validate the manifest file
-            Manifest manifest;
+            IManifest manifest;
             {
-                // check if manifest file exists
-                FileInfo manifestFile = new(Path.Combine(this.ProjectDir, "manifest.json"));
-                if (!manifestFile.Exists)
-                {
-                    this.Log.LogError("[mod build package] The mod does not have a manifest.json file.");
-                    return false;
-                }
-
-                // check if the json is valid
                 try
                 {
-                    new JsonHelper().ReadJsonFileIfExists(manifestFile.FullName, out manifest);
+                    string manifestPath = Path.Combine(this.ProjectDir, "manifest.json");
+                    if (!new JsonHelper().ReadJsonFileIfExists(manifestPath, out Manifest rawManifest))
+                    {
+                        this.Log.LogError("[mod build package] The mod's manifest.json file doesn't exist.");
+                        return false;
+                    }
+                    manifest = rawManifest;
                 }
                 catch (JsonReaderException ex)
                 {
                     // log the inner exception, otherwise the message will be generic
                     Exception exToShow = ex.InnerException ?? ex;
-                    this.Log.LogError($"[mod build package] Failed to parse manifest.json: {exToShow.Message}");
+                    this.Log.LogError($"[mod build package] The mod's manifest.json file isn't valid JSON: {exToShow.Message}");
                     return false;
                 }
 
-                // validate the manifest's fields
-                if (!ManifestValidator.TryValidate(manifest, out string error))
+                // validate manifest fields
+                if (!ManifestValidator.TryValidateFields(manifest, out string error))
                 {
-                    this.Log.LogError($"[mod build package] The mod manifest is invalid: {error}");
+                    this.Log.LogError($"[mod build package] The mod's manifest.json file is invalid: {error}");
                     return false;
                 }
             }
diff --git a/src/SMAPI.Toolkit/Framework/ManifestValidator.cs b/src/SMAPI.Toolkit/Framework/ManifestValidator.cs
index 62cfd8e9..461dc325 100644
--- a/src/SMAPI.Toolkit/Framework/ManifestValidator.cs
+++ b/src/SMAPI.Toolkit/Framework/ManifestValidator.cs
@@ -1,4 +1,5 @@
 using System.Collections.Generic;
+using System.Diagnostics.CodeAnalysis;
 using System.IO;
 using System.Linq;
 using StardewModdingAPI.Toolkit.Utilities;
@@ -8,40 +9,47 @@ namespace StardewModdingAPI.Toolkit.Framework
     /// <summary>Validates manifest fields.</summary>
     public static class ManifestValidator
     {
-        /// <summary>Try to validate a manifest's fields. Fails if any invalid field is found.</summary>
+        /// <summary>Validate a manifest's fields.</summary>
         /// <param name="manifest">The manifest to validate.</param>
-        /// <param name="error">The error message to display to the user.</param>
-        /// <returns>Returns whether the manifest was validated successfully.</returns>
-        public static bool TryValidate(IManifest manifest, out string error)
+        /// <param name="error">The error message indicating why validation failed, if applicable.</param>
+        /// <returns>Returns whether all manifest fields validated successfully.</returns>
+        [SuppressMessage("ReSharper", "ConditionIsAlwaysTrueOrFalseAccordingToNullableAPIContract", Justification = "This is the method that ensures those annotations are respected.")]
+        public static bool TryValidateFields(IManifest manifest, out string error)
         {
-            // validate DLL / content pack fields
+            //
+            // Note: SMAPI assumes that it can grammatically append the returned sentence in the
+            // form "failed loading <mod> because its <error>". Any errors returned should be valid
+            // in that format, unless the SMAPI call is adjusted accordingly.
+            //
+
             bool hasDll = !string.IsNullOrWhiteSpace(manifest.EntryDll);
             bool isContentPack = manifest.ContentPackFor != null;
 
-            // validate field presence
-            if (!hasDll && !isContentPack)
-            {
-                error = $"manifest has no {nameof(IManifest.EntryDll)} or {nameof(IManifest.ContentPackFor)} field; must specify one.";
-                return false;
-            }
-            if (hasDll && isContentPack)
+            // validate use of EntryDll vs ContentPackFor fields
+            if (hasDll == isContentPack)
             {
-                error = $"manifest sets both {nameof(IManifest.EntryDll)} and {nameof(IManifest.ContentPackFor)}, which are mutually exclusive.";
+                error = hasDll
+                    ? $"manifest sets both {nameof(IManifest.EntryDll)} and {nameof(IManifest.ContentPackFor)}, which are mutually exclusive."
+                    : $"manifest has no {nameof(IManifest.EntryDll)} or {nameof(IManifest.ContentPackFor)} field; must specify one.";
                 return false;
             }
 
-            // validate DLL filename format
-            if (hasDll && manifest.EntryDll!.Intersect(Path.GetInvalidFileNameChars()).Any())
+            // validate EntryDll/ContentPackFor format
+            if (hasDll)
             {
-                error = $"manifest has invalid filename '{manifest.EntryDll}' for the EntryDLL field.";
-                return false;
+                if (manifest.EntryDll!.Intersect(Path.GetInvalidFileNameChars()).Any())
+                {
+                    error = $"manifest has invalid filename '{manifest.EntryDll}' for the {nameof(IManifest.EntryDll)} field.";
+                    return false;
+                }
             }
-
-            // validate content pack ID
-            else if (isContentPack && string.IsNullOrWhiteSpace(manifest.ContentPackFor!.UniqueID))
+            else
             {
-                error = $"manifest declares {nameof(IManifest.ContentPackFor)} without its required {nameof(IManifestContentPackFor.UniqueID)} field.";
-                return false;
+                if (string.IsNullOrWhiteSpace(manifest.ContentPackFor!.UniqueID))
+                {
+                    error = $"manifest declares {nameof(IManifest.ContentPackFor)} without its required {nameof(IManifestContentPackFor.UniqueID)} field.";
+                    return false;
+                }
             }
 
             // validate required fields
@@ -69,24 +77,21 @@ namespace StardewModdingAPI.Toolkit.Framework
                 return false;
             }
 
-            // validate dependencies
+            // validate dependency format
             foreach (IManifestDependency? dependency in manifest.Dependencies)
             {
-                // null dependency
                 if (dependency == null)
                 {
                     error = $"manifest has a null entry under {nameof(IManifest.Dependencies)}.";
                     return false;
                 }
 
-                // missing ID
                 if (string.IsNullOrWhiteSpace(dependency.UniqueID))
                 {
                     error = $"manifest has a {nameof(IManifest.Dependencies)} entry with no {nameof(IManifestDependency.UniqueID)} field.";
                     return false;
                 }
 
-                // invalid ID
                 if (!PathUtilities.IsSlug(dependency.UniqueID))
                 {
                     error = $"manifest has a {nameof(IManifest.Dependencies)} entry with an invalid {nameof(IManifestDependency.UniqueID)} field (IDs must only contain letters, numbers, underscores, periods, or hyphens).";
diff --git a/src/SMAPI/Framework/ModLoading/ModResolver.cs b/src/SMAPI/Framework/ModLoading/ModResolver.cs
index 0b4fe3e9..9db9db99 100644
--- a/src/SMAPI/Framework/ModLoading/ModResolver.cs
+++ b/src/SMAPI/Framework/ModLoading/ModResolver.cs
@@ -126,7 +126,14 @@ namespace StardewModdingAPI.Framework.ModLoading
                     continue;
                 }
 
-                // check for dll if it's supposed to have one
+                // validate manifest format
+                if (!ManifestValidator.TryValidateFields(mod.Manifest, out string manifestError))
+                {
+                    mod.SetStatus(ModMetadataStatus.Failed, ModFailReason.InvalidManifest, $"its {manifestError}");
+                    continue;
+                }
+
+                // check that DLL exists if applicable
                 if (!string.IsNullOrEmpty(mod.Manifest.EntryDll) && validateFilesExist)
                 {
                     IFileLookup pathLookup = getFileLookup(mod.DirectoryPath);
@@ -137,13 +144,6 @@ namespace StardewModdingAPI.Framework.ModLoading
                         continue;
                     }
                 }
-
-                // validate manifest
-                if (!ManifestValidator.TryValidate(mod.Manifest, out string manifestError))
-                {
-                    mod.SetStatus(ModMetadataStatus.Failed, ModFailReason.InvalidManifest, $"its {manifestError}");
-                    continue;
-                }
             }
 
             // validate IDs are unique