build(server): enforce JSpecify nullness with NullAway

Author: FelixTJDietrich

.changeset/bright-ravens-check.md

@@ -0,0 +1,5 @@
+---
+"hephaestus": patch
+---
+
+Pull request responses no longer require a URL when the upstream provider does not supply one.

docs/contributor/coding-guidelines.mdx

@@ -67,6 +67,14 @@ These guidelines keep every service aligned. Keep changes focused. Mention any d
 - Expose dedicated DTO mappers in the package instead of sprinkling conversion logic across layers.
 - Treat `Optional` as a signal for missing data. Avoid returning `null` from repository methods.
 
+### Null safety
+
+- Add a JSpecify `@NullMarked` `package-info.java` to every handwritten Java package.
+- Bare types are non-null. Use `@Nullable` only when absence is part of the contract. On API DTOs, use explicit `@NonNull` for components that must be `required` in OpenAPI.
+- Type-use annotations describe exactly what is nullable: `List<@Nullable String>` permits null elements; `String @Nullable []` permits a null array reference.
+- Fix NullAway errors at the contract or implementation boundary. Suppressions and classes without an explicit null-marking scope fail the quality gate.
+- Run `pnpm run check:java-nullness` for the policy check and `cd server && ./mvnw test-compile -P'!quick' -DskipTests` for compiler analysis.
+
 ### Safe workflows
 
 - Configuration overrides stay in `application-*.yml`. Do not commit `application-local.yml`.

package.json

@@ -48,7 +48,7 @@
 		"format": "pnpm run format:server && pnpm run format:client && pnpm run format:agents",
 		"format:check": "pnpm run format:server:check && pnpm run format:client:check && pnpm run format:agents:check",
 		"lint": "run-s lint:server lint:client lint:agents",
-		"check": "run-s check:biome-pin check:server check:client check:agents test:agents check:stories check:story-sort check:components check:diagrams check:env check:contracts check:instructions docs:lint",
+		"check": "run-s check:biome-pin check:java-nullness check:server check:client check:agents test:agents check:stories check:story-sort check:components check:diagrams check:env check:contracts check:instructions docs:lint",
 		"typecheck": "pnpm run typecheck:webapp && pnpm run typecheck:scripts && pnpm run typecheck:agents",
 		"db:draft-changelog": "scripts/db-utils.sh draft-changelog",
 		"db:generate-erd-docs": "scripts/db-utils.sh generate-erd",
@@ -75,6 +75,7 @@
 		"check:diagrams": "node scripts/check-mermaid-diagrams.ts",
 		"check:env": "node scripts/check-env-defaults.ts && node scripts/check-env-roles.ts && node --test scripts/check-env-roles.test.ts",
 		"check:biome-pin": "node scripts/check-biome-pin.ts",
+		"check:java-nullness": "node scripts/check-java-nullness.ts && node --test scripts/check-java-nullness.test.ts",
 		"check:instructions": "node scripts/check-agent-instructions.ts && node --test scripts/check-agent-instructions.test.ts"
 	},
 	"devDependencies": {

scripts/check-java-nullness.test.ts

@@ -0,0 +1,44 @@
+import assert from "node:assert/strict";
+import test from "node:test";
+import { nullnessPolicyViolations } from "./check-java-nullness.ts";
+
+const source = (content: string) => [{ path: "Example.java", content }];
+
+await test("accepts unrelated suppressions", () => {
+	assert.deepEqual(
+		nullnessPolicyViolations(source('@SuppressWarnings({ "unchecked", "deprecation" })')),
+		[],
+	);
+});
+
+await test("rejects direct and namespaced NullAway suppressions", () => {
+	for (const warning of ["NullAway", "NullAway.Init", "NullAway.Optional"]) {
+		assert.deepEqual(nullnessPolicyViolations(source(`@SuppressWarnings("${warning}")`)), [
+			"Example.java",
+		]);
+	}
+	assert.deepEqual(nullnessPolicyViolations(source('@java.lang.SuppressWarnings("NullAway")')), [
+		"Example.java",
+	]);
+});
+
+await test("rejects suppression arrays, concatenation, and Unicode escapes", () => {
+	const examples = [
+		'@SuppressWarnings({ "unchecked", "NullAway" })',
+		'@SuppressWarnings("Null" + "Away")',
+		'@SuppressWarnings("Null\\u0041way")',
+	];
+	for (const example of examples) {
+		assert.deepEqual(nullnessPolicyViolations(source(example)), ["Example.java"]);
+	}
+});
+
+await test("rejects suppression names hidden behind constants", () => {
+	assert.deepEqual(nullnessPolicyViolations(source("@SuppressWarnings(NULL_AWAY)")), [
+		"Example.java",
+	]);
+});
+
+await test("ignores the warning name outside SuppressWarnings", () => {
+	assert.deepEqual(nullnessPolicyViolations(source('String checker = "NullAway";')), []);
+});

scripts/check-java-nullness.ts

@@ -0,0 +1,95 @@
+#!/usr/bin/env node
+import { execFile } from "node:child_process";
+import { readFile } from "node:fs/promises";
+import { resolve } from "node:path";
+import { promisify } from "node:util";
+
+const execFileAsync = promisify(execFile);
+const REPO_ROOT = resolve(import.meta.dirname, "..");
+const JAVA_SOURCE = /^server\/src\/(?:main|test)\/java\/.*\.java$/;
+
+export interface JavaSource {
+	readonly path: string;
+	readonly content: string;
+}
+
+function unicodeEscapes(source: string): string {
+	return source.replace(/\\u+([0-9a-fA-F]{4})/g, (_, hex: string) =>
+		String.fromCharCode(Number.parseInt(hex, 16)),
+	);
+}
+
+function suppressionBodies(source: string): readonly string[] {
+	const text = unicodeEscapes(source);
+	const bodies: string[] = [];
+	const annotation = /@(?:java\.lang\.)?SuppressWarnings\s*\(/g;
+	for (let match = annotation.exec(text); match !== null; match = annotation.exec(text)) {
+		const start = annotation.lastIndex;
+		let depth = 1;
+		let quoted = false;
+		let escaped = false;
+		for (let index = start; index < text.length; index++) {
+			const character = text[index];
+			if (quoted) {
+				if (escaped) escaped = false;
+				else if (character === "\\") escaped = true;
+				else if (character === '"') quoted = false;
+				continue;
+			}
+			if (character === '"') quoted = true;
+			else if (character === "(") depth++;
+			else if (character === ")" && --depth === 0) {
+				bodies.push(text.slice(start, index));
+				annotation.lastIndex = index + 1;
+				break;
+			}
+		}
+	}
+	return bodies;
+}
+
+function stringValues(body: string): string {
+	return [...body.matchAll(/"((?:\\.|[^"\\])*)"/g)]
+		.map((match) => match[1]?.replaceAll(/\\(["\\])/g, "$1") ?? "")
+		.join("");
+}
+
+function violatesPolicy(body: string): boolean {
+	if (stringValues(body).includes("NullAway")) return true;
+	const withoutStrings = body.replaceAll(/"(?:\\.|[^"\\])*"/g, "");
+	return !/^[\s{},+]*$/.test(withoutStrings);
+}
+
+export function nullnessPolicyViolations(sources: readonly JavaSource[]): readonly string[] {
+	return sources
+		.filter(({ content }) => suppressionBodies(content).some(violatesPolicy))
+		.map(({ path }) => path);
+}
+
+async function main(): Promise<void> {
+	const { stdout } = await execFileAsync(
+		"git",
+		["ls-files", "--cached", "--others", "--exclude-standard"],
+		{
+			cwd: REPO_ROOT,
+		},
+	);
+	const paths = stdout.split("\n").filter((path) => JAVA_SOURCE.test(path));
+	const sources = await Promise.all(
+		paths.map(async (path) => ({
+			path,
+			content: await readFile(resolve(REPO_ROOT, path), "utf8"),
+		})),
+	);
+	const suppressed = nullnessPolicyViolations(sources);
+	if (suppressed.length > 0) {
+		throw new Error(
+			`NullAway suppressions and indirect suppression names are forbidden; fix the contract or implementation:\n${suppressed.map((path) => `  ${path}`).join("\n")}`,
+		);
+	}
+	console.log(
+		`Java nullness policy: ${sources.length} handwritten source file(s), no NullAway suppressions.`,
+	);
+}
+
+if (import.meta.main) await main();

server/.mvn/jvm.config

@@ -1,3 +1,13 @@
 --enable-native-access=ALL-UNNAMED
 -XX:+EnableDynamicAgentLoading
 -Xmx4g
+--add-exports=jdk.compiler/com.sun.tools.javac.api=ALL-UNNAMED
+--add-exports=jdk.compiler/com.sun.tools.javac.file=ALL-UNNAMED
+--add-exports=jdk.compiler/com.sun.tools.javac.main=ALL-UNNAMED
+--add-exports=jdk.compiler/com.sun.tools.javac.model=ALL-UNNAMED
+--add-exports=jdk.compiler/com.sun.tools.javac.parser=ALL-UNNAMED
+--add-exports=jdk.compiler/com.sun.tools.javac.processing=ALL-UNNAMED
+--add-exports=jdk.compiler/com.sun.tools.javac.tree=ALL-UNNAMED
+--add-exports=jdk.compiler/com.sun.tools.javac.util=ALL-UNNAMED
+--add-opens=jdk.compiler/com.sun.tools.javac.code=ALL-UNNAMED
+--add-opens=jdk.compiler/com.sun.tools.javac.comp=ALL-UNNAMED

server/AGENTS.md

@@ -74,6 +74,17 @@ authorization logic.
 never log a token) · `@Transactional` on a controller (service layer only) · expose an entity from a
 controller.
 
+## Null-safety
+
+NullAway checks all handwritten production and test code in JSpecify mode; generated sources are
+excluded. Every new package needs a `package-info.java` containing
+`@org.jspecify.annotations.NullMarked`, and the build rejects missing null-marking scopes and
+`NullAway` suppressions. Use `@Nullable` only for genuine absence and place it on the precise type:
+`List<@Nullable String>` permits null elements; `String @Nullable []` permits a null array reference.
+Fix violations at the contract or implementation boundary. In tests, refine a nullable result once
+before using it rather than adding duplicate assertions. Run `./mvnw test -P'!quick'` after changing a
+nullness contract.
+
 ## Test tiers
 
 | Tag | Runs | Command |
@@ -141,9 +152,9 @@ or on "the only" result, and never write cleanup that another test depends on ha
   `OpenAPIConfiguration.ALLOWED_DOMAIN_OBJECTS`. The webapp client then simply has no type for it, with
   no error anywhere. Domain types the API deliberately exposes (`ProblemDetail`, `PracticeBinding`, …)
   are there for this reason.
-- DTOs are records, and `@NonNull` (`org.jspecify.annotations`) on a component is what puts it in the
-  generated schema's `required` list. A component the API may omit is left off that list — annotate it
-  `@Nullable` or leave it bare; neither changes the spec.
+- DTOs are records. All bare components are non-null under `@NullMarked`; add JSpecify `@NonNull` when
+  that component must also appear in the generated schema's `required` list. A component the API may
+  omit is `@Nullable`, never bare.
 - **Never wrap a DTO component in `Optional<>`.** springdoc unwraps it to the value type but still marks
   it required, so the generated TypeScript declares it non-optional and its response transformer
   converts it unconditionally — a value the server never sends is typed as one it always sends. Use

server/openapi.yaml

@@ -11905,7 +11905,6 @@ components:
           type: string
           description: Title of the pull request
       required:
-      - htmlUrl
       - id
       - isDraft
       - isMerged
@@ -11995,7 +11994,6 @@ components:
       - additions
       - commentsCount
       - deletions
-      - htmlUrl
       - id
       - isDraft
       - isMerged

server/pom.xml

@@ -111,15 +111,13 @@
         <openapi-generator.version>7.19.0</openapi-generator.version>
         <postgresql.version>42.7.11</postgresql.version>
         <therapi.version>0.15.0</therapi.version>
+        <error-prone.version>2.50.0</error-prone.version>
+        <nullaway.version>0.14.0</nullaway.version>
         <shedlock.version>5.16.0</shedlock.version>
         <bucket4j.version>8.19.0</bucket4j.version>
     </properties>
     <dependencies>
-        <!-- JSpecify nullness annotations (@NonNull/@Nullable). The forward-looking standard
-             adopted by Spring Framework 7 / Boot 4, replacing the deprecated
-             org.springframework.lang.* annotations. Version managed by the Spring Boot BOM.
-             A custom ModelConverter (JSpecifyRequiredModelConverter) teaches SpringDoc to read
-             these for OpenAPI `required`, since swagger-core only recognises the Spring ones. -->
+        <!-- JSpecify annotations also drive the OpenAPI required-field converter. -->
         <dependency>
             <groupId>org.jspecify</groupId>
             <artifactId>jspecify</artifactId>
@@ -498,6 +496,11 @@
                     <!-- Suppress warnings for generated code using deprecated GitHub API values -->
                     <compilerArgs>
                         <arg>-Xlint:all,-deprecation</arg>
+                        <arg>-XDcompilePolicy=simple</arg>
+                        <arg>--should-stop=ifError=FLOW</arg>
+                        <arg>-XDaddTypeAnnotationsToSymbol=true</arg>
+                        <!-- javac requires the Error Prone plugin and its options in one compiler argument. -->
+                        <arg>-Xplugin:ErrorProne -XepDisableAllChecks -Xep:NullAway:ERROR -Xep:RequireExplicitNullMarking:ERROR -XepExcludedPaths:.*[\\/]target[\\/]generated-sources[\\/].* -XepOpt:NullAway:AnnotatedPackages=de.tum.cit.aet.hephaestus -XepOpt:NullAway:JSpecifyMode=true -XepOpt:NullAway:TreatGeneratedAsUnannotated=true -XepOpt:NullAway:HandleTestAssertionLibraries=true</arg>
                     </compilerArgs>
                     <showWarnings>false</showWarnings>
                     <annotationProcessorPaths>
@@ -514,6 +517,16 @@
                             <artifactId>therapi-runtime-javadoc-scribe</artifactId>
                             <version>${therapi.version}</version>
                         </path>
+                        <path>
+                            <groupId>com.google.errorprone</groupId>
+                            <artifactId>error_prone_core</artifactId>
+                            <version>${error-prone.version}</version>
+                        </path>
+                        <path>
+                            <groupId>com.uber.nullaway</groupId>
+                            <artifactId>nullaway</artifactId>
+                            <version>${nullaway.version}</version>
+                        </path>
                     </annotationProcessorPaths>
                 </configuration>
             </plugin>

server/src/main/java/de/tum/cit/aet/hephaestus/account/AccountController.java

@@ -6,6 +6,7 @@
 import io.swagger.v3.oas.annotations.tags.Tag;
 import jakarta.validation.Valid;
 import java.util.Optional;
+import org.jspecify.annotations.Nullable;
 import org.slf4j.Logger;
 import org.slf4j.LoggerFactory;
 import org.springframework.http.HttpStatus;
@@ -76,7 +77,7 @@ public ResponseEntity<UserSettingsDTO> updateUserSettings(
         return ResponseEntity.ok(preferencesService.updateUserSettings(user.get(), userSettings, subjectId));
     }
 
-    private JwtAuthenticationToken resolveAuthentication(JwtAuthenticationToken injectedToken) {
+    private @Nullable JwtAuthenticationToken resolveAuthentication(@Nullable JwtAuthenticationToken injectedToken) {
         if (injectedToken != null) {
             return injectedToken;
         }