perwendel / spark

A simple expressive web framework for java. Spark has a kotlin DSL https://github.com/perwendel/spark-kotlin
Apache License 2.0
9.64k stars 1.56k forks source link

Replaced explicit null-checks by a declarative approach #951

Closed denis-zhdanov closed 5 years ago

denis-zhdanov commented 6 years ago

The idea is to do the following:

  1. Remove explicit null-checks
  2. Mark corresponding method parameters by Nonnull annotation
  3. Configure the project to use Traute javac plugin to enhance resulting bytecode by null-checks for the target method parameters

Example: consider ClassPathResource:

Before

public ClassPathResource(String path, ClassLoader classLoader) {
    Assert.notNull(path, "Path must not be null");
    String pathToUse = StringUtils.cleanPath(path);
    ....

After

public ClassPathResource(@Nonnull String path, ClassLoader classLoader) {
    String pathToUse = StringUtils.cleanPath(path);
    ....

Resulting bytecode:

javap -c ./target/classes/spark/resource/ClassPathResource.class
...
  public spark.resource.ClassPathResource(java.lang.String, java.lang.ClassLoader);
    Code:
       0: aload_0
       1: invokespecial #2                  // Method spark/resource/AbstractFileResolvingResource."<init>":()V
       4: aload_1
       5: ifnonnull     18
       8: new           #3                  // class java/lang/IllegalArgumentException
      11: dup
      12: ldc           #4                  // String 'path' must not be null
      14: invokespecial #5                  // Method java/lang/IllegalArgumentException."<init>":(Ljava/lang/String;)V
      17: athrow

Benefits:

TESTED: mvn clean package & ensured that resulting bytecode contains null-checks

tipsy commented 6 years ago

We meet again @denis-zhdanov. I'm not sure we want to introduce this in 2.X. @perwendel can decide.

denis-zhdanov commented 6 years ago

Hi again :)

Any particular concern about that?

tipsy commented 6 years ago

Not really any concerns, but the current version is supposed to be the last 2.X, with the exception of bugfixes.

denis-zhdanov commented 6 years ago

Ah, fair enough. I don't think it's necessary to make a dedicated release for that, i.e. it's enough to apply the feature and then release when there is a significant feature/bugfix to deliver to end-users.

denis-zhdanov commented 6 years ago

Hey @perwendel

Could you please share your thoughts on the idea of adding Traute to your project?

Regards, Denis

perwendel commented 5 years ago

Thanks! Currently there are much higher prioritized bugs/features that needs attention. Adding another lib for null checks does not feel important, even though I like how it seems to work.