Skip to content

Code change detection API #4998

@Earthcomputer

Description

@Earthcomputer

Add an API to detect changes in library codebases (primarily Minecraft). This API would go in a new test-only module for unit testing (which we surprisingly don't yet have).

Description

At an abstract level, when a unit test written with this API is first run, it will scrape some data from the library jar, write it to a file, and pass the unit test. This file should be committed to the repo. Each subsequent time the test is run, the same data is scraped from the library jar, and compared against the data already written to that file. If the data is the same, the test will pass, if it is different (which may happen when the library is updated and changed in a way you are interested in) then the test will fail. If the file is deleted, the test will run anew, write the new data to the file, and then the test will be passed until the data changes again.

Here are some examples of useful change detection tests of this format:

  • Decompilations of classes, or individual methods, with Vineflower (requiring Vineflower to be on the test classpath). The source code generated by Vineflower will be written to the file.
  • Usages of a class, method, or field. The library (or maybe the whole library classpath) will be scanned for usages of a particular class, method, or field. The usages will be sorted in some deterministic order and written to the file.
  • Implementations of a class or interface. The library (or maybe the whole library classpath) will be scanned for implementations of a particular class or interface. The implementations will be sorted in some deterministic order and written to the file.

The API could be made extensible. Every one of this type of test basically asks for a Supplier<String> which returns some String constructed based on the current environment. The API could handle all the writing to files.

Motivation

Normally when you're using a library, most changes of code that you depend on are caught by javac as compile errors, which remain the most significant aid to porting. However, compile errors are of no help when transforming code. Here, the mixin audit test helps a lot to make sure the mixins at least don't crash, but even if the mixins don't crash, they are prone to logic errors after porting. Additionally, if we ever have to copy vanilla code (which we usually try to avoid, but it's sometimes unavoidable), we would like to be notified when the code we copied from is updated so that we can update our copy.

  • LiquidBlockRendererMixin is targeting code that is quite hostile to mixins, so we have resorted to several rather brittle injections that interact with each other via @Share. Mojang could easily change this method without us noticing until a user reports a bug about the API not working properly. If we listened for changes in the render method we would know straight away when porting.
  • The fluid API would like to replace many, but not all, uses of FluidTags.WATER, and in differing ways. We have until now avoided making this part of the API due to the maintenance burden. Having a unit test keep track of uses of FluidTags.WATER would make this API feasible for us.
  • FluidHandlerRegistry hardcodes isBlockTransparent to a copy of the vanilla code, also in LiquidBlockRenderer. Again we could listen for this change.

And it's not just Fabric API itself that could benefit:

  • Lithium could listen for changes in methods they target so that their optimizations, and particularly their @Overwrites, don't over time end up causing changes compared to vanilla behavior.
  • The clientarguments library, which needs to copy most/all vanilla argument types for client-side use, could listen for changes in all those argument types, and for new argument type implementations.
  • ViaFabricPlus, and also the ViaVersion team, can write tests to listen for changes affecting the protocol, player movement, and other things they need to be updated on.
  • Clientcommands, which already has regression tests for this purpose, can be changed to use this API
  • The bingo-extras mod I wrote uses mass ASM to replace all dimension checks so that vanilla dimension-specific features still work on fantasy mirror dimensions. It's somewhat risky to use mass ASM for this purpose and I am probably still missing some stuff because of it (for example the clock and the compass). Fantasy itself could possibly also benefit, not sure. I'm also likely not the only one who has written mass ASM or a ton of mixins for this exact purpose.
  • Most likely many modders have over time winced at the duplication of code from vanilla or some brittle target and worried about what might happen when it changes. This API would bring them the option of doing it with a peace of mind when there is no other option.

Prior art

Vineflower

Vineflower itself has a suite of regression tests almost exactly like this, for a slightly different purpose: testing Vineflower. Their job is to make sure that changes to Vineflower don't have unexpected negative consequences to decompilation, and that any changes to the decompiler output in those tests are at least seen and acknowledged by the developer.

clientcommands

Clientcommands has a suite of so-called regression tests, inspired by Vineflower and working in a similar way. They are more or less what I am proposing for this API, although the exact API could be refined a bit. The regression test can be found here, with the output files here. My usecase is a little more complicated than most mods require, with CallHierarchyWalker walking up the usage tree multiple steps - most mods will only require the simple usage of one field/method. The regression tests have already caught multiple changes that I wouldn't have spotted otherwise without checking the call hierarchies manually for each port.

MusicalCode

MusicalCode is an older experimental project of mine for the purpose of listening for changes in vanilla code. This project was soon abandoned, and I consider clientcommands' regression tests experiment to be an overall better and more powerful method of listening for changes. Lithium briefly adopted MusicalCode in an attempt to solve their worries about their @Overwrites getting out of sync.

ViaVersion

ViaVersion and other protocol translators and protocol translator adjacent projects need to listen for changes beyond what would be caught by a simple compile error and mixin audit based port. An early tool to check for protocol changes, block and item changes, etc was Burger by Pokechu22. Since then, KennyTV has developed a tool which dumps all StreamCodecs to a file, which can then be diffed.

Metadata

Metadata

Assignees

No one assigned

    Labels

    enhancementNew feature or request

    Projects

    No projects

    Milestone

    No milestone

    Relationships

    None yet

    Development

    No branches or pull requests

    Issue actions