root
/
spring-boot
mirror of https://github.com/spring-projects/spring-boot.git
1
0
Fork 0
Code Issues Actions 12 Packages Projects Releases Wiki Activity

Polish "Add rule to prevent calls to Objects.requireNonNull()" 

See gh-41611
This commit is contained in:
Andy Wilkinson 2024-07-29 10:48:45 +01:00
parent fb22c189f4
commit 4ee24bf9bd
9 changed files with 54 additions and 111 deletions
Show Stats Download Patch File Download Diff File Expand all files Collapse all files

37
buildSrc/src/main/java/org/springframework/boot/build/architecture/ArchitectureCheck.java
View File

@ -1,5 +1,5 @@
/*
* Copyright 2022-2023 the original author or authors.
* Copyright 2022-2024 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.
@ -22,6 +22,7 @@ import java.net.URLDecoder;
import java.net.URLEncoder;
import java.nio.file.Files;
import java.nio.file.StandardOpenOption;
import java.util.Collections;
import java.util.List;
import java.util.Objects;
import java.util.function.Supplier;
@ -49,6 +50,7 @@ import org.gradle.api.file.DirectoryProperty;
import org.gradle.api.file.FileCollection;
import org.gradle.api.file.FileTree;
import org.gradle.api.provider.ListProperty;
import org.gradle.api.provider.Property;
import org.gradle.api.tasks.IgnoreEmptyDirectories;
import org.gradle.api.tasks.Input;
import org.gradle.api.tasks.InputFiles;
@ -65,6 +67,7 @@ import org.gradle.api.tasks.TaskAction;
*
* @author Andy Wilkinson
* @author Yanming Zhou
* @author Ivan Malutin
*/
public abstract class ArchitectureCheck extends DefaultTask {
@ -72,14 +75,15 @@ public abstract class ArchitectureCheck extends DefaultTask {
public ArchitectureCheck() {
getOutputDirectory().convention(getProject().getLayout().getBuildDirectory().dir(getName()));
getProhibitObjectsRequireNonNull().convention(true);
getRules().addAll(allPackagesShouldBeFreeOfTangles(),
allBeanPostProcessorBeanMethodsShouldBeStaticAndHaveParametersThatWillNotCausePrematureInitialization(),
allBeanFactoryPostProcessorBeanMethodsShouldBeStaticAndHaveNoParameters(),
noClassesShouldCallStepVerifierStepVerifyComplete(),
noClassesShouldConfigureDefaultStepVerifierTimeout(), noClassesShouldCallCollectorsToList(),
noClassesShouldCallURLEncoderWithStringEncoding(), noClassesShouldCallURLDecoderWithStringEncoding(),
noClassesShouldCallObjectsRequireNonNullWithMessage(),
noClassesShouldCallObjectsRequireNonNullWithSupplier());
noClassesShouldCallURLEncoderWithStringEncoding(), noClassesShouldCallURLDecoderWithStringEncoding());
getRules().addAll(getProhibitObjectsRequireNonNull()
.map((prohibit) -> prohibit ? noClassesShouldCallObjectsRequireNonNull() : Collections.emptyList()));
getRuleDescriptions().set(getRules().map((rules) -> rules.stream().map(ArchRule::getDescription).toList()));
}
@ -212,18 +216,16 @@ public abstract class ArchitectureCheck extends DefaultTask {
.because("java.net.URLDecoder.decode(String s, Charset charset) should be used instead");
}
private ArchRule noClassesShouldCallObjectsRequireNonNullWithMessage() {
return ArchRuleDefinition.noClasses()
.should()
.callMethod(Objects.class, "requireNonNull", Object.class, String.class)
.because("Use org.springframework.utils.Assert.notNull(Object, String) should be used instead");
}
private ArchRule noClassesShouldCallObjectsRequireNonNullWithSupplier() {
return ArchRuleDefinition.noClasses()
.should()
.callMethod(Objects.class, "requireNonNull", Object.class, Supplier.class)
.because("Use org.springframework.utils.Assert.notNull(Object, Supplier) should be used instead");
private List<ArchRule> noClassesShouldCallObjectsRequireNonNull() {
return List.of(
ArchRuleDefinition.noClasses()
.should()
.callMethod(Objects.class, "requireNonNull", Object.class, String.class)
.because("org.springframework.utils.Assert.notNull(Object, String) should be used instead"),
ArchRuleDefinition.noClasses()
.should()
.callMethod(Objects.class, "requireNonNull", Object.class, Supplier.class)
.because("org.springframework.utils.Assert.notNull(Object, Supplier) should be used instead"));
}
public void setClasses(FileCollection classes) {
@ -254,6 +256,9 @@ public abstract class ArchitectureCheck extends DefaultTask {
@Internal
public abstract ListProperty<ArchRule> getRules();
@Internal
public abstract Property<Boolean> getProhibitObjectsRequireNonNull();
@Input
// The rules themselves can't be an input as they aren't serializable so we use
// their

27
buildSrc/src/test/java/org/springframework/boot/build/architecture/ArchitectureCheckTests.java
View File

@ -1,5 +1,5 @@
/*
* Copyright 2012-2023 the original author or authors.
* Copyright 2012-2024 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.
@ -36,6 +36,7 @@ import static org.assertj.core.api.Assertions.assertThatExceptionOfType;
* Tests for {@link ArchitectureCheck}.
*
* @author Andy Wilkinson
* @author Ivan Malutin
*/
class ArchitectureCheckTests {
@ -122,18 +123,18 @@ class ArchitectureCheckTests {
}
@Test
void whenClassCallsObjectsRequireNonNullWithMessageTaskFailsAndWritesReport() throws Exception {
prepareTask("objects/requireNonNullWithMessage", (architectureCheck) -> {
assertThatExceptionOfType(GradleException.class).isThrownBy(architectureCheck::checkArchitecture);
assertThat(failureReport(architectureCheck)).isNotEmpty();
void whenClassDoesNotCallObjectsRequireNonNullTaskSucceedsAndWritesAnEmptyReport() throws Exception {
prepareTask("objects/noRequireNonNull", (architectureCheck) -> {
architectureCheck.checkArchitecture();
assertThat(failureReport(architectureCheck)).isEmpty();
});
}
@Test
void whenClassDoesNotCallObjectsRequireNonNullWithMessageTaskSucceedsAndWritesAnEmptyReport() throws Exception {
prepareTask("objects/noRequireNonNullWithMessage", (architectureCheck) -> {
architectureCheck.checkArchitecture();
assertThat(failureReport(architectureCheck)).isEmpty();
void whenClassCallsObjectsRequireNonNullWithMessageTaskFailsAndWritesReport() throws Exception {
prepareTask("objects/requireNonNullWithString", (architectureCheck) -> {
assertThatExceptionOfType(GradleException.class).isThrownBy(architectureCheck::checkArchitecture);
assertThat(failureReport(architectureCheck)).isNotEmpty();
});
}
@ -145,14 +146,6 @@ class ArchitectureCheckTests {
});
}
@Test
void whenClassDoesNotCallObjectsRequireNonNullWithSupplierTaskSucceedsAndWritesAnEmptyReport() throws Exception {
prepareTask("objects/noRequireNonNullWithSupplier", (architectureCheck) -> {
architectureCheck.checkArchitecture();
assertThat(failureReport(architectureCheck)).isEmpty();
});
}
private void prepareTask(String classes, Callback<ArchitectureCheck> callback) throws Exception {
File projectDir = new File(this.temp, "project");
projectDir.mkdirs();

21
buildSrc/src/test/java/org/springframework/boot/build/architecture/objects/noRequireNonNullWithMessage/NoRequireNonNullWithMessageUsage.java → buildSrc/src/test/java/org/springframework/boot/build/architecture/objects/noRequireNonNull/NoRequireNonNull.java
View File

@ -14,24 +14,19 @@
* limitations under the License.
*/
package org.springframework.boot.build.architecture.objects.noRequireNonNullWithMessage;
package org.springframework.boot.build.architecture.objects.noRequireNonNull;
import java.util.Collections;
import org.springframework.util.Assert;
/**
* This class uses `Assert.notNull(Object, String)` instead of
* `Objects.requireNonNull(Object, String)`, and should pass the architecture check.
*
* @author Ivan Malutin
*/
public class NoRequireNonNullWithMessageUsage {
class NoRequireNonNull {
/**
* Example method that uses `Assert.notNull(Object, String)`, which should not be
* flagged by the architecture check.
*/
public void exampleMethod() {
void exampleMethod() {
Assert.notNull(new Object(), "Object must not be null");
// Compilation of a method reference generates code that uses
// Objects.requireNonNull(Object). Check that it doesn't cause a failure.
Collections.emptyList().forEach(System.out::println);
}
}

37
buildSrc/src/test/java/org/springframework/boot/build/architecture/objects/noRequireNonNullWithSupplier/NoRequireNonNullWithSupplierUsage.java
View File

@ -1,37 +0,0 @@
/*
* Copyright 2012-2024 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.
* You may obtain a copy of the License at
*
* https://www.apache.org/licenses/LICENSE-2.0
*
* Unless required by applicable law or agreed to in writing, software
* distributed under the License is distributed on an "AS IS" BASIS,
* WITHOUT WARRANTIES OR CONDITIONS OF ANY KIND, either express or implied.
* See the License for the specific language governing permissions and
* limitations under the License.
*/
package org.springframework.boot.build.architecture.objects.noRequireNonNullWithSupplier;
import org.springframework.util.Assert;
/**
* This class uses `Assert.notNull(Object, Supplier)` instead of
* `Objects.requireNonNull(Object, Supplier)`, and should pass the architecture check.
*
* @author Ivan Malutin
*/
public class NoRequireNonNullWithSupplierUsage {
/**
* Example method that uses `Assert.notNull(Object, Supplier)`, which should not be
* flagged by the architecture check.
*/
public void exampleMethod() {
Assert.notNull(new Object(), () -> "Object must not be null");
}
}

16
buildSrc/src/test/java/org/springframework/boot/build/architecture/objects/requireNonNullWithMessage/RequireNonNullWithMessageUsage.java → buildSrc/src/test/java/org/springframework/boot/build/architecture/objects/requireNonNullWithString/RequireNonNullWithString.java
View File

@ -14,23 +14,13 @@
* limitations under the License.
*/
package org.springframework.boot.build.architecture.objects.requireNonNullWithMessage;
package org.springframework.boot.build.architecture.objects.requireNonNullWithString;
import java.util.Objects;
/**
* This class is intentionally designed to test the use of `Objects.requireNonNull(Object,
* String)`, which should trigger a failure in the architecture check.
*
* @author Ivan Malutin
*/
public class RequireNonNullWithMessageUsage {
class RequireNonNullWithString {
/**
* Example method that uses `Objects.requireNonNull(Object, String)`, which should be
* flagged by the architecture check.
*/
public void exampleMethod() {
void exampleMethod() {
Objects.requireNonNull(new Object(), "Object cannot be null");
}

14
buildSrc/src/test/java/org/springframework/boot/build/architecture/objects/requireNonNullWithSupplier/RequireNonNullWithSupplierUsage.java → buildSrc/src/test/java/org/springframework/boot/build/architecture/objects/requireNonNullWithSupplier/RequireNonNullWithSupplier.java
View File

@ -18,19 +18,9 @@ package org.springframework.boot.build.architecture.objects.requireNonNullWithSu
import java.util.Objects;
/**
* This class is intentionally designed to test the use of `Objects.requireNonNull(Object,
* Supplier)`, which should trigger a failure in the architecture check.
*
* @author Ivan Malutin
*/
public class RequireNonNullWithSupplierUsage {
class RequireNonNullWithSupplier {
/**
* Example method that uses `Objects.requireNonNull(Object, Supplier)`, which should
* be flagged by the architecture check.
*/
public void exampleMethod() {
void exampleMethod() {
Objects.requireNonNull(new Object(), () -> "Object cannot be null");
}

4
spring-boot-project/spring-boot-tools/spring-boot-loader-classic/build.gradle
View File

@ -21,3 +21,7 @@ dependencies {
testRuntimeOnly("org.bouncycastle:bcprov-jdk18on:1.78.1")
testRuntimeOnly("org.springframework:spring-webmvc")
}
tasks.named("checkArchitectureMain").configure {
prohibitObjectsRequireNonNull = false
}

4
spring-boot-project/spring-boot-tools/spring-boot-loader/build.gradle
View File

@ -21,3 +21,7 @@ dependencies {
testRuntimeOnly("org.bouncycastle:bcprov-jdk18on:1.78.1")
testRuntimeOnly("org.springframework:spring-webmvc")
}
tasks.named("checkArchitectureMain").configure {
prohibitObjectsRequireNonNull = false
}

5
spring-boot-project/spring-boot/src/test/java/org/springframework/boot/context/properties/bind/ValueObjectBinderTests.java
View File

@ -24,7 +24,6 @@ import java.util.ArrayList;
import java.util.EnumMap;
import java.util.List;
import java.util.Map;
import java.util.Objects;
import java.util.Optional;
import com.jayway.jsonpath.JsonPath;
@ -616,8 +615,8 @@ class ValueObjectBinderTests {
private final Object value;
ExampleFailingConstructorBean(String name, String value) {
Objects.requireNonNull(name, "'name' must be not null.");
Objects.requireNonNull(value, "'value' must be not null.");
Assert.notNull(name, "'name' must be not null.");
Assert.notNull(value, "'value' must be not null.");
this.name = name;
this.value = value;
}