Skip to content

Commit

Permalink
Fix for #1018: save source context in uncaught exception error message
Browse files Browse the repository at this point in the history
  • Loading branch information
@eric-milles
eric-milles committed Jan 10, 2020
1 parent 9ff1a7e commit 442974f
Show file tree
Hide file tree
Showing 5 changed files with 114 additions and 28 deletions.
4 changes: 2 additions & 2 deletions ...e.tests.builder/src/org/eclipse/jdt/core/groovy/tests/locations/SourceLocationsTests.java
View file
Original file line number Diff line number Diff line change
@@ -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.
Expand Down Expand Up @@ -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" +
Expand Down
36 changes: 36 additions & 0 deletions base/org.codehaus.groovy24/src/org/codehaus/groovy/control/CompilationUnit.java
View file
Original file line number Diff line number Diff line change
Expand Up @@ -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.
Expand Down Expand Up @@ -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;
Expand All @@ -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)) {
Expand Down
36 changes: 36 additions & 0 deletions base/org.codehaus.groovy25/src/org/codehaus/groovy/control/CompilationUnit.java
View file
Original file line number Diff line number Diff line change
Expand Up @@ -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.
Expand Down Expand Up @@ -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;
Expand All @@ -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)) {
Expand Down
39 changes: 38 additions & 1 deletion base/org.codehaus.groovy30/src/org/codehaus/groovy/control/CompilationUnit.java
View file
Original file line number Diff line number Diff line change
Expand Up @@ -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;
Expand Down Expand Up @@ -66,6 +67,7 @@
import java.util.LinkedList;
import java.util.List;
import java.util.Map;
import java.util.Optional;
import java.util.Set;

/**
Expand Down Expand Up @@ -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;
Expand All @@ -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)) {
Expand Down Expand Up @@ -1263,4 +1300,4 @@ public void tweak(boolean isReconcile) {
*/
public final String excludeGlobalASTScan;
// GRECLIPSE end
}
}
27 changes: 2 additions & 25 deletions ...e/src/org/codehaus/jdt/groovy/internal/compiler/ast/GroovyCompilationUnitDeclaration.java
View file
Original file line number Diff line number Diff line change
Expand Up @@ -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());
}
Expand Down Expand Up @@ -288,27 +286,6 @@ public boolean processToPhase(int phase) {
return false;
}

/** Unwraps any SyntaxExceptions embedded within a GroovyRuntimeException. */
private void fixGroovyRuntimeException(MultipleCompilationErrorsException mce) {
List<SyntaxException> syntaxErrors = new ArrayList<>();

for (Iterator<? extends Message> 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);
}
}

//--------------------------------------------------------------------------

/**
Expand Down

0 comments on commit 442974f

Please sign in to comment.