Skip to content

[Breaking] remove confusing descriptor leniency#18

Open
lukebemish wants to merge 4 commits intoneoforged:mainfrom
lukebemish:remove-confusing-leniency
Open

[Breaking] remove confusing descriptor leniency#18
lukebemish wants to merge 4 commits intoneoforged:mainfrom
lukebemish:remove-confusing-leniency

Conversation

@lukebemish
Copy link
Contributor

Currently, AT allows some leniency in descriptor parsing, allowing descriptors such as Ljava.lang.String; to be parsed as equivalent to Ljava/lang/String;. This is to match similar behaviour from when the old antlr grammar was used.

However, this actually allows a bit more leniency than the old pre-antlr parser, which for various reasons had inconsistent behaviour -- namely, the arguments and returnType of a MethodTarget used the .-to-/-translated descriptors but the targetName, used in some places, did not. Thus, .-containing descriptors would parse fine but fail to apply (not match any target) on old AT, whereas they both parse and apply on modern AT.

This has caused some confusion: https://discord.com/channels/313125603924639766/570666026077913098/1418191103638638734. Namely, it means there are cases where when modding for 1.21.1, ATs will validate and apply just fine when the dev env is set up with JST, but fail to apply at runtime, if they rely on the new behaviour here. This leads to dev vs prod behavioural differences, which are bad. Removing the leniency outright -- leniency which, to be fair, doesn't really have a correlate anywhere else -- seems like the most sensible solution here.

@neoforged-pr-publishing
Copy link

  • Publish PR to GitHub Packages

Copy link
Contributor

@shartte shartte left a comment

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Have to do a major bump for this

Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment

Labels

None yet

Projects

None yet

Development

Successfully merging this pull request may close these issues.

2 participants