From f2f6ced5a5e8854a8d747d1a5c469038dfedebf2 Mon Sep 17 00:00:00 2001 From: Mwexim <38397639+Mwexim@users.noreply.github.com> Date: Tue, 31 Oct 2023 14:12:40 +0100 Subject: [PATCH] PatternParser: add warning to specific edge-case and show registration errors (#141) Co-authored-by: Mwexim --- .../github/syst3ms/skriptparser/Parser.java | 4 +-- .../skriptparser/lang/VariableString.java | 7 +----- .../skriptparser/log/SkriptLogger.java | 18 +++++++------ .../skriptparser/pattern/PatternParser.java | 20 ++++++++++++++- .../registration/DefaultRegistration.java | 2 +- .../registration/PatternInfos.java | 8 ++++-- .../registration/SkriptRegistration.java | 25 ++++++++++++++++--- .../parsing/PatternParserTest.java | 17 ++++++++----- 8 files changed, 71 insertions(+), 30 deletions(-) diff --git a/src/main/java/io/github/syst3ms/skriptparser/Parser.java b/src/main/java/io/github/syst3ms/skriptparser/Parser.java index fb82bd2e..b3b3d9c0 100644 --- a/src/main/java/io/github/syst3ms/skriptparser/Parser.java +++ b/src/main/java/io/github/syst3ms/skriptparser/Parser.java @@ -130,9 +130,7 @@ public static void init(String[] mainPackages, String[] subPackages, String[] pr if (!logs.isEmpty()) { System.out.print(ConsoleColors.PURPLE); System.out.println("Registration log:"); - } - printLogs(logs, time, false); - if (!logs.isEmpty()) { + printLogs(logs, time, true); System.out.println(); } } diff --git a/src/main/java/io/github/syst3ms/skriptparser/lang/VariableString.java b/src/main/java/io/github/syst3ms/skriptparser/lang/VariableString.java index 74c57b71..917c40c6 100644 --- a/src/main/java/io/github/syst3ms/skriptparser/lang/VariableString.java +++ b/src/main/java/io/github/syst3ms/skriptparser/lang/VariableString.java @@ -5,8 +5,6 @@ import io.github.syst3ms.skriptparser.parsing.ParseContext; import io.github.syst3ms.skriptparser.parsing.ParserState; import io.github.syst3ms.skriptparser.parsing.SyntaxParser; -import io.github.syst3ms.skriptparser.registration.ExpressionInfo; -import io.github.syst3ms.skriptparser.registration.SyntaxManager; import io.github.syst3ms.skriptparser.registration.tags.Tag; import io.github.syst3ms.skriptparser.registration.tags.TagManager; import io.github.syst3ms.skriptparser.types.TypeManager; @@ -22,7 +20,6 @@ /** * A string that possibly contains expressions inside it, meaning that its value may be unknown at parse time */ -@SuppressWarnings("ConfusingArgumentToVarargsMethod") public class VariableString extends TaggedExpression { /** * An array containing raw data for this {@link VariableString}. @@ -268,9 +265,7 @@ public String defaultVariableName() { sb.append(((Tag) o).toString(false)); } else { assert o instanceof Expression; - Optional, ?>> exprInfo = SyntaxManager.getExpressionExact((Expression) o); - assert exprInfo.isPresent(); - sb.append("<").append(exprInfo.get().getReturnType().toString()).append(">"); + sb.append("<").append(((Expression) o).getReturnType().toString()).append(">"); } } return sb.toString(); diff --git a/src/main/java/io/github/syst3ms/skriptparser/log/SkriptLogger.java b/src/main/java/io/github/syst3ms/skriptparser/log/SkriptLogger.java index fba4cc6f..5e5308e0 100644 --- a/src/main/java/io/github/syst3ms/skriptparser/log/SkriptLogger.java +++ b/src/main/java/io/github/syst3ms/skriptparser/log/SkriptLogger.java @@ -188,7 +188,16 @@ public void error(String message, ErrorType errorType, @Nullable String tip) { * @param message the warning message */ public void warn(String message) { - log(message, LogType.WARNING, null, null); + warn(message, null); + } + + /** + * Logs a warning message + * @param message the warning message + * @param tip the tip for solving the warning + */ + public void warn(String message, @Nullable String tip) { + log(message, LogType.WARNING, null, tip); } /** @@ -283,11 +292,4 @@ public boolean isDebug() { public String getFileName() { return fileName; } - - /** - * @return whether or not this Logger has an error stored - */ - public boolean hasError() { - return hasError; - } } diff --git a/src/main/java/io/github/syst3ms/skriptparser/pattern/PatternParser.java b/src/main/java/io/github/syst3ms/skriptparser/pattern/PatternParser.java index 9ab4b082..545c6852 100644 --- a/src/main/java/io/github/syst3ms/skriptparser/pattern/PatternParser.java +++ b/src/main/java/io/github/syst3ms/skriptparser/pattern/PatternParser.java @@ -93,7 +93,25 @@ private static Optional parsePattern(String pattern, S if (matcher.matches() && vertParts.get().length == 1) { var mark = matcher.group(1); optionalGroup = parsePattern(matcher.group(3), logger) - .map(el -> new ChoiceElement(el, mark)) + .map(el -> { + if (el instanceof ChoiceGroup && mark.isEmpty()) + // Patterns like '[:(first|second)]' act differently than in the original Skript. + // Developers probably want to write ':[first|second]' instead, but the two actually + // have different behavior. + logger.warn( + "The parse mark in the '" + + s.get() + + "' part of the pattern will not be applied to all elements of the '" + + matcher.group(3) + + "' choice group, because it belongs to the whole optional group: [" + + s.get() + + "]", + "If this is unwanted behavior, use the syntax sugar ':[" + + matcher.group(3).substring(1, matcher.group(3).length() - 1) + + "]' to denote optional choice groups" + ); + return new ChoiceElement(el, mark); + }) .map(el -> new ChoiceGroup(Collections.singletonList(el))) .map(OptionalGroup::new); } else { diff --git a/src/main/java/io/github/syst3ms/skriptparser/registration/DefaultRegistration.java b/src/main/java/io/github/syst3ms/skriptparser/registration/DefaultRegistration.java index 3ba2ec2e..8ef77755 100644 --- a/src/main/java/io/github/syst3ms/skriptparser/registration/DefaultRegistration.java +++ b/src/main/java/io/github/syst3ms/skriptparser/registration/DefaultRegistration.java @@ -327,6 +327,6 @@ public Relation apply(Duration duration, Duration duration2) { registration.addConverter(SkriptDate.class, Time.class, da -> Optional.of(Time.of(da))); - registration.register(); // Ignoring logs here, we control the input + registration.register(true); // Ignoring logs here, we control the input } } \ No newline at end of file diff --git a/src/main/java/io/github/syst3ms/skriptparser/registration/PatternInfos.java b/src/main/java/io/github/syst3ms/skriptparser/registration/PatternInfos.java index 6fa08662..6644155c 100644 --- a/src/main/java/io/github/syst3ms/skriptparser/registration/PatternInfos.java +++ b/src/main/java/io/github/syst3ms/skriptparser/registration/PatternInfos.java @@ -51,8 +51,12 @@ public String[] getPatterns() { */ public String toChoiceGroup() { var builder = new StringBuilder("("); - for (int i = 0; i < patterns.length; i++) - builder.append(i).append(':').append(patterns[i]).append('|'); + for (int i = 0; i < patterns.length; i++) { + builder.append(i).append(':').append(patterns[i]); + if (i + 1 < patterns.length) { + builder.append("|"); + } + } return builder.append(')').toString(); } } diff --git a/src/main/java/io/github/syst3ms/skriptparser/registration/SkriptRegistration.java b/src/main/java/io/github/syst3ms/skriptparser/registration/SkriptRegistration.java index 7ee13aa1..bfe9be51 100644 --- a/src/main/java/io/github/syst3ms/skriptparser/registration/SkriptRegistration.java +++ b/src/main/java/io/github/syst3ms/skriptparser/registration/SkriptRegistration.java @@ -535,16 +535,31 @@ public void addTag(Class c, int priority) { /** * Adds all currently registered syntaxes to Skript's usable database. + * @return all possible errors, warnings and other logs that occurred while parsing the patterns */ public List register() { + return register(false); + } + + /** + * Adds all currently registered syntaxes to Skript's usable database. + * @param ignoreLogs whether to return the logs and close the logger, + * or just ignore and clear them while keeping the logger open + * @return all possible errors, warnings and other logs that occurred while parsing the patterns + */ + public List register(boolean ignoreLogs) { SyntaxManager.register(this); ContextValues.register(this); TypeManager.register(this); TagManager.register(this); Converters.registerConverters(this); Converters.createMissingConverters(); - logger.finalizeLogs(); - return logger.close(); + if (ignoreLogs) { + logger.clearLogs(); + return new ArrayList<>(); + } else { + return logger.close(); + } } public interface Registrar { @@ -662,7 +677,11 @@ protected List parsePatterns() { boolean computePriority = priority == -1; priority = computePriority ? 5 : priority; return patterns.stream() - .map(s -> PatternParser.parsePattern(s, logger).orElse(null)) + .map(s -> { + var result = PatternParser.parsePattern(s, logger).orElse(null); + logger.finalizeLogs(); + return result; + }) .filter(Objects::nonNull) .peek(e -> { if (computePriority) diff --git a/src/test/java/io/github/syst3ms/skriptparser/parsing/PatternParserTest.java b/src/test/java/io/github/syst3ms/skriptparser/parsing/PatternParserTest.java index a80c4773..6e72247c 100644 --- a/src/test/java/io/github/syst3ms/skriptparser/parsing/PatternParserTest.java +++ b/src/test/java/io/github/syst3ms/skriptparser/parsing/PatternParserTest.java @@ -134,15 +134,20 @@ public void testParsePattern() { // Optional with nested group with parse marks assertEqualsOptional( new OptionalGroup( - new CompoundElement( - new TextElement("optional "), - new ChoiceGroup( - new ChoiceElement(new TextElement("first choice"), null), - new ChoiceElement(new TextElement("second choice"), "second") + new ChoiceGroup( + new ChoiceElement( + new CompoundElement( + new TextElement("optional "), + new ChoiceGroup( + new ChoiceElement(new TextElement("first choice"), null), + new ChoiceElement(new TextElement("second choice"), "second") + ) + ), + "optional" ) ) ), - parsePattern("[optional (first choice|second:second choice)]", logger) + parsePattern("[:optional (first choice|second:second choice)]", logger) ); // Regex group