-Reports enum constants that are not properly prefixed by the mod id + Reports enum constants that are not properly prefixed by the mod id +
+ ++ Enum constants added by mixins share the target enum with constants added by every other mod. Prefixing their names with the + mod id avoids conflicts between mods that add constants with the same name. The required pattern can be configured. +
+
+@Mixin(Color.class)
+enum ColorMixin {
+ PURPLE, // bad
+ MY_MOD_PURPLE // good
+}
+-Reports added members not matching the correct name format + Reports added members not matching the correct name format +
+ +
+ Members added by mixins share the target class with members added by every other mod. Prefixing their names with the mod id
+ avoids conflicts between mods that add members with the same name. The required pattern and which members are reported can
+ be configured. This should be done when @Unique can't be used.
+
+@Override public void doSomething() { ... } // bad
+@Override public void myMod$doSomething() { ... } // good
+-Reports ambiguous references in Mixin annotations + Reports ambiguous references in Mixin annotations +
+ +
+ When a method name matches several overloads in the target class, Mixin targets all the overloads, which is often
+ not intended. Specify the descriptor to target a specific overload, or add the * wildcard to explicitly
+ target all of them.
+
+@Inject(method = "add", ...) // bad
+@Inject(method = "add(Ljava/lang/String;)V", ...) // good: one overload
+@Inject(method = "add*", ...) // if all overloads are intended
+
+ Reports Mixin annotations outside of @Mixin classes
+
+ Mixin only processes these annotations in mixin classes. Anywhere else, they are ignored. +
Index: src/main/resources/inspectionDescriptions/AtUsage.html =================================================================== --- src/main/resources/inspectionDescriptions/AtUsage.html (revision ea0444f7c6e18e3b6038b83b6030405830f0b7b9) +++ src/main/resources/inspectionDescriptions/AtUsage.html (revision cf5ea915cc6073030781b3a37e698e8b65302537) @@ -20,6 +20,13 @@ +-The declared access transformer is never used + The declared access transformer is never used +
+ ++ An unused entry changes the access of a Minecraft member for no reason. Unused entries are often left behind when the code that + needed them is removed, or the member is renamed in a new version of Minecraft. +
Index: src/main/resources/inspectionDescriptions/BukkitListenerImplemented.html =================================================================== --- src/main/resources/inspectionDescriptions/BukkitListenerImplemented.html (revision ea0444f7c6e18e3b6038b83b6030405830f0b7b9) +++ src/main/resources/inspectionDescriptions/BukkitListenerImplemented.html (revision cf5ea915cc6073030781b3a37e698e8b65302537) @@ -20,6 +20,13 @@ -All Bukkit @EventHandler methods must reside in a class that implements Listener. +
+ All Bukkit @EventHandler methods must reside in a class that implements Listener.
+
+ Bukkit only registers event handlers of objects passed to PluginManager.registerEvents, which requires a
+ Listener.
+
+ All BungeeCord @EventHandler methods must reside in a class that implements Listener.
+
+ BungeeCord only registers event handlers of objects passed to PluginManager.registerListener, which requires a
+ Listener.
+
+ Reports missing or unused cancellable @Inject usages
+
+ Calling cancel() or setReturnValue() on the callback info of an @Inject that is not
+ marked cancellable = true throws an exception at runtime. Conversely, an @Inject marked
+ cancellable = true that is never canceled makes Mixin generate an unnecessary check in the target method.
+
+// bad
+@Inject(method = "foo", at = @At("HEAD"))
+private void onFoo(CallbackInfo ci) {
+ ci.cancel();
+}
+
+// good
+@Inject(method = "foo", at = @At("HEAD"), cancellable = true)
+private void onFoo(CallbackInfo ci) {
+ ci.cancel();
+}
+- Reports local variable declarations that are followed by an assignment and can be combined into a single statement. -
-- Joining declarations and assignments helps simplify the code and clarifies the scope of variables. -
-Example:
-
-String s;
-s = "foo";
-After the fix:
-
-String s = "foo";
-- Reports local variable declarations that can be removed and their initializers inlined. -
-- Removing unnecessary local variables helps simplify the code and clarifies its intent. -
-Example:
-
-String s = "foo";
-return s;
-After the fix:
-
-return "foo";
-
+ Reports when two @Shares of the same variable have different types
+
+ All @Share parameters with the same name in the same target method refer to the same variable, so they must have
+ the same type.
+
+// bad
+@Inject(method = "foo", at = @At("HEAD"))
+private void before(CallbackInfo ci, @Share("count") LocalIntRef count) { ... }
+
+@Inject(method = "foo", at = @At("TAIL"))
+private void after(CallbackInfo ci, @Share("count") LocalLongRef count) { ... }
+
+ Reports when @At("CTOR_HEAD") is missing unsafe
+
+ Sponge Mixin restricts where injectors can inject into constructors, and CTOR_HEAD fails without
+ unsafe = true. Fabric's version of Mixin doesn't have this restriction, so by default this inspection is
+ disabled when Fabric Mixin is present.
+
+@Inject(method = "<init>", at = @At("CTOR_HEAD")) // bad
+@Inject(method = "<init>", at = @At(value = "CTOR_HEAD", unsafe = true)) // good
+
+ Reports when CTOR_HEAD with enforce=POST_INIT doesn't target a field
+
+ With enforce=POST_INIT, CTOR_HEAD injects after the field initializers of a constructor. If
+ Mixin's heuristic to detect field initializers fails, it will default to injecting after the delegate constructor call.
+ This inspection reports when this happens.
+
+ Reports when CTOR_HEAD is used without targeting a constructor method
+
+ In methods that aren't constructors, CTOR_HEAD behaves the same as HEAD. Use HEAD
+ instead to make the intent clear.
+
+ Detects usage of translations that are removed or renamed in deprecated.json
+
+ Minecraft's deprecated.json lists translation keys that are no longer used. A removed key no longer has
+ a translation, and a renamed key should be referred to by its new name. Using such a key will cause your translation
+ key not to be translated.
+
+ Reports targets not supported by an injector +
+ ++ Each injector can only operate on certain kinds of instructions, and Mixin fails when an injector targets any other kind. +
+
+// bad: @ModifyArg can only target method invocations
+@ModifyArg(method = "foo", at = @At(value = "FIELD", target = "..."), index = 0)
+-Reports when a discouraged injection point is used + Reports when a discouraged injection point is used +
+ +
+ For example, JUMP selects jump instructions by their opcode and position in the method, which can change when the
+ target method is modified even slightly or is recompiled. Consider targeting a more specific instruction.
+
-Reports discouraged usages of shifting in injection points + Reports discouraged usages of shifting in injection points +
+ +
+ Shifting moves the injection point by a number of bytecode instructions, which depends on exactly how the target method was
+ compiled and breaks easily when it changes. Shifting is allowed where it is commonly needed, for example shifting
+ INVOKE and FIELD injection points to after the instruction.
+
+ A common mistake is to use `Shift.BEFORE` on injection points that already target before, such as `INVOKE`. This results in + the injection being shifted one instruction too early. +
+
+// bad
+@At(value = "INVOKE", target = "...", shift = At.Shift.BY, by = 3)
+
+// good
+@At(value = "INVOKE", target = "...", shift = At.Shift.AFTER)
+@Interface classes in an @Implements annotation.
Index: src/main/resources/inspectionDescriptions/DuplicateInterfacePrefix.html
===================================================================
--- src/main/resources/inspectionDescriptions/DuplicateInterfacePrefix.html (revision ea0444f7c6e18e3b6038b83b6030405830f0b7b9)
+++ src/main/resources/inspectionDescriptions/DuplicateInterfacePrefix.html (revision cf5ea915cc6073030781b3a37e698e8b65302537)
@@ -20,6 +20,19 @@
-Reports duplicate @Interface prefixes in an @Implements annotation.
+
+ Reports duplicate @Interface prefixes in an @Implements annotation.
+
+ Methods whose name starts with an @Interface prefix are renamed when the mixin is applied, to soft-implement
+ a method of that interface. If two interfaces share a prefix, it is ambiguous which interface a prefixed method belongs to.
+
+@Implements({
+ @Interface(iface = Foo.class, prefix = "foo$"),
+ @Interface(iface = Bar.class, prefix = "foo$") // Duplicate prefix 'foo$'
+})
+@Implements annotations (without an @Interface)
Index: src/main/resources/inspectionDescriptions/FabricModJsonEntrypointsValidation.html
===================================================================
--- src/main/resources/inspectionDescriptions/FabricModJsonEntrypointsValidation.html (revision ea0444f7c6e18e3b6038b83b6030405830f0b7b9)
+++ src/main/resources/inspectionDescriptions/FabricModJsonEntrypointsValidation.html (revision cf5ea915cc6073030781b3a37e698e8b65302537)
@@ -20,6 +20,14 @@
-Validates entrypoints declared in Fabric mod JSON files.
+
+ Validates entrypoints declared in fabric.mod.json files.
+
+ Fabric Loader creates entrypoints from the classes, methods and fields listed in fabric.mod.json, and fails to
+ load the mod if it can't. For example, an entrypoint class must have an empty constructor, and an entrypoint method must be
+ public and have no parameters.
+
@SideOnly(Side.SERVER) cannot contain a field which is annotated as @SideOnly(Side.CLIENT). Since
+a class that is annotated with @SideOnly brings everything with it, @SideOnly annotated fields are
+usually useless
Index: src/main/resources/inspectionDescriptions/ImplicitConstructorInvoker.html
===================================================================
--- src/main/resources/inspectionDescriptions/ImplicitConstructorInvoker.html (revision ea0444f7c6e18e3b6038b83b6030405830f0b7b9)
+++ src/main/resources/inspectionDescriptions/ImplicitConstructorInvoker.html (revision cf5ea915cc6073030781b3a37e698e8b65302537)
@@ -20,6 +20,20 @@
-Implicit constructor invoker (fails outside of dev)
++ Reports implicit constructor invokers (fails outside of dev) +
+ +
+ An @Invoker without a target, whose name starts with create or new, invokes a
+ constructor. This works in the development environment, but fails in production. Specify the constructor explicitly instead.
+
+@Invoker // bad
+static Foo createFoo(int x) { throw new AssertionError(); }
+
+@Invoker("<init>") // good
+static Foo createFoo(int x) { throw new AssertionError(); }
+
+ Reports when cancellable @Injects are used before a delegate constructor call
+
+ A constructor must call the delegate constructor before it returns. Cancelling an injector before that call would return + without it, which is not valid bytecode. +
Index: src/main/resources/inspectionDescriptions/InjectCouldBeOverwrite.html =================================================================== --- src/main/resources/inspectionDescriptions/InjectCouldBeOverwrite.html (revision ea0444f7c6e18e3b6038b83b6030405830f0b7b9) +++ src/main/resources/inspectionDescriptions/InjectCouldBeOverwrite.html (revision cf5ea915cc6073030781b3a37e698e8b65302537) @@ -20,6 +20,30 @@ -Reports when an @Inject is better written as an @Overwrite, because the @Inject always cancels and could cause silent mod incompatibilities +
+ Reports when an @Inject can be written as an @Overwrite
+
+ Such an @Inject effectively replaces the target method, so any other mod's injectors into the target method
+ silently stop working. An @Overwrite makes this explicit, and Mixin reports conflicts between overwrites.
+
+// bad
+@Inject(method = "getSpeed", at = @At("HEAD"), cancellable = true)
+private void getSpeed(CallbackInfoReturnable<Float> cir) {
+ cir.setReturnValue(2.0f);
+}
+
+// good
+/**
+ * @author MyMod
+ * @reason Double the speed
+ */
+@Overwrite
+public float getSpeed() {
+ return 2.0f;
+}
+
+ @Inject into Constructor
+
+ Reports @Inject injectors that inject into a constructor at an instruction other than RETURN,
+ without unsafe = true.
+
+ Sponge Mixin only allows @Inject into constructors at RETURN by default, because injecting earlier
+ risks running code before the object is initialized. Fabric's version of Mixin doesn't have this restriction, so by default
+ it is allowed when Fabric Mixin is present.
+
+@Inject(method = "<init>", at = @At(value = "INVOKE", target = "...")) // bad
+@Inject(method = "<init>", at = @At(value = "INVOKE", target = "...", unsafe = true)) // good
+@Inject(method = "<init>", at = @At("RETURN")) // good
+
+ Reports when @Inject local capture can be replaced with @Local
+
+ Local capture requires the injector to declare every local variable up to the ones it needs, in order, so it breaks whenever
+ the local variables of the target method change. @Local captures only the locals that are needed.
+
+// bad
+@Inject(method = "foo", at = @At("TAIL"), locals = LocalCapture.CAPTURE_FAILHARD)
+private void onFoo(CallbackInfo ci, int i, String name, List<String> list) {
+ System.out.println(name);
+}
+
+// good
+@Inject(method = "foo", at = @At("TAIL"))
+private void onFoo(CallbackInfo ci, @Local String name) {
+ System.out.println(name);
+}
+
+ Reports usages of @Interface with a regular class instead of an interface.
+
+ @Implements makes the target class implement interfaces, so it can't be used with a class.
+
-Reports invalid values in Mixin configuration files. + Reports invalid values in Mixin configuration files. +
+ ++ Mixin fails to load a configuration file whose values don't have the expected types. +
Index: src/main/resources/inspectionDescriptions/InvalidInjectorMethodSignature.html =================================================================== --- src/main/resources/inspectionDescriptions/InvalidInjectorMethodSignature.html (revision ea0444f7c6e18e3b6038b83b6030405830f0b7b9) +++ src/main/resources/inspectionDescriptions/InvalidInjectorMethodSignature.html (revision cf5ea915cc6073030781b3a37e698e8b65302537) @@ -20,6 +20,24 @@ +-Reports problems related to the method signature of Mixin injectors + Reports problems related to the method signature of Mixin injectors +
+ ++ Mixin expects specific parameters and return type, depending on the injector and its target. Mixin fails to apply an + injector whose signature doesn't match. +
+
+// target: public int getCount(String name)
+
+// bad
+@Inject(method = "getCount", at = @At("HEAD"))
+private void onGetCount(CallbackInfo ci) { ... }
+
+// good
+@Inject(method = "getCount", at = @At("HEAD"))
+private void onGetCount(String name, CallbackInfoReturnable<Integer> cir) { ... }
+
+ Reports invalid prefixes in @Interface annotations. The prefixes must end with a dollar sign ($).
+
+ Mixin rejects such prefixes when the mixin is applied. +
+
+@Interface(iface = Foo.class, prefix = "foo") // bad
+@Interface(iface = Foo.class, prefix = "foo$") // good
++ Reports member references in Mixin annotations that Mixin can't parse. +
+ +
+ Mixin fails to apply an annotation whose method or target it can't parse. A member reference
+ consists of an optional owner class, the member name and an optional descriptor, in one of these forms:
+
Lcom/example/ExampleClass;execute(II)VLcom/example/ExampleClass;count:I
+ The name can be followed by a quantifier such as *. Regex selectors, such as /^execute$/, and dynamic
+ selectors, such as @Desc(id), are also supported.
+
+@At(value = "INVOKE", target = "com.example.ExampleClass#execute") // bad
+@At(value = "INVOKE", target = "Lcom/example/ExampleClass;execute(II)V") // good
+-Reports invalid Mixin plugins in Mixin configuration files. + Reports invalid Mixin plugins in Mixin configuration files. +
+ +
+ Mixin creates an instance of the plugin class, so it must implement IMixinConfigPlugin, and be a non-abstract
+ class with a public no-argument constructor.
+
-Reports invalid Mixin classes in Mixin configuration files. + Reports invalid Mixin classes in Mixin configuration files. +
+ +
+ Only @Mixin classes can be listed as mixins.
+
-A Mixin class must target either one or more classes or provide one or more string targets + A Mixin class must target either one or more classes or provide one or more string targets +
+ ++ Mixin needs to know which classes to apply the mixin to. +
+
+@Mixin // bad
+@Mixin(Target.class) // good
+@Mixin(targets = "a.b.Target") // good, for classes that can't be referenced
+
+ Reports when INVOKE_ASSIGN could be replaced with a MixinExtras expression.
+
+ Expressions are preferred over INVOKE_ASSIGN because they fail when an assignment doesn't exist.
+
+// bad
+@Inject(method = "foo", at = @At(value = "INVOKE_ASSIGN", target = "Lcom/example/Foo;getName()Ljava/lang/String;"))
+
+// good
+@Definition(id = "getName", method = "Lcom/example/Foo;getName()Ljava/lang/String;")
+@Expression("? = ?.getName()")
+@Inject(method = "foo", at = @At(value = "MIXINEXTRAS:EXPRESSION", shift = At.Shift.AFTER))
+-Reports useless event cancellation checks + Reports useless event cancellation checks +
+ +
+ Some event listeners only receive events that are not canceled, so checking whether the event is canceled always gives the
+ same result. On Bukkit, this applies to handlers with ignoreCancelled = true. On Sponge, listeners only receive
+ events that are not canceled, unless annotated with @IsCancelled.
+
+@EventHandler(ignoreCancelled = true)
+public void onBreak(BlockBreakEvent event) {
+ if (event.isCancelled()) { // always false
+ return;
+ }
+ ...
+}
+-报告无用的 event 取消检查 + 报告无用的 event 取消检查 +
+ +
+ 某些事件监听器只会收到未被取消的事件，因此检查事件是否已取消总是得到相同的结果。在 Bukkit 上，这适用于带有
+ ignoreCancelled = true 的处理器。在 Sponge 上，除非标注了 @IsCancelled，监听器只会收到未被取消的事件。
+
+@EventHandler(ignoreCancelled = true)
+public void onBreak(BlockBreakEvent event) {
+ if (event.isCancelled()) { // 总是 false
+ return;
+ }
+ ...
+}
+LocalCapture.CAPTURE_FAILEXCEPTION is usually a mistake and should be replaced with
-LocalCapture.CAPTURE_FAILHARD. CAPTURE_FAILEXCEPTION generates code which throws an
-exception when the callback is reached, if the locals do not match. If this is really what you want, you can
-suppress this warning.
+
+ Reports usages of LocalCapture.CAPTURE_FAILEXCEPTION.
+
+ If the captured locals don't match the target method, CAPTURE_FAILEXCEPTION generates code which throws an
+ exception when the callback is reached, instead of failing when the mixin is applied. This hides the problem until that code
+ runs, so it is usually a mistake.
+
+@Inject(method = "foo", at = @At("TAIL"), locals = LocalCapture.CAPTURE_FAILEXCEPTION) // bad
+@Inject(method = "foo", at = @At("TAIL"), locals = LocalCapture.CAPTURE_FAILHARD) // good
+
+ Checks that @Local has argsOnly if it targets arguments, which improves performance of the mixin
+
+ With argsOnly = true, MixinExtras only searches the method's parameters, so it doesn't have to analyze the local
+ variables of the target method.
+
+@Local String name // bad
+@Local(argsOnly = true) String name // good
+
+ Reports @Local relying on index or ordinal that may use a name instead
+
+ The index and ordinal of a local variable change whenever local variables are added to or removed from the target method, + while its name usually stays the same. Names are also easier to understand. This is only reported where local variable names + are available at runtime. +
+
+@Local(ordinal = 1) int count // bad
+@Local(name = "count") int count // good
+
+ A variable whose class declaration is annotated with @SideOnly for one side cannot be declared in a class or
+ method that does not match the same side.
+
+ Code annotated with @SideOnly is removed when running on the other side, so code that uses it crashes there.
+
+// bad: runs on both sides
+public void update() {
+ Minecraft mc = Minecraft.getMinecraft(); // Minecraft is @SideOnly(Side.CLIENT)
+}
+
+// good
+@SideOnly(Side.CLIENT)
+public void update() {
+ Minecraft mc = Minecraft.getMinecraft();
+}
+
+ Methods which are declared with a @SideOnly annotation can only be used in matching @SideOnly classes and methods.
+
+ Code annotated with @SideOnly is removed when running on the other side, so code that uses it crashes there.
+
+// bad: runs on both sides
+public void update() {
+ getRenderer().render(); // render() is @SideOnly(Side.CLIENT)
+}
+
+// good
+@SideOnly(Side.CLIENT)
+public void update() {
+ getRenderer().render();
+}
+
+ Reports methods annotated with a different @SideOnly side than their class, and methods returning a type
+ annotated with a different @SideOnly side than the method or its class.
+
+ A class annotated with @SideOnly is removed together with everything in it on the other side, so a method in it
+ can't be for a different side, and @SideOnly annotations on its methods are usually useless. Likewise, a method
+ can't return a type that doesn't exist on its side.
+
+@SideOnly(Side.SERVER)
+public class ServerData {
+ @SideOnly(Side.CLIENT) // bad
+ public void render() { ... }
+}
+-Reports missing abstract shadows in enum mixins + Reports missing abstract shadows in enum mixins +
+ +
+ If the target enum has abstract methods, every enum constant must implement them. Declaring them as abstract
+ @Shadow methods in the mixin makes the compiler check that the constants added by the mixin implement them.
+ Otherwise, the missing implementations cause errors at runtime. In obfuscated environments, the override will not be
+ remapped if the shadow isn't present.
+
+enum Shape {
+ CIRCLE { @Override double area(double r) { ... } };
+ abstract double area(double r);
+}
+
+@Mixin(Shape.class)
+enum ShapeMixin {
+ MYMOD_SQUARE { @Override double area(double r) { ... } };
+
+ @Shadow
+ abstract double area(double r);
+}
+-Detects missing format arguments for translations + Detects missing format arguments for translations +
+ +
+ If a translation contains more placeholders such as %s than there are arguments, it can't be formatted and isn't
+ displayed correctly.
+
+// "my_mod.greeting": "Hello, %s!"
+Component.translatable("my_mod.greeting") // bad
+Component.translatable("my_mod.greeting", name) // good
+
+ All implementations of IMessage and IMessageHandler must have an empty constructor
+
+ When a message is received, Forge creates the message and its handler using their empty constructors, before reading the + message's contents. Without an empty constructor, this fails. +
+
+public class MyMessage implements IMessage {
+ private int value;
+
+ public MyMessage() {} // required
+
+ public MyMessage(int value) {
+ this.value = value;
+ }
+ ...
+}
+-Reports missing or invalid usages of opcode + Reports missing or invalid usages of opcode +
+ +
+ Some injection points, such as FIELD, match several kinds of instructions, and it's good to specify an opcode
+ to say which kind to target. For example, without an opcode, FIELD matches both reads and writes of the field,
+ which is rarely intended.
+
+ Other injection points ignore the opcode, so specifying one there is misleading. +
+
+// bad
+@At(value = "FIELD", target = "Lcom/example/Foo;bar:I")
+
+// good
+@At(value = "FIELD", target = "Lcom/example/Foo;bar:I", opcode = Opcodes.PUTFIELD)
+
+ Reports missing @Unique annotations
+
+ Many mixins can add members to the same target class. Without @Unique, if two mixins add a member with the
+ same name and descriptor, one silently replaces the other. With @Unique, Mixin renames a conflicting private
+ member, and reports an error for a conflicting public member.
+
+private int count; // bad
+@Unique private int count; // good
+-Verifies targets of mixin annotations + Verifies targets of mixin annotations +
+ ++ When Mixin can't find the method or instruction an annotation targets, the injector fails when the mixin is applied. It either + crashes the game, or, if the injector is not required, silently does nothing. +
Index: src/main/resources/inspectionDescriptions/MixinConstructorDoesntExist.html =================================================================== --- src/main/resources/inspectionDescriptions/MixinConstructorDoesntExist.html (revision ea0444f7c6e18e3b6038b83b6030405830f0b7b9) +++ src/main/resources/inspectionDescriptions/MixinConstructorDoesntExist.html (revision cf5ea915cc6073030781b3a37e698e8b65302537) @@ -20,6 +20,33 @@ +-Reports when a mixin class is instantiated within that same class using a constructor which doesn't exist in the target class + Reports when a mixin class is instantiated within that same class using a constructor which doesn't exist in the target class +
+ +
+ A mixin class is merged into its target class, so calls to its constructors become calls to constructors of the target class.
+ If the target class has no constructor with matching parameters, this fails at runtime. For example, this applies to enum
+ constants added by an enum mixin. Add a @Shadow constructor matching a constructor in the target class.
+
+enum Color {
+ RED(0xFF0000);
+ Color(int rgb) { ... }
+}
+
+@Mixin(Color.class)
+enum ColorMixin {
+ MYMOD_PURPLE; // bad (calls implicit shadow constructor, which doesn't exist in target class)
+}
+
+@Mixin(Color.class)
+enum ColorMixin {
+ MYMOD_PURPLE(0x800080); // good
+
+ @Shadow
+ ColorMixin(int rgb) {}
+}
+
+ Operation.call called with the wrong parameter types
+
+ Operation.call accepts any arguments at compile time, but at runtime they must match the parameters of the
+ wrapped operation: the receiver, if any, followed by the arguments of the original call. Otherwise the call fails with an
+ exception.
+
+// target: this.setHealth(health);
+
+@WrapOperation(method = "foo", at = @At(value = "INVOKE", target = "..."))
+private void wrapSetHealth(Entity entity, float health, Operation<Void> original) {
+ original.call(health); // bad
+ original.call(entity, health); // good
+}
+-Unnecessary mutable reference to captured local + Unnecessary mutable reference to captured local +
+ +
+ Reports @Local parameters of a LocalRef type whose value is only read, never set.
+
+ A mutable reference makes MixinExtras wrap the local in a reference object and write its value back afterwards. If the value + is never set, capture the local directly. +
+
+// bad
+private void onFoo(CallbackInfo ci, @Local LocalIntRef count) {
+ System.out.println(count.get());
+}
+
+// good
+private void onFoo(CallbackInfo ci, @Local int count) {
+ System.out.println(count);
+}
+-Reports invalid usages of inner classes in Mixins. + Reports invalid usages of inner classes in Mixins. +
+ ++ A nested mixin is loaded independently, without an instance of the outer class, so it must be static. Mixin copies anonymous + classes declared in a mixin into the target, but doesn't support classes nested inside those anonymous classes. +
Index: src/main/resources/inspectionDescriptions/MixinSuperClass.html =================================================================== --- src/main/resources/inspectionDescriptions/MixinSuperClass.html (revision ea0444f7c6e18e3b6038b83b6030405830f0b7b9) +++ src/main/resources/inspectionDescriptions/MixinSuperClass.html (revision cf5ea915cc6073030781b3a37e698e8b65302537) @@ -20,6 +20,30 @@ -Reports invalid @Mixin super classes. +
+ Reports invalid @Mixin super classes.
+
+ The superclass of a mixin gives the mixin access to the members of a superclass of its target. It must therefore be a class in + the hierarchy of every target class, and can't be the target class itself. If the superclass is another mixin, it must target a + class in the hierarchy of every target class. +
+
+class Animal {}
+class Dog extends Animal {}
+
+@Mixin(Dog.class)
+class DogMixin extends Animal {} // good
+
+@Mixin(Dog.class)
+class DogMixin extends AnimalMixin {} // good
+
+@Mixin(Dog.class)
+class DogMixin extends Dog {} // bad: extends the target class
+
+@Mixin(Dog.class)
+class DogMixin extends Vehicle {} // bad: not in Dog's hierarchy
+-Reports when a Mixin uses the wrong class type + Reports when a Mixin uses the wrong class type +
+ ++ A mixin is merged into its target class, so it must be the same kind of type. Use an interface mixin to target interfaces, and + a class mixin to target classes. Enum mixins are only supported for enum targets. Classes can also be used to target enums and + records. +
+
+interface Target {}
+
+@Mixin(Target.class)
+abstract class TargetMixin {} // bad
+
+@Mixin(Target.class)
+interface TargetMixin {} // good
+
+ Checks that @ModifyVariable has argsOnly if it targets arguments, which improves performance of the mixin
+
+ With argsOnly = true, Mixin only searches the method's parameters, so it doesn't have to analyze the local
+ variables of the target method.
+
+// bad
+@ModifyVariable(method = "setName", at = @At("HEAD"))
+private String modifyName(String name) { ... }
+
+// good
+@ModifyVariable(method = "setName", at = @At("HEAD"), argsOnly = true)
+private String modifyName(String name) { ... }
+
+ Reports @ModifyVariable injectors relying on index or ordinal that may use a name instead
+
+ The index and ordinal of a local variable change whenever local variables are added to or removed from the target method, + while its name usually stays the same. Names are also easier to understand. This is only reported where local variable names + are available at runtime. +
+
+@ModifyVariable(method = "foo", at = @At("STORE"), ordinal = 1) // bad
+@ModifyVariable(method = "foo", at = @At("STORE"), name = "count") // good
+
+ Checks mods.toml files for errors
+
+ Forge fails to load a mod whose mods.toml contains invalid values, such as an invalid mod ID or a value of the
+ wrong type.
+
-Reports when a parameter name in a Mixin does not match the target class + Reports when a parameter name in a Mixin does not match the target class +
+ ++ Naming parameters the same as the corresponding parameters or local variables in the target class makes the injector easier + to read. A mismatched name can also indicate that the parameter doesn't refer to the variable it was meant to. +
+
+// target: public void setHealth(float health)
+
+// bad
+@Inject(method = "setHealth", at = @At("HEAD"))
+private void onSetHealth(float value, CallbackInfo ci) { ... }
+
+// good
+@Inject(method = "setHealth", at = @At("HEAD"))
+private void onSetHealth(float health, CallbackInfo ci) { ... }
+
+ Reports nested classes annotated with a different @SideOnly side than their containing class.
+
+ A class annotated with @SideOnly is removed together with everything in it on the other side, so a nested class
+ can't be for a different side. @SideOnly annotations on nested classes are therefore usually useless.
+
+@SideOnly(Side.CLIENT)
+public class Renderer {
+ @SideOnly(Side.SERVER) // bad
+ public static class Data { ... }
+}
+
+ Reports instantiations of classes annotated with @SideOnly in classes or methods that are annotated differently,
+ or not at all.
+
+ Code annotated with @SideOnly is removed when running on the other side, so code that runs on both sides can't
+ use it either.
+
+// bad: runs on both sides
+public void update() {
+ new GuiMyScreen(); // GuiMyScreen is @SideOnly(Side.CLIENT)
+}
+
+// good
+@SideOnly(Side.CLIENT)
+public void update() {
+ new GuiMyScreen();
+}
+StatCollector.translateToLocal(), StatCollector.translateToLocalFormatted() or I18n.format() exists.
++ Reports translation keys that don't have a translation. +
+ ++ If a translation key doesn't exist, Minecraft displays the key itself instead of a translated text. This is usually caused by + a typo in the key, or a translation missing from the language file. +
+
+// en_us.json: { "my_mod.greeting": "Hello!" }
+Component.translatable("my_mod.greting") // bad
+Component.translatable("my_mod.greeting") // good
+-Mixins should be written in Java. See this Mixin issue + Mixins should be written in Java. See this Mixin issue +
+ ++ Mixin works directly on the bytecode of mixin classes, and only supports the bytecode produced by the Java compiler. + Compilers for other languages, such as Kotlin, generate extra classes, synthetic members and calls to runtime helpers that + Mixin can't handle, which causes errors when the mixin is applied. +
Index: src/main/resources/inspectionDescriptions/OverwriteAuthorRequired.html =================================================================== --- src/main/resources/inspectionDescriptions/OverwriteAuthorRequired.html (revision ea0444f7c6e18e3b6038b83b6030405830f0b7b9) +++ src/main/resources/inspectionDescriptions/OverwriteAuthorRequired.html (revision cf5ea915cc6073030781b3a37e698e8b65302537) @@ -20,6 +20,21 @@ +-For maintainability reasons, the Sponge project requires @Overwrite methods to declare an @author JavaDoc tag. + For maintainability reasons, the Sponge project requires @Overwrite methods to declare an @author JavaDoc tag. +
+ +
+ An @Overwrite replaces the target method entirely and can conflict with other mods. The @author tag
+ says who to contact about the overwrite, and the @reason tag explains why it was needed.
+
+/**
+ * @author MyMod
+ * @reason Fix the speed calculation for slow entities
+ */
+@Overwrite
+public float getSpeed() { ... }
+-Validates the modifiers of @Overwrite methods + Validates the modifiers of @Overwrite methods +
+ +
+ An @Overwrite replaces the target method, so it must have the same modifiers, and can't have lower visibility
+ than the target method. Annotations of the target method, such as nullability annotations, should also be present on the
+ overwrite so it keeps the same contract.
+
+// target: public static int getMaxCount()
+
+@Overwrite private int getMaxCount() { ... } // bad
+@Overwrite public static int getMaxCount() { ... } // good
+-Reports issues in Bukkit-like plugin.yml files + Reports issues in Bukkit-like plugin.yml files +
+ +
+ The server fails to load a plugin whose plugin.yml refers to a main class that doesn't exist or doesn't
+ implement the plugin interface.
+
-A mixin class does not exist at runtime, and thus having them not private does not make sense. Make the field/method private instead. + A mixin class does not exist at runtime, and thus having them not private does not make sense. Make the field/method private instead. +
+ +
+ Mixin rejects non-private static fields and methods when the mixin is applied. @Shadow, @Overwrite,
+ @Accessor and @Invoker members are allowed.
+
+ To share static members, move them to a separate class that is not a mixin. +
Index: src/main/resources/inspectionDescriptions/ReferenceToMixin.html =================================================================== --- src/main/resources/inspectionDescriptions/ReferenceToMixin.html (revision ea0444f7c6e18e3b6038b83b6030405830f0b7b9) +++ src/main/resources/inspectionDescriptions/ReferenceToMixin.html (revision cf5ea915cc6073030781b3a37e698e8b65302537) @@ -20,6 +20,22 @@ +-A Mixin class doesn't exist at runtime, and thus cannot be referenced directly. + A Mixin class doesn't exist at runtime, and thus cannot be referenced directly. +
+ ++ Instead of the mixin class, use the target class, or an interface implemented by the mixin. +
+
+// bad
+((EntityMixin) (Object) entity).myMod$getData();
+
+// good: EntityMixin implements MyModEntity
+((MyModEntity) entity).myMod$getData();
++ Accessor mixins, and references from a mixin to its own super mixins, are allowed. +
Index: src/main/resources/inspectionDescriptions/ShadowFieldPrefix.html =================================================================== --- src/main/resources/inspectionDescriptions/ShadowFieldPrefix.html (revision ea0444f7c6e18e3b6038b83b6030405830f0b7b9) +++ src/main/resources/inspectionDescriptions/ShadowFieldPrefix.html (revision cf5ea915cc6073030781b3a37e698e8b65302537) @@ -20,6 +20,18 @@ -Reports @Shadow fields with prefixes +
+ Reports @Shadow fields with prefixes
+
+ The @Shadow prefix allows a shadow method to avoid a name clash with another method in the mixin. Mixin doesn't
+ support prefixes for fields, and fails when applying the mixin.
+
+@Shadow private int shadow$count; // bad
+@Shadow(prefix = "myMod$") private int myMod$count; // bad
+@Shadow private int count; // good
+@Final annotated fields cannot be modified, as the field it is targeting is final. This can be overridden with @Mutable.
Index: src/main/resources/inspectionDescriptions/ShadowModifiers.html
===================================================================
--- src/main/resources/inspectionDescriptions/ShadowModifiers.html (revision ea0444f7c6e18e3b6038b83b6030405830f0b7b9)
+++ src/main/resources/inspectionDescriptions/ShadowModifiers.html (revision cf5ea915cc6073030781b3a37e698e8b65302537)
@@ -20,6 +20,20 @@
-Validates access modifiers of @Shadow members
+
+ Validates access modifiers of @Shadow members
+
+ A @Shadow member stands for a member of the target class, so its static modifier and access level
+ must match the target member. Shadows of final fields must be annotated with @Final, so that
+ assignments to them are reported.
+
+// target: private static final Logger LOGGER
+
+@Shadow public static Logger LOGGER; // bad
+@Shadow @Final private static Logger LOGGER; // good
+-Reports when a parameter name in a shadow method does not match the target method + Reports when a parameter name in a shadow method does not match the target method +
+ ++ Naming the parameters the same as in the target method makes the shadow easier to read and compare with the target method. +
+
+// target: public void setHealth(float health)
+
+@Shadow public abstract void setHealth(float value); // bad
+@Shadow public abstract void setHealth(float health); // good
+-Reports when a @Share parameter is of a non-LocalRef type + Reports when a @Share parameter is of a non-LocalRef type +
+ +
+ A @Share variable is shared by reference between injectors, so the parameter must be a LocalRef
+ type, such as LocalIntRef or LocalRef<String>.
+
+@Share("count") int count // bad
+@Share("count") LocalIntRef count // good
+-Reports soft-implemented methods in Mixins that do not override a method in the target classes. + Reports soft-implemented methods in Mixins that do not override a method in the target classes. +
+ +
+ A method with an @Interface prefix is meant to implement a method of that interface once the prefix is removed.
+ If no such interface method exists, the method doesn't implement anything, which usually means there is a typo in its name or
+ parameter types.
+
+@Mixin(Target.class)
+@Implements(@Interface(iface = Foo.class, prefix = "foo$"))
+public class TargetMixin {
+ // Foo has no method getNmae()
+ public String foo$getNmae() { ... }
+}
+
+ Invalid @Inject usage in Sponge plugin class.
+
+ Sponge injects these values when it creates the plugin, and fails when an injection can't be satisfied. For example,
+ primitive types can't be injected, constructor injections can't be optional, and an injected Asset must be
+ annotated with an @AssetId that refers to an existing file.
+
+ @Getter must target a method accessible from the event class of this listener
+
+ Sponge fills in the parameter by calling the method named by @Getter on the event, so the method must exist on
+ the event class.
+
+@Listener
+public void onJoin(ServerSideConnectionEvent.Join event, @Getter("getPlayer") ServerPlayer player) { ... } // good
+
+ Reports usages of java.util.logging.Logger in Sponge plugins.
+
+ Sponge provides an org.slf4j.Logger for each plugin, which can be injected using @Inject in the plugin
+ class. It is set up by Sponge, so its messages are identified as coming from your plugin and are logged consistently with the
+ rest of the server.
+
+ Checks the Sponge plugin class is valid. +
+ +
+ Sponge creates an instance of the plugin class, so it must have either a non-private constructor without parameters, or a
+ single constructor annotated with @Inject. The plugin ID must also only contain lowercase letters, numbers,
+ dashes and underscores.
+
+ @Getter requires the parameter's type to be assignable from the annotation's target method return type
+
+ Sponge passes the value returned by the method to the parameter, so the parameter's type must be able to hold it. +
+
+// event.getPlayer() returns ServerPlayer
+@Getter("getPlayer") String player // bad
+@Getter("getPlayer") ServerPlayer player // good
+
+ Reports comparisons of an ItemStack with ItemStack.EMPTY.
+
+ An empty stack isn't always the ItemStack.EMPTY instance. When a stack in an inventory is shrunk, the instance is
+ not replaced with ItemStack.EMPTY, but the stack should still be considered empty.
+
+if (stack == ItemStack.EMPTY) { ... } // bad
+if (stack.isEmpty()) { ... } // good
+-Detect superfluous format arguments for translations + Detect superfluous format arguments for translations +
+ ++ Arguments that the translation has no placeholders for are ignored, which usually means the wrong translation key was used or + the translation is missing a placeholder. +
+
+// "my_mod.greeting": "Hello!"
+Component.translatable("my_mod.greeting", name) // bad
+-Reports when a switch statement or expression is used on an enum mixin + Reports when a switch statement or expression is used on an enum mixin +
+ ++ The Java compiler compiles a switch on an enum into code that uses the ordinals in the enum mixin directly. The ordinals + in the enum mixins don't match the ordinals as they will be once merged into the target class. Cast the value to the + target class instead. +
+
+@Mixin(Color.class)
+enum ColorMixin {
+ ;
+ int bad() {
+ switch (this) { ... }
+ }
+ int good() {
+ switch ((Color) (Object) this) { ... }
+ }
+}
+-Reports an unknown namespace in a ClassTweaker header + Reports an unknown namespace in a ClassTweaker header +
+ +
+ The namespace says which mappings the entries are written in, so that they can be remapped. It must be one of the mapping
+ namespaces of the project, such as official or named.
+
+ Reports unnecessary qualified member references in method = "..."
+
+ An injector always targets methods of the mixin's target classes, so the owner class in the reference is redundant. +
+
+@Inject(method = "Lcom/example/Foo;bar()V", ...) // bad
+@Inject(method = "bar()V", ...) // good
+
+ Reports unnecessary unsafe = true
+
+ unsafe = true only relaxes restrictions on injecting into constructors, so it has no effect on injectors that
+ don't target a constructor. Fabric's version of Mixin doesn't have these restrictions at all, so by default
+ unsafe = true is always reported as unnecessary when Fabric Mixin is present.
+
+ Reports unqualified member references in @At targets
+
+ @At requires member references to be qualified.
+
+@At(value = "INVOKE", target = "bar") // bad
+@At(value = "INVOKE", target = "Lcom/example/Foo;bar()V") // good
+-Reports unresolved references in Mixin configuration files. + Reports unresolved references in Mixin configuration files. +
+ ++ Mixin fails to load mixins, packages or other classes listed in a configuration file that don't exist. +
Index: src/main/resources/inspectionDescriptions/UnresolvedFabricModJsonReference.html =================================================================== --- src/main/resources/inspectionDescriptions/UnresolvedFabricModJsonReference.html (revision ea0444f7c6e18e3b6038b83b6030405830f0b7b9) +++ src/main/resources/inspectionDescriptions/UnresolvedFabricModJsonReference.html (revision cf5ea915cc6073030781b3a37e698e8b65302537) @@ -20,6 +20,12 @@ -Reports unresolved references in Fabric mod JSON files. +
+ Reports unresolved references in fabric.mod.json files.
+
+ Fabric Loader fails to load the mod if fabric.mod.json refers to classes or members that don't exist.
+
+ Verifies targets of @Local annotations
+
+ A @Local must match exactly one local variable at the injection point, otherwise MixinExtras fails to apply the
+ injector. When there are several locals of the same type, specify which one with name, ordinal or
+ index.
+
-Reports unresolved references in Mixin annotations + Reports unresolved references in Mixin annotations +
+ ++ Mixin fails to apply an annotation whose target doesn't exist. +
Index: src/main/resources/inspectionDescriptions/UnusedInjectLocalCapture.html =================================================================== --- src/main/resources/inspectionDescriptions/UnusedInjectLocalCapture.html (revision ea0444f7c6e18e3b6038b83b6030405830f0b7b9) +++ src/main/resources/inspectionDescriptions/UnusedInjectLocalCapture.html (revision cf5ea915cc6073030781b3a37e698e8b65302537) @@ -20,6 +20,26 @@ -Reports when an @Inject local capture is unused +
+ Reports when an @Inject local capture is unused
+
+ Local capture depends on the exact local variables of the target method, so it breaks easily when the target method changes. + If none of the captured locals are used, remove the capture. +
+
+// bad
+@Inject(method = "foo", at = @At("TAIL"), locals = LocalCapture.CAPTURE_FAILHARD)
+private void onFoo(CallbackInfo ci, int count, String name) {
+ System.out.println("foo");
+}
+
+// good
+@Inject(method = "foo", at = @At("TAIL"))
+private void onFoo(CallbackInfo ci) {
+ System.out.println("foo");
+}
+-Ensures that all Mixin classes are referenced from a Mixin configuration + Ensures that all Mixin classes are referenced from a Mixin configuration +
+ ++ Mixin only applies the mixins listed in a Mixin configuration file, so a mixin that isn't listed has no effect. +
Index: src/main/resources/inspectionDescriptions/UnusedShadowMethodPrefix.html =================================================================== --- src/main/resources/inspectionDescriptions/UnusedShadowMethodPrefix.html (revision ea0444f7c6e18e3b6038b83b6030405830f0b7b9) +++ src/main/resources/inspectionDescriptions/UnusedShadowMethodPrefix.html (revision cf5ea915cc6073030781b3a37e698e8b65302537) @@ -20,6 +20,20 @@ -Reports unused prefixes of @Shadow methods +
+ Reports unused prefixes of @Shadow methods
+
+ The prefix is only removed from method names that start with it, so a prefix that doesn't match the method's name has no + effect. +
+
+@Shadow(prefix = "shadow$")
+public abstract int getCount(); // bad
+
+@Shadow(prefix = "shadow$")
+public abstract int shadow$getCount(); // good
+
+ Reports uses of fields annotated with @SideOnly, or declared in a class annotated with @SideOnly, in
+ classes or methods that are annotated differently, or not at all.
+
+ Code annotated with @SideOnly is removed when running on the other side, so code that uses it crashes there.
+
+@SideOnly(Side.CLIENT)
+private int renderColor;
+
+// bad: runs on both sides
+public int getColor() {
+ return renderColor;
+}
+
+// good
+@SideOnly(Side.CLIENT)
+public int getColor() {
+ return renderColor;
+}
+
+ Reports when @WrapWithCondition targets a non-void instruction followed by a pop
+
+ When the condition is false, @WrapWithCondition skips the call, so the call has no return value. This only works
+ because the return value is discarded, and breaks if the target method is changed to use it, including by another Mixin.
+ Use @WrapOperation instead, which can provide a return value.
+
+// target: list.add(item);
+
+// bad
+@WrapWithCondition(method = "foo", at = @At(value = "INVOKE", target = "Ljava/util/List;add(Ljava/lang/Object;)Z"))
+private boolean shouldAdd(List<Item> list, Item item) {
+ return item != null;
+}
+
+// good
+@WrapOperation(method = "foo", at = @At(value = "INVOKE", target = "Ljava/util/List;add(Ljava/lang/Object;)Z"))
+private boolean add(List<Item> list, Item item, Operation<Boolean> original) {
+ return item != null && original.call(list, item);
+}
++ Reports when the class passed to a synched entity data definition is not the same as the containing entity class. +
+ ++ Synched entity data is numbered per entity class, based on how many parameters that class and its superclasses define. + Defining a data accessor with a different class can give it an ID that conflicts with another accessor, which breaks entity data + syncing. +
+
+public class MyEntity extends Entity {
+ // bad
+ private static final EntityDataAccessor<Integer> DATA = SynchedEntityData.defineId(Entity.class, EntityDataSerializers.INT);
+
+ // good
+ private static final EntityDataAccessor<Integer> DATA = SynchedEntityData.defineId(MyEntity.class, EntityDataSerializers.INT);
+}
++ 当传递给 synched entity data 定义的类与包含的实体类不一致时，会发出报告。 +
+ ++ Synched entity data 按实体类编号，编号取决于该类及其父类定义了多少个参数。使用其他类定义 data accessor 可能会使其 ID 与另一个 + data accessor 冲突，从而破坏 entity data 的同步。 +
+
+public class MyEntity extends Entity {
+ // 错误
+ private static final EntityDataAccessor<Integer> DATA = SynchedEntityData.defineId(Entity.class, EntityDataSerializers.INT);
+
+ // 正确
+ private static final EntityDataAccessor<Integer> DATA = SynchedEntityData.defineId(MyEntity.class, EntityDataSerializers.INT);
+}
+-Detect wrong argument types in translation arguments + Detect wrong argument types in translation arguments +
+ +
+ Translation arguments must be a String, Number, Boolean or Component.
+ Minecraft can't serialize arguments of other types, which causes errors, for example when the component is sent over the
+ network. Convert such arguments to strings, or use Component.translatableEscape.
+
+Component.translatable("my_mod.position", pos) // bad
+Component.translatable("my_mod.position", String.valueOf(pos)) // good
+