From e9ea4ca169d64aca36e7bf6fb0a9ec9a6ac15c4a Mon Sep 17 00:00:00 2001 From: matyrobbrt <65940752+Matyrobbrt@users.noreply.github.com> Date: Fri, 16 Dec 2022 18:44:56 +0200 Subject: [PATCH] Separate checkAndFix from the check* tasks. (#9213) --- build.gradle | 45 ++- buildSrc/build.gradle | 1 + .../forge/tasks/CheckATs.groovy | 267 --------------- .../forge/tasks/CheckExcs.groovy | 94 ------ .../forge/tasks/CheckPatches.groovy | 219 ------------- .../forge/tasks/CheckSAS.groovy | 107 ------ .../forge/tasks/InheritanceData.groovy | 49 +++ .../forge/tasks/checks/CheckATs.groovy | 310 ++++++++++++++++++ .../forge/tasks/checks/CheckExcs.groovy | 93 ++++++ .../forge/tasks/checks/CheckMode.groovy | 9 + .../forge/tasks/checks/CheckPatches.groovy | 177 ++++++++++ .../forge/tasks/checks/CheckSAS.groovy | 110 +++++++ .../forge/tasks/checks/CheckTask.groovy | 109 ++++++ .../resources/META-INF/accesstransformer.cfg | 5 +- src/main/resources/forge.exc | 2 +- src/main/resources/forge.sas | 6 +- 16 files changed, 894 insertions(+), 709 deletions(-) delete mode 100644 buildSrc/src/main/groovy/net/minecraftforge/forge/tasks/CheckATs.groovy delete mode 100644 buildSrc/src/main/groovy/net/minecraftforge/forge/tasks/CheckExcs.groovy delete mode 100644 buildSrc/src/main/groovy/net/minecraftforge/forge/tasks/CheckPatches.groovy delete mode 100644 buildSrc/src/main/groovy/net/minecraftforge/forge/tasks/CheckSAS.groovy create mode 100644 buildSrc/src/main/groovy/net/minecraftforge/forge/tasks/InheritanceData.groovy create mode 100644 buildSrc/src/main/groovy/net/minecraftforge/forge/tasks/checks/CheckATs.groovy create mode 100644 buildSrc/src/main/groovy/net/minecraftforge/forge/tasks/checks/CheckExcs.groovy create mode 100644 buildSrc/src/main/groovy/net/minecraftforge/forge/tasks/checks/CheckMode.groovy create mode 100644 buildSrc/src/main/groovy/net/minecraftforge/forge/tasks/checks/CheckPatches.groovy create mode 100644 buildSrc/src/main/groovy/net/minecraftforge/forge/tasks/checks/CheckSAS.groovy create mode 100644 buildSrc/src/main/groovy/net/minecraftforge/forge/tasks/checks/CheckTask.groovy diff --git a/build.gradle b/build.gradle index c4878c597f..e074dfb004 100644 --- a/build.gradle +++ b/build.gradle @@ -1,4 +1,10 @@ import groovy.json.JsonBuilder +import net.minecraftforge.forge.tasks.checks.CheckATs +import net.minecraftforge.forge.tasks.checks.CheckExcs +import net.minecraftforge.forge.tasks.checks.CheckPatches +import net.minecraftforge.forge.tasks.checks.CheckSAS +import net.minecraftforge.forge.tasks.checks.CheckTask +import net.minecraftforge.forge.tasks.checks.CheckMode import java.nio.file.Files import java.util.Date @@ -1168,34 +1174,45 @@ project(':forge') { } } - task checkATs(type: CheckATs, dependsOn: [extractInheritance, createSrg2Mcp]) { - inheritance = extractInheritance.output + tasks.register('checkAll') { + dependsOn 'checkLicenses' + group = 'checks' + } + tasks.register('checkAllAndFix') { + dependsOn 'findFieldInstanceChecks', 'checkLicenses' + group = 'checks' + } + + CheckTask.registerTask(tasks, 'ATs', CheckATs) { + dependsOn extractInheritance, createSrg2Mcp ats.from patcher.accessTransformers + inheritance = extractInheritance.output mappings = createSrg2Mcp.output } - task checkSAS(type: CheckSAS, dependsOn: extractInheritance) { - inheritance = extractInheritance.output + CheckTask.registerTask(tasks, 'SAS', CheckSAS) { + dependsOn extractInheritance sass.from patcher.sideAnnotationStrippers + inheritance = extractInheritance.output } - task checkExcs(type: CheckExcs, dependsOn: jar) { + CheckTask.registerTask(tasks, 'Excs', CheckExcs) { + dependsOn jar binary = jar.archiveFile.get().asFile excs.from patcher.excs } - task checkPatchesAndFix(type: CheckPatches, dependsOn: genPatches) { + CheckTask.registerTask(tasks, 'Patches', CheckPatches) { + dependsOn genPatches patchDir = file("$rootDir/patches") - autoFix = true - } - - task checkPatches(type: CheckPatches, dependsOn: genPatches) { - patchDir = file("$rootDir/patches") - autoFix = false + patchesWithS2SArtifact = [ + 'minecraft/net/minecraft/client/renderer/ViewArea.java.patch', + 'minecraft/net/minecraft/data/models/blockstates/Variant.java.patch', + ] } genPatches { - finalizedBy checkPatches + finalizedBy checkAndFixPatches autoHeader true lineEnding = '\n' } @@ -1251,8 +1268,6 @@ project(':forge') { commonLibraries.from(project.configurations.moduleonly) } - task checkAll(dependsOn: [checkATs, checkSAS, checkExcs, findFieldInstanceChecks, checkPatches, checkLicenses]){} - task launcherJson(type: LauncherJson) { packedDependencies = [':fmlloader:jar'] doFirst { diff --git a/buildSrc/build.gradle b/buildSrc/build.gradle index 72b9a64784..3762587968 100644 --- a/buildSrc/build.gradle +++ b/buildSrc/build.gradle @@ -8,4 +8,5 @@ dependencies { implementation 'org.ow2.asm:asm-tree:9.3' implementation 'net.minecraftforge:srgutils:0.4.+' implementation 'commons-io:commons-io:2.8.0' + implementation 'com.google.code.gson:gson:2.10' } \ No newline at end of file diff --git a/buildSrc/src/main/groovy/net/minecraftforge/forge/tasks/CheckATs.groovy b/buildSrc/src/main/groovy/net/minecraftforge/forge/tasks/CheckATs.groovy deleted file mode 100644 index 85a7ab610a..0000000000 --- a/buildSrc/src/main/groovy/net/minecraftforge/forge/tasks/CheckATs.groovy +++ /dev/null @@ -1,267 +0,0 @@ -package net.minecraftforge.forge.tasks - -import net.minecraftforge.srgutils.IMappingFile -import org.gradle.api.file.ConfigurableFileCollection -import org.gradle.api.file.RegularFileProperty - -import org.gradle.api.DefaultTask -import org.gradle.api.tasks.InputFile -import org.gradle.api.tasks.InputFiles -import org.gradle.api.tasks.Optional -import org.gradle.api.tasks.TaskAction -import org.objectweb.asm.Opcodes - -abstract class CheckATs extends DefaultTask { - @InputFile abstract RegularFileProperty getInheritance() - @InputFiles abstract ConfigurableFileCollection getAts() - @InputFile @Optional abstract RegularFileProperty getMappings() - - @TaskAction - protected void exec() { - def mappings = mappings.map { IMappingFile.load(it.asFile) }.getOrNull() - Util.init() - def parse = { line -> - def idx = line.indexOf('#') - def comment = idx == -1 ? null : line.substring(idx) - if (idx != -1) line = line.substring(0, idx - 1) - def (modifier, cls, desc) = (line.trim() + ' ').split(' ', -1) - def key = cls + (desc.isEmpty() ? '' : ' ' + desc) - return [modifier, cls, desc, comment, key] - } - def accessLevel = { access -> - if ((access & Opcodes.ACC_PUBLIC) != 0) return 3 - if ((access & Opcodes.ACC_PROTECTED) != 0) return 2 - if ((access & Opcodes.ACC_PRIVATE) != 0) return 0 - return 1 - } - def accessStr = { access -> - if (access.endsWith('-f') || access.endsWith('+f')) - return 4 - switch (access.toLowerCase()) { - case 'public': return 3 - case 'protected': return 2 - case 'default': return 1 - case 'private': return 0 - default: return -1 - } - } - def json = inheritance.get().asFile.json() - - ats.each { f -> - TreeMap lines = [:] - def group = null - for (def line : f.readLines()) { - if (line.isEmpty()) continue - if (line.startsWith('#group ')) { - def (modifier, cls, desc, comment, key) = parse.call(line.substring(7)) - - if (desc != '*' && desc != '*()' && desc != '') - throw new IllegalStateException('Invalid group: ' + line) - - group = [modifier: modifier, cls: cls, desc: desc, comment: comment, - existing: [] as Set, - children: [] as TreeSet, - group: true - ] - if (lines.containsKey(key)) - throw new IllegalStateException('Duplicate group: ' + line) - - lines[key] = group - } else if (group != null) { - if (line.startsWith('#endgroup')) { - group = null - } else { - def (modifier, cls, desc, comment, key) = parse.call(line) - group['existing'].add(key) - } - } else if (line.startsWith('#endgroup')) { - throw new IllegalStateException('Invalid group ending: ' + line) - } else if (line.startsWith('#')) { - //Nom - } else { - def (modifier, cls, desc, comment, key) = parse.call(line) - if (lines.containsKey(key)) { - println('Duplicate: ' + line) - continue - } - lines[key] = [modifier: modifier, cls: cls, desc: desc, comment: comment, group: false] - } - } - - // Process Groups, this will remove any entries outside the group that is covered by the group - for (def key : new ArrayList<>(lines.keySet())) { - def entry = lines.get(key) - if (entry != null && entry['group']) { - def cls = entry['cls'] - def jcls = json.get(cls.replaceAll('\\.', '/')) - if (jcls == null) { - lines.remove(key) - println('Invalid Group: ' + key) - } else if ('*' == entry['desc']) { - if (!jcls.containsKey('fields')) { - lines.remove(key) - println('Invalid Group, Class has no fields: ' + key) - } else { - jcls['fields'].each { field, value -> - def fkey = cls + ' ' + field - if (accessLevel.call(value['access']) < accessStr.call(entry['modifier'])) { - if (lines.containsKey(fkey)) { - lines.remove(fkey) - } else if (!entry['existing'].contains(fkey)) { - println('Added: ' + fkey) - } - entry['children'].add(fkey) - } else if (lines.containsKey(fkey)) { - lines.remove(fkey) - println('Removed: ' + fkey) - } - } - entry['existing'].stream().findAll{ !entry['children'].contains(it) }.each{ println('Removed: ' + it) } - } - } else if ('*()' == entry['desc']) { - if (!jcls.containsKey('methods')) { - lines.remove(key) - println('Invalid Group, Class has no methods: ' + key) - } else { - jcls['methods'].each{ mtd, value -> - if (mtd.startsWith('') || mtd.startsWith('lambda$')) - return - key = cls + ' ' + mtd.replace(' ', '') - if (accessLevel.call(value['access']) < accessStr.call(entry['modifier'])) { - if (lines.containsKey(key)) { - lines.remove(key) - } else if (!entry['existing'].contains(key)) { - println('Added: ' + key) - } - entry['children'].add(key) - } else if (lines.containsKey(key)) { - lines.remove(key) - println('Removed: ' + key) - } - } - entry['existing'].stream().findAll{ !entry['children'].contains(it) }.each{ println('Removed: ' + it) } - } - } else if ('' == entry['desc']) { //Make all public non-abstract subclasses - json.each{ tcls,value -> - if (!value.containsKey('methods') || ((value['access'] & Opcodes.ACC_ABSTRACT) != 0)) - return - def parents = [] as Set - def parent = tcls - while (parent != null && json.containsKey(parent)) { - parents.add(parent) - def p = json[parent] - parent = p == null ? null : p['superName'] - } - if (parents.contains(cls.replaceAll('\\.', '/'))) { - value['methods'].each{ mtd, v -> - if (mtd.startsWith('')) { - def child = tcls.replaceAll('/', '\\.') + ' ' + mtd.replace(' ', '') - if (accessLevel.call(v['access']) < 3) { - if (lines.containsKey(child)) { - lines.remove(child) - } else if (!entry['existing'].contains(child)) { - println('Added: ' + child) - } - entry['children'].add(child) - } else if (lines.containsKey(child)) { - lines.remove(child) - println('Removed: ' + child) - } - } - } - } - } - entry['existing'].stream().findAll{ !entry['children'].contains(it) }.each{ println('Removed: ' + it) } - } - } - } - - // Process normal lines, remove invalid and remove narrowing - for (def key : new ArrayList<>(lines.keySet())) { - def entry = lines.get(key) - if (entry != null && !entry['group']) { - def cls = entry['cls'] - def jcls = json.get(cls.replaceAll('\\.', '/')) - if (jcls == null) { - lines.remove(key) - println('Invalid: ' + key) - } else if (entry['desc'] == '') { - if (accessLevel.call(jcls['access']) > accessStr.call(entry['modifier']) && (entry.comment == null || !entry.comment.startsWith('#force '))) { - lines.remove(key) - println('Invalid Narrowing: ' + key) - } - } else if (!entry['desc'].contains('(')) { - if (!jcls.containsKey('fields') || !jcls['fields'].containsKey(entry['desc'])) { - lines.remove(key) - println('Invalid: ' + key) - } else { - def value = jcls['fields'][entry['desc']] - if (accessLevel.call(value['access']) > accessStr.call(entry['modifier']) && (entry.comment == null || !entry.comment.startsWith('#force '))) { - lines.remove(key) - println('Invalid Narrowing: ' + key) - println(entry.comment) - } - } - } else { - def jdesc = entry['desc'].replace('(', ' (') - if (!jcls.containsKey('methods') || !jcls['methods'].containsKey(jdesc)) { - lines.remove(key) - println('Invalid: ' + key) - } else { - def value = jcls['methods'][jdesc] - if (accessLevel.call(value['access']) > accessStr.call(entry['modifier']) && (entry.comment == null || !entry.comment.startsWith('#force '))) { - lines.remove(key) - println('Invalid Narrowing: ' + key) - } - } - } - } - } - - - def data = [] - def remapComment = { entry -> - if (!mappings || !entry || !entry.desc) return null - def comment = entry.comment?.substring(1)?.trim() - def jsonCls = json.get(entry.cls.replaceAll('\\.', '/')) - def mappingsClass = mappings?.getClass(jsonCls.name) - if (!mappingsClass) return entry.comment - def idx = entry.desc.indexOf('(') - def mappedName = idx == -1 - ? mappingsClass.remapField(entry.desc) - : mappingsClass.remapMethod(entry.desc.substring(0, idx), entry.desc.substring(idx)) - if (!mappedName) return entry.comment - if (mappedName == '') - mappedName = 'constructor' - - if (comment?.startsWith(mappedName)) - return '# ' + comment - if (comment && comment.indexOf(' ') != -1) { - def split = comment.split(" - ").toList() - if (split[0].indexOf(' ') != -1) - // The first string is more than one word, so append before it - return "# ${mappedName} - ${comment}" - split.remove(0) - return "# ${mappedName} - ${String.join(' - ', split)}" - } - return '# ' + mappedName - } - lines.each { key,value -> - if (!value.group) { - def comment = remapComment.call(value) - data.add(value.modifier + ' ' + key + (comment ? ' ' + comment : '')) - } else { - data.add('#group ' + value.modifier + ' ' + key + (value.comment == null ? '' : ' ' + value.comment)) - value.children.each { - def line = value.modifier + ' ' + it - def (modifier, cls, desc, comment) = parse.call(line) - comment = remapComment.call([modifier: modifier, cls: cls, desc: desc, comment: comment]) - data.add(line + (comment ? ' ' + comment : '')) - } - data.add('#endgroup') - } - } - f.text = data.join('\n') - } - } -} diff --git a/buildSrc/src/main/groovy/net/minecraftforge/forge/tasks/CheckExcs.groovy b/buildSrc/src/main/groovy/net/minecraftforge/forge/tasks/CheckExcs.groovy deleted file mode 100644 index cdc65a5859..0000000000 --- a/buildSrc/src/main/groovy/net/minecraftforge/forge/tasks/CheckExcs.groovy +++ /dev/null @@ -1,94 +0,0 @@ -package net.minecraftforge.forge.tasks - -import org.gradle.api.file.ConfigurableFileCollection -import org.gradle.api.file.RegularFileProperty - -import org.gradle.api.DefaultTask -import org.gradle.api.tasks.InputFile -import org.gradle.api.tasks.InputFiles -import org.gradle.api.tasks.TaskAction - -import org.objectweb.asm.ClassReader -import org.objectweb.asm.ClassVisitor -import org.objectweb.asm.MethodVisitor -import org.objectweb.asm.Opcodes -import org.objectweb.asm.Type - -import java.util.zip.ZipEntry -import java.util.zip.ZipInputStream - -abstract class CheckExcs extends DefaultTask { - @InputFile abstract RegularFileProperty getBinary() - @InputFiles abstract ConfigurableFileCollection getExcs() - - @TaskAction - protected void exec() { - Util.init() - def known = [] - binary.get().asFile.withInputStream { i -> - new ZipInputStream(i).withCloseable { zin -> - def visitor = new ClassVisitor(Opcodes.ASM9) { - private String cls - @Override - public void visit(int version, int access, String name, String signature, String superName, String[] interfaces) { - this.cls = name - } - - @Override - public MethodVisitor visitMethod(int access, String name, String descriptor, String signature, String[] exceptions) { - known.add(this.cls + '.' + name + descriptor) - super.visitMethod(access, name, descriptor, signature, exceptions) - } - } - ZipEntry zein - while ((zein = zin.nextEntry) != null) { - if (zein.name.endsWith('.class')) { - ClassReader reader = new ClassReader(zin) - reader.accept(visitor, ClassReader.SKIP_CODE | ClassReader.SKIP_DEBUG | ClassReader.SKIP_FRAMES) - } - } - } - } - - excs.each { f -> - def lines = [] - f.eachLine { line -> - def idx = line.indexOf('#') - if (idx == 0 || line.isEmpty()) { - return - } - - def comment = idx == -1 ? null : line.substring(idx) - if (idx != -1) line = line.substring(0, idx - 1) - - if (!line.contains('=')) { - println('Invalid: ' + line) - return - } - - def (key, value) = line.split('=', 2) - if (!known.contains(key)) { - println('Unknown: ' + line) - return - } - - def (cls, desc) = key.split('\\.', 2) - if (!desc.contains('(')) { - println('Invalid: ' + line) - return - } - def name = desc.split('\\(', 2)[0] - desc = '(' + desc.split('\\(', 2)[1] - - def (exceptions, args) = value.contains('|') ? value.split('|', 2) : [value, ''] - - if (args.split(',').length != Type.getArgumentTypes(desc).length) { - println('Invalid: ' + line) - return - } - lines.add(line) - } - f.text = lines.sort().join('\n') - } - } -} diff --git a/buildSrc/src/main/groovy/net/minecraftforge/forge/tasks/CheckPatches.groovy b/buildSrc/src/main/groovy/net/minecraftforge/forge/tasks/CheckPatches.groovy deleted file mode 100644 index 64a0407552..0000000000 --- a/buildSrc/src/main/groovy/net/minecraftforge/forge/tasks/CheckPatches.groovy +++ /dev/null @@ -1,219 +0,0 @@ -package net.minecraftforge.forge.tasks - -import org.gradle.api.DefaultTask -import org.gradle.api.file.DirectoryProperty -import org.gradle.api.tasks.Input -import org.gradle.api.tasks.InputDirectory -import org.gradle.api.tasks.TaskAction - -import java.nio.file.Files -import java.nio.file.Paths -import java.util.regex.Pattern - -abstract class CheckPatches extends DefaultTask { - @InputDirectory abstract DirectoryProperty getPatchDir() - @Input boolean autoFix = false - - @TaskAction - protected void exec() { - def hasS2SArtifact = [ - Paths.get("patches/minecraft/net/minecraft/client/renderer/ViewArea.java.patch"), - Paths.get("patches/minecraft/net/minecraft/data/models/blockstates/Variant.java.patch") - ] - - def verified = true; - patchDir.get().asFileTree.each { patch -> - def patchPath = project.rootDir.toPath().relativize(patch.toPath()) - verified &= verifyPatch(patch, autoFix, patchPath.toString(), hasS2SArtifact.contains(patchPath)) - } - - if (!verified) - throw new RuntimeException('One or more patches failed verification. Check the log for errors.') - } - - def verifyPatch(patch, fix, patchPath, hasS2SArtifact) { - def hunk_start_pattern = Pattern.compile('^@@ -[0-9,]* \\+[0-9,_]* @@$') - def white_space_pattern = Pattern.compile('^[+\\-]\\s*$') - def import_pattern = Pattern.compile('^[+\\-]\\s*import.*') - def field_pattern = Pattern.compile('^[+\\-][\\s]*((public|protected|private)[\\s]*)?(static[\\s]*)?(final)?([^=;]*)(=.*)?;\\s*$') - def method_pattern = Pattern.compile('^[+\\-][\\s]*((public|protected|private)[\\s]*)?(static[\\s]*)?(final)?([^(]*)[(]([^)]*)?[)]\\s*[{]\\s*$') - def class_pattern = Pattern.compile('^[+\\-][\\s]*((public|protected|private)[\\s]*)?(static[\\s]*)?(final[\\s]*)?(class|interface)([^{]*)[{]\\s*$') - - def accessMap = [("private"):0, (null):1, ("protected"):2, ("public"):3] - - def hasProblem = false; - def lines = patch.readLines() - - def hunksStart = 0 - def onlyWhiteSpace = false - - def didFix = false - def newLines = [] - def whiteSpaceErrors = [] - - // First two lines are file name ++/-- and we do not care - newLines.add(lines[0] + '\n') - newLines.add(lines[1] + '\n') - - int i = 2 - for (; i < lines.size(); ++i) { - def line = lines[i] - newLines.add(line + '\n') - - if (hunk_start_pattern.matcher(line).find()) { - if (onlyWhiteSpace) { - if (fix || hasS2SArtifact) { - logger.lifecycle("Removing white space hunk starting at line {}, file: {}", hunksStart + 1, patchPath) - def toRemove = i - hunksStart; - while (toRemove-- > 0) - newLines.remove(newLines.size() - 1) - didFix = true - } - else { - logger.lifecycle("Patch contains only white space hunk starting at line {}, file: {}", hunksStart + 1, patchPath) - hasProblem = true - } - } - else { - if (!whiteSpaceErrors.empty) - hasProblem = true - whiteSpaceErrors.each { error -> logger.lifecycle(error) } - whiteSpaceErrors.clear() - } - hunksStart = i - onlyWhiteSpace = true - continue - } - - if (line.startsWithAny('+','-')) { - def prefixChange = false - def prevLine = lines[i - 1] - - if (line.charAt(0) == (char)'+' && prevLine.charAt(0) == (char)'-') { - def prevTrim = prevLine.substring(1).replaceAll("\\s", "") - def currTrim = line.substring(1).replaceAll("\\s", "") - - if (prevTrim.equals(currTrim)) { - prefixChange = true - } - - def pMatcher = field_pattern.matcher(prevLine) - def cMatcher = field_pattern.matcher(line) - - if (pMatcher.find() && cMatcher.find() && - pMatcher.group(6) == cMatcher.group(6) && // = ... - pMatcher.group(5) == cMatcher.group(5) && // field name - pMatcher.group(3) == cMatcher.group(3) && // static - (accessMap[pMatcher.group(2)] < accessMap[cMatcher.group(2)] || pMatcher.group(4) != cMatcher.group(4))) { - logger.lifecycle("Patch contains access changes or final removal at line {}, file: {}", i + 1, patchPath) - hasProblem = true - } - - pMatcher = method_pattern.matcher(prevLine) - cMatcher = method_pattern.matcher(line) - - if (pMatcher.find() && cMatcher.find() && - pMatcher.group(6) == cMatcher.group(6) && // params - pMatcher.group(5) == cMatcher.group(5) && // void name - pMatcher.group(3) == cMatcher.group(3) && // static - (accessMap[pMatcher.group(2)] < accessMap[cMatcher.group(2)] || pMatcher.group(4) != cMatcher.group(4))) { - logger.lifecycle("Patch contains access changes or final removal at line {}, file: {}", i + 1, patchPath) - hasProblem = true - } - - pMatcher = class_pattern.matcher(prevLine) - cMatcher = class_pattern.matcher(line) - - if (pMatcher.find() && cMatcher.find() && - pMatcher.group(6) == cMatcher.group(6) && // ClassName<> extends ... - pMatcher.group(5) == cMatcher.group(5) && // class | interface - pMatcher.group(3) == cMatcher.group(3) && // static - (accessMap[pMatcher.group(2)] < accessMap[cMatcher.group(2)] || pMatcher.group(4) != cMatcher.group(4))) { - logger.lifecycle("Patch contains access changes or final removal at line {}, file: {}", i + 1, patchPath) - hasProblem = true - } - } - - if (line.charAt(0) == (char)'-' && i + 1 < lines.size()) { - def nextLine = lines[i + 1] - if (nextLine.charAt(0) == (char)'+') { - def nextTrim = nextLine.substring(1).replaceAll("\\s", "") - def currTrim = line.substring(1).replaceAll("\\s", "") - - if (nextTrim.equals(currTrim)) { - prefixChange = true - } - } - } - - def isWhiteSpaceChange = white_space_pattern.matcher(line).find() - - if (!prefixChange && !isWhiteSpaceChange) { - onlyWhiteSpace = hasS2SArtifact && import_pattern.matcher(line).find() - } - else if (isWhiteSpaceChange) { - def prevLineChange = prevLine.startsWithAny('+','-') - def nextLineChange = i + 1 < lines.size() && lines[i + 1].startsWithAny('+','-') - - if (!prevLineChange && !nextLineChange) { - whiteSpaceErrors.add(String.format("Patch contains white space change in valid hunk at line %d (cannot auto fix), file: %s", i + 1, patchPath)) - } - } - - if (line.contains("\t")) { - if (!fix) { - logger.lifecycle("Patch contains tabs on line {}, file: {}", i + 1, patchPath) - hasProblem = true - } - else { - logger.lifecycle("Fixing tabs on line {}, file: {}", i + 1, patchPath) - line = line.replaceAll('\t', ' ') - newLines.remove(newLines.size() - 1) - newLines.add(line + '\n') - didFix = true - } - } - - if (import_pattern.matcher(line).find() && !hasS2SArtifact) { - logger.lifecycle("Patch contains import change on line {}, file: {}", i + 1, patchPath) - hasProblem = true - } - } - } - - if (onlyWhiteSpace) { - if (fix || hasS2SArtifact) { - logger.lifecycle("Removing white space hunk starting at line {}, file: {}", hunksStart + 1, patchPath) - def toRemove = i - hunksStart; - while (toRemove-- > 0) - newLines.remove(newLines.size() - 1) - didFix = true - } - else { - logger.lifecycle("Patch contains only white space hunk starting at line {}, file: {}", hunksStart + 1, patchPath) - hasProblem = true - } - } - else { - if (!whiteSpaceErrors.empty) - hasProblem = true - whiteSpaceErrors.each { error -> logger.lifecycle(error) } - } - - if (didFix) { - if (newLines.size() <= 2) { - logger.lifecycle("Patch is now empty removing, file: {}", patchPath) - Files.delete(patch.toPath()) - } - else { - if (!hasS2SArtifact) - logger.lifecycle("*** Updating patch file. Please run setup then genPatches again. ***") - patch.withWriter('UTF-8') { writer -> - newLines.each { l -> writer.write(l) } - } - } - } - - return !hasProblem - } -} diff --git a/buildSrc/src/main/groovy/net/minecraftforge/forge/tasks/CheckSAS.groovy b/buildSrc/src/main/groovy/net/minecraftforge/forge/tasks/CheckSAS.groovy deleted file mode 100644 index f9c44cfb1a..0000000000 --- a/buildSrc/src/main/groovy/net/minecraftforge/forge/tasks/CheckSAS.groovy +++ /dev/null @@ -1,107 +0,0 @@ -package net.minecraftforge.forge.tasks - -import org.gradle.api.file.ConfigurableFileCollection -import org.gradle.api.file.RegularFileProperty - -import org.gradle.api.DefaultTask -import org.gradle.api.tasks.InputFile -import org.gradle.api.tasks.InputFiles -import org.gradle.api.tasks.TaskAction - -abstract class CheckSAS extends DefaultTask { - @InputFile abstract RegularFileProperty getInheritance() - @InputFiles abstract ConfigurableFileCollection getSass() - - @TaskAction - protected void exec() { - Util.init() - def json = inheritance.get().asFile.json() - - sass.each { f -> - def lines = [] - f.eachLine { line -> - if (line[0] == '\t') return //Skip any tabbed lines, those are ones we add - def idx = line.indexOf('#') - if (idx == 0 || line.isEmpty()) { - lines.add(line) - return - } - def comment = idx == -1 ? null : line.substring(idx) - if (idx != -1) line = line.substring(0, idx - 1) - def (cls, name, desc) = (line.trim().replace('(', ' (') + ' ').split(' ', -1) - cls = cls.replaceAll('\\.', '/') - - if (json[cls] == null) { - println('Invalid: ' + line) - } else if (name.isEmpty()) { //Class SAS - def toAdd = [] - def clsSided = isSided(json[cls]) - def sided = clsSided - - /* TODO: MergeTool doesn't do fields - if (json[cls]['fields'] != null) { - for (entry in json[cls]['fields']) { - if (isSided(entry.value)) { - sided = true - toAdd.add('\t' + cls + ' ' + entry.key) - } - } - } - */ - if (json[cls]['methods'] != null) { - for (entry in json[cls]['methods']) { - if (isSided(entry.value)) { - sided = true - toAdd.add('\t' + cls + ' ' + entry.key.replaceAll(' ', '')) - findChildMethods(json, cls, entry.key).each { lines.add('\t' + it) } - findChildMethods(json, cls, entry.key).each { println(line + ' -- ' + it) } - } else if (clsSided) { - findChildMethods(json, cls, entry.key).each { lines.add('\t' + it) } - findChildMethods(json, cls, entry.key).each { println(line + ' -- ' + it) } - } - } - } - - if (sided) { - lines.add(cls + (comment == null ? '' : ' ' + comment)) - lines.addAll(toAdd.sort()) - } else - println('Invalid: ' + line) - - } else if (desc.isEmpty()) { // Fields - /* TODO: MergeTool doesn't do fields - if (json[cls]['fields'] != null && isSided(json[cls]['fields'][name])) - lines.add(cls + ' ' + name + (comment == null ? '' : ' ' + comment)) - else - */ - println('Invalid: ' + line) - } else { // Methods - if (json[cls]['methods'] == null || !isSided(json[cls]['methods'][name + ' ' + desc])) - println('Invalid: ' + line) - else { - lines.add(cls + ' ' + name + desc + (comment == null ? '' : ' ' + comment)) - findChildMethods(json, cls, name + ' ' + desc).each { println(line + ' -- ' + it) } - findChildMethods(json, cls, name + ' ' + desc).each { lines.add('\t' + it) } - } - } - } - f.text = lines.join('\n') - } - } - - protected static isSided(json) { - if (json == null || json['annotations'] == null) - return false - for (ann in json['annotations']) { - if ('Lnet/minecraftforge/api/distmarker/OnlyIn;'.equals(ann.desc)) - return true - } - return false - } - - protected static findChildMethods(json, cls, desc) - { - return json.values().findAll{ it.methods != null && it.methods[desc] != null && it.methods[desc].override == cls && isSided(it.methods[desc]) } - .collect { it.name + ' ' + desc.replace(' ', '') } as TreeSet - } -} diff --git a/buildSrc/src/main/groovy/net/minecraftforge/forge/tasks/InheritanceData.groovy b/buildSrc/src/main/groovy/net/minecraftforge/forge/tasks/InheritanceData.groovy new file mode 100644 index 0000000000..546aec2018 --- /dev/null +++ b/buildSrc/src/main/groovy/net/minecraftforge/forge/tasks/InheritanceData.groovy @@ -0,0 +1,49 @@ +package net.minecraftforge.forge.tasks + +import com.google.gson.Gson +import com.google.gson.reflect.TypeToken +import groovy.transform.CompileStatic +import groovy.transform.TupleConstructor + +@CompileStatic +@TupleConstructor +final class InheritanceData implements Annotatable { + String name + int access + String superName + List interfaces = [] + Map methods = [:] + Map fields = [:] + List annotations = [] + + @TupleConstructor + static final class Method implements Annotatable { + int access + String override + List annotations = [] + } + + @TupleConstructor + static final class Field implements Annotatable { + int access + String desc + List annotations = [] + } + + @TupleConstructor + static final class Annotation { + String desc + } + + private static final Gson GSON = new Gson() + static Map parse(File file) { + try (final reader = file.newReader()) { + return GSON.fromJson(reader, new TypeToken>() {}) + } + } +} + +@CompileStatic +interface Annotatable { + List getAnnotations() +} diff --git a/buildSrc/src/main/groovy/net/minecraftforge/forge/tasks/checks/CheckATs.groovy b/buildSrc/src/main/groovy/net/minecraftforge/forge/tasks/checks/CheckATs.groovy new file mode 100644 index 0000000000..96f557d7e5 --- /dev/null +++ b/buildSrc/src/main/groovy/net/minecraftforge/forge/tasks/checks/CheckATs.groovy @@ -0,0 +1,310 @@ +package net.minecraftforge.forge.tasks.checks + +import groovy.transform.CompileStatic +import groovy.transform.TupleConstructor +import net.minecraftforge.forge.tasks.InheritanceData +import net.minecraftforge.srgutils.IMappingFile +import org.gradle.api.file.ConfigurableFileCollection +import org.gradle.api.file.RegularFile +import org.gradle.api.file.RegularFileProperty +import org.gradle.api.tasks.InputFile +import org.gradle.api.tasks.InputFiles +import org.gradle.api.tasks.Optional +import org.objectweb.asm.Opcodes + +@CompileStatic +abstract class CheckATs extends CheckTask { + + @InputFile abstract RegularFileProperty getInheritance() + @InputFiles abstract ConfigurableFileCollection getAts() + @InputFile @Optional abstract RegularFileProperty getMappings() + + @Override + void check(Reporter reporter, boolean fix) { + final IMappingFile mappings = mappings.map { RegularFile it -> IMappingFile.load(it.asFile) }.getOrNull() + final inheritance = InheritanceData.parse(this.inheritance.get().asFile) + + ats.each { + final lines = process(it, reporter, inheritance) + if (fix) { + it.text = joinBack(lines, inheritance, mappings).join('\n') + } + } + } + + private static TreeMap process(File file, Reporter reporter, Map inheritance) { + final TreeMap lines = ATParser.parse(file.readLines(), reporter) + final Map constructorGroups = [:] + + final itr = lines.entrySet().iterator() + final toRemove = [] + while (itr.hasNext()) { + final next = itr.next() + String key = next.key + final entry = next.value + if (entry === null) continue + + final binaryName = entry.cls.replaceAll('\\.', '/') + + // Process Groups, this will remove any entries outside the group that is covered by the group + if (entry.group) { + final jcls = inheritance[binaryName] + if (jcls === null) { + itr.remove() + reporter.report("Invalid group: $key") + } else if ('*' == entry.desc) { + if (!jcls.fields) { + itr.remove() + reporter.report("Invalid group, class has no fields: $key") + } else { + jcls.fields.each { field, value -> + final fkey = entry.cls + ' ' + field + if (accessLevel(value.access) < accessStr(entry.modifier)) { + if (lines.containsKey(fkey)) { + toRemove.add(fkey) + } else if (!entry.existing.contains(fkey)) { + reporter.report("Missing group entry: $fkey") + } + entry.children.add(fkey) + } else if (lines.containsKey(fkey)) { + toRemove.add(fkey) + reporter.report("Found invalid group entry: $fkey") + } + } + entry.existing.findAll { it !in entry.children }.each { println('Removed: ' + it) } + } + } else if ('*()' == entry.desc) { + if (!jcls.methods) { + itr.remove() + reporter.report("Invalid group, class has no methods: $key") + } else { + jcls.methods.each { mtd, value -> + if (mtd.startsWith('') || mtd.startsWith('lambda$')) return + key = entry.cls + ' ' + mtd.replace(' ', '') + if (accessLevel(value.access) < accessStr(entry.modifier)) { + if (lines.containsKey(key)) { + toRemove.add(key) + } else if (!entry.existing.contains(key)) { + reporter.report("Missing group entry: $key") + } + entry.children.add(key) + } else if (lines.containsKey(key)) { + toRemove.add(key) + reporter.report("Found invalid group entry: $key") + } + } + entry.existing.findAll { it !in entry.children }.each { println('Removed: ' + it) } + } + } else if ('' == entry.desc) { //Make all public non-abstract subclasses + constructorGroups.put(binaryName, entry) + } + } + + // Process normal lines, remove invalid and remove narrowing + else { + def jcls = inheritance.get(binaryName) + if (jcls === null) { + itr.remove() + reporter.report("Invalid: $key") + } else if (entry.desc == '') { + if (accessLevel(jcls.access) > accessStr(entry.modifier) && (entry.comment === null || !entry.comment.startsWith('#force '))) { + itr.remove() + reporter.report("Invalid Narrowing: $key") + } + } else if (!entry.desc.contains('(')) { + if (!jcls.fields || !jcls.fields.containsKey(entry.desc)) { + itr.remove() + reporter.report("Invalid: $key") + } else { + final value = jcls.fields[entry.desc] + if (accessLevel(value.access) > accessStr(entry.modifier) && (entry.comment === null || !entry.comment.startsWith('#force '))) { + itr.remove() + reporter.report("Invalid Narrowing: $key - ${entry.comment}") + } + } + } else { + final jdesc = entry.desc.replace('(', ' (') + if (!jcls.methods || !jcls.methods.containsKey(jdesc)) { + itr.remove() + reporter.report("Invalid: $key") + } else { + final value = jcls.methods[jdesc] + if (accessLevel(value.access) > accessStr(entry.modifier) && (entry.comment === null || !entry.comment.startsWith('#force '))) { + itr.remove() + reporter.report("Invalid Narrowing: $key") + } + } + } + } + } + + inheritance.each { tcls, value -> + if (!value.methods || ((value.access & Opcodes.ACC_ABSTRACT) !== 0)) return + String parent = tcls + while (parent !== null) { + constructorGroups[parent]?.tap { entry -> + value.methods.each { mtd, v -> + if (mtd.startsWith('')) { + final child = tcls.replaceAll('/', '\\.') + ' ' + mtd.replace(' ', '') + if (accessLevel(v.access) < 3) { + if (lines.containsKey(child)) { + toRemove.add(child) + } else if (child !in entry.existing) { + reporter.report("Missing group entry: $child") + } + entry.children.add(child) + } else if (lines.containsKey(child)) { + toRemove.add(child) + reporter.report("Found invalid group entry: $child") + } + } + } + } + parent = inheritance[parent]?.superName + } + } + constructorGroups.values().each { entry -> entry.existing.findAll { it !in entry.children }.each{ reporter.report("Found invalid group entry: $it") } } + + toRemove.each(lines.&remove) + + return lines + } + + private static List joinBack(TreeMap lines, Map inheritance, IMappingFile mappings) { + final data = [] + final remapComment = { ATParser.Entry entry -> + if (!mappings || !entry || !entry.desc) return null + final comment = entry.comment?.substring(1)?.trim() + final jsonCls = inheritance.get(entry.cls.replaceAll('\\.', '/')) + final mappingsClass = mappings?.getClass(jsonCls.name) + if (mappingsClass === null) return entry.comment + final idx = entry.desc.indexOf('(') + + String mappedName = idx == -1 + ? mappingsClass.remapField(entry.desc) + : mappingsClass.remapMethod(entry.desc.substring(0, idx), entry.desc.substring(idx)) + if (!mappedName) return entry.comment + if (mappedName == '') + mappedName = 'constructor' + + if (comment?.startsWith(mappedName)) + return '# ' + comment + if (comment && comment.indexOf(' ') !== -1) { + def split = comment.split(' - ').toList() + if (split[0].indexOf(' ') !== -1) + // The first string is more than one word, so append before it + return "# ${mappedName} - ${comment}" + split.remove(0) + return "# ${mappedName} - ${String.join(' - ', split)}" + } + return '# ' + mappedName + } + lines.each { key, value -> + if (!value.group) { + def comment = remapComment.call(value) + data.add(value.modifier + ' ' + key + (comment ? ' ' + comment : '')) + } else { + data.add(('#group ' + value.modifier + ' ' + key + ' ' + (value.comment ?: '')).trim()) + value.children.each { + final line = value.modifier + ' ' + it + final entry = ATParser.parseEntry(line) + final comment = remapComment(entry) + data.add(line + (comment ? ' ' + comment : '')) + } + data.add('#endgroup') + } + } + return data + } + + static int accessStr(String access) { + if (access.endsWith('-f') || access.endsWith('+f')) return 4 + switch (access.toLowerCase()) { + case 'public': return 3 + case 'protected': return 2 + case 'default': return 1 + case 'private': return 0 + default: return -1 + } + } + + static int accessLevel(int access) { + if ((access & Opcodes.ACC_PUBLIC) !== 0) return 3 + if ((access & Opcodes.ACC_PROTECTED) !== 0) return 2 + if ((access & Opcodes.ACC_PRIVATE) !== 0) return 0 + return 1 + } +} + +@CompileStatic +class ATParser { + static TreeMap parse(List lines, CheckTask.Reporter reporter) { + TreeMap outLines = new TreeMap<>() + Entry group = null + for (final line : lines) { + if (line.isEmpty()) continue + if (line.startsWith('#group ')) { + final entry = parseEntry(line.substring(7)) + + if (entry.desc != '*' && entry.desc != '*()' && entry.desc != '') { + reporter.report("Invalid group: $line", false) + } + + entry.group = true + entry.children = [] + entry.existing = [] + + group = entry + + if (outLines.containsKey(entry.key)) { + reporter.report("Duplicate group: $line", false) + } + + outLines[entry.key] = group + } else if (group !== null) { + if (line.startsWith('#endgroup')) { + group = null + } else { + final key = parseEntry(line).key + group.existing.add(key) + } + } else if (line.startsWith('#endgroup')) { + reporter.report("Invalid group ending: $line", false) + } else if (line.startsWith('#')) { + //Nom + } else { + final entry = parseEntry(line) + if (outLines.containsKey(entry.key)) { + reporter.report("Found duplicate: $line") + continue + } + outLines[entry.key] = entry + } + } + return outLines + } + + static Entry parseEntry(String line) { + final idx = line.indexOf('#') + final String comment = idx === -1 ? null : line.substring(idx) + if (idx !== -1) line = line.substring(0, idx - 1) + final data = (line.trim() + ' ').split(' ', -1) + new Entry(data[0], data[1], data[2], comment) + } + + @TupleConstructor + static final class Entry { + String modifier, cls, desc, comment + + Set existing + TreeSet children + boolean group = false + + @Lazy + String key = {cls + (desc.isEmpty() ? '' : ' ' + desc)}() + + Object getAt(String key) { + return getProperty(key) + } + } +} diff --git a/buildSrc/src/main/groovy/net/minecraftforge/forge/tasks/checks/CheckExcs.groovy b/buildSrc/src/main/groovy/net/minecraftforge/forge/tasks/checks/CheckExcs.groovy new file mode 100644 index 0000000000..254596bdd8 --- /dev/null +++ b/buildSrc/src/main/groovy/net/minecraftforge/forge/tasks/checks/CheckExcs.groovy @@ -0,0 +1,93 @@ +package net.minecraftforge.forge.tasks.checks + +import org.gradle.api.file.ConfigurableFileCollection +import org.gradle.api.file.RegularFileProperty +import org.gradle.api.tasks.InputFile +import org.gradle.api.tasks.InputFiles +import org.objectweb.asm.ClassReader +import org.objectweb.asm.ClassVisitor +import org.objectweb.asm.MethodVisitor +import org.objectweb.asm.Opcodes +import org.objectweb.asm.Type + +import java.util.zip.ZipEntry +import java.util.zip.ZipInputStream + +abstract class CheckExcs extends CheckTask { + + @InputFile abstract RegularFileProperty getBinary() + @InputFiles abstract ConfigurableFileCollection getExcs() + + @Override + void check(Reporter reporter, boolean fix) { + final Set known = [] + collectKnown(known) + + excs.each { f -> + final lines = [] + f.eachLine { line -> + def idx = line.indexOf('#') + if (idx == 0 || line.isEmpty()) { + return + } + + if (idx != -1) line = line.substring(0, idx - 1) + + if (!line.contains('=')) { + reporter.report("Invalid: $line") + return + } + + def (String key, String value) = line.split('=', 2) + if (!known.contains(key)) { + reporter.report("Unknown: $line") + return + } + + String desc = key.split('\\.', 2)[1] + if (!desc.contains('(')) { + reporter.report("Invalid: $line") + return + } + desc = '(' + desc.split('\\(', 2)[1] + + def (exceptions, String args) = value.contains('|') ? value.split('|', 2) : [value, ''] + + if (args.split(',').length !== Type.getArgumentTypes(desc).length) { + reporter.report("Invalid: $line") + return + } + lines.add(line) + } + + if (fix) f.text = lines.sort().join('\n') + } + } + + private void collectKnown(Collection known) { + binary.get().asFile.withInputStream { i -> + new ZipInputStream(i).withCloseable { zin -> + final visitor = new ClassVisitor(Opcodes.ASM9) { + private String cls + @Override + void visit(int version, int access, String name, String signature, String superName, String[] interfaces) { + this.cls = name + } + + @Override + MethodVisitor visitMethod(int access, String name, String descriptor, String signature, String[] exceptions) { + known.add(this.cls + '.' + name + descriptor) + super.visitMethod(access, name, descriptor, signature, exceptions) + } + } + ZipEntry zein + while ((zein = zin.nextEntry) !== null) { + if (zein.name.endsWith('.class')) { + ClassReader reader = new ClassReader(zin) + reader.accept(visitor, ClassReader.SKIP_CODE | ClassReader.SKIP_DEBUG | ClassReader.SKIP_FRAMES) + } + } + } + } + } +} diff --git a/buildSrc/src/main/groovy/net/minecraftforge/forge/tasks/checks/CheckMode.groovy b/buildSrc/src/main/groovy/net/minecraftforge/forge/tasks/checks/CheckMode.groovy new file mode 100644 index 0000000000..7778bfe789 --- /dev/null +++ b/buildSrc/src/main/groovy/net/minecraftforge/forge/tasks/checks/CheckMode.groovy @@ -0,0 +1,9 @@ +package net.minecraftforge.forge.tasks.checks + +import groovy.transform.CompileStatic + +@CompileStatic +enum CheckMode { + CHECK, + FIX +} diff --git a/buildSrc/src/main/groovy/net/minecraftforge/forge/tasks/checks/CheckPatches.groovy b/buildSrc/src/main/groovy/net/minecraftforge/forge/tasks/checks/CheckPatches.groovy new file mode 100644 index 0000000000..afcbe4e580 --- /dev/null +++ b/buildSrc/src/main/groovy/net/minecraftforge/forge/tasks/checks/CheckPatches.groovy @@ -0,0 +1,177 @@ +package net.minecraftforge.forge.tasks.checks + +import groovy.transform.CompileStatic +import org.gradle.api.file.DirectoryProperty +import org.gradle.api.provider.ListProperty +import org.gradle.api.tasks.Input +import org.gradle.api.tasks.InputDirectory +import org.gradle.api.tasks.Optional + +import java.nio.file.Files +import java.nio.file.Path +import java.util.regex.Pattern + +@CompileStatic +abstract class CheckPatches extends CheckTask { + + private static final Pattern HUNK_START_PATTERN = Pattern.compile('^@@ -[0-9,]* \\+[0-9,_]* @@$') + private static final Pattern WHITESPACE_PATTERN = Pattern.compile('^[+\\-]\\s*$') + private static final Pattern IMPORT_PATTERN = Pattern.compile('^[+\\-]\\s*import.*') + private static final Pattern FIELD_PATTERN = Pattern.compile('^[+\\-][\\s]*((public|protected|private)[\\s]*)?(static[\\s]*)?(final)?([^=;]*)(=.*)?;\\s*$') + private static final Pattern METHOD_PATTERN = Pattern.compile('^[+\\-][\\s]*((public|protected|private)[\\s]*)?(static[\\s]*)?(final)?([^(]*)[(]([^)]*)?[)]\\s*[{]\\s*$') + private static final Pattern CLASS_PATTERN = Pattern.compile('^[+\\-][\\s]*((public|protected|private)[\\s]*)?(static[\\s]*)?(final[\\s]*)?(class|interface)([^{]*)[{]\\s*$') + private static final Map ACCESS_MAP = [private: 0, protected: 2, public: 3].tap { it.put(null, 1) } + + @InputDirectory abstract DirectoryProperty getPatchDir() + @Input @Optional abstract ListProperty getPatchesWithS2SArtifact() + + @Override + void check(Reporter reporter, boolean fix) { + final patchDir = getPatchDir().get().asFile.toPath() + Files.walk(patchDir).withCloseable { + it.filter(Files.&isRegularFile).forEach { path -> + final String relativeName = patchDir.relativize(path).toString() + verifyPatch(path, reporter, fix, relativeName, patchesWithS2SArtifact.get().contains(relativeName.replace('\\', '/'))) + } + } + } + + void verifyPatch(Path patch, Reporter reporter, boolean fix, String patchPath, boolean hasS2SArtifact) { + final oldFixedErrors = reporter.fixed.size() + + final lines = Files.readAllLines(patch) + + int hunksStart = 0 + boolean onlyWhiteSpace = false + + final List newLines = [] + + // First two lines are file name ++/-- and we do not care + newLines.add(lines[0] + '\n') + newLines.add(lines[1] + '\n') + + int i + for (i = 2; i < lines.size(); ++i) { + def line = lines[i] + newLines.add(line + '\n') + + if (HUNK_START_PATTERN.matcher(line).find()) { + if (onlyWhiteSpace) { + if (!hasS2SArtifact) + reporter.report("Patch contains only white space hunk starting at line ${hunksStart + 1}, file: $patchPath") + int toRemove = i - hunksStart + while (toRemove-- > 0) + newLines.remove(newLines.size() - 1) + } + hunksStart = i + onlyWhiteSpace = true + continue + } + + if (line.startsWithAny('+','-')) { + def prefixChange = false + def prevLine = lines[i - 1] + + if (line.charAt(0) == (char)'+' && prevLine.charAt(0) == (char)'-') { + def prevTrim = prevLine.substring(1).replaceAll("\\s", "") + def currTrim = line.substring(1).replaceAll("\\s", "") + + if (prevTrim == currTrim) { + prefixChange = true + } + + def pMatcher = FIELD_PATTERN.matcher(prevLine) + def cMatcher = FIELD_PATTERN.matcher(line) + + if (pMatcher.find() && cMatcher.find() && + pMatcher.group(6) == cMatcher.group(6) && // = ... + pMatcher.group(5) == cMatcher.group(5) && // field name + pMatcher.group(3) == cMatcher.group(3) && // static + (ACCESS_MAP[pMatcher.group(2)] < ACCESS_MAP[cMatcher.group(2)] || pMatcher.group(4) != cMatcher.group(4))) { + reporter.report("Patch contains access changes or final removal at line ${i + 1}, file: $patchPath", false) + } + + pMatcher = METHOD_PATTERN.matcher(prevLine) + cMatcher = METHOD_PATTERN.matcher(line) + + if (pMatcher.find() && cMatcher.find() && + pMatcher.group(6) == cMatcher.group(6) && // params + pMatcher.group(5) == cMatcher.group(5) && // void name + pMatcher.group(3) == cMatcher.group(3) && // static + (ACCESS_MAP[pMatcher.group(2)] < ACCESS_MAP[cMatcher.group(2)] || pMatcher.group(4) != cMatcher.group(4))) { + reporter.report("Patch contains access changes or final removal at line ${i + 1}, file: $patchPath", false) + } + + pMatcher = CLASS_PATTERN.matcher(prevLine) + cMatcher = CLASS_PATTERN.matcher(line) + + if (pMatcher.find() && cMatcher.find() && + pMatcher.group(6) == cMatcher.group(6) && // ClassName<> extends ... + pMatcher.group(5) == cMatcher.group(5) && // class | interface + pMatcher.group(3) == cMatcher.group(3) && // static + (ACCESS_MAP[pMatcher.group(2)] < ACCESS_MAP[cMatcher.group(2)] || pMatcher.group(4) != cMatcher.group(4))) { + reporter.report("Patch contains access changes or final removal at line ${i + 1}, file: $patchPath", false) + } + } + + if (line.charAt(0) == (char)'-' && i + 1 < lines.size()) { + final nextLine = lines[i + 1] + if (nextLine.charAt(0) == (char)'+') { + final nextTrim = nextLine.substring(1).replaceAll("\\s", "") + final currTrim = line.substring(1).replaceAll("\\s", "") + + if (nextTrim == currTrim) { + prefixChange = true + } + } + } + + final isWhiteSpaceChange = WHITESPACE_PATTERN.matcher(line).find() + + if (!prefixChange && !isWhiteSpaceChange) { + onlyWhiteSpace = hasS2SArtifact && IMPORT_PATTERN.matcher(line).find() + } else if (isWhiteSpaceChange) { + final prevLineChange = prevLine.startsWithAny('+','-') + final nextLineChange = i + 1 < lines.size() && lines[i + 1].startsWithAny('+','-') + + if (!prevLineChange && !nextLineChange) { + reporter.report("Patch contains white space change in valid hunk at line ${i + 1}, file: $patchPath", false) + } + } + + if (line.contains('\t')) { + reporter.report("Patch contains tabs on line ${i + 1}, file: $patchPath") + line = line.replaceAll('\t', ' ') + newLines.remove(newLines.size() - 1) + newLines.add(line + '\n') + } + + if (IMPORT_PATTERN.matcher(line).find() && !hasS2SArtifact) { + reporter.report("Patch contains import change on line ${i + 1}, file: $patchPath", false) + } + } + } + + if (onlyWhiteSpace) { + if (!hasS2SArtifact) + reporter.report("Patch contains only white space hunk starting at line ${hunksStart + 1}, file: $patchPath") + def toRemove = i - hunksStart; + while (toRemove-- > 0) + newLines.remove(newLines.size() - 1) + } + + if ((reporter.fixed.size() > oldFixedErrors && fix) || hasS2SArtifact) { + if (newLines.size() <= 2) { + logger.lifecycle("Patch is now empty removing, file: {}", patchPath) + Files.delete(patch) + } + else { + if (!hasS2SArtifact) + logger.lifecycle("*** Updating patch file. Please run setup then genPatches again. ***") + Files.newBufferedWriter(patch).withCloseable { + newLines.each { l -> it.write(l) } + } + } + } + } +} diff --git a/buildSrc/src/main/groovy/net/minecraftforge/forge/tasks/checks/CheckSAS.groovy b/buildSrc/src/main/groovy/net/minecraftforge/forge/tasks/checks/CheckSAS.groovy new file mode 100644 index 0000000000..062221a609 --- /dev/null +++ b/buildSrc/src/main/groovy/net/minecraftforge/forge/tasks/checks/CheckSAS.groovy @@ -0,0 +1,110 @@ +package net.minecraftforge.forge.tasks.checks + +import groovy.transform.CompileStatic +import net.minecraftforge.forge.tasks.Annotatable +import net.minecraftforge.forge.tasks.InheritanceData +import org.gradle.api.file.ConfigurableFileCollection +import org.gradle.api.file.RegularFileProperty +import org.gradle.api.tasks.InputFile +import org.gradle.api.tasks.InputFiles + +@CompileStatic +abstract class CheckSAS extends CheckTask { + + @InputFile abstract RegularFileProperty getInheritance() + @InputFiles abstract ConfigurableFileCollection getSass() + + @Override + void check(Reporter reporter, boolean fix) { + final inheritance = InheritanceData.parse(this.inheritance.get().asFile) + + sass.each { f -> + final lines = [] + f.eachLine { line -> + if (line[0] == '\t') return // Skip any tabbed lines, those are ones we add + final idx = line.indexOf('#') + if (idx == 0 || line.isEmpty()) { + lines.add(line) + return + } + def comment = idx == -1 ? null : line.substring(idx) + if (idx != -1) line = line.substring(0, idx - 1) + final spl = (line.trim().replace('(', ' (') + ' ').split(' ', -1) + def (String cls, String name, String desc) = [spl[0], spl[1], spl[2]] + cls = cls.replaceAll('\\.', '/') + + if (inheritance[cls] === null) { + reporter.report("Invalid: $line") + } else if (name.isEmpty()) { //Class SAS + final toAdd = [] + final clsSided = isSided(inheritance[cls]) + boolean sided = clsSided + + /* TODO: MergeTool doesn't do fields + if (json[cls]['fields'] != null) { + for (entry in json[cls]['fields']) { + if (isSided(entry.value)) { + sided = true + toAdd.add('\t' + cls + ' ' + entry.key) + } + } + } */ + + final clsInh = inheritance[cls] + if (clsInh.methods) { + for (entry in clsInh.methods) { + if (isSided(entry.value)) { + sided = true + toAdd.add('\t' + cls + ' ' + entry.key.replaceAll(' ', '')) + findChildMethods(inheritance, cls, entry.key).each { lines.add('\t' + it) } + findChildMethods(inheritance, cls, entry.key).each { println(line + ' -- ' + it) } + } else if (clsSided) { + findChildMethods(inheritance, cls, entry.key).each { lines.add('\t' + it) } + findChildMethods(inheritance, cls, entry.key).each { println(line + ' -- ' + it) } + } + } + } + + if (sided) { + lines.add(cls + (comment == null ? '' : ' ' + comment)) + lines.addAll(toAdd.sort()) + } else { + reporter.report("Invalid: $line") + } + + } else if (desc.isEmpty()) { // Fields + /* TODO: MergeTool doesn't do fields + if (json[cls]['fields'] != null && isSided(json[cls]['fields'][name])) + lines.add(cls + ' ' + name + (comment == null ? '' : ' ' + comment)) + else */ + reporter.report("Invalid: $line") + } else { // Methods + final clsInh = inheritance[cls] + if (clsInh.methods === null || !isSided(clsInh.methods[name + ' ' + desc])) + reporter.report("Invalid: $line") + else { + lines.add(cls + ' ' + name + desc + (comment == null ? '' : ' ' + comment)) + findChildMethods(inheritance, cls, name + ' ' + desc).each { println(line + ' -- ' + it) } + findChildMethods(inheritance, cls, name + ' ' + desc).each { lines.add('\t' + it) } + } + } + } + + if (fix) f.text = lines.join('\n') + } + } + + protected static isSided(Annotatable annotatable) { + if (annotatable === null) return false + for (ann in annotatable.annotations) { + if ('Lnet/minecraftforge/api/distmarker/OnlyIn;' == ann.desc) + return true + } + return false + } + + protected static findChildMethods(Map json, String cls, String desc) { + return json.values().findAll{ it.methods != null && it.methods[desc] != null && it.methods[desc].override == cls && isSided(it.methods[desc]) } + .collect { it.name + ' ' + desc.replace(' ', '') } as TreeSet + } +} diff --git a/buildSrc/src/main/groovy/net/minecraftforge/forge/tasks/checks/CheckTask.groovy b/buildSrc/src/main/groovy/net/minecraftforge/forge/tasks/checks/CheckTask.groovy new file mode 100644 index 0000000000..27548c04f3 --- /dev/null +++ b/buildSrc/src/main/groovy/net/minecraftforge/forge/tasks/checks/CheckTask.groovy @@ -0,0 +1,109 @@ +package net.minecraftforge.forge.tasks.checks + +import groovy.transform.CompileStatic +import net.minecraftforge.forge.tasks.Util +import org.gradle.api.Action +import org.gradle.api.DefaultTask +import org.gradle.api.logging.LogLevel +import org.gradle.api.provider.Property +import org.gradle.api.tasks.Input +import org.gradle.api.tasks.TaskAction +import org.gradle.api.tasks.TaskContainer +import org.gradle.api.tasks.VerificationTask + +@CompileStatic +abstract class CheckTask extends DefaultTask implements VerificationTask { + @Input + abstract Property getMode() + + private boolean ignoreFailures = false + + @Input + @Override + boolean getIgnoreFailures() { + return ignoreFailures + } + + @Override + void setIgnoreFailures(boolean ignoreFailures) { + this.ignoreFailures = ignoreFailures + } + + @TaskAction + void run() { + Util.init() + + final doFix = getMode().get() === CheckMode.FIX + final Reporter reporter = new Reporter(doFix) + check(reporter, doFix) + + if (reporter.messages) { + if (getMode().get() === CheckMode.CHECK) { + logger.error("Check task '{}' found errors:\n{}", name, reporter.messages.join('\n')) + if (!ignoreFailures) { + throw new IllegalArgumentException("${reporter.messages.size()} errors were found!") + } + } else { + if (logger.isEnabled(LogLevel.DEBUG)) { + logger.warn("Check task '{}' found {} errors and fixed {}:\n{}", name, reporter.messages.size(), reporter.fixed.size(), reporter.fixed.join('\n')) + } else { + logger.warn("Check task '{}' found {} errors and fixed {}.", name, reporter.messages.size(), reporter.fixed.size()) + } + + if (reporter.notFixed) { + logger.error('{} errors could not be fixed:\n{}', reporter.notFixed.size(), reporter.notFixed.join('\n')) + if (!ignoreFailures) { + throw new IllegalArgumentException("${reporter.notFixed.size()} errors which cannot be fixed were found!") + } + } + } + } + } + + abstract void check(Reporter reporter, boolean fix) + + @CompileStatic + static final class Reporter { + final boolean trackFixed + Reporter(boolean trackFixed) { + this.trackFixed = trackFixed + } + + public final List messages = [] + + public final List fixed = [] + public final List notFixed = [] + + void report(String message, boolean canBeFixed = true) { + messages.add(message) + + if (trackFixed) { + if (canBeFixed) { + fixed.add(message) + } else { + notFixed.add(message) + } + } + } + } + + static void registerTask(TaskContainer tasks, String taskName, @DelegatesTo.Target('type') Class clazz, + @DelegatesTo(genericTypeIndex = 0, target = 'type') Closure configuration) { + taskName = taskName.capitalize() + tasks.register("check$taskName", clazz) { CheckTask task -> + configuration.setDelegate((T) task) + configuration.call(task) + task.mode.set(CheckMode.CHECK) + task.group = 'checks' + } + tasks.named('checkAll').configure { it.dependsOn("check$taskName") } + + tasks.register("checkAndFix$taskName", clazz) { CheckTask task -> + configuration.setDelegate((T) task) + configuration.call(task) + task.mode.set(CheckMode.FIX) + task.group = 'checks' + } + tasks.named('checkAllAndFix').configure { it.dependsOn("checkAndFix$taskName") } + } +} diff --git a/src/main/resources/META-INF/accesstransformer.cfg b/src/main/resources/META-INF/accesstransformer.cfg index 4b52a01f52..c0bbcb3090 100644 --- a/src/main/resources/META-INF/accesstransformer.cfg +++ b/src/main/resources/META-INF/accesstransformer.cfg @@ -1,7 +1,6 @@ public net.minecraft.advancements.CriteriaTriggers m_10595_(Lnet/minecraft/advancements/CriterionTrigger;)Lnet/minecraft/advancements/CriterionTrigger; # register default net.minecraft.client.KeyMapping f_90817_ # isDown public net.minecraft.client.Minecraft f_90987_ # textureManager -public net.minecraft.client.Minecraft m_91122_(Lnet/minecraft/world/item/ItemStack;Lnet/minecraft/world/level/block/entity/BlockEntity;)Lnet/minecraft/world/item/ItemStack; # addCustomNbtData public net.minecraft.client.Minecraft m_91271_()V # createSearchTrees public-f net.minecraft.client.Options f_92059_ # keyMappings #group protected net.minecraft.client.gui.Gui * @@ -151,6 +150,7 @@ protected net.minecraft.core.IdMapper f_122653_ # nextId protected net.minecraft.core.IdMapper f_122654_ # tToId - internal map protected net.minecraft.core.IdMapper f_122655_ # idToT - internal index list protected net.minecraft.core.MappedRegistry f_244282_ # unregisteredIntrusiveHolders +public net.minecraft.core.RegistrySynchronization$NetworkedRegistryData public net.minecraft.core.particles.ParticleType (ZLnet/minecraft/core/particles/ParticleOptions$Deserializer;)V # constructor public net.minecraft.core.particles.SimpleParticleType (Z)V # constructor protected net.minecraft.data.loot.BlockLootSubProvider m_245335_(Lnet/minecraft/world/level/ItemLike;)Lnet/minecraft/world/level/storage/loot/LootTable$Builder; # createSilkTouchOnlyTable @@ -186,10 +186,9 @@ public net.minecraft.data.recipes.packs.VanillaRecipeProvider f_244430_ # EMERAL public net.minecraft.data.recipes.packs.VanillaRecipeProvider f_244565_ # REDSTONE_SMELTABLES public net.minecraft.data.recipes.packs.VanillaRecipeProvider f_244628_ # LAPIS_SMELTABLES public net.minecraft.data.tags.IntrinsicHolderTagsProvider$IntrinsicTagAppender -public net.minecraft.data.tags.TagsProvider$TagAppender -public net.minecraft.core.RegistrySynchronization$NetworkedRegistryData protected net.minecraft.data.tags.TagsProvider f_126543_ # builders public-f net.minecraft.data.tags.TagsProvider m_6055_()Ljava/lang/String; # getName +public net.minecraft.data.tags.TagsProvider$TagAppender public net.minecraft.gametest.framework.GameTestServer (Ljava/lang/Thread;Lnet/minecraft/world/level/storage/LevelStorageSource$LevelStorageAccess;Lnet/minecraft/server/packs/repository/PackRepository;Lnet/minecraft/server/WorldStem;Ljava/util/Collection;Lnet/minecraft/core/BlockPos;)V # constructor public net.minecraft.network.protocol.status.ClientboundStatusResponsePacket f_134885_ # GSON public net.minecraft.resources.ResourceLocation m_135835_(C)Z # validNamespaceChar diff --git a/src/main/resources/forge.exc b/src/main/resources/forge.exc index bd4158af43..4b83ec41ed 100644 --- a/src/main/resources/forge.exc +++ b/src/main/resources/forge.exc @@ -87,4 +87,4 @@ net/minecraft/world/level/block/LiquidBlock.(Ljava/util/function/Supplier; net/minecraft/world/level/block/PoweredRailBlock.(Lnet/minecraft/world/level/block/state/BlockBehaviour$Properties;Z)V=|p_55218_,isPoweredRail net/minecraft/world/level/entity/PersistentEntitySectionManager.addEntityWithoutEvent(Lnet/minecraft/world/level/entity/EntityAccess;Z)Z=|p_157539_,p_157540_ net/minecraft/world/level/levelgen/structure/templatesystem/StructureTemplate.processEntityInfos(Lnet/minecraft/world/level/levelgen/structure/templatesystem/StructureTemplate;Lnet/minecraft/world/level/LevelAccessor;Lnet/minecraft/core/BlockPos;Lnet/minecraft/world/level/levelgen/structure/templatesystem/StructurePlaceSettings;Ljava/util/List;)Ljava/util/List;=|template,p_215387_0_,p_215387_1_,p_215387_2_,p_215387_3_ -net/minecraft/world/level/storage/loot/LootPool.([Lnet/minecraft/world/level/storage/loot/entries/LootPoolEntryContainer;[Lnet/minecraft/world/level/storage/loot/predicates/LootItemCondition;[Lnet/minecraft/world/level/storage/loot/functions/LootItemFunction;Lnet/minecraft/world/level/storage/loot/providers/number/NumberProvider;Lnet/minecraft/world/level/storage/loot/providers/number/NumberProvider;Ljava/lang/String;)V=|p_165128_,p_165129_,p_165130_,p_165131_,p_165132_,name +net/minecraft/world/level/storage/loot/LootPool.([Lnet/minecraft/world/level/storage/loot/entries/LootPoolEntryContainer;[Lnet/minecraft/world/level/storage/loot/predicates/LootItemCondition;[Lnet/minecraft/world/level/storage/loot/functions/LootItemFunction;Lnet/minecraft/world/level/storage/loot/providers/number/NumberProvider;Lnet/minecraft/world/level/storage/loot/providers/number/NumberProvider;Ljava/lang/String;)V=|p_165128_,p_165129_,p_165130_,p_165131_,p_165132_,name \ No newline at end of file diff --git a/src/main/resources/forge.sas b/src/main/resources/forge.sas index 077f0c5b14..e9a3db032a 100644 --- a/src/main/resources/forge.sas +++ b/src/main/resources/forge.sas @@ -1,7 +1,7 @@ # Forge Sided Annotation Stripper Config -# Please keep this file organized. And use the forge:checkSAS task to generate/validate inheretance. +# Please keep this file organized. And use the forge:checkAndFixSAS task to generate/validate inheritance. # So only add the root function that needs to have the annotation stripped. -# checkSAS will populate all overrides as nessasary. Prefixing added lines with \t -# checkSAS also supports names using . so simplest way to find the correct line to add is to get the AT line from the bot +# checkAndFixSAS will populate all overrides as nessasary. Prefixing added lines with \t +# checkAndFixSAS also supports names using . so simplest way to find the correct line to add is to get the AT line from the bot # and remove the access modifier. #================================================================================================================================== \ No newline at end of file