Skip to content
New issue

Have a question about this project? Sign up for a free GitHub account to open an issue and contact its maintainers and the community.

By clicking “Sign up for GitHub”, you agree to our terms of service and privacy statement. We’ll occasionally send you account related emails.

Already on GitHub? Sign in to your account

Package renaming inconsistency for T/CSRG mappings #85

Closed
Su5eD opened this issue Aug 14, 2022 · 1 comment · Fixed by #86
Closed

Package renaming inconsistency for T/CSRG mappings #85

Su5eD opened this issue Aug 14, 2022 · 1 comment · Fixed by #86

Comments

@Su5eD
Copy link
Contributor

Su5eD commented Aug 14, 2022

The Issue

Commit 147a04d introduced an update to JarMapping#parseCsrgLine that would remove the / suffix from old package names in CSRG mappings in order to comply with the mapped package names on bukkit, which did not have a trailing slash.

if (reverse) {
packages.put(newClassName, oldClassName.substring(0, oldClassName.length() - 1));
} else {
packages.put(oldClassName.substring(0, oldClassName.length() - 1), newClassName);
}

However, this causes an inconsistency in names that breaks renaming packages.

Normally, both old and new SRG package names are expected to have trailing slashes. This should apply to other SRG-based formats as well.

// package names always either 1) suffixed with '/', or 2) equal to '.' to signify default package

Reproduction

Remaping the class name com/example/foo/Main with any of the following TSRG lines will result in invalid class names:

  • com/example/foo/ com/example/bar/ -> com/example/bar//Main, expected: com/example/bar/Main
  • com/example/foo/ . -> /Main, expected: Main

Expected behavior

SpecialSource should be able to properly handle package names' trailing slashes for all SRG-based formats.

Proposed solution

Adding the existing trailing slash handler code from parseSrgLine to parseCsrgLine as well solves the issue. It's a simple and backwards-compatible solution.

if (!newPackageName.equals(".") && !newPackageName.endsWith("/")) {
newPackageName += "/";
}
if (!oldPackageName.equals(".") && !oldPackageName.endsWith("/")) {
oldPackageName += "/";
}

@md-5
Copy link
Owner

md-5 commented Aug 16, 2022

Please open a PR?

Su5eD added a commit to Su5eD/SpecialSource that referenced this issue Aug 16, 2022
@md-5 md-5 closed this as completed in #86 Aug 20, 2022
md-5 pushed a commit that referenced this issue Aug 20, 2022
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 a pull request may close this issue.

2 participants