Comments (2)
Hi Frank!
You brought up many of the same issues in #977. Some things have changed since then, but some haven't. One thing I notice that hasn't changed is the Eclipse bug where it synthesizes a @NonNull
annotation on every unannotated type in the scope of @NonNullByDefault
, in flagrant violation of the spec. You logged an Eclipse bug about this in March 2021 but there has been no activity on it.
One thing that has changed is that we attempt to discover if there is a @Nullable
type-use annotation in the code, by looking at parameter and return types of methods in the @AutoValue
class, or by consulting a compiler option. If so then we will apply that @Nullable
to the parameter of the generated equals
method and to the type of each builder field that is not a primitive. For Eclipse, if your class doesn't already have @Nullable
in its methods, you could either compile with -Acom.google.auto.value.NullableTypeAnnotation=org.eclipse.jdt.annotation.Nullable
or you could add a dummy equals
method like this to your AutoValue class:
@Override public abstract boolean equals(@Nullable Object o);
Then AutoValue will discover and use that @Nullable
.
We could indeed copy every property field into a local variable, or perhaps just every field for a property that is neither primitive nor @Nullable
. I'll note as an aside that there are two approaches to null checking here, which we might call optimistic and pessimistic. The optimistic approach assumes that if a method has observed a field being non-null then it will remain that way for the duration of the method. The pessimistic approach assumes that even if a field was observed to be non-null, some other thread could have come along and set it back to null in the meantime. Eclipse is pessimistic, which here is counterproductive because the non-null value can't in fact be set back to null: the setFoo
methods all have null guards.
Doing this would close off another path, of tracking all non-@Nullable
properties with bitmasks, and then using a single bitmask check to ensure they are all set. We already do this for primitive properties, and we do it in AutoBuilder for all properties when building Kotlin classes with default constructor properties.
The other main issue concerns @NonNull
. It is exacerbated by the Eclipse bug, but it would still be present anyway: users might annotate their @AutoValue
properties with @NonNull
, and then that annotation would be copied to the builder fields, yielding private @Nullable @NonNull bar;
. We could imagine a special check that would remove the @NonNull
here.
I believe you're also saying that the Eclipse compiler issues a warning if you use @NonNull
inside @NonNullByDefault
. So we get that warning on the generated setBar
methods, because the cited Eclipse bug makes it look as if the annotation was there in the source, leading AutoValue to copy it. I don't expect we will try to compensate for this bug.
So, to summarize:
- We probably should subtract
@NonNull
from the type of builder property fields. - We might introduce local variables so pessimistic nullness analysis can see that the constructor invocation is null-safe.
- We won't go looking for Eclipse's
@NonNullByDefault
to work around the buggy behaviour it induces.
from auto.
(@eamonnmcmanus , I'm giving this a priority to satisfy our triage process, but please adjust as appropriate.)
from auto.
Related Issues (20)
- AutoService should verify presence of a no-argument public constructor HOT 8
- ServiceLoader$LazyIterator.nextService NullPointerException HOT 2
- noSuchElementException HOT 3
- AutoService breaks with Immutables HOT 8
- Shaded guava triggers CVE checks HOT 1
- AutoValue functional tests not being run by Maven
- Intermittent `Stream closed` exception after upgrading to 1.10.2 HOT 16
- auto-value 1.10.2 breaks compatibility with Kotlin 1.6 HOT 8
- AutoBuilder will not build Kotlin data class with Duration HOT 3
- Require Java 11+ for compilation HOT 1
- Use Jakarta's @Generated annotation if available HOT 2
- AutoValue `toBuilder()` works poorly if an extension subclasses `Builder`; possible INCOMPATIBLE change HOT 1
- Android does not use AutoService. See the example code HOT 2
- Is cross-process support available? HOT 3
- Release AutoFactory 1.1.0 HOT 1
- Remove `o == this` check from generated `equals`? HOT 2
- Possible documentation issue around collection builders HOT 1
- google/auto
- Annotation Processor output is not deterministic HOT 3
Recommend Projects
-
React
A declarative, efficient, and flexible JavaScript library for building user interfaces.
-
Vue.js
🖖 Vue.js is a progressive, incrementally-adoptable JavaScript framework for building UI on the web.
-
Typescript
TypeScript is a superset of JavaScript that compiles to clean JavaScript output.
-
TensorFlow
An Open Source Machine Learning Framework for Everyone
-
Django
The Web framework for perfectionists with deadlines.
-
Laravel
A PHP framework for web artisans
-
D3
Bring data to life with SVG, Canvas and HTML. 📊📈🎉
-
Recommend Topics
-
javascript
JavaScript (JS) is a lightweight interpreted programming language with first-class functions.
-
web
Some thing interesting about web. New door for the world.
-
server
A server is a program made to process requests and deliver data to clients.
-
Machine learning
Machine learning is a way of modeling and interpreting data that allows a piece of software to respond intelligently.
-
Visualization
Some thing interesting about visualization, use data art
-
Game
Some thing interesting about game, make everyone happy.
Recommend Org
-
Facebook
We are working to build community through open source technology. NB: members must have two-factor auth.
-
Microsoft
Open source projects and samples from Microsoft.
-
Google
Google ❤️ Open Source for everyone.
-
Alibaba
Alibaba Open Source for everyone
-
D3
Data-Driven Documents codes.
-
Tencent
China tencent open source team.
from auto.