Skip to content

Commit

Permalink
Merge pull request #673 from Feuermagier/problem-type-desc
Browse files Browse the repository at this point in the history
Conditional message overrides
  • Loading branch information
Luro02 authored Jan 11, 2025
2 parents 8ed1456 + 592eddb commit 7d349cc
Show file tree
Hide file tree
Showing 13 changed files with 256 additions and 70 deletions.
Original file line number Diff line number Diff line change
@@ -1,13 +1,14 @@
package de.firemage.autograder.api;

import fluent.bundle.FluentBundle;
import fluent.bundle.FluentResource;

import java.io.IOException;
import java.nio.file.Path;
import java.util.ArrayList;
import java.util.HashMap;
import java.util.List;
import java.util.Locale;
import java.util.Map;
import java.util.function.Consumer;

public interface AbstractLinter {
Expand All @@ -25,6 +26,7 @@ class Builder {
private ClassLoader classLoader;
private int maxProblemsPerCheck = -1;
private List<FluentResource> messageOverrides = new ArrayList<>();
private Map<AbstractProblemType, List<String>> conditionalOverrides = new HashMap<>();

private Builder(Locale locale) {
this.locale = locale;
Expand Down Expand Up @@ -70,6 +72,11 @@ public Locale getLocale() {
return locale;
}

/**
* Add message overrides that always apply, regardless of problem type. Conditional overrides take precedence.
* @param bundle
* @return this
*/
public Builder messagesOverride(FluentResource bundle) {
this.messageOverrides.add(bundle);
return this;
Expand All @@ -78,5 +85,22 @@ public Builder messagesOverride(FluentResource bundle) {
public List<FluentResource> getMessageOverrides() {
return this.messageOverrides;
}

/**
* Add a message override that only applies if the message was emitted for the specified problem type.
* Conditional overrides override all other overrides. The (problemType, key) pair must be unique.
* @param problemType
* @param key
* @param value
* @return this
*/
public Builder conditionalOverride(AbstractProblemType problemType, String key, String value) {
this.conditionalOverrides.computeIfAbsent(problemType, k -> new ArrayList<>()).add(key + " = " + value);
return this;
}

public Map<AbstractProblemType, List<String>> getConditionalOverrides() {
return this.conditionalOverrides;
}
}
}
Original file line number Diff line number Diff line change
@@ -0,0 +1,8 @@
package de.firemage.autograder.api;

import fluent.bundle.FluentBundle;

public interface AbstractTranslations {
FluentBundle getMainTranslations();
FluentBundle getConditionalTranslations(AbstractProblemType problemType);
}
Original file line number Diff line number Diff line change
Expand Up @@ -2,7 +2,23 @@

import fluent.bundle.FluentBundle;

import java.util.Optional;

@FunctionalInterface
public interface Translatable {
String format(FluentBundle bundle);
/**
*
* @param bundle The bundle to format the message with
* @return an empty optional if the message key is unknown.
* Does not return an empty optional if the message key is known but the message cannot be formatted.
*/
Optional<String> tryFormat(FluentBundle bundle);

default String format(FluentBundle bundle) {
return tryFormat(bundle).orElseThrow(() -> new IllegalStateException("Unknown message"));
}

default String format(AbstractTranslations translations) {
return format(translations.getMainTranslations());
}
}
Original file line number Diff line number Diff line change
@@ -1,28 +1,20 @@
package de.firemage.autograder.core;

import de.firemage.autograder.api.AbstractLinter;
import de.firemage.autograder.api.AbstractProblemType;
import de.firemage.autograder.api.CheckConfiguration;
import de.firemage.autograder.api.AbstractLinter;
import de.firemage.autograder.api.JavaVersion;
import de.firemage.autograder.api.LinterException;
import de.firemage.autograder.api.Translatable;
import de.firemage.autograder.core.check.Check;
import de.firemage.autograder.core.check.ExecutableCheck;
import de.firemage.autograder.api.JavaVersion;
import de.firemage.autograder.core.file.TempLocation;
import de.firemage.autograder.core.file.UploadedFile;
import de.firemage.autograder.core.parallel.AnalysisResult;
import de.firemage.autograder.core.parallel.AnalysisScheduler;
import fluent.bundle.FluentBundle;
import fluent.bundle.FluentResource;
import fluent.functions.icu.ICUFunctionFactory;
import fluent.syntax.parser.FTLParser;
import fluent.syntax.parser.FTLStream;
import org.reflections.Reflections;
import org.reflections.scanners.Scanners;
import org.reflections.util.ConfigurationBuilder;

import java.io.IOException;
import java.nio.charset.StandardCharsets;
import java.nio.file.Path;
import java.util.ArrayList;
import java.util.Collection;
Expand All @@ -38,9 +30,9 @@
public final class Linter implements AbstractLinter {
private final int threads;
private final TempLocation tempLocation;
private final FluentBundle fluentBundle;
private final ClassLoader classLoader;
private final int maxProblemsPerCheck;
private final Translations translations;

public static Linter defaultLinter(Locale locale) {
return new Linter(AbstractLinter.builder(locale));
Expand All @@ -49,41 +41,15 @@ public static Linter defaultLinter(Locale locale) {
public Linter(
AbstractLinter.Builder builder
) {
var locale = builder.getLocale();
String filename = switch (locale.getLanguage()) {
case "de" -> "/strings.de.ftl";
case "en" -> "/strings.en.ftl";
default -> throw new IllegalArgumentException("No translation available for the locale " + locale);
};
try {
var fluentBuilder = FluentBundle.builder(locale, ICUFunctionFactory.INSTANCE);

// The name FluentBuilder#addResourceOverriding is a lie; it does not override preexisting messages

// message overrides
for (var bundle : builder.getMessageOverrides()) {
fluentBuilder.addResourceOverriding(bundle);
}

// default messages
FluentResource defaultMessages = FTLParser.parse(FTLStream.of(
new String(this.getClass().getResourceAsStream(filename).readAllBytes(), StandardCharsets.UTF_8)
));
fluentBuilder.addResourceOverriding(defaultMessages);

this.fluentBundle = fluentBuilder.build();
} catch (IOException e) {
throw new IllegalStateException(e);
}

this.translations = new Translations(builder.getLocale(), builder.getMessageOverrides(), builder.getConditionalOverrides());
this.tempLocation = builder.getTempLocation() != null ? (TempLocation) builder.getTempLocation() : TempLocation.random();
this.threads = builder.getThreads();
this.classLoader = builder.getClassLoader();
this.maxProblemsPerCheck = builder.getMaxProblemsPerCheck();
}

public FluentBundle getFluentBundle() {
return fluentBundle;
public Translations getTranslations() {
return translations;
}

@Override
Expand Down Expand Up @@ -232,14 +198,9 @@ private List<Problem> mergeProblems(Collection<? extends Problem> unreducedProbl
return result;
}

@Override
public String translateMessage(Translatable message) {
String output = message.format(this.fluentBundle);

if (output.startsWith("Unknown messageID '")) {
throw new IllegalStateException(output);
}

return output;
return message.format(this.translations);
}

private static final Collection<Class<?>> CHECKS = new LinkedHashSet<>(
Expand Down
Original file line number Diff line number Diff line change
Expand Up @@ -2,16 +2,35 @@

import de.firemage.autograder.api.Translatable;
import fluent.bundle.FluentBundle;
import org.slf4j.Logger;
import org.slf4j.LoggerFactory;

import java.util.ArrayList;
import java.util.List;
import java.util.Map;
import java.util.Optional;

public record LocalizedMessage(String key, Map<String, ?> parameters) implements Translatable {
private static final Logger logger = LoggerFactory.getLogger(LocalizedMessage.class);

public LocalizedMessage(String key) {
this(key, Map.of());
}

@Override
public String format(FluentBundle bundle) {
return bundle.format(this.key, this.parameters);
public Optional<String> tryFormat(FluentBundle bundle) {
var pattern = bundle.getMessagePattern(this.key);
if (pattern.isEmpty()) {
return Optional.empty();
}

List<Exception> errors = new ArrayList<>(1);
var output = bundle.formatPattern(pattern.get(), this.parameters, errors);
if (!errors.isEmpty()) {
// To stay consistent with Fluent's FluentBundle#format(String key) method, we do not throw an exception here
logger.error("Failed to format message '{}': {}", this.key, errors);
}

return Optional.of(output);
}
}
Original file line number Diff line number Diff line change
@@ -0,0 +1,33 @@
package de.firemage.autograder.core;

import de.firemage.autograder.api.AbstractTranslations;
import de.firemage.autograder.api.Translatable;
import fluent.bundle.FluentBundle;

import java.util.Optional;

public record LocalizedMessageForProblem(Translatable translatable, ProblemType problemType) implements Translatable {
public LocalizedMessageForProblem {
if (translatable instanceof LocalizedMessageForProblem) {
throw new IllegalArgumentException("LocalizedMessageForProblem cannot be nested");
}
}

@Override
public String format(AbstractTranslations translations) {
var conditionalBundle = translations.getConditionalTranslations(problemType);
if (conditionalBundle != null) {
var result = translatable.tryFormat(conditionalBundle);
if (result.isPresent()) {
return result.get();
}
}

return translatable.format(translations.getMainTranslations());
}

@Override
public Optional<String> tryFormat(FluentBundle bundle) {
return translatable.tryFormat(bundle);
}
}
Original file line number Diff line number Diff line change
Expand Up @@ -42,7 +42,7 @@ private static Translatable makeExplanation(Problem first, Collection<? extends
problems.stream().map(Problem::getPosition)
)
)
).format(bundle);
).tryFormat(bundle);
};
}

Expand Down
Original file line number Diff line number Diff line change
@@ -0,0 +1,72 @@
package de.firemage.autograder.core;

import de.firemage.autograder.api.AbstractProblemType;
import de.firemage.autograder.api.AbstractTranslations;
import fluent.bundle.FluentBundle;
import fluent.bundle.FluentResource;
import fluent.functions.icu.ICUFunctionFactory;
import fluent.syntax.parser.FTLParser;
import fluent.syntax.parser.FTLStream;

import java.io.IOException;
import java.nio.charset.StandardCharsets;
import java.util.HashMap;
import java.util.List;
import java.util.Locale;
import java.util.Map;

public class Translations implements AbstractTranslations {
private final FluentBundle mainTranslations;
private final Map<AbstractProblemType, FluentBundle> conditionalTranslations;

public Translations(Locale locale, List<FluentResource> mainOverrides, Map<AbstractProblemType, List<String>> conditionalOverrides) {
String filename = switch (locale.getLanguage()) {
case "de" -> "/strings.de.ftl";
case "en" -> "/strings.en.ftl";
default -> throw new IllegalArgumentException("No translation available for the locale " + locale);
};

try {
// == Normal messages
var fluentBuilder = FluentBundle.builder(locale, ICUFunctionFactory.INSTANCE);

// The name FluentBuilder#addResourceOverriding is a lie; it does not override preexisting messages

// message overrides
for (var bundle : mainOverrides) {
fluentBuilder.addResourceOverriding(bundle);
}

// default messages
try (var stream = this.getClass().getResourceAsStream(filename)) {
if (stream == null) {
throw new IllegalStateException("Could not find the autograder messages");
}
fluentBuilder.addResourceOverriding(FTLParser.parse(FTLStream.of(new String(stream.readAllBytes(), StandardCharsets.UTF_8))));
}

this.mainTranslations = fluentBuilder.build();

// == Conditional messages
this.conditionalTranslations = new HashMap<>();
for (var entry : conditionalOverrides.entrySet()) {
var conditionalBuilder = FluentBundle.builder(locale, ICUFunctionFactory.INSTANCE);
var content = String.join("\n", entry.getValue());
conditionalBuilder.addResource(FTLParser.parse(FTLStream.of(content)));
this.conditionalTranslations.put(entry.getKey(), conditionalBuilder.build());
}
} catch (IOException e) {
throw new IllegalStateException(e);
}
}

@Override
public FluentBundle getMainTranslations() {
return this.mainTranslations;
}

@Override
public FluentBundle getConditionalTranslations(AbstractProblemType problemType) {
return this.conditionalTranslations.get(problemType);
}
}
Original file line number Diff line number Diff line change
@@ -1,6 +1,7 @@
package de.firemage.autograder.core.integrated;

import de.firemage.autograder.core.CodePosition;
import de.firemage.autograder.core.LocalizedMessageForProblem;
import de.firemage.autograder.core.Problem;
import de.firemage.autograder.core.LocalizedMessage;
import de.firemage.autograder.core.ProblemType;
Expand All @@ -19,11 +20,11 @@ public abstract class IntegratedCheck implements Check {
protected IntegratedCheck() {}

protected void addLocalProblem(CtElement element, Translatable explanation, ProblemType problemType) {
this.problems.add(new IntegratedInCodeProblem(this, element, explanation, problemType, this.sourceInfo));
this.problems.add(new IntegratedInCodeProblem(this, element, new LocalizedMessageForProblem(explanation, problemType), problemType, this.sourceInfo));
}

protected void addLocalProblem(CodePosition position, Translatable explanation, ProblemType problemType) {
this.problems.add(new Problem(this, position, explanation, problemType) {});
this.problems.add(new Problem(this, position, new LocalizedMessageForProblem(explanation, problemType), problemType) {});
}

public List<Problem> run(StaticAnalysis staticAnalysis, SourceInfo sourceInfo) {
Expand Down
Loading

0 comments on commit 7d349cc

Please sign in to comment.