From 442974f8e3f5d4d0f06e1b4abc6b21a532f406aa Mon Sep 17 00:00:00 2001 From: Eric Milles Date: Fri, 10 Jan 2020 11:45:28 -0600 Subject: [PATCH] Fix for #1018: save source context in uncaught exception error message --- .../tests/locations/SourceLocationsTests.java | 4 +- .../groovy/control/CompilationUnit.java | 36 +++++++++++++++++ .../groovy/control/CompilationUnit.java | 36 +++++++++++++++++ .../groovy/control/CompilationUnit.java | 39 ++++++++++++++++++- .../ast/GroovyCompilationUnitDeclaration.java | 27 +------------ 5 files changed, 114 insertions(+), 28 deletions(-) diff --git a/base-test/org.eclipse.jdt.groovy.core.tests.builder/src/org/eclipse/jdt/core/groovy/tests/locations/SourceLocationsTests.java b/base-test/org.eclipse.jdt.groovy.core.tests.builder/src/org/eclipse/jdt/core/groovy/tests/locations/SourceLocationsTests.java index 55c3bf0779..9a25a5c5ba 100644 --- a/base-test/org.eclipse.jdt.groovy.core.tests.builder/src/org/eclipse/jdt/core/groovy/tests/locations/SourceLocationsTests.java +++ b/base-test/org.eclipse.jdt.groovy.core.tests.builder/src/org/eclipse/jdt/core/groovy/tests/locations/SourceLocationsTests.java @@ -1,5 +1,5 @@ /* - * Copyright 2009-2019 the original author or authors. + * Copyright 2009-2020 the original author or authors. * * Licensed under the Apache License, Version 2.0 (the "License"); * you may not use this file except in compliance with the License. @@ -704,7 +704,7 @@ public void testSourceLocationsTrailingWhitespace4() throws Exception { @Test // STS-3878 public void testErrorPositionForUnsupportedOperation() throws Exception { - assumeTrue(!isAtLeastGroovy(26)); + assumeTrue(!isAtLeastGroovy(30)); String source = "def a = 'a'\n" + "def b = 'b'\n" + diff --git a/base/org.codehaus.groovy24/src/org/codehaus/groovy/control/CompilationUnit.java b/base/org.codehaus.groovy24/src/org/codehaus/groovy/control/CompilationUnit.java index 4a280089a2..0c5ce453c9 100644 --- a/base/org.codehaus.groovy24/src/org/codehaus/groovy/control/CompilationUnit.java +++ b/base/org.codehaus.groovy24/src/org/codehaus/groovy/control/CompilationUnit.java @@ -64,6 +64,7 @@ import java.util.LinkedList; import java.util.List; import java.util.Map; +import java.util.Optional; /** * The CompilationUnit collects all compilation data as it is generated by the compiler system. @@ -1146,18 +1147,52 @@ public void applyToPrimaryClassNodes(PrimaryClassNodeOperation body) throws Comp } catch (GroovyBugError e) { changeBugText(e, context); throw e; + /* GRECLIPSE edit } catch (NoClassDefFoundError e) { // effort to get more logging in case a dependency of a class is loaded // although it shouldn't have convertUncaughtExceptionToCompilationError(e); } catch (Exception e) { convertUncaughtExceptionToCompilationError(e); + */ + } catch (Exception | LinkageError e) { + ErrorCollector errorCollector = null; + // check for a nested compilation exception + for (Throwable t = e.getCause(); t != e && t != null; t = t.getCause()) { + if (t instanceof MultipleCompilationErrorsException) { + MultipleCompilationErrorsException mcee = (MultipleCompilationErrorsException) t; + errorCollector = mcee.getErrorCollector(); + break; + } + } + + if (errorCollector != null) { + getErrorCollector().addCollectorContents(errorCollector); + } else { + if (e instanceof GroovyRuntimeException) { + GroovyRuntimeException gre = (GroovyRuntimeException) e; + context = Optional.ofNullable(gre.getModule()).map(ModuleNode::getContext).orElse(context); + } + if (context != null) { + if (e instanceof SyntaxException) { + getErrorCollector().addError((SyntaxException) e, context); + } else if (e.getCause() instanceof SyntaxException) { + getErrorCollector().addError((SyntaxException) e.getCause(), context); + } else { + getErrorCollector().addException(e instanceof Exception ? (Exception) e : new RuntimeException(e), context); + } + } else { + getErrorCollector().addError(new ExceptionMessage(e instanceof Exception ? (Exception) e : new RuntimeException(e), debug, this)); + } + } + // GRECLIPSE end } } getErrorCollector().failIfErrors(); } + /* GRECLIPSE edit private void convertUncaughtExceptionToCompilationError(final Throwable e) { // check the exception for a nested compilation exception ErrorCollector nestedCollector = null; @@ -1175,6 +1210,7 @@ private void convertUncaughtExceptionToCompilationError(final Throwable e) { getErrorCollector().addError(new ExceptionMessage(err, configuration.getDebug(), this)); } } + */ public void applyToGeneratedGroovyClasses(GroovyClassOperation body) throws CompilationFailedException { if (this.phase != Phases.OUTPUT && !(this.phase == Phases.CLASS_GENERATION && this.phaseComplete)) { diff --git a/base/org.codehaus.groovy25/src/org/codehaus/groovy/control/CompilationUnit.java b/base/org.codehaus.groovy25/src/org/codehaus/groovy/control/CompilationUnit.java index 858113fe71..5aaa66c32f 100644 --- a/base/org.codehaus.groovy25/src/org/codehaus/groovy/control/CompilationUnit.java +++ b/base/org.codehaus.groovy25/src/org/codehaus/groovy/control/CompilationUnit.java @@ -64,6 +64,7 @@ import java.util.LinkedList; import java.util.List; import java.util.Map; +import java.util.Optional; /** * The CompilationUnit collects all compilation data as it is generated by the compiler system. @@ -1124,16 +1125,50 @@ public void applyToPrimaryClassNodes(PrimaryClassNodeOperation body) throws Comp } catch (GroovyBugError e) { changeBugText(e, context); throw e; + /* GRECLIPSE edit } catch (NoClassDefFoundError | Exception e) { // effort to get more logging in case a dependency of a class is loaded // although it shouldn't have convertUncaughtExceptionToCompilationError(e); + */ + } catch (Exception | LinkageError e) { + ErrorCollector errorCollector = null; + // check for a nested compilation exception + for (Throwable t = e.getCause(); t != e && t != null; t = t.getCause()) { + if (t instanceof MultipleCompilationErrorsException) { + MultipleCompilationErrorsException mcee = (MultipleCompilationErrorsException) t; + errorCollector = mcee.getErrorCollector(); + break; + } + } + + if (errorCollector != null) { + getErrorCollector().addCollectorContents(errorCollector); + } else { + if (e instanceof GroovyRuntimeException) { + GroovyRuntimeException gre = (GroovyRuntimeException) e; + context = Optional.ofNullable(gre.getModule()).map(ModuleNode::getContext).orElse(context); + } + if (context != null) { + if (e instanceof SyntaxException) { + getErrorCollector().addError((SyntaxException) e, context); + } else if (e.getCause() instanceof SyntaxException) { + getErrorCollector().addError((SyntaxException) e.getCause(), context); + } else { + getErrorCollector().addException(e instanceof Exception ? (Exception) e : new RuntimeException(e), context); + } + } else { + getErrorCollector().addError(new ExceptionMessage(e instanceof Exception ? (Exception) e : new RuntimeException(e), debug, this)); + } + } + // GRECLIPSE end } } getErrorCollector().failIfErrors(); } + /* GRECLIPSE edit private void convertUncaughtExceptionToCompilationError(final Throwable e) { // check the exception for a nested compilation exception ErrorCollector nestedCollector = null; @@ -1151,6 +1186,7 @@ private void convertUncaughtExceptionToCompilationError(final Throwable e) { getErrorCollector().addError(new ExceptionMessage(err, configuration.getDebug(), this)); } } + */ public void applyToGeneratedGroovyClasses(GroovyClassOperation body) throws CompilationFailedException { if (this.phase != Phases.OUTPUT && !(this.phase == Phases.CLASS_GENERATION && this.phaseComplete)) { diff --git a/base/org.codehaus.groovy30/src/org/codehaus/groovy/control/CompilationUnit.java b/base/org.codehaus.groovy30/src/org/codehaus/groovy/control/CompilationUnit.java index 4ea48c7566..78522dfbdb 100644 --- a/base/org.codehaus.groovy30/src/org/codehaus/groovy/control/CompilationUnit.java +++ b/base/org.codehaus.groovy30/src/org/codehaus/groovy/control/CompilationUnit.java @@ -19,6 +19,7 @@ package org.codehaus.groovy.control; import groovy.lang.GroovyClassLoader; +import groovy.lang.GroovyRuntimeException; import groovy.transform.CompilationUnitAware; import org.codehaus.groovy.GroovyBugError; import org.codehaus.groovy.ast.ClassHelper; @@ -66,6 +67,7 @@ import java.util.LinkedList; import java.util.List; import java.util.Map; +import java.util.Optional; import java.util.Set; /** @@ -1138,16 +1140,50 @@ public void applyToPrimaryClassNodes(PrimaryClassNodeOperation body) throws Comp } catch (GroovyBugError e) { changeBugText(e, context); throw e; + /* GRECLIPSE edit } catch (NoClassDefFoundError | Exception e) { // effort to get more logging in case a dependency of a class is loaded // although it shouldn't have convertUncaughtExceptionToCompilationError(e); + */ + } catch (Exception | LinkageError e) { + ErrorCollector errorCollector = null; + // check for a nested compilation exception + for (Throwable t = e.getCause(); t != e && t != null; t = t.getCause()) { + if (t instanceof MultipleCompilationErrorsException) { + MultipleCompilationErrorsException mcee = (MultipleCompilationErrorsException) t; + errorCollector = mcee.getErrorCollector(); + break; + } + } + + if (errorCollector != null) { + getErrorCollector().addCollectorContents(errorCollector); + } else { + if (e instanceof GroovyRuntimeException) { + GroovyRuntimeException gre = (GroovyRuntimeException) e; + context = Optional.ofNullable(gre.getModule()).map(ModuleNode::getContext).orElse(context); + } + if (context != null) { + if (e instanceof SyntaxException) { + getErrorCollector().addError((SyntaxException) e, context); + } else if (e.getCause() instanceof SyntaxException) { + getErrorCollector().addError((SyntaxException) e.getCause(), context); + } else { + getErrorCollector().addException(e instanceof Exception ? (Exception) e : new RuntimeException(e), context); + } + } else { + getErrorCollector().addError(new ExceptionMessage(e instanceof Exception ? (Exception) e : new RuntimeException(e), debug, this)); + } + } + // GRECLIPSE end } } getErrorCollector().failIfErrors(); } + /* GRECLIPSE edit private void convertUncaughtExceptionToCompilationError(final Throwable e) { // check the exception for a nested compilation exception ErrorCollector nestedCollector = null; @@ -1165,6 +1201,7 @@ private void convertUncaughtExceptionToCompilationError(final Throwable e) { getErrorCollector().addError(new ExceptionMessage(err, configuration.getDebug(), this)); } } + */ public void applyToGeneratedGroovyClasses(GroovyClassOperation body) throws CompilationFailedException { if (this.phase != Phases.OUTPUT && !(this.phase == Phases.CLASS_GENERATION && this.phaseComplete)) { @@ -1263,4 +1300,4 @@ public void tweak(boolean isReconcile) { */ public final String excludeGlobalASTScan; // GRECLIPSE end -} +} \ No newline at end of file diff --git a/base/org.eclipse.jdt.groovy.core/src/org/codehaus/jdt/groovy/internal/compiler/ast/GroovyCompilationUnitDeclaration.java b/base/org.eclipse.jdt.groovy.core/src/org/codehaus/jdt/groovy/internal/compiler/ast/GroovyCompilationUnitDeclaration.java index 306f2a0a14..040a8ac1c1 100644 --- a/base/org.eclipse.jdt.groovy.core/src/org/codehaus/jdt/groovy/internal/compiler/ast/GroovyCompilationUnitDeclaration.java +++ b/base/org.eclipse.jdt.groovy.core/src/org/codehaus/jdt/groovy/internal/compiler/ast/GroovyCompilationUnitDeclaration.java @@ -229,13 +229,11 @@ public boolean processToPhase(int phase) { ErrorCollector collector = groovySourceUnit.getErrorCollector(); if (collector.hasErrors() || collector.hasWarnings()) { recordProblems(collector.getErrors(), collector.getWarnings()); - return !collector.hasErrors(); - } else { + } + if (!collector.hasErrors()) { return true; } } catch (MultipleCompilationErrorsException e) { - fixGroovyRuntimeException(e); - if (GroovyLogManager.manager.hasLoggers()) { GroovyLogManager.manager.log(TraceCategory.COMPILER, e.getMessage()); } @@ -288,27 +286,6 @@ public boolean processToPhase(int phase) { return false; } - /** Unwraps any SyntaxExceptions embedded within a GroovyRuntimeException. */ - private void fixGroovyRuntimeException(MultipleCompilationErrorsException mce) { - List syntaxErrors = new ArrayList<>(); - - for (Iterator it = mce.getErrorCollector().getErrors().iterator(); it.hasNext();) { - Message m = it.next(); - if (m instanceof ExceptionMessage) { - ExceptionMessage em = (ExceptionMessage) m; - if (em.getCause() instanceof GroovyRuntimeException && - ((GroovyRuntimeException) em.getCause()).getCause() instanceof SyntaxException) { - syntaxErrors.add((SyntaxException) em.getCause().getCause()); - it.remove(); - } - } - } - - for (SyntaxException se : syntaxErrors) { - mce.getErrorCollector().addError(se, groovySourceUnit); - } - } - //-------------------------------------------------------------------------- /**