Jetbrains annotations, warnings, and more visual tools for modders.

[LambdAurora]

Ok, so randomly while porting to 1.19.1-rc3 I may have touched a function and was like "what about @Contract". I don't know how that thought came but it sparked a discussion we all should have.

Static analysis

Static analysis is... cool! Honestly it's very nice and it's becoming something more and more important in development spaces (to the point of having languages doing as much at compile time).

For those who don't know, static analysis is stuff that runs at compile-time, or in this case is directly visible through warnings in Intellij IDEs. It's.. quite important tools.

QSL and static analysis

So far we've been using Jetbrains annotations with @ApiStatus and @Nullable, and some others.

But that's not the only annotations:

• there's @NotNull that we haven't used since we consider stuff to be non null by default.
  But Kotlin modders claim that it would make Kotlin mods safer as lot of compile-time checks could then be executed.
• there's @Contract which allows to:
  • mark a function as pure (no side effects)
  • mutates a parameter (experimental so might not stay in future versions of the library)
  • assign a contract clause to a function, which can be very rich like null, null -> null; _, null -> param1; null, _ -> param2; !null, !null -> new.

Said contract clause can generate warnings like:

Limitations

Most of those annotations are, sadly, only useful for Intellij IDEA users.

It does not affect Gradle, Eclipse does not recognize them, nor VSCode.

The Question

All of that is good and all, but so far those are not really used.

One of the main issue is: annotation hell Using more these might make code harder to read or annoying to write, lots and lots of annotations due to Java's lack of native static analysis tools.

So the question is:

• do the benefits outweights the con?
• are tools like this what modders want?
• should we start using those?

[Leo40Git]

I am all for the IDE yelling at me for being dumb /joke-but-not-really

IMO, we should start using these.

[Gaming32]

I agree. It would be super useful, especially with the nullness annotations, since those are used directly by Kotlin (even if you don't use IDEA for Kotlin).

[Jamalam360]

It would definitely be useful imo

[pluiedev]

Kotlin user here, we should definitely start using them :)

[0xJoeMama]

I've had this conversation in the past with people from the kotlin team. However, I was told it doesn't cause that big an issue since native types exist.

[SilverAndro]

To clarify the specific behavior of kotlin and java types, I recommend the kotlin docs on platform types https://kotlinlang.org/docs/java-interop.html#null-safety-and-platform-types

When you call methods on variables of platform types, Kotlin does not issue nullability errors at compile time, but the call may fail at runtime, because of a null-pointer exception or an assertion that Kotlin generates to prevent nulls from propagating

Essentially, if you know that QSL always denotes nullable types, kotlin users can make the same assumptions as java users

[supersaiyansubtlety]

I also like my IDE to yell when I'm dumb.

The only thing I'm not sure is worth it is @Contract. It looks like it would take the most work to maintain and also adds the most clutter to source code.

[LambdAurora]

The only thing I'm not sure is worth it is @Contract. It looks like it would take the most work to maintain and also adds the most clutter to source code.

One thing I realized later is that @Contract doesn't just yell at modders, it also yells at QSL implementers!

If you define a method contract to be _ -> null but returns something not null, it'll warn inside the implementation saying that the implementation is wrong. WHICH IS SO GOOD.

Another thing, @Contract allows to say if a method is pure, so if you introduce side effects it'll be able to warn you.

[supersaiyansubtlety]

I mean, all sounds good if devs are ok with maintaining it.

[Gaming32]

VS Code and Eclipse, as I recently learned, can actually recognize @Nullable and @NotNull if you configure them to.

[kb-1000]

It may also make sense to use @Mutable and @ReadOnly from the kotlin-annotations-jvm module, to mark collection arguments and return values as mutable or read-only, respectively. Just like with @NotNull, this not only improves API surface from Kotlin, but also serves a documentation purpose for everyone else. (This module is written in Java and doesn't require any other parts of Kotlin.)

[LambdAurora]

Worth noting that Jetbrains annotations already contain @Unmodifiable and @UnmodifiableView. Which seem more precise than @ReadOnly.

[DanielGolan-mc]

I also like my IDE to yell when I'm dumb.

The only thing I'm not sure is worth it is @Contract. It looks like it would take the most work to maintain and also adds the most clutter to source code.

I disagree here, for a few reasons:

1. @Contract, after learning to use it, is very readable. And I'm quite sure IDEA is able to collapse them.
2. It helps to set expectations from the function - not to change reference objects (will be way more useful when Java's custom primitives will release), to return X and Y. It shortens the length of javadocs by a lot, as users can expect IDEA to answer certain questions, so a longer javadoc isn't needed.

I have 2 more points, relating to IDEs and annotation hell: 3. I would argue that IDEA is the recommended IDE for Minecraft mod development, especially Quilt (if not java as a whole), and users that don't use the recommended software, have to deal with the negatives. 4. JetBrains annotations are supported on two IDEs - Intelij IDEA, and JetBrains fleet (that has a more VSCode-like usage style for those who prefer it), so modders aren't locked into a specific IDE.

I know I'm very late to the discussion, but whatever, I thought the date was 4d, not 4m.

[kb-1000]

Worth noting that Jetbrains annotations already contain @Unmodifiable and @UnmodifiableView. Which seem more precise than @ReadOnly.

But unfortunately those don't show up in Kotlin, as far as I can tell.

[lukebemish]

I'm all for using more annotation documentation - especially Contract. It is, at worst, machine readable documentation that some IDEs won't be able to read. At best, it's an extremely powerful tool to ensure stuff works as intended. And what's more, it's useful even when IDEs don't recognize it - modders or qsl contributors using stuff besides IntelliJ IDEA can still read the annotations

[LambdAurora]

As I'm seeing all of this more and more used in recent PRs and new QSL APIs, I think it's fair to say we have adopted the usage of such annotations and we will continue for the foreseeable future.

I personally haven't noticed any huge readability issues so far and have benefited from the static analysis.

Closing this as I believe we've reached a conclusion. Feel free to reopen if we ever need to revisit the idea in the future.