From ac4dcfbf9af59bf69d171b7dcbf363581f80b27c Mon Sep 17 00:00:00 2001
From: Stephane Nicoll <snicoll@pivotal.io>
Date: Fri, 26 Jul 2019 12:01:35 +0200
Subject: [PATCH] Improve detection of NoSuchMethodError cause

Closes gh-17649
---
 .../analyzer/NoSuchMethodFailureAnalyzer.java | 93 ++++++++++++++++---
 .../NoSuchMethodFailureAnalyzerTests.java     | 48 +++++++++-
 2 files changed, 123 insertions(+), 18 deletions(-)

diff --git a/spring-boot-project/spring-boot/src/main/java/org/springframework/boot/diagnostics/analyzer/NoSuchMethodFailureAnalyzer.java b/spring-boot-project/spring-boot/src/main/java/org/springframework/boot/diagnostics/analyzer/NoSuchMethodFailureAnalyzer.java
index 739ffb5b344..af9a9821cec 100644
--- a/spring-boot-project/spring-boot/src/main/java/org/springframework/boot/diagnostics/analyzer/NoSuchMethodFailureAnalyzer.java
+++ b/spring-boot-project/spring-boot/src/main/java/org/springframework/boot/diagnostics/analyzer/NoSuchMethodFailureAnalyzer.java
@@ -31,12 +31,26 @@ import org.springframework.util.ClassUtils;
  * NoSuchMethodErrors}.
  *
  * @author Andy Wilkinson
+ * @author Stephane Nicoll
  */
 class NoSuchMethodFailureAnalyzer extends AbstractFailureAnalyzer<NoSuchMethodError> {
 
 	@Override
 	protected FailureAnalysis analyze(Throwable rootFailure, NoSuchMethodError cause) {
-		String className = extractClassName(cause);
+		NoSuchMethodDescriptor descriptor = getNoSuchMethodDescriptor(cause.getMessage());
+		if (descriptor == null) {
+			return null;
+		}
+		String description = getDescription(cause, descriptor);
+		return new FailureAnalysis(description,
+				"Correct the classpath of your application so that it contains a single, compatible version of "
+						+ descriptor.getClassName(),
+				cause);
+	}
+
+	protected NoSuchMethodDescriptor getNoSuchMethodDescriptor(String cause) {
+		String message = cleanMessage(cause);
+		String className = extractClassName(message);
 		if (className == null) {
 			return null;
 		}
@@ -48,24 +62,36 @@ class NoSuchMethodFailureAnalyzer extends AbstractFailureAnalyzer<NoSuchMethodEr
 		if (actual == null) {
 			return null;
 		}
-		String description = getDescription(cause, className, candidates, actual);
-		return new FailureAnalysis(description,
-				"Correct the classpath of your application so that it contains a single," + " compatible version of "
-						+ className,
-				cause);
+		return new NoSuchMethodDescriptor(message, className, candidates, actual);
 	}
 
-	private String extractClassName(NoSuchMethodError cause) {
-		int descriptorIndex = cause.getMessage().indexOf('(');
+	private String cleanMessage(String message) {
+		int loadedFromIndex = message.indexOf(" (loaded from");
+		if (loadedFromIndex == -1) {
+			return message;
+		}
+		return message.substring(0, loadedFromIndex);
+	}
+
+	private String extractClassName(String message) {
+		if (message.startsWith("'") && message.endsWith("'")) {
+			int splitIndex = message.indexOf(' ');
+			if (splitIndex == -1) {
+				return null;
+			}
+			message = message.substring(splitIndex + 1);
+		}
+		int descriptorIndex = message.indexOf('(');
 		if (descriptorIndex == -1) {
 			return null;
 		}
-		String classAndMethodName = cause.getMessage().substring(0, descriptorIndex);
+		String classAndMethodName = message.substring(0, descriptorIndex);
 		int methodNameIndex = classAndMethodName.lastIndexOf('.');
 		if (methodNameIndex == -1) {
 			return null;
 		}
-		return classAndMethodName.substring(0, methodNameIndex);
+		String className = classAndMethodName.substring(0, methodNameIndex);
+		return className.replace('/', '.');
 	}
 
 	private List<URL> findCandidates(String className) {
@@ -87,7 +113,7 @@ class NoSuchMethodFailureAnalyzer extends AbstractFailureAnalyzer<NoSuchMethodEr
 		}
 	}
 
-	private String getDescription(NoSuchMethodError cause, String className, List<URL> candidates, URL actual) {
+	private String getDescription(NoSuchMethodError cause, NoSuchMethodDescriptor descriptor) {
 		StringWriter description = new StringWriter();
 		PrintWriter writer = new PrintWriter(description);
 		writer.println("An attempt was made to call a method that does not"
@@ -99,11 +125,12 @@ class NoSuchMethodFailureAnalyzer extends AbstractFailureAnalyzer<NoSuchMethodEr
 		writer.println("The following method did not exist:");
 		writer.println();
 		writer.print("    ");
-		writer.println(cause.getMessage());
+		writer.println(descriptor.getErrorMessage());
 		writer.println();
-		writer.println("The method's class, " + className + ", is available from the following locations:");
+		writer.println(
+				"The method's class, " + descriptor.getClassName() + ", is available from the following locations:");
 		writer.println();
-		for (URL candidate : candidates) {
+		for (URL candidate : descriptor.getCandidateLocations()) {
 			writer.print("    ");
 			writer.println(candidate);
 		}
@@ -111,8 +138,44 @@ class NoSuchMethodFailureAnalyzer extends AbstractFailureAnalyzer<NoSuchMethodEr
 		writer.println("It was loaded from the following location:");
 		writer.println();
 		writer.print("    ");
-		writer.println(actual);
+		writer.println(descriptor.getActualLocation());
 		return description.toString();
 	}
 
+	protected static class NoSuchMethodDescriptor {
+
+		private final String errorMessage;
+
+		private final String className;
+
+		private final List<URL> candidateLocations;
+
+		private final URL actualLocation;
+
+		public NoSuchMethodDescriptor(String errorMessage, String className, List<URL> candidateLocations,
+				URL actualLocation) {
+			this.errorMessage = errorMessage;
+			this.className = className;
+			this.candidateLocations = candidateLocations;
+			this.actualLocation = actualLocation;
+		}
+
+		public String getErrorMessage() {
+			return this.errorMessage;
+		}
+
+		public String getClassName() {
+			return this.className;
+		}
+
+		public List<URL> getCandidateLocations() {
+			return this.candidateLocations;
+		}
+
+		public URL getActualLocation() {
+			return this.actualLocation;
+		}
+
+	}
+
 }
diff --git a/spring-boot-project/spring-boot/src/test/java/org/springframework/boot/diagnostics/analyzer/NoSuchMethodFailureAnalyzerTests.java b/spring-boot-project/spring-boot/src/test/java/org/springframework/boot/diagnostics/analyzer/NoSuchMethodFailureAnalyzerTests.java
index 2e80d8953d0..0c09c2b0e51 100644
--- a/spring-boot-project/spring-boot/src/test/java/org/springframework/boot/diagnostics/analyzer/NoSuchMethodFailureAnalyzerTests.java
+++ b/spring-boot-project/spring-boot/src/test/java/org/springframework/boot/diagnostics/analyzer/NoSuchMethodFailureAnalyzerTests.java
@@ -23,6 +23,7 @@ import org.junit.Test;
 import org.junit.runner.RunWith;
 
 import org.springframework.boot.diagnostics.FailureAnalysis;
+import org.springframework.boot.diagnostics.analyzer.NoSuchMethodFailureAnalyzer.NoSuchMethodDescriptor;
 import org.springframework.boot.testsupport.runner.classpath.ClassPathOverrides;
 import org.springframework.boot.testsupport.runner.classpath.ModifiedClassPathRunner;
 
@@ -33,11 +34,54 @@ import static org.mockito.Mockito.mock;
  * Tests for {@link NoSuchMethodFailureAnalyzer}.
  *
  * @author Andy Wilkinson
+ * @author Stephane Nicoll
  */
 @RunWith(ModifiedClassPathRunner.class)
 @ClassPathOverrides("javax.servlet:servlet-api:2.5")
 public class NoSuchMethodFailureAnalyzerTests {
 
+	@Test
+	public void parseJava8ErrorMessage() {
+		NoSuchMethodDescriptor descriptor = new NoSuchMethodFailureAnalyzer().getNoSuchMethodDescriptor(
+				"javax.servlet.ServletContext.addServlet(Ljava/lang/String;Ljavax/servlet/Servlet;)"
+						+ "Ljavax/servlet/ServletRegistration$Dynamic;");
+		assertThat(descriptor).isNotNull();
+		assertThat(descriptor.getErrorMessage())
+				.isEqualTo("javax.servlet.ServletContext.addServlet(Ljava/lang/String;Ljavax/servlet/Servlet;)"
+						+ "Ljavax/servlet/ServletRegistration$Dynamic;");
+		assertThat(descriptor.getClassName()).isEqualTo("javax.servlet.ServletContext");
+		assertThat(descriptor.getCandidateLocations().size()).isGreaterThan(1);
+		assertThat(descriptor.getActualLocation().toString()).contains("servlet-api-2.5.jar");
+	}
+
+	@Test
+	public void parseJavaOpenJ9ErrorMessage() {
+		NoSuchMethodDescriptor descriptor = new NoSuchMethodFailureAnalyzer().getNoSuchMethodDescriptor(
+				"javax/servlet/ServletContext.addServlet(Ljava/lang/String;Ljavax/servlet/Servlet;)"
+						+ "Ljavax/servlet/ServletRegistration$Dynamic; (loaded from file...");
+		assertThat(descriptor).isNotNull();
+		assertThat(descriptor.getErrorMessage())
+				.isEqualTo("javax/servlet/ServletContext.addServlet(Ljava/lang/String;Ljavax/servlet/Servlet;)"
+						+ "Ljavax/servlet/ServletRegistration$Dynamic;");
+		assertThat(descriptor.getClassName()).isEqualTo("javax.servlet.ServletContext");
+		assertThat(descriptor.getCandidateLocations().size()).isGreaterThan(1);
+		assertThat(descriptor.getActualLocation().toString()).contains("servlet-api-2.5.jar");
+	}
+
+	@Test
+	public void parseJavaImprovedHotspotErrorMessage() {
+		NoSuchMethodDescriptor descriptor = new NoSuchMethodFailureAnalyzer().getNoSuchMethodDescriptor(
+				"'javax.servlet.ServletRegistration$Dynamic javax.servlet.ServletContext.addServlet("
+						+ "java.lang.String, javax.servlet.Servlet)'");
+		assertThat(descriptor).isNotNull();
+		assertThat(descriptor.getErrorMessage())
+				.isEqualTo("'javax.servlet.ServletRegistration$Dynamic javax.servlet.ServletContext.addServlet("
+						+ "java.lang.String, javax.servlet.Servlet)'");
+		assertThat(descriptor.getClassName()).isEqualTo("javax.servlet.ServletContext");
+		assertThat(descriptor.getCandidateLocations().size()).isGreaterThan(1);
+		assertThat(descriptor.getActualLocation().toString()).contains("servlet-api-2.5.jar");
+	}
+
 	@Test
 	public void noSuchMethodErrorIsAnalyzed() {
 		Throwable failure = createFailure();
@@ -45,9 +89,7 @@ public class NoSuchMethodFailureAnalyzerTests {
 		FailureAnalysis analysis = new NoSuchMethodFailureAnalyzer().analyze(failure);
 		assertThat(analysis).isNotNull();
 		assertThat(analysis.getDescription())
-				.contains(NoSuchMethodFailureAnalyzerTests.class.getName() + ".createFailure(")
-				.contains("javax.servlet.ServletContext.addServlet" + "(Ljava/lang/String;Ljavax/servlet/Servlet;)"
-						+ "Ljavax/servlet/ServletRegistration$Dynamic;")
+				.contains(NoSuchMethodFailureAnalyzerTests.class.getName() + ".createFailure(").contains("addServlet(")
 				.contains("class, javax.servlet.ServletContext,");
 	}