From 508132dff16d8ffed1da813f98b184924dd127a0 Mon Sep 17 00:00:00 2001 From: Jakub Valtar Date: Thu, 15 Dec 2016 23:10:29 +0100 Subject: [PATCH 1/6] Clean up ErrorMessageSimplifier Remove weird unused thread-spawning constructor Accept IProblem instead of whole JavaProblem Move fallback process() from JavaProblem to EMSimplifier as a default case Add debug switch and move debug logging into if blocks --- .../java/pdex/ErrorMessageSimplifier.java | 62 ++++++++++++------- .../mode/java/pdex/JavaProblem.java | 31 +--------- 2 files changed, 40 insertions(+), 53 deletions(-) diff --git a/java/src/processing/mode/java/pdex/ErrorMessageSimplifier.java b/java/src/processing/mode/java/pdex/ErrorMessageSimplifier.java index df7909e8e..efb8d2e70 100644 --- a/java/src/processing/mode/java/pdex/ErrorMessageSimplifier.java +++ b/java/src/processing/mode/java/pdex/ErrorMessageSimplifier.java @@ -23,11 +23,14 @@ package processing.mode.java.pdex; import java.lang.reflect.Field; import java.lang.reflect.Modifier; import java.util.TreeMap; +import java.util.regex.Matcher; +import java.util.regex.Pattern; import org.eclipse.jdt.core.compiler.IProblem; import org.eclipse.jdt.internal.compiler.problem.DefaultProblem; import processing.app.Language; +import processing.app.Messages; import processing.core.PApplet; import processing.data.StringList; @@ -42,25 +45,20 @@ public class ErrorMessageSimplifier { */ private static TreeMap constantsMap; + private static final boolean DEBUG = false; - public ErrorMessageSimplifier() { - - new Thread() { - public void run() { - prepareConstantsList(); - } - }.start(); - } - + private static final Pattern tokenRegExp = Pattern.compile("\\b token\\b"); private static void prepareConstantsList() { - constantsMap = new TreeMap(); + constantsMap = new TreeMap<>(); Class probClass = DefaultProblem.class; Field f[] = probClass.getFields(); for (Field field : f) { if (Modifier.isStatic(field.getModifiers())) try { - //System.out.println(field.getName() + " :" + field.get(null)); + if (DEBUG) { + Messages.log(field.getName() + " :" + field.get(null)); + } Object val = field.get(null); if (val instanceof Integer) { constantsMap.put((Integer) (val), field.getName()); @@ -70,7 +68,9 @@ public class ErrorMessageSimplifier { break; } } - //System.out.println("Total items: " + constantsMap.size()); + if (DEBUG) { + Messages.log("Total items: " + constantsMap.size()); + } } @@ -85,17 +85,19 @@ public class ErrorMessageSimplifier { /** * Tones down the jargon in the ecj reported errors. */ - public static String getSimplifiedErrorMessage(JavaProblem problem) { - if (problem == null) return null; + public static String getSimplifiedErrorMessage(IProblem iprob) { + if (iprob == null) return null; - IProblem iprob = problem.getIProblem(); String args[] = iprob.getArguments(); -// Base.log("Simplifying message: " + problem.getMessage() + " ID: " -// + getIDName(iprob.getID())); -// Base.log("Arg count: " + args.length); -// for (int i = 0; i < args.length; i++) { -// Base.log("Arg " + args[i]); -// } + + if (DEBUG) { + Messages.log("Simplifying message: " + iprob.getMessage() + + " ID: " + getIDName(iprob.getID())); + Messages.log("Arg count: " + args.length); + for (String arg : args) { + Messages.log("Arg " + arg); + } + } String result = null; @@ -259,10 +261,24 @@ public class ErrorMessageSimplifier { if (args.length > 0) { 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; + } } - //log("Simplified Error Msg: " + result); - return (result == null) ? problem.getMessage() : result; + if (DEBUG) { + Messages.log("Simplified Error Msg: " + result); + } + + return result; } diff --git a/java/src/processing/mode/java/pdex/JavaProblem.java b/java/src/processing/mode/java/pdex/JavaProblem.java index bb4da8898..1aeb38c85 100644 --- a/java/src/processing/mode/java/pdex/JavaProblem.java +++ b/java/src/processing/mode/java/pdex/JavaProblem.java @@ -20,9 +20,6 @@ along with this program; if not, write to the Free Software Foundation, Inc. package processing.mode.java.pdex; -import java.util.regex.Matcher; -import java.util.regex.Pattern; - import org.eclipse.jdt.core.compiler.IProblem; import processing.app.Problem; @@ -83,9 +80,7 @@ public class JavaProblem implements Problem { } this.tabIndex = tabIndex; this.lineNumber = lineNumber; - this.message = process(iProblem); - this.message = ErrorMessageSimplifier.getSimplifiedErrorMessage(this); - //ErrorMessageSimplifier.getSimplifiedErrorMessage(this); + this.message = ErrorMessageSimplifier.getSimplifiedErrorMessage(iProblem); } public void setPDEOffsets(int startOffset, int stopOffset){ @@ -157,30 +152,6 @@ public class JavaProblem implements Problem { importSuggestions = a; } - private static final Pattern tokenRegExp = Pattern.compile("\\b token\\b"); - - public static String process(IProblem problem) { - return process(problem.getMessage()); - } - - /** - * Processes error messages and attempts to make them a bit more english like. - * Currently performs: - *
  • Remove all instances of token. "Syntax error on token 'blah', delete this token" - * becomes "Syntax error on 'blah', delete this" - * @param message - The message to be processed - * @return String - The processed message - */ - public static String process(String message) { - // Remove all instances of token - // "Syntax error on token 'blah', delete this token" - if(message == null) return null; - Matcher matcher = tokenRegExp.matcher(message); - message = matcher.replaceAll(""); - - return message; - } - // Split camel case words into separate words. // "VaraibleDeclaration" becomes "Variable Declaration" // But sadly "PApplet" become "P Applet" and so on. From d3a6e7ec5b7f665590c78aaaeb96e309232fdbbf Mon Sep 17 00:00:00 2001 From: Jakub Valtar Date: Thu, 15 Dec 2016 23:11:40 +0100 Subject: [PATCH 2/6] Fix ErrorMessageSimplifier indents (no functional changes) --- .../java/pdex/ErrorMessageSimplifier.java | 263 +++++++++--------- 1 file changed, 131 insertions(+), 132 deletions(-) diff --git a/java/src/processing/mode/java/pdex/ErrorMessageSimplifier.java b/java/src/processing/mode/java/pdex/ErrorMessageSimplifier.java index efb8d2e70..c77584e21 100644 --- a/java/src/processing/mode/java/pdex/ErrorMessageSimplifier.java +++ b/java/src/processing/mode/java/pdex/ErrorMessageSimplifier.java @@ -75,7 +75,7 @@ public class ErrorMessageSimplifier { public static String getIDName(int id) { - if (constantsMap == null){ + if (constantsMap == null) { prepareConstantsList(); } return constantsMap.get(id); @@ -103,175 +103,174 @@ public class ErrorMessageSimplifier { switch (iprob.getID()) { - case IProblem.ParsingError: - if (args.length > 0) { - result = Language.interpolate("editor.status.error_on", args[0]); - } - break; + case IProblem.ParsingError: + if (args.length > 0) { + result = Language.interpolate("editor.status.error_on", args[0]); + } + break; - case IProblem.ParsingErrorDeleteToken: - if (args.length > 0) { - result = Language.interpolate("editor.status.error_on", args[0]); - } - break; + case IProblem.ParsingErrorDeleteToken: + if (args.length > 0) { + result = Language.interpolate("editor.status.error_on", args[0]); + } + break; - case IProblem.ParsingErrorInsertToComplete: - if (args.length > 0) { - if (args[0].length() == 1) { - result = getErrorMessageForBracket(args[0].charAt(0)); - - } else { - if (args[0].equals("AssignmentOperator Expression")) { - result = Language.interpolate("editor.status.missing.add", "="); - - } else if (args[0].equalsIgnoreCase(") Statement")) { + case IProblem.ParsingErrorInsertToComplete: + if (args.length > 0) { + if (args[0].length() == 1) { result = getErrorMessageForBracket(args[0].charAt(0)); } else { - result = Language.interpolate("editor.status.error_on", args[0]); + if (args[0].equals("AssignmentOperator Expression")) { + result = Language.interpolate("editor.status.missing.add", "="); + + } else if (args[0].equalsIgnoreCase(") Statement")) { + result = getErrorMessageForBracket(args[0].charAt(0)); + + } else { + result = Language.interpolate("editor.status.error_on", args[0]); + } } } - } - break; + break; - case IProblem.ParsingErrorInvalidToken: - if (args.length > 0) { - if (args[1].equals("VariableDeclaratorId")) { - if (args[0].equals("int")) { - result = Language.text ("editor.status.reserved_words"); + case IProblem.ParsingErrorInvalidToken: + if (args.length > 0) { + if (args[1].equals("VariableDeclaratorId")) { + if (args[0].equals("int")) { + result = Language.text("editor.status.reserved_words"); + } else { + result = Language.interpolate("editor.status.error_on", args[0]); + } } else { result = Language.interpolate("editor.status.error_on", args[0]); } - } else { - result = Language.interpolate("editor.status.error_on", args[0]); } - } - break; + break; - case IProblem.ParsingErrorInsertTokenAfter: - if (args.length > 0) { - if (args[1].length() == 1) { - result = getErrorMessageForBracket(args[1].charAt(0)); - } - else { - // https://github.com/processing/processing/issues/3104 - if (args[1].equalsIgnoreCase("Statement")) { - result = Language.interpolate("editor.status.error_on", args[0]); + case IProblem.ParsingErrorInsertTokenAfter: + if (args.length > 0) { + if (args[1].length() == 1) { + result = getErrorMessageForBracket(args[1].charAt(0)); } else { - result = - Language.interpolate("editor.status.error_on", args[0]) + " " + - Language.interpolate("editor.status.missing.add", args[1]); + // https://github.com/processing/processing/issues/3104 + if (args[1].equalsIgnoreCase("Statement")) { + result = Language.interpolate("editor.status.error_on", args[0]); + } else { + result = + Language.interpolate("editor.status.error_on", args[0]) + " " + + Language.interpolate("editor.status.missing.add", args[1]); + } } } - } - break; + break; - case IProblem.UndefinedConstructor: - if (args.length == 2) { - String constructorName = args[0]; - // For messages such as "contructor sketch_name.ClassXYZ() is undefined", change - // constructor name to "ClassXYZ()". See #3434 - if (constructorName.contains(".")) { - // arg[0] contains sketch name twice: sketch_150705a.sketch_150705a.Thing - constructorName = constructorName.substring(constructorName.indexOf('.') + 1); - constructorName = constructorName.substring(constructorName.indexOf('.') + 1); + case IProblem.UndefinedConstructor: + if (args.length == 2) { + String constructorName = args[0]; + // For messages such as "contructor sketch_name.ClassXYZ() is undefined", change + // constructor name to "ClassXYZ()". See #3434 + if (constructorName.contains(".")) { + // arg[0] contains sketch name twice: sketch_150705a.sketch_150705a.Thing + constructorName = constructorName.substring(constructorName.indexOf('.') + 1); + constructorName = constructorName.substring(constructorName.indexOf('.') + 1); + } + String constructorArgs = removePackagePrefixes(args[args.length - 1]); + result = Language.interpolate("editor.status.undefined_constructor", constructorName, constructorArgs); } - String constructorArgs = removePackagePrefixes(args[args.length - 1]); - result = Language.interpolate("editor.status.undefined_constructor", constructorName, constructorArgs); - } - break; + break; - case IProblem.UndefinedMethod: - if (args.length > 2) { - String methodName = args[args.length - 2]; - String methodArgs = removePackagePrefixes(args[args.length - 1]); - result = Language.interpolate("editor.status.undefined_method", methodName, methodArgs); - } - break; + case IProblem.UndefinedMethod: + if (args.length > 2) { + String methodName = args[args.length - 2]; + String methodArgs = removePackagePrefixes(args[args.length - 1]); + result = Language.interpolate("editor.status.undefined_method", methodName, methodArgs); + } + break; - case IProblem.ParameterMismatch: - if (args.length > 3) { - // 2nd arg is method name, 3rd arg is correct param list - if (args[2].trim().length() == 0) { - // the case where no params are needed. - result = Language.interpolate("editor.status.empty_param", args[1]); + case IProblem.ParameterMismatch: + if (args.length > 3) { + // 2nd arg is method name, 3rd arg is correct param list + if (args[2].trim().length() == 0) { + // the case where no params are needed. + result = Language.interpolate("editor.status.empty_param", args[1]); - } else { - result = Language.interpolate("editor.status.wrong_param", - args[1], args[1], removePackagePrefixes(args[2])); + } else { + result = Language.interpolate("editor.status.wrong_param", + args[1], args[1], removePackagePrefixes(args[2])); // String method = q(args[1]); // String methodDef = " \"" + args[1] + "(" + getSimpleName(args[2]) + ")\""; // result = result.replace("method", method); // result += methodDef; + } } - } - break; + break; - case IProblem.UndefinedField: - if (args.length > 0) { - result = Language.interpolate("editor.status.undef_global_var", args[0]); - } - break; + case IProblem.UndefinedField: + if (args.length > 0) { + result = Language.interpolate("editor.status.undef_global_var", args[0]); + } + break; - case IProblem.UndefinedType: - if (args.length > 0) { - result = Language.interpolate("editor.status.undef_class", args[0]); - } - break; + case IProblem.UndefinedType: + if (args.length > 0) { + result = Language.interpolate("editor.status.undef_class", args[0]); + } + break; - case IProblem.UnresolvedVariable: - if (args.length > 0) { - result = Language.interpolate("editor.status.undef_var", args[0]); - } - break; + case IProblem.UnresolvedVariable: + if (args.length > 0) { + result = Language.interpolate("editor.status.undef_var", args[0]); + } + break; - case IProblem.UndefinedName: - if (args.length > 0) { - result = Language.interpolate("editor.status.undef_name", args[0]); - } - break; + case IProblem.UndefinedName: + if (args.length > 0) { + result = Language.interpolate("editor.status.undef_name", args[0]); + } + break; - case IProblem.TypeMismatch: - if (args.length > 1) { - result = Language.interpolate("editor.status.type_mismatch", args[0], args[1]); + case IProblem.TypeMismatch: + if (args.length > 1) { + result = Language.interpolate("editor.status.type_mismatch", args[0], args[1]); // result = result.replace("typeA", q(args[0])); // result = result.replace("typeB", q(args[1])); - } - break; + } + break; - case IProblem.LocalVariableIsNeverUsed: - if (args.length > 0) { - result = Language.interpolate("editor.status.unused_variable", args[0]); - } - break; + case IProblem.LocalVariableIsNeverUsed: + if (args.length > 0) { + result = Language.interpolate("editor.status.unused_variable", args[0]); + } + break; - case IProblem.UninitializedLocalVariable: - if (args.length > 0) { - result = Language.interpolate("editor.status.uninitialized_variable", args[0]); - } - break; + case IProblem.UninitializedLocalVariable: + if (args.length > 0) { + result = Language.interpolate("editor.status.uninitialized_variable", args[0]); + } + break; - case IProblem.AssignmentHasNoEffect: - if (args.length > 0) { - result = Language.interpolate("editor.status.no_effect_assignment", args[0]); - } - break; + case IProblem.AssignmentHasNoEffect: + if (args.length > 0) { + result = Language.interpolate("editor.status.no_effect_assignment", args[0]); + } + break; - case IProblem.HidingEnclosingType: - if (args.length > 0) { - result = Language.interpolate("editor.status.hiding_enclosing_type", args[0]); - } - break; + case IProblem.HidingEnclosingType: + if (args.length > 0) { + 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; - } + 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 (DEBUG) { From 5f464e9558c516df4535f7d20fd398993ce53208 Mon Sep 17 00:00:00 2001 From: Jakub Valtar Date: Thu, 15 Dec 2016 22:47:09 +0100 Subject: [PATCH 3/6] PDEX: simplify import suggestion handling --- java/src/processing/mode/java/pdex/PDEX.java | 51 +++++++++----------- 1 file changed, 23 insertions(+), 28 deletions(-) diff --git a/java/src/processing/mode/java/pdex/PDEX.java b/java/src/processing/mode/java/pdex/PDEX.java index 6182faed3..37b0d9b8e 100644 --- a/java/src/processing/mode/java/pdex/PDEX.java +++ b/java/src/processing/mode/java/pdex/PDEX.java @@ -36,8 +36,10 @@ import java.util.Arrays; import java.util.Collections; import java.util.Comparator; import java.util.Deque; +import java.util.HashMap; import java.util.List; import java.util.Map; +import java.util.Objects; import java.util.concurrent.Executors; import java.util.concurrent.ScheduledExecutorService; import java.util.concurrent.ScheduledFuture; @@ -1074,6 +1076,9 @@ public class PDEX { private void handleSketchProblems(PreprocessedSketch ps) { + Map suggCache = + JavaMode.importSuggestEnabled ? new HashMap<>() : Collections.emptyMap(); + // Process problems IProblem[] iproblems = ps.compilationUnit.getProblems(); final List problems = Arrays.stream(iproblems) @@ -1095,38 +1100,20 @@ public class PDEX { int line = ps.tabOffsetToTabLine(in.tabIndex, in.startTabOffset); JavaProblem p = new JavaProblem(iproblem, in.tabIndex, line); p.setPDEOffsets(in.startTabOffset, in.stopTabOffset); + + // Handle import suggestions + if (JavaMode.importSuggestEnabled && isUndefinedTypeProblem(iproblem)) { + ClassPath cp = ps.searchClassPath; + String[] s = suggCache.computeIfAbsent(iproblem.getArguments()[0], + name -> getImportSuggestions(cp, name)); + p.setImportSuggestions(s); + } + return p; }) - .filter(p -> p != null) + .filter(Objects::nonNull) .collect(Collectors.toList()); - // Handle import suggestions - if (JavaMode.importSuggestEnabled) { - Map> undefinedTypeProblems = problems.stream() - // Get only problems with undefined types/names - .filter(p -> { - int id = ((JavaProblem) p).getIProblem().getID(); - return id == IProblem.UndefinedType || - id == IProblem.UndefinedName || - id == IProblem.UnresolvedVariable; - }) - // Group problems by the missing type/name - .collect(Collectors.groupingBy(p -> ((JavaProblem) p).getIProblem().getArguments()[0])); - - if (!undefinedTypeProblems.isEmpty()) { - final ClassPath cp = ps.searchClassPath; - - // Get suggestions for each missing type, update the problems - undefinedTypeProblems.entrySet().stream() - .forEach(entry -> { - String missingClass = entry.getKey(); - List affectedProblems = entry.getValue(); - String[] suggestions = getImportSuggestions(cp, missingClass); - affectedProblems.forEach(p -> ((JavaProblem) p).setImportSuggestions(suggestions)); - }); - } - } - if (scheduledUiUpdate != null) { scheduledUiUpdate.cancel(true); } @@ -1142,6 +1129,14 @@ public class PDEX { } + private boolean isUndefinedTypeProblem(IProblem iproblem) { + int id = iproblem.getID(); + return id == IProblem.UndefinedType || + id == IProblem.UndefinedName || + id == IProblem.UnresolvedVariable; + } + + public static String[] getImportSuggestions(ClassPath cp, String className) { RegExpResourceFilter regf = new RegExpResourceFilter( Pattern.compile(".*"), From dd31bf2fbea2946996e9ae36d01d69e870919297 Mon Sep 17 00:00:00 2001 From: Jakub Valtar Date: Fri, 16 Dec 2016 00:15:10 +0100 Subject: [PATCH 4/6] Clean up JavaProblem, remove IProblem field --- .../mode/java/pdex/JavaProblem.java | 65 ++++--------------- 1 file changed, 13 insertions(+), 52 deletions(-) diff --git a/java/src/processing/mode/java/pdex/JavaProblem.java b/java/src/processing/mode/java/pdex/JavaProblem.java index 1aeb38c85..bb6a7fb58 100644 --- a/java/src/processing/mode/java/pdex/JavaProblem.java +++ b/java/src/processing/mode/java/pdex/JavaProblem.java @@ -30,10 +30,6 @@ import processing.app.Problem; * according to its tab, including the original IProblem object */ public class JavaProblem implements Problem { - /** - * The IProblem which is being wrapped - */ - private IProblem iProblem; /** * The tab number to which the error belongs to */ @@ -71,11 +67,9 @@ public class JavaProblem implements Problem { * @param lineNumber - Line number(pde code) of the error */ public JavaProblem(IProblem iProblem, int tabIndex, int lineNumber) { - this.iProblem = iProblem; if(iProblem.isError()) { type = ERROR; - } - else if(iProblem.isWarning()) { + } else if (iProblem.isWarning()) { type = WARNING; } this.tabIndex = tabIndex; @@ -88,62 +82,41 @@ public class JavaProblem implements Problem { this.stopOffset = stopOffset; } + @Override public int getStartOffset() { return startOffset; } + @Override public int getStopOffset() { return stopOffset; } - public String toString() { - return new String("TAB " + tabIndex + ",LN " + lineNumber + "LN START OFF: " - + startOffset + ",LN STOP OFF: " + stopOffset + ",PROB: " - + message); - } - + @Override public boolean isError() { return type == ERROR; } + @Override public boolean isWarning() { return type == WARNING; } + @Override public String getMessage() { return message; } - public IProblem getIProblem() { - return iProblem; - } - + @Override public int getTabIndex() { return tabIndex; } + @Override public int getLineNumber() { return lineNumber; } - /** - * Remember to subtract a -1 to line number because in compile check code an - * extra package statement is added, so all line numbers are increased by 1 - * - * @return - */ - public int getSourceLineNumber() { - return iProblem.getSourceLineNumber(); - } - - public void setType(int ProblemType){ - if(ProblemType == ERROR) - type = ERROR; - else if(ProblemType == WARNING) - type = WARNING; - else throw new IllegalArgumentException("Illegal Problem type passed to Problem.setType(int)"); - } - public String[] getImportSuggestions() { return importSuggestions; } @@ -152,23 +125,11 @@ public class JavaProblem implements Problem { importSuggestions = a; } - // Split camel case words into separate words. - // "VaraibleDeclaration" becomes "Variable Declaration" - // But sadly "PApplet" become "P Applet" and so on. - public static String splitCamelCaseWord(String word) { - String newWord = ""; - for (int i = 1; i < word.length(); i++) { - if (Character.isUpperCase(word.charAt(i))) { - // System.out.println(word.substring(0, i) + " " - // + word.substring(i)); - newWord += word.substring(0, i) + " "; - word = word.substring(i); - i = 1; - } - } - newWord += word; - // System.out.println(newWord); - return newWord.trim(); + @Override + public String toString() { + return "TAB " + tabIndex + ",LN " + lineNumber + "LN START OFF: " + + startOffset + ",LN STOP OFF: " + stopOffset + ",PROB: " + + message; } } From b72a39564bdf0d7de94e527b6e6c783564317cb1 Mon Sep 17 00:00:00 2001 From: Jakub Valtar Date: Fri, 16 Dec 2016 04:36:23 +0100 Subject: [PATCH 5/6] Detect missing braces in tabs, ignore other problems until fixed --- .../mode/java/pdex/JavaProblem.java | 15 ++- java/src/processing/mode/java/pdex/PDEX.java | 108 ++++++++++++------ .../mode/java/pdex/PreprocessedSketch.java | 7 ++ .../mode/java/pdex/PreprocessingService.java | 11 +- .../mode/java/pdex/SourceUtils.java | 41 +++++++ 5 files changed, 145 insertions(+), 37 deletions(-) diff --git a/java/src/processing/mode/java/pdex/JavaProblem.java b/java/src/processing/mode/java/pdex/JavaProblem.java index bb6a7fb58..bfe39b007 100644 --- a/java/src/processing/mode/java/pdex/JavaProblem.java +++ b/java/src/processing/mode/java/pdex/JavaProblem.java @@ -60,21 +60,28 @@ public class JavaProblem implements Problem { public static final int ERROR = 1, WARNING = 2; + public JavaProblem(String message, int type, int tabIndex, int lineNumber) { + this.message = message; + this.type = type; + this.tabIndex = tabIndex; + this.lineNumber = lineNumber; + } + /** * * @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 */ - public JavaProblem(IProblem iProblem, int tabIndex, int lineNumber) { + public static JavaProblem fromIProblem(IProblem iProblem, int tabIndex, int lineNumber) { + int type = 0; if(iProblem.isError()) { type = ERROR; } else if (iProblem.isWarning()) { type = WARNING; } - this.tabIndex = tabIndex; - this.lineNumber = lineNumber; - this.message = ErrorMessageSimplifier.getSimplifiedErrorMessage(iProblem); + String message = ErrorMessageSimplifier.getSimplifiedErrorMessage(iProblem); + return new JavaProblem(message, type, tabIndex, lineNumber); } public void setPDEOffsets(int startOffset, int stopOffset){ diff --git a/java/src/processing/mode/java/pdex/PDEX.java b/java/src/processing/mode/java/pdex/PDEX.java index 37b0d9b8e..b6a7e0ffb 100644 --- a/java/src/processing/mode/java/pdex/PDEX.java +++ b/java/src/processing/mode/java/pdex/PDEX.java @@ -1079,40 +1079,69 @@ public class PDEX { Map suggCache = JavaMode.importSuggestEnabled ? new HashMap<>() : Collections.emptyMap(); - // Process problems + final List problems = new ArrayList<>(); + IProblem[] iproblems = ps.compilationUnit.getProblems(); - final List problems = Arrays.stream(iproblems) - // Filter Warnings if they are not enabled - .filter(iproblem -> !(iproblem.isWarning() && !JavaMode.warningsEnabled)) - // Hide a useless error which is produced when a line ends with - // an identifier without a semicolon. "Missing a semicolon" is - // also produced and is preferred over this one. - // (Syntax error, insert ":: IdentifierOrNew" to complete Expression) - // See: https://bugs.eclipse.org/bugs/show_bug.cgi?id=405780 - .filter(iproblem -> !iproblem.getMessage() - .contains("Syntax error, insert \":: IdentifierOrNew\"")) - // Transform into our Problems - .map(iproblem -> { - int start = iproblem.getSourceStart(); - int stop = iproblem.getSourceEnd() + 1; // make it exclusive - SketchInterval in = ps.mapJavaToSketch(start, stop); - if (in == SketchInterval.BEFORE_START) return null; - int line = ps.tabOffsetToTabLine(in.tabIndex, in.startTabOffset); - JavaProblem p = new JavaProblem(iproblem, in.tabIndex, line); - p.setPDEOffsets(in.startTabOffset, in.stopTabOffset); - // Handle import suggestions - if (JavaMode.importSuggestEnabled && isUndefinedTypeProblem(iproblem)) { - ClassPath cp = ps.searchClassPath; - String[] s = suggCache.computeIfAbsent(iproblem.getArguments()[0], - name -> getImportSuggestions(cp, name)); - p.setImportSuggestions(s); - } + { // Handle missing brace problems + IProblem missingBraceProblem = Arrays.stream(iproblems) + .filter(ErrorChecker::isMissingBraceProblem) + .findFirst() + // Ignore if it is at the end of file + .filter(p -> p.getSourceEnd() + 1 < ps.javaCode.length()) + // Ignore if the tab number does not match our detected tab number + .filter(p -> ps.missingBraceProblems.isEmpty() || + ps.missingBraceProblems.get(0).getTabIndex() == + ps.mapJavaToSketch(p.getSourceStart(), p.getSourceEnd()+1).tabIndex + ) + .orElse(null); - return p; - }) - .filter(Objects::nonNull) - .collect(Collectors.toList()); + // If there is missing brace ignore all other problems + if (missingBraceProblem != null) { + // Prefer ECJ problem, shows location more accurately + iproblems = new IProblem[]{missingBraceProblem}; + } else if (!ps.missingBraceProblems.isEmpty()) { + // Fallback to manual detection + problems.addAll(ps.missingBraceProblems); + } + } + + if (problems.isEmpty()) { + List cuProblems = Arrays.stream(iproblems) + // Filter Warnings if they are not enabled + .filter(iproblem -> !(iproblem.isWarning() && !JavaMode.warningsEnabled)) + // Hide a useless error which is produced when a line ends with + // an identifier without a semicolon. "Missing a semicolon" is + // also produced and is preferred over this one. + // (Syntax error, insert ":: IdentifierOrNew" to complete Expression) + // See: https://bugs.eclipse.org/bugs/show_bug.cgi?id=405780 + .filter(iproblem -> !iproblem.getMessage() + .contains("Syntax error, insert \":: IdentifierOrNew\"")) + // Transform into our Problems + .map(iproblem -> { + int start = iproblem.getSourceStart(); + int stop = iproblem.getSourceEnd() + 1; // make it exclusive + 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); + p.setPDEOffsets(in.startTabOffset, in.stopTabOffset); + + // Handle import suggestions + if (JavaMode.importSuggestEnabled && isUndefinedTypeProblem(iproblem)) { + ClassPath cp = ps.searchClassPath; + String[] s = suggCache.computeIfAbsent(iproblem.getArguments()[0], + name -> getImportSuggestions(cp, name)); + p.setImportSuggestions(s); + } + + return p; + }) + .filter(Objects::nonNull) + .collect(Collectors.toList()); + + problems.addAll(cuProblems); + } if (scheduledUiUpdate != null) { scheduledUiUpdate.cancel(true); @@ -1129,13 +1158,28 @@ public class PDEX { } - private boolean isUndefinedTypeProblem(IProblem iproblem) { + static private boolean isUndefinedTypeProblem(IProblem iproblem) { int id = iproblem.getID(); return id == IProblem.UndefinedType || id == IProblem.UndefinedName || id == IProblem.UnresolvedVariable; } + static private boolean isMissingBraceProblem(IProblem iproblem) { + switch (iproblem.getID()) { + case IProblem.ParsingErrorInsertToComplete: { + char brace = iproblem.getArguments()[0].charAt(0); + return brace == '{' || brace == '}'; + } + case IProblem.ParsingErrorInsertTokenAfter: { + char brace = iproblem.getArguments()[1].charAt(0); + return brace == '{' || brace == '}'; + } + default: + return false; + } + } + public static String[] getImportSuggestions(ClassPath cp, String className) { RegExpResourceFilter regf = new RegExpResourceFilter( diff --git a/java/src/processing/mode/java/pdex/PreprocessedSketch.java b/java/src/processing/mode/java/pdex/PreprocessedSketch.java index 9f4fd2005..53fa7402f 100644 --- a/java/src/processing/mode/java/pdex/PreprocessedSketch.java +++ b/java/src/processing/mode/java/pdex/PreprocessedSketch.java @@ -11,6 +11,7 @@ import java.util.Arrays; import java.util.Collections; import java.util.List; +import processing.app.Problem; import processing.app.Sketch; import processing.core.PApplet; import processing.mode.java.pdex.TextTransform.OffsetMapper; @@ -34,6 +35,8 @@ public class PreprocessedSketch { public final OffsetMapper offsetMapper; + public final List missingBraceProblems; + public final boolean hasSyntaxErrors; public final boolean hasCompilationErrors; @@ -212,6 +215,8 @@ public class PreprocessedSketch { public OffsetMapper offsetMapper; + public final List missingBraceProblems = new ArrayList<>(0); + public boolean hasSyntaxErrors; public boolean hasCompilationErrors; @@ -246,6 +251,8 @@ public class PreprocessedSketch { offsetMapper = b.offsetMapper != null ? b.offsetMapper : OffsetMapper.EMPTY_MAPPER; + missingBraceProblems = Collections.unmodifiableList(b.missingBraceProblems); + hasSyntaxErrors = b.hasSyntaxErrors; hasCompilationErrors = b.hasCompilationErrors; diff --git a/java/src/processing/mode/java/pdex/PreprocessingService.java b/java/src/processing/mode/java/pdex/PreprocessingService.java index 8a3005256..6ac0981ef 100644 --- a/java/src/processing/mode/java/pdex/PreprocessingService.java +++ b/java/src/processing/mode/java/pdex/PreprocessingService.java @@ -392,6 +392,15 @@ public class PreprocessingService { } } + { // Check for missing braces + List missingBraceProblems = + SourceUtils.checkForMissingBraces(workBuffer, result.tabStartOffsets); + if (!missingBraceProblems.isEmpty()) { + result.missingBraceProblems.addAll(missingBraceProblems); + result.hasSyntaxErrors = true; + } + } + // Transform code to parsable state String parsableStage = toParsable.apply(); OffsetMapper parsableMapper = toParsable.getMapper(); @@ -414,7 +423,7 @@ public class PreprocessingService { makeAST(parser, compilableStageChars, COMPILER_OPTIONS); // Get syntax problems from compilable AST - result.hasSyntaxErrors = Arrays.stream(compilableCU.getProblems()) + result.hasSyntaxErrors |= Arrays.stream(compilableCU.getProblems()) .anyMatch(IProblem::isError); // Generate bindings after getting problems - avoids diff --git a/java/src/processing/mode/java/pdex/SourceUtils.java b/java/src/processing/mode/java/pdex/SourceUtils.java index 531c8a3ed..cddfcff1c 100644 --- a/java/src/processing/mode/java/pdex/SourceUtils.java +++ b/java/src/processing/mode/java/pdex/SourceUtils.java @@ -323,4 +323,45 @@ public class SourceUtils { } + static public List checkForMissingBraces(StringBuilder p, int[] tabStartOffsets) { + List problems = new ArrayList<>(0); + tabLoop: for (int tabIndex = 0; tabIndex < tabStartOffsets.length; tabIndex++) { + int tabStartOffset = tabStartOffsets[tabIndex]; + int tabEndOffset = (tabIndex < tabStartOffsets.length - 1) ? + tabStartOffsets[tabIndex + 1] : p.length(); + int depth = 0; + int lineNumber = 0; + for (int i = tabStartOffset; i < tabEndOffset; i++) { + char ch = p.charAt(i); + switch (ch) { + case '{': + depth++; + break; + case '}': + depth--; + break; + case '\n': + lineNumber++; + break; + } + if (depth < 0) { + JavaProblem problem = + new JavaProblem("Found one too many } characters without { to match it.", + JavaProblem.ERROR, tabIndex, lineNumber); + problem.setPDEOffsets(i - tabStartOffset, i - tabStartOffset + 1); + problems.add(problem); + continue tabLoop; + } + } + if (depth > 0) { + JavaProblem problem = + new JavaProblem("Found one too many { characters without } to match it.", + JavaProblem.ERROR, tabIndex, lineNumber - 1); + problem.setPDEOffsets(tabEndOffset - tabStartOffset - 2, tabEndOffset - tabStartOffset - 1); + problems.add(problem); + } + } + return problems; + } + } From 0a02898ff11ffe6ebf9334d8a43162884308542c Mon Sep 17 00:00:00 2001 From: Jakub Valtar Date: Sun, 18 Dec 2016 00:36:22 +0100 Subject: [PATCH 6/6] Don't cache search classpath, only build it when looking for a class Fixes #4748 Libraries imported in opened sketches will be still locked by latest preprocessed CompilationUnit. Otherwise we would have to copy library jars somewhere else so they can be available to CompilationUnit while Contribution Manager updates original unlocked jars. --- java/src/processing/mode/java/pdex/PDEX.java | 7 ++++++- java/src/processing/mode/java/pdex/PreprocessedSketch.java | 6 +++--- .../processing/mode/java/pdex/PreprocessingService.java | 7 +++---- 3 files changed, 12 insertions(+), 8 deletions(-) diff --git a/java/src/processing/mode/java/pdex/PDEX.java b/java/src/processing/mode/java/pdex/PDEX.java index b6a7e0ffb..9c2d0b30d 100644 --- a/java/src/processing/mode/java/pdex/PDEX.java +++ b/java/src/processing/mode/java/pdex/PDEX.java @@ -1,6 +1,7 @@ package processing.mode.java.pdex; import com.google.classpath.ClassPath; +import com.google.classpath.ClassPathFactory; import com.google.classpath.RegExpResourceFilter; import org.eclipse.jdt.core.compiler.IProblem; @@ -44,6 +45,7 @@ import java.util.concurrent.Executors; import java.util.concurrent.ScheduledExecutorService; import java.util.concurrent.ScheduledFuture; import java.util.concurrent.TimeUnit; +import java.util.concurrent.atomic.AtomicReference; import java.util.function.Consumer; import java.util.function.Predicate; import java.util.regex.Pattern; @@ -1106,6 +1108,8 @@ public class PDEX { } } + AtomicReference searchClassPath = new AtomicReference<>(null); + if (problems.isEmpty()) { List cuProblems = Arrays.stream(iproblems) // Filter Warnings if they are not enabled @@ -1129,7 +1133,8 @@ public class PDEX { // Handle import suggestions if (JavaMode.importSuggestEnabled && isUndefinedTypeProblem(iproblem)) { - ClassPath cp = ps.searchClassPath; + ClassPath cp = searchClassPath.updateAndGet(prev -> prev != null ? + prev : new ClassPathFactory().createFromPaths(ps.searchClassPathArray)); String[] s = suggCache.computeIfAbsent(iproblem.getArguments()[0], name -> getImportSuggestions(cp, name)); p.setImportSuggestions(s); diff --git a/java/src/processing/mode/java/pdex/PreprocessedSketch.java b/java/src/processing/mode/java/pdex/PreprocessedSketch.java index 53fa7402f..50838eff8 100644 --- a/java/src/processing/mode/java/pdex/PreprocessedSketch.java +++ b/java/src/processing/mode/java/pdex/PreprocessedSketch.java @@ -26,7 +26,7 @@ public class PreprocessedSketch { public final ClassPath classPath; public final URLClassLoader classLoader; - public final ClassPath searchClassPath; + public final String[] searchClassPathArray; public final int[] tabStartOffsets; @@ -206,7 +206,7 @@ public class PreprocessedSketch { public ClassPath classPath; public URLClassLoader classLoader; - public ClassPath searchClassPath; + public String[] searchClassPathArray; public int[] tabStartOffsets = new int[0]; @@ -242,7 +242,7 @@ public class PreprocessedSketch { classPath = b.classPath; classLoader = b.classLoader; - searchClassPath = b.searchClassPath; + searchClassPathArray = b.searchClassPathArray; tabStartOffsets = b.tabStartOffsets; diff --git a/java/src/processing/mode/java/pdex/PreprocessingService.java b/java/src/processing/mode/java/pdex/PreprocessingService.java index 6ac0981ef..ce535a92d 100644 --- a/java/src/processing/mode/java/pdex/PreprocessingService.java +++ b/java/src/processing/mode/java/pdex/PreprocessingService.java @@ -341,7 +341,7 @@ public class PreprocessingService { boolean rebuildClassPath = reloadCodeFolder || rebuildLibraryClassPath || prevResult.classLoader == null || prevResult.classPath == null || - prevResult.classPathArray == null || prevResult.searchClassPath == null; + prevResult.classPathArray == null || prevResult.searchClassPathArray == null; if (reloadCodeFolder) { codeFolderClassPath = buildCodeFolderClassPath(sketch); @@ -381,13 +381,12 @@ public class PreprocessingService { searchClassPath.addAll(coreLibraryClassPath); searchClassPath.addAll(codeFolderClassPath); - String[] searchClassPathArray = searchClassPath.stream().toArray(String[]::new); - result.searchClassPath = classPathFactory.createFromPaths(searchClassPathArray); + result.searchClassPathArray = searchClassPath.stream().toArray(String[]::new); } } else { result.classLoader = prevResult.classLoader; result.classPath = prevResult.classPath; - result.searchClassPath = prevResult.searchClassPath; + result.searchClassPathArray = prevResult.searchClassPathArray; result.classPathArray = prevResult.classPathArray; } }