From e58e8a22836081ec4baffa5a9b4b093a329f3d88 Mon Sep 17 00:00:00 2001 From: Jesse Plamondon-Willard Date: Thu, 7 Apr 2022 01:38:02 -0400 Subject: [PATCH] enable nullable annotations for manifests (#837) --- src/SMAPI.Tests/Core/ModResolverTests.cs | 43 ++++---- src/SMAPI.Toolkit.CoreInterfaces/IManifest.cs | 8 +- .../IManifestContentPackFor.cs | 4 +- .../IManifestDependency.cs | 4 +- .../Framework/ModScanning/ModScanner.cs | 15 --- .../Serialization/Models/Manifest.cs | 102 ++++++++++++------ .../Models/ManifestContentPackFor.cs | 33 +++++- .../Models/ManifestDependency.cs | 44 ++++++-- src/SMAPI/Framework/ModLoading/ModResolver.cs | 2 +- 9 files changed, 164 insertions(+), 91 deletions(-) diff --git a/src/SMAPI.Tests/Core/ModResolverTests.cs b/src/SMAPI.Tests/Core/ModResolverTests.cs index 2ce1c74e..e1b56559 100644 --- a/src/SMAPI.Tests/Core/ModResolverTests.cs +++ b/src/SMAPI.Tests/Core/ModResolverTests.cs @@ -1,5 +1,3 @@ -#nullable disable - using System; using System.Collections.Generic; using System.IO; @@ -52,11 +50,11 @@ namespace SMAPI.Tests.Core // act IModMetadata[] mods = new ModResolver().ReadManifests(new ModToolkit(), rootFolder, new ModDatabase()).ToArray(); - IModMetadata mod = mods.FirstOrDefault(); + IModMetadata? mod = mods.FirstOrDefault(); // assert Assert.AreEqual(1, mods.Length, 0, $"Expected to find one manifest, found {mods.Length} instead."); - Assert.AreEqual(ModMetadataStatus.Failed, mod.Status, "The mod metadata was not marked failed."); + Assert.AreEqual(ModMetadataStatus.Failed, mod!.Status, "The mod metadata was not marked failed."); Assert.IsNotNull(mod.Error, "The mod metadata did not have an error message set."); } @@ -91,12 +89,12 @@ namespace SMAPI.Tests.Core // act IModMetadata[] mods = new ModResolver().ReadManifests(new ModToolkit(), rootFolder, new ModDatabase()).ToArray(); - IModMetadata mod = mods.FirstOrDefault(); + IModMetadata? mod = mods.FirstOrDefault(); // assert Assert.AreEqual(1, mods.Length, 0, "Expected to find one manifest."); Assert.IsNotNull(mod, "The loaded manifest shouldn't be null."); - Assert.AreEqual(null, mod.DataRecord, "The data record should be null since we didn't provide one."); + Assert.AreEqual(null, mod!.DataRecord, "The data record should be null since we didn't provide one."); Assert.AreEqual(modFolder, mod.DirectoryPath, "The directory path doesn't match."); Assert.AreEqual(null, mod.Error, "The error should be null since parsing should have succeeded."); Assert.AreEqual(ModMetadataStatus.Found, mod.Status, "The status doesn't match."); @@ -215,7 +213,7 @@ namespace SMAPI.Tests.Core // create DLL string modFolder = Path.Combine(this.GetTempFolderPath(), Guid.NewGuid().ToString("N")); Directory.CreateDirectory(modFolder); - File.WriteAllText(Path.Combine(modFolder, manifest.EntryDll), ""); + File.WriteAllText(Path.Combine(modFolder, manifest.EntryDll!), ""); // arrange Mock mock = new Mock(MockBehavior.Strict); @@ -480,21 +478,20 @@ namespace SMAPI.Tests.Core /// The value. /// The value. /// The value. - private Manifest GetManifest(string id = null, string name = null, string version = null, string entryDll = null, string contentPackForID = null, string minimumApiVersion = null, IManifestDependency[] dependencies = null) + private Manifest GetManifest(string? id = null, string? name = null, string? version = null, string? entryDll = null, string? contentPackForID = null, string? minimumApiVersion = null, IManifestDependency[]? dependencies = null) { - return new Manifest - { - UniqueID = id ?? $"{Sample.String()}.{Sample.String()}", - Name = name ?? id ?? Sample.String(), - Author = Sample.String(), - Description = Sample.String(), - Version = version != null ? new SemanticVersion(version) : new SemanticVersion(Sample.Int(), Sample.Int(), Sample.Int(), Sample.String()), - EntryDll = entryDll ?? $"{Sample.String()}.dll", - ContentPackFor = contentPackForID != null ? new ManifestContentPackFor { UniqueID = contentPackForID } : null, - MinimumApiVersion = minimumApiVersion != null ? new SemanticVersion(minimumApiVersion) : null, - Dependencies = dependencies ?? Array.Empty(), - UpdateKeys = Array.Empty() - }; + return new Manifest( + uniqueId: id ?? $"{Sample.String()}.{Sample.String()}", + name: name ?? id ?? Sample.String(), + author: Sample.String(), + description: Sample.String(), + version: version != null ? new SemanticVersion(version) : new SemanticVersion(Sample.Int(), Sample.Int(), Sample.Int(), Sample.String()), + entryDll: entryDll ?? $"{Sample.String()}.dll", + contentPackFor: contentPackForID != null ? new ManifestContentPackFor(contentPackForID, null) : null, + minimumApiVersion: minimumApiVersion != null ? new SemanticVersion(minimumApiVersion) : null, + dependencies: dependencies ?? Array.Empty(), + updateKeys: Array.Empty() + ); } /// Get a randomized basic manifest. @@ -510,7 +507,7 @@ namespace SMAPI.Tests.Core /// Whether the code being tested is allowed to change the mod status. private Mock GetMetadata(string uniqueID, string[] dependencies, bool allowStatusChange = false) { - IManifest manifest = this.GetManifest(id: uniqueID, version: "1.0", dependencies: dependencies?.Select(dependencyID => (IManifestDependency)new ManifestDependency(dependencyID, null)).ToArray()); + IManifest manifest = this.GetManifest(id: uniqueID, version: "1.0", dependencies: dependencies?.Select(dependencyID => (IManifestDependency)new ManifestDependency(dependencyID, null as ISemanticVersion)).ToArray()); return this.GetMetadata(manifest, allowStatusChange); } @@ -538,7 +535,7 @@ namespace SMAPI.Tests.Core /// Set up a mock mod metadata for . /// The mock mod metadata. /// The extra metadata about the mod from SMAPI's internal data (if any). - private void SetupMetadataForValidation(Mock mod, ModDataRecordVersionedFields modRecord = null) + private void SetupMetadataForValidation(Mock mod, ModDataRecordVersionedFields? modRecord = null) { mod.Setup(p => p.Status).Returns(ModMetadataStatus.Found); mod.Setup(p => p.DataRecord).Returns(() => null); diff --git a/src/SMAPI.Toolkit.CoreInterfaces/IManifest.cs b/src/SMAPI.Toolkit.CoreInterfaces/IManifest.cs index a9251446..ee6cc0b6 100644 --- a/src/SMAPI.Toolkit.CoreInterfaces/IManifest.cs +++ b/src/SMAPI.Toolkit.CoreInterfaces/IManifest.cs @@ -1,5 +1,3 @@ -#nullable disable - using System.Collections.Generic; namespace StardewModdingAPI @@ -23,16 +21,16 @@ namespace StardewModdingAPI ISemanticVersion Version { get; } /// The minimum SMAPI version required by this mod, if any. - ISemanticVersion MinimumApiVersion { get; } + ISemanticVersion? MinimumApiVersion { get; } /// The unique mod ID. string UniqueID { get; } /// The name of the DLL in the directory that has the Entry method. Mutually exclusive with . - string EntryDll { get; } + string? EntryDll { get; } /// The mod which will read this as a content pack. Mutually exclusive with . - IManifestContentPackFor ContentPackFor { get; } + IManifestContentPackFor? ContentPackFor { get; } /// The other mods that must be loaded before this mod. IManifestDependency[] Dependencies { get; } diff --git a/src/SMAPI.Toolkit.CoreInterfaces/IManifestContentPackFor.cs b/src/SMAPI.Toolkit.CoreInterfaces/IManifestContentPackFor.cs index d898b716..52ac8f1c 100644 --- a/src/SMAPI.Toolkit.CoreInterfaces/IManifestContentPackFor.cs +++ b/src/SMAPI.Toolkit.CoreInterfaces/IManifestContentPackFor.cs @@ -1,5 +1,3 @@ -#nullable disable - namespace StardewModdingAPI { /// Indicates which mod can read the content pack represented by the containing manifest. @@ -9,6 +7,6 @@ namespace StardewModdingAPI string UniqueID { get; } /// The minimum required version (if any). - ISemanticVersion MinimumVersion { get; } + ISemanticVersion? MinimumVersion { get; } } } diff --git a/src/SMAPI.Toolkit.CoreInterfaces/IManifestDependency.cs b/src/SMAPI.Toolkit.CoreInterfaces/IManifestDependency.cs index 49b7aed6..58425eb2 100644 --- a/src/SMAPI.Toolkit.CoreInterfaces/IManifestDependency.cs +++ b/src/SMAPI.Toolkit.CoreInterfaces/IManifestDependency.cs @@ -1,5 +1,3 @@ -#nullable disable - namespace StardewModdingAPI { /// A mod dependency listed in a mod manifest. @@ -12,7 +10,7 @@ namespace StardewModdingAPI string UniqueID { get; } /// The minimum required version (if any). - ISemanticVersion MinimumVersion { get; } + ISemanticVersion? MinimumVersion { get; } /// Whether the dependency must be installed to use the mod. bool IsRequired { get; } diff --git a/src/SMAPI.Toolkit/Framework/ModScanning/ModScanner.cs b/src/SMAPI.Toolkit/Framework/ModScanning/ModScanner.cs index 4deaf19b..621f1e28 100644 --- a/src/SMAPI.Toolkit/Framework/ModScanning/ModScanner.cs +++ b/src/SMAPI.Toolkit/Framework/ModScanning/ModScanner.cs @@ -171,14 +171,6 @@ namespace StardewModdingAPI.Toolkit.Framework.ModScanning } } - // normalize display fields - if (manifest != null) - { - manifest.Name = this.StripNewlines(manifest.Name); - manifest.Description = this.StripNewlines(manifest.Description); - manifest.Author = this.StripNewlines(manifest.Author); - } - // get mod type ModType type; { @@ -365,12 +357,5 @@ namespace StardewModdingAPI.Toolkit.Framework.ModScanning return hasVortexMarker; } - - /// Strip newlines from a string. - /// The input to strip. - private string StripNewlines(string input) - { - return input?.Replace("\r", "").Replace("\n", ""); - } } } diff --git a/src/SMAPI.Toolkit/Serialization/Models/Manifest.cs b/src/SMAPI.Toolkit/Serialization/Models/Manifest.cs index a5dbf604..01010602 100644 --- a/src/SMAPI.Toolkit/Serialization/Models/Manifest.cs +++ b/src/SMAPI.Toolkit/Serialization/Models/Manifest.cs @@ -1,8 +1,6 @@ -#nullable disable - using System; using System.Collections.Generic; -using System.Runtime.Serialization; +using System.Diagnostics.CodeAnalysis; using Newtonsoft.Json; using StardewModdingAPI.Toolkit.Serialization.Converters; @@ -15,48 +13,45 @@ namespace StardewModdingAPI.Toolkit.Serialization.Models ** Accessors *********/ /// The mod name. - public string Name { get; set; } + public string Name { get; } /// A brief description of the mod. - public string Description { get; set; } + public string Description { get; } /// The mod author's name. - public string Author { get; set; } + public string Author { get; } /// The mod version. - public ISemanticVersion Version { get; set; } + public ISemanticVersion Version { get; } /// The minimum SMAPI version required by this mod, if any. - public ISemanticVersion MinimumApiVersion { get; set; } + public ISemanticVersion? MinimumApiVersion { get; } /// The name of the DLL in the directory that has the Entry method. Mutually exclusive with . - public string EntryDll { get; set; } + public string? EntryDll { get; } /// The mod which will read this as a content pack. Mutually exclusive with . [JsonConverter(typeof(ManifestContentPackForConverter))] - public IManifestContentPackFor ContentPackFor { get; set; } + public IManifestContentPackFor? ContentPackFor { get; } /// The other mods that must be loaded before this mod. [JsonConverter(typeof(ManifestDependencyArrayConverter))] - public IManifestDependency[] Dependencies { get; set; } + public IManifestDependency[] Dependencies { get; } /// The namespaced mod IDs to query for updates (like Nexus:541). - public string[] UpdateKeys { get; set; } + public string[] UpdateKeys { get; private set; } /// The unique mod ID. - public string UniqueID { get; set; } + public string UniqueID { get; } /// Any manifest fields which didn't match a valid field. [JsonExtensionData] - public IDictionary ExtraFields { get; set; } + public IDictionary ExtraFields { get; set; } = new Dictionary(); /********* ** Public methods *********/ - /// Construct an instance. - public Manifest() { } - /// Construct an instance for a transitional content pack. /// The unique mod ID. /// The mod name. @@ -64,24 +59,71 @@ namespace StardewModdingAPI.Toolkit.Serialization.Models /// A brief description of the mod. /// The mod version. /// The modID which will read this as a content pack. - public Manifest(string uniqueID, string name, string author, string description, ISemanticVersion version, string contentPackFor = null) + public Manifest(string uniqueID, string name, string author, string description, ISemanticVersion version, string? contentPackFor = null) + : this( + uniqueId: uniqueID, + name: name, + author: author, + description: description, + version: version, + minimumApiVersion: null, + entryDll: null, + contentPackFor: contentPackFor != null + ? new ManifestContentPackFor(contentPackFor, null) + : null, + dependencies: null, + updateKeys: null + ) + { } + + /// Construct an instance for a transitional content pack. + /// The unique mod ID. + /// The mod name. + /// The mod author's name. + /// A brief description of the mod. + /// The mod version. + /// The minimum SMAPI version required by this mod, if any. + /// The name of the DLL in the directory that has the Entry method. Mutually exclusive with . + /// The modID which will read this as a content pack. + /// The other mods that must be loaded before this mod. + /// The namespaced mod IDs to query for updates (like Nexus:541). + [JsonConstructor] + public Manifest(string uniqueId, string name, string author, string description, ISemanticVersion version, ISemanticVersion? minimumApiVersion, string? entryDll, IManifestContentPackFor? contentPackFor, IManifestDependency[]? dependencies, string[]? updateKeys) { - this.Name = name; - this.Author = author; - this.Description = description; + this.UniqueID = this.NormalizeWhitespace(uniqueId); + this.Name = this.NormalizeWhitespace(name); + this.Author = this.NormalizeWhitespace(author); + this.Description = this.NormalizeWhitespace(description); this.Version = version; - this.UniqueID = uniqueID; - this.UpdateKeys = Array.Empty(); - this.ContentPackFor = new ManifestContentPackFor { UniqueID = contentPackFor }; + this.MinimumApiVersion = minimumApiVersion; + this.EntryDll = this.NormalizeWhitespace(entryDll); + this.ContentPackFor = contentPackFor; + this.Dependencies = dependencies ?? Array.Empty(); + this.UpdateKeys = updateKeys ?? Array.Empty(); } - /// Normalize the model after it's deserialized. - /// The deserialization context. - [OnDeserialized] - public void OnDeserialized(StreamingContext context) + /// Override the update keys loaded from the mod info. + /// The new update keys to set. + internal void OverrideUpdateKeys(params string[] updateKeys) { - this.Dependencies ??= Array.Empty(); - this.UpdateKeys ??= Array.Empty(); + this.UpdateKeys = updateKeys; + } + + + /********* + ** Private methods + *********/ + /// Normalize whitespace in a raw string. + /// The input to strip. +#if NET5_0_OR_GREATER + [return: NotNullIfNotNull("input")] +#endif + private string? NormalizeWhitespace(string? input) + { + return input + ?.Trim() + .Replace("\r", "") + .Replace("\n", ""); } } } diff --git a/src/SMAPI.Toolkit/Serialization/Models/ManifestContentPackFor.cs b/src/SMAPI.Toolkit/Serialization/Models/ManifestContentPackFor.cs index ea5f0e6c..f7dc8aa8 100644 --- a/src/SMAPI.Toolkit/Serialization/Models/ManifestContentPackFor.cs +++ b/src/SMAPI.Toolkit/Serialization/Models/ManifestContentPackFor.cs @@ -1,4 +1,4 @@ -#nullable disable +using System.Diagnostics.CodeAnalysis; namespace StardewModdingAPI.Toolkit.Serialization.Models { @@ -9,9 +9,36 @@ namespace StardewModdingAPI.Toolkit.Serialization.Models ** Accessors *********/ /// The unique ID of the mod which can read this content pack. - public string UniqueID { get; set; } + public string UniqueID { get; } /// The minimum required version (if any). - public ISemanticVersion MinimumVersion { get; set; } + public ISemanticVersion? MinimumVersion { get; } + + + /********* + ** Public methods + *********/ + /// Construct an instance. + /// The unique ID of the mod which can read this content pack. + /// The minimum required version (if any). + public ManifestContentPackFor(string uniqueId, ISemanticVersion? minimumVersion) + { + this.UniqueID = this.NormalizeWhitespace(uniqueId); + this.MinimumVersion = minimumVersion; + } + + + /********* + ** Private methods + *********/ + /// Normalize whitespace in a raw string. + /// The input to strip. +#if NET5_0_OR_GREATER + [return: NotNullIfNotNull("input")] +#endif + private string? NormalizeWhitespace(string? input) + { + return input?.Trim(); + } } } diff --git a/src/SMAPI.Toolkit/Serialization/Models/ManifestDependency.cs b/src/SMAPI.Toolkit/Serialization/Models/ManifestDependency.cs index f52dd5ee..e7acf71d 100644 --- a/src/SMAPI.Toolkit/Serialization/Models/ManifestDependency.cs +++ b/src/SMAPI.Toolkit/Serialization/Models/ManifestDependency.cs @@ -1,4 +1,5 @@ -#nullable disable +using System.Diagnostics.CodeAnalysis; +using Newtonsoft.Json; namespace StardewModdingAPI.Toolkit.Serialization.Models { @@ -9,13 +10,13 @@ namespace StardewModdingAPI.Toolkit.Serialization.Models ** Accessors *********/ /// The unique mod ID to require. - public string UniqueID { get; set; } + public string UniqueID { get; } /// The minimum required version (if any). - public ISemanticVersion MinimumVersion { get; set; } + public ISemanticVersion? MinimumVersion { get; } /// Whether the dependency must be installed to use the mod. - public bool IsRequired { get; set; } + public bool IsRequired { get; } /********* @@ -26,12 +27,39 @@ namespace StardewModdingAPI.Toolkit.Serialization.Models /// The minimum required version (if any). /// Whether the dependency must be installed to use the mod. public ManifestDependency(string uniqueID, string minimumVersion, bool required = true) + : this( + uniqueID: uniqueID, + minimumVersion: !string.IsNullOrWhiteSpace(minimumVersion) + ? new SemanticVersion(minimumVersion) + : null, + required: required + ) + { } + + /// Construct an instance. + /// The unique mod ID to require. + /// The minimum required version (if any). + /// Whether the dependency must be installed to use the mod. + [JsonConstructor] + public ManifestDependency(string uniqueID, ISemanticVersion? minimumVersion, bool required = true) { - this.UniqueID = uniqueID; - this.MinimumVersion = !string.IsNullOrWhiteSpace(minimumVersion) - ? new SemanticVersion(minimumVersion) - : null; + this.UniqueID = this.NormalizeWhitespace(uniqueID); + this.MinimumVersion = minimumVersion; this.IsRequired = required; } + + + /********* + ** Private methods + *********/ + /// Normalize whitespace in a raw string. + /// The input to strip. +#if NET5_0_OR_GREATER + [return: NotNullIfNotNull("input")] +#endif + private string? NormalizeWhitespace(string? input) + { + return input?.Trim(); + } } } diff --git a/src/SMAPI/Framework/ModLoading/ModResolver.cs b/src/SMAPI/Framework/ModLoading/ModResolver.cs index 51463048..2842c11a 100644 --- a/src/SMAPI/Framework/ModLoading/ModResolver.cs +++ b/src/SMAPI/Framework/ModLoading/ModResolver.cs @@ -35,7 +35,7 @@ namespace StardewModdingAPI.Framework.ModLoading // apply defaults if (manifest != null && dataRecord?.UpdateKey is not null) - manifest.UpdateKeys = new[] { dataRecord.UpdateKey }; + manifest.OverrideUpdateKeys(dataRecord.UpdateKey); // build metadata bool shouldIgnore = folder.Type == ModType.Ignored;