Skip to content
New issue

Have a question about this project? Sign up for a free GitHub account to open an issue and contact its maintainers and the community.

By clicking “Sign up for GitHub”, you agree to our terms of service and privacy statement. We’ll occasionally send you account related emails.

Already on GitHub? Sign in to your account

NonNull generating incorrect null check in constructor #1018

Closed
linuskamb opened this issue Feb 9, 2016 · 2 comments
Closed

NonNull generating incorrect null check in constructor #1018

linuskamb opened this issue Feb 9, 2016 · 2 comments

Comments

@linuskamb
Copy link

I'm quite new to Lombok, so forgive me if I'm doing something wrong.

It seems the emitted code is checking the value of the object field before setting the field with the value of the parameter.

Notice the difference between the constructors' parameter names versus the setter parameter name. In the setter, the parameter is named _id, shadowing the field, whereas in the constructor, it is named id without the underscore, yet the null check in both cases checks _id.

my lombok config includes:
lombok.accessors.prefix+=_

I have a test case:

@DaTa
@requiredargsconstructor
@AllArgsConstructor
public class TestBok {

@NonNull
private Integer _id;
private String _foo;    
public static void main(String[] args) {
    try {
        Integer id = new Integer(42);
        String foo = "bar";
        TestBok b = new TestBok(id, foo);
        System.out.println(b);
    } catch (Exception e) {
        e.printStackTrace();
    }
}

}

Output is:
java.lang.NullPointerException: _id
at gov.noaa.pmel.tsunami.model.TestBok.(TestBok.java:10)
at gov.noaa.pmel.tsunami.model.TestBok.main(TestBok.java:23)

Delomboked (partial) code:

import lombok.NonNull;

public class TestBok {
@nonnull
private Integer _id;
private String _foo;

public static void main(String[] args) {
    try {
        Integer id = new Integer(42);
        String foo = "bar";
        TestBok b = new TestBok(id, foo);
        System.out.println(b);
    } catch (Exception e) {
        e.printStackTrace();
    }
}

[...]
@java.beans.ConstructorProperties({"id"})
@java.lang.SuppressWarnings("all")
@javax.annotation.Generated("lombok")
public TestBok(@nonnull final Integer id) {
if (_id == null) {
throw new java.lang.NullPointerException("_id");
}
this._id = id;
}

@java.beans.ConstructorProperties({"id", "foo"})
@java.lang.SuppressWarnings("all")
@javax.annotation.Generated("lombok")
public TestBok(@NonNull final Integer id, final String foo) {
    if (_id == null) {
        throw new java.lang.NullPointerException("_id");
    }
    this._id = id;
    this._foo = foo;
}
@java.lang.SuppressWarnings("all")
@javax.annotation.Generated("lombok")
public void setId(@NonNull final Integer _id) {
    if (_id == null) {
        throw new java.lang.NullPointerException("_id");
    }
    this._id = _id;
}

}

@lfielke
Copy link

lfielke commented Feb 18, 2016

I'm experiencing this issue as well. It's also possibly related to #869.

I've found a small test case to trigger the problem; a single class Issue.java:

import lombok.*;
import lombok.experimental.Accessors;

@Accessors(prefix = "m")
@AllArgsConstructor
public class Issue {
    @NonNull
    String mName;
}

Compiled with lombok snapshot (from today) with command:
javac -cp lombok-edge.jar Issue.java

Disassembling the class file, I can see the constructor null checks the field, instead of the parameter:

public class Issue {
    @NonNull
    String mName;

    @ConstructorProperties({"name"})
    public Issue(@NonNull String var1) {
        if(this.mName == null) {
            throw new NullPointerException("mName");
        } else {
            this.mName = var1;
        }
    }
}

Removing either of the @Accessors or @NonNull annotations changes the constructor to correctly null check the param.

@tdoshea90
Copy link

What is the status of this bug

kchirls added a commit to kchirls/lombok that referenced this issue Feb 1, 2017
Generates the null check on the constructor parameter instead of the
instance field. Fix for issues projectlombok#869 and projectlombok#1018.
kchirls added a commit to kchirls/lombok that referenced this issue Mar 15, 2017
…mbok#869 and projectlombok#1018.

Added name to AUTHORS file. Added ECJ fix and test. Made one of the
variables final to also verify the final error case.
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
None yet
Projects
None yet
Development

No branches or pull requests

3 participants