From a9e3458a3b93649f62919566c3ffacf27f16332c Mon Sep 17 00:00:00 2001 From: Jesse Plamondon-Willard Date: Thu, 29 Mar 2018 00:39:25 -0400 Subject: add success/error banner to log parser page --- docs/release-notes.md | 4 ++++ 1 file changed, 4 insertions(+) (limited to 'docs') diff --git a/docs/release-notes.md b/docs/release-notes.md index b3300800..a3e1abbd 100644 --- a/docs/release-notes.md +++ b/docs/release-notes.md @@ -15,6 +15,10 @@ * Added support for beta releases on the home page. --> +## vNext +* For the [log parser][]: + * Tweaked UI. + ## 2.5.4 * For players: * Fixed some textures not updated when a mod changes them. -- cgit From 30e89b3a3373d3a73f62e7297ac27db8de70246b Mon Sep 17 00:00:00 2001 From: Jesse Plamondon-Willard Date: Fri, 30 Mar 2018 22:51:34 -0400 Subject: fix mods not being loaded if an optional dependency is installed but skipped --- docs/release-notes.md | 5 ++++- src/SMAPI/Framework/ModLoading/ModResolver.cs | 1 + 2 files changed, 5 insertions(+), 1 deletion(-) (limited to 'docs') diff --git a/docs/release-notes.md b/docs/release-notes.md index a3e1abbd..3406e735 100644 --- a/docs/release-notes.md +++ b/docs/release-notes.md @@ -15,7 +15,10 @@ * Added support for beta releases on the home page. --> -## vNext +## 2.5.5 +* For players: + * Fixed mods not being loaded if an optional dependency is installed but skipped. + * For the [log parser][]: * Tweaked UI. diff --git a/src/SMAPI/Framework/ModLoading/ModResolver.cs b/src/SMAPI/Framework/ModLoading/ModResolver.cs index f878a1b9..a9896278 100644 --- a/src/SMAPI/Framework/ModLoading/ModResolver.cs +++ b/src/SMAPI/Framework/ModLoading/ModResolver.cs @@ -352,6 +352,7 @@ namespace StardewModdingAPI.Framework.ModLoading { // sorted successfully case ModDependencyStatus.Sorted: + case ModDependencyStatus.Failed when !dependency.IsRequired: // ignore failed optional dependency break; // failed, which means this mod can't be loaded either -- cgit From f52f7ca36f2ecf3d4478c5bc1e9cd95e5ff53929 Mon Sep 17 00:00:00 2001 From: Jesse Plamondon-Willard Date: Mon, 9 Apr 2018 19:32:00 -0400 Subject: add mod code analyzers to detect implicit net field conversion issues (#471) --- build/prepare-nuget-package.targets | 20 ++ docs/mod-build-config.md | 63 +++++- docs/screenshots/code-analyzer-example.png | Bin 0 -> 5696 bytes docs/technical-docs.md | 29 +++ .../Framework/DiagnosticResult.cs | 88 ++++++++ .../Framework/DiagnosticVerifier.Helper.cs | 171 ++++++++++++++ .../Framework/DiagnosticVerifier.cs | 248 +++++++++++++++++++++ .../SMAPI.ModBuildConfig.Analyzer.Tests.csproj | 18 ++ .../UnitTests.cs | 118 ++++++++++ .../ImplicitNetFieldCastAnalyzer.cs | 107 +++++++++ .../Properties/AssemblyInfo.cs | 4 + ...tardewModdingAPI.ModBuildConfig.Analyzer.csproj | 23 ++ .../tools/install.ps1 | 58 +++++ .../tools/uninstall.ps1 | 65 ++++++ .../StardewModdingAPI.ModBuildConfig.csproj | 1 + src/SMAPI.ModBuildConfig/package.nuspec | 24 +- src/SMAPI.Tests/StardewModdingAPI.Tests.csproj | 3 + src/SMAPI.sln | 33 +++ 18 files changed, 1045 insertions(+), 28 deletions(-) create mode 100644 build/prepare-nuget-package.targets create mode 100644 docs/screenshots/code-analyzer-example.png create mode 100644 src/SMAPI.ModBuildConfig.Analyzer.Tests/Framework/DiagnosticResult.cs create mode 100644 src/SMAPI.ModBuildConfig.Analyzer.Tests/Framework/DiagnosticVerifier.Helper.cs create mode 100644 src/SMAPI.ModBuildConfig.Analyzer.Tests/Framework/DiagnosticVerifier.cs create mode 100644 src/SMAPI.ModBuildConfig.Analyzer.Tests/SMAPI.ModBuildConfig.Analyzer.Tests.csproj create mode 100644 src/SMAPI.ModBuildConfig.Analyzer.Tests/UnitTests.cs create mode 100644 src/SMAPI.ModBuildConfig.Analyzer/ImplicitNetFieldCastAnalyzer.cs create mode 100644 src/SMAPI.ModBuildConfig.Analyzer/Properties/AssemblyInfo.cs create mode 100644 src/SMAPI.ModBuildConfig.Analyzer/StardewModdingAPI.ModBuildConfig.Analyzer.csproj create mode 100644 src/SMAPI.ModBuildConfig.Analyzer/tools/install.ps1 create mode 100644 src/SMAPI.ModBuildConfig.Analyzer/tools/uninstall.ps1 (limited to 'docs') diff --git a/build/prepare-nuget-package.targets b/build/prepare-nuget-package.targets new file mode 100644 index 00000000..5dbc5508 --- /dev/null +++ b/build/prepare-nuget-package.targets @@ -0,0 +1,20 @@ + + + + + $(SolutionDir)\..\bin\Pathoschild.Stardew.ModBuildConfig + + + + + + + + + + diff --git a/docs/mod-build-config.md b/docs/mod-build-config.md index 0d72e4d9..71a2518a 100644 --- a/docs/mod-build-config.md +++ b/docs/mod-build-config.md @@ -3,15 +3,16 @@ for SMAPI mods. The package... -* lets your code compile on any computer (Linux/Mac/Windows) without needing to change the assembly - references or game path. -* packages the mod into the game's `Mods` folder when you rebuild the code (configurable). -* configures Visual Studio so you can debug into the mod code when the game is running (_Windows - only_). +* detects your game install path; +* adds the assembly references you need (with automatic support for Linux/Mac/Windows); +* packages the mod into your `Mods` folder when you rebuild the code (configurable); +* configures Visual Studio to enable debugging into the code when the game is running (_Windows only_); +* adds C# analyzers to warn for Stardew Valley-specific issues. ## Contents * [Install](#install) * [Configure](#configure) +* [Code analysis warnings](#code-analysis-warnings) * [Troubleshoot](#troubleshoot) * [Release notes](#release-notes) @@ -121,7 +122,7 @@ The configuration will check your custom path first, then fall back to the defau still compile on a different computer). ### Unit test projects -**(upcoming in 2.0.3)** +**(upcoming in 2.1)** You can use the package in unit test projects too. Its optional unit test mode... @@ -134,15 +135,63 @@ To enable it, add this above the first `` in your `.csproj`: True ``` +## Code warnings +### Overview +The NuGet package adds code warnings in Visual Studio specific to Stardew Valley. For example: +![](screenshots/code-analyzer-example.png) + +You can hide the warnings... +* [for specific code](https://docs.microsoft.com/en-us/dotnet/csharp/language-reference/preprocessor-directives/preprocessor-pragma-warning); +* for a method using this attribute: + ```cs + [System.Diagnostics.CodeAnalysis.SuppressMessage("SMAPI.CommonErrors", "SMAPI001")] // implicit net field conversion + ``` +* for an entire project: + 1. Expand the _References_ node for the project in Visual Studio. + 2. Right-click on _Analyzers_ and choose _Open Active Rule Set_. + 4. Expand _StardewModdingAPI.ModBuildConfig.Analyzer_ and uncheck the warnings you want to hide. + +See below for help with each specific warning. + +### SMAPI001 +**Implicit net field conversion:** +> This implicitly converts '{{expression}}' from {{net type}} to {{other type}}, but +> {{net type}} has unintuitive implicit conversion rules. Consider comparing against the actual +> value instead to avoid bugs. + +Stardew Valley uses a set of net fields (like `NetBool` or `NetInt`) to handle multiplayer sync. +These types can implicitly convert to their equivalent normal values (like `bool x = new NetBool()`), +but their conversion rules can be unintuitive and error-prone. For example, +`item?.category == null && item?.category != null` can both be true at once, and +`building.indoors != null` will be true for a null value in some cases. + +Suggested fix: +* For a reference type (i.e. one that can contain `null`), you can use the `.Value` property: + ```c# + if (building.indoors.Value == null) + ``` + Or convert the value before comparison: + ```c# + GameLocation indoors = building.indoors; + if(indoors == null) + // ... + ``` +* For a value type (i.e. one that can't contain `null`), make sure to check for null first if + applicable: + ```cs + if (item != null && item.category == 0) + ``` + ## Troubleshoot ### "Failed to find the game install path" That error means the package couldn't find your game. You can specify the game path yourself; see _[Game path](#game-path)_ above. ## Release notes -### 2.0.3 alpha +### 2.1 alpha * Added support for Stardew Valley 1.3. * Added support for unit test projects. +* Added C# analyzers to warn about implicit conversions of Netcode fields in Stardew Valley 1.3. ### 2.0.2 * Fixed compatibility issue on Linux. diff --git a/docs/screenshots/code-analyzer-example.png b/docs/screenshots/code-analyzer-example.png new file mode 100644 index 00000000..2723b164 Binary files /dev/null and b/docs/screenshots/code-analyzer-example.png differ diff --git a/docs/technical-docs.md b/docs/technical-docs.md index 37ec7f69..9e1a49e7 100644 --- a/docs/technical-docs.md +++ b/docs/technical-docs.md @@ -20,6 +20,7 @@ mods, this section isn't relevant to you; see the main README to use or create m * [Development](#development-2) * [Local development](#local-development) * [Deploying to Amazon Beanstalk](#deploying-to-amazon-beanstalk) +* [Mod build config package](#mod-build-config-package) # SMAPI ## Development @@ -222,3 +223,31 @@ property name | description `LogParser:SectionUrl` | The root URL of the log page, like `https://log.smapi.io/`. `ModUpdateCheck:GitHubPassword` | The password with which to authenticate to GitHub when fetching release info. `ModUpdateCheck:GitHubUsername` | The username with which to authenticate to GitHub when fetching release info. + +## Mod build config package +### Overview +The mod build config package is a NuGet package that mods reference to automatically set up +references, configure the build, and add analyzers specific to Stardew Valley mods. + +This involves three projects: + +project | purpose +------------------------------------------------- | ---------------- +`StardewModdingAPI.ModBuildConfig` | Configures the build (references, deploying the mod files, setting up debugging, etc). +`StardewModdingAPI.ModBuildConfig.Analyzer` | Adds C# analyzers which show code warnings in Visual Studio. +`StardewModdingAPI.ModBuildConfig.Analyzer.Tests` | Unit tests for the C# analyzers. + +When the projects are built, the relevant files are copied into `bin/Pathoschild.Stardew.ModBuildConfig`. + +### Preparing a build +To prepare a build of the NuGet package: +1. Install the [NuGet CLI](https://docs.microsoft.com/en-us/nuget/install-nuget-client-tools#nugetexe-cli). +1. Change the version and release notes in `package.nuspec`. +2. Rebuild the solution in _Release_ mode. +3. Open a terminal in the `bin/Pathoschild.Stardew.ModBuildConfig` package and run this command: + ```bash + nuget.exe pack + ``` + +That will create a `Pathoschild.Stardew.ModBuildConfig-.nupkg` file in the same directory +which can be uploaded to NuGet or referenced directly. diff --git a/src/SMAPI.ModBuildConfig.Analyzer.Tests/Framework/DiagnosticResult.cs b/src/SMAPI.ModBuildConfig.Analyzer.Tests/Framework/DiagnosticResult.cs new file mode 100644 index 00000000..896c2cb8 --- /dev/null +++ b/src/SMAPI.ModBuildConfig.Analyzer.Tests/Framework/DiagnosticResult.cs @@ -0,0 +1,88 @@ +// +using Microsoft.CodeAnalysis; +using System; + +namespace SMAPI.ModBuildConfig.Analyzer.Tests.Framework +{ + /// + /// Location where the diagnostic appears, as determined by path, line number, and column number. + /// + public struct DiagnosticResultLocation + { + public DiagnosticResultLocation(string path, int line, int column) + { + if (line < -1) + { + throw new ArgumentOutOfRangeException(nameof(line), "line must be >= -1"); + } + + if (column < -1) + { + throw new ArgumentOutOfRangeException(nameof(column), "column must be >= -1"); + } + + this.Path = path; + this.Line = line; + this.Column = column; + } + + public string Path { get; } + public int Line { get; } + public int Column { get; } + } + + /// + /// Struct that stores information about a Diagnostic appearing in a source + /// + public struct DiagnosticResult + { + private DiagnosticResultLocation[] locations; + + public DiagnosticResultLocation[] Locations + { + get + { + if (this.locations == null) + { + this.locations = new DiagnosticResultLocation[] { }; + } + return this.locations; + } + + set + { + this.locations = value; + } + } + + public DiagnosticSeverity Severity { get; set; } + + public string Id { get; set; } + + public string Message { get; set; } + + public string Path + { + get + { + return this.Locations.Length > 0 ? this.Locations[0].Path : ""; + } + } + + public int Line + { + get + { + return this.Locations.Length > 0 ? this.Locations[0].Line : -1; + } + } + + public int Column + { + get + { + return this.Locations.Length > 0 ? this.Locations[0].Column : -1; + } + } + } +} diff --git a/src/SMAPI.ModBuildConfig.Analyzer.Tests/Framework/DiagnosticVerifier.Helper.cs b/src/SMAPI.ModBuildConfig.Analyzer.Tests/Framework/DiagnosticVerifier.Helper.cs new file mode 100644 index 00000000..d33455fc --- /dev/null +++ b/src/SMAPI.ModBuildConfig.Analyzer.Tests/Framework/DiagnosticVerifier.Helper.cs @@ -0,0 +1,171 @@ +// +using Microsoft.CodeAnalysis; +using Microsoft.CodeAnalysis.CSharp; +using Microsoft.CodeAnalysis.Diagnostics; +using Microsoft.CodeAnalysis.Text; +using System; +using System.Collections.Generic; +using System.Collections.Immutable; +using System.Linq; + +namespace SMAPI.ModBuildConfig.Analyzer.Tests.Framework +{ + /// + /// Class for turning strings into documents and getting the diagnostics on them + /// All methods are static + /// + public abstract partial class DiagnosticVerifier + { + private static readonly MetadataReference CorlibReference = MetadataReference.CreateFromFile(typeof(object).Assembly.Location); + private static readonly MetadataReference SystemCoreReference = MetadataReference.CreateFromFile(typeof(Enumerable).Assembly.Location); + private static readonly MetadataReference CSharpSymbolsReference = MetadataReference.CreateFromFile(typeof(CSharpCompilation).Assembly.Location); + private static readonly MetadataReference CodeAnalysisReference = MetadataReference.CreateFromFile(typeof(Compilation).Assembly.Location); + + internal static string DefaultFilePathPrefix = "Test"; + internal static string CSharpDefaultFileExt = "cs"; + internal static string VisualBasicDefaultExt = "vb"; + internal static string TestProjectName = "TestProject"; + + #region Get Diagnostics + + /// + /// Given classes in the form of strings, their language, and an IDiagnosticAnalyzer to apply to it, return the diagnostics found in the string after converting it to a document. + /// + /// Classes in the form of strings + /// The language the source classes are in + /// The analyzer to be run on the sources + /// An IEnumerable of Diagnostics that surfaced in the source code, sorted by Location + private static Diagnostic[] GetSortedDiagnostics(string[] sources, string language, DiagnosticAnalyzer analyzer) + { + return GetSortedDiagnosticsFromDocuments(analyzer, GetDocuments(sources, language)); + } + + /// + /// Given an analyzer and a document to apply it to, run the analyzer and gather an array of diagnostics found in it. + /// The returned diagnostics are then ordered by location in the source document. + /// + /// The analyzer to run on the documents + /// The Documents that the analyzer will be run on + /// An IEnumerable of Diagnostics that surfaced in the source code, sorted by Location + protected static Diagnostic[] GetSortedDiagnosticsFromDocuments(DiagnosticAnalyzer analyzer, Document[] documents) + { + var projects = new HashSet(); + foreach (var document in documents) + { + projects.Add(document.Project); + } + + var diagnostics = new List(); + foreach (var project in projects) + { + var compilationWithAnalyzers = project.GetCompilationAsync().Result.WithAnalyzers(ImmutableArray.Create(analyzer)); + var diags = compilationWithAnalyzers.GetAnalyzerDiagnosticsAsync().Result; + foreach (var diag in diags) + { + if (diag.Location == Location.None || diag.Location.IsInMetadata) + { + diagnostics.Add(diag); + } + else + { + for (int i = 0; i < documents.Length; i++) + { + var document = documents[i]; + var tree = document.GetSyntaxTreeAsync().Result; + if (tree == diag.Location.SourceTree) + { + diagnostics.Add(diag); + } + } + } + } + } + + var results = SortDiagnostics(diagnostics); + diagnostics.Clear(); + return results; + } + + /// + /// Sort diagnostics by location in source document + /// + /// The list of Diagnostics to be sorted + /// An IEnumerable containing the Diagnostics in order of Location + private static Diagnostic[] SortDiagnostics(IEnumerable diagnostics) + { + return diagnostics.OrderBy(d => d.Location.SourceSpan.Start).ToArray(); + } + + #endregion + + #region Set up compilation and documents + /// + /// Given an array of strings as sources and a language, turn them into a project and return the documents and spans of it. + /// + /// Classes in the form of strings + /// The language the source code is in + /// A Tuple containing the Documents produced from the sources and their TextSpans if relevant + private static Document[] GetDocuments(string[] sources, string language) + { + if (language != LanguageNames.CSharp && language != LanguageNames.VisualBasic) + { + throw new ArgumentException("Unsupported Language"); + } + + var project = CreateProject(sources, language); + var documents = project.Documents.ToArray(); + + if (sources.Length != documents.Length) + { + throw new InvalidOperationException("Amount of sources did not match amount of Documents created"); + } + + return documents; + } + + /// + /// Create a Document from a string through creating a project that contains it. + /// + /// Classes in the form of a string + /// The language the source code is in + /// A Document created from the source string + protected static Document CreateDocument(string source, string language = LanguageNames.CSharp) + { + return CreateProject(new[] { source }, language).Documents.First(); + } + + /// + /// Create a project using the inputted strings as sources. + /// + /// Classes in the form of strings + /// The language the source code is in + /// A Project created out of the Documents created from the source strings + private static Project CreateProject(string[] sources, string language = LanguageNames.CSharp) + { + string fileNamePrefix = DefaultFilePathPrefix; + string fileExt = language == LanguageNames.CSharp ? CSharpDefaultFileExt : VisualBasicDefaultExt; + + var projectId = ProjectId.CreateNewId(debugName: TestProjectName); + + var solution = new AdhocWorkspace() + .CurrentSolution + .AddProject(projectId, TestProjectName, TestProjectName, language) + .AddMetadataReference(projectId, CorlibReference) + .AddMetadataReference(projectId, SystemCoreReference) + .AddMetadataReference(projectId, CSharpSymbolsReference) + .AddMetadataReference(projectId, CodeAnalysisReference); + + int count = 0; + foreach (var source in sources) + { + var newFileName = fileNamePrefix + count + "." + fileExt; + var documentId = DocumentId.CreateNewId(projectId, debugName: newFileName); + solution = solution.AddDocument(documentId, newFileName, SourceText.From(source)); + count++; + } + return solution.GetProject(projectId); + } + #endregion + } +} + diff --git a/src/SMAPI.ModBuildConfig.Analyzer.Tests/Framework/DiagnosticVerifier.cs b/src/SMAPI.ModBuildConfig.Analyzer.Tests/Framework/DiagnosticVerifier.cs new file mode 100644 index 00000000..edaaabd4 --- /dev/null +++ b/src/SMAPI.ModBuildConfig.Analyzer.Tests/Framework/DiagnosticVerifier.cs @@ -0,0 +1,248 @@ +// +using System.Collections.Generic; +using System.Linq; +using System.Text; +using Microsoft.CodeAnalysis; +using Microsoft.CodeAnalysis.Diagnostics; +using NUnit.Framework; + +namespace SMAPI.ModBuildConfig.Analyzer.Tests.Framework +{ + /// + /// Superclass of all Unit Tests for DiagnosticAnalyzers + /// + public abstract partial class DiagnosticVerifier + { + #region To be implemented by Test classes + /// + /// Get the CSharp analyzer being tested - to be implemented in non-abstract class + /// + protected virtual DiagnosticAnalyzer GetCSharpDiagnosticAnalyzer() + { + return null; + } + + /// + /// Get the Visual Basic analyzer being tested (C#) - to be implemented in non-abstract class + /// + protected virtual DiagnosticAnalyzer GetBasicDiagnosticAnalyzer() + { + return null; + } + #endregion + + #region Verifier wrappers + + /// + /// Called to test a C# DiagnosticAnalyzer when applied on the single inputted string as a source + /// Note: input a DiagnosticResult for each Diagnostic expected + /// + /// A class in the form of a string to run the analyzer on + /// DiagnosticResults that should appear after the analyzer is run on the source + protected void VerifyCSharpDiagnostic(string source, params DiagnosticResult[] expected) + { + VerifyDiagnostics(new[] { source }, LanguageNames.CSharp, GetCSharpDiagnosticAnalyzer(), expected); + } + + /// + /// Called to test a C# DiagnosticAnalyzer when applied on the inputted strings as a source + /// Note: input a DiagnosticResult for each Diagnostic expected + /// + /// An array of strings to create source documents from to run the analyzers on + /// DiagnosticResults that should appear after the analyzer is run on the sources + protected void VerifyCSharpDiagnostic(string[] sources, params DiagnosticResult[] expected) + { + VerifyDiagnostics(sources, LanguageNames.CSharp, GetCSharpDiagnosticAnalyzer(), expected); + } + + /// + /// General method that gets a collection of actual diagnostics found in the source after the analyzer is run, + /// then verifies each of them. + /// + /// An array of strings to create source documents from to run the analyzers on + /// The language of the classes represented by the source strings + /// The analyzer to be run on the source code + /// DiagnosticResults that should appear after the analyzer is run on the sources + private void VerifyDiagnostics(string[] sources, string language, DiagnosticAnalyzer analyzer, params DiagnosticResult[] expected) + { + var diagnostics = GetSortedDiagnostics(sources, language, analyzer); + VerifyDiagnosticResults(diagnostics, analyzer, expected); + } + + #endregion + + #region Actual comparisons and verifications + /// + /// Checks each of the actual Diagnostics found and compares them with the corresponding DiagnosticResult in the array of expected results. + /// Diagnostics are considered equal only if the DiagnosticResultLocation, Id, Severity, and Message of the DiagnosticResult match the actual diagnostic. + /// + /// The Diagnostics found by the compiler after running the analyzer on the source code + /// The analyzer that was being run on the sources + /// Diagnostic Results that should have appeared in the code + private static void VerifyDiagnosticResults(IEnumerable actualResults, DiagnosticAnalyzer analyzer, params DiagnosticResult[] expectedResults) + { + int expectedCount = expectedResults.Count(); + int actualCount = actualResults.Count(); + + if (expectedCount != actualCount) + { + string diagnosticsOutput = actualResults.Any() ? FormatDiagnostics(analyzer, actualResults.ToArray()) : " NONE."; + + Assert.IsTrue(false, + string.Format("Mismatch between number of diagnostics returned, expected \"{0}\" actual \"{1}\"\r\n\r\nDiagnostics:\r\n{2}\r\n", expectedCount, actualCount, diagnosticsOutput)); + } + + for (int i = 0; i < expectedResults.Length; i++) + { + var actual = actualResults.ElementAt(i); + var expected = expectedResults[i]; + + if (expected.Line == -1 && expected.Column == -1) + { + if (actual.Location != Location.None) + { + Assert.IsTrue(false, + string.Format("Expected:\nA project diagnostic with No location\nActual:\n{0}", + FormatDiagnostics(analyzer, actual))); + } + } + else + { + VerifyDiagnosticLocation(analyzer, actual, actual.Location, expected.Locations.First()); + var additionalLocations = actual.AdditionalLocations.ToArray(); + + if (additionalLocations.Length != expected.Locations.Length - 1) + { + Assert.IsTrue(false, + string.Format("Expected {0} additional locations but got {1} for Diagnostic:\r\n {2}\r\n", + expected.Locations.Length - 1, additionalLocations.Length, + FormatDiagnostics(analyzer, actual))); + } + + for (int j = 0; j < additionalLocations.Length; ++j) + { + VerifyDiagnosticLocation(analyzer, actual, additionalLocations[j], expected.Locations[j + 1]); + } + } + + if (actual.Id != expected.Id) + { + Assert.IsTrue(false, + string.Format("Expected diagnostic id to be \"{0}\" was \"{1}\"\r\n\r\nDiagnostic:\r\n {2}\r\n", + expected.Id, actual.Id, FormatDiagnostics(analyzer, actual))); + } + + if (actual.Severity != expected.Severity) + { + Assert.IsTrue(false, + string.Format("Expected diagnostic severity to be \"{0}\" was \"{1}\"\r\n\r\nDiagnostic:\r\n {2}\r\n", + expected.Severity, actual.Severity, FormatDiagnostics(analyzer, actual))); + } + + if (actual.GetMessage() != expected.Message) + { + Assert.IsTrue(false, + string.Format("Expected diagnostic message to be \"{0}\" was \"{1}\"\r\n\r\nDiagnostic:\r\n {2}\r\n", + expected.Message, actual.GetMessage(), FormatDiagnostics(analyzer, actual))); + } + } + } + + /// + /// Helper method to VerifyDiagnosticResult that checks the location of a diagnostic and compares it with the location in the expected DiagnosticResult. + /// + /// The analyzer that was being run on the sources + /// The diagnostic that was found in the code + /// The Location of the Diagnostic found in the code + /// The DiagnosticResultLocation that should have been found + private static void VerifyDiagnosticLocation(DiagnosticAnalyzer analyzer, Diagnostic diagnostic, Location actual, DiagnosticResultLocation expected) + { + var actualSpan = actual.GetLineSpan(); + + Assert.IsTrue(actualSpan.Path == expected.Path || (actualSpan.Path != null && actualSpan.Path.Contains("Test0.") && expected.Path.Contains("Test.")), + string.Format("Expected diagnostic to be in file \"{0}\" was actually in file \"{1}\"\r\n\r\nDiagnostic:\r\n {2}\r\n", + expected.Path, actualSpan.Path, FormatDiagnostics(analyzer, diagnostic))); + + var actualLinePosition = actualSpan.StartLinePosition; + + // Only check line position if there is an actual line in the real diagnostic + if (actualLinePosition.Line > 0) + { + if (actualLinePosition.Line + 1 != expected.Line) + { + Assert.IsTrue(false, + string.Format("Expected diagnostic to be on line \"{0}\" was actually on line \"{1}\"\r\n\r\nDiagnostic:\r\n {2}\r\n", + expected.Line, actualLinePosition.Line + 1, FormatDiagnostics(analyzer, diagnostic))); + } + } + + // Only check column position if there is an actual column position in the real diagnostic + if (actualLinePosition.Character > 0) + { + if (actualLinePosition.Character + 1 != expected.Column) + { + Assert.IsTrue(false, + string.Format("Expected diagnostic to start at column \"{0}\" was actually at column \"{1}\"\r\n\r\nDiagnostic:\r\n {2}\r\n", + expected.Column, actualLinePosition.Character + 1, FormatDiagnostics(analyzer, diagnostic))); + } + } + } + #endregion + + #region Formatting Diagnostics + /// + /// Helper method to format a Diagnostic into an easily readable string + /// + /// The analyzer that this verifier tests + /// The Diagnostics to be formatted + /// The Diagnostics formatted as a string + private static string FormatDiagnostics(DiagnosticAnalyzer analyzer, params Diagnostic[] diagnostics) + { + var builder = new StringBuilder(); + for (int i = 0; i < diagnostics.Length; ++i) + { + builder.AppendLine("// " + diagnostics[i].ToString()); + + var analyzerType = analyzer.GetType(); + var rules = analyzer.SupportedDiagnostics; + + foreach (var rule in rules) + { + if (rule != null && rule.Id == diagnostics[i].Id) + { + var location = diagnostics[i].Location; + if (location == Location.None) + { + builder.AppendFormat("GetGlobalResult({0}.{1})", analyzerType.Name, rule.Id); + } + else + { + Assert.IsTrue(location.IsInSource, + $"Test base does not currently handle diagnostics in metadata locations. Diagnostic in metadata: {diagnostics[i]}\r\n"); + + string resultMethodName = diagnostics[i].Location.SourceTree.FilePath.EndsWith(".cs") ? "GetCSharpResultAt" : "GetBasicResultAt"; + var linePosition = diagnostics[i].Location.GetLineSpan().StartLinePosition; + + builder.AppendFormat("{0}({1}, {2}, {3}.{4})", + resultMethodName, + linePosition.Line + 1, + linePosition.Character + 1, + analyzerType.Name, + rule.Id); + } + + if (i != diagnostics.Length - 1) + { + builder.Append(','); + } + + builder.AppendLine(); + break; + } + } + } + return builder.ToString(); + } + #endregion + } +} diff --git a/src/SMAPI.ModBuildConfig.Analyzer.Tests/SMAPI.ModBuildConfig.Analyzer.Tests.csproj b/src/SMAPI.ModBuildConfig.Analyzer.Tests/SMAPI.ModBuildConfig.Analyzer.Tests.csproj new file mode 100644 index 00000000..8d3815c3 --- /dev/null +++ b/src/SMAPI.ModBuildConfig.Analyzer.Tests/SMAPI.ModBuildConfig.Analyzer.Tests.csproj @@ -0,0 +1,18 @@ + + + + netcoreapp2.0 + + + + + + + + + + + + + + diff --git a/src/SMAPI.ModBuildConfig.Analyzer.Tests/UnitTests.cs b/src/SMAPI.ModBuildConfig.Analyzer.Tests/UnitTests.cs new file mode 100644 index 00000000..d5e4ef52 --- /dev/null +++ b/src/SMAPI.ModBuildConfig.Analyzer.Tests/UnitTests.cs @@ -0,0 +1,118 @@ +using Microsoft.CodeAnalysis; +using Microsoft.CodeAnalysis.Diagnostics; +using NUnit.Framework; +using SMAPI.ModBuildConfig.Analyzer.Tests.Framework; +using StardewModdingAPI.ModBuildConfig.Analyzer; + +namespace SMAPI.ModBuildConfig.Analyzer.Tests +{ + /// Unit tests for the C# analyzers. + [TestFixture] + public class UnitTests : DiagnosticVerifier + { + /********* + ** Properties + *********/ + /// Sample C# code which contains a simplified representation of Stardew Valley's Netcode types, and sample mod code with a {{test-code}} placeholder for the code being tested. + const string SampleProgram = @" + using System; + using Netcode; + + namespace Netcode + { + public class NetInt : NetFieldBase { } + public class NetRef : NetFieldBase { } + public class NetFieldBase where TSelf : NetFieldBase + { + public T Value { get; set; } + public static implicit operator T(NetFieldBase field) => field.Value; + } + } + + namespace SampleMod + { + class Item + { + public NetInt category { get; } = new NetInt { Value = 42 }; + } + + class ModEntry + { + public void Entry() + { + NetInt intField = new NetInt { Value = 42 }; + NetRef refField = new NetRef { Value = null }; + Item item = null; + + // this line should raise diagnostics + {{test-code}} // line 32 + + // these lines should not + if ((int)field != 42); + } + } + } + "; + + /// The line number where the unit tested code is injected into . + private const int SampleCodeLine = 32; + + /// The column number where the unit tested code is injected into . + private const int SampleCodeColumn = 25; + + + /********* + ** Unit tests + *********/ + /// Test that no diagnostics are raised for an empty code block. + [TestCase] + public void EmptyCode_HasNoDiagnostics() + { + // arrange + string test = @""; + + // assert + this.VerifyCSharpDiagnostic(test); + } + + /// Test that the expected diagnostic message is raised. + /// The code line to test. + /// The column within the code line where the diagnostic message should be reported. + /// The expression which should be reported. + /// The source type name which should be reported. + /// The target type name which should be reported. + [TestCase("if (intField < 42);", 4, "intField", "NetInt", "Int32")] + [TestCase("if (intField <= 42);", 4, "intField", "NetInt", "Int32")] + [TestCase("if (intField > 42);", 4, "intField", "NetInt", "Int32")] + [TestCase("if (intField >= 42);", 4, "intField", "NetInt", "Int32")] + [TestCase("if (intField == 42);", 4, "intField", "NetInt", "Int32")] + [TestCase("if (intField != 42);", 4, "intField", "NetInt", "Int32")] + [TestCase("if (refField != null);", 4, "refField", "NetRef", "Object")] + [TestCase("if (item?.category != 42);", 4, "item?.category", "NetInt", "Int32")] + public void ImplicitComparisons_RaiseDiagnostics(string codeText, int column, string expression, string fromType, string toType) + { + // arrange + string code = UnitTests.SampleProgram.Replace("{{test-code}}", codeText); + DiagnosticResult expected = new DiagnosticResult + { + Id = "SMAPI001", + Message = $"This implicitly converts '{expression}' from {fromType} to {toType}, but {fromType} has unintuitive implicit conversion rules. Consider comparing against the actual value instead to avoid bugs. See https://smapi.io/buildmsg/SMAPI001 for details.", + Severity = DiagnosticSeverity.Warning, + Locations = new[] { new DiagnosticResultLocation("Test0.cs", UnitTests.SampleCodeLine, UnitTests.SampleCodeColumn + column) } + }; + + // assert + this.VerifyCSharpDiagnostic(code, expected); + } + + + /********* + ** Helpers + *********/ + /// Get the analyzer being tested. + protected override DiagnosticAnalyzer GetCSharpDiagnosticAnalyzer() + { + return new ImplicitNetFieldCastAnalyzer(); + } + } +} diff --git a/src/SMAPI.ModBuildConfig.Analyzer/ImplicitNetFieldCastAnalyzer.cs b/src/SMAPI.ModBuildConfig.Analyzer/ImplicitNetFieldCastAnalyzer.cs new file mode 100644 index 00000000..d23bdc2e --- /dev/null +++ b/src/SMAPI.ModBuildConfig.Analyzer/ImplicitNetFieldCastAnalyzer.cs @@ -0,0 +1,107 @@ +using System; +using System.Collections.Immutable; +using Microsoft.CodeAnalysis; +using Microsoft.CodeAnalysis.CSharp; +using Microsoft.CodeAnalysis.CSharp.Syntax; +using Microsoft.CodeAnalysis.Diagnostics; + +namespace StardewModdingAPI.ModBuildConfig.Analyzer +{ + /// Detects implicit conversion from Stardew Valley's Netcode types. These have very unintuitive implicit conversion rules, so mod authors should always explicitly convert the type with appropriate null checks. + [DiagnosticAnalyzer(LanguageNames.CSharp)] + public class ImplicitNetFieldCastAnalyzer : DiagnosticAnalyzer + { + /********* + ** Properties + *********/ + /// The namespace for Stardew Valley's Netcode types. + private const string NetcodeNamespace = "Netcode"; + + /// Describes the diagnostic rule covered by the analyzer. + private static readonly DiagnosticDescriptor Rule = new DiagnosticDescriptor( + id: "SMAPI001", + title: "Netcode types shouldn't be implicitly converted", + messageFormat: "This implicitly converts '{0}' from {1} to {2}, but {1} has unintuitive implicit conversion rules. Consider comparing against the actual value instead to avoid bugs. See https://smapi.io/buildmsg/SMAPI001 for details.", + category: "SMAPI.CommonErrors", + defaultSeverity: DiagnosticSeverity.Warning, + isEnabledByDefault: true, + description: "", + helpLinkUri: "https://smapi.io/buildmsg/SMAPI001" + ); + + + /********* + ** Accessors + *********/ + /// The descriptors for the diagnostics that this analyzer is capable of producing. + public override ImmutableArray SupportedDiagnostics { get; } + + + /********* + ** Public methods + *********/ + /// Construct an instance. + public ImplicitNetFieldCastAnalyzer() + { + this.SupportedDiagnostics = ImmutableArray.Create(ImplicitNetFieldCastAnalyzer.Rule); + } + + /// Called once at session start to register actions in the analysis context. + /// The analysis context. + public override void Initialize(AnalysisContext context) + { + context.RegisterSyntaxNodeAction( + this.Analyse, + SyntaxKind.EqualsExpression, + SyntaxKind.NotEqualsExpression, + SyntaxKind.GreaterThanExpression, + SyntaxKind.GreaterThanOrEqualExpression, + SyntaxKind.LessThanExpression, + SyntaxKind.LessThanOrEqualExpression + ); + } + + + /********* + ** Private methods + *********/ + /// Analyse a syntax node and add a diagnostic message if applicable. + /// The analysis context. + private void Analyse(SyntaxNodeAnalysisContext context) + { + try + { + BinaryExpressionSyntax node = (BinaryExpressionSyntax)context.Node; + bool leftHasWarning = this.Analyze(context, node.Left); + if (!leftHasWarning) + this.Analyze(context, node.Right); + } + catch (Exception ex) + { + throw new InvalidOperationException($"Failed processing expression: '{context.Node}'. Exception details: {ex.ToString().Replace('\r', ' ').Replace('\n', ' ')}"); + } + } + + /// Analyse one operand in a binary expression (like a and b in a == b) and add a diagnostic message if applicable. + /// The analysis context. + /// The operand expression. + /// Returns whether a diagnostic message was raised. + private bool Analyze(SyntaxNodeAnalysisContext context, ExpressionSyntax operand) + { + const string netcodeNamespace = ImplicitNetFieldCastAnalyzer.NetcodeNamespace; + + TypeInfo operandType = context.SemanticModel.GetTypeInfo(operand); + string fromNamespace = operandType.Type?.ContainingNamespace?.Name; + string toNamespace = operandType.ConvertedType?.ContainingNamespace?.Name; + if (fromNamespace == netcodeNamespace && fromNamespace != toNamespace && toNamespace != null) + { + string fromTypeName = operandType.Type.Name; + string toTypeName = operandType.ConvertedType.Name; + context.ReportDiagnostic(Diagnostic.Create(ImplicitNetFieldCastAnalyzer.Rule, context.Node.GetLocation(), operand, fromTypeName, toTypeName)); + return true; + } + + return false; + } + } +} diff --git a/src/SMAPI.ModBuildConfig.Analyzer/Properties/AssemblyInfo.cs b/src/SMAPI.ModBuildConfig.Analyzer/Properties/AssemblyInfo.cs new file mode 100644 index 00000000..1cc41000 --- /dev/null +++ b/src/SMAPI.ModBuildConfig.Analyzer/Properties/AssemblyInfo.cs @@ -0,0 +1,4 @@ +using System.Reflection; + +[assembly: AssemblyTitle("SMAPI.ModBuildConfig.Analyzer")] +[assembly: AssemblyDescription("")] diff --git a/src/SMAPI.ModBuildConfig.Analyzer/StardewModdingAPI.ModBuildConfig.Analyzer.csproj b/src/SMAPI.ModBuildConfig.Analyzer/StardewModdingAPI.ModBuildConfig.Analyzer.csproj new file mode 100644 index 00000000..c32343e3 --- /dev/null +++ b/src/SMAPI.ModBuildConfig.Analyzer/StardewModdingAPI.ModBuildConfig.Analyzer.csproj @@ -0,0 +1,23 @@ + + + + netstandard1.3 + false + false + bin + + + + + + + + + + + + + + + + diff --git a/src/SMAPI.ModBuildConfig.Analyzer/tools/install.ps1 b/src/SMAPI.ModBuildConfig.Analyzer/tools/install.ps1 new file mode 100644 index 00000000..ff051759 --- /dev/null +++ b/src/SMAPI.ModBuildConfig.Analyzer/tools/install.ps1 @@ -0,0 +1,58 @@ +param($installPath, $toolsPath, $package, $project) + +if($project.Object.SupportsPackageDependencyResolution) +{ + if($project.Object.SupportsPackageDependencyResolution()) + { + # Do not install analyzers via install.ps1, instead let the project system handle it. + return + } +} + +$analyzersPaths = Join-Path (Join-Path (Split-Path -Path $toolsPath -Parent) "analyzers") * -Resolve + +foreach($analyzersPath in $analyzersPaths) +{ + if (Test-Path $analyzersPath) + { + # Install the language agnostic analyzers. + foreach ($analyzerFilePath in Get-ChildItem -Path "$analyzersPath\*.dll" -Exclude *.resources.dll) + { + if($project.Object.AnalyzerReferences) + { + $project.Object.AnalyzerReferences.Add($analyzerFilePath.FullName) + } + } + } +} + +# $project.Type gives the language name like (C# or VB.NET) +$languageFolder = "" +if($project.Type -eq "C#") +{ + $languageFolder = "cs" +} +if($project.Type -eq "VB.NET") +{ + $languageFolder = "vb" +} +if($languageFolder -eq "") +{ + return +} + +foreach($analyzersPath in $analyzersPaths) +{ + # Install language specific analyzers. + $languageAnalyzersPath = join-path $analyzersPath $languageFolder + if (Test-Path $languageAnalyzersPath) + { + foreach ($analyzerFilePath in Get-ChildItem -Path "$languageAnalyzersPath\*.dll" -Exclude *.resources.dll) + { + if($project.Object.AnalyzerReferences) + { + $project.Object.AnalyzerReferences.Add($analyzerFilePath.FullName) + } + } + } +} diff --git a/src/SMAPI.ModBuildConfig.Analyzer/tools/uninstall.ps1 b/src/SMAPI.ModBuildConfig.Analyzer/tools/uninstall.ps1 new file mode 100644 index 00000000..4bed3337 --- /dev/null +++ b/src/SMAPI.ModBuildConfig.Analyzer/tools/uninstall.ps1 @@ -0,0 +1,65 @@ +param($installPath, $toolsPath, $package, $project) + +if($project.Object.SupportsPackageDependencyResolution) +{ + if($project.Object.SupportsPackageDependencyResolution()) + { + # Do not uninstall analyzers via uninstall.ps1, instead let the project system handle it. + return + } +} + +$analyzersPaths = Join-Path (Join-Path (Split-Path -Path $toolsPath -Parent) "analyzers") * -Resolve + +foreach($analyzersPath in $analyzersPaths) +{ + # Uninstall the language agnostic analyzers. + if (Test-Path $analyzersPath) + { + foreach ($analyzerFilePath in Get-ChildItem -Path "$analyzersPath\*.dll" -Exclude *.resources.dll) + { + if($project.Object.AnalyzerReferences) + { + $project.Object.AnalyzerReferences.Remove($analyzerFilePath.FullName) + } + } + } +} + +# $project.Type gives the language name like (C# or VB.NET) +$languageFolder = "" +if($project.Type -eq "C#") +{ + $languageFolder = "cs" +} +if($project.Type -eq "VB.NET") +{ + $languageFolder = "vb" +} +if($languageFolder -eq "") +{ + return +} + +foreach($analyzersPath in $analyzersPaths) +{ + # Uninstall language specific analyzers. + $languageAnalyzersPath = join-path $analyzersPath $languageFolder + if (Test-Path $languageAnalyzersPath) + { + foreach ($analyzerFilePath in Get-ChildItem -Path "$languageAnalyzersPath\*.dll" -Exclude *.resources.dll) + { + if($project.Object.AnalyzerReferences) + { + try + { + $project.Object.AnalyzerReferences.Remove($analyzerFilePath.FullName) + } + catch + { + + } + } + } + } +} diff --git a/src/SMAPI.ModBuildConfig/StardewModdingAPI.ModBuildConfig.csproj b/src/SMAPI.ModBuildConfig/StardewModdingAPI.ModBuildConfig.csproj index d2e06e05..2e3ba356 100644 --- a/src/SMAPI.ModBuildConfig/StardewModdingAPI.ModBuildConfig.csproj +++ b/src/SMAPI.ModBuildConfig/StardewModdingAPI.ModBuildConfig.csproj @@ -57,4 +57,5 @@ + \ No newline at end of file diff --git a/src/SMAPI.ModBuildConfig/package.nuspec b/src/SMAPI.ModBuildConfig/package.nuspec index d24e15be..a280a268 100644 --- a/src/SMAPI.ModBuildConfig/package.nuspec +++ b/src/SMAPI.ModBuildConfig/package.nuspec @@ -2,7 +2,7 @@ Pathoschild.Stardew.ModBuildConfig - 2.0.3-alpha20180325 + 2.1-alpha20180409 Build package for SMAPI mods Pathoschild Pathoschild @@ -12,28 +12,10 @@ https://raw.githubusercontent.com/Pathoschild/SMAPI/develop/src/SMAPI.ModBuildConfig/assets/nuget-icon.png Automates the build configuration for crossplatform Stardew Valley SMAPI mods. - 2.0: - - Added: mods are now copied into the `Mods` folder automatically (configurable). - - Added: release zips are now created automatically in your build output folder (configurable). - - Added: mod deploy and release zips now exclude Json.NET automatically, since it's provided by SMAPI. - - Added mod's version to release zip filename. - - Improved errors to simplify troubleshooting. - - Fixed release zip not having a mod folder. - - Fixed release zip failing if mod name contains characters that aren't valid in a filename. - - 2.0.1: - - Fixed mod deploy failing to create subfolders if they don't already exist. - - 2.0.2: - - Fixed compatibility issue on Linux. - - 2.0.3: + 2.1: - Added support for Stardew Valley 1.3. - Added support for unit test projects. + - Added C# analyzers to warn about implicit conversions of Netcode fields in Stardew Valley 1.3. - - - - diff --git a/src/SMAPI.Tests/StardewModdingAPI.Tests.csproj b/src/SMAPI.Tests/StardewModdingAPI.Tests.csproj index e4b2c8c6..858a0d4a 100644 --- a/src/SMAPI.Tests/StardewModdingAPI.Tests.csproj +++ b/src/SMAPI.Tests/StardewModdingAPI.Tests.csproj @@ -66,6 +66,9 @@ StardewModdingAPI + + + \ No newline at end of file diff --git a/src/SMAPI.sln b/src/SMAPI.sln index 6577b42e..56898a32 100644 --- a/src/SMAPI.sln +++ b/src/SMAPI.sln @@ -45,9 +45,17 @@ Project("{2150E333-8FDC-42A3-9474-1A3956D46DE8}") = "build", "build", "{09CF91E5 ..\build\common.targets = ..\build\common.targets ..\build\GlobalAssemblyInfo.cs = ..\build\GlobalAssemblyInfo.cs ..\build\prepare-install-package.targets = ..\build\prepare-install-package.targets + ..\build\prepare-nuget-package.targets = ..\build\prepare-nuget-package.targets EndProjectSection EndProject Project("{FAE04EC0-301F-11D3-BF4B-00C04F79EFBC}") = "StardewModdingAPI.ModBuildConfig", "SMAPI.ModBuildConfig\StardewModdingAPI.ModBuildConfig.csproj", "{EA4F1E80-743F-4A1D-9757-AE66904A196A}" + ProjectSection(ProjectDependencies) = postProject + {80AD8528-AA49-4731-B4A6-C691845815A1} = {80AD8528-AA49-4731-B4A6-C691845815A1} + EndProjectSection +EndProject +Project("{9A19103F-16F7-4668-BE54-9A1E7A4F7556}") = "StardewModdingAPI.ModBuildConfig.Analyzer", "SMAPI.ModBuildConfig.Analyzer\StardewModdingAPI.ModBuildConfig.Analyzer.csproj", "{80AD8528-AA49-4731-B4A6-C691845815A1}" +EndProject +Project("{9A19103F-16F7-4668-BE54-9A1E7A4F7556}") = "SMAPI.ModBuildConfig.Analyzer.Tests", "SMAPI.ModBuildConfig.Analyzer.Tests\SMAPI.ModBuildConfig.Analyzer.Tests.csproj", "{0CF97929-B0D0-4D73-B7BF-4FF7191035F9}" EndProject Global GlobalSection(SharedMSBuildProjectFiles) = preSolution @@ -137,6 +145,30 @@ Global {EA4F1E80-743F-4A1D-9757-AE66904A196A}.Release|Mixed Platforms.Build.0 = Release|x86 {EA4F1E80-743F-4A1D-9757-AE66904A196A}.Release|x86.ActiveCfg = Release|x86 {EA4F1E80-743F-4A1D-9757-AE66904A196A}.Release|x86.Build.0 = Release|x86 + {80AD8528-AA49-4731-B4A6-C691845815A1}.Debug|Any CPU.ActiveCfg = Debug|Any CPU + {80AD8528-AA49-4731-B4A6-C691845815A1}.Debug|Any CPU.Build.0 = Debug|Any CPU + {80AD8528-AA49-4731-B4A6-C691845815A1}.Debug|Mixed Platforms.ActiveCfg = Debug|Any CPU + {80AD8528-AA49-4731-B4A6-C691845815A1}.Debug|Mixed Platforms.Build.0 = Debug|Any CPU + {80AD8528-AA49-4731-B4A6-C691845815A1}.Debug|x86.ActiveCfg = Debug|Any CPU + {80AD8528-AA49-4731-B4A6-C691845815A1}.Debug|x86.Build.0 = Debug|Any CPU + {80AD8528-AA49-4731-B4A6-C691845815A1}.Release|Any CPU.ActiveCfg = Release|Any CPU + {80AD8528-AA49-4731-B4A6-C691845815A1}.Release|Any CPU.Build.0 = Release|Any CPU + {80AD8528-AA49-4731-B4A6-C691845815A1}.Release|Mixed Platforms.ActiveCfg = Release|Any CPU + {80AD8528-AA49-4731-B4A6-C691845815A1}.Release|Mixed Platforms.Build.0 = Release|Any CPU + {80AD8528-AA49-4731-B4A6-C691845815A1}.Release|x86.ActiveCfg = Release|Any CPU + {80AD8528-AA49-4731-B4A6-C691845815A1}.Release|x86.Build.0 = Release|Any CPU + {0CF97929-B0D0-4D73-B7BF-4FF7191035F9}.Debug|Any CPU.ActiveCfg = Debug|Any CPU + {0CF97929-B0D0-4D73-B7BF-4FF7191035F9}.Debug|Any CPU.Build.0 = Debug|Any CPU + {0CF97929-B0D0-4D73-B7BF-4FF7191035F9}.Debug|Mixed Platforms.ActiveCfg = Debug|Any CPU + {0CF97929-B0D0-4D73-B7BF-4FF7191035F9}.Debug|Mixed Platforms.Build.0 = Debug|Any CPU + {0CF97929-B0D0-4D73-B7BF-4FF7191035F9}.Debug|x86.ActiveCfg = Debug|Any CPU + {0CF97929-B0D0-4D73-B7BF-4FF7191035F9}.Debug|x86.Build.0 = Debug|Any CPU + {0CF97929-B0D0-4D73-B7BF-4FF7191035F9}.Release|Any CPU.ActiveCfg = Release|Any CPU + {0CF97929-B0D0-4D73-B7BF-4FF7191035F9}.Release|Any CPU.Build.0 = Release|Any CPU + {0CF97929-B0D0-4D73-B7BF-4FF7191035F9}.Release|Mixed Platforms.ActiveCfg = Release|Any CPU + {0CF97929-B0D0-4D73-B7BF-4FF7191035F9}.Release|Mixed Platforms.Build.0 = Release|Any CPU + {0CF97929-B0D0-4D73-B7BF-4FF7191035F9}.Release|x86.ActiveCfg = Release|Any CPU + {0CF97929-B0D0-4D73-B7BF-4FF7191035F9}.Release|x86.Build.0 = Release|Any CPU EndGlobalSection GlobalSection(SolutionProperties) = preSolution HideSolutionNode = FALSE @@ -147,6 +179,7 @@ Global {2AA02FB6-FF03-41CF-A215-2EE60AB4F5DC} = {82D22ED7-A0A7-4D64-8E92-4B6A5E74ED11} {EB35A917-67B9-4EFA-8DFC-4FB49B3949BB} = {86C452BE-D2D8-45B4-B63F-E329EB06CEDA} {09CF91E5-5BAB-4650-A200-E5EA9A633046} = {86C452BE-D2D8-45B4-B63F-E329EB06CEDA} + {0CF97929-B0D0-4D73-B7BF-4FF7191035F9} = {82D22ED7-A0A7-4D64-8E92-4B6A5E74ED11} EndGlobalSection GlobalSection(ExtensibilityGlobals) = postSolution SolutionGuid = {70143042-A862-47A8-A677-7C819DDC90DC} -- cgit From 4f5f463bd227a81c26224a81b178e2221946b9b2 Mon Sep 17 00:00:00 2001 From: Jesse Plamondon-Willard Date: Mon, 9 Apr 2018 22:33:45 -0400 Subject: warn when directly using a net field that has a non-net wrapper (#471) --- docs/mod-build-config.md | 9 + docs/screenshots/code-analyzer-example.png | Bin 5696 -> 4022 bytes .../UnitTests.cs | 43 ++++- .../ImplicitNetFieldCastAnalyzer.cs | 208 ++++++++++++++++++--- 4 files changed, 228 insertions(+), 32 deletions(-) (limited to 'docs') diff --git a/docs/mod-build-config.md b/docs/mod-build-config.md index 71a2518a..44242160 100644 --- a/docs/mod-build-config.md +++ b/docs/mod-build-config.md @@ -182,6 +182,15 @@ Suggested fix: if (item != null && item.category == 0) ``` +### SMAPI002 +**Avoid net fields when possible:** +> '{{expression}}' is a {{net type}} field; consider using the {{property name}} property instead. + +Your code accesses a net field, which has some unusual behavior (see [SMAPI001](#SMAPI001)). This +field has an equivalent non-net property that avoids those issues. + +Suggested fix: access the suggested property name instead. + ## Troubleshoot ### "Failed to find the game install path" That error means the package couldn't find your game. You can specify the game path yourself; see diff --git a/docs/screenshots/code-analyzer-example.png b/docs/screenshots/code-analyzer-example.png index 2723b164..3b930dc5 100644 Binary files a/docs/screenshots/code-analyzer-example.png and b/docs/screenshots/code-analyzer-example.png differ diff --git a/src/SMAPI.ModBuildConfig.Analyzer.Tests/UnitTests.cs b/src/SMAPI.ModBuildConfig.Analyzer.Tests/UnitTests.cs index d5e4ef52..b4f54234 100644 --- a/src/SMAPI.ModBuildConfig.Analyzer.Tests/UnitTests.cs +++ b/src/SMAPI.ModBuildConfig.Analyzer.Tests/UnitTests.cs @@ -16,6 +16,7 @@ namespace SMAPI.ModBuildConfig.Analyzer.Tests /// Sample C# code which contains a simplified representation of Stardew Valley's Netcode types, and sample mod code with a {{test-code}} placeholder for the code being tested. const string SampleProgram = @" using System; + using StardewValley; using Netcode; namespace Netcode @@ -29,13 +30,16 @@ namespace SMAPI.ModBuildConfig.Analyzer.Tests } } - namespace SampleMod + namespace StardewValley { class Item { public NetInt category { get; } = new NetInt { Value = 42 }; } + } + namespace SampleMod + { class ModEntry { public void Entry() @@ -45,17 +49,17 @@ namespace SMAPI.ModBuildConfig.Analyzer.Tests Item item = null; // this line should raise diagnostics - {{test-code}} // line 32 + {{test-code}} // line 36 // these lines should not - if ((int)field != 42); + if ((int)intField != 42); } } } "; /// The line number where the unit tested code is injected into . - private const int SampleCodeLine = 32; + private const int SampleCodeLine = 36; /// The column number where the unit tested code is injected into . private const int SampleCodeColumn = 25; @@ -75,7 +79,7 @@ namespace SMAPI.ModBuildConfig.Analyzer.Tests this.VerifyCSharpDiagnostic(test); } - /// Test that the expected diagnostic message is raised. + /// Test that the expected diagnostic message is raised for implicit net field comparisons. /// The code line to test. /// The column within the code line where the diagnostic message should be reported. /// The expression which should be reported. @@ -89,7 +93,7 @@ namespace SMAPI.ModBuildConfig.Analyzer.Tests [TestCase("if (intField != 42);", 4, "intField", "NetInt", "Int32")] [TestCase("if (refField != null);", 4, "refField", "NetRef", "Object")] [TestCase("if (item?.category != 42);", 4, "item?.category", "NetInt", "Int32")] - public void ImplicitComparisons_RaiseDiagnostics(string codeText, int column, string expression, string fromType, string toType) + public void AvoidImplicitNetFieldComparisons_RaisesDiagnostic(string codeText, int column, string expression, string fromType, string toType) { // arrange string code = UnitTests.SampleProgram.Replace("{{test-code}}", codeText); @@ -105,6 +109,31 @@ namespace SMAPI.ModBuildConfig.Analyzer.Tests this.VerifyCSharpDiagnostic(code, expected); } + /// Test that the expected diagnostic message is raised for avoidable net field references. + /// The code line to test. + /// The column within the code line where the diagnostic message should be reported. + /// The expression which should be reported. + /// The net type name which should be reported. + /// The suggested property name which should be reported. + [TestCase("int category = item.category;", 15, "item.category", "NetInt", "Category")] + [TestCase("int category = (item).category;", 15, "(item).category", "NetInt", "Category")] + [TestCase("int category = ((Item)item).category;", 15, "((Item)item).category", "NetInt", "Category")] + public void AvoidNetFields_RaisesDiagnostic(string codeText, int column, string expression, string netType, string suggestedProperty) + { + // arrange + string code = UnitTests.SampleProgram.Replace("{{test-code}}", codeText); + DiagnosticResult expected = new DiagnosticResult + { + Id = "SMAPI002", + Message = $"'{expression}' is a {netType} field; consider using the {suggestedProperty} property instead. See https://smapi.io/buildmsg/SMAPI002 for details.", + Severity = DiagnosticSeverity.Warning, + Locations = new[] { new DiagnosticResultLocation("Test0.cs", UnitTests.SampleCodeLine, UnitTests.SampleCodeColumn + column) } + }; + + // assert + this.VerifyCSharpDiagnostic(code, expected); + } + /********* ** Helpers @@ -112,7 +141,7 @@ namespace SMAPI.ModBuildConfig.Analyzer.Tests /// Get the analyzer being tested. protected override DiagnosticAnalyzer GetCSharpDiagnosticAnalyzer() { - return new ImplicitNetFieldCastAnalyzer(); + return new NetFieldAnalyzer(); } } } diff --git a/src/SMAPI.ModBuildConfig.Analyzer/ImplicitNetFieldCastAnalyzer.cs b/src/SMAPI.ModBuildConfig.Analyzer/ImplicitNetFieldCastAnalyzer.cs index d23bdc2e..d64b1486 100644 --- a/src/SMAPI.ModBuildConfig.Analyzer/ImplicitNetFieldCastAnalyzer.cs +++ b/src/SMAPI.ModBuildConfig.Analyzer/ImplicitNetFieldCastAnalyzer.cs @@ -1,4 +1,5 @@ using System; +using System.Collections.Generic; using System.Collections.Immutable; using Microsoft.CodeAnalysis; using Microsoft.CodeAnalysis.CSharp; @@ -9,7 +10,7 @@ namespace StardewModdingAPI.ModBuildConfig.Analyzer { /// Detects implicit conversion from Stardew Valley's Netcode types. These have very unintuitive implicit conversion rules, so mod authors should always explicitly convert the type with appropriate null checks. [DiagnosticAnalyzer(LanguageNames.CSharp)] - public class ImplicitNetFieldCastAnalyzer : DiagnosticAnalyzer + public class NetFieldAnalyzer : DiagnosticAnalyzer { /********* ** Properties @@ -17,17 +18,136 @@ namespace StardewModdingAPI.ModBuildConfig.Analyzer /// The namespace for Stardew Valley's Netcode types. private const string NetcodeNamespace = "Netcode"; + /// Maps net fields to their equivalent non-net properties where available. + private readonly IDictionary NetFieldWrapperProperties = new Dictionary + { + // Character + ["StardewValley.Character::currentLocationRef"] = "currentLocation", + ["StardewValley.Character::facingDirection"] = "FacingDirection", + ["StardewValley.Character::name"] = "Name", + ["StardewValley.Character::position"] = "Position", + ["StardewValley.Character::scale"] = "Scale", + ["StardewValley.Character::speed"] = "Speed", + ["StardewValley.Character::sprite"] = "Sprite", + + // Chest + ["StardewValley.Objects.Chest::tint"] = "Tint", + + // Farmer + ["StardewValley.Farmer::houseUpgradeLevel"] = "HouseUpgradeLevel", + ["StardewValley.Farmer::isMale"] = "IsMale", + ["StardewValley.Farmer::items"] = "Items", + ["StardewValley.Farmer::magneticRadius"] = "MagneticRadius", + ["StardewValley.Farmer::stamina"] = "Stamina", + ["StardewValley.Farmer::uniqueMultiplayerID"] = "UniqueMultiplayerID", + ["StardewValley.Farmer::usingTool"] = "UsingTool", + + // Forest + ["StardewValley.Locations.Forest::netTravelingMerchantDay"] = "travelingMerchantDay", + ["StardewValley.Locations.Forest::netLog"] = "log", + + // FruitTree + ["StardewValley.TerrainFeatures.FruitTree::greenHouseTileTree"] = "GreenHouseTileTree", + ["StardewValley.TerrainFeatures.FruitTree::greenHouseTree"] = "GreenHouseTree", + + // GameLocation + ["StardewValley.GameLocation::isFarm"] = "IsFarm", + ["StardewValley.GameLocation::isOutdoors"] = "IsOutdoors", + ["StardewValley.GameLocation::lightLevel"] = "LightLevel", + ["StardewValley.GameLocation::name"] = "Name", + + // Item + ["StardewValley.Item::category"] = "Category", + ["StardewValley.Item::netName"] = "Name", + ["StardewValley.Item::parentSheetIndex"] = "ParentSheetIndex", + ["StardewValley.Item::specialVariable"] = "SpecialVariable", + + // Junimo + ["StardewValley.Characters.Junimo::eventActor"] = "EventActor", + + // LightSource + ["StardewValley.LightSource::identifier"] = "Identifier", + + // Monster + ["StardewValley.Monsters.Monster::damageToFarmer"] = "DamageToFarmer", + ["StardewValley.Monsters.Monster::experienceGained"] = "ExperienceGained", + ["StardewValley.Monsters.Monster::health"] = "Health", + ["StardewValley.Monsters.Monster::maxHealth"] = "MaxHealth", + ["StardewValley.Monsters.Monster::netFocusedOnFarmers"] = "focusedOnFarmers", + ["StardewValley.Monsters.Monster::netWildernessFarmMonster"] = "wildernessFarmMonster", + ["StardewValley.Monsters.Monster::slipperiness"] = "Slipperiness", + + // NPC + ["StardewValley.NPC::age"] = "Age", + ["StardewValley.NPC::birthday_Day"] = "Birthday_Day", + ["StardewValley.NPC::birthday_Season"] = "Birthday_Season", + ["StardewValley.NPC::breather"] = "Breather", + ["StardewValley.NPC::defaultMap"] = "DefaultMap", + ["StardewValley.NPC::gender"] = "Gender", + ["StardewValley.NPC::hideShadow"] = "HideShadow", + ["StardewValley.NPC::isInvisible"] = "IsInvisible", + ["StardewValley.NPC::isWalkingTowardPlayer"] = "IsWalkingTowardPlayer", + ["StardewValley.NPC::manners"] = "Manners", + ["StardewValley.NPC::optimism"] = "Optimism", + ["StardewValley.NPC::socialAnxiety"] = "SocialAnxiety", + + // Object + ["StardewValley.Object::canBeGrabbed"] = "CanBeGrabbed", + ["StardewValley.Object::canBeSetDown"] = "CanBeSetDown", + ["StardewValley.Object::edibility"] = "Edibility", + ["StardewValley.Object::flipped"] = "Flipped", + ["StardewValley.Object::fragility"] = "Fragility", + ["StardewValley.Object::hasBeenPickedUpByFarmer"] = "HasBeenPickedUpByFarmer", + ["StardewValley.Object::isHoedirt"] = "IsHoeDirt", + ["StardewValley.Object::isOn"] = "IsOn", + ["StardewValley.Object::isRecipe"] = "IsRecipe", + ["StardewValley.Object::isSpawnedObject"] = "IsSpawnedObject", + ["StardewValley.Object::minutesUntilReady"] = "MinutesUntilReady", + ["StardewValley.Object::netName"] = "name", + ["StardewValley.Object::price"] = "Price", + ["StardewValley.Object::quality"] = "Quality", + ["StardewValley.Object::scale"] = "Scale", + ["StardewValley.Object::stack"] = "Stack", + ["StardewValley.Object::tileLocation"] = "TileLocation", + ["StardewValley.Object::type"] = "Type", + + // Projectile + ["StardewValley.Projectiles.Projectile::ignoreLocationCollision"] = "IgnoreLocationCollision", + + // Tool + ["StardewValley.Tool::currentParentTileIndex"] = "CurrentParentTileIndex", + ["StardewValley.Tool::indexOfMenuItemView"] = "IndexOfMenuItemView", + ["StardewValley.Tool::initialParentTileIndex"] = "InitialParentTileIndex", + ["StardewValley.Tool::instantUse"] = "InstantUse", + ["StardewValley.Tool::netName"] = "BaseName", + ["StardewValley.Tool::stackable"] = "Stackable", + ["StardewValley.Tool::upgradeLevel"] = "UpgradeLevel" + }; + /// Describes the diagnostic rule covered by the analyzer. - private static readonly DiagnosticDescriptor Rule = new DiagnosticDescriptor( - id: "SMAPI001", - title: "Netcode types shouldn't be implicitly converted", - messageFormat: "This implicitly converts '{0}' from {1} to {2}, but {1} has unintuitive implicit conversion rules. Consider comparing against the actual value instead to avoid bugs. See https://smapi.io/buildmsg/SMAPI001 for details.", - category: "SMAPI.CommonErrors", - defaultSeverity: DiagnosticSeverity.Warning, - isEnabledByDefault: true, - description: "", - helpLinkUri: "https://smapi.io/buildmsg/SMAPI001" - ); + private readonly IDictionary Rules = new Dictionary + { + ["SMAPI001"] = new DiagnosticDescriptor( + id: "SMAPI001", + title: "Netcode types shouldn't be implicitly converted", + messageFormat: "This implicitly converts '{0}' from {1} to {2}, but {1} has unintuitive implicit conversion rules. Consider comparing against the actual value instead to avoid bugs. See https://smapi.io/buildmsg/SMAPI001 for details.", + category: "SMAPI.CommonErrors", + defaultSeverity: DiagnosticSeverity.Warning, + isEnabledByDefault: true, + description: "", + helpLinkUri: "https://smapi.io/buildmsg/SMAPI001" + ), + ["SMAPI002"] = new DiagnosticDescriptor( + id: "SMAPI002", + title: "Avoid Netcode types when possible", + messageFormat: "'{0}' is a {1} field; consider using the {2} property instead. See https://smapi.io/buildmsg/SMAPI002 for details.", + category: "SMAPI.CommonErrors", + defaultSeverity: DiagnosticSeverity.Warning, + isEnabledByDefault: true, + description: "", + helpLinkUri: "https://smapi.io/buildmsg/SMAPI001" + ) + }; /********* @@ -41,17 +161,24 @@ namespace StardewModdingAPI.ModBuildConfig.Analyzer ** Public methods *********/ /// Construct an instance. - public ImplicitNetFieldCastAnalyzer() + public NetFieldAnalyzer() { - this.SupportedDiagnostics = ImmutableArray.Create(ImplicitNetFieldCastAnalyzer.Rule); + this.SupportedDiagnostics = ImmutableArray.CreateRange(this.Rules.Values); } /// Called once at session start to register actions in the analysis context. /// The analysis context. public override void Initialize(AnalysisContext context) { + // SMAPI002: avoid net fields if possible + context.RegisterSyntaxNodeAction( + this.AnalyzeAvoidableNetField, + SyntaxKind.SimpleMemberAccessExpression + ); + + // SMAPI001: avoid implicit net field conversion context.RegisterSyntaxNodeAction( - this.Analyse, + this.AnalyseNetFieldConversions, SyntaxKind.EqualsExpression, SyntaxKind.NotEqualsExpression, SyntaxKind.GreaterThanExpression, @@ -65,16 +192,44 @@ namespace StardewModdingAPI.ModBuildConfig.Analyzer /********* ** Private methods *********/ - /// Analyse a syntax node and add a diagnostic message if applicable. + /// Analyse a syntax node and add a diagnostic message if it references a net field when there's a non-net equivalent available. + /// The analysis context. + private void AnalyzeAvoidableNetField(SyntaxNodeAnalysisContext context) + { + try + { + // check member type + MemberAccessExpressionSyntax node = (MemberAccessExpressionSyntax)context.Node; + TypeInfo memberType = context.SemanticModel.GetTypeInfo(node); + if (!this.IsNetType(memberType.Type)) + return; + + // get reference info + ITypeSymbol declaringType = context.SemanticModel.GetTypeInfo(node.Expression).Type; + string propertyName = node.Name.Identifier.Text; + + // suggest replacement + if (this.NetFieldWrapperProperties.TryGetValue($"{declaringType}::{propertyName}", out string suggestedPropertyName)) + { + context.ReportDiagnostic(Diagnostic.Create(this.Rules["SMAPI002"], context.Node.GetLocation(), node, memberType.Type.Name, suggestedPropertyName)); + } + } + catch (Exception ex) + { + throw new InvalidOperationException($"Failed processing expression: '{context.Node}'. Exception details: {ex.ToString().Replace('\r', ' ').Replace('\n', ' ')}"); + } + } + + /// Analyse a syntax node and add a diagnostic message if it implicitly converts a net field. /// The analysis context. - private void Analyse(SyntaxNodeAnalysisContext context) + private void AnalyseNetFieldConversions(SyntaxNodeAnalysisContext context) { try { BinaryExpressionSyntax node = (BinaryExpressionSyntax)context.Node; - bool leftHasWarning = this.Analyze(context, node.Left); + bool leftHasWarning = this.WarnIfOperandImplicitlyConvertsNetField(context, node.Left); if (!leftHasWarning) - this.Analyze(context, node.Right); + this.WarnIfOperandImplicitlyConvertsNetField(context, node.Right); } catch (Exception ex) { @@ -86,22 +241,25 @@ namespace StardewModdingAPI.ModBuildConfig.Analyzer /// The analysis context. /// The operand expression. /// Returns whether a diagnostic message was raised. - private bool Analyze(SyntaxNodeAnalysisContext context, ExpressionSyntax operand) + private bool WarnIfOperandImplicitlyConvertsNetField(SyntaxNodeAnalysisContext context, ExpressionSyntax operand) { - const string netcodeNamespace = ImplicitNetFieldCastAnalyzer.NetcodeNamespace; - TypeInfo operandType = context.SemanticModel.GetTypeInfo(operand); - string fromNamespace = operandType.Type?.ContainingNamespace?.Name; - string toNamespace = operandType.ConvertedType?.ContainingNamespace?.Name; - if (fromNamespace == netcodeNamespace && fromNamespace != toNamespace && toNamespace != null) + if (this.IsNetType(operandType.Type) && !this.IsNetType(operandType.ConvertedType)) { string fromTypeName = operandType.Type.Name; string toTypeName = operandType.ConvertedType.Name; - context.ReportDiagnostic(Diagnostic.Create(ImplicitNetFieldCastAnalyzer.Rule, context.Node.GetLocation(), operand, fromTypeName, toTypeName)); + context.ReportDiagnostic(Diagnostic.Create(this.Rules["SMAPI001"], context.Node.GetLocation(), operand, fromTypeName, toTypeName)); return true; } return false; } + + /// Get whether a type symbol references a Netcode type. + /// The type symbol. + private bool IsNetType(ITypeSymbol typeSymbol) + { + return typeSymbol?.ContainingNamespace?.Name == NetFieldAnalyzer.NetcodeNamespace; + } } } -- cgit From 9fba3c12667555958e2eb6ecf1d2ebe47fd77c58 Mon Sep 17 00:00:00 2001 From: Jesse Plamondon-Willard Date: Tue, 10 Apr 2018 18:20:49 -0400 Subject: add context properties for multiplayer, update release notes (#453) --- docs/release-notes.md | 4 +++- src/SMAPI/Context.cs | 8 +++++++- 2 files changed, 10 insertions(+), 2 deletions(-) (limited to 'docs') diff --git a/docs/release-notes.md b/docs/release-notes.md index 3406e735..c04b6bdf 100644 --- a/docs/release-notes.md +++ b/docs/release-notes.md @@ -3,12 +3,14 @@ ## 2.6 alpha * For players: * Added support for Stardew Valley 1.3+; no longer compatible with earlier versions. + * Added `Context.IsMultiplayer` and `Context.IsMainPlayer` flags. * Fixed SMAPI update alerts linking to the GitHub repository instead of [smapi.io](https://smapi.io). * Fixed SMAPI update checks not showing newer beta versions when using a beta version. * For modders: * Dropped some deprecated APIs. - * Fixed some assets not being editable. + * Fixed assets loaded by temporary content managers not being editable. + * Fixed issue where assets didn't reload correctly when the player switches language. * For SMAPI developers: * Added prerelease versions to the mod update-check API response where available (GitHub only). diff --git a/src/SMAPI/Context.cs b/src/SMAPI/Context.cs index 119e14c8..7ed9b052 100644 --- a/src/SMAPI/Context.cs +++ b/src/SMAPI/Context.cs @@ -1,4 +1,4 @@ -using StardewModdingAPI.Events; +using StardewModdingAPI.Events; using StardewValley; using StardewValley.Menus; @@ -25,6 +25,12 @@ namespace StardewModdingAPI /// Whether the game is currently running the draw loop. This isn't relevant to most mods, since you should use to draw to the screen. public static bool IsInDrawLoop { get; internal set; } + /// Whether and the player loaded the save in multiplayer mode (regardless of whether any other players are connected). + public static bool IsMultiplayer => Context.IsWorldReady && Game1.multiplayerMode != Game1.singlePlayer; + + /// Whether and the current player is the main player. This is always true in single-player, and true when hosting in multiplayer. + public static bool IsMainPlayer => Context.IsWorldReady && Game1.IsMasterGame; + /**** ** Internal ****/ -- cgit From c6c2302baf830a441b269ef7402068bd9dece22e Mon Sep 17 00:00:00 2001 From: Jesse Plamondon-Willard Date: Tue, 10 Apr 2018 18:23:39 -0400 Subject: tweak analyzer code & documentation (#471) --- docs/mod-build-config.md | 17 ++++++++++------- src/SMAPI.ModBuildConfig.Analyzer.Tests/UnitTests.cs | 4 ++-- src/SMAPI.ModBuildConfig.Analyzer/NetFieldAnalyzer.cs | 10 ++++------ 3 files changed, 16 insertions(+), 15 deletions(-) (limited to 'docs') diff --git a/docs/mod-build-config.md b/docs/mod-build-config.md index 44242160..00f9a356 100644 --- a/docs/mod-build-config.md +++ b/docs/mod-build-config.md @@ -159,13 +159,16 @@ See below for help with each specific warning. > {{net type}} has unintuitive implicit conversion rules. Consider comparing against the actual > value instead to avoid bugs. -Stardew Valley uses a set of net fields (like `NetBool` or `NetInt`) to handle multiplayer sync. -These types can implicitly convert to their equivalent normal values (like `bool x = new NetBool()`), -but their conversion rules can be unintuitive and error-prone. For example, +Stardew Valley uses net types (like `NetBool` and `NetInt`) to handle multiplayer sync. These types +can implicitly convert to their equivalent normal values (like `bool x = new NetBool()`), but their +conversion rules are unintuitive and error-prone. For example, `item?.category == null && item?.category != null` can both be true at once, and `building.indoors != null` will be true for a null value in some cases. Suggested fix: +* Some net fields have an equivalent non-net property like `monster.Health` (`int`) instead of + `monster.health` (`NetInt`). The package will add a separate [SMAPI002](#smapi002) warning for + these. Use the suggested property instead. * For a reference type (i.e. one that can contain `null`), you can use the `.Value` property: ```c# if (building.indoors.Value == null) @@ -176,17 +179,17 @@ Suggested fix: if(indoors == null) // ... ``` -* For a value type (i.e. one that can't contain `null`), make sure to check for null first if - applicable: +* For a value type (i.e. one that can't contain `null`), check if the object is null (if applicable) + and compare with `.Value`: ```cs - if (item != null && item.category == 0) + if (item != null && item.category.Value == 0) ``` ### SMAPI002 **Avoid net fields when possible:** > '{{expression}}' is a {{net type}} field; consider using the {{property name}} property instead. -Your code accesses a net field, which has some unusual behavior (see [SMAPI001](#SMAPI001)). This +Your code accesses a net field, which has some unusual behavior (see [SMAPI001](#smapi001)). This field has an equivalent non-net property that avoids those issues. Suggested fix: access the suggested property name instead. diff --git a/src/SMAPI.ModBuildConfig.Analyzer.Tests/UnitTests.cs b/src/SMAPI.ModBuildConfig.Analyzer.Tests/UnitTests.cs index 51e0b059..8ca27847 100644 --- a/src/SMAPI.ModBuildConfig.Analyzer.Tests/UnitTests.cs +++ b/src/SMAPI.ModBuildConfig.Analyzer.Tests/UnitTests.cs @@ -92,7 +92,7 @@ namespace SMAPI.ModBuildConfig.Analyzer.Tests DiagnosticResult expected = new DiagnosticResult { Id = "SMAPI001", - Message = $"This implicitly converts '{expression}' from {fromType} to {toType}, but {fromType} has unintuitive implicit conversion rules. Consider comparing against the actual value instead to avoid bugs. See https://smapi.io/buildmsg/SMAPI001 for details.", + Message = $"This implicitly converts '{expression}' from {fromType} to {toType}, but {fromType} has unintuitive implicit conversion rules. Consider comparing against the actual value instead to avoid bugs. See https://smapi.io/buildmsg/smapi001 for details.", Severity = DiagnosticSeverity.Warning, Locations = new[] { new DiagnosticResultLocation("Test0.cs", UnitTests.SampleCodeLine, UnitTests.SampleCodeColumn + column) } }; @@ -118,7 +118,7 @@ namespace SMAPI.ModBuildConfig.Analyzer.Tests DiagnosticResult expected = new DiagnosticResult { Id = "SMAPI002", - Message = $"'{expression}' is a {netType} field; consider using the {suggestedProperty} property instead. See https://smapi.io/buildmsg/SMAPI002 for details.", + Message = $"'{expression}' is a {netType} field; consider using the {suggestedProperty} property instead. See https://smapi.io/buildmsg/smapi002 for details.", Severity = DiagnosticSeverity.Warning, Locations = new[] { new DiagnosticResultLocation("Test0.cs", UnitTests.SampleCodeLine, UnitTests.SampleCodeColumn + column) } }; diff --git a/src/SMAPI.ModBuildConfig.Analyzer/NetFieldAnalyzer.cs b/src/SMAPI.ModBuildConfig.Analyzer/NetFieldAnalyzer.cs index ae646fd8..915a50e8 100644 --- a/src/SMAPI.ModBuildConfig.Analyzer/NetFieldAnalyzer.cs +++ b/src/SMAPI.ModBuildConfig.Analyzer/NetFieldAnalyzer.cs @@ -130,22 +130,20 @@ namespace StardewModdingAPI.ModBuildConfig.Analyzer ["SMAPI001"] = new DiagnosticDescriptor( id: "SMAPI001", title: "Netcode types shouldn't be implicitly converted", - messageFormat: "This implicitly converts '{0}' from {1} to {2}, but {1} has unintuitive implicit conversion rules. Consider comparing against the actual value instead to avoid bugs. See https://smapi.io/buildmsg/SMAPI001 for details.", + messageFormat: "This implicitly converts '{0}' from {1} to {2}, but {1} has unintuitive implicit conversion rules. Consider comparing against the actual value instead to avoid bugs. See https://smapi.io/buildmsg/smapi001 for details.", category: "SMAPI.CommonErrors", defaultSeverity: DiagnosticSeverity.Warning, isEnabledByDefault: true, - description: "", - helpLinkUri: "https://smapi.io/buildmsg/SMAPI001" + helpLinkUri: "https://smapi.io/buildmsg/smapi001" ), ["SMAPI002"] = new DiagnosticDescriptor( id: "SMAPI002", title: "Avoid Netcode types when possible", - messageFormat: "'{0}' is a {1} field; consider using the {2} property instead. See https://smapi.io/buildmsg/SMAPI002 for details.", + messageFormat: "'{0}' is a {1} field; consider using the {2} property instead. See https://smapi.io/buildmsg/smapi002 for details.", category: "SMAPI.CommonErrors", defaultSeverity: DiagnosticSeverity.Warning, isEnabledByDefault: true, - description: "", - helpLinkUri: "https://smapi.io/buildmsg/SMAPI001" + helpLinkUri: "https://smapi.io/buildmsg/smapi001" ) }; -- cgit From 13f31e8b725e46ca8442a943a5675723d22b4fdc Mon Sep 17 00:00:00 2001 From: Jesse Plamondon-Willard Date: Tue, 10 Apr 2018 18:23:57 -0400 Subject: warn for fields which no longer work (#471) --- docs/mod-build-config.md | 6 + .../Mock/StardewValley/Farmer.cs | 11 ++ .../NetFieldAnalyzerTests.cs | 140 +++++++++++++++++++++ .../ObsoleteFieldAnalyzerTests.cs | 88 +++++++++++++ .../UnitTests.cs | 140 --------------------- .../ObsoleteFieldAnalyzer.cs | 98 +++++++++++++++ 6 files changed, 343 insertions(+), 140 deletions(-) create mode 100644 src/SMAPI.ModBuildConfig.Analyzer.Tests/Mock/StardewValley/Farmer.cs create mode 100644 src/SMAPI.ModBuildConfig.Analyzer.Tests/NetFieldAnalyzerTests.cs create mode 100644 src/SMAPI.ModBuildConfig.Analyzer.Tests/ObsoleteFieldAnalyzerTests.cs delete mode 100644 src/SMAPI.ModBuildConfig.Analyzer.Tests/UnitTests.cs create mode 100644 src/SMAPI.ModBuildConfig.Analyzer/ObsoleteFieldAnalyzer.cs (limited to 'docs') diff --git a/docs/mod-build-config.md b/docs/mod-build-config.md index 00f9a356..99a567f2 100644 --- a/docs/mod-build-config.md +++ b/docs/mod-build-config.md @@ -194,6 +194,12 @@ field has an equivalent non-net property that avoids those issues. Suggested fix: access the suggested property name instead. +### SMAPI003 +**Avoid obsolete fields:** +> The '{{old field}}' field is obsolete and should be replaced with '{{new field}}'. + +Your code accesses a field which is obsolete or no longer works. Use the suggested field instead. + ## Troubleshoot ### "Failed to find the game install path" That error means the package couldn't find your game. You can specify the game path yourself; see diff --git a/src/SMAPI.ModBuildConfig.Analyzer.Tests/Mock/StardewValley/Farmer.cs b/src/SMAPI.ModBuildConfig.Analyzer.Tests/Mock/StardewValley/Farmer.cs new file mode 100644 index 00000000..e0f0e30c --- /dev/null +++ b/src/SMAPI.ModBuildConfig.Analyzer.Tests/Mock/StardewValley/Farmer.cs @@ -0,0 +1,11 @@ +// ReSharper disable CheckNamespace, InconsistentNaming -- matches Stardew Valley's code +using System.Collections.Generic; + +namespace StardewValley +{ + /// A simplified version of Stardew Valley's StardewValley.Farmer class for unit testing. + internal class Farmer + { + public IDictionary friendships; + } +} diff --git a/src/SMAPI.ModBuildConfig.Analyzer.Tests/NetFieldAnalyzerTests.cs b/src/SMAPI.ModBuildConfig.Analyzer.Tests/NetFieldAnalyzerTests.cs new file mode 100644 index 00000000..101f4c21 --- /dev/null +++ b/src/SMAPI.ModBuildConfig.Analyzer.Tests/NetFieldAnalyzerTests.cs @@ -0,0 +1,140 @@ +using Microsoft.CodeAnalysis; +using Microsoft.CodeAnalysis.Diagnostics; +using NUnit.Framework; +using SMAPI.ModBuildConfig.Analyzer.Tests.Framework; +using StardewModdingAPI.ModBuildConfig.Analyzer; + +namespace SMAPI.ModBuildConfig.Analyzer.Tests +{ + /// Unit tests for . + [TestFixture] + public class NetFieldAnalyzerTests : DiagnosticVerifier + { + /********* + ** Properties + *********/ + /// Sample C# mod code, with a {{test-code}} placeholder for the code in the Entry method to test. + const string SampleProgram = @" + using System; + using StardewValley; + using Netcode; + using SObject = StardewValley.Object; + + namespace SampleMod + { + class ModEntry + { + public void Entry() + { + {{test-code}} + } + } + } + "; + + /// The line number where the unit tested code is injected into . + private const int SampleCodeLine = 13; + + /// The column number where the unit tested code is injected into . + private const int SampleCodeColumn = 25; + + + /********* + ** Unit tests + *********/ + /// Test that no diagnostics are raised for an empty code block. + [TestCase] + public void EmptyCode_HasNoDiagnostics() + { + // arrange + string test = @""; + + // assert + this.VerifyCSharpDiagnostic(test); + } + + /// Test that the expected diagnostic message is raised for implicit net field comparisons. + /// The code line to test. + /// The column within the code line where the diagnostic message should be reported. + /// The expression which should be reported. + /// The source type name which should be reported. + /// The target type name which should be reported. + [TestCase("Item item = null; if (item.netIntField < 42);", 22, "item.netIntField", "NetInt", "int")] + [TestCase("Item item = null; if (item.netIntField <= 42);", 22, "item.netIntField", "NetInt", "int")] + [TestCase("Item item = null; if (item.netIntField > 42);", 22, "item.netIntField", "NetInt", "int")] + [TestCase("Item item = null; if (item.netIntField >= 42);", 22, "item.netIntField", "NetInt", "int")] + [TestCase("Item item = null; if (item.netIntField == 42);", 22, "item.netIntField", "NetInt", "int")] + [TestCase("Item item = null; if (item.netIntField != 42);", 22, "item.netIntField", "NetInt", "int")] + [TestCase("Item item = null; if (item?.netIntField != 42);", 22, "item?.netIntField", "NetInt", "int")] + [TestCase("Item item = null; if (item?.netIntField != null);", 22, "item?.netIntField", "NetInt", "object")] + [TestCase("Item item = null; if (item.netIntProperty < 42);", 22, "item.netIntProperty", "NetInt", "int")] + [TestCase("Item item = null; if (item.netIntProperty <= 42);", 22, "item.netIntProperty", "NetInt", "int")] + [TestCase("Item item = null; if (item.netIntProperty > 42);", 22, "item.netIntProperty", "NetInt", "int")] + [TestCase("Item item = null; if (item.netIntProperty >= 42);", 22, "item.netIntProperty", "NetInt", "int")] + [TestCase("Item item = null; if (item.netIntProperty == 42);", 22, "item.netIntProperty", "NetInt", "int")] + [TestCase("Item item = null; if (item.netIntProperty != 42);", 22, "item.netIntProperty", "NetInt", "int")] + [TestCase("Item item = null; if (item?.netIntProperty != 42);", 22, "item?.netIntProperty", "NetInt", "int")] + [TestCase("Item item = null; if (item?.netIntProperty != null);", 22, "item?.netIntProperty", "NetInt", "object")] + [TestCase("Item item = null; if (item.netRefField == null);", 22, "item.netRefField", "NetRef", "object")] + [TestCase("Item item = null; if (item.netRefField != null);", 22, "item.netRefField", "NetRef", "object")] + [TestCase("Item item = null; if (item.netRefProperty == null);", 22, "item.netRefProperty", "NetRef", "object")] + [TestCase("Item item = null; if (item.netRefProperty != null);", 22, "item.netRefProperty", "NetRef", "object")] + [TestCase("SObject obj = null; if (obj.netIntField != 42);", 24, "obj.netIntField", "NetInt", "int")] // ↓ same as above, but inherited from base class + [TestCase("SObject obj = null; if (obj.netIntProperty != 42);", 24, "obj.netIntProperty", "NetInt", "int")] + [TestCase("SObject obj = null; if (obj.netRefField == null);", 24, "obj.netRefField", "NetRef", "object")] + [TestCase("SObject obj = null; if (obj.netRefField != null);", 24, "obj.netRefField", "NetRef", "object")] + [TestCase("SObject obj = null; if (obj.netRefProperty == null);", 24, "obj.netRefProperty", "NetRef", "object")] + [TestCase("SObject obj = null; if (obj.netRefProperty != null);", 24, "obj.netRefProperty", "NetRef", "object")] + public void AvoidImplicitNetFieldComparisons_RaisesDiagnostic(string codeText, int column, string expression, string fromType, string toType) + { + // arrange + string code = NetFieldAnalyzerTests.SampleProgram.Replace("{{test-code}}", codeText); + DiagnosticResult expected = new DiagnosticResult + { + Id = "SMAPI001", + Message = $"This implicitly converts '{expression}' from {fromType} to {toType}, but {fromType} has unintuitive implicit conversion rules. Consider comparing against the actual value instead to avoid bugs. See https://smapi.io/buildmsg/smapi001 for details.", + Severity = DiagnosticSeverity.Warning, + Locations = new[] { new DiagnosticResultLocation("Test0.cs", NetFieldAnalyzerTests.SampleCodeLine, NetFieldAnalyzerTests.SampleCodeColumn + column) } + }; + + // assert + this.VerifyCSharpDiagnostic(code, expected); + } + + /// Test that the expected diagnostic message is raised for avoidable net field references. + /// The code line to test. + /// The column within the code line where the diagnostic message should be reported. + /// The expression which should be reported. + /// The net type name which should be reported. + /// The suggested property name which should be reported. + [TestCase("Item item = null; int category = item.category;", 33, "item.category", "NetInt", "Category")] + [TestCase("Item item = null; int category = (item).category;", 33, "(item).category", "NetInt", "Category")] + [TestCase("Item item = null; int category = ((Item)item).category;", 33, "((Item)item).category", "NetInt", "Category")] + [TestCase("SObject obj = null; int category = obj.category;", 35, "obj.category", "NetInt", "Category")] + public void AvoidNetFields_RaisesDiagnostic(string codeText, int column, string expression, string netType, string suggestedProperty) + { + // arrange + string code = NetFieldAnalyzerTests.SampleProgram.Replace("{{test-code}}", codeText); + DiagnosticResult expected = new DiagnosticResult + { + Id = "SMAPI002", + Message = $"'{expression}' is a {netType} field; consider using the {suggestedProperty} property instead. See https://smapi.io/buildmsg/smapi002 for details.", + Severity = DiagnosticSeverity.Warning, + Locations = new[] { new DiagnosticResultLocation("Test0.cs", NetFieldAnalyzerTests.SampleCodeLine, NetFieldAnalyzerTests.SampleCodeColumn + column) } + }; + + // assert + this.VerifyCSharpDiagnostic(code, expected); + } + + + /********* + ** Helpers + *********/ + /// Get the analyzer being tested. + protected override DiagnosticAnalyzer GetCSharpDiagnosticAnalyzer() + { + return new NetFieldAnalyzer(); + } + } +} diff --git a/src/SMAPI.ModBuildConfig.Analyzer.Tests/ObsoleteFieldAnalyzerTests.cs b/src/SMAPI.ModBuildConfig.Analyzer.Tests/ObsoleteFieldAnalyzerTests.cs new file mode 100644 index 00000000..dc7476ef --- /dev/null +++ b/src/SMAPI.ModBuildConfig.Analyzer.Tests/ObsoleteFieldAnalyzerTests.cs @@ -0,0 +1,88 @@ +using Microsoft.CodeAnalysis; +using Microsoft.CodeAnalysis.Diagnostics; +using NUnit.Framework; +using SMAPI.ModBuildConfig.Analyzer.Tests.Framework; +using StardewModdingAPI.ModBuildConfig.Analyzer; + +namespace SMAPI.ModBuildConfig.Analyzer.Tests +{ + /// Unit tests for . + [TestFixture] + public class ObsoleteFieldAnalyzerTests : DiagnosticVerifier + { + /********* + ** Properties + *********/ + /// Sample C# mod code, with a {{test-code}} placeholder for the code in the Entry method to test. + const string SampleProgram = @" + using System; + using StardewValley; + using Netcode; + using SObject = StardewValley.Object; + + namespace SampleMod + { + class ModEntry + { + public void Entry() + { + {{test-code}} + } + } + } + "; + + /// The line number where the unit tested code is injected into . + private const int SampleCodeLine = 13; + + /// The column number where the unit tested code is injected into . + private const int SampleCodeColumn = 25; + + + /********* + ** Unit tests + *********/ + /// Test that no diagnostics are raised for an empty code block. + [TestCase] + public void EmptyCode_HasNoDiagnostics() + { + // arrange + string test = @""; + + // assert + this.VerifyCSharpDiagnostic(test); + } + + /// Test that the expected diagnostic message is raised for an obsolete field reference. + /// The code line to test. + /// The column within the code line where the diagnostic message should be reported. + /// The old field name which should be reported. + /// The new field name which should be reported. + [TestCase("var x = new Farmer().friendships;", 8, "StardewValley.Farmer.friendships", "friendshipData")] + public void AvoidObsoleteField_RaisesDiagnostic(string codeText, int column, string oldName, string newName) + { + // arrange + string code = ObsoleteFieldAnalyzerTests.SampleProgram.Replace("{{test-code}}", codeText); + DiagnosticResult expected = new DiagnosticResult + { + Id = "SMAPI003", + Message = $"The '{oldName}' field is obsolete and should be replaced with '{newName}'. See https://smapi.io/buildmsg/smapi003 for details.", + Severity = DiagnosticSeverity.Warning, + Locations = new[] { new DiagnosticResultLocation("Test0.cs", ObsoleteFieldAnalyzerTests.SampleCodeLine, ObsoleteFieldAnalyzerTests.SampleCodeColumn + column) } + }; + + // assert + this.VerifyCSharpDiagnostic(code, expected); + } + + + /********* + ** Helpers + *********/ + /// Get the analyzer being tested. + protected override DiagnosticAnalyzer GetCSharpDiagnosticAnalyzer() + { + return new ObsoleteFieldAnalyzer(); + } + } +} diff --git a/src/SMAPI.ModBuildConfig.Analyzer.Tests/UnitTests.cs b/src/SMAPI.ModBuildConfig.Analyzer.Tests/UnitTests.cs deleted file mode 100644 index 8ca27847..00000000 --- a/src/SMAPI.ModBuildConfig.Analyzer.Tests/UnitTests.cs +++ /dev/null @@ -1,140 +0,0 @@ -using Microsoft.CodeAnalysis; -using Microsoft.CodeAnalysis.Diagnostics; -using NUnit.Framework; -using SMAPI.ModBuildConfig.Analyzer.Tests.Framework; -using StardewModdingAPI.ModBuildConfig.Analyzer; - -namespace SMAPI.ModBuildConfig.Analyzer.Tests -{ - /// Unit tests for the C# analyzers. - [TestFixture] - public class UnitTests : DiagnosticVerifier - { - /********* - ** Properties - *********/ - /// Sample C# code which contains a simplified representation of Stardew Valley's Netcode types, and sample mod code with a {{test-code}} placeholder for the code being tested. - const string SampleProgram = @" - using System; - using StardewValley; - using Netcode; - using SObject = StardewValley.Object; - - namespace SampleMod - { - class ModEntry - { - public void Entry() - { - {{test-code}} - } - } - } - "; - - /// The line number where the unit tested code is injected into . - private const int SampleCodeLine = 13; - - /// The column number where the unit tested code is injected into . - private const int SampleCodeColumn = 25; - - - /********* - ** Unit tests - *********/ - /// Test that no diagnostics are raised for an empty code block. - [TestCase] - public void EmptyCode_HasNoDiagnostics() - { - // arrange - string test = @""; - - // assert - this.VerifyCSharpDiagnostic(test); - } - - /// Test that the expected diagnostic message is raised for implicit net field comparisons. - /// The code line to test. - /// The column within the code line where the diagnostic message should be reported. - /// The expression which should be reported. - /// The source type name which should be reported. - /// The target type name which should be reported. - [TestCase("Item item = null; if (item.netIntField < 42);", 22, "item.netIntField", "NetInt", "int")] - [TestCase("Item item = null; if (item.netIntField <= 42);", 22, "item.netIntField", "NetInt", "int")] - [TestCase("Item item = null; if (item.netIntField > 42);", 22, "item.netIntField", "NetInt", "int")] - [TestCase("Item item = null; if (item.netIntField >= 42);", 22, "item.netIntField", "NetInt", "int")] - [TestCase("Item item = null; if (item.netIntField == 42);", 22, "item.netIntField", "NetInt", "int")] - [TestCase("Item item = null; if (item.netIntField != 42);", 22, "item.netIntField", "NetInt", "int")] - [TestCase("Item item = null; if (item?.netIntField != 42);", 22, "item?.netIntField", "NetInt", "int")] - [TestCase("Item item = null; if (item?.netIntField != null);", 22, "item?.netIntField", "NetInt", "object")] - [TestCase("Item item = null; if (item.netIntProperty < 42);", 22, "item.netIntProperty", "NetInt", "int")] - [TestCase("Item item = null; if (item.netIntProperty <= 42);", 22, "item.netIntProperty", "NetInt", "int")] - [TestCase("Item item = null; if (item.netIntProperty > 42);", 22, "item.netIntProperty", "NetInt", "int")] - [TestCase("Item item = null; if (item.netIntProperty >= 42);", 22, "item.netIntProperty", "NetInt", "int")] - [TestCase("Item item = null; if (item.netIntProperty == 42);", 22, "item.netIntProperty", "NetInt", "int")] - [TestCase("Item item = null; if (item.netIntProperty != 42);", 22, "item.netIntProperty", "NetInt", "int")] - [TestCase("Item item = null; if (item?.netIntProperty != 42);", 22, "item?.netIntProperty", "NetInt", "int")] - [TestCase("Item item = null; if (item?.netIntProperty != null);", 22, "item?.netIntProperty", "NetInt", "object")] - [TestCase("Item item = null; if (item.netRefField == null);", 22, "item.netRefField", "NetRef", "object")] - [TestCase("Item item = null; if (item.netRefField != null);", 22, "item.netRefField", "NetRef", "object")] - [TestCase("Item item = null; if (item.netRefProperty == null);", 22, "item.netRefProperty", "NetRef", "object")] - [TestCase("Item item = null; if (item.netRefProperty != null);", 22, "item.netRefProperty", "NetRef", "object")] - [TestCase("SObject obj = null; if (obj.netIntField != 42);", 24, "obj.netIntField", "NetInt", "int")] // ↓ same as above, but inherited from base class - [TestCase("SObject obj = null; if (obj.netIntProperty != 42);", 24, "obj.netIntProperty", "NetInt", "int")] - [TestCase("SObject obj = null; if (obj.netRefField == null);", 24, "obj.netRefField", "NetRef", "object")] - [TestCase("SObject obj = null; if (obj.netRefField != null);", 24, "obj.netRefField", "NetRef", "object")] - [TestCase("SObject obj = null; if (obj.netRefProperty == null);", 24, "obj.netRefProperty", "NetRef", "object")] - [TestCase("SObject obj = null; if (obj.netRefProperty != null);", 24, "obj.netRefProperty", "NetRef", "object")] - public void AvoidImplicitNetFieldComparisons_RaisesDiagnostic(string codeText, int column, string expression, string fromType, string toType) - { - // arrange - string code = UnitTests.SampleProgram.Replace("{{test-code}}", codeText); - DiagnosticResult expected = new DiagnosticResult - { - Id = "SMAPI001", - Message = $"This implicitly converts '{expression}' from {fromType} to {toType}, but {fromType} has unintuitive implicit conversion rules. Consider comparing against the actual value instead to avoid bugs. See https://smapi.io/buildmsg/smapi001 for details.", - Severity = DiagnosticSeverity.Warning, - Locations = new[] { new DiagnosticResultLocation("Test0.cs", UnitTests.SampleCodeLine, UnitTests.SampleCodeColumn + column) } - }; - - // assert - this.VerifyCSharpDiagnostic(code, expected); - } - - /// Test that the expected diagnostic message is raised for avoidable net field references. - /// The code line to test. - /// The column within the code line where the diagnostic message should be reported. - /// The expression which should be reported. - /// The net type name which should be reported. - /// The suggested property name which should be reported. - [TestCase("Item item = null; int category = item.category;", 33, "item.category", "NetInt", "Category")] - [TestCase("Item item = null; int category = (item).category;", 33, "(item).category", "NetInt", "Category")] - [TestCase("Item item = null; int category = ((Item)item).category;", 33, "((Item)item).category", "NetInt", "Category")] - [TestCase("SObject obj = null; int category = obj.category;", 35, "obj.category", "NetInt", "Category")] - public void AvoidNetFields_RaisesDiagnostic(string codeText, int column, string expression, string netType, string suggestedProperty) - { - // arrange - string code = UnitTests.SampleProgram.Replace("{{test-code}}", codeText); - DiagnosticResult expected = new DiagnosticResult - { - Id = "SMAPI002", - Message = $"'{expression}' is a {netType} field; consider using the {suggestedProperty} property instead. See https://smapi.io/buildmsg/smapi002 for details.", - Severity = DiagnosticSeverity.Warning, - Locations = new[] { new DiagnosticResultLocation("Test0.cs", UnitTests.SampleCodeLine, UnitTests.SampleCodeColumn + column) } - }; - - // assert - this.VerifyCSharpDiagnostic(code, expected); - } - - - /********* - ** Helpers - *********/ - /// Get the analyzer being tested. - protected override DiagnosticAnalyzer GetCSharpDiagnosticAnalyzer() - { - return new NetFieldAnalyzer(); - } - } -} diff --git a/src/SMAPI.ModBuildConfig.Analyzer/ObsoleteFieldAnalyzer.cs b/src/SMAPI.ModBuildConfig.Analyzer/ObsoleteFieldAnalyzer.cs new file mode 100644 index 00000000..00565329 --- /dev/null +++ b/src/SMAPI.ModBuildConfig.Analyzer/ObsoleteFieldAnalyzer.cs @@ -0,0 +1,98 @@ +using System; +using System.Collections.Generic; +using System.Collections.Immutable; +using Microsoft.CodeAnalysis; +using Microsoft.CodeAnalysis.CSharp; +using Microsoft.CodeAnalysis.CSharp.Syntax; +using Microsoft.CodeAnalysis.Diagnostics; + +namespace StardewModdingAPI.ModBuildConfig.Analyzer +{ + /// Detects references to a field which has been replaced. + [DiagnosticAnalyzer(LanguageNames.CSharp)] + public class ObsoleteFieldAnalyzer : DiagnosticAnalyzer + { + /********* + ** Properties + *********/ + /// Maps obsolete fields/properties to their non-obsolete equivalent. + private readonly IDictionary ReplacedFields = new Dictionary + { + // Farmer + ["StardewValley.Farmer::friendships"] = "friendshipData" + }; + + /// Describes the diagnostic rule covered by the analyzer. + private readonly IDictionary Rules = new Dictionary + { + ["SMAPI003"] = new DiagnosticDescriptor( + id: "SMAPI003", + title: "Reference to obsolete field", + messageFormat: "The '{0}' field is obsolete and should be replaced with '{1}'. See https://smapi.io/buildmsg/smapi003 for details.", + category: "SMAPI.CommonErrors", + defaultSeverity: DiagnosticSeverity.Warning, + isEnabledByDefault: true, + helpLinkUri: "https://smapi.io/buildmsg/smapi003" + ) + }; + + + /********* + ** Accessors + *********/ + /// The descriptors for the diagnostics that this analyzer is capable of producing. + public override ImmutableArray SupportedDiagnostics { get; } + + + /********* + ** Public methods + *********/ + /// Construct an instance. + public ObsoleteFieldAnalyzer() + { + this.SupportedDiagnostics = ImmutableArray.CreateRange(this.Rules.Values); + } + + /// Called once at session start to register actions in the analysis context. + /// The analysis context. + public override void Initialize(AnalysisContext context) + { + // SMAPI003: avoid obsolete fields + context.RegisterSyntaxNodeAction( + this.AnalyzeObsoleteFields, + SyntaxKind.SimpleMemberAccessExpression + ); + } + + + /********* + ** Private methods + *********/ + /// Analyse a syntax node and add a diagnostic message if it references an obsolete field. + /// The analysis context. + private void AnalyzeObsoleteFields(SyntaxNodeAnalysisContext context) + { + try + { + // get reference info + MemberAccessExpressionSyntax node = (MemberAccessExpressionSyntax)context.Node; + ITypeSymbol declaringType = context.SemanticModel.GetTypeInfo(node.Expression).Type; + string propertyName = node.Name.Identifier.Text; + + // suggest replacement + for (ITypeSymbol type = declaringType; type != null; type = type.BaseType) + { + if (this.ReplacedFields.TryGetValue($"{type}::{propertyName}", out string replacement)) + { + context.ReportDiagnostic(Diagnostic.Create(this.Rules["SMAPI003"], context.Node.GetLocation(), $"{type}.{propertyName}", replacement)); + break; + } + } + } + catch (Exception ex) + { + throw new InvalidOperationException($"Failed processing expression: '{context.Node}'. Exception details: {ex.ToString().Replace('\r', ' ').Replace('\n', ' ')}"); + } + } + } +} -- cgit From b74b530a8acd850a0fa04f24347b91f02be7cbb6 Mon Sep 17 00:00:00 2001 From: Jesse Plamondon-Willard Date: Tue, 10 Apr 2018 18:25:57 -0400 Subject: add code analyzers to release notes --- docs/release-notes.md | 3 ++- 1 file changed, 2 insertions(+), 1 deletion(-) (limited to 'docs') diff --git a/docs/release-notes.md b/docs/release-notes.md index 3406e735..030af3ed 100644 --- a/docs/release-notes.md +++ b/docs/release-notes.md @@ -7,8 +7,9 @@ * Fixed SMAPI update checks not showing newer beta versions when using a beta version. * For modders: - * Dropped some deprecated APIs. + * Added code analysis to mod build config package to flag common issues as warnings. * Fixed some assets not being editable. + * Dropped some deprecated APIs. * For SMAPI developers: * Added prerelease versions to the mod update-check API response where available (GitHub only). -- cgit From fa335f80beee17f17ecedcfa7bcb6f9967a39776 Mon Sep 17 00:00:00 2001 From: Jesse Plamondon-Willard Date: Wed, 11 Apr 2018 15:41:32 -0400 Subject: fix crash when player has duplicate item references --- docs/release-notes.md | 3 ++- src/SMAPI/Framework/SGame.cs | 2 +- 2 files changed, 3 insertions(+), 2 deletions(-) (limited to 'docs') diff --git a/docs/release-notes.md b/docs/release-notes.md index c65d6e0d..66d911b7 100644 --- a/docs/release-notes.md +++ b/docs/release-notes.md @@ -4,7 +4,6 @@ * For players: * Added support for Stardew Valley 1.3+; no longer compatible with earlier versions. * Added `Context.IsMultiplayer` and `Context.IsMainPlayer` flags. - * Fixed SMAPI update alerts linking to the GitHub repository instead of [smapi.io](https://smapi.io). * Fixed SMAPI update checks not showing newer beta versions when using a beta version. * For modders: @@ -21,6 +20,8 @@ ## 2.5.5 * For players: * Fixed mods not being loaded if an optional dependency is installed but skipped. + * Fixed rare crash when the game duplicates an item. + * Fixed SMAPI update alerts linking to the GitHub repository instead of [smapi.io](https://smapi.io). * For the [log parser][]: * Tweaked UI. diff --git a/src/SMAPI/Framework/SGame.cs b/src/SMAPI/Framework/SGame.cs index ae702711..9af02dd4 100644 --- a/src/SMAPI/Framework/SGame.cs +++ b/src/SMAPI/Framework/SGame.cs @@ -569,7 +569,7 @@ namespace StardewModdingAPI.Framework this.PreviousForagingLevel = Game1.player.foragingLevel; this.PreviousMiningLevel = Game1.player.miningLevel; this.PreviousLuckLevel = Game1.player.luckLevel; - this.PreviousItems = Game1.player.items.Where(n => n != null).ToDictionary(n => n, n => n.Stack); + this.PreviousItems = Game1.player.items.Where(n => n != null).Distinct().ToDictionary(n => n, n => n.Stack); this.PreviousLocationObjects = this.GetHash(Game1.currentLocation.objects); this.PreviousTime = Game1.timeOfDay; this.PreviousMineLevel = Game1.mine?.mineLevel ?? 0; -- cgit From e0488fa5b2a0a5236a7a034d6712392eb8ed1918 Mon Sep 17 00:00:00 2001 From: Jesse Plamondon-Willard Date: Wed, 11 Apr 2018 16:08:58 -0400 Subject: fix error when a remote mod version is invalid (#462) --- docs/release-notes.md | 5 +++-- src/SMAPI/Program.cs | 2 +- 2 files changed, 4 insertions(+), 3 deletions(-) (limited to 'docs') diff --git a/docs/release-notes.md b/docs/release-notes.md index 66d911b7..1a28af70 100644 --- a/docs/release-notes.md +++ b/docs/release-notes.md @@ -19,9 +19,10 @@ ## 2.5.5 * For players: - * Fixed mods not being loaded if an optional dependency is installed but skipped. - * Fixed rare crash when the game duplicates an item. + * Fixed mod not loaded if it has an optional dependency that's loaded but skipped. + * Fixed mod update alerts not shown if one mod has an invalid remote version. * Fixed SMAPI update alerts linking to the GitHub repository instead of [smapi.io](https://smapi.io). + * Fixed rare crash if the game duplicates an item. * For the [log parser][]: * Tweaked UI. diff --git a/src/SMAPI/Program.cs b/src/SMAPI/Program.cs index 1b8cb2ba..5a1f7409 100644 --- a/src/SMAPI/Program.cs +++ b/src/SMAPI/Program.cs @@ -629,7 +629,7 @@ namespace StardewModdingAPI // compare versions bool isUpdate = remoteVersion.IsNewerThan(localVersion); - this.VerboseLog($" {mod.DisplayName} ({result.Key}): {(isUpdate ? $"{mod.Manifest.Version}{(!localVersion.Equals(mod.Manifest.Version) ? $" [{localVersion}]" : "")} => {remoteInfo.Version}{(!remoteVersion.Equals(new SemanticVersion(remoteInfo.Version)) ? $" [{remoteVersion}]" : "")}" : "okay")}."); + this.VerboseLog($" {mod.DisplayName} ({result.Key}): {(isUpdate ? $"{mod.Manifest.Version}{(!localVersion.Equals(mod.Manifest.Version) ? $" [{localVersion}]" : "")} => {remoteInfo.Version}" : "okay")}."); if (isUpdate) { if (!updatesByMod.TryGetValue(mod, out ModInfoModel other) || remoteVersion.IsNewerThan(other.Version)) -- cgit From e4222ad1fd5b32ca81ee55e025b4e421582cb2d3 Mon Sep 17 00:00:00 2001 From: Jesse Plamondon-Willard Date: Wed, 11 Apr 2018 16:17:23 -0400 Subject: fix error when two content packs use different capitalisation for the same required mod ID (#469) --- docs/release-notes.md | 1 + src/SMAPI/Program.cs | 2 +- 2 files changed, 2 insertions(+), 1 deletion(-) (limited to 'docs') diff --git a/docs/release-notes.md b/docs/release-notes.md index 1a28af70..a5bd205f 100644 --- a/docs/release-notes.md +++ b/docs/release-notes.md @@ -22,6 +22,7 @@ * Fixed mod not loaded if it has an optional dependency that's loaded but skipped. * Fixed mod update alerts not shown if one mod has an invalid remote version. * Fixed SMAPI update alerts linking to the GitHub repository instead of [smapi.io](https://smapi.io). + * Fixed error when two content packs use different capitalisation for the same required mod ID. * Fixed rare crash if the game duplicates an item. * For the [log parser][]: diff --git a/src/SMAPI/Program.cs b/src/SMAPI/Program.cs index 5a1f7409..ff4e9a50 100644 --- a/src/SMAPI/Program.cs +++ b/src/SMAPI/Program.cs @@ -736,7 +736,7 @@ namespace StardewModdingAPI // get content packs by mod ID IDictionary contentPacksByModID = loadedContentPacks - .GroupBy(p => p.Manifest.ContentPackFor.UniqueID) + .GroupBy(p => p.Manifest.ContentPackFor.UniqueID, StringComparer.InvariantCultureIgnoreCase) .ToDictionary( group => group.Key, group => group.Select(metadata => metadata.ContentPack).ToArray(), -- cgit From 2d47e479a5e48a575db3ca9998653c0435419440 Mon Sep 17 00:00:00 2001 From: Jesse Plamondon-Willard Date: Wed, 11 Apr 2018 19:55:01 -0400 Subject: fix draft releases being detected as update candidates --- docs/release-notes.md | 1 + src/SMAPI.Web/Framework/Clients/GitHub/GitHubClient.cs | 2 +- src/SMAPI.Web/Framework/Clients/GitHub/GitRelease.cs | 4 ++++ src/SMAPI.Web/appsettings.json | 2 +- 4 files changed, 7 insertions(+), 2 deletions(-) (limited to 'docs') diff --git a/docs/release-notes.md b/docs/release-notes.md index a5bd205f..e68720da 100644 --- a/docs/release-notes.md +++ b/docs/release-notes.md @@ -22,6 +22,7 @@ * Fixed mod not loaded if it has an optional dependency that's loaded but skipped. * Fixed mod update alerts not shown if one mod has an invalid remote version. * Fixed SMAPI update alerts linking to the GitHub repository instead of [smapi.io](https://smapi.io). + * Fixed SMAPI update alerts for draft releases. * Fixed error when two content packs use different capitalisation for the same required mod ID. * Fixed rare crash if the game duplicates an item. diff --git a/src/SMAPI.Web/Framework/Clients/GitHub/GitHubClient.cs b/src/SMAPI.Web/Framework/Clients/GitHub/GitHubClient.cs index 4abe0737..2cfc6903 100644 --- a/src/SMAPI.Web/Framework/Clients/GitHub/GitHubClient.cs +++ b/src/SMAPI.Web/Framework/Clients/GitHub/GitHubClient.cs @@ -59,7 +59,7 @@ namespace StardewModdingAPI.Web.Framework.Clients.GitHub GitRelease[] results = await this.Client .GetAsync(string.Format(this.AnyReleaseUrlFormat, repo)) .AsArray(); - return results.FirstOrDefault(); + return results.FirstOrDefault(p => !p.IsDraft); } return await this.Client diff --git a/src/SMAPI.Web/Framework/Clients/GitHub/GitRelease.cs b/src/SMAPI.Web/Framework/Clients/GitHub/GitRelease.cs index 827374fb..d0db5297 100644 --- a/src/SMAPI.Web/Framework/Clients/GitHub/GitRelease.cs +++ b/src/SMAPI.Web/Framework/Clients/GitHub/GitRelease.cs @@ -19,6 +19,10 @@ namespace StardewModdingAPI.Web.Framework.Clients.GitHub /// The Markdown description for the release. public string Body { get; set; } + /// Whether this is a draft version. + [JsonProperty("draft")] + public bool IsDraft { get; set; } + /// Whether this is a prerelease version. [JsonProperty("prerelease")] public bool IsPrerelease { get; set; } diff --git a/src/SMAPI.Web/appsettings.json b/src/SMAPI.Web/appsettings.json index 03ca31ed..095707a8 100644 --- a/src/SMAPI.Web/appsettings.json +++ b/src/SMAPI.Web/appsettings.json @@ -25,7 +25,7 @@ "GitHubBaseUrl": "https://api.github.com", "GitHubStableReleaseUrlFormat": "repos/{0}/releases/latest", - "GitHubAnyReleaseUrlFormat": "repos/{0}/releases?per_page=1", + "GitHubAnyReleaseUrlFormat": "repos/{0}/releases?per_page=2", // allow for draft release (only visible if GitHub repo is owned by same account as the update check credentials) "GitHubAcceptHeader": "application/vnd.github.v3+json", "GitHubUsername": null, // see top note "GitHubPassword": null, // see top note -- cgit