doc: clarifies contracts of @DerivedProperty #1613
Conversation
|
I do not understand purpose of this change. But I do not need to know everything. I just cannot review this PR. I personaly would keep this annotation in the model. It had a meening |
| @@ -175,6 +176,8 @@ public void replace(CtStatement element) { | |||
| if (type != null) { | |||
| type.setParent(this); | |||
| } | |||
| // even if the setter delegates to getExecutable, it makes sense to create an event | |||
| getFactory().getEnvironment().getModelChangeListener().onObjectUpdate(this, TYPE, type, getExecutable().getType()); | |||
There was a problem hiding this comment.
So model change listener will be called twice for single modification? I do not like it.
@monperrus Could you explain why it is good?
There was a problem hiding this comment.
you're right, it's not good.
monperrus
force-pushed
the
noDerivedProperty
branch
from
094842c to
6b80f04
Compare
|
The most convincing reason to keep @DerivedProperty is in ASTChecker with this contract. Contract: a setter with @DerivedProperty never triggers a model intercession event. So let's make this PR only a clarification one. |
monperrus
changed the title
| * For instance {@link CtType#getFields()} is derived from {@link CtType#getTypeMembers()} | ||
| * | ||
| * This annotation is used for specifying CtScanner: derived properties are never scanned. | ||
| * It can be put on getter ond setters. |
| * It can be put on getter ond setters. | ||
| * | ||
| * Contracts: | ||
| * - A setter with @DerivedProperty never triggers a model intercession event. |
There was a problem hiding this comment.
intercession event
Do you mean something like this?
class CtConstructorCallImpl
...
@DerivedProperty
public <T extends CtActualTypeContainer> T addActualTypeArgument(CtTypeReference<?> actualTypeArgument) {
if (getExecutable() != null) {
getExecutable().addActualTypeArgument(actualTypeArgument);
}
return (T) this;
}It triggers the event, but not directly on the this node, but on some delegate? Did I understood it well? Improve description?
Is there a test for setters which does not start with set prefix, but with add/remove?
|
clarified contract. |
per our discussion on #1586, PropertyGetter can be used for @DerivedProperty as well