Bug 533830: not all compilation errors are sent to the DiagnosticListener.I20181109-0350I20181108-1800

Signed-off-by: Frits Jalvingh <jal@etc.to>
Change-Id: I4ce1afe0a5e6c80982f8ffd859e1e196203b9ab6

6 files changed, 264 insertions, 14 deletions

diff --git a/org.eclipse.jdt.compiler.tool.tests/src/org/eclipse/jdt/compiler/tool/tests/CompilerToolJava9Tests.java b/org.eclipse.jdt.compiler.tool.tests/src/org/eclipse/jdt/compiler/tool/tests/CompilerToolJava9Tests.java
index 90606e7c05..dcb6b28d55 100644
--- a/org.eclipse.jdt.compiler.tool.tests/src/org/eclipse/jdt/compiler/tool/tests/CompilerToolJava9Tests.java
+++ b/org.eclipse.jdt.compiler.tool.tests/src/org/eclipse/jdt/compiler/tool/tests/CompilerToolJava9Tests.java
@@ -21,6 +21,7 @@ import java.io.FileWriter;
 import java.io.IOException;
 import java.io.PrintWriter;
 import java.io.StringWriter;
+import java.io.Writer;
 import java.net.URL;
 import java.nio.charset.Charset;
 import java.nio.file.Path;
@@ -486,14 +487,15 @@ public class CompilerToolJava9Tests extends TestCase {
 		options.add("7");
 		ByteArrayOutputStream errBuffer = new ByteArrayOutputStream();
 		PrintWriter err = new PrintWriter(errBuffer);
-		CompilerInvocationDiagnosticListener listener = new CompilerInvocationDiagnosticListener(err) {
-			@Override
-			public void report(Diagnostic<? extends JavaFileObject> diagnostic) {
-				JavaFileObject source = diagnostic.getSource();
-				assertNotNull("No source", source);
-				super.report(diagnostic);
-			}
-		};
+		CompilerInvocationDiagnosticListener listener = new CompilerInvocationDiagnosticListener(err);
+//		{ bug 533830 removed: an option error has no source associated with it, so the below was copied wrongly.
+//			@Override
+//			public void report(Diagnostic<? extends JavaFileObject> diagnostic) {
+//				JavaFileObject source = diagnostic.getSource();
+//				assertNotNull("No source", source);
+//				super.report(diagnostic);
+//			}
+//		};
 		try {
 			compiler.getTask(printWriter, forwardingJavaFileManager, listener, options, null, units);
 			fail("compilation didn't fail as expected");
@@ -593,6 +595,174 @@ public class CompilerToolJava9Tests extends TestCase {
 	public void testAsPath() {
 		if (this.isJREBelow9) return;
 	}
+	
+	/*-- Code for testing bug 533830 --*/
+	
+	public void testBug533830_1() {
+		if (this.isJREBelow9) return;
+		
+		File src = createClassSource(
+			"package p;\n"
+		+	"public class X {}"	
+		);
+		
+		CompilerBuilder b = new CompilerBuilder()
+			.option("--release", "8")
+			.option("-source", "7")
+			.file(src)
+			;
+		try {
+			b.compile();
+			fail("compilation didn't fail as expected");
+		} catch(IllegalArgumentException iae) {
+			assertEquals("option -source is not supported when --release is used", iae.getMessage());
+		}
+		
+		//-- We must now also have a diagnostic
+		assertTrue("The diagnostic listener did not receive an error for the illegal option", b.listener().hasDiagnostic("option -source is not supported when --release is used"));
+	}
+	
+	/**
+	 * Helps with building a compiler invocation, handling the common parts of testing.
+	 */
+	private final class CompilerBuilder {
+		private final JavaCompiler compiler;
+		
+		private final List<String> options = new ArrayList<>();
+		
+		private final List<File> files = new ArrayList<>();
+		
+		private final StringWriter errorWriter = new StringWriter();
+		
+		private DiagListener listener; 
+		
+		CompilerBuilder() {
+			compiler = compilers[1];
+		}
+
+		public void compile() {
+			String tmpFolder = System.getProperty("java.io.tmpdir");
+			StandardJavaFileManager manager = compiler.getStandardFileManager(null, Locale.getDefault(), Charset.defaultCharset());
+
+			Iterable<? extends JavaFileObject> units = manager.getJavaFileObjectsFromFiles(files);
+			PrintWriter errWriter = new PrintWriter(errorWriter);
+
+			if(! options.contains("-d")) {
+				option("-d", tmpFolder);
+			}
+			
+			listener = new DiagListener(errWriter);
+			
+			ForwardingJavaFileManager<StandardJavaFileManager> forwardingManager = createFileManager(manager);
+			compiler.getTask(errWriter, forwardingManager, listener, options, null, units);
+		}
+		
+		public CompilerBuilder option(String... s) {
+			for(int i = 0; i < s.length; i++)
+				options.add(s[i]);
+			return this;
+		}
+		
+		public CompilerBuilder file(File f) {
+			files.add(f);
+			return this;
+		}
+		
+		public DiagListener listener() {
+			if(null == listener)
+				throw new IllegalStateException("Call compile() before using the listener");
+			return listener;
+		}
+		
+		public ForwardingJavaFileManager<StandardJavaFileManager> createFileManager(StandardJavaFileManager manager) {
+			ForwardingJavaFileManager<StandardJavaFileManager> forwardingJavaFileManager = new ForwardingJavaFileManager<StandardJavaFileManager>(manager) {
+				@Override
+				public FileObject getFileForInput(Location location, String packageName, String relativeName)
+						throws IOException {
+					if (DEBUG) {
+						System.out.println("Create file for input : " + packageName + " " + relativeName + " in location " + location);
+					}
+					return super.getFileForInput(location, packageName, relativeName);
+				}
+				@Override
+				public JavaFileObject getJavaFileForInput(Location location, String className, Kind kind)
+						throws IOException {
+					if (DEBUG) {
+						System.out.println("Create java file for input : " + className + " in location " + location);
+					}
+					return super.getJavaFileForInput(location, className, kind);
+				}
+				@Override
+				public JavaFileObject getJavaFileForOutput(Location location,
+						String className,
+						Kind kind,
+						FileObject sibling) throws IOException {
+
+					if (DEBUG) {
+						System.out.println("Create .class file for " + className + " in location " + location + " with sibling " + sibling.toUri());
+					}
+					JavaFileObject javaFileForOutput = super.getJavaFileForOutput(location, className, kind, sibling);
+					if (DEBUG) {
+						System.out.println(javaFileForOutput.toUri());
+					}
+					return javaFileForOutput;
+				}
+			};
+			return forwardingJavaFileManager;
+		}
+	}
+	
+	static private final class DiagListener extends CompilerInvocationDiagnosticListener {
+		private List<Diagnostic<? extends JavaFileObject>> errorList = new ArrayList<>();
+
+		DiagListener(PrintWriter err) {
+			super(err);
+		}
+		
+		@Override
+		public void report(Diagnostic<? extends JavaFileObject> diagnostic) {
+			errorList.add(diagnostic);
+			super.report(diagnostic);
+		}
+		
+		public int getErrorCount() {
+			return errorList.size();
+		}
+		
+		public boolean hasDiagnostic(String match) {
+			for(Diagnostic<? extends JavaFileObject> d: errorList) {
+				String msg = d.getMessage(Locale.US).toLowerCase();
+				if(msg.contains(match.toLowerCase()))
+					return true;
+			}
+			return false;
+		}
+	}
+	
+	private File createClassSource(String source) {
+		String tmpFolder = System.getProperty("java.io.tmpdir");
+		File inputFile = new File(tmpFolder, "X.java");
+		Writer writer = null;
+		try {
+			writer = new FileWriter(inputFile);
+			writer.write(source);
+			writer.close();
+			return inputFile;
+		} catch (IOException e) {
+			throw new RuntimeException(e);
+		} finally {
+			if (writer != null) {
+				try {
+					writer.close();
+				} catch (IOException e) {
+					// ignore
+				}
+			}
+		}
+	}
+	
+	/*-- end code for bug 533830 --*/
+	
 	/**
 	 * Recursively delete the contents of a directory, including any subdirectories.
 	 * This is not optimized to handle very large or deep directory trees efficiently.
diff --git a/org.eclipse.jdt.compiler.tool/src/org/eclipse/jdt/internal/compiler/tool/EclipseCompiler.java b/org.eclipse.jdt.compiler.tool/src/org/eclipse/jdt/internal/compiler/tool/EclipseCompiler.java
index 90c8db12c7..4b3d38a849 100644
--- a/org.eclipse.jdt.compiler.tool/src/org/eclipse/jdt/internal/compiler/tool/EclipseCompiler.java
+++ b/org.eclipse.jdt.compiler.tool/src/org/eclipse/jdt/internal/compiler/tool/EclipseCompiler.java
@@ -10,6 +10,7 @@
  *
  * Contributors:
  *     IBM Corporation - initial API and implementation
+ *     Frits Jalvingh  - fix for bug 533830.
  *******************************************************************************/
 package org.eclipse.jdt.internal.compiler.tool;
 
@@ -169,6 +170,8 @@ public class EclipseCompiler implements JavaCompiler {
 		try {
 			eclipseCompiler2.configure(optionsToProcess);
 		} catch (IllegalArgumentException e) {
+			if(null != someDiagnosticListener)
+				someDiagnosticListener.report(new ExceptionDiagnostic(e));
 			throw e;
 		}
 
diff --git a/org.eclipse.jdt.compiler.tool/src/org/eclipse/jdt/internal/compiler/tool/EclipseCompilerImpl.java b/org.eclipse.jdt.compiler.tool/src/org/eclipse/jdt/internal/compiler/tool/EclipseCompilerImpl.java
index 1d27e54358..d1f50cac62 100644
--- a/org.eclipse.jdt.compiler.tool/src/org/eclipse/jdt/internal/compiler/tool/EclipseCompilerImpl.java
+++ b/org.eclipse.jdt.compiler.tool/src/org/eclipse/jdt/internal/compiler/tool/EclipseCompilerImpl.java
@@ -13,6 +13,7 @@
  *    IBM Corporation - fix for 342936
  *    Kenneth Olson - Contribution for bug 188796 - [jsr199] Using JSR199 to extend ECJ
  *    Dennis Hendriks - Contribution for bug 188796 - [jsr199] Using JSR199 to extend ECJ
+ *    Frits Jalvingh  - fix for bug 533830.
  *******************************************************************************/
 package org.eclipse.jdt.internal.compiler.tool;
 
@@ -93,6 +94,7 @@ public class EclipseCompilerImpl extends Main {
 				performCompilation();
 			}
 		} catch(IllegalArgumentException e) {
+			diagnosticListener.report(new ExceptionDiagnostic(e));
 			this.logger.logException(e);
 			if (this.systemExitWhenFinished) {
 				cleanup();
@@ -100,6 +102,7 @@ public class EclipseCompilerImpl extends Main {
 			}
 			return false;
 		} catch (RuntimeException e) { // internal compiler failure
+			diagnosticListener.report(new ExceptionDiagnostic(e));
 			e.printStackTrace();
 			this.logger.logException(e);
 			return false;
diff --git a/org.eclipse.jdt.compiler.tool/src/org/eclipse/jdt/internal/compiler/tool/ExceptionDiagnostic.java b/org.eclipse.jdt.compiler.tool/src/org/eclipse/jdt/internal/compiler/tool/ExceptionDiagnostic.java
new file mode 100644
index 0000000000..8822b364fc
--- /dev/null
+++ b/org.eclipse.jdt.compiler.tool/src/org/eclipse/jdt/internal/compiler/tool/ExceptionDiagnostic.java
@@ -0,0 +1,73 @@
+/*******************************************************************************
+ * Copyright (c) 2007, 2018 IBM Corporation and others.
+ *
+ * This program and the accompanying materials
+ * are made available under the terms of the Eclipse Public License 2.0
+ * which accompanies this distribution, and is available at
+ * https://www.eclipse.org/legal/epl-2.0/
+ *
+ * SPDX-License-Identifier: EPL-2.0
+ *
+ * Contributors:
+ *    Frits Jalvingh  - fix for bug 533830.
+ *******************************************************************************/
+package org.eclipse.jdt.internal.compiler.tool;
+
+import java.util.Locale;
+
+import javax.tools.Diagnostic;
+import javax.tools.JavaFileObject;
+import javax.tools.Diagnostic.Kind;
+
+final class ExceptionDiagnostic implements Diagnostic<JavaFileObject> {
+	private final Exception exception;
+
+	ExceptionDiagnostic(Exception e) {
+		this.exception = e;
+	}
+
+	@Override
+	public String getCode() {
+		return "exception";
+	}
+
+	@Override
+	public long getColumnNumber() {
+		return 0;
+	}
+
+	@Override
+	public long getEndPosition() {
+		return 0;
+	}
+
+	@Override
+	public Kind getKind() {
+		return Kind.ERROR;
+	}
+
+	@Override
+	public long getLineNumber() {
+		return 0;
+	}
+
+	@Override
+	public String getMessage(Locale arg0) {
+		return exception.toString();
+	}
+
+	@Override
+	public long getPosition() {
+		return 0;
+	}
+
+	@Override
+	public JavaFileObject getSource() {
+		return null;
+	}
+
+	@Override
+	public long getStartPosition() {
+		return 0;
+	}
+}
\ No newline at end of file
diff --git a/org.eclipse.jdt.core/batch/org/eclipse/jdt/internal/compiler/batch/Main.java b/org.eclipse.jdt.core/batch/org/eclipse/jdt/internal/compiler/batch/Main.java
index 614e0198fd..ec77b2ada8 100644
--- a/org.eclipse.jdt.core/batch/org/eclipse/jdt/internal/compiler/batch/Main.java
+++ b/org.eclipse.jdt.core/batch/org/eclipse/jdt/internal/compiler/batch/Main.java
@@ -28,6 +28,7 @@
  *     Jesper S Moller   - Contributions for
  *								bug 407297 - [1.8][compiler] Control generation of parameter names by option
  *    Mat Booth - Contribution for bug 405176 
+ *    Frits Jalvingh - fix for bug 533830.
  *******************************************************************************/
 package org.eclipse.jdt.internal.compiler.batch;
 
@@ -103,6 +104,7 @@ import org.eclipse.jdt.internal.compiler.lookup.PackageBinding;
 import org.eclipse.jdt.internal.compiler.lookup.ReferenceBinding;
 import org.eclipse.jdt.internal.compiler.lookup.TypeConstants;
 import org.eclipse.jdt.internal.compiler.parser.Parser;
+import org.eclipse.jdt.internal.compiler.problem.DefaultProblem;
 import org.eclipse.jdt.internal.compiler.problem.DefaultProblemFactory;
 import org.eclipse.jdt.internal.compiler.problem.ProblemReporter;
 import org.eclipse.jdt.internal.compiler.problem.ProblemSeverities;
@@ -799,6 +801,9 @@ public class Main implements ProblemSeverities, SuffixConstants {
 
 		private void logProblem(CategorizedProblem problem, int localErrorCount,
 			int globalErrorCount, char[] unitSource) {
+			if(problem instanceof DefaultProblem) {
+				((DefaultProblem) problem).reportError();
+			}
 			if ((this.tagBits & Logger.EMACS) != 0) {
 				String severity = problem.isError() ? "output.emacs.error" : //$NON-NLS-1$
 									problem.isInfo() ? "output.emacs.info" //$NON-NLS-1$
diff --git a/org.eclipse.jdt.core/compiler/org/eclipse/jdt/internal/compiler/ast/CompilationUnitDeclaration.java b/org.eclipse.jdt.core/compiler/org/eclipse/jdt/internal/compiler/ast/CompilationUnitDeclaration.java
index d9bbcecdbb..3bc5761558 100644
--- a/org.eclipse.jdt.core/compiler/org/eclipse/jdt/internal/compiler/ast/CompilationUnitDeclaration.java
+++ b/org.eclipse.jdt.core/compiler/org/eclipse/jdt/internal/compiler/ast/CompilationUnitDeclaration.java
@@ -12,7 +12,8 @@
  *     IBM Corporation - initial API and implementation
  *     Stephan Herrmann  - Contribution for bug 295551
  *     Jesper S Moller   - Contributions for
- *							  Bug 405066 - [1.8][compiler][codegen] Implement code generation infrastructure for JSR335             
+ *							  Bug 405066 - [1.8][compiler][codegen] Implement code generation infrastructure for JSR335
+ *     Frits Jalvingh    - contributions for bug 533830.             
  *******************************************************************************/
 package org.eclipse.jdt.internal.compiler.ast;
 
@@ -241,11 +242,6 @@ public void finalizeProblems() {
 	int problemCount = this.compilationResult.problemCount;
 	CategorizedProblem[] problems = this.compilationResult.problems;
 	if (this.suppressWarningsCount == 0) {
-		 for (int iProblem = 0, length = problemCount; iProblem < length; iProblem++) {
-			 if (problems[iProblem] instanceof DefaultProblem) {
-				 ((DefaultProblem)problems[iProblem]).reportError();
-			 }
-		 }
 		return;
 	}
 	int removed = 0;