Improve bom checking and address reported problems

Closes gh-38250

buildSrc/src/main/java/org/springframework/boot/build/bom/BomPlugin.java

@@ -62,7 +62,8 @@ public class BomPlugin implements Plugin<Project> {
 		createApiEnforcedConfiguration(project);
 		BomExtension bom = project.getExtensions()
 			.create("bom", BomExtension.class, project.getDependencies(), project);
-		project.getTasks().create("bomrCheck", CheckBom.class, bom);
+		CheckBom checkBom = project.getTasks().create("bomrCheck", CheckBom.class, bom);
+		project.getTasks().named("check").configure((check) -> check.dependsOn(checkBom));
 		project.getTasks().create("bomrUpgrade", UpgradeBom.class, bom);
 		project.getTasks().create("moveToSnapshots", MoveToSnapshots.class, bom);
 		new PublishingCustomizer(project, bom).customize();

buildSrc/src/main/java/org/springframework/boot/build/bom/CheckBom.java

@@ -16,18 +16,25 @@
 
 package org.springframework.boot.build.bom;
 
+import java.util.ArrayList;
+import java.util.List;
 import java.util.Set;
 import java.util.TreeSet;
 import java.util.stream.Collectors;
 
 import javax.inject.Inject;
 
+import org.apache.maven.artifact.versioning.ArtifactVersion;
+import org.apache.maven.artifact.versioning.DefaultArtifactVersion;
+import org.apache.maven.artifact.versioning.Restriction;
+import org.apache.maven.artifact.versioning.VersionRange;
 import org.gradle.api.DefaultTask;
-import org.gradle.api.InvalidUserDataException;
+import org.gradle.api.GradleException;
 import org.gradle.api.tasks.TaskAction;
 
 import org.springframework.boot.build.bom.Library.Group;
 import org.springframework.boot.build.bom.Library.Module;
+import org.springframework.boot.build.bom.Library.ProhibitedVersion;
 import org.springframework.boot.build.bom.bomr.version.DependencyVersion;
 
 /**
@@ -46,18 +53,41 @@ public class CheckBom extends DefaultTask {
 
 	@TaskAction
 	void checkBom() {
+		List<String> errors = new ArrayList<>();
 		for (Library library : this.bom.getLibraries()) {
-			for (Group group : library.getGroups()) {
-				for (Module module : group.getModules()) {
-					if (!module.getExclusions().isEmpty()) {
-						checkExclusions(group.getId(), module, library.getVersion().getVersion());
-					}
+			checkLibrary(library, errors);
+		}
+		if (!errors.isEmpty()) {
+			System.out.println();
+			errors.forEach(System.out::println);
+			System.out.println();
+			throw new GradleException("Bom check failed. See previous output for details.");
+		}
+	}
+
+	private void checkLibrary(Library library, List<String> errors) {
+		List<String> libraryErrors = new ArrayList<>();
+		checkExclusions(library, libraryErrors);
+		checkProhibitedVersions(library, libraryErrors);
+		if (!libraryErrors.isEmpty()) {
+			errors.add(library.getName());
+			for (String libraryError : libraryErrors) {
+				errors.add("    - " + libraryError);
+			}
+		}
+	}
+
+	private void checkExclusions(Library library, List<String> errors) {
+		for (Group group : library.getGroups()) {
+			for (Module module : group.getModules()) {
+				if (!module.getExclusions().isEmpty()) {
+					checkExclusions(group.getId(), module, library.getVersion().getVersion(), errors);
 				}
 			}
 		}
 	}
 
-	private void checkExclusions(String groupId, Module module, DependencyVersion version) {
+	private void checkExclusions(String groupId, Module module, DependencyVersion version, List<String> errors) {
 		Set<String> resolved = getProject().getConfigurations()
 			.detachedConfiguration(
 					getProject().getDependencies().create(groupId + ":" + module.getName() + ":" + version))
@@ -87,8 +117,34 @@ public class CheckBom extends DefaultTask {
 		}
 		exclusions.removeAll(resolved);
 		if (!unused.isEmpty()) {
-			throw new InvalidUserDataException(
-					"Unnecessary exclusions on " + groupId + ":" + module.getName() + ": " + exclusions);
+			errors.add("Unnecessary exclusions on " + groupId + ":" + module.getName() + ": " + exclusions);
+		}
+	}
+
+	private void checkProhibitedVersions(Library library, List<String> errors) {
+		ArtifactVersion currentVersion = new DefaultArtifactVersion(library.getVersion().getVersion().toString());
+		for (ProhibitedVersion prohibited : library.getProhibitedVersions()) {
+			if (prohibited.isProhibited(library.getVersion().getVersion().toString())) {
+				errors.add("Current version " + currentVersion + " is prohibited");
+			}
+			else {
+				VersionRange versionRange = prohibited.getRange();
+				if (versionRange != null) {
+					for (Restriction restriction : versionRange.getRestrictions()) {
+						ArtifactVersion upperBound = restriction.getUpperBound();
+						if (upperBound == null) {
+							return;
+						}
+						int comparison = currentVersion.compareTo(upperBound);
+						if ((restriction.isUpperBoundInclusive() && comparison <= 0)
+								|| ((!restriction.isUpperBoundInclusive()) && comparison < 0)) {
+							return;
+						}
+					}
+					errors.add("Version range " + versionRange + " is ineffective as the current version, "
+							+ currentVersion + ", is greater than its upper bound");
+				}
+			}
 		}
 	}
 

buildSrc/src/main/java/org/springframework/boot/build/bom/Library.java

@@ -20,6 +20,7 @@ import java.util.Collections;
 import java.util.List;
 import java.util.Locale;
 
+import org.apache.maven.artifact.versioning.DefaultArtifactVersion;
 import org.apache.maven.artifact.versioning.VersionRange;
 
 import org.springframework.boot.build.bom.bomr.version.DependencyVersion;
@@ -141,6 +142,16 @@ public class Library {
 			return this.reason;
 		}
 
+		public boolean isProhibited(String candidate) {
+			boolean result = false;
+			result = result
+					|| (this.range != null && this.range.containsVersion(new DefaultArtifactVersion(candidate)));
+			result = result || this.startsWith.stream().anyMatch(candidate::startsWith);
+			result = result || this.endsWith.stream().anyMatch(candidate::endsWith);
+			result = result || this.contains.stream().anyMatch(candidate::contains);
+			return result;
+		}
+
 	}
 
 	public static class LibraryVersion {

buildSrc/src/main/java/org/springframework/boot/build/bom/bomr/UpgradeDependencies.java

@@ -34,8 +34,6 @@ import java.util.stream.Collectors;
 
 import javax.inject.Inject;
 
-import org.apache.maven.artifact.versioning.DefaultArtifactVersion;
-import org.apache.maven.artifact.versioning.VersionRange;
 import org.gradle.api.DefaultTask;
 import org.gradle.api.InvalidUserDataException;
 import org.gradle.api.internal.tasks.userinput.UserInputHandler;
@@ -49,7 +47,6 @@ import org.gradle.api.tasks.options.Option;
 
 import org.springframework.boot.build.bom.BomExtension;
 import org.springframework.boot.build.bom.Library;
-import org.springframework.boot.build.bom.Library.ProhibitedVersion;
 import org.springframework.boot.build.bom.bomr.github.GitHub;
 import org.springframework.boot.build.bom.bomr.github.GitHubRepository;
 import org.springframework.boot.build.bom.bomr.github.Issue;
@@ -247,17 +244,7 @@ public abstract class UpgradeDependencies extends DefaultTask {
 	private boolean isNotProhibited(Library library, DependencyVersion candidate) {
 		return !library.getProhibitedVersions()
 			.stream()
-			.anyMatch((prohibited) -> isProhibited(prohibited, candidate.toString()));
-	}
-
-	private boolean isProhibited(ProhibitedVersion prohibited, String candidate) {
-		boolean result = false;
-		VersionRange range = prohibited.getRange();
-		result = result || (range != null && range.containsVersion(new DefaultArtifactVersion(candidate)));
-		result = result || prohibited.getStartsWith().stream().anyMatch(candidate::startsWith);
-		result = result || prohibited.getStartsWith().stream().anyMatch(candidate::endsWith);
-		result = result || prohibited.getStartsWith().stream().anyMatch(candidate::contains);
-		return result;
+			.anyMatch((prohibited) -> prohibited.isProhibited(candidate.toString()));
 	}
 
 	private List<Library> matchingLibraries() {

spring-boot-project/spring-boot-dependencies/build.gradle

@@ -35,9 +35,7 @@ bom {
 				"activemq-jms-pool",
 				"activemq-kahadb-store",
 				"activemq-karaf",
-				"activemq-leveldb-store" {
-					exclude group: "commons-logging", module: "commons-logging"
-				},
+				"activemq-leveldb-store",
 				"activemq-log4j-appender",
 				"activemq-mqtt",
 				"activemq-openwire-generator",
@@ -86,22 +84,15 @@ bom {
 				"artemis-commons" {
 					exclude group: "commons-logging", module: "commons-logging"
 				},
-				"artemis-core-client" {
-					exclude group: "org.apache.geronimo.specs", module: "geronimo-json_1.0_spec"
-				},
+				"artemis-core-client",
 				"artemis-jdbc-store",
-				"artemis-jms-client" {
-					exclude group: "org.apache.geronimo.specs", module: "geronimo-json_1.0_spec"
-				},
-				"artemis-jms-server" {
-					exclude group: "org.apache.geronimo.specs", module: "geronimo-json_1.0_spec"
-				},
+				"artemis-jms-client",
+				"artemis-jms-server",
 				"artemis-journal",
 				"artemis-quorum-api",
 				"artemis-selector",
 				"artemis-server" {
 					exclude group: "commons-logging", module: "commons-logging"
-					exclude group: "org.apache.geronimo.specs", module: "geronimo-json_1.0_spec"
 				},
 				"artemis-service-extensions"
 			]
@@ -197,9 +188,7 @@ bom {
 				"java-driver-bom"
 			]
 			modules = [
-				"java-driver-core" {
-					exclude group: "org.slf4j", module: "jcl-over-slf4j"
-				}
+				"java-driver-core"
 			]
 		}
 	}