minecraftforge / forgeflower Goto Github PK

Forge's modifications to FernFlower. Fixing various bugs/inconsistencies. Main Repo: https://github.com/MinecraftForge/FernFlower

License: Apache License 2.0

forgeflower's Introduction

MinecraftForge

Forge is a free, open-source modding API all of your favourite mods use!

Version	Support
1.20.x	Active
1.19.x	Legacy

Installing Forge

Go to the Forge website and select the Minecraft version you wish to get Forge for from the list.

You can download the installer for the Recommended Build or the Latest build there. Latest builds may have newer features but may be more unstable as a result. The installer will attempt to install Forge into your vanilla launcher environment, where you can then create a new profile using that version and play the game!

For support and questions, visit the Support Forum or the Forge Discord server.

Creating Mods

See the "Getting Started" section in the Forge Documentation.

Contribute to Forge

If you wish to actually inspect Forge, submit PRs or otherwise work with Forge itself, you're in the right place!

See the guide to setting up a Forge workspace.

Pull requests

See the "Making Changes and Pull Requests" section in the Forge documentation.

Please read the contributing guidelines found here before making a pull request.

Contributor License Agreement

We require all contributors to acknowledge the Forge Contributor License Agreement. Please ensure you have a valid email address associated with your GitHub account to do this. If you have previously signed it, you should be OK.

Donate

Forge is a large project with many collaborators working on it around the clock. Forge is and will always remain free to use and modify. However, it costs money to run such a large project as this, so please consider becoming a patron.

forgeflower's People

Contributors

Stargazers

Watchers

forgeflower's Issues

Possible enhanced for loop failure

MC 1.16-pre1 BackupList.func_230753_a_

...
      try {
         JsonElement jsonelement = jsonparser.parse(p_230753_0_).getAsJsonObject().get("backups");
         if (jsonelement.isJsonArray()) {
            Iterator<JsonElement> iterator = jsonelement.getAsJsonArray().iterator();

            while(iterator.hasNext()) {
               backuplist.field_230560_a_.add(Backup.func_230750_a_(iterator.next()));
            }
         }
      } catch (Exception exception) {
         field_230561_b_.error("Could not parse BackupList: " + exception.getMessage());
      }

      return backuplist;

There are a few cases like this. The enhanced for loop fails to generate as we expect the first statement in the loop to be iterator.next() being assigned to a variable. In this case, it has already been collapsed into the following statement.

I say this is a possible failure as the iterator variable definition has the correct generic type, which suggest the method was actually written this way.

try-with-resource support.

See 1.13 PngSizeInfo.

Some enchanced for loops are not enhanced.

See 1.13, NetHandlerPlayClient.func_191980_a

    label57:
      switch(spacketrecipebook$state) {
      case REMOVE:
         Iterator iterator1 = p_191980_1_.func_192595_a().iterator();

         while(true) {
            if (!iterator1.hasNext()) {
               break label57;
            }

            ResourceLocation resourcelocation3 = (ResourceLocation)iterator1.next();
            IRecipe irecipe3 = this.field_199528_o.func_199517_a(resourcelocation3);
            if (irecipe3 != null) {
               recipebookclient.func_193831_b(irecipe3);
            }
         }
      case INIT:
         for(ResourceLocation resourcelocation1 : p_191980_1_.func_192595_a()) {
            IRecipe irecipe1 = this.field_199528_o.func_199517_a(resourcelocation1);
            if (irecipe1 != null) {
               recipebookclient.func_194073_a(irecipe1);
            }
         }

         Iterator iterator = p_191980_1_.func_193644_b().iterator();

         while(true) {
            if (!iterator.hasNext()) {
               break label57;
            }

            ResourceLocation resourcelocation2 = (ResourceLocation)iterator.next();
            IRecipe irecipe2 = this.field_199528_o.func_199517_a(resourcelocation2);
            if (irecipe2 != null) {
               recipebookclient.func_193825_e(irecipe2);
            }
         }
      case ADD:
         for(ResourceLocation resourcelocation : p_191980_1_.func_192595_a()) {
            IRecipe irecipe = this.field_199528_o.func_199517_a(resourcelocation);
            if (irecipe != null) {
               recipebookclient.func_194073_a(irecipe);
               recipebookclient.func_193825_e(irecipe);
               RecipeToast.func_193665_a(this.field_147299_f.func_193033_an(), irecipe);
            }
         }
      }

Nested lambdas sometimes do not get variables propagated from top.

Lambdas will sometimes be decompiled with only the immediate outer name.
See BlockPos.func_191531_b

Misuse of debug names when debug names are disable

When running ForgeFlower with -udv=0, debug names are showing up in the output. Here are the options I'm running with:

"-din=1", "-rbr=0", "-dgs=1", "-asc=1", "-hdc=0", "-rsy=1", "-iec=1", "-udv=0", "-jvn=1", "-log=WARN"

Here's a diff from vanilla FernFlower (build from a few days ago) and ForgeFlower 1.5.478.6 (06/13/20 10:44 PM):

@@ -96,17 +95,17 @@
    private final BiomeManager biomeManager;
    private final ResourceKey<Level> dimension;
    private final ResourceKey<DimensionType> dimensionTypeKey;
 
    protected Level(WritableLevelData var1, ResourceKey<Level> var2, ResourceKey<DimensionType> var3, DimensionType var4, Supplier<ProfilerFiller> var5, boolean var6, boolean var7, long var8) {
-      this.profiler = var5;
-      this.levelData = var1;
-      this.dimensionType = var4;
-      this.dimension = var2;
-      this.dimensionTypeKey = var3;
-      this.isClientSide = var6;
-      if (var4.shrunk()) {
+      this.profiler = ☃;
+      this.levelData = ☃;
+      this.dimensionType = ☃;
+      this.dimension = ☃;
+      this.dimensionTypeKey = ☃;
+      this.isClientSide = ☃;
+      if (☃.shrunk()) {
          this.worldBorder = new WorldBorder() {
             public double getCenterX() {
                return super.getCenterX() / 8.0D;
             }

full diff of Level
This happens in every class

Some short lived variables are not collapsed.

See 1.13's WorldServer init
SingleBiomeProvider.func_205004_a
BlockStateFlattenVillageCrops.func_210590_b
BlockStateFlattenGenOptions.func_209636_a

Deompile exception after updating to 1.5.605.12

Commit 8af08ea seems to introduce an exception when decompiling certain classes. One example of the stacktrace:

pool-1-thread-2: ERROR: Class net/minecraft/server/level/TicketType couldn't be fully decompiled.
java.lang.NullPointerException: Cannot invoke "org.jetbrains.java.decompiler.struct.StructClass.getMethods()" because "content" is null
	at org.jetbrains.java.decompiler.modules.decompiler.exps.InvocationExprent.getInferredExprType(InvocationExprent.java:361)
	at org.jetbrains.java.decompiler.modules.decompiler.ExprProcessor.getCastedExprent(ExprProcessor.java:1067)
	at org.jetbrains.java.decompiler.modules.decompiler.ExprProcessor.getCastedExprent(ExprProcessor.java:1040)
	at org.jetbrains.java.decompiler.main.ClassWriter.fieldToJava(ClassWriter.java:579)
	at org.jetbrains.java.decompiler.main.ClassWriter.classToJava(ClassWriter.java:268)
	at org.jetbrains.java.decompiler.main.ClassesProcessor.writeClass(ClassesProcessor.java:46)
	at org.jetbrains.java.decompiler.main.Fernflower.getClassContent(Fernflower.java:175)
	at org.jetbrains.java.decompiler.struct.ContextUnit.lambda$save$3(ContextUnit.java:198)
	at java.base/java.util.concurrent.Executors$RunnableAdapter.call(Executors.java:53)
	at java.base/java.util.concurrent.FutureTask.run(FutureTask.java:264)
	at java.base/java.util.concurrent.ThreadPoolExecutor.runWorker(ThreadPoolExecutor.java:1136)
	at java.base/java.util.concurrent.ThreadPoolExecutor$Worker.run(ThreadPoolExecutor.java:635)
	at java.base/java.lang.Thread.run(Thread.java:833)

WorldEntitySpawner.func_234964_a_ - the loop has gotten completely smashed apart

   public static WorldEntitySpawner.EntityDensityManager func_234964_a_(int p_234964_0_, Iterable<Entity> p_234964_1_, WorldEntitySpawner.IInitialDensityAdder p_234964_2_) {
      MobDensityTracker mobdensitytracker = new MobDensityTracker();
      Object2IntOpenHashMap<EntityClassification> object2intopenhashmap = new Object2IntOpenHashMap<>();
      Iterator iterator = p_234964_1_.iterator();

      while(true) {
         Entity entity;
         while(true) {
            if (!iterator.hasNext()) {
               return new WorldEntitySpawner.EntityDensityManager(p_234964_0_, object2intopenhashmap, mobdensitytracker);
            }

            entity = (Entity)iterator.next();
            if (!(entity instanceof MobEntity)) {
               break;
            }

            MobEntity mobentity = (MobEntity)entity;
            if (!mobentity.func_104002_bU() && !mobentity.func_213392_I()) {
               break;
            }
         }

         EntityClassification entityclassification = entity.func_200600_R().func_220339_d();
         if (entityclassification != EntityClassification.MISC) {
            BlockPos blockpos = entity.func_233580_cy_();
            long i = ChunkPos.func_77272_a(blockpos.func_177958_n() >> 4, blockpos.func_177952_p() >> 4);
            p_234964_2_.query(i, (p_234971_5_) -> {
               Biome biome = func_234980_b_(blockpos, p_234971_5_);
               Biome.EntityDensity biome$entitydensity = biome.func_235058_a_(entity.func_200600_R());
               if (biome$entitydensity != null) {
                  mobdensitytracker.func_234998_a_(entity.func_233580_cy_(), biome$entitydensity.func_235120_b_());
               }

               object2intopenhashmap.addTo(entityclassification, 1);
            });
         }
      }
   }

This loop structure looks really weird, as if it was unable to figure out a for loop properly, with a continue condition or something similar.

Empty protected constructors for public/inner classes are stripped.

See net.minecraft.client.renderer.model.BlockPart$Deserializer in 1.15

For loop with nested try-with

1.13.1 TextureGlyphProviderUnicode constructor

      this.field_211257_b = p_i49737_1_;
      this.field_211258_c = p_i49737_2_;
      this.field_211259_d = p_i49737_3_;

      label322:
      for(int i = 0; i < 256; ++i) {
         char c0 = (char)(i * 256);
         ResourceLocation resourcelocation = this.func_211623_c(c0);

         try (
            IResource iresource = this.field_211257_b.func_199002_a(resourcelocation);
            NativeImage nativeimage = NativeImage.func_211679_a(NativeImage.PixelFormat.RGBA, iresource.func_199027_b());
         ) {
            if (nativeimage.func_195702_a() == 256 && nativeimage.func_195714_b() == 256) {
               int j = 0;

               while(true) {
                  if (j >= 256) {
                     continue label322;
                  }

                  byte b0 = p_i49737_2_[c0 + j];
                  if (b0 != 0 && func_212453_a(b0) > func_212454_b(b0)) {
                     p_i49737_2_[c0 + j] = 0;
                  }

                  ++j;
               }
            }
         } catch (IOException var43) {
            ;
         }

         Arrays.fill(p_i49737_2_, c0, c0 + 256, (byte)0);
      }

   }

Add support for switch on string

See DimensionGeneratorSettings for an example, which currently looks like this:

      switch(s4.hashCode()) {
      case -1100099890:
         if (s4.equals("largebiomes")) {
            b0 = 3;
         }
         break;
      case 3145593:
         if (s4.equals("flat")) {
            b0 = 0;
         }
         break;
      case 1045526590:
         if (s4.equals("debug_all_block_states")) {
            b0 = 1;
         }
         break;
      case 1271599715:
         if (s4.equals("amplified")) {
            b0 = 2;
         }
      }

      switch(b0) {

The whole upper switch can be removed, and the lower switch should switch on the string.

Variable scoping leaking around ifs?

See 1.13-pre10 to 1.13

Simple lambdas not condensed.

See Registry class in Minecraft. Simple lambdas, that can be expressed in a single line are not printed as a single line:

public static final Registry<Biome> BIOME = forge("biome", Biome.class, () -> {
    return Biomes.DEFAULT;
});

vs
public static final Registry<Biome> BIOME = forge("biome", Biome.class, () -> Biomes.DEFAULT);

Boxed and primitive overloads causing recursion - 1.16/1.15 SuffixArray#func_194058_a

      IntComparator intcomparator = new IntComparator() {
         public int compare(int p_compare_1_, int p_compare_2_) {
            return aint1[p_compare_1_] == aint1[p_compare_2_] ? Integer.compare(aint2[p_compare_1_], aint2[p_compare_2_]) : Integer.compare(aint1[p_compare_1_], aint1[p_compare_2_]);
         }

         public int compare(Integer p_compare_1_, Integer p_compare_2_) {
            return this.compare(p_compare_1_, p_compare_2_);
         }
      };

2 compare methods one using int, one using Integer. The Integer compare should reference the int compare but is instead referencing itself.

Some variables have LVT tags when other var exprents dont.

MineshaftPieces.Rooms has a bunch of loops that share a index. These variable should not be merged, but as they are. We need them all to share the same name.

          int k;
          for(lvt_5_1_ = 0; k < this.field_74887_e.func_78883_b(); k = k + 4) {
             k = k + p_74861_3_.nextInt(this.field_74887_e.func_78883_b());
             if (k + 3 > this.field_74887_e.func_78883_b()) {
                break;

Private implementation detail of `System.arraycopy` in OpenJ9 builds results in casts to parameters being generated for all invocations of method

When OpenJ9 is used during ForgeGradle's decompile process, additional parameter casts are generated when calling System#arrayCopy(Object, int, Object, int, int) in the produced source code.

For HotSpot builds, the following code may be produced:

System.arraycopy(arr1, 0, arr2, 0, arr2.length);

When using OpenJ9 builds, the following code will instead be generated:

System.arraycopy((Object) arr1, 0, (Object) arr2, 0, arr2.length);

This can lead to issues such as MinecraftForge/ForgeGradle#597, where patches expecting there to be no additional cast fail.

The only difference that can be seen between the OpenJ9 and HotSpot builds for this method is that OpenJ9 includes a private reference implementation alongside it. I suspect this is the reason the cast is produced, as two methods exist with different parameter types.

Primitive class wrappers are not unwrapped

(Originally opened as MinecraftForge/ForgeGradle#713 on the wrong repo)
This is a very minor enchancement, but it should be pretty easy to implement

See InputMapping clinit:

      MethodType methodtype = MethodType.methodType(Boolean.TYPE);
      MethodHandle methodhandle = null;
      int i = 0;

      try {
         methodhandle = lookup.findStatic(GLFW.class, "glfwRawMouseMotionSupported", methodtype);
         MethodHandle methodhandle1 = lookup.findStaticGetter(GLFW.class, "GLFW_RAW_MOUSE_MOTION", Integer.TYPE);

Integer.TYPE and Boolean.TYPE should be replaced with int.class and boolean.class respectively, as that's more easy to understand (the compiler converts any primitive .class call to the respective wrapper class .TYPE)

Unneeded imports for lambdas.

Scope Leak in enchanced for loops.

   public void func_216099_a(ValidationResults p_216099_1_, Function<ResourceLocation, LootTable> p_216099_2_, Set<ResourceLocation> p_216099_3_, LootParameterSet p_216099_4_) {
      for(int i = 0; i < this.poolConditions.size(); ++i) {
         this.poolConditions.get(i).func_215856_a(p_216099_1_.func_216108_b(".condition[" + i + "]"), p_216099_2_, p_216099_3_, p_216099_4_);
      }

      for(int j = 0; j < this.field_216102_d.length; ++j) {
         this.field_216102_d[j].func_215856_a(p_216099_1_.func_216108_b(".functions[" + j + "]"), p_216099_2_, p_216099_3_, p_216099_4_);
      }

      for(int k = 0; k < this.lootEntries.size(); ++k) {
         this.lootEntries.get(k).func_216142_a(p_216099_1_.func_216108_b(".entries[" + k + "]"), p_216099_2_, p_216099_3_, p_216099_4_);
      }

   }

All indexes SHOULD be 'i', as they do not leak out and should not effect each other's naming schemes.

Field initalizers arnt properly extracted.

When class has multiple constructors, and one calls the other
See 1.13.1 GuiTextField

      this(p_i45542_1_, p_i45542_2_, p_i45542_3_, p_i45542_4_, p_i45542_5_, p_i45542_6_, (GuiTextField)null);
   }

   public GuiTextField(int p_i49853_1_, FontRenderer p_i49853_2_, int p_i49853_3_, int p_i49853_4_, int p_i49853_5_, int p_i49853_6_, @Nullable GuiTextField p_i49853_7_) {
      this.field_146216_j = "";
      this.field_146217_k = 32;
      this.field_146215_m = true;
      this.field_146212_n = true;
      this.field_146226_p = true;
      this.field_146222_t = 14737632;
      this.field_146221_u = 7368816;
      this.field_146220_v = true;
      this.field_175209_y = Predicates.<String>alwaysTrue();
      this.field_195613_A = (p_195610_0_, p_195610_1_) -> {
         return p_195610_0_;
      };
//Everything above this should be extracted
      this.field_175208_g = p_i49853_1_;
      this.field_146211_a = p_i49853_2_;
      this.field_146209_f = p_i49853_3_;
      this.field_146210_g = p_i49853_4_;
      this.field_146218_h = p_i49853_5_;
      this.field_146219_i = p_i49853_6_;
      if (p_i49853_7_ != null) {
         this.func_146180_a(p_i49853_7_.func_146179_b());
      }

   }

Support JAD parameter names as configurable option

https://github.com/MinecraftForge/ForgeFlower/blob/master/FernFlower-Patches/0012-JAD-Style-variable-naming.patch#L477

This is hard coded false with no visible way to change it.

I'm having trouble building the project, so hoping someone can assist in making this option configurable?

Local variable names in inner class methods aren't fixed when they conflict with outer names

This is due to https://github.com/MinecraftForge/ForgeFlower/blob/master/FernFlower-Patches/0037-Do-not-rebuild-variable-names-in-lambdas.patch

For background on what that code is doing (re: the patch message):
That allows the decompiler to fix variable names in a nested class or lambda expression which clashes with the name of an outer variable.

What's happening is it's checking the names for the current method against the variable names defined in the outer scope. That's why setNewOuterNames is passed to VarNamesCollector, then checked for in VarNamesCollector.getFreeName() - the outer names are compared against the names for the nested method.

What FernFlower's code originally did was append x to local variable names when conflicts came up - but now it doesn't do that because of that patch. Considering that's the only thing that method is actually doing I don't know what the patch is actually trying to fix. The commit which added the patch said it fixes #88, but there is no issue 88 yet.

This comes up relatively infrequently, and I guess never in MCP due to how LVT is handled, but it causes an issue with my usage. This quick and dirty fix is because of this issue - previous versions of FernFlower would rename the inner i variable to ix. https://github.com/PaperMC/Paper/blob/mappings/mojang/Spigot-Server-Patches/0266-Optimize-BlockPosition-helper-methods.patch#L113-L134

Field initialisers aren't inlined when they should be, mixing up the initialisation order

https://github.com/MinecraftForge/ForgeFlower/blob/master/FernFlower-Patches/0004-Fix-field-initalizers.patch#L62

Super calls on interfaces are not properly qualified.

See 1.13.1 TextureGlyphProvider$GlyphInfo.getBearingY

      public float getBearingY() {
         return super.getBearingY() + 7.0F - (float)this.field_211589_h;
      }

Should be

      public float getBearingY() {
         return IGlyphInfo.super.getBearingY() + 7.0F - (float)this.field_211589_h;
      }

Threads hang when decompiling project

When decompiling the following project then threads start to hang: https://gofile.io/d/6FlfC2

The used command is with "Build: 1.5.478.16 - Wed Aug 05 01:34:59 GMT 2020" is:
java -mx1G -jar forgeflower.jar classes2-dex2jar.jar outDir

Record Canonical Constructor's not cleaned/written correctly.

1.18.1, net.minecraft.world.ticks.ScheduledTick, has a canonical constructor that makes the position immutible. This causes issues with eclipe's compiler due to ambiguous constructors. {Honestly I think this is a issue in eclipse compiler but it's also an issue with us}
Current decompile

   public ScheduledTick(T p_193383_, BlockPos p_193384_, long p_193385_, long p_193386_) {
      this(p_193383_, p_193384_, p_193385_, TickPriority.NORMAL, p_193386_);
   }

   public ScheduledTick(T type, BlockPos pos, long triggerTick, TickPriority priority, long subTickOrder) {
      pos = pos.immutable();
      this.type = type;
      this.pos = pos;
      this.triggerTick = triggerTick;
      this.priority = priority;
      this.subTickOrder = subTickOrder;
   }

Proper decompile:

   public ScheduledTick(T p_193383_, BlockPos p_193384_, long p_193385_, long p_193386_) {
      this(p_193383_, p_193384_, p_193385_, TickPriority.NORMAL, p_193386_);
   }

   public ScheduledTick {
      pos = pos.immutable();
   }

See: https://docs.oracle.com/en/java/javase/15/language/records.html#GUID-6699E26F-4A9B-4393-A08B-1E47D4B2D263__GUID-0630D05B-DCF5-484D-8D46-A78DB6C8B2F0

AbstractOption : autounboxing is being used in decompile, creating compilation errors

   public static final SliderPercentageOption field_216704_k = new SliderPercentageOption("options.mipmapLevels", 0.0D, 4.0D, 1.0F, (p_216667_0_) -> {
      return (double)p_216667_0_.field_151442_I;
   }, (p_216585_0_, p_216585_1_) -> {
      p_216585_0_.field_151442_I = (int)p_216585_1_;
   }, (p_216629_0_, p_216629_1_) -> {
      double d0 = p_216629_1_.func_216729_a(p_216629_0_);
      IFormattableTextComponent iformattabletextcomponent = p_216629_1_.func_238238_a_();
      return d0 == 0.0D ? iformattabletextcomponent.func_230529_a_(DialogTexts.field_240631_b_) : iformattabletextcomponent.func_240702_b_(Integer.toString((int)d0));
   });

The int cast is decompiled incorrectly - it should be intValue() on the parameter, or doubleValue even. This works in 1.15.2.

JAD varnaming problem

JAD var generator won't regenerate the parameter names of the method. Because of this, parameter names may conflict with the generated names.

While loop created outside try block with a return statement - 1.16/1.15 ClientThread#run

   public void run() {
      while(true) {
         try {
            if (!this.field_72619_a) {
               return;
            }

            BufferedInputStream bufferedinputstream = new BufferedInputStream(this.field_72659_h.getInputStream());
            int i = bufferedinputstream.read(this.field_72660_i, 0, 1460);
            if (10 <= i) {
               int j = 0;
               int k = RConUtils.func_72665_b(this.field_72660_i, 0, i);
               if (k != i - 4) {
                  return;
               }

               j = j + 4;
               int l = RConUtils.func_72665_b(this.field_72660_i, j, i);
               j = j + 4;
               int i1 = RConUtils.func_72662_b(this.field_72660_i, j);
               j = j + 4;
               switch(i1) {
               case 2:
                  if (this.field_72657_g) {
                     String s1 = RConUtils.func_72661_a(this.field_72660_i, j, i);

                     try {
                        this.func_72655_a(l, this.field_232651_i_.func_71252_i(s1));
                     } catch (Exception exception) {
                        this.func_72655_a(l, "Error executing: " + s1 + " (" + exception.getMessage() + ")");
                     }
                     continue;
                  }

                  this.func_72656_f();
                  continue;
               case 3:
                  String s = RConUtils.func_72661_a(this.field_72660_i, j, i);
                  int j1 = j + s.length();
                  if (!s.isEmpty() && s.equals(this.field_72658_j)) {
                     this.field_72657_g = true;
                     this.func_72654_a(l, 2, "");
                     continue;
                  }

                  this.field_72657_g = false;
                  this.func_72656_f();
                  continue;
               default:
                  this.func_72655_a(l, String.format("Unknown request %s", Integer.toHexString(i1)));
                  continue;
               }
            }
         } catch (SocketTimeoutException sockettimeoutexception) {
            return;
         } catch (IOException ioexception) {
            return;
         } catch (Exception exception1) {
            field_164005_h.error("Exception whilst parsing RCON input", (Throwable)exception1);
            return;
         } finally {
            this.func_72653_g();
            field_164005_h.info("Thread {} shutting down", (Object)this.field_164003_c);
            this.field_72619_a = false;
         }
         return;
      }
   }

The loop is created outside the try block and has a return statement immediately after it. It should be created inside the try block

Local Variable Improper Merging

package pkg;

import java.util.ArrayList;
import java.util.Iterator;
import java.util.List;

public class TestLocalVarMerging {
    public static void test() {
        for (Iterator<String> itr = null; itr.hasNext();) {
            itr.remove();
        }
        List<Integer> lst = new ArrayList<>();
        lst.add(1);
    }
}

Bad:


import java.util.ArrayList;

public class TestLocalVarMerging {
   public static void test() {
      ArrayList var0 = null;

      while(var0.hasNext()) {
         var0.remove();
      }

      var0 = new ArrayList();
      var0.add(1);
   }
}

Invalid reference to outer class instance.

1.16-pre1 has a few lambdas in EditGamerulesScreen that reference EditGamerulesScreen.this as EditGamerulesScreen.thisx

(21w13a) net.minecraft.world.level.levelgen.SimpleRandomSource.nextGaussian fails to decompile

Method nextGaussian ()D in net/minecraft/world/level/levelgen/SimpleRandomSource couldn't be decompiled.
java.lang.IndexOutOfBoundsException: Index: 0, Size: 0
        at java.util.ArrayList.rangeCheck(ArrayList.java:659)
        at java.util.ArrayList.get(ArrayList.java:435)
        at org.jetbrains.java.decompiler.modules.decompiler.IfHelper.buildGraph(IfHelper.java:366)
        at org.jetbrains.java.decompiler.modules.decompiler.IfHelper.mergeIfs(IfHelper.java:72)
        at org.jetbrains.java.decompiler.modules.decompiler.SimplifyExprentsHelper.simplifyStackVarsStatement(SimplifyExprentsHelper.java:67)
        at org.jetbrains.java.decompiler.modules.decompiler.SimplifyExprentsHelper.simplifyStackVarsStatement(SimplifyExprentsHelper.java:65)
        at org.jetbrains.java.decompiler.modules.decompiler.SimplifyExprentsHelper.simplifyStackVarsStatement(SimplifyExprentsHelper.java:65)
        at org.jetbrains.java.decompiler.modules.decompiler.EliminateLoopsHelper.eliminateLoops(EliminateLoopsHelper.java:27)
        at org.jetbrains.java.decompiler.main.rels.MethodProcessorRunnable.codeToJava(MethodProcessorRunnable.java:157)
        at org.jetbrains.java.decompiler.main.rels.ClassWrapper.init(ClassWrapper.java:61)
        at org.jetbrains.java.decompiler.main.ClassesProcessor.initWrappers(ClassesProcessor.java:470)
        at org.jetbrains.java.decompiler.main.ClassesProcessor.processClass(ClassesProcessor.java:386)
        at org.jetbrains.java.decompiler.main.Fernflower.processClass(Fernflower.java:149)
        at org.jetbrains.java.decompiler.struct.ContextUnit.save(ContextUnit.java:217)
        at org.jetbrains.java.decompiler.struct.StructContext.saveContext(StructContext.java:63)
        at org.jetbrains.java.decompiler.main.Fernflower.decompileContext(Fernflower.java:120)
        at org.spongepowered.gradle.vanilla.worker.JarDecompileWorker.execute(JarDecompileWorker.java:90)
        at org.gradle.workers.internal.DefaultWorkerServer.execute(DefaultWorkerServer.java:63)
        at org.gradle.workers.internal.AbstractClassLoaderWorker$1.create(AbstractClassLoaderWorker.java:49)
        at org.gradle.workers.internal.AbstractClassLoaderWorker$1.create(AbstractClassLoaderWorker.java:43)
        at org.gradle.internal.classloader.ClassLoaderUtils.executeInClassloader(ClassLoaderUtils.java:97)
        at org.gradle.workers.internal.AbstractClassLoaderWorker.executeInClassLoader(AbstractClassLoaderWorker.java:43)
        at org.gradle.workers.internal.IsolatedClassloaderWorker.run(IsolatedClassloaderWorker.java:49)
        at org.gradle.workers.internal.IsolatedClassloaderWorker.run(IsolatedClassloaderWorker.java:30)
        at org.gradle.workers.internal.WorkerDaemonServer.run(WorkerDaemonServer.java:85)
        at org.gradle.workers.internal.WorkerDaemonServer.run(WorkerDaemonServer.java:55)
        at org.gradle.process.internal.worker.request.WorkerAction$1.call(WorkerAction.java:138)
        at org.gradle.process.internal.worker.child.WorkerLogEventListener.withWorkerLoggingProtocol(WorkerLogEventListener.java:41)
        at org.gradle.process.internal.worker.request.WorkerAction.run(WorkerAction.java:135)
        at sun.reflect.NativeMethodAccessorImpl.invoke0(Native Method)
        at sun.reflect.NativeMethodAccessorImpl.invoke(NativeMethodAccessorImpl.java:62)
        at sun.reflect.DelegatingMethodAccessorImpl.invoke(DelegatingMethodAccessorImpl.java:43)
        at java.lang.reflect.Method.invoke(Method.java:498)
        at org.gradle.internal.dispatch.ReflectionDispatch.dispatch(ReflectionDispatch.java:36)
        at org.gradle.internal.dispatch.ReflectionDispatch.dispatch(ReflectionDispatch.java:24)
        at org.gradle.internal.remote.internal.hub.MessageHubBackedObjectConnection$DispatchWrapper.dispatch(MessageHubBackedObjectConnection.java:182)
        at org.gradle.internal.remote.internal.hub.MessageHubBackedObjectConnection$DispatchWrapper.dispatch(MessageHubBackedObjectConnection.java:164)
        at org.gradle.internal.remote.internal.hub.MessageHub$Handler.run(MessageHub.java:414)
        at org.gradle.internal.concurrent.ExecutorPolicy$CatchAndRecordFailures.onExecute(ExecutorPolicy.java:64)
        at org.gradle.internal.concurrent.ManagedExecutorImpl$1.run(ManagedExecutorImpl.java:48)
        at java.util.concurrent.ThreadPoolExecutor.runWorker(ThreadPoolExecutor.java:1149)
        at java.util.concurrent.ThreadPoolExecutor$Worker.run(ThreadPoolExecutor.java:624)
        at org.gradle.internal.concurrent.ThreadFactoryImpl$ManagedThreadRunnable.run(ThreadFactoryImpl.java:56)
        at java.lang.Thread.run(Thread.java:748)

this came up when decompiling this week's snapshot. haven't had a chance to look into it further, can do so later.

Shortnames will be imported incorrectly when shadowed by super class inner classes.

See ItemAxe trying to use ImmutableMap.Builder but using Item.Builder.

Decompile output is inconsistent between Java 17 and 19

The following is a diff from running the latest FF on Java 17 vs 19.

diff --git a/17/net/minecraft/world/level/border/WorldBorder.java b/19/net/minecraft/world/level/border/WorldBorder.java
index 28fd83b..0dff02b 100644
--- a/17/net/minecraft/world/level/border/WorldBorder.java
+++ b/19/net/minecraft/world/level/border/WorldBorder.java
@@ -15,7 +15,7 @@ import net.minecraft.world.phys.shapes.Shapes;
 import net.minecraft.world.phys.shapes.VoxelShape;

 public class WorldBorder {
-    public static final double MAX_SIZE = 5.9999968E7D;
+    public static final double MAX_SIZE = (double)5.999997E7F;
     public static final double MAX_CENTER_COORDINATE = 2.9999984E7D;
     private final List<BorderChangeListener> listeners = Lists.newArrayList();
     private double damagePerBlock = 0.2D;
@@ -25,8 +25,8 @@ public class WorldBorder {
     private double centerX;
     private double centerZ;
     int absoluteMaxSize = 29999984;
-    private WorldBorder.BorderExtent extent = new WorldBorder.StaticBorderExtent(5.9999968E7D);
-    public static final WorldBorder.Settings DEFAULT_SETTINGS = new WorldBorder.Settings(0.0D, 0.0D, 0.2D, 5.0D, 5, 15, 5.9999968E7D, 0L, 0.0D);
+    private WorldBorder.BorderExtent extent = new WorldBorder.StaticBorderExtent((double)5.999997E7F);
+    public static final WorldBorder.Settings DEFAULT_SETTINGS = new WorldBorder.Settings(0.0D, 0.0D, 0.2D, 5.0D, 5, 15, (double)5.999997E7F, 0L, 0.0D);

     public boolean isWithinBounds(BlockPos pos) {
         return (double)(pos.getX() + 1) > this.getMinX() && (double)pos.getX() < this.getMaxX() && (double)(pos.getZ() + 1) > this.getMinZ() && (double)pos.getZ() < this.getMaxZ();

This is likely caused by https://bugs.openjdk.org/browse/JDK-8291475

Potential NPE in InvocationExprent.appendParamList

If a class that is being checked for overloaded unboxing is not found in the current context, likely due to libraries not being found on the classpath, a NullPointerException is thrown. The relevant code is here:

ForgeFlower/FernFlower-Patches/0014-Fix-primitive-un-boxing-issues.patch

Line 189 in cc2014d

+ for (StructMethod mt : stClass.getMethods()) {

Placing a null check before the loop creates no problems and the method decompiles fine.

Static field initializes that reference itself or other static fields not inlined/compileable.

So, apparently static fields can reference itself, and other static fields that are declared after it in their initalizer. IF they are fully qualified. I think this is a bug in the java spec/compiler, because the value for these fields will always be null... But hey Minecraft exposed it, so lets fix it.
See 1.16-pre EntityPredicate.

TODO: Reevaluate include-entire-classpath

The point of this is to make generics and type information better.
However it has a major memory cost as it reads ALL the classes in the classpath and parses out their metadata.

A better idea would be to make the metadata system lazy, but that runs into threading and race issues.

So, this is the TODO to look at it... eventually..

Improper for loop promotion from while

1.16-pre1 RealmsSelectWorldTemplate.func_224497_a

WorldTemplatePaginatedList worldtemplatepaginatedlist = p_224497_1_;
Either<WorldTemplatePaginatedList, String> either;
for(RealmsClient realmsclient = RealmsClient.func_224911_a(); worldtemplatepaginatedlist != null; worldtemplatepaginatedlist = RealmsSelectWorldTemplateScreen.this.field_230706_i_.func_213169_a(() -> {
   if (either.right().isPresent()) {
      ...
   } else {
      ...
   }
}).join()) {
   either = RealmsSelectWorldTemplateScreen.this.func_224509_a(worldtemplatepaginatedlist, realmsclient);
}

WorldTemplatePaginatedList worldtemplatepaginatedlist = p_224497_1_;
RealmsClient realmsclient = RealmsClient.func_224911_a();
while (worldtemplatepaginatedlist != null) {
   Either<WorldTemplatePaginatedList, String> either = RealmsSelectWorldTemplateScreen.this.func_224509_a(worldtemplatepaginatedlist, realmsclient);
   worldtemplatepaginatedlist = RealmsSelectWorldTemplateScreen.this.field_230706_i_.func_213169_a(() -> {
      if (either.right().isPresent()) {
         ...
      } else {
         ...
      }
   };
}

Method local inner record decompiled into wrong location

In the vanilla minecraft server source (1.19.3), in net.minecraft.world.level.levelgen.WorldDimensions (mojang mappings) the following happens when decompiling

public WorldDimensions.Complete bake(Registry<LevelStem> existingRegistry) {
        Stream<ResourceKey<LevelStem>> stream = Stream.concat(existingRegistry.registryKeySet().stream(), this.dimensions.registryKeySet().stream()).distinct();
        List<Entry> list = new ArrayList<>();
        keysInOrder(stream).forEach((key) -> {
            existingRegistry.getOptional(key).or(() -> {
                return this.dimensions.getOptional(key);
            }).ifPresent((dimensionOptions) -> {
                record Entry(ResourceKey<LevelStem> key, LevelStem value) {
                    Lifecycle lifecycle() {
                        return WorldDimensions.checkStability(this.key, this.value);
                    }
                }

                list.add(new Entry(key, dimensionOptions));
            });
        });
        Lifecycle lifecycle = list.size() == VANILLA_DIMENSION_COUNT ? Lifecycle.stable() : Lifecycle.experimental();
        WritableRegistry<LevelStem> writableRegistry = new MappedRegistry<>(Registries.LEVEL_STEM, lifecycle);
        list.forEach((entry) -> {
            writableRegistry.register(entry.key, entry.value, entry.lifecycle());
        });
        Registry<LevelStem> registry = writableRegistry.freeze();
        PrimaryLevelData.SpecialWorldProperty specialWorldProperty = specialWorldProperty(registry);
        return new WorldDimensions.Complete(registry.freeze(), specialWorldProperty);
    }

Need Max line length config

With @JDLogic's pr to inline simple lambdas, some complex minecraft classes are producing lines in the 1000+ characters, so we need to introduce max line lengths.

More Enhanced For loop issues

See 1.13.1 EntityPlayerSP func_71007_a

                        Iterator<AxisAlignedBB> iterator = this.field_70170_p.func_212388_b(this, axisalignedbb).<AxisAlignedBB>flatMap((p_212329_0_) -> {
                           return p_212329_0_.func_197756_d().stream();
                        }).iterator();
                        float f11 = Float.MIN_VALUE;

                        while(iterator.hasNext()) {
                           AxisAlignedBB axisalignedbb1 = iterator.next();

Enhanced For Loop Failure

1.16-pre1 net\minecraft\client\gui\widget\list\OptionsRowList

   public Optional<Widget> func_238518_c_(double p_238518_1_, double p_238518_3_) {
      label21:
      for(OptionsRowList.Row optionsrowlist$row : this.func_230969_at__()) {
         Iterator iterator = optionsrowlist$row.field_214385_a.iterator();

         Widget widget;
         while(true) {
            if (!iterator.hasNext()) {
               continue label21;
            }

            widget = (Widget)iterator.next();
            if (widget.func_230990_b_(p_238518_1_, p_238518_3_)) {
               break;
            }
         }

         return Optional.of(widget);
      }

      return Optional.empty();
   }

Improper variable merging - 1.16/1.15 Particle#func_187110_a

   public void func_187110_a(double p_187110_1_, double p_187110_3_, double p_187110_5_) {
      if (!this.field_228343_B_) {
         double d0 = p_187110_1_;
         double d1 = p_187110_3_;
         if (this.field_190017_n && (p_187110_1_ != 0.0D || p_187110_3_ != 0.0D || p_187110_5_ != 0.0D)) {
            Vector3d vector3d = Entity.func_223307_a((Entity)null, new Vector3d(p_187110_1_, p_187110_3_, p_187110_5_), this.func_187116_l(), this.field_187122_b, ISelectionContext.func_216377_a(), new ReuseableStream<>(Stream.empty()));
            p_187110_1_ = vector3d.field_72450_a;
            p_187110_3_ = vector3d.field_72448_b;
            p_187110_5_ = vector3d.field_72449_c;
         }

         if (p_187110_1_ != 0.0D || p_187110_3_ != 0.0D || p_187110_5_ != 0.0D) {
            this.func_187108_a(this.func_187116_l().func_72317_d(p_187110_1_, p_187110_3_, p_187110_5_));
            this.func_187118_j();
         }

         if (Math.abs(d1) >= (double)1.0E-5F && Math.abs(p_187110_3_) < (double)1.0E-5F) {
            this.field_228343_B_ = true;
         }

         this.field_187132_l = d1 != p_187110_3_ && d1 < 0.0D;
         if (d0 != p_187110_1_) {
            this.field_187129_i = 0.0D;
         }

         if (p_187110_5_ != p_187110_5_) {
            this.field_187131_k = 0.0D;
         }

      }
   }

the p_187110_5_ param can be modified inside the function and should be compared to its original value. Instead, it is compared to itself p_187110_5_ != p_187110_5_ which is always false.

Some variables are not condensed to invocations.

1.16-pre1 ScaffoldingBlock's static class doesn't condense stack variables into the argument list.
This causes the field initializes to not be inlined.

      VoxelShape voxelshape = Block.func_208617_a(0.0D, 14.0D, 0.0D, 16.0D, 16.0D, 16.0D);
      VoxelShape voxelshape1 = Block.func_208617_a(0.0D, 0.0D, 0.0D, 2.0D, 16.0D, 2.0D);
      VoxelShape voxelshape2 = Block.func_208617_a(14.0D, 0.0D, 0.0D, 16.0D, 16.0D, 2.0D);
      VoxelShape voxelshape3 = Block.func_208617_a(0.0D, 0.0D, 14.0D, 2.0D, 16.0D, 16.0D);
      VoxelShape voxelshape4 = Block.func_208617_a(14.0D, 0.0D, 14.0D, 16.0D, 16.0D, 16.0D);
      field_220121_d = VoxelShapes.func_216384_a(voxelshape, voxelshape1, voxelshape2, voxelshape3, voxelshape4);
      VoxelShape voxelshape5 = Block.func_208617_a(0.0D, 0.0D, 0.0D, 2.0D, 2.0D, 16.0D);
      VoxelShape voxelshape6 = Block.func_208617_a(14.0D, 0.0D, 0.0D, 16.0D, 2.0D, 16.0D);
      VoxelShape voxelshape7 = Block.func_208617_a(0.0D, 0.0D, 14.0D, 16.0D, 2.0D, 16.0D);
      VoxelShape voxelshape8 = Block.func_208617_a(0.0D, 0.0D, 0.0D, 16.0D, 2.0D, 2.0D);
      field_220122_e = VoxelShapes.func_216384_a(field_220123_f, field_220121_d, voxelshape6, voxelshape5, voxelshape8, voxelshape7);
   }

Edit: Looked into it more and it is correct to not move the initalizers up, as it would change the order from the bytecode. So when we fix this we also need to fix that check...

Casts missing parentheses and causing precision loss

Could it be FernFlower causing this bug? MinecraftForge/MinecraftForge#2321

The (int) cast needs to be applied to the entire p_181663_1_ * 127 expression, but the parentheses are left out so that it only casts p_181663_1_, changing the calculation.