Skip to content

Commit

Permalink
extract "net/minecraft/unused/packageinfo/" constant
Browse files Browse the repository at this point in the history
bring the finalening, the thisening, and the line wrappening to the lint package
slightly refactor SpellingChecker::checkPlural
  • Loading branch information
supersaiyansubtlety committed Oct 22, 2024
1 parent dbfe1fb commit 02ae830
Show file tree
Hide file tree
Showing 11 changed files with 102 additions and 64 deletions.
Original file line number Diff line number Diff line change
Expand Up @@ -3,7 +3,6 @@
import org.gradle.api.DefaultTask;

public abstract class DefaultMappingsTask extends DefaultTask implements MappingsTask {
// TODO eliminate this, eliminate subclasses that just specify group
public DefaultMappingsTask(String group) {
this.setGroup(group);
}
Expand Down
Original file line number Diff line number Diff line change
@@ -1,14 +1,8 @@
package quilt.internal.tasks;

import org.gradle.api.Task;
import org.gradle.api.model.ObjectFactory;

import javax.inject.Inject;

/**
* All tasks added by {@link quilt.internal.QuiltMappingsPlugin QuiltMappingsPlugin} share this type.
*/
public interface MappingsTask extends Task {
@Inject
ObjectFactory getObjects();
}
public interface MappingsTask extends Task { }
Original file line number Diff line number Diff line change
Expand Up @@ -31,6 +31,8 @@
public abstract class GeneratePackageInfoMappingsTask extends DefaultMappingsTask implements MappingsDirOutputtingTask {
public static final String GENERATE_PACKAGE_INFO_MAPPINGS_TASK_NAME = "generatePackageInfoMappings";

public static final String DEFAULT_PACKAGE_NAME = "net/minecraft/unused/packageinfo/";

@Input
public abstract Property<String> getPackageName();

Expand All @@ -43,7 +45,7 @@ public abstract class GeneratePackageInfoMappingsTask extends DefaultMappingsTas
public GeneratePackageInfoMappingsTask() {
super(Constants.Groups.BUILD_MAPPINGS);

this.getPackageName().convention("net/minecraft/unused/packageinfo/");
this.getPackageName().convention(DEFAULT_PACKAGE_NAME);

this.getOutputDir().convention(this.getMappingsDir().zip(this.getPackageName(), Directory::dir));
}
Expand Down
Original file line number Diff line number Diff line change
Expand Up @@ -17,9 +17,6 @@ public abstract class TinyJarTask extends Jar implements MappingsTask {
public TinyJarTask() {
this.setGroup(Constants.Groups.BUILD_MAPPINGS);

// TODO see if this is necessary
this.getArchiveClassifier().convention("");

this.from(this.getMappings()).rename(original -> JAR_MAPPINGS_PATH);
}
}
8 changes: 6 additions & 2 deletions buildSrc/src/main/java/quilt/internal/tasks/lint/Checker.java
Original file line number Diff line number Diff line change
Expand Up @@ -33,7 +33,11 @@ public interface Checker<E extends Entry<?>> extends Serializable {
* @param accessProvider a function that provides the access flags of an entry
* @param errorReporter the error reporter
*/
void check(E entry, EntryMapping mapping, Function<Entry<?>, AccessFlags> accessProvider, ErrorReporter errorReporter);
void check(
E entry, EntryMapping mapping,
Function<Entry<?>, AccessFlags> accessProvider,
ErrorReporter errorReporter
);

/**
* Updates the checker with extension properties
Expand All @@ -53,7 +57,7 @@ default void update(MappingLintTask.LintParameters parameters) {
default Checker<Entry<?>> withTypeGuard(Class<E> entryType) {
return (entry, mapping, access, errorReporter) -> {
if (entryType.isInstance(entry)) {
check(entryType.cast(entry), mapping, access, errorReporter);
this.check(entryType.cast(entry), mapping, access, errorReporter);
}
};
}
Expand Down
Original file line number Diff line number Diff line change
Expand Up @@ -13,7 +13,11 @@ public class EntryNamingChecker implements Checker<Entry<?>> {
private static final String UNMAPPED_CLASS_PACKAGE = "net/minecraft/unmapped/";

@Override
public void check(Entry<?> entry, EntryMapping mapping, Function<Entry<?>, AccessFlags> accessProvider, ErrorReporter errorReporter) {
public void check(
Entry<?> entry, EntryMapping mapping,
Function<Entry<?>, AccessFlags> accessProvider,
ErrorReporter errorReporter
) {
if (mapping.targetName() == null) {
return;
}
Expand Down
Original file line number Diff line number Diff line change
Expand Up @@ -13,7 +13,7 @@ public interface ErrorReporter extends BiConsumer<Severity, String> {
* @param message the error message
*/
default void error(String message) {
accept(Severity.ERROR, message);
this.accept(Severity.ERROR, message);
}

/**
Expand All @@ -22,6 +22,6 @@ default void error(String message) {
* @param message the error message
*/
default void warning(String message) {
accept(Severity.WARNING, message);
this.accept(Severity.WARNING, message);
}
}
Original file line number Diff line number Diff line change
Expand Up @@ -11,22 +11,28 @@

public final class FieldNamingChecker implements Checker<FieldEntry> {
@Override
public void check(FieldEntry entry, EntryMapping mapping, Function<Entry<?>, AccessFlags> accessProvider, ErrorReporter errorReporter) {
public void check(
FieldEntry entry, EntryMapping mapping,
Function<Entry<?>, AccessFlags> accessProvider,
ErrorReporter errorReporter
) {
if (mapping.targetName() == null) {
return;
}

AccessFlags access = accessProvider.apply(entry);
final AccessFlags access = accessProvider.apply(entry);

if (access == null) {
throw new RuntimeException("Invalid mappings detected. Please run './gradlew dropInvalidMappings'.");
}

TypeDescriptor descriptor = entry.getDesc();
boolean isAtomic = descriptor.isType() && descriptor.getTypeEntry().getFullName().toLowerCase(Locale.ROOT).contains("atomic");
final TypeDescriptor descriptor = entry.getDesc();
final boolean isAtomic = descriptor.isType()
&& descriptor.getTypeEntry().getFullName().toLowerCase(Locale.ROOT).contains("atomic");
if (isAtomic) {
if (startsWithUppercase(mapping.targetName())) {
errorReporter.error("atomic field starts with uppercase character '" + mapping.targetName().charAt(0) + "'");
errorReporter
.error("atomic field starts with uppercase character '" + mapping.targetName().charAt(0) + "'");
}
} else if (access.isStatic() && access.isFinal()) {
if (!isConstantCase(mapping.targetName())) {
Expand All @@ -38,8 +44,9 @@ public void check(FieldEntry entry, EntryMapping mapping, Function<Entry<?>, Acc
}
} else {
if (startsWithUppercase(mapping.targetName())) {
String error = access.isFinal() ? "non-static" : "non-final";
errorReporter.error(error + " field starts with uppercase character '" + mapping.targetName().charAt(0) + "'");
final String error = access.isFinal() ? "non-static" : "non-final";
errorReporter
.error(error + " field starts with uppercase character '" + mapping.targetName().charAt(0) + "'");
}
}
}
Expand Down
Original file line number Diff line number Diff line change
Expand Up @@ -13,8 +13,12 @@ public final class JavadocChecker implements Checker<Entry<?>> {
private static final Pattern PARAM_DOC_LINE = Pattern.compile("^@param\\s+[^<].*$");

@Override
public void check(Entry<?> entry, EntryMapping mapping, Function<Entry<?>, AccessFlags> accessProvider, ErrorReporter errorReporter) {
String javadoc = mapping.javadoc();
public void check(
Entry<?> entry, EntryMapping mapping,
Function<Entry<?>, AccessFlags> accessProvider,
ErrorReporter errorReporter
) {
final String javadoc = mapping.javadoc();

if (javadoc != null && !javadoc.isEmpty()) {
if (entry instanceof LocalVariableEntry lv && lv.isArgument()) {
Expand All @@ -32,7 +36,8 @@ public void check(Entry<?> entry, EntryMapping mapping, Function<Entry<?>, Acces
}
} else if (entry instanceof MethodEntry) {
if (javadoc.lines().anyMatch(JavadocChecker::isRegularMethodParameter)) {
errorReporter.error("method javadoc contains parameter docs, which should be on the parameter itself");
errorReporter
.error("method javadoc contains parameter docs, which should be on the parameter itself");
}
}
}
Expand Down
Original file line number Diff line number Diff line change
Expand Up @@ -19,28 +19,35 @@
import org.quiltmc.enigma.api.translation.representation.entry.ClassEntry;
import org.quiltmc.enigma.api.translation.representation.entry.Entry;
import org.quiltmc.enigma.api.translation.representation.entry.MethodEntry;
import quilt.internal.tasks.build.GeneratePackageInfoMappingsTask;

import static quilt.internal.tasks.lint.SpellingChecker.PluralSuffixes.IES;
import static quilt.internal.tasks.lint.SpellingChecker.PluralSuffixes.SES;
import static quilt.internal.tasks.lint.SpellingChecker.PluralSuffixes.VES;

public class SpellingChecker implements Checker<Entry<?>> {
// TODO make this an input and centralize the string
private static final String PACKAGE_INFO_CLASS_PACKAGE = "net/minecraft/unused/packageinfo/";
private static final String METHOD_PREFIX = "m_";
private static final String CLASS_PREFIX = "C_";
private static final char PACKAGE_SEPARATOR = '/';

private static final Set<String> MINECRAFT_WORDS = new HashSet<>();
static {
// collect minecraft words
InputStream parsedFile = SpellingChecker.class.getClassLoader().getResourceAsStream("minecraft_specific_words.txt");
final InputStream parsedFile =
SpellingChecker.class.getClassLoader().getResourceAsStream("minecraft_specific_words.txt");
if (parsedFile == null) {
throw new RuntimeException("could not find minecraft word file for spelling check in mapping lint task!");
}

Set<String> minecraftWords = getLines(parsedFile);
final Set<String> minecraftWords = getLines(parsedFile);
MINECRAFT_WORDS.addAll(minecraftWords);
}

private static Set<String> getLines(InputStream stream) {
return new BufferedReader(new InputStreamReader(stream)).lines().filter(line -> !line.isBlank() && !line.startsWith("//")).collect(Collectors.toSet());
return new BufferedReader(new InputStreamReader(stream))
.lines()
.filter(line -> !line.isBlank() && !line.startsWith("//"))
.collect(Collectors.toSet());
}

private static final Set<String> words = new HashSet<>();
Expand All @@ -50,20 +57,22 @@ public void update(MappingLintTask.LintParameters parameters) {
words.addAll(MINECRAFT_WORDS);

try {
FileInputStream spellingFile = new FileInputStream(parameters.getSpellingFile().get().getAsFile());
Set<String> spellingWords = getLines(spellingFile);
final FileInputStream spellingFile = new FileInputStream(parameters.getSpellingFile().get().getAsFile());
final Set<String> spellingWords = getLines(spellingFile);
words.addAll(spellingWords);
} catch (FileNotFoundException e) {
throw new RuntimeException("Unable to find spelling file!");
}
}

@Override
public void check(Entry<?> entry, EntryMapping mapping, Function<Entry<?>, AccessFlags> accessProvider, ErrorReporter errorReporter) {
String name = mapping.targetName();
public void check(
Entry<?> entry, EntryMapping mapping, Function<Entry<?>, AccessFlags> accessProvider, ErrorReporter errorReporter
) {
final String name = mapping.targetName();

if (name != null
&& !name.startsWith(PACKAGE_INFO_CLASS_PACKAGE)
&& !name.startsWith(GeneratePackageInfoMappingsTask.DEFAULT_PACKAGE_NAME)
// ignore unmapped methods and classes
&& !(entry instanceof MethodEntry && name.startsWith(METHOD_PREFIX))
&& !(entry instanceof ClassEntry && name.startsWith(CLASS_PREFIX))) {
Expand All @@ -75,10 +84,11 @@ public void check(Entry<?> entry, EntryMapping mapping, Function<Entry<?>, Acces

private static void checkJavadoc(String javadoc, ErrorReporter errorReporter) {
if (javadoc != null && !javadoc.isEmpty()) {
// in this case we just have to run the world's slowest regex, there's no clever optimisations based on type to be had
// in this case we just have to run the world's slowest regex,
// there's no clever optimisations based on type to be had
// note: this regex does not exclude dashes, we cannot handle every single "x-like" scattered around
List<String> splitJavadoc = splitWords(List.of(javadoc), word -> true, "[^A-Za-z']");
for (String word : splitJavadoc) {
final List<String> splitJavadoc = splitWords(List.of(javadoc), word -> true, "[^A-Za-z']");
for (final String word : splitJavadoc) {
if (!word.isBlank()) {
checkWord(word.toLowerCase(), word, errorReporter, MappingType.JAVADOC);
}
Expand All @@ -87,28 +97,29 @@ private static void checkJavadoc(String javadoc, ErrorReporter errorReporter) {
}

private static void checkMapping(Entry<?> entry, String name, ErrorReporter errorReporter) {
List<String> namesToSplit = new ArrayList<>();
final List<String> namesToSplit = new ArrayList<>();

// a contains check is necessary here because inner classes do not have package data
if (entry instanceof ClassEntry && name.contains(String.valueOf(PACKAGE_SEPARATOR))) {
// add class name - we don't have to handle underscores
namesToSplit.add(name.substring(name.lastIndexOf(PACKAGE_SEPARATOR) + 1));
// add package names and handle underscores
String[] packageNames = name.substring(0, name.lastIndexOf(PACKAGE_SEPARATOR)).split(String.valueOf(PACKAGE_SEPARATOR));
for (String packageName : packageNames) {
String[] split = packageName.split("_");
final String[] packageNames =
name.substring(0, name.lastIndexOf(PACKAGE_SEPARATOR)).split(String.valueOf(PACKAGE_SEPARATOR));
for (final String packageName : packageNames) {
final String[] split = packageName.split("_");
namesToSplit.addAll(List.of(split));
}
} else {
// handle underscores
String[] split = name.split("_");
final String[] split = name.split("_");
namesToSplit.addAll(List.of(split));
}

List<String> splitNames = splitWords(namesToSplit, word -> word.chars().anyMatch(Character::isDigit), "\\d");
final List<String> splitNames = splitWords(namesToSplit, word -> word.chars().anyMatch(Character::isDigit), "\\d");

// check
for (String word : splitNames) {
for (final String word : splitNames) {
checkWord(word, word, errorReporter, MappingType.ENTRY);
}
}
Expand All @@ -122,27 +133,30 @@ private static void checkMapping(Entry<?> entry, String name, ErrorReporter erro
*/
private static List<String> splitWords(List<String> words, Predicate<String> strippingCheck, String strippingRegex) {
// split by uppercase characters and numbers and preserve
List<String> splitNames = new ArrayList<>();
for (String nameToSplit : words) {
final List<String> splitNames = new ArrayList<>();
for (final String nameToSplit : words) {
// skip regex on all uppercase words
if (!nameToSplit.matches("[A-Z]+")) {
// split by uppercase letters and preserve them in the split strings
// also map to all lowercase
Set<String> split = Arrays.stream(nameToSplit.split("(?<=[a-zA-Z])(?=[A-Z])")).map(String::toLowerCase).collect(Collectors.toSet());
final Set<String> split = Arrays.stream(nameToSplit.split("(?<=[a-zA-Z])(?=[A-Z])"))
.map(String::toLowerCase)
.collect(Collectors.toSet());

splitNames.addAll(split);
} else {
splitNames.add(nameToSplit.toLowerCase());
}
}

List<String> splitWords = new ArrayList<>();
final List<String> splitWords = new ArrayList<>();

for (String word : splitNames) {
for (final String word : splitNames) {
// ignore numbers
// this requires special handling since we can't use the same regex we use for uppercase characters
// that way it would preserve the numbers, and we'd have to strip them anyway
if (strippingCheck.test(word)) {
String[] splitByInts = word.split(strippingRegex);
final String[] splitByInts = word.split(strippingRegex);
splitWords.addAll(Arrays.asList(splitByInts));
} else {
splitWords.add(word);
Expand Down Expand Up @@ -174,20 +188,21 @@ private static void checkWord(String word, String reportedWord, ErrorReporter re
}

private static boolean checkPlural(String word) {
String nonPlural;
if (word.endsWith("ies")) { // berries -> berry
nonPlural = word.substring(word.length() - 3) + "y";
} else if (word.endsWith("ves")) { // leaves -> leaf
String substring = word.substring(word.length() - 3);
nonPlural = substring + "f";
if (!words.contains(nonPlural)) { // knives -> knife
nonPlural = substring + "fe";
} else {
final String nonPlural;
if (word.endsWith(IES)) { // berries -> berry
nonPlural = word.substring(word.length() - IES.length()) + "y";
} else if (word.endsWith(VES)) { // leaves -> leaf
final String substring = word.substring(word.length() - VES.length());

if (words.contains(substring + "f")) {
// word has been found, we can skip the next contains check
return true;
} else { // knives -> knife
nonPlural = substring + "fe";
}
} else if (word.endsWith("ses")) { // glasses -> glass
nonPlural = word.substring(word.length() - 2);
} else if (word.endsWith(SES)) { // glasses -> glass
// leave one "s"
nonPlural = word.substring(word.length() - (SES.length() - 1));
} else { // ones -> one
nonPlural = word.substring(word.length() - 1);
}
Expand All @@ -214,4 +229,10 @@ public String toString() {
return this.getName();
}
}

interface PluralSuffixes {
String IES = "ies";
String VES = "ves";
String SES = "ses";
}
}
Loading

2 comments on commit 02ae830

@github-actions
Copy link

Choose a reason for hiding this comment

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

No difference between head and target.

@github-actions
Copy link

Choose a reason for hiding this comment

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

No difference between head and target.

Please sign in to comment.