Skip to content

Commit

Permalink
merge invalid-member rewrite finders
Browse files Browse the repository at this point in the history
This fixes redundant messages for members with mismatched types. For example:
```
reference to StardewValley.Event.id (no such field)
reference to StardewValley.Event.id (field returns string, not int)
```
  • Loading branch information
Pathoschild committed Feb 22, 2024
1 parent 3c7b8fb commit 720ebf2
Show file tree
Hide file tree
Showing 4 changed files with 43 additions and 128 deletions.
10 changes: 3 additions & 7 deletions docs/release-notes.md
Original file line number Diff line number Diff line change
Expand Up @@ -4,6 +4,7 @@
## Upcoming release for Stardew Valley 1.6
* For players:
* Updated for Stardew Valley 1.6.
* Added support for overriding SMAPI configuration per `Mods` folder (thanks to Shockah!).
* Improved performance.
* Improved compatibility rewriting to handle more cases (thanks to SinZ!).
* Removed the bundled `ErrorHandler` mod (now integrated into Stardew Valley 1.6).
Expand All @@ -14,16 +15,11 @@
* For mod authors:
* Updated to .NET 6.
* Added `RenderingStep` and `RenderedStep` events, which let you handle a specific step in the game's render cycle.
* Added support for [custom update manifests](https://stardewvalleywiki.com/Modding:Modder_Guide/APIs/Update_checks#Custom_update_manifest) (thanks to Jamie Taylor!).
* Removed all deprecated APIs.
* SMAPI no longer intercepts output written to the console. Mods which directly access `Console` will be listed under mod warnings.
* Calling `Monitor.VerboseLog` with an interpolated string no longer evaluates the string if verbose mode is disabled (thanks to atravita!). This only applies to mods compiled in SMAPI 4.0.0 or later.

## Upcoming release
* For players:
* Added support for overriding SMAPI configuration per `Mods` folder (thanks to Shockah!).

* For mod authors:
* Added support for [custom update manifests](https://stardewvalleywiki.com/Modding:Modder_Guide/APIs/Update_checks#Custom_update_manifest) (thanks to Jamie Taylor!).
* Fixed redundant `TRACE` logs for a broken mod which references members with the wrong types.

* For the web UI:
* Fixed uploaded log/JSON file expiry alway shown as renewed.
Expand Down
Original file line number Diff line number Diff line change
Expand Up @@ -7,9 +7,9 @@

namespace StardewModdingAPI.Framework.ModLoading.Finders
{
/// <summary>Finds references to a field, property, or method which returns a different type than the code expects.</summary>
/// <summary>Finds references to a field, property, or method which either doesn't exist or returns a different type than the code expects.</summary>
/// <remarks>This implementation is purely heuristic. It should never return a false positive, but won't detect all cases.</remarks>
internal class ReferenceToMemberWithUnexpectedTypeFinder : BaseInstructionHandler
internal class ReferenceToInvalidMemberFinder : BaseInstructionHandler
{
/*********
** Fields
Expand All @@ -23,7 +23,7 @@ internal class ReferenceToMemberWithUnexpectedTypeFinder : BaseInstructionHandle
*********/
/// <summary>Construct an instance.</summary>
/// <param name="validateReferencesToAssemblies">The assembly names to which to heuristically detect broken references.</param>
public ReferenceToMemberWithUnexpectedTypeFinder(ISet<string> validateReferencesToAssemblies)
public ReferenceToInvalidMemberFinder(ISet<string> validateReferencesToAssemblies)
: base(defaultPhrase: "")
{
this.ValidateReferencesToAssemblies = validateReferencesToAssemblies;
Expand All @@ -36,37 +36,45 @@ public override bool Handle(ModuleDefinition module, ILProcessor cil, Instructio
FieldReference? fieldRef = RewriteHelper.AsFieldReference(instruction);
if (fieldRef != null && this.ShouldValidate(fieldRef.DeclaringType))
{
// get target field
FieldDefinition? targetField = fieldRef.DeclaringType.Resolve()?.Fields.FirstOrDefault(p => p.Name == fieldRef.Name);
if (targetField == null)
return false;

// validate return type
if (!RewriteHelper.LooksLikeSameType(fieldRef.FieldType, targetField.FieldType))
{
// wrong return type
if (targetField != null && !RewriteHelper.LooksLikeSameType(fieldRef.FieldType, targetField.FieldType))
this.MarkFlag(InstructionHandleResult.NotCompatible, $"reference to {fieldRef.DeclaringType.FullName}.{fieldRef.Name} (field returns {this.GetFriendlyTypeName(targetField.FieldType)}, not {this.GetFriendlyTypeName(fieldRef.FieldType)})");
return false;
}

// missing
else if (targetField == null || targetField.HasConstant || !RewriteHelper.HasSameNamespaceAndName(fieldRef.DeclaringType, targetField.DeclaringType))
this.MarkFlag(InstructionHandleResult.NotCompatible, $"reference to {fieldRef.DeclaringType.FullName}.{fieldRef.Name} (no such field)");

return false;
}

// method reference
MethodReference? methodReference = RewriteHelper.AsMethodReference(instruction);
if (methodReference != null && !this.IsUnsupported(methodReference) && this.ShouldValidate(methodReference.DeclaringType))
MethodReference? methodRef = RewriteHelper.AsMethodReference(instruction);
if (methodRef != null && !this.IsUnsupported(methodRef))
{
// get potential targets
MethodDefinition[]? candidateMethods = methodReference.DeclaringType.Resolve()?.Methods.Where(found => found.Name == methodReference.Name).ToArray();
if (candidateMethods == null || !candidateMethods.Any())
return false;
MethodDefinition? methodDef = methodRef.Resolve();

// compare return types
MethodDefinition? methodDef = methodReference.Resolve();
if (methodDef == null)
return false; // validated by ReferenceToMissingMemberFinder
// wrong return type
if (methodDef != null && this.ShouldValidate(methodRef.DeclaringType))
{
MethodDefinition[]? candidateMethods = methodRef.DeclaringType.Resolve()?.Methods.Where(found => found.Name == methodRef.Name).ToArray();
if (candidateMethods?.Any() is true && candidateMethods.All(method => !RewriteHelper.LooksLikeSameType(method.ReturnType, methodDef.ReturnType)))
this.MarkFlag(InstructionHandleResult.NotCompatible, $"reference to {methodDef.DeclaringType.FullName}.{methodDef.Name} (no such method returns {this.GetFriendlyTypeName(methodDef.ReturnType)})");
}

if (candidateMethods.All(method => !RewriteHelper.LooksLikeSameType(method.ReturnType, methodDef.ReturnType)))
// missing
else if (methodDef is null)
{
this.MarkFlag(InstructionHandleResult.NotCompatible, $"reference to {methodDef.DeclaringType.FullName}.{methodDef.Name} (no such method returns {this.GetFriendlyTypeName(methodDef.ReturnType)})");
return false;
string phrase;
if (this.IsProperty(methodRef))
phrase = $"reference to {methodRef.DeclaringType.FullName}.{methodRef.Name.Substring(4)} (no such property)";
else if (methodRef.Name == ".ctor")
phrase = $"reference to {methodRef.DeclaringType.FullName}.{methodRef.Name} (no matching constructor)";
else
phrase = $"reference to {methodRef.DeclaringType.FullName}.{methodRef.Name} (no such method)";

this.MarkFlag(InstructionHandleResult.NotCompatible, phrase);
}
}

Expand Down Expand Up @@ -116,5 +124,12 @@ private string GetFriendlyTypeName(TypeReference type)

return type.FullName;
}

/// <summary>Get whether a method reference is a property getter or setter.</summary>
/// <param name="method">The method reference.</param>
private bool IsProperty(MethodReference method)
{
return method.Name.StartsWith("get_") || method.Name.StartsWith("set_");
}
}
}

This file was deleted.

3 changes: 1 addition & 2 deletions src/SMAPI/Metadata/InstructionMetadata.cs
Original file line number Diff line number Diff line change
Expand Up @@ -240,8 +240,7 @@ public IEnumerable<IInstructionHandler> GetHandlers(bool paranoidMode, bool rewr
** detect mod issues
****/
// broken code
yield return new ReferenceToMissingMemberFinder(this.ValidateReferencesToAssemblies);
yield return new ReferenceToMemberWithUnexpectedTypeFinder(this.ValidateReferencesToAssemblies);
yield return new ReferenceToInvalidMemberFinder(this.ValidateReferencesToAssemblies);

// code which may impact game stability
yield return new FieldFinder(typeof(SaveGame).FullName!, new[] { nameof(SaveGame.serializer), nameof(SaveGame.farmerSerializer), nameof(SaveGame.locationSerializer) }, InstructionHandleResult.DetectedSaveSerializer);
Expand Down

0 comments on commit 720ebf2

Please sign in to comment.