Skip to content

Commit

Permalink
Treat warnings as errors (#5)
Browse files Browse the repository at this point in the history
* adr: Treat warnings as errors

* build: Treat warnings as errors

* fix: Compilation warnings
  • Loading branch information
guillermocalvo authored Feb 28, 2024
1 parent 4a5845f commit 2173e3b
Show file tree
Hide file tree
Showing 4 changed files with 51 additions and 9 deletions.
Original file line number Diff line number Diff line change
Expand Up @@ -35,4 +35,10 @@ tasks.check {
java {
sourceCompatibility = JavaVersion.toVersion("21")
targetCompatibility = JavaVersion.toVersion("21")
}
}

tasks.withType<JavaCompile> {
val compilerArgs = options.compilerArgs
compilerArgs.add("-Werror")
compilerArgs.add("-Xlint:all,-serial,-processing")
}
36 changes: 36 additions & 0 deletions doc/adr/0018-treat-warnings-as-errors.md
Original file line number Diff line number Diff line change
@@ -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.

This comment has been minimized.

Copy link
@jeffscottbrown

jeffscottbrown Mar 12, 2024

Contributor

With Corretto-21.0.2.13.1 (build 21.0.2+13-LTS) I am currently seeing the following warning(s):

Deprecated Gradle features were used in this build, making it incompatible with Gradle 9.0.

You can use '--warning-mode all' to show the individual deprecation warnings and determine if they come from your own scripts or plugins.

For more on this, please refer to https://docs.gradle.org/8.5/userguide/command_line_interface.html#sec:command_line_warnings in the Gradle documentation.

This comment has been minimized.

Copy link
@guillermocalvo

guillermocalvo Mar 13, 2024

Author Contributor

@osscontributor Thank you!



## 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.
Original file line number Diff line number Diff line change
Expand Up @@ -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<QuestionNotFoundException, HttpResponse> {
public class QuestionNotFoundExceptionHandler implements ExceptionHandler<QuestionNotFoundException, HttpResponse<?>> {

private final ErrorResponseProcessor<?> errorResponseProcessor;
private final ViewsRenderer viewsRenderer;
private final ViewsRenderer<Map<?, ?>, HttpRequest<?>> viewsRenderer;

public QuestionNotFoundExceptionHandler(ErrorResponseProcessor<?> errorResponseProcessor,
ViewsRenderer viewsRenderer) {
ViewsRenderer<Map<?, ?>, 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)
Expand Down
Original file line number Diff line number Diff line change
Expand Up @@ -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))
Expand All @@ -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))
Expand All @@ -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)
Expand Down

0 comments on commit 2173e3b

Please sign in to comment.