Development: Enforce consistent usage of nullness annotations (#6872)

src/main/java/de/tum/in/www1/artemis/repository/RepositoryImpl.java

@@ -3,6 +3,7 @@
 import java.io.Serializable;
 import java.util.Collections;
 
+import javax.annotation.Nullable;
 import javax.persistence.EntityManager;
 import javax.persistence.TypedQuery;
 import javax.persistence.criteria.*;
@@ -11,7 +12,6 @@
 import org.springframework.data.jpa.repository.support.JpaEntityInformation;
 import org.springframework.data.jpa.repository.support.JpaEntityInformationSupport;
 import org.springframework.data.jpa.repository.support.SimpleJpaRepository;
-import org.springframework.lang.Nullable;
 import org.springframework.util.Assert;
 
 public class RepositoryImpl<T, ID extends Serializable> extends SimpleJpaRepository<T, ID> {

src/main/java/de/tum/in/www1/artemis/service/connectors/localvc/LocalVCFetchFilter.java

@@ -6,8 +6,8 @@
 import javax.servlet.ServletException;
 import javax.servlet.http.HttpServletRequest;
 import javax.servlet.http.HttpServletResponse;
+import javax.validation.constraints.NotNull;
 
-import org.jetbrains.annotations.NotNull;
 import org.slf4j.Logger;
 import org.slf4j.LoggerFactory;
 import org.springframework.web.filter.OncePerRequestFilter;

src/main/java/de/tum/in/www1/artemis/service/connectors/localvc/LocalVCPushFilter.java

@@ -6,8 +6,8 @@
 import javax.servlet.ServletException;
 import javax.servlet.http.HttpServletRequest;
 import javax.servlet.http.HttpServletResponse;
+import javax.validation.constraints.NotNull;
 
-import org.jetbrains.annotations.NotNull;
 import org.slf4j.Logger;
 import org.slf4j.LoggerFactory;
 import org.springframework.web.filter.OncePerRequestFilter;

src/main/java/de/tum/in/www1/artemis/service/notifications/GeneralInstantNotificationService.java

@@ -5,7 +5,8 @@
 
 import java.util.List;
 
-import org.jetbrains.annotations.NotNull;
+import javax.validation.constraints.NotNull;
+
 import org.springframework.scheduling.annotation.Async;
 import org.springframework.stereotype.Service;
 

src/test/java/de/tum/in/www1/artemis/ArchitectureTest.java

@@ -1,17 +1,21 @@
 package de.tum.in.www1.artemis;
 
-import static com.tngtech.archunit.base.DescribedPredicate.not;
+import static com.tngtech.archunit.base.DescribedPredicate.*;
 import static com.tngtech.archunit.core.domain.JavaClass.Predicates.*;
-import static com.tngtech.archunit.lang.conditions.ArchPredicates.are;
-import static com.tngtech.archunit.lang.conditions.ArchPredicates.have;
+import static com.tngtech.archunit.lang.SimpleConditionEvent.violated;
+import static com.tngtech.archunit.lang.conditions.ArchPredicates.*;
 import static com.tngtech.archunit.lang.syntax.ArchRuleDefinition.*;
 
+import java.util.*;
 import java.util.stream.Collectors;
 
 import org.junit.jupiter.api.*;
 import org.junit.jupiter.params.ParameterizedTest;
 
-import com.tngtech.archunit.lang.ArchRule;
+import com.tngtech.archunit.base.DescribedPredicate;
+import com.tngtech.archunit.core.domain.JavaAnnotation;
+import com.tngtech.archunit.core.domain.JavaMethod;
+import com.tngtech.archunit.lang.*;
 
 class ArchitectureTest extends AbstractArchitectureTest {
 
@@ -52,4 +56,44 @@ void testNoCollectorsToList() {
                 .because("You should use .toList() or .collect(Collectors.toCollection(ArrayList::new)) instead");
         toListUsage.check(allClasses);
     }
+
+    @Test
+    void testNullnessAnnotations() {
+        var notNullPredicate = and(not(resideInPackageAnnotation("javax.validation.constraints")), simpleNameAnnotation("NotNull"));
+        var nonNullPredicate = simpleNameAnnotation("NonNull");
+        var nullablePredicate = and(not(resideInPackageAnnotation("javax.annotation")), simpleNameAnnotation("Nullable"));
+
+        Set<DescribedPredicate<? super JavaAnnotation<?>>> allPredicates = Set.of(notNullPredicate, nonNullPredicate, nullablePredicate);
+
+        for (var predicate : allPredicates) {
+            ArchRule units = noCodeUnits().should().beAnnotatedWith(predicate);
+            ArchRule parameters = methods().should(notHaveAnyParameterAnnotatedWith(predicate));
+
+            units.check(allClasses);
+            parameters.check(allClasses);
+        }
+    }
+
+    // Custom Predicates for JavaAnnotations since ArchUnit only defines them for classes
+
+    private DescribedPredicate<? super JavaAnnotation<?>> simpleNameAnnotation(String name) {
+        return equalTo(name).as("Annotation with simple name " + name).onResultOf(annotation -> annotation.getRawType().getSimpleName());
+    }
+
+    private DescribedPredicate<? super JavaAnnotation<?>> resideInPackageAnnotation(String packageName) {
+        return equalTo(packageName).as("Annotation in package " + packageName).onResultOf(annotation -> annotation.getRawType().getPackageName());
+    }
+
+    private ArchCondition<JavaMethod> notHaveAnyParameterAnnotatedWith(DescribedPredicate<? super JavaAnnotation<?>> annotationPredicate) {
+        return new ArchCondition<>("have parameters annotated with ") {
+
+            @Override
+            public void check(JavaMethod item, ConditionEvents events) {
+                boolean satisfied = item.getParameterAnnotations().stream().flatMap(Collection::stream).noneMatch(annotationPredicate);
+                if (!satisfied) {
+                    events.add(violated(item, String.format("Method %s has parameter violating %s", item.getFullName(), annotationPredicate.getDescription())));
+                }
+            }
+        };
+    }
 }

src/test/java/de/tum/in/www1/artemis/metis/AbstractConversationTest.java

@@ -7,7 +7,6 @@
 import java.util.Arrays;
 import java.util.Set;
 
-import org.jetbrains.annotations.NotNull;
 import org.junit.jupiter.api.BeforeEach;
 import org.springframework.beans.factory.annotation.Autowired;
 import org.springframework.data.domain.Pageable;
@@ -16,7 +15,6 @@
 
 import de.tum.in.www1.artemis.AbstractSpringIntegrationBambooBitbucketJiraTest;
 import de.tum.in.www1.artemis.course.CourseUtilService;
-import de.tum.in.www1.artemis.domain.Course;
 import de.tum.in.www1.artemis.domain.User;
 import de.tum.in.www1.artemis.domain.enumeration.CourseInformationSharingConfiguration;
 import de.tum.in.www1.artemis.domain.enumeration.DisplayPriority;
@@ -169,13 +167,11 @@ void removeUsersFromConversation(Long conversationId, String... userLoginsWithou
         }
     }
 
-    @NotNull
-    Course setCourseInformationSharingConfiguration(CourseInformationSharingConfiguration courseInformationSharingConfiguration) {
+    void setCourseInformationSharingConfiguration(CourseInformationSharingConfiguration courseInformationSharingConfiguration) {
         var persistedCourse = courseRepository.findByIdElseThrow(exampleCourseId);
         persistedCourse.setCourseInformationSharingConfiguration(courseInformationSharingConfiguration);
         persistedCourse = courseRepository.saveAndFlush(persistedCourse);
         assertThat(persistedCourse.getCourseInformationSharingConfiguration()).isEqualTo(courseInformationSharingConfiguration);
-        return persistedCourse;
     }
 
     ChannelDTO createChannel(boolean isPublicChannel, String name) throws Exception {

src/test/java/de/tum/in/www1/artemis/metis/PostIntegrationTest.java

@@ -11,8 +11,8 @@
 import javax.validation.Validation;
 import javax.validation.Validator;
 import javax.validation.ValidatorFactory;
+import javax.validation.constraints.NotNull;
 
-import org.jetbrains.annotations.NotNull;
 import org.junit.jupiter.api.AfterEach;
 import org.junit.jupiter.api.BeforeEach;
 import org.junit.jupiter.api.Test;