Overview

This commit adds the JavadocPackage Checkstyle module to the project's configuration file (checkstyle.xml). The module is configured to emit a warning message when a package-info.java file is missing, rather than throwing an exception. In addition, suppressions have been added to prevent the JavadocPackage checks from being applied to packages outside of the src/main directory. These suppressions ensure that the module only checks the packages that are relevant to the project and prevents false positives from being reported.

Deliverables

  • [x] Configure the JavadocPackage Checkstyle module to require package-info.java files for all packages under src/main.
  • [x] Determine if it is possible to require that package-info.java files include null-safety annotations – for example, via Checkstyle.
  • [x] If it's possible to require that package-info.java files include null-safety annotations, configure the necessary infrastructure.
  • [x] Introduce missing package-info.java files with package-level Javadoc and null-safety annotations.

Comment From: simonbasle

(didn't see the issue reference, which probably provides some context. my bad)

Comment From: simonbasle

@edyda99 I think the RegexpJavaSingleLine variant does the job (unless you've spotted occurrences of multilines nullability annotations?) The other one is also harder to read and has its own caveats with comments, so I'd go with the first one.

Comment From: edyda99

Hi @simonbasle, I will go with RegexpJavaSingleLine. No, I didn't spot any occurrences of multilines nullability, but it was about preventing it from happening. So should I remove the whitespace warning? ( I would suggest keeping it and increasing the severity to "error", so whenever someone writes @\n<annotation_name> an error will be thrown)

Comment From: simonbasle

( I would suggest keeping it and increasing the severity to "error", so whenever someone writes @\n<annotation_name> an error will be thrown)

Perhaps a simpler one line regexp like @\s*$ (at then any number of whitespace then the end of the line) would fit the bill? That said, I think this PR can focus on the actual null-safety annotations.

Comment From: edyda99

Done! In case all was good, I will start with the last point: 'Introduce missing package-info.java files with package-level Javadoc and null-safety annotations.' But I have two questions: - Do you suggest doing micro commits for each missing package-info.java? - What do I need to rely on before adding null-safety annotations on a newly added package-info?

Comment From: simonbasle

  • Do you suggest doing micro commits for each missing package-info.java?

Not necessarily, if that's the only thing in the commit.

  • What do I need to rely on before adding null-safety annotations on a newly added package-info?

It depends on how many package-info are missing. I would locally prepare the change and run some static analysis trying to find null-safety issues raising from that change. If the build doesn't break, I'm not sure I would make it a goal of this PR to fix the null-safety warnings though. Wdyt @sbrannen (also cc @sdeleuze)

Comment From: sbrannen

  • Do you suggest doing micro commits for each missing package-info.java?

Not necessarily, if that's the only thing in the commit.

I agree with @simonbasle.

Ideally the final result will be 3-4 commits.

  • enforce that package-info.java files exist and contain Javadoc
  • build should fail
  • introduce missing package-info.java files
  • build should pass
  • enforce that package-info.java files declare @NonNullApi and @NonNullFields
  • build should fail
  • introduce missing @NonNullApi and @NonNullFields declarations
  • build should pass

Of course, we may squash all commits in the PR into a single commit if that turns out to be easier.

  • What do I need to rely on before adding null-safety annotations on a newly added package-info?

I don't understand the question. Can you please expound?

It depends on how many package-info are missing. I would locally prepare the change and run some static analysis trying to find null-safety issues raising from that change. If the build doesn't break, I'm not sure I would make it a goal of this PR to fix the null-safety warnings though. Wdyt @sbrannen (also cc @sdeleuze)

Ensuring that proper null-safety semantics are enforced within code in newly annotated packages is beyond the scope of this PR/issue. I've created #30083 to address that.

Comment From: sbrannen

You also need to suppress violations in the framework-docs module, since that source code is only used in the reference manual.

And for the following packages in spring-core.

  • spring-core/src/main/java/org/springframework/asm
  • spring-core/src/main/java/org/springframework/cglib
  • spring-core/src/main/java/org/springframework/objenesis
  • spring-core/src/main/java/org/springframework/javapoet
  • spring-core/src/main/java/org/springframework/lang

Comment From: edyda99

Hi again,

First sorry for the continuous Force pushes, but I wanted to reduce my commits. I reduced them to 4 micro commits, with the last one containing the full commit message. All comments are resolved.

The merge request is ready for the final review

Thank you again!

Comment From: sbrannen

Thanks for making the latest changes, @edyda99!

Except for creation of the missing package-info.java files, the PR is effectively complete.

While reviewing the work locally on my machine, I have discovered a few issues with regular expressions, the wrong severity, additional packages that need to be excluded (suppressed), etc.

I have therefore converted this PR to a draft to signal that I will make the final changes locally after merging in your work.

Comment From: edyda99

@sbrannen Ok Great Could you suggest something else for me?

Comment From: sbrannen

This has been merged into main in 268e3fec9921653de17ab2f495039239f6b6152e and 7e32f504b021015ca8a7eaed7280453fad724b0f and revised and completed in 99e54fec3ac8dfedd2537fde5f19fbcc4f1f6202.

The latter turned out to be a bit more work than expected. 👀

Thanks again for the contribution, @edyda99!

Comment From: sbrannen

Could you suggest something else for me?

We have an ideal-for-contribution label for such suggestions. Though, unfortunately there's only one issue in that category right now.

In light of that, you can search for existing issues that appeal to you and for which you feel qualified to tackle. Then ask on the issue if a PR would be welcome.