From 2173e3bed6d9650df7c19d4198c58b195f58753c Mon Sep 17 00:00:00 2001 From: Guillermo Calvo Date: Wed, 28 Feb 2024 12:48:28 -0800 Subject: [PATCH] Treat warnings as errors (#5) * adr: Treat warnings as errors * build: Treat warnings as errors * fix: Compilation warnings --- ...heckins.java-common-conventions.gradle.kts | 8 ++++- doc/adr/0018-treat-warnings-as-errors.md | 36 +++++++++++++++++++ .../QuestionNotFoundExceptionHandler.java | 10 +++--- .../DelegatingAuthenticationProviderTest.java | 6 ++-- 4 files changed, 51 insertions(+), 9 deletions(-) create mode 100644 doc/adr/0018-treat-warnings-as-errors.md diff --git a/buildSrc/src/main/kotlin/org.projectcheckins.java-common-conventions.gradle.kts b/buildSrc/src/main/kotlin/org.projectcheckins.java-common-conventions.gradle.kts index 840dfcb4..601d8dc6 100644 --- a/buildSrc/src/main/kotlin/org.projectcheckins.java-common-conventions.gradle.kts +++ b/buildSrc/src/main/kotlin/org.projectcheckins.java-common-conventions.gradle.kts @@ -35,4 +35,10 @@ tasks.check { java { sourceCompatibility = JavaVersion.toVersion("21") targetCompatibility = JavaVersion.toVersion("21") -} \ No newline at end of file +} + +tasks.withType { + val compilerArgs = options.compilerArgs + compilerArgs.add("-Werror") + compilerArgs.add("-Xlint:all,-serial,-processing") +} diff --git a/doc/adr/0018-treat-warnings-as-errors.md b/doc/adr/0018-treat-warnings-as-errors.md new file mode 100644 index 00000000..6b2ab485 --- /dev/null +++ b/doc/adr/0018-treat-warnings-as-errors.md @@ -0,0 +1,36 @@ +# 18. Treat Warnings as Errors + +Date: 2024-02-28 +Creator: Guillermo Calvo +Reviewer: Sergio del Amo + +## Status + +Accepted + + +## Context + +We want to avoid as many bugs as possible. + +Some compilation warnings rarely cause any problems, but some others can lead to program malfunction. + +There are codebases in which a regular compilation generates a lot of "known warnings" that have to be ignored. +In those scenarios, new warnings tend to be completely ignored. + +We'd like to make sure that new compilation warnings are looked into. + +While a sensible correction of the offending code is the most desirable outcome of the research, +it is possible that the specific warning has to be suppressed via Java annotations or a similar mechanism. +In any case, solving or suppressing each warning must be the result of a thoughtful decision. + + +## Decision + +We will treat all compilation warnings as errors. + + +## Consequences + +- The build will fail if changes introduce any compilation warnings. +- Adopting this philosophy early in the project will make it easier to maintain overall quality throughout development. diff --git a/http/src/main/java/org/projectcheckins/http/exceptionshandlers/QuestionNotFoundExceptionHandler.java b/http/src/main/java/org/projectcheckins/http/exceptionshandlers/QuestionNotFoundExceptionHandler.java index c2c3075c..b8c23a8f 100644 --- a/http/src/main/java/org/projectcheckins/http/exceptionshandlers/QuestionNotFoundExceptionHandler.java +++ b/http/src/main/java/org/projectcheckins/http/exceptionshandlers/QuestionNotFoundExceptionHandler.java @@ -9,28 +9,28 @@ import io.micronaut.http.server.exceptions.ExceptionHandler; import io.micronaut.http.server.exceptions.response.ErrorContext; import io.micronaut.http.server.exceptions.response.ErrorResponseProcessor; -import io.micronaut.views.ModelAndView; import io.micronaut.views.ViewsRenderer; import jakarta.inject.Singleton; +import java.util.Map; import org.projectcheckins.core.exceptions.QuestionNotFoundException; import java.util.Collections; @Produces @Singleton -public class QuestionNotFoundExceptionHandler implements ExceptionHandler { +public class QuestionNotFoundExceptionHandler implements ExceptionHandler> { private final ErrorResponseProcessor errorResponseProcessor; - private final ViewsRenderer viewsRenderer; + private final ViewsRenderer, HttpRequest> viewsRenderer; public QuestionNotFoundExceptionHandler(ErrorResponseProcessor errorResponseProcessor, - ViewsRenderer viewsRenderer) { + ViewsRenderer, HttpRequest> viewsRenderer) { this.errorResponseProcessor = errorResponseProcessor; this.viewsRenderer = viewsRenderer; } @Override - public HttpResponse handle(HttpRequest request, QuestionNotFoundException e) { + public HttpResponse handle(@SuppressWarnings("rawtypes") HttpRequest request, QuestionNotFoundException e) { if (acceptHtml(request)) { Writable writable = viewsRenderer.render("error/404", Collections.emptyMap(), request); return HttpResponse.notFound(writable) diff --git a/security/src/test/java/org/projectcheckins/security/DelegatingAuthenticationProviderTest.java b/security/src/test/java/org/projectcheckins/security/DelegatingAuthenticationProviderTest.java index 19e5e691..a914b794 100644 --- a/security/src/test/java/org/projectcheckins/security/DelegatingAuthenticationProviderTest.java +++ b/security/src/test/java/org/projectcheckins/security/DelegatingAuthenticationProviderTest.java @@ -29,7 +29,7 @@ class DelegatingAuthenticationProviderTest { private static final String WRONG_PASSWORD = "wrongpassword"; @Test - void testUserNotFoundAuthentication(DelegatingAuthenticationProvider authenticationProvider) { + void testUserNotFoundAuthentication(DelegatingAuthenticationProvider authenticationProvider) { UsernamePasswordCredentials credentials = new UsernamePasswordCredentials(NOT_FOUND_EMAIL, CORRECT_PASSWORD); assertThat(authenticationProvider.authenticate(null, credentials)) .matches(not(AuthenticationResponse::isAuthenticated)) @@ -39,7 +39,7 @@ void testUserNotFoundAuthentication(DelegatingAuthenticationProvider authenticat } @Test - void testUserCredentialsDontMatch(DelegatingAuthenticationProvider authenticationProvider) { + void testUserCredentialsDontMatch(DelegatingAuthenticationProvider authenticationProvider) { UsernamePasswordCredentials credentials = new UsernamePasswordCredentials(FOUND_EMAIL, WRONG_PASSWORD); assertThat(authenticationProvider.authenticate(null, credentials)) .matches(not(AuthenticationResponse::isAuthenticated)) @@ -49,7 +49,7 @@ void testUserCredentialsDontMatch(DelegatingAuthenticationProvider authenticatio } @Test - void testUserFoundAuthentication(DelegatingAuthenticationProvider authenticationProvider) { + void testUserFoundAuthentication(DelegatingAuthenticationProvider authenticationProvider) { UsernamePasswordCredentials credentials = new UsernamePasswordCredentials(FOUND_EMAIL, CORRECT_PASSWORD); assertThat(authenticationProvider.authenticate(null, credentials)) .matches(AuthenticationResponse::isAuthenticated)