From cd7430fc108893daba0d44314bc7516605ae3f0b Mon Sep 17 00:00:00 2001 From: George Bateman Date: Sat, 24 Jun 2017 14:35:18 +0100 Subject: [PATCH] Handle curly quotes in error checker Also now prioritizes error messages on a single line for display to the user, since ECJ doesn't always get that right, reported mismatched argument lists when there's a syntax error and so on. --- app/src/processing/app/ui/Editor.java | 5 +- build/shared/lib/languages/PDE.properties | 1 + java/src/processing/mode/java/JavaEditor.java | 20 +++++ .../java/pdex/ErrorMessageSimplifier.java | 64 +++++++++++---- .../mode/java/pdex/JavaProblem.java | 77 +++++++++++++++++-- java/src/processing/mode/java/pdex/PDEX.java | 5 +- .../mode/java/pdex/SourceUtils.java | 4 +- 7 files changed, 153 insertions(+), 23 deletions(-) diff --git a/app/src/processing/app/ui/Editor.java b/app/src/processing/app/ui/Editor.java index 4cdc9817a..5a4969e5a 100644 --- a/app/src/processing/app/ui/Editor.java +++ b/app/src/processing/app/ui/Editor.java @@ -3106,9 +3106,10 @@ public abstract class Editor extends JFrame implements RunnerListener { /** - * @return the Problem for the first error or warning on 'line' + * @return the Problem for the most relevant error or warning on 'line', + * defaulting to the first. */ - Problem findProblem(int line) { + protected Problem findProblem(int line) { int currentTab = getSketch().getCurrentCodeIndex(); return problems.stream() .filter(p -> p.getTabIndex() == currentTab) diff --git a/build/shared/lib/languages/PDE.properties b/build/shared/lib/languages/PDE.properties index d68177d7d..7acab2a19 100644 --- a/build/shared/lib/languages/PDE.properties +++ b/build/shared/lib/languages/PDE.properties @@ -370,6 +370,7 @@ editor.status.undef_global_var = The global variable "%s" does not exist editor.status.undef_class = The class "%s" does not exist editor.status.undef_var = The variable "%s" does not exist editor.status.undef_name = The name "%s" cannot be recognized +editor.status.unterm_string_curly = String literal is not closed by a straight double quote. Curly quotes like %s won't help. editor.status.type_mismatch = Type mismatch, "%s" does not match with "%s" editor.status.unused_variable = The value of the local variable "%s" is not used editor.status.uninitialized_variable = The local variable "%s" may not have been initialized diff --git a/java/src/processing/mode/java/JavaEditor.java b/java/src/processing/mode/java/JavaEditor.java index 562efca8a..e9de556b3 100644 --- a/java/src/processing/mode/java/JavaEditor.java +++ b/java/src/processing/mode/java/JavaEditor.java @@ -2285,6 +2285,26 @@ public class JavaEditor extends Editor { } + /** + * @return the Problem for the most relevant error or warning on 'line', + * defaulting to the first. + */ + @Override + protected Problem findProblem(int line) { + List l = findProblems(line); + if (l.size() == 0) return null; + Problem worst = l.get(0); + + for (Problem p : l) { + if (p instanceof JavaProblem && ((!(worst instanceof JavaProblem)) || + ((JavaProblem)p).getPriority() > ((JavaProblem)worst).getPriority())) { + worst = p; + } + } + return worst; + } + + /** * Updates the error table in the Error Window. * Overridden to handle the fugly import suggestions text. diff --git a/java/src/processing/mode/java/pdex/ErrorMessageSimplifier.java b/java/src/processing/mode/java/pdex/ErrorMessageSimplifier.java index c77584e21..f165798d0 100644 --- a/java/src/processing/mode/java/pdex/ErrorMessageSimplifier.java +++ b/java/src/processing/mode/java/pdex/ErrorMessageSimplifier.java @@ -85,7 +85,7 @@ public class ErrorMessageSimplifier { /** * Tones down the jargon in the ecj reported errors. */ - public static String getSimplifiedErrorMessage(IProblem iprob) { + public static String getSimplifiedErrorMessage(IProblem iprob, String badCode) { if (iprob == null) return null; String args[] = iprob.getArguments(); @@ -97,6 +97,7 @@ public class ErrorMessageSimplifier { for (String arg : args) { Messages.log("Arg " + arg); } + Messages.log("Bad code: " + badCode); } String result = null; @@ -111,6 +112,15 @@ public class ErrorMessageSimplifier { case IProblem.ParsingErrorDeleteToken: if (args.length > 0) { + if (args[0].equalsIgnoreCase("Invalid Character")) { + result = getErrorMessageForCurlyQuote(badCode); + } + } + break; + + case IProblem.ParsingErrorDeleteTokens: + result = getErrorMessageForCurlyQuote(badCode); + if (result == null) { result = Language.interpolate("editor.status.error_on", args[0]); } break; @@ -136,13 +146,16 @@ public class ErrorMessageSimplifier { case IProblem.ParsingErrorInvalidToken: if (args.length > 0) { - if (args[1].equals("VariableDeclaratorId")) { - if (args[0].equals("int")) { + if (args[0].equals("int")) { + if (args[1].equals("VariableDeclaratorId")) { result = Language.text("editor.status.reserved_words"); } else { result = Language.interpolate("editor.status.error_on", args[0]); } - } else { + } else if (args[0].equalsIgnoreCase("Invalid Character")) { + result = getErrorMessageForCurlyQuote(badCode); + } + if (result == null) { result = Language.interpolate("editor.status.error_on", args[0]); } } @@ -165,6 +178,9 @@ public class ErrorMessageSimplifier { } break; + case IProblem.ParsingErrorReplaceTokens: + result = getErrorMessageForCurlyQuote(badCode); + case IProblem.UndefinedConstructor: if (args.length == 2) { String constructorName = args[0]; @@ -230,6 +246,13 @@ public class ErrorMessageSimplifier { } break; + case IProblem.UnterminatedString: + if (badCode.contains("“") || badCode.contains("”")) { + result = Language.interpolate("editor.status.unterm_string_curly", + badCode.replaceAll("[^“”]", "")); + } + break; + case IProblem.TypeMismatch: if (args.length > 1) { result = Language.interpolate("editor.status.type_mismatch", args[0], args[1]); @@ -261,16 +284,17 @@ public class ErrorMessageSimplifier { result = Language.interpolate("editor.status.hiding_enclosing_type", args[0]); } break; + } - default: - String message = iprob.getMessage(); - if (message != null) { - // Remove all instances of token - // "Syntax error on token 'blah', delete this token" - Matcher matcher = tokenRegExp.matcher(message); - message = matcher.replaceAll(""); - result = message; - } + if (result == null) { + String message = iprob.getMessage(); + if (message != null) { + // Remove all instances of token + // "Syntax error on token 'blah', delete this token" + Matcher matcher = tokenRegExp.matcher(message); + message = matcher.replaceAll(""); + result = message; + } } if (DEBUG) { @@ -323,6 +347,20 @@ public class ErrorMessageSimplifier { } + /** + * @param badCode The code which may contain curly quotes + * @return Friendly error message if there is a curly quote in badCode, + * null otherwise. + */ + static private String getErrorMessageForCurlyQuote(String badCode) { + if (badCode.contains("‘") || badCode.contains("’") || + badCode.contains("“") || badCode.contains("”")) { + return Language.interpolate("editor.status.bad_curly_quote", + badCode.replaceAll("[^‘’“”]", "")); + } else return null; + } + + // static private final String q(Object quotable) { // return "\"" + quotable + "\""; // } diff --git a/java/src/processing/mode/java/pdex/JavaProblem.java b/java/src/processing/mode/java/pdex/JavaProblem.java index bfe39b007..597648d31 100644 --- a/java/src/processing/mode/java/pdex/JavaProblem.java +++ b/java/src/processing/mode/java/pdex/JavaProblem.java @@ -20,7 +20,10 @@ along with this program; if not, write to the Free Software Foundation, Inc. package processing.mode.java.pdex; +import java.util.Arrays; + import org.eclipse.jdt.core.compiler.IProblem; +import static org.eclipse.jdt.core.compiler.IProblem.*; import processing.app.Problem; @@ -53,6 +56,52 @@ public class JavaProblem implements Problem { */ private int type; + /** + * Priority: bigger = higher. Currently 7 to 10 for errors, + * 4 for warning. + *

+ * The logic of the numbers in the priorityN arrays is that if ECJ wants a + * token got rid of entirely it's most likely the root of the problem. If it + * wants more tokens, that might have been caused by an unterminated string + * or something but it's also likely to be the “real” error. Only if the + * syntax is good are mismatched argument lists and so on of any real + * significance. These rankings are entirely made up so can be changed to + * support any other plausible scenario. + */ + private int priority; + + static private final int[] priority10 = { + ParsingError, + ParsingErrorDeleteToken, + ParsingErrorDeleteTokens, + ParsingErrorInvalidToken, + ParsingErrorMergeTokens, + ParsingErrorMisplacedConstruct, + ParsingErrorNoSuggestion, + ParsingErrorNoSuggestionForTokens, + ParsingErrorOnKeyword, + ParsingErrorOnKeywordNoSuggestion, + ParsingErrorReplaceTokens, + ParsingErrorUnexpectedEOF + }; + static private final int[] priority9 = { + InvalidCharacterConstant, + UnterminatedString + }; + static private final int[] priority8 = { + ParsingErrorInsertToComplete, + ParsingErrorInsertToCompletePhrase, + ParsingErrorInsertToCompleteScope, + ParsingErrorInsertTokenAfter, + ParsingErrorInsertTokenBefore, + }; + // Sorted so I can do a one-line binary search later. + static { + Arrays.sort(priority10); + Arrays.sort(priority9); + Arrays.sort(priority8); + } + /** * If the error is a 'cannot find type' contains the list of suggested imports */ @@ -60,11 +109,12 @@ public class JavaProblem implements Problem { public static final int ERROR = 1, WARNING = 2; - public JavaProblem(String message, int type, int tabIndex, int lineNumber) { + public JavaProblem(String message, int type, int tabIndex, int lineNumber, int priority) { this.message = message; this.type = type; this.tabIndex = tabIndex; this.lineNumber = lineNumber; + this.priority = priority; } /** @@ -72,16 +122,29 @@ public class JavaProblem implements Problem { * @param iProblem - The IProblem which is being wrapped * @param tabIndex - The tab number to which the error belongs to * @param lineNumber - Line number(pde code) of the error + * @param badCode - The code iProblem refers to. */ - public static JavaProblem fromIProblem(IProblem iProblem, int tabIndex, int lineNumber) { + public static JavaProblem fromIProblem(IProblem iProblem, + int tabIndex, int lineNumber, String badCode) { int type = 0; - if(iProblem.isError()) { + int priority = 0; + if (iProblem.isError()) { type = ERROR; + if (Arrays.binarySearch(priority10, iProblem.getID()) >= 0) { + priority = 10; + } else if (Arrays.binarySearch(priority9, iProblem.getID()) >= 0) { + priority = 9; + } else if (Arrays.binarySearch(priority8, iProblem.getID()) >= 0) { + priority = 8; + } else { + priority = 7; + } } else if (iProblem.isWarning()) { type = WARNING; + priority = 4; } - String message = ErrorMessageSimplifier.getSimplifiedErrorMessage(iProblem); - return new JavaProblem(message, type, tabIndex, lineNumber); + String message = ErrorMessageSimplifier.getSimplifiedErrorMessage(iProblem, badCode); + return new JavaProblem(message, type, tabIndex, lineNumber, priority); } public void setPDEOffsets(int startOffset, int stopOffset){ @@ -132,6 +195,10 @@ public class JavaProblem implements Problem { importSuggestions = a; } + public int getPriority() { + return priority; + } + @Override public String toString() { return "TAB " + tabIndex + ",LN " + lineNumber + "LN START OFF: " diff --git a/java/src/processing/mode/java/pdex/PDEX.java b/java/src/processing/mode/java/pdex/PDEX.java index 36add7bd1..80de6b574 100644 --- a/java/src/processing/mode/java/pdex/PDEX.java +++ b/java/src/processing/mode/java/pdex/PDEX.java @@ -1126,7 +1126,10 @@ public class PDEX { SketchInterval in = ps.mapJavaToSketch(start, stop); if (in == SketchInterval.BEFORE_START) return null; int line = ps.tabOffsetToTabLine(in.tabIndex, in.startTabOffset); - JavaProblem p = JavaProblem.fromIProblem(iproblem, in.tabIndex, line); + ps.sketch.updateSketchCodes(); // seems to be needed + String badCode = ps.sketch.getCode(in.tabIndex).getProgram() + .substring(in.startTabOffset, in.stopTabOffset); + JavaProblem p = JavaProblem.fromIProblem(iproblem, in.tabIndex, line, badCode); p.setPDEOffsets(in.startTabOffset, in.stopTabOffset); // Handle import suggestions diff --git a/java/src/processing/mode/java/pdex/SourceUtils.java b/java/src/processing/mode/java/pdex/SourceUtils.java index fecde5c94..9e575af88 100644 --- a/java/src/processing/mode/java/pdex/SourceUtils.java +++ b/java/src/processing/mode/java/pdex/SourceUtils.java @@ -355,7 +355,7 @@ public class SourceUtils { if (depth < 0) { JavaProblem problem = new JavaProblem("Found one too many } characters without { to match it.", - JavaProblem.ERROR, tabIndex, lineNumber); + JavaProblem.ERROR, tabIndex, lineNumber, 8); problem.setPDEOffsets(i - tabStartOffset, i - tabStartOffset + 1); problems.add(problem); continue tabLoop; @@ -364,7 +364,7 @@ public class SourceUtils { if (depth > 0) { JavaProblem problem = new JavaProblem("Found one too many { characters without } to match it.", - JavaProblem.ERROR, tabIndex, lineNumber - 1); + JavaProblem.ERROR, tabIndex, lineNumber - 1, 8); problem.setPDEOffsets(tabEndOffset - tabStartOffset - 2, tabEndOffset - tabStartOffset - 1); problems.add(problem); }