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

Getter construction defective with small boolean lazy field #449

Closed
lombokissues opened this Issue Jul 14, 2015 · 8 comments

Comments

Projects
None yet
1 participant
@lombokissues
Collaborator

lombokissues commented Jul 14, 2015

Migrated from Google Code (issue 376)

@lombokissues

This comment has been minimized.

Show comment
Hide comment
@lombokissues

lombokissues Jul 14, 2015

Collaborator

👤 k.tinnefeld   🕗 May 24, 2012 at 11:14 UTC

What steps will reproduce the problem?

  1. Create test class in eclipse

import lombok.Data;
import lombok.Getter;

@ Data
public class BooleanLazyDefective {

@ Getter(lazy = true)
private final boolean flag = expensive();

private boolean expensive() {
    return true;
}

}

  1. enjoy seven errors "The method getFlag() is undefined for the type BooleanLazyDefective"

What is the expected output? What do you see instead?

The lombok computed getter "isFlag" is being used in the places mentioned. Tries to read "getFlag" instead.

Using delombok, the error is the other way round: a lazy getter "getFlag" is generated, but "isFlag" is accessed in seven locations.

The single part annotations @ ToString etc. behave exactly the same, restricted to their resp. domain.

What version of the product are you using? On what operating system?

Lombok 0.11.0 on Eclipse 3.7.2, Win 7 64 Bit, Java 6 u 30.

Please provide any additional information below.

Collaborator

lombokissues commented Jul 14, 2015

👤 k.tinnefeld   🕗 May 24, 2012 at 11:14 UTC

What steps will reproduce the problem?

  1. Create test class in eclipse

import lombok.Data;
import lombok.Getter;

@ Data
public class BooleanLazyDefective {

@ Getter(lazy = true)
private final boolean flag = expensive();

private boolean expensive() {
    return true;
}

}

  1. enjoy seven errors "The method getFlag() is undefined for the type BooleanLazyDefective"

What is the expected output? What do you see instead?

The lombok computed getter "isFlag" is being used in the places mentioned. Tries to read "getFlag" instead.

Using delombok, the error is the other way round: a lazy getter "getFlag" is generated, but "isFlag" is accessed in seven locations.

The single part annotations @ ToString etc. behave exactly the same, restricted to their resp. domain.

What version of the product are you using? On what operating system?

Lombok 0.11.0 on Eclipse 3.7.2, Win 7 64 Bit, Java 6 u 30.

Please provide any additional information below.

@lombokissues

This comment has been minimized.

Show comment
Hide comment
@lombokissues

lombokissues Jul 14, 2015

Collaborator

👤 k.tinnefeld   🕗 Jun 12, 2012 at 19:55 UTC

The javac/delombok issue is quite easily fixed by memorizing the original getter name.

cf. https://bitbucket.org/tinne/lombok-fixes/changeset/e67fab1703c7

Collaborator

lombokissues commented Jul 14, 2015

👤 k.tinnefeld   🕗 Jun 12, 2012 at 19:55 UTC

The javac/delombok issue is quite easily fixed by memorizing the original getter name.

cf. https://bitbucket.org/tinne/lombok-fixes/changeset/e67fab1703c7

@lombokissues

This comment has been minimized.

Show comment
Hide comment
@lombokissues

lombokissues Jul 14, 2015

Collaborator

👤 k.tinnefeld   🕗 Jun 12, 2012 at 21:19 UTC

The eclipse issue is harder as it needs to check isProperty getters not only on boolean, but also on AtomicReference<AtomicReference> fields, and those must not refer to big Boolean properties.

This is solved by https://bitbucket.org/tinne/lombok-fixes/changeset/47a17156675a

Test file:

import java.util.concurrent.atomic.AtomicReference;
import lombok.Data;
import lombok.Getter;

@ Data
public class BoolTest {

@ Getter(lazy = true)
private final boolean flag = expensive();

@ Getter(lazy = true)
private final Boolean falsePositive = ! expensive(); 

public AtomicReference<AtomicReference<Boolean>> isFalsePositive() { return null; }  

private boolean expensive() {
    return true;
}

}

Collaborator

lombokissues commented Jul 14, 2015

👤 k.tinnefeld   🕗 Jun 12, 2012 at 21:19 UTC

The eclipse issue is harder as it needs to check isProperty getters not only on boolean, but also on AtomicReference<AtomicReference> fields, and those must not refer to big Boolean properties.

This is solved by https://bitbucket.org/tinne/lombok-fixes/changeset/47a17156675a

Test file:

import java.util.concurrent.atomic.AtomicReference;
import lombok.Data;
import lombok.Getter;

@ Data
public class BoolTest {

@ Getter(lazy = true)
private final boolean flag = expensive();

@ Getter(lazy = true)
private final Boolean falsePositive = ! expensive(); 

public AtomicReference<AtomicReference<Boolean>> isFalsePositive() { return null; }  

private boolean expensive() {
    return true;
}

}

@lombokissues

This comment has been minimized.

Show comment
Hide comment
@lombokissues

lombokissues Jul 14, 2015

Collaborator

👤 r.spilker   🕗 Jun 18, 2012 at 18:14 UTC

Hi K,

Thanks for taking the time to investigate this problem. I didn't look at your patch. We can (regrettable) legally only allow patches via a pull request on github. If you do that you can also add your name to the AUTHORS file so you get the credits and we are legally safe.

After you submit a pull request I can look into the code, potentially give feedback and/or merge it.

I can confirm that javac/delombok generates getXxx instead of isXxx but calls isXxx in equals, hashCode and toString. On the other hand, in Eclipse the right method name is generated, but the wrong one is called in equals, hashCode and toString, even leading to the generation of intermediate local variables of type Object.

Collaborator

lombokissues commented Jul 14, 2015

👤 r.spilker   🕗 Jun 18, 2012 at 18:14 UTC

Hi K,

Thanks for taking the time to investigate this problem. I didn't look at your patch. We can (regrettable) legally only allow patches via a pull request on github. If you do that you can also add your name to the AUTHORS file so you get the credits and we are legally safe.

After you submit a pull request I can look into the code, potentially give feedback and/or merge it.

I can confirm that javac/delombok generates getXxx instead of isXxx but calls isXxx in equals, hashCode and toString. On the other hand, in Eclipse the right method name is generated, but the wrong one is called in equals, hashCode and toString, even leading to the generation of intermediate local variables of type Object.

@lombokissues

This comment has been minimized.

Show comment
Hide comment
@lombokissues

lombokissues Jul 14, 2015

Collaborator

👤 r.spilker   🕗 Jun 18, 2012 at 18:58 UTC

The fix for javac is to move the call to determine the name to before generating the method body, in which the type is changed.

Collaborator

lombokissues commented Jul 14, 2015

👤 r.spilker   🕗 Jun 18, 2012 at 18:58 UTC

The fix for javac is to move the call to determine the name to before generating the method body, in which the type is changed.

@lombokissues

This comment has been minimized.

Show comment
Hide comment
@lombokissues

lombokissues Jul 14, 2015

Collaborator

👤 k.tinnefeld   🕗 Jun 18, 2012 at 20:48 UTC

On github, this is now https://github.com/tinne/lombok/commit/e67fab1703c76cfceb089ac7c9e14090a2ddc6f6

Collaborator

lombokissues commented Jul 14, 2015

👤 k.tinnefeld   🕗 Jun 18, 2012 at 20:48 UTC

On github, this is now https://github.com/tinne/lombok/commit/e67fab1703c76cfceb089ac7c9e14090a2ddc6f6

@lombokissues

This comment has been minimized.

Show comment
Hide comment
@lombokissues

lombokissues Jul 14, 2015

Collaborator

👤 r.spilker   🕗 Jun 18, 2012 at 21:45 UTC

Fixed in 37cbe2d

Collaborator

lombokissues commented Jul 14, 2015

👤 r.spilker   🕗 Jun 18, 2012 at 21:45 UTC

Fixed in 37cbe2d

@lombokissues

This comment has been minimized.

Show comment
Hide comment
@lombokissues

lombokissues Jul 14, 2015

Collaborator

End of migration

Collaborator

lombokissues commented Jul 14, 2015

End of migration

Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment